-
-
Notifications
You must be signed in to change notification settings - Fork 400
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: issue #9876 import spreadsheet template with mandatory fields #9934
fix: issue #9876 import spreadsheet template with mandatory fields #9934
Conversation
lib/ProductOpener/Config_off.pm
Outdated
@@ -180,6 +180,7 @@ use ProductOpener::Config2; | |||
stephane | |||
tacinte | |||
teolemon | |||
himanshisrestha |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good for testing, but it should not be included in the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@himanshisrestha, when you test, use stephane or tacinte account to avoid having to change this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexgarel could you please guide me on how to use it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you just go to sign-in -- create account and use the username "stephane"
.env
Outdated
@@ -65,3 +65,4 @@ LOG_LEVEL_ROOT=TRACE | |||
LOG_LEVEL_MONGODB=TRACE | |||
|
|||
BUILD_CACHE_REPO=openfoodfacts/openfoodfacts-build-cache | |||
CPANMOPTS=--with-develop --with-feature=off_server_dev_tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be included in the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@himanshisrestha if you use linux or mac, you might use direnv
Hello @himanshisrestha , thank you for the PR! I made some comments. In addition, there are some code that was changed that we want to keep:
This is needed in order to have "Description" translated in other languages. Could you change the PR so that it contains only the one change for keeping only the mandatory fields? Thanks! |
Also you can run "make lint" to automatically format the code. |
Yes, I can. I'm changing all what you mentioned and will update you soon. |
Quality Gate passedIssues Measures |
@@ -64,4 +64,4 @@ ELASTICSEARCH_HOSTS= | |||
LOG_LEVEL_ROOT=TRACE | |||
LOG_LEVEL_MONGODB=TRACE | |||
|
|||
BUILD_CACHE_REPO=openfoodfacts/openfoodfacts-build-cache | |||
BUILD_CACHE_REPO=openfoodfacts/openfoodfacts-build-cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is unrelated, could you remove it from the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the last empty line which was removed. It sometimes depends on the IDE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! (except for the modification in .env file)
Thank you!
Happy to mark my contribution |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9934 +/- ##
==========================================
- Coverage 49.54% 49.52% -0.02%
==========================================
Files 67 71 +4
Lines 20650 20899 +249
Branches 4980 5019 +39
==========================================
+ Hits 10231 10351 +120
- Misses 9131 9254 +123
- Partials 1288 1294 +6 ☔ View full report in Codecov by Sentry. |
What
This solves the issue #9876 import spreadsheet template with mandatory fields only on the producers platform.
In the import template generated by https://world.pro.openfoodfacts.org/cgi/generate_sample_import_file.pl , added a parameter "?fields=mandatory" to output the excel file with just the mandatory fields.
The issue was solved using param() function of the CGI.pm module. Before this, the code was setup to just find the importance of the header cell and then apply styling accordingly to write the cell. Using the param() function, the passed argument "fields=mandatory" was accepted as a name=value pair which made it easier to decide whether to add the the cell or not.
The change can be seen in the image attached, which returns an excel sheet consisting of 28 columns of mandatory fields only.
Screenshot
Related issue(s) and discussion