feature/#551 - towards a refresh of stale data on the Product page #571
Conversation
…oduct page New files: * `product_timestamp.dart`: Abstract timestamp getter for Product. * `product_timestamp_list.dart`: List of ProductTimestamps. * `product_database_timestamp.dart`: Product timestamp taken from the local database upsert timestamp. * `product_off_timestamp.dart`: Product timestamp taken from the "last modified" OFF field. Impacted file: * `product_page.dart`: added a check on the product timestamp, with the goal of stale data refresh
| @override | ||
| Widget build(BuildContext context) { | ||
| final LocalDatabase localDatabase = context.watch<LocalDatabase>(); | ||
| final AppLocalizations appLocalizations = AppLocalizations.of(context)!; | ||
| if (_first) { | ||
| _first = false; | ||
| _product = widget.product; | ||
| _refreshIfNeeded( |
There was a problem hiding this comment.
This should probably be done inside a lifecycle method like didChangeDependencies().
There was a problem hiding this comment.
I didn't know didChangeDependencies().
I'm not sure I understand the notion of "change dependencies"; in my mind I just want the code to be called once, for the first build, and that's it.
| @@ -47,6 +51,7 @@ class ProductPage extends StatefulWidget { | |||
|
|
|||
| class _ProductPageState extends State<ProductPage> { | |||
| late Product _product; | |||
| static const Duration _duration = Duration(minutes: 5); | |||
There was a problem hiding this comment.
Fair enough.
| @@ -73,13 +78,35 @@ class _ProductPageState extends State<ProductPage> { | |||
| _updateHistory(context); | |||
| } | |||
|
|
|||
| Future<void> _refreshIfNeeded(final ProductTimestamp productTimestamp) async { | |||
| final int? millisecondsSinceEpoch = | |||
| await productTimestamp.getTimestamp(widget.product); | |||
There was a problem hiding this comment.
Would it be simpler to just take the maximum of product.lastModified and DaoProduct(localDatabase).getLastUpdate(product.barcode) instead using all that abstraction? Or do you expect the ProductTimestamp hierarchy be useful anywhere else?
There was a problem hiding this comment.
We could probably manage to code that without abstraction, but it's not the way I code.
I do expect the ProductTimestamp to be used in a different use case: the timestamp for this product on the server. So far we deal with product "staleness" - and we could also deal with the product "freshness" on the server.
| final int elapsedInMillis = nowInMillis - millisecondsSinceEpoch; | ||
| print('last modified was ${elapsedInMillis ~/ 1000} seconds ago'); | ||
| if (_duration.inMilliseconds < elapsedInMillis) { | ||
| print('should refresh'); |
There was a problem hiding this comment.
Do you intend to update this before merging?
There was a problem hiding this comment.
Definitely.
For the moment @stephanegigandet and I disagree about the way to refresh data: he would refresh automatically, and I would display a snackbar saying "This product data is 6 minutes old" / "Refresh action".
I'll probably implement the former or the latter, and if needed the code conversion from one version to the other shouldn't be problematic afterwards.
|
I haven't read through the code yet, but I assume that when the product page is opened, the last modification time of the cached product is compared with the backend and updated if necessary. Am I correct with this assumption |
No you're not. |
|
This is the way it looks with the |
|
Thanks for the explanation @monsieurtanuki, I understand your thought behind the PR but I don't think the current way makes that much sense. Just because the data is stale doesn't mean it's out of date. The idea to pay attention to the time is good in any case, I started #573 to discuss how and what we want to cache so that we hopefully come to a final concept. |
|
Dismissed as in contradiction with #573. |
New files:
product_timestamp.dart: Abstract timestamp getter for Product.product_timestamp_list.dart: List of ProductTimestamps.product_database_timestamp.dart: Product timestamp taken from the local database upsert timestamp.product_off_timestamp.dart: Product timestamp taken from the "last modified" OFF field.Impacted file:
product_page.dart: added a check on the product timestamp, with the goal of stale data refresh