Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify the user country in API queries #133

Closed
stephanegigandet opened this issue Jan 30, 2021 · 18 comments
Closed

Specify the user country in API queries #133

stephanegigandet opened this issue Jan 30, 2021 · 18 comments
Assignees

Comments

@stephanegigandet
Copy link
Contributor

The returned results may differ by country of request. e.g. we currently disable the Eco-Score attribute unless the country is set to France (either by having the "cc" parameter set to "fr", or the query sent to fr.openfoodfacts.org). This may require changes in the openfoodfacts-dart package, I haven't checked yet.

@stephanegigandet
Copy link
Contributor Author

It does require a change in openfoodfacts-dart: openfoodfacts/openfoodfacts-dart#76

@stephanegigandet
Copy link
Contributor Author

The openfoodfacts-dart change has been merged: openfoodfacts/openfoodfacts-dart#76

@monsieurtanuki
Copy link
Contributor

@stephanegigandet I cannot see the direct impact of that merge for the moment.
The current code for keyword search is something like that:

await OpenFoodAPIClient.searchProducts(
  ProductQuery.SMOOTH_USER,
  ProductSearchQueryConfiguration(
    fields: ProductQuery.fields,
    parametersList: <Parameter>[
      const PageSize(size: 500),
      TagFilter(
        tagType: 'categories',
        contains: true,
        tagName: keywords,
      )
    ],
    language: LanguageHelper.fromJson(languageCode),
  ),
);

How should we alter the code? adding something like that to the Parameter list?

TagFilter(
  tagType: 'cc',
  contains: true,
  tagName: 'fr',
)

And it is available now or should we use a fresher version than openfoodfacts: ^0.3.14+1?

@stephanegigandet
Copy link
Contributor Author

The current code for keyword search is something like that:

Ah right, I hadn't noticed there was a specific ProductSearchConfiguration, I will update it.

Then it should be:

await OpenFoodAPIClient.searchProducts(
  ProductQuery.SMOOTH_USER,
  ProductSearchQueryConfiguration(
    fields: ProductQuery.fields,
    parametersList: <Parameter>[
      const PageSize(size: 500),
      TagFilter(
        tagType: 'categories',
        contains: true,
        tagName: keywords,
      )
    ],
    language: LanguageHelper.fromJson(languageCode),
    cc: "fr",
  ),
);

@stephanegigandet
Copy link
Contributor Author

And it is available now or should we use a fresher version than openfoodfacts: ^0.3.14+1?

I'll push it in 0.3.15 and publish it: openfoodfacts/openfoodfacts-dart#81

@monsieurtanuki
Copy link
Contributor

@stephanegigandet OK then! Tell me when openfoodfacts: ^0.3.15 is available.

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki : here is the pull request that adds the cc parameter to ProductSearchQueryConfiguration: openfoodfacts/openfoodfacts-dart#82

@stephanegigandet
Copy link
Contributor Author

0.3.15 is deployed: https://pub.dev/packages/openfoodfacts

@monsieurtanuki monsieurtanuki self-assigned this Feb 5, 2021
@monsieurtanuki
Copy link
Contributor

@stephanegigandet Cool!

For ProductSearchQueryConfiguration it's OK, I could replace

language: LanguageHelper.fromJson(languageCode),

with

lc: locale.languageCode,
cc: locale.countryCode,

But for Pnns search PnnsGroupQueryConfiguration is used, and doesn't seem to have been impacted the same way...

@monsieurtanuki
Copy link
Contributor

Additional remarks:

monsieurtanuki referenced this issue in monsieurtanuki/smooth-app Feb 5, 2021
Impacted files:
* `choose_page.dart`: added the country code parameter when calling `KeywordsProductQuery`
* `keywords_product_query.dart`: replaced `language` parameter by `lc` and `cc`; refactored
* `pubspec.yaml`: upgraded `openfoodfacts` to brand new version `^0.3.15`
monsieurtanuki added a commit that referenced this issue Feb 5, 2021
feature/#133 - added country code parameter for keyword search
@stephanegigandet
Copy link
Contributor Author

  • adding cc=fr the size of the result is 5,481,717 bytes - it's bigger, though my assumption was that it would be smaller (AND/OR confusion in the filters?)

well you get 500 results in both cases. try with a smaller country: cc=re (Reunion) and you will get 2 results.

but with cc=fr, you get extra data in the attribute_groups as you get the Eco-Score attribute (but the search results are different, so it can also change the size)

@stephanegigandet
Copy link
Contributor Author

But for Pnns search PnnsGroupQueryConfiguration is used, and doesn't seem to have been impacted the same way...

We can add it, but eventually we will need to remove the queryPnnsGroup functions from the Dart package, they should not be needed. The PNNS queries are just search queries with a filter on a parameter, just like the keywords search queries etc.

@monsieurtanuki
Copy link
Contributor

@stephanegigandet My remarks:

  • understood for cc=re
  • but the iso country code is supposed to be in capital letters, and that's what I pass as a parameter: country code in capital letters
  • and the result is different for cc=re (2 results) and cc=RE (500 results, like for cc=fror no cc; it looks like the unknown cc was discarded)
  • hence my question: should we lowercase the country code in flutter or should you manage uppercase country codes on the server side?
  • I'm also interested in your comment about PNNS queries - I guess it would make sense to create a dedicated issue about it (should we replace them with our current keywords searches? should we remove the category search from the UI?)

@stephanegigandet
Copy link
Contributor Author

@monsieurtanuki : I'll fix the cc=RE server side: openfoodfacts/openfoodfacts-server#4833

@stephanegigandet
Copy link
Contributor Author

  • I'm also interested in your comment about PNNS queries - I guess it would make sense to create a dedicated issue about it (should we replace them with our current keywords searches? should we remove the category search from the UI?)

@monsieurtanuki : I was more thinking about the implementation of those PNNS queries in openfoodfacts-dart: we can get the exact same functionality without special code for it. I just filed an issue regarding that: openfoodfacts/openfoodfacts-dart#92

The PNNS category browsing feature is interesting, I was not suggesting to remove it.

@monsieurtanuki
Copy link
Contributor

cf. #19 I guess

@monsieurtanuki
Copy link
Contributor

Hey, by the way this issue can now be closed, on the flutter side at least.

@stephanegigandet
Copy link
Contributor Author

Server side fix for the uppercase country code: openfoodfacts/openfoodfacts-server#4834
It should be deployed soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants