refactoring/#237 - deprecated product fields countries, categories, labels

[monsieurtanuki]

No description provided.

[stephanegigandet]

So in fact 'en:deutschland' and 'en:frankreich' are incorrect values, the canonical names in the taxonomy use English: en:germany, en:france (the en: prefix means it's in English).

This made me realize that the issue is more complex than that.

For reading products we should not use the "countries" field returned by the API, because it contains values in any language (the last language used when this field was updated).

On the server side, we store what we got in the "countries" field in the "countries" field, and then we use the language code passed by the client ("lc" field) to canonicalize the countries. So if "lc" was "de", and the client passed "Deutschland, Frankreich" in "countries", we will store the canonical values in countries_tags : "en:germany" and "en:france".

Then the next time we read the product using countries_tags we get the canonical value. And if a French client requests countries_tags_fr, it gets "Allemagne", "France".

But we still need a way for apps to send values in a specific language. If we deprecate "countries", "categories", and "labels", they won't be able to do it.

I think the right solution would be to still deprecate "countries" etc. but allow clients to use the countriesTagsInLanguages field so that a French app can send "Allemagne", "Espagne" etc.

I'm not sure if that's supported already, we don't seem to have tests for it.

[monsieurtanuki]

@stephanegigandet I'm not 100% sure that answers your comment, but this is what I tried in api_saveProduct_test.dart / test 6:

test('add new product test 6', () async {
  Product product = Product(
    barcode: '7340011364184',
// ...
    countriesTagsInLanguages: {OpenFoodFactsLanguage.FRENCH: ['Allemagne', 'Italie']} // MY TEST
  );
Status status = await OpenFoodAPIClient.saveProduct(TestConstants.TEST_USER,product);
// ...
ProductQueryConfiguration configurations = ProductQueryConfiguration('7340011364184');
ProductResult result = await OpenFoodAPIClient.getProduct(configurations, user: TestConstants.TEST_USER);
// ...
print('t1:${result.product!.countriesTagsInLanguages}'); // TEST 1: prints "t1:null"
print('t2:${result.product!.countriesTags}'); // TEST 2: prints "t2:[en:united-kingdom]"

Where does that en:united-kingdom comes from?

[stephanegigandet]

en:united-kingdom is the country of the already existing product with that barcode on the .net server: https://world.openfoodfacts.net/product/7340011364184/chili-beans-null (login and password: off )

[monsieurtanuki]

@stephanegigandet OK. I've just tested to save the product with those parameters:

countriesTagsInLanguages: {OpenFoodFactsLanguage.FRENCH: ['Allemagne', 'Italie']},
countries: 'Spain',

and this is what I get:

t1:null
t2:[en:spain]

Then, I've tried with countries: 'Deutschland', and the result is t2:[en:deutschland]...

It looks like

• only countries are saved
• they are saved as en:

[monsieurtanuki]

IMHO it would make sense to:

• create an enum named OpenFoodFactsCountry, as a reference of all countries
• populate the enum for all countries
• create an extension for this enum, with the corresponding en:germany tags (just in English for the moment) (or forever)
• instead of countriesTags as List<String>?, use a new parameter called countriesRefs as List<OpenFoodFactsCountry>?
• in a first version at least, have countriesRefs automatically override the values of the deprecated countries
• the idea is to fix the current bug by adding a conceptual layer; it doesn't look too demanding to ask flutter developers for OpenFoodFactsCountrys in the write operations, while still providing the translated values in the read operations

[monsieurtanuki]

@stephanegigandet Is there an open issue on the backend side so that we can write countries with 'countries_tags' or 'countries_tags_in_languages'?

[monsieurtanuki]

@stephanegigandet Somehow related to #233: another option would be to use a country enum, linked to both the 2-character ISO country code and the name in English.

[monsieurtanuki]

I've just converted this PR temporarily to draft; I don't want it to be accidentally merged because yes the code has been approved, but in fact there are things to be changed on the backend beforehand.

[monsieurtanuki]

@teolemon Actually this PR has been stalled for months. Unless you have good news about the server side, I'd even say we'd be better off closing it altogether.