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

Add a system to refresh all cached products for Nutri-Score v2 #4545

Open
1 of 3 tasks
Tracked by #4523
teolemon opened this issue Aug 15, 2023 · 10 comments
Open
1 of 3 tasks
Tracked by #4523

Add a system to refresh all cached products for Nutri-Score v2 #4545

teolemon opened this issue Aug 15, 2023 · 10 comments
Labels
fixed ? This bug might already be fixed. If so, close it. local cache Nutri-Score v2 🚥 Nutri-Score P2

Comments

@teolemon
Copy link
Member

teolemon commented Aug 15, 2023

What

Tasks

  1. data refresh local cache

Part of

@monsieurtanuki
Copy link
Contributor

About the refresh of local products:

  1. that could be a specific but reusable background task ("refresh all local products")
  2. it would be triggered at app init time, with a flag in the preferences so that we call it only once
  3. in a best world scenario the server would provide a URL that returns the current version of the score algorithms, which we would compare to a locally stored value
  4. in a best world scenario the server would also provide some html text, to be displayed on the specific "onboarding" page
  5. with all that in place we could manage additional nutriscore changes without any impact on the Smoothie code.

Beyond that is the problem of using 2 nutriscores for the same product.
If the "new" nutriscore has the same tag as now, that's ok, and there could be an additional nutriscore_previous field for whoever wants to see the evolution.

@monsieurtanuki
Copy link
Contributor

The good thing is that we already have a background task that refreshes all the local products: BackgroundTaskFullRefresh.
Currently we use it only in dev mode, when the user specifically asks us to refresh all the local products.

@teolemon @stephanegigandet Would the server be ready to provide us with a URL like off.org/latest-algorithm.pl?cc=fr&lc=fr, returning

  • a unique tag about the current score computation algorithm (e.g. '2017-nutri')
  • a localized explanation of the current score evolutions (blablabla olive oil...)

@stephanegigandet
Copy link
Contributor

We should probably weight how much complexity we add versus the benefits we get. Having an old Nutri-Score in the local cache may not be a big problem, especially if we refresh those items when they actually get displayed to the user.

We could just consider that the local cache is just a cache, that it can be outdated (it's already the case for any other edit to the product page), and that it's ok.

Under what conditions / actions do we use the local cache? If I have network connectivity, do we just always get fresh content anyway?

@monsieurtanuki
Copy link
Contributor

@stephanegigandet Actually we use the local cache (local SQLite database) as the main source. Instant results.
We don't refresh automatically.
The user needs to explicitly refresh (e.g. swipe down gesture), or after a successful local edit the product will be automatically refreshed with the latest server data.

What we could do do implement a lazy refresh, is to async'ly download the product each time we go to the product page - while still displaying the local cache version. After the product is successfully re-downloaded, the displayed data will be refreshed.
How often should we do it? In which pages? Those are just details.

@teolemon
Copy link
Member Author

I think whenever we reopen the product page, we should attempt to refresh, and flash the content whenever that happens (possibly with an animated transition)

@monsieurtanuki
Copy link
Contributor

That wouldn't be a problem: we already have a BackgroundTaskRefreshLater that downloads the product 10 minutes later (cf. Robotoff). We can reuse it and set a 0 second delay instead of 10 minutes.

I'm a bit worried about how to prevent cluttering, as we use a single queue for our background tasks. And about low connectivity context.

Perhaps we could add this in dev mode only first, in order to monitor it.

@M123-dev
Copy link
Member

M123-dev commented Sep 2, 2023

Keep in mind I am not up to date with the current background manager implementation, but we could add a kind of priority system, so that things like this are always executed first.

+ Other use: when a query fails (image too small) we could downgrade its importance so that other working queries are not blocked forever

@monsieurtanuki
Copy link
Contributor

Keep in mind I am not up to date with the current background manager implementation, but we could add a kind of priority system, so that things like this are always executed first.

Given how the background manager works and the pain in the neck it would be to amend it (assuming that the concept of priority would be helpful), a moderately easy way to implement a priority could be to use 2 managers, one for fast queries (like product refresh) and one for slower queries (like image upload).
Or to just play it lazy with a simple async refresh called without await in initState.
But anyway, with slow connection we're doomed.

Other use: when a query fails (image too small) we could downgrade its importance so that other working queries are not blocked forever

  • the other working queries are never blocked
  • the "image too small" will fail forever even with a low priority, and that causes discrepancies between the local data and the server data (cf. related issues)
    => I would be more than happy to hear suggestions about how to deal UX-wise with obvious failed queries (image too small or wrong password) and the way to warn the user.

@monsieurtanuki
Copy link
Contributor

cf. #5016 (comment)

For refresh, the target is refresh everything to Nutriscore v2, with the ability to test it for privileged users before going public.

We can easily have all the products refreshed: we would just need to clear all local products "download language" field, and start a fake "app language change background task".
@teolemon Would you like me to add a dev mode action about it?

Regarding "normal" users, the trick would be to check a one-shot preference value at init time (e.g. "if 0 then set to 1 and refresh all products"). Possibly with a date check "only if date is after April 1st 2024".

@M123-dev
Copy link
Member

M123-dev commented Jun 7, 2024

It has been a while, I guess the nutriscore v2 was rolled out already. Is this still a concern @teolemon

@teolemon teolemon added the fixed ? This bug might already be fixed. If so, close it. label Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed ? This bug might already be fixed. If so, close it. local cache Nutri-Score v2 🚥 Nutri-Score P2
Projects
Status: Todo
Status: 💬 To discuss and validate
Development

No branches or pull requests

4 participants