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: 4165 - new cases of default language for OCR #4227

Merged
merged 1 commit into from
Jun 24, 2023

Conversation

monsieurtanuki
Copy link
Contributor

What

  • There were some side effect cases where we could not compute the language (e.g. ingredient language) from the product data.
  • It meant "no default language" and "no language selector" for OCR page - we could not OCR a Japanese image in an app in French.
  • A simple fix inspired by Impossible to extract ingredients on this product #4165 was to say: "if we have an ingredient image in a specific language, that language may be used in the OCR page". In Impossible to extract ingredients on this product #4165 the bug is fixed - we land on the page with a Japanese image, therefore Japanese may be used as a default language.
  • Extending that principle, now in "new product" pages we have a language selector that uses by default the app language - and the user can change it.

Screenshot

OCR landing page OCR after OCR new product basic details with language
Screenshot_2023-06-24-17-23-45 Screenshot_2023-06-24-17-23-53 Screenshot_2023-06-24-17-39-24

Fixes bug(s)

Impacted files

  • add_basic_details_page.dart: added parameters for init method.
  • edit_ocr_page.dart: added parameters for init method.
  • multilingual_helper.dart: added parameters for init method, that helps us compute the default language

Impacted files:
* `add_basic_details_page.dart`: added parameters for `init` method.
* `edit_ocr_page.dart`: added parameters for `init` method.
* `multilingual_helper.dart`: added parameters for `init` method, that helps us compute the default language
@codecov-commenter
Copy link

Codecov Report

Merging #4227 (436d54f) into develop (95c3a67) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #4227      +/-   ##
===========================================
- Coverage    11.00%   10.99%   -0.02%     
===========================================
  Files          282      282              
  Lines        14015    14029      +14     
===========================================
  Hits          1543     1543              
- Misses       12472    12486      +14     
Impacted Files Coverage Δ
..._app/lib/pages/product/add_basic_details_page.dart 0.00% <0.00%> (ø)
...es/smooth_app/lib/pages/product/edit_ocr_page.dart 0.00% <0.00%> (ø)
...oth_app/lib/pages/product/multilingual_helper.dart 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@g123k g123k left a comment

Choose a reason for hiding this comment

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

I approve in terms of code, but I let @teolemon the final decision in terms of products.
An even better solution would be to select the closest translation available (e.g.: for your case in French, the German image would be better than the Japanese one)

@monsieurtanuki
Copy link
Contributor Author

Thank you @g123k for your review!

I approve in terms of code, but I let @teolemon the final decision in terms of products.

I spare @teolemon the need to review that PR.
No doubt that if new issues need to be created later, they will be!

An even better solution would be to select the closest translation available (e.g.: for your case in French, the German image would be better than the Japanese one)

In that specific case the problem is that we have NO translations, just images. And just Japanese images, which solves the case.
If you find a real case with possible improvements regarding the choice of the default language, please create a new issue and mention the related barcode(s).

@monsieurtanuki monsieurtanuki merged commit dc5806a into openfoodfacts:develop Jun 24, 2023
6 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.

Impossible to extract ingredients on this product
3 participants