feat: Allow to pass a country and/or a language to the register user method

[g123k]

Hi everyone,

Today, all users are registered on the world.openfoodfacts.org subdomain.
If they choose to also subscribe to the newsletter, they will receive it in English, but we have translated issues.

That's why we can now pass a country and/or a language.

It will fix #753

[monsieurtanuki]

@g123k The code looks perfect, but I suggest that you add some related tests.
More specifically, I had a bad experience with fr-fr. that was not really accepted by the server (perhaps redirected to fr. and something was lost in the redirection).
Same for be-nl. that may redirect to be. too (be-fr. being unchanged).

[g123k]

@g123k The code looks perfect, but I suggest that you add some related tests. More specifically, I had a bad experience with fr-fr. that was not really accepted by the server (perhaps redirected to fr. and something was lost in the redirection). Same for be-nl. that may redirect to be. too (be-fr. being unchanged).

Mmm @teolemon, you told me it was OK for the combinations.
What should I do?

[monsieurtanuki]

Mmm @teolemon, you told me it was OK for the combinations. What should I do?

@g123k You may just add country and language parameters to the existing register tests. Typically fr-fr and be-fr.

[g123k]

Yes, true, but wouldn't it be better to also add an exhaustive list of countries/languages?

[monsieurtanuki]

Yes, true, but wouldn't it be better to also add an exhaustive list of countries/languages?

I don't think it would be better to add tests that explore all the country/language combinations in this PR.
So far I detected strange behavior of fr-fr, that's why I strongly suggest tests on it - especially given that we probably have French users.

That said, if there is such thing as a list of country-language redirections on the server (e.g. fr-fr becomes fr), we could implement it in dart, but that would deserve a specific issue and a specific PR.

Regarding this PR, please just add fr-fr parameters to the existing register tests.

[g123k]

Yes, true, but wouldn't it be better to also add an exhaustive list of countries/languages?

I don't think it would be better to add tests that explore all the country/language combinations in this PR. So far I detected strange behavior of fr-fr, that's why I strongly suggest tests on it - especially given that we probably have French users.

That said, if there is such thing as a list of country-language redirections on the server (e.g. fr-fr becomes fr), we could implement it in dart, but that would deserve a specific issue and a specific PR.

Regarding this PR, please just add fr-fr parameters to the existing register tests.

Ok, I have duplicated some tests to specificacy (re)test cases with FR-fr.
If that's OK, it would be nice to release a new version, as we would like to also release a new version of the app, to fix it

[monsieurtanuki]

Ok, I have duplicated some tests to specificacy (re)test cases with FR-fr. If that's OK, it would be nice to release a new version, as we would like to also release a new version of the app, to fix it

Ok for a new release ASAP.
That said, correct me if I'm wrong but having a look at the failed test report, the register tests are not actually run, are they?

[monsieurtanuki]

@g123k That said, if you can confirm that you ran successfully the new register unit tests locally, I'm ready to approve this PR.

[g123k]

@g123k That said, if you can confirm that you ran successfully the new register unit tests locally, I'm ready to approve this PR.

I understand why it fails: because error messages are translated ("Ce nom d'utilisateur existe déjà, choisissez en un autre.")
Any suggestion on an implementation for this?

[monsieurtanuki]

I understand why it fails: because error messages are translated ("Ce nom d'utilisateur existe déjà, choisissez en un autre.") Any suggestion on an implementation for this?

Just put the translated error message, specifically in French.
As said before, not sure if testing all country/language configurations is relevant / a high-level priority, so translating in one language is good enough.

That said, perhaps it would make sense for the server to return specific error codes in addition to localized error messages, but that's another story.

[monsieurtanuki]

@g123k Still don't understand why the register tests are not even run, but as long as you ran them successfully locally that's good enough for me.

[g123k]

@g123k Still don't understand why the register tests are not even run, but as long as you ran them successfully locally that's good enough for me.

+1, that's weird that the GitHub Action seems inconsistent.
But yes, on my side, it's OK.

So I merge this PR. Thanks a lot for your review!

[g123k]

For the release process, actually I don't know how it works, so if you can do it, that would be nice.
Thanks 👍

[monsieurtanuki]

@g123k Just approve and merge #750, et voilà!

[g123k]

@g123k Just approve and merge #750, et voilà!

Indeed, not that difficult 😅