feat!: hunger games

[VaiTon]

What

• This PR adds a sort of hunger games to the mobile app, enhancing the already existent question_page.dart to make it automatically download Robotoff questions if no product/list of questions is passed.

Also Changed

• refactor: move CongratsWidget to its own file congrats.dart
• refactor: move question_page.dart and congrats.dart to the subfolder hunger_games
• refactor: rename haveInsightAnnotationsVoted to areQuestionsAlreadyVoted to better reflect it's action
• refactor: extract question_answers_options.dart and question_card.dart from question_page.dart (@monsieurtanuki)

Video

2022-10-16.20-17-50.mp4

Fixes

• Closes Hunger Games in Smoothie #2517

[VaiTon]

@openfoodfacts/smooth-app please review this first implementation code-wise

[monsieurtanuki]

@VaiTon I'm speaking only for myself, but:

• your PR has no description
• your PR is only in draft mode

That's not very encouraging, is it?

In addition to that, as even in draft it's a PR, every time you'll decide to commit a change, I'll receive an email for that, which is a bit annoying. Unless I mute it.

My suggestion: if you want to code something, you can talk about it in an issue, exchange about the UI/UX and the code potential problems and so on.
And only then you code and commit and PR, and not in draft mode.

Actually I'm currently a bit saturating - nothing personal, it's just that I see this PR now.
Especially because I'm working on a quite problematic background task / offline / refresh issue that requires full attention, and I get pinged constantly, which diverts my attention.
I prefer to get pinged for something that matters like "Hey, I have this interesting PR we've already talked about!" or "I'm blocked, help, how do we get this info?" rather than "Guys, I've coded something, and it's just a draft".

[VaiTon]

@monsieurtanuki thank you for your comment, although I don't get a few things:

• The PR is to implement a feature we already talked in the issue, mock-wise and SDK-wise, so I thought that could be enough.
• I'm sorry that you feel in that way but I asked for a code review at this time because I know that a PR with lots of files and changes is almost impossible to review. I will obviously put up a video of UX and a summary of changes when ready, but I'm still changing lots of parts so that's not really something useful.

In addition to that, as even in draft it's a PR, every time you'll decide to commit a change, I'll receive an email for that, which is a bit annoying. Unless I mute it.

I mean, that's what drafts, are for:
https://github.blog/2019-02-14-introducing-draft-pull-requests/

At GitHub, we’ve always felt that you should be able to open a pull request to start a conversation with your collaborators as soon as your brilliant idea or code is ready to take shape. Even if you end up closing the pull request for something else, or refactoring the code entirely, a good pull request is as much about collaboration as it is about code.

The fact that you're being notified is I think due to the fact that we have codeowners set up. That's a problem on it's own I think and I will now remove it from the reviewers so that nobody gets pinged every time I push a commit.

[monsieurtanuki]

Hi @VaiTon!

The PR is to implement a feature we already talked in the issue, mock-wise and SDK-wise, so I thought that could be enough.

Indeed we both exchanged comments about Hunger Games this afternoon.
Does that mean that it's exactly what you're PR'ing right now? Does that mean that if I'm off 2 days I'll remember what the PR was about? Does that mean that only me or someone who followed our discussion this afternoon should know the context of it?

I asked for a code review at this time because I know that a PR with lots of files and changes is almost impossible to review.

Not always. And so far we're talking about 8 files, that's manageable.

I'm still changing lots of parts so that's not really something useful.

So what would be this code review for, at such an early stage of your code, with already 2 pushed commits on top by now? Again, a bit discouraging.

At GitHub, we’ve always felt that you should be able to open a pull request to start a conversation with your collaborators

They don't say "you should be able to open a pull request to start a conversation with your collaborators". It's just a possibility. And of course, in some cases that can probably make sense to create a draft PR.

a good pull request is as much about collaboration as it is about code.

I totally agree with that, except that in my mind that collaboration happens before the PR, in an issue for instance. My best experience on github was with osmdroid, where we sometimes spent dozens of comments about very hard issues. And then, once we listed the potential pitfalls, remembered what happened the previous year with similar issues, double-checked how it worked on other apps and agreed on the solution, or not 100% agreed but agreed to start, then it was time to code: when everything was ready to be translated into code.
If you do it on a PR, you get all the code noise ("put that class rather in that file", "reformat", "I don't know what you want to do actually but the code looks correct", "finally I do exactly the same thing but in a private method"), and a vague urge feeling (should this PR be rapidly merged?).
It's a bit like listing items with a pencil on a piece of paper, and doing the same thing with a smartphone. If you write on paper, you write on paper. If you have a smartphone, you say "still waiting for the next item, I have time to check my emails, just one second" and you do something else that what you were supposed to do in the first place: list items.
For that, I miss my experience with osmdroid: exchange about issues before PR'ing.

The fact that you're being notified is I think due to the fact that we have codeowners set up.

I don't know very much about github (I don't plan to), but indeed if we could receive less mails that would be very cool - the icing on the cake being getting rid of codecov that AFAIK never worked.

Good luck with this PR!

[VaiTon]

Hey @monsieurtanuki, thank you for your detailed explanation and I will keep it in mind next time.

About notifications, I'll see what can be done

[VaiTon]

Actually blocked on openfoodfacts/openfoodfacts-dart#573

[teolemon]

@VaiTon little conficts to resolve, can you put a temporary screenshot ?

[VaiTon]

@teolemon I'm waiting for the SDK update and then I'll finalize the PR

[M123-dev]

It's released, I'm about to update smoothie to support the new version

[codecov-commenter]

Codecov Report

Merging #3102 (01801bf) into develop (adb7716) will decrease coverage by 0.01%.
The diff coverage is 0.34%.

❗ Current head 01801bf differs from pull request most recent head ba5d9ad. Consider uploading reports for the commit ba5d9ad to get more accurate results

@@            Coverage Diff             @@
##           develop   #3102      +/-   ##
==========================================
- Coverage     6.19%   6.17%   -0.02%     
==========================================
  Files          248     252       +4     
  Lines        12439   12495      +56     
==========================================
+ Hits           771     772       +1     
- Misses       11668   11723      +55     

Impacted Files	Coverage Δ
...th_app/lib/cards/data_cards/image_upload_card.dart	0.00% <ø> (ø)
...ib/cards/product_cards/product_image_carousel.dart	0.00% <0.00%> (ø)
...mooth_app/lib/helpers/robotoff_insight_helper.dart	0.00% <0.00%> (ø)
...es/smooth_app/lib/pages/hunger_games/congrats.dart	0.00% <0.00%> (ø)
...b/pages/hunger_games/question_answers_options.dart	0.00% <0.00%> (ø)
...ooth_app/lib/pages/hunger_games/question_card.dart	0.00% <0.00%> (ø)
...pages/preferences/user_preferences_contribute.dart	4.72% <0.00%> (-0.14%)	⬇️
...b/pages/preferences/user_preferences_dev_mode.dart	0.00% <0.00%> (ø)
...ges/smooth_app/lib/pages/product/summary_card.dart	0.00% <0.00%> (ø)
.../smooth_app/lib/query/product_questions_query.dart	0.00% <0.00%> (ø)
... and 2 more

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

[monsieurtanuki]

Hi @VaiTon!
Looks rather good to me, except the QuestionPage: it's 478 line long and I suggest that you split it in different files, if that sounds reasonable to you.

[M123-dev]

Also can we put this in DevMode for testing, maybe some fine-tuning is needed. Not codewise but robotoff wise. I think I read something in slack about it needing further work in some categories before releasing it on a large scale

[monsieurtanuki]

Thank you @VaiTon for the refactoring: now it's easier to read and review.
Minor comments now: feel free to ignore them.

[monsieurtanuki]

@VaiTon Btw I wonder about feat! versus feat: if it's a mistake please fix it, if it's what you wanted leave it that way.

[VaiTon]

Hey @monsieurtanuki! It's part of the conventional commits format, it's to signal a breaking change (or an important one)

https://www.conventionalcommits.org/en/v1.0.0/

[VaiTon]

Thank you everybody for your time!

[raphael0202]

Very nice work 🎉!
For everything related to Robotoff and Hunger Games don't hesitate to ping or add me or Alexandre F. as reviewer for feedbacks :)
About this feature, why is there a "Thank for contributing" page from time to time?

[VaiTon]

@raphael0202 this is the first "iteration", so no loading in the background. Instead we just query 3 random questions and when we're finished we ask if the user wants to continue or exit.