Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Added a description line to the import spreadsheet template #9920

Merged
merged 7 commits into from
Mar 15, 2024

Conversation

anaritadauane
Copy link
Contributor

@anaritadauane anaritadauane commented Mar 14, 2024

What

I added a description row to the import spreadsheet template, which now includes all the comments previously located in the header line.

Screenshot

Screenshot 2024-03-15 at 01 36 53

Related issue(s) and discussion

@anaritadauane anaritadauane requested a review from a team as a code owner March 14, 2024 15:30
@anaritadauane anaritadauane changed the title Fix issue #9879 : Added a description line to the import spreadsheet template fix: Added a description line to the import spreadsheet template Mar 14, 2024
@stephanegigandet
Copy link
Contributor

@anaritadauane Thank you! For some reason, I get a completely empty spreadsheet when I run your branch, that's weird.

@anaritadauane
Copy link
Contributor Author

@anaritadauane Thank you! For some reason, I get a completely empty spreadsheet when I run your branch, that's weird.

Really? Let me check

@stephanegigandet
Copy link
Contributor

This is what I get in the logs:

stephane@stephane-jaguar-NS50-70MU:~/openfoodfacts-server/logs/apache2$ tail -f modperl_error_log 
[..]
[Thu Mar 14 15:58:22.438705 2024] [:error] [pid 22] [Thu Mar 14 15:58:22 2024] -e: Can't locate object method "add_format" via package "workbook" (perhaps you forgot to load "workbook"?) at /opt/product-opener/cgi/generate_sample_import_file.pl line 57.\n

There's a missing $

@stephanegigandet
Copy link
Contributor

To see errors, you can do ~/openfoodfacts-server/logs/apache2$ tail -f modperl_error_log
after adding the missing $, there's another error

@anaritadauane
Copy link
Contributor Author

anaritadauane commented Mar 14, 2024

To see errors, you can do ~/openfoodfacts-server/logs/apache2$ tail -f modperl_error_log after adding the missing $, there's another error

Thanks, I had to fork the repo to pull the request, then copy the changes and add some changes to the unforked repo to make a PR. I think I needed to go back and test the code to see if there were any errors.
Going to fix the code

@codecov-commenter
Copy link

codecov-commenter commented Mar 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.52%. Comparing base (dc04d18) to head (ae2dd05).
Report is 115 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9920      +/-   ##
==========================================
- Coverage   49.54%   49.52%   -0.03%     
==========================================
  Files          67       71       +4     
  Lines       20650    20897     +247     
  Branches     4980     5018      +38     
==========================================
+ Hits        10231    10349     +118     
- Misses       9131     9254     +123     
- Partials     1288     1294       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@anaritadauane
Copy link
Contributor Author

I have fixed the errors

@stephanegigandet
Copy link
Contributor

@anaritadauane It works now, thank you!

This is what I'm getting when I open the file with LibreOffice Calc, is there a way to get more height in Description cells?

image

my $comment;

$worksheet->set_column('A:A', 30);
$worksheet->write( $description_row, 0, "Description", $formats{'description'});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make "Description" a translatable string?

To do that, you need to put lang("description") instead, and then add a stringid "description" in po/common.po and po/en.po (and it would be good to add it in another language so that you can test that you get a different string if you load for instance the site in Spanish: http://es.openfoodfacts.localhost/cgi/generate_sample_import_file.pl

Once you have added / changed a string in po/common.pot / en.po / es.po , you need to run:
make build_lang
make restart

and then you can use lang("description") in the Perl file instead of "Description".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @stephanegigandet,

Yes, I can. But I don't understand if I should add it as stringid or as msid, like in the po/common.po file?

Screenshot 2024-03-15 at 15 10 12

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followed the instructions and turned "Description" into a translatable string

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anaritadauane Great :)

Can you also add this to en.po so that we have an English translation (English is the default):

msgctxt "description"
msgid "Description"
msgstr "Description"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@stephanegigandet
Copy link
Contributor

@anaritadauane looks good, thank you! just one request to make the "Description" text translatable

@anaritadauane
Copy link
Contributor Author

@anaritadauane looks good, thank you! just one request to make the "Description" text translatable

Great! I will add the additional requests.

po/common/es.po Outdated
@@ -489,6 +489,10 @@ msgctxt "delete_confirmation"
msgid "This will delete your user details and anonymise all of your contributions. Please re-enter your user name to confirm."
msgstr "Esto eliminará sus datos de usuario y anonimizará todas sus contribuciones. Vuelva a ingresar su nombre de usuario para confirmar."

msgctxt "description"
msgid "Descripton"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small typo, msgid should always be the same as the English translation

Suggested change
msgid "Descripton"
msgid "Description"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry 😅, fixed the typo and changed the en.po file

@stephanegigandet
Copy link
Contributor

@anaritadauane Could you run "make lint" to fix the formatting? Thank you!

Copy link

sonarcloud bot commented Mar 15, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@anaritadauane
Copy link
Contributor Author

@anaritadauane Could you run "make lint" to fix the formatting? Thank you!

Yes

Copy link
Contributor

@stephanegigandet stephanegigandet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you! :)

@stephanegigandet stephanegigandet merged commit 3138a44 into openfoodfacts:main Mar 15, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a description line to the import spreadsheet template on the producers platform
3 participants