Problem/Motivation

Phpstan reports 3 undefined variables.
For me it is unclear what these intend to do.


 ------ ------------------------------------------- 
  Line   l10n_community/src/Form/TranslateForm.php  
 ------ ------------------------------------------- 
  882    Undefined variable: $user                  
         🪪  variable.undefined                     
 ------ ------------------------------------------- 

 ------ ------------------------------------ 
  Line   l10n_packager/src/L10nPackager.php  
 ------ ------------------------------------ 
  297    Undefined variable: $extra          
         🪪  variable.undefined              
 ------ ------------------------------------ 

 ------ --------------------------------------------- 
  Line   l10n_remote/src/TranslationContribution.php  
 ------ --------------------------------------------- 
  67     Undefined variable: $data                    
         🪪  variable.undefined                       
 ------ ---------------------------------------------

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork l10n_server-3563020

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

donquixote created an issue. See original summary.

donquixote’s picture

donquixote commented

First one introduced in

commit a93acb125c9072eeec4bb2a1bcce2b0eefc8a5cc
Author: Felip Manyer i Ballester <git @ res-telae.cat>
Date:   Fri Nov 11 17:18:19 2022 +0100

    Issue #3282696: Port of l10n_community: translation form (submit)

Second one introduced in 

commit 8a11aff3155bce7c21ed7f7a9aad751478ab1c92
Author: sanduhrs <stefan @ auditor.email>
Date:   Thu Jun 23 08:45:36 2022 +0200

    initial draft of l10n packager

Third one introduced in 

commit a953d29ecdb1be75c6db2355dd1ee0ec3e0cc25a
Author: Felip Manyer i Ballester <git @ res-telae.cat>
Date:   Fri Aug 23 10:42:52 2024 +0200

    Replaced "Contributon" by "Contribution"
garvitasakhrani’s picture

garvitasakhrani commented
Assigned: Unassigned » garvitasakhrani

Working on it.

garvitasakhrani opened merge request !104

garvitasakhrani’s picture

garvitasakhrani commented
Assigned: garvitasakhrani » Unassigned
Status: Active » Needs review

I have fixed the undefined variable warnings and removed the $extra variable, as it was not used anywhere in the code. Please review the merge request and let me know if any updates are required.

fmb’s picture

fmb commented

@garvitasakhrani I agree with changes in TranslateForm and TranslationContribution. As for L10nPackager, the $extra variable was already undefined in the Drupal 7 version, so indeed it does look like it should be removed, but nevertheless I am going to ask on Slack if we are missing something here.

donquixote’s picture

donquixote commented

I identified the same changes when i looked deeper in the phpstan issue.

The '%extra' part in a url no longer works in that function since a05f31ecb70322cde2b from D7, even though other places with $extra still exist in that commit (but don't do anything useful).
https://git.drupalcode.org/project/l10n_server/-/commit/a05f31ecb70322cd...

donquixote’s picture

donquixote commented

All 3 fixes make sense to me.

fmb’s picture

fmb commented
Status: Needs review » Needs work

Indeed, see this discussion on Slack about $extra, which is the most sensitive part here. As Gábor said, we need to make sure that files get properly generated for RC, beta, etc. releases with the proper name, so at least test manually, ideally write some automated test if it makes sense here.

garvitasakhrani’s picture

garvitasakhrani commented
Status: Needs work » Needs review

I’ve updated the merge request to keep the %extra placeholder while fixing the undefined variable, as suggested.
Please review and let me know if any additional updates are needed.

fmb’s picture

fmb commented
Status: Needs review » Fixed

Thanks. I opened #3563407: Test files generated by packager as a follow-up for tests.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.