feat: 4020 - instant answers for hunger games #4099

monsieurtanuki · 2023-06-08T09:05:12Z

What

Now, for the user, the transition between questions is instant.
How come?
- now we use a new background task for hunger games - when the user enters an answer, we create a background task and immediately go to the next question
- when we download the questions at the beginning, we also download the related products. Doing so, we don't have to download a fresh version of the next product for the next question - we already have all the needed data.

Fixes bug(s)

Closes: Don't block the screen while saving a Hunger Games answer #4020

Files

New files:

background_task_hunger_games.dart: Background task about answering a hunger games question.
random_questions_query.dart: Robotoff questions helper, for random product questions.

Deleted file:

product_question_page.dart

Impacted files:

abstract_background_task.dart: added "hunger games"
background_task_crop.dart: minor refactoring
background_task_details.dart: minor refactoring
background_task_image.dart: minor refactoring
background_task_manager.dart: simplified the code, as we now always work on tasks with different stamps
background_task_refresh.dart: minor refactoring
background_task_unselect.dart: minor refactoring
new_product_page.dart: minor refactoring
offline_data_page.dart: minor refactoring
operation_type.dart: added a type for "hunger games" background task
product_list_import_export.dart: minor refactoring
product_list_page.dart: minor refactoring
product_questions_query.dart: pre-loads the product if relevant
product_questions_widget.dart: minor refactoring
product_refresher.dart: new standard configuration for product lists; new silent download of product lists; minor refactoring
question_card.dart: now downloading the product only if not in the local database
question_page.dart: now saving the answer with background tasks; refactored
questions_query.dart: refactored as abstract, with code moved to new file random_questions_query.dart
robotoff_insight_helper.dart: minor refactoring

New files: * `background_task_hunger_games.dart`: Background task about answering a hunger games question. * `random_questions_query.dart`: Robotoff questions helper, for random product questions. Deleted file: * `product_question_page.dart` Impacted files: * `abstract_background_task.dart`: added "hunger games" * `background_task_crop.dart`: minor refactoring * `background_task_details.dart`: minor refactoring * `background_task_image.dart`: minor refactoring * `background_task_manager.dart`: simplified the code, as we now always work on tasks with different stamps * `background_task_refresh.dart`: minor refactoring * `background_task_unselect.dart`: minor refactoring * `new_product_page.dart`: minor refactoring * `offline_data_page.dart`: minor refactoring * `operation_type.dart`: added a type for "hunger games" background task * `product_list_import_export.dart`: minor refactoring * `product_list_page.dart`: minor refactoring * `product_questions_query.dart`: pre-loads the product if relevant * `product_questions_widget.dart`: minor refactoring * `product_refresher.dart`: new standard configuration for product lists; new silent download of product lists; minor refactoring * `question_card.dart`: now downloading the product only if not in the local database * `question_page.dart`: now saving the answer with background tasks; refactored * `questions_query.dart`: refactored as abstract, with code moved to new file `random_questions_query.dart` * `robotoff_insight_helper.dart`: minor refactoring

codecov-commenter · 2023-06-08T10:48:50Z

Codecov Report

Merging #4099 (9cb8d2f) into develop (bfe1d60) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #4099      +/-   ##
===========================================
- Coverage    10.88%   10.84%   -0.05%     
===========================================
  Files          271      273       +2     
  Lines        13466    13517      +51     
===========================================
  Hits          1466     1466              
- Misses       12000    12051      +51

Impacted Files	Coverage Δ
...h_app/lib/background/abstract_background_task.dart	`0.00% <0.00%> (ø)`
...mooth_app/lib/background/background_task_crop.dart	`0.00% <0.00%> (ø)`
...th_app/lib/background/background_task_details.dart	`0.00% <ø> (ø)`
...p/lib/background/background_task_hunger_games.dart	`0.00% <0.00%> (ø)`
...ooth_app/lib/background/background_task_image.dart	`0.00% <0.00%> (ø)`
...th_app/lib/background/background_task_manager.dart	`0.00% <ø> (ø)`
.../lib/background/background_task_refresh_later.dart	`0.00% <0.00%> (ø)`
...h_app/lib/background/background_task_unselect.dart	`0.00% <0.00%> (ø)`
...ges/smooth_app/lib/data_models/operation_type.dart	`0.00% <0.00%> (ø)`
...kages/smooth_app/lib/helpers/analytics_helper.dart	`0.00% <0.00%> (ø)`
... and 14 more

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

g123k

Just a few feedbacks, but in general LGTM

g123k · 2023-06-08T13:18:00Z

packages/smooth_app/lib/background/background_task_crop.dart

@@ -198,7 +198,7 @@ class BackgroundTaskCrop extends BackgroundTaskUpload {
    final LocalDatabase localDatabase,
    final bool success,
  ) async {
-    localDatabase.upToDate.terminate(uniqueId);
+    await super.postExecute(localDatabase, success);


If you say await here, wouldn't be better to also have an async call for the delete just below?
(Just a suggestion)

If you say await here, wouldn't be better to also have an async call for the delete just below? (Just a suggestion)

That would probably work the same, wouldn't it? Especially here, where we delete just one file in a background task.

g123k · 2023-06-08T13:19:00Z

packages/smooth_app/lib/background/background_task_hunger_games.dart

+        return result;
+      }
+    } catch (e) {
+      //


I think there's something missing here :)

I think there's something missing here :)

Actually, nothing's missing here. We just try to fromJson a map, and very often it's going to fail because some expected fields won't be there - if it is a background task from a different class. All other background tasks work on the same principle. cf. AbstractBackgroundTask.fromJson.
That said, I agree that it's not very elegant.
Feel free to create an issue to refactor that fromJson.

g123k · 2023-06-08T13:19:26Z

packages/smooth_app/lib/background/background_task_hunger_games.dart

+      insightAnnotation.value,
+      uniqueId,
+    );
+    await task.addToManager(localDatabase, widget: widget);


why not return here, instead of the await?

why not return here, instead of the await?

As far as I know that would be exactly the same, wouldn't it?

g123k · 2023-06-08T13:21:15Z

packages/smooth_app/lib/background/background_task_hunger_games.dart

+  Future<void> upload() async {
+    final InsightAnnotation? annotation =
+        _getInsightAnnotation(insightAnnotation);
+    if (annotation == null) {


Wouldn't be a good idea to log this kind of error to measure the volume of potentials errors?

It's very very unlikely to happen, as we previously populated the task field as InsightAnnotation.
Feel free to create an issue if really needed.

monsieurtanuki · 2023-06-08T14:07:12Z

Thank you @g123k for your review!

Feel free to approve this PR if my answers made sense to you.

teolemon

After a quick discussion with @g123k, let's merge as it

monsieurtanuki requested a review from a team as a code owner June 8, 2023 09:05

github-actions bot added background tasks Hunger Games labels Jun 8, 2023

github-actions bot assigned monsieurtanuki Jun 8, 2023

github-actions bot added Product addition The easier it is to add a product and get Nutri-Score, Eco-Score, the happier the users. 🥫 Product page User lists labels Jun 8, 2023

monsieurtanuki requested a review from teolemon June 8, 2023 09:12

monsieurtanuki added 2 commits June 8, 2023 11:14

Merge branch 'develop' into feat/4020

b9809f3

feat: 4020 - unrelated package upgrade

cce1ad9

github-actions bot added the dependencies label Jun 8, 2023

monsieurtanuki linked an issue Jun 8, 2023 that may be closed by this pull request

Error in trunk #4098

Closed

fix: 4098 - new enum value from iOS 12

01de218

Merge branch 'develop' into feat/4020

9cb8d2f

github-actions bot removed the dependencies label Jun 8, 2023

g123k reviewed Jun 8, 2023

View reviewed changes

teolemon approved these changes Jun 8, 2023

View reviewed changes

monsieurtanuki merged commit cb815b0 into openfoodfacts:develop Jun 8, 2023

github-actions bot mentioned this pull request Jun 8, 2023

chore(develop): release 4.8.0 #3937

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 4020 - instant answers for hunger games #4099

feat: 4020 - instant answers for hunger games #4099

monsieurtanuki commented Jun 8, 2023

codecov-commenter commented Jun 8, 2023 •

edited

Loading

g123k left a comment

g123k Jun 8, 2023

monsieurtanuki Jun 8, 2023

g123k Jun 8, 2023

monsieurtanuki Jun 8, 2023

g123k Jun 8, 2023

monsieurtanuki Jun 8, 2023

g123k Jun 8, 2023

monsieurtanuki Jun 8, 2023

monsieurtanuki commented Jun 8, 2023

teolemon left a comment

feat: 4020 - instant answers for hunger games #4099

feat: 4020 - instant answers for hunger games #4099

Conversation

monsieurtanuki commented Jun 8, 2023

What

Fixes bug(s)

Files

codecov-commenter commented Jun 8, 2023 • edited Loading

Codecov Report

g123k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monsieurtanuki commented Jun 8, 2023

teolemon left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 8, 2023 •

edited

Loading