fix: 3018 - instant upload of images #3329

monsieurtanuki · 2022-11-17T12:29:24Z

New files:

ocr_widget.dart: Widget dedicated to OCR, with 3 actions: upload image, extract data, save. Used to be in edit_ingredients_page.dart
transient_file.dart: Helper class about transient files (= not fully uploaded yet).

Impacted files:

add_basic_details_page.dart: minor refactoring
add_new_product_page.dart: refactored around the class image repository
app_en.arb: added 1 label when OCR fails
app_fr.arb: added 1 label when OCR fails
background_task_details.dart: minor refactoring
background_task_image.dart: immediate execution with just one try
confirm_and_upload_picture.dart: minor refactoring with new private class _OutlinedButton
edit_ingredients_page.dart: moved OcrWidget code to dedicated new dart file; fixed the extract button - that cannot be reached if the image is not uploaded yet
image_crop_page.dart: refactored with more explicit method names
image_upload_card.dart: refactored with TransientFile
new_product_page.dart: minor refactoring
product_cards_helper.dart: minor refactoring
product_image_carousel.dart: minor refactoring
product_image_gallery_view.dart: refactored with new method confirmAndUploadNewPicture and new class TransientFile
product_image_viewer.dart: refactored with new class TransientFile
question_card.dart: minor refactoring
smooth_product_image.dart: refactored with new class TransientFile

What

The whole point of this PR is the use of new class TransientFile.
With this class, we can
- instantaneously fake in the whole app that an image is uploaded, as soon as the user clicks on the "upload" button
- when the upload is actually finished, the app will be refreshed and take into account the "server version" of the image (the "transient version" being deleted locally)
- as with "details", for the moment we work on a "I'm lucky" mode - we try once, and rollback if it fails

Part of

Background + offline incoherences on multiple offline edits #3018

New files: * `ocr_widget.dart`: Widget dedicated to OCR, with 3 actions: upload image, extract data, save. Used to be in `edit_ingredients_page.dart` * `transient_file.dart`: Helper class about transient files (= not fully uploaded yet). Impacted files: * `add_basic_details_page.dart`: minor refactoring * `add_new_product_page.dart`: refactored around the class image repository * `app_en.arb`: added 1 label when OCR fails * `app_fr.arb`: added 1 label when OCR fails * `background_task_details.dart`: minor refactoring * `background_task_image.dart`: immediate execution with just one try * `confirm_and_upload_picture.dart`: minor refactoring with new private class `_OutlinedButton` * `edit_ingredients_page.dart`: moved `OcrWidget` code to dedicated new dart file; fixed the extract button - that cannot be reached if the image is not uploaded yet * `image_crop_page.dart`: refactored with more explicit method names * `image_upload_card.dart`: refactored with `TransientFile` * `new_product_page.dart`: minor refactoring * `product_cards_helper.dart`: minor refactoring * `product_image_carousel.dart`: minor refactoring * `product_image_gallery_view.dart`: refactored with new method `confirmAndUploadNewPicture` and new class `TransientFile` * `product_image_viewer.dart`: refactored with new class `TransientFile` * `question_card.dart`: minor refactoring * `smooth_product_image.dart`: refactored with new class `TransientFile`

codecov-commenter · 2022-11-17T12:35:37Z

Codecov Report

Merging #3329 (73d2307) into develop (b0b52c5) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff            @@
##           develop    #3329   +/-   ##
========================================
  Coverage    10.48%   10.49%           
========================================
  Files          251      253    +2     
  Lines        12322    12313    -9     
========================================
  Hits          1292     1292           
+ Misses       11030    11021    -9

Impacted Files	Coverage Δ
...th_app/lib/background/background_task_details.dart	`0.00% <ø> (ø)`
...ooth_app/lib/background/background_task_image.dart	`0.00% <0.00%> (ø)`
...th_app/lib/cards/data_cards/image_upload_card.dart	`0.00% <0.00%> (ø)`
...ib/cards/product_cards/product_image_carousel.dart	`0.00% <ø> (ø)`
...ckages/smooth_app/lib/database/transient_file.dart	`0.00% <0.00%> (ø)`
.../lib/generic_lib/widgets/smooth_product_image.dart	`0.00% <0.00%> (ø)`
...s/smooth_app/lib/helpers/product_cards_helper.dart	`0.00% <0.00%> (ø)`
...ooth_app/lib/pages/hunger_games/question_card.dart	`0.00% <ø> (ø)`
packages/smooth_app/lib/pages/image_crop_page.dart	`1.38% <0.00%> (-0.26%)`	⬇️
..._app/lib/pages/product/add_basic_details_page.dart	`0.00% <ø> (ø)`
... and 7 more

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

AshAman999

Looks good to me!

I tried to run it locally , my initial impressions were good , it works

monsieurtanuki · 2022-11-17T16:25:08Z

Looks good to me!
I tried to run it locally , my initial impressions were good , it works

Please approve it if relevant.

AshAman999 · 2022-11-17T16:52:24Z

Looks good to me!
I tried to run it locally , my initial impressions were good , it works

Please approve it if relevant.

I just had a rough look, looking into more details, will update you soon
Meanwhile was waiting for some other eyes to be on this as well

M123-dev · 2022-11-17T17:01:51Z

Don't have time today, I'm having a look at it tomorrow

M123-dev

Looks great @monsieurtanuki especially that you thought about the transient image for ocr. Added some minor comments

M123-dev · 2022-11-19T12:51:59Z

packages/smooth_app/lib/helpers/product_cards_helper.dart

+
+ProductImageData getProductImageData(
+  final Product product,
+  final AppLocalizations? appLocalizations,


Why is this nullable

The thing is that ProductImageData does 2 different things: it computes the image provider or url, and it populates the title and button labels.
In some cases we don't use the labels, therefore we don't need the AppLocalizations. I just wanted to avoid calling a AppLocalizations.of(context) just for something I don't care about - or maybe the context was not available there, don't remember.
Could be refactored later, especially now that I created specific methods for each use-case (button label, title label, url, provider).

M123-dev · 2022-11-19T12:54:53Z

packages/smooth_app/lib/pages/product/confirm_and_upload_picture.dart

@@ -215,3 +166,34 @@ class _ConfirmAndUploadPictureState extends State<ConfirmAndUploadPicture> {
    await model.onCreateProduct(barcode); // TODO(monsieurtanuki): a bit fishy
  }
 }
+
+/// Standard button for this page.
+class _OutlinedButton extends StatelessWidget {


I'n not a fan of having different private buttons per page can't we use one we already have

Before in this page the same code was repeated 3 times (once for each button), so it's an improvement, isn't it?

That said, feel free to create an issue about uniformisation of buttons in pages: probably not all identical, but at least "classes" of buttons that can be reused.

packages/smooth_app/lib/pages/product/ocr_widget.dart

monsieurtanuki requested a review from a team as a code owner November 17, 2022 12:29

monsieurtanuki requested review from AshAman999 and M123-dev November 17, 2022 12:29

github-actions bot added database Hunger Games 🌐 l10n 🖼️ Photos - Cropping Product addition The easier it is to add a product and get Nutri-Score, Eco-Score, the happier the users. 🥫 Product page labels Nov 17, 2022

github-actions bot assigned monsieurtanuki Nov 17, 2022

Merge branch 'develop' into fix_7/3018

07b81aa

AshAman999 reviewed Nov 17, 2022

View reviewed changes

monsieurtanuki mentioned this pull request Nov 17, 2022

Infinite loop while uploading packaging image for new product #3043

Closed

monsieurtanuki mentioned this pull request Nov 18, 2022

We're getting black screens again #3316

Closed

M123-dev approved these changes Nov 19, 2022

View reviewed changes

monsieurtanuki added 2 commits November 19, 2022 14:36

Suggested by reviewer

3c00e8d

Merge branch 'develop' into fix_7/3018

73d2307

monsieurtanuki merged commit fba2851 into openfoodfacts:develop Nov 19, 2022

github-actions bot mentioned this pull request Nov 19, 2022

chore(develop): release 4.1.0 #3305

Merged

monsieurtanuki mentioned this pull request Nov 19, 2022

Background + offline incoherences on multiple offline edits #3018

Closed

omkarChend1kar mentioned this pull request Nov 26, 2022

feat: 3129 Allow to swipe between product images on the full screen image view #3325

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: 3018 - instant upload of images #3329

fix: 3018 - instant upload of images #3329

monsieurtanuki commented Nov 17, 2022

codecov-commenter commented Nov 17, 2022 •

edited

AshAman999 left a comment

monsieurtanuki commented Nov 17, 2022

AshAman999 commented Nov 17, 2022

M123-dev commented Nov 17, 2022

M123-dev left a comment

M123-dev Nov 19, 2022

monsieurtanuki Nov 19, 2022

M123-dev Nov 19, 2022

monsieurtanuki Nov 19, 2022

fix: 3018 - instant upload of images #3329

fix: 3018 - instant upload of images #3329

Conversation

monsieurtanuki commented Nov 17, 2022

What

Part of

codecov-commenter commented Nov 17, 2022 • edited

Codecov Report

AshAman999 left a comment

Choose a reason for hiding this comment

monsieurtanuki commented Nov 17, 2022

AshAman999 commented Nov 17, 2022

M123-dev commented Nov 17, 2022

M123-dev left a comment

Choose a reason for hiding this comment

M123-dev Nov 19, 2022

Choose a reason for hiding this comment

monsieurtanuki Nov 19, 2022

Choose a reason for hiding this comment

M123-dev Nov 19, 2022

Choose a reason for hiding this comment

monsieurtanuki Nov 19, 2022

Choose a reason for hiding this comment

codecov-commenter commented Nov 17, 2022 •

edited