feat: Add offline mode data manager #2322
Conversation
stored items in db
fresh data from server
There was a problem hiding this comment.
Great start @AshAman999 I added some comments. Mostly about in code comments
There was a problem hiding this comment.
Hi @AshAman999!
I've seen many things that puzzled me, among them:
- in
DaoProductyou've put some code regarding access to the server - in aDAOwe don't give a damn about the rest of the world, we just read and write elements. If you need an access to the server to populate your table, do it elsewhere - it looks like you generously delete all the database, including the history and the scan history. I'm not familiar with the concepts of offline mode for smoothie, but that does not seem appropriate.
And I've written some other comments too...
|
Hello @monsieurtanuki
await daoProduct.clearAll();
await daoProductList.clearAll();
|
Codecov Report
@@ Coverage Diff @@
## develop #2322 +/- ##
==========================================
- Coverage 8.86% 7.10% -1.76%
==========================================
Files 161 220 +59
Lines 6623 10762 +4139
==========================================
+ Hits 587 765 +178
- Misses 6036 9997 +3961
Help us with your feedback. Take ten seconds to tell us how you rate us. |
There was a problem hiding this comment.
Thank you @AshAman999 for your changes!
- Yeah deleted the user list on purpose, In case I don't delete that, The user searches for something he already has searched and it shows no product available that was fetched from history, so to remove the confusion did the changes.
I don't understand what you're up to. So far, the core of your code is about refreshing the products that are already there, am I right? Nothing to do with user search.
If you want to impact user search, you have to code inside the user search. For instance, if you would like the user to type the start of a barcode and automatically suggest all matching barcodes, that's something you'll have to do in the search itself. And you don't need to erase all the database or even refresh the products for that btw.
Therefore, here's my question: What is your quest?
- Getting confused about putting this in transaction, counting on you for some help here
My bad: the notion of transaction is a SQL notion. That would have made sense if both DaoProduct and DaoProductList were in SQL, but that is not the case.
As an added benefit I wanted to let the users free their memory from the app in case of necessity, maybe I have an old Samsung or just that I don't want so much cache
Suppose I mostly use the offline scanned items for my scans, So whenever I visit my grocery store there is poor reception of network so I in advance cache some items about different products and also some products that I mostly scan. The next time I visit stores I want to be sure that the product attributes have not changed. So to refresh all the items in the local cache there is this button so that if I refresh the cache after a month I can be sure that the products and the date is reliable again now Found the idea here, somewhere on google docs presented to me , and that's my goal here, so taking one step at a time
|
did changes requested
Yes you can: just go to your app manager, click on your app and say "delete the app data".
No you can't: you've just erased your scan product list.
So this has nothing to do with offline cache of 5K products, right? It's just about refreshing all the products, like in #1651. My suggestions:
|
|
Thanks again @monsieurtanuki Sorry for all the confusion that you went through, I guess the implications of why I went ahead to add a menu is something that might be confusing here 😕
Meant to say, the user just refreshed the product not deleted them.
It's currently in
Actually a really great idea, I can include that feature in the next upcoming PRs, I just wanted to get some work already done here before the first evaluation 😅
It does actually refresh the product list as well, just wasn't able to update the timestamp for the product list, I had added a For time being if it's about the implication we are confused about, I can discuss it with Pierre in the next smoothie meeting, and rest thanks again for correcting my code to be more efficient, I hope at least the code part is now in good condition to maybe merge if needed |
teolemon
changed the title
| return rowsDeleted; | ||
| } | ||
|
|
||
| Future<int> getLength() async { |
There was a problem hiding this comment.
A getter would be better here
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| final LocalDatabase localDatabase = context.watch<LocalDatabase>(); |
There was a problem hiding this comment.
Please split this Widget into multiple ones.
This one is way to bigger for a Stateful one
There was a problem hiding this comment.
Crafting the constant widgets to be seperate
…into new-menu-for-offline-mode
|
There's a merge conflict to solve @AshAman999 to keep the PR mergeable |
There's are some more changes to do, and this section will make more sense in the app after we start storing big number of data converting to draft for now :) |
…into new-menu-for-offline-mode
…into new-menu-for-offline-mode
| @@ -73,6 +73,18 @@ class DaoProduct extends AbstractSqlDao | |||
| return result; | |||
| } | |||
|
|
|||
| // Returns the no of rows deleted/effected from the table | |||
There was a problem hiding this comment.
| // Returns the no of rows deleted/effected from the table | |
| // Returns the number of rows deleted/effected from the table |
| } | ||
| } | ||
|
|
||
| class OfflineDataDetials extends StatelessWidget { |
| } | ||
| } | ||
|
|
||
| class AppBar extends StatelessWidget { |
There was a problem hiding this comment.
- the name is confusing with the official
AppBar. Please use another one
| ) ?? | ||
| false; | ||
| if (confirmed) { | ||
| final double size = await localDatabase.getSizeinMb(); |
| @@ -276,6 +280,53 @@ class DaoProductList extends AbstractDao { | |||
| return result; | |||
| } | |||
|
|
|||
| /// Returns the number of items cleared from the box | |||
| Future<int> clearAll() async { | |||
| final int clearedItems = await _getBox().clear(); | |||
There was a problem hiding this comment.
You should use a single line instruction here
| @@ -43,4 +43,7 @@ class DaoStringList extends AbstractDao { | |||
| } | |||
| return false; | |||
| } | |||
|
|
|||
| // Returns the number of items removed from the box | |||
| Future<int> removeAll() async => _getBox().clear(); | |||
| databasesRootPath = await getDatabasesPath(); | ||
| } | ||
| final String databasePath = join(databasesRootPath, 'smoothie.db'); | ||
| final String databasePath = await _getDatabasePath(); | ||
| final Database database = await openDatabase( | ||
| databasePath, | ||
| version: 2, |
There was a problem hiding this comment.
Could you make this a constant
| //Keeping at a cap of 500 for better performance | ||
| final List<Product> productList = <Product>[]; | ||
| List<String> chunks = <String>[]; | ||
| for (int i = 0; i < barcodes.length; i += 500) { |
There was a problem hiding this comment.
What is 500? Please may it a constant
What
NOTE:
Translations left to be done later
Screenshot
Fixes bug(s)
Part of