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: 4564 - product immediately visible even if not on the server yet #4584

Merged
merged 4 commits into from
Sep 19, 2023

Conversation

monsieurtanuki
Copy link
Contributor

What

  • For the record in our local database we only store full product data from the server. And we also store somewhere else pending changes to be applied on top. With this mechanism we can display fresh data locally before it's uploaded.
  • The fix here was mainly on ProductModel.
  • The idea was to say: if you don't find the product in the database, it's perhaps because it's not been sent to the server yet.
  • The issue was about: it's not in the database and it's not on the server (which happens for products created locally before the upload), with the "unable to fetch information from the server" message.
  • Instead we add an additional case: not in the database but with pending changes.

Screenshot

How to reproduce:

  1. connected to internet
  2. I select a barcode that does not exist (here, a fake book barcode)
  3. it's unknown on the server: the app suggests me to create the product
  4. I disconnect from internet
  5. I add a detail (product name), and save the product.
  6. I go to my "history"
  7. I see the product I created (that's basically what the issue was about) though it is not on the server
  8. I see a "PENDING..." banner that implies that pending changes have not been completely processed yet

Screenshot_2023-08-21-17-27-26

Fixes bug(s)

Impacted files

  • fetched_product.dart: moved code from ProductRefresher
  • product_cards_helper.dart: unrelated bug fix
  • product_dialog_helper.dart: special case when the product was created locally but not uploaded/refreshed yet
  • product_field_editor.dart: unrelated bug fix
  • product_list_item_simple.dart: moved code to model
  • product_model.dart: special case when the product was created locally but not uploaded/refreshed yet
  • product_refresher.dart: moved code to FetchedProduct; minor refactoring
  • smooth_product_card_found.dart: added a banner for product with pending changes.
  • up_to_date_product_provider.dart: minor refactoring

Impacted files:
* `fetched_product.dart`: moved code from `ProductRefresher`
* `product_cards_helper.dart`: unrelated bug fix
* `product_dialog_helper.dart`: special case when the product was created locally but not uploaded/refreshed yet
* `product_field_editor.dart`: unrelated bug fix
* `product_list_item_simple.dart`: moved code to model
* `product_model.dart`: special case when the product was created locally but not uploaded/refreshed yet
* `product_refresher.dart`: moved code to `FetchedProduct`; minor refactoring
* `smooth_product_card_found.dart`: added a banner for product with pending changes.
* `up_to_date_product_provider.dart`: minor refactoring
@monsieurtanuki
Copy link
Contributor Author

For the record, this is how Evernote marks notes as "not sync'ed yet". The sync'ed notes don't have the green triangle:
Screenshot_2023-08-26-07-37-35-1

@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2023

Codecov Report

Merging #4584 (b45875a) into develop (05d0490) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff            @@
##           develop    #4584   +/-   ##
========================================
  Coverage    10.03%   10.03%           
========================================
  Files          312      312           
  Lines        15642    15635    -7     
========================================
  Hits          1569     1569           
+ Misses       14073    14066    -7     
Files Changed Coverage Δ
...cards/product_cards/smooth_product_card_found.dart 0.00% <0.00%> (ø)
...es/smooth_app/lib/data_models/fetched_product.dart 13.04% <0.00%> (-20.29%) ⬇️
...p/lib/data_models/up_to_date_product_provider.dart 0.00% <0.00%> (ø)
...s/smooth_app/lib/helpers/product_cards_helper.dart 0.00% <0.00%> (ø)
...ib/pages/product/common/product_dialog_helper.dart 0.00% <0.00%> (ø)
...pages/product/common/product_list_item_simple.dart 0.00% <0.00%> (ø)
...th_app/lib/pages/product/common/product_model.dart 0.00% <0.00%> (ø)
...pp/lib/pages/product/common/product_refresher.dart 0.00% <0.00%> (ø)
...th_app/lib/pages/product/product_field_editor.dart 0.00% <0.00%> (ø)

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

@teolemon
Copy link
Member

@g123k can you have a look at the PR ?

@teolemon teolemon added the Product addition The easier it is to add a product and get Nutri-Score, Eco-Score, the happier the users. label Aug 26, 2023
@github-actions github-actions bot removed the Product addition The easier it is to add a product and get Nutri-Score, Eco-Score, the happier the users. label Sep 12, 2023
Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

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

I think the idea is good to visualize pending changes, but let's hide it in the dev mode first to think about where we have to display it too

Product page?
Product edit page?
The images if they are uploaded?
...

Impacted file:
* `smooth_product_card_found.dart`: removed the "pending..." banner
@monsieurtanuki
Copy link
Contributor Author

Thank you @M123-dev for your review!
I've just removed the "pending..." banner as it was not at all the core of the fix, but rather a trial ballon.
If one day the "pending" banner is to be implemented, it will be in another PR.

Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

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

Looks right to me, thanks @monsieurtanuki

@monsieurtanuki
Copy link
Contributor Author

Thank you @M123-dev for your review!

@monsieurtanuki monsieurtanuki merged commit e84826b into openfoodfacts:develop Sep 19, 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.

An issue with background tasks?
4 participants