fix: 4066 - top n product download split in smaller robust parts #4166
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
New files: * `background_offline_barcodes.dart`: Background subtask about pre-downloading top n barcodes. * `background_offline_products.dart`: Background subtask about downloading products. * `dao_work_barcode.dart`: Work table that contains barcodes. Impacted files: * `background_task_offline.dart`: now we clean the new `DaoWorkBarcode` and we just call the new `BackgroundtaskOfflineBarcodes` * `local_database.dart`: upgraded to version 3, with the new `DaoWorkBarcode`. * `operation_type.dart: added the new 2 background tasks. TODO * `backgrount_task_manager.dart`: make it possible to actually chain tasks with immediate execution
|
@monsieurtanuki Could you clarify what's left to move from draft to reviewable ? |
In terms of code and files, no. It looks like I needed that "push as draft" step in order to focus on a specific feature and a different file. I'll push the rest by the end of the week. |
|
I'll undraft it tomorrow.
Looking for a product when offline (immediate result):
|
New files: * `background_task_paged.dart`: Abstract background task with paged actions. * `background_task_progressing.dart`: Abstract background task with work in progress actions. Impacted files: * `background_task_crop.dart`: minor refactoring * `background_task_details.dart`: minor refactoring * `background_task_hunger_games.dart`: minor refactoring * `background_task_image.dart`: minor refactoring * `background_task_refresh_later.dart`: minor refactoring * `background_task_unselect.dart`: minor refactoring * `background_task_upload.dart`: minor refactoring * `dao_work_barcode.dart`: minor refactoring * `edit_product_page.dart`: minor refactoring * `local_database.dart`: minor refactoring * `new_product_page.dart`: minor refactoring * `offline_tasks_page.dart`: minor refactoring; displays progress percentage * `operation_type.dart`: moved file; minor refactoring for progress percentage * `product_image_gallery_view.dart`: minor refactoring * `up_to_date_changes.dart`: minor refactoring Later: * `background_task.dart` * `background_task_download_products.dart` * `background_task_full_refresh.dart` * `background_task_manager.dart` * `background_task_offline.dart` * `background_task_top_barcodes.dart` * `offline_data_page.dart`
github-actions
bot
added
background tasks
Product addition
Impacted files: * `app_en.arb`: added a label for topN download with parameter * `background_task.dart`: added a bool getter for immediate exec of the next task; minor refactoring * `background_task_download_products.dart`: now as a _progressing_ background task, reused for top N, with KP and without KP * `background_task_full_refresh.dart`: now as a _paged_ background task, that opens two progressing download tasks (one with KP, one without KP) * `background_task_manager.dart`: now working as singleton; added a special case for immediate exec of the next task * `background_task_offline.dart`: now as a _progressing_ background task * `background_task_top_barcodes.dart`: now as a _progressing_ background task * `dao_work_barcodes.dart`: fixed the bulk insert and delete for more than 1K products (actually, 1K SQL parameters) * `offline_data_page.dart`: now using topN and pageSize parameters * `offline_tasks_page.dart`: added a "work" label for the tasks * `operation_type.dart`: now embeds the "work" in the task id
Codecov Report
@@ Coverage Diff @@
## develop #4166 +/- ##
===========================================
+ Coverage 10.88% 11.00% +0.12%
===========================================
Files 276 282 +6
Lines 13581 14015 +434
===========================================
+ Hits 1478 1543 +65
- Misses 12103 12472 +369
... and 39 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Final touch is the refresh local database part:
|
teolemon
reviewed
Jun 18, 2023
packages/smooth_app/lib/background/background_task_download_products.dart
Outdated
Show resolved
Hide resolved
teolemon
reviewed
Jun 18, 2023
packages/smooth_app/lib/background/background_task_download_products.dart
Outdated
Show resolved
Hide resolved
teolemon
reviewed
Jun 18, 2023
teolemon
reviewed
Jun 18, 2023
packages/smooth_app/lib/background/background_task_progressing.dart
Outdated
Show resolved
Hide resolved
teolemon
reviewed
Jun 18, 2023
packages/smooth_app/lib/background/background_task_progressing.dart
Outdated
Show resolved
Hide resolved
…oducts.dart Co-authored-by: Pierre Slamich <pierre.slamich@gmail.com>
…oducts.dart Co-authored-by: Pierre Slamich <pierre.slamich@gmail.com>
….dart Co-authored-by: Pierre Slamich <pierre.slamich@gmail.com>
….dart Co-authored-by: Pierre Slamich <pierre.slamich@gmail.com>
M123-dev
approved these changes
Jun 21, 2023
There was a problem hiding this comment.
Heyyy @monsieurtanuki looks good to me. You are the one with the background task understanding. Just a few general comments
| pageSize, | ||
| ); | ||
| if (barcodes.isEmpty) { | ||
| // we're done! |
packages/smooth_app/lib/background/background_task_download_products.dart
Show resolved
Hide resolved
| ); | ||
| await task.addToManager(localDatabase, widget: widget); | ||
| } | ||
|
|
||
| @override | ||
| String? getSnackBarMessage(final AppLocalizations appLocalizations) => | ||
| 'Starting the download of the most popular products'; // TODO(monsieurtanuki): localize | ||
| 'Starting the download of the most popular products'; |
|
Thank you @M123-dev for your review! I added localizations, after your comment. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Currently working on the top n offline downloads.
The main idea is to
Doing so we have less potential performance and connection problems, and we don't block the other user actions.
The only thing that is missing now is having a background task calling the next background task (e.g. the next download "page") with immediate execution. That's the first time we would need that in the app - so far we have the "upload image" + "refresh the product 10 minutes later", which is not immediate.
The same split (get barcodes then download products) would be applied to the local database refresh from the server.
Screenshot
Fixes bug(s)
Files
New files:
background_offline_barcodes.dart: Background subtask about pre-downloading top n barcodes.background_offline_products.dart: Background subtask about downloading products.dao_work_barcode.dart: Work table that contains barcodes.Impacted files:
background_task_offline.dart: now we clean the newDaoWorkBarcodeand we just call the newBackgroundtaskOfflineBarcodeslocal_database.dart: upgraded to version 3, with the newDaoWorkBarcode.operation_type.dart: added the new 2 background tasks.