-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
added field ecoscore_score and ecoscore_data to product #63
added field ecoscore_score and ecoscore_data to product #63
Conversation
lib/model/EcoscoreAdjustments.dart
Outdated
@JsonKey(name: "origins_of_ingredients", includeIfNull: false) | ||
IngredientsOrigins ingredientsOrigins; | ||
|
||
EcoscoreAdjustments({this.packaging, this.ingredientsOrigins}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 2 more adjustments, for the production system (e.g. organic labels) and for the threatened species (e.g. palm oil).
example products:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, but I believe there is plenty of other missing fields as well (packagings, agribalyse stuff, ...).
I did not put everything for several reasons:
- it's a first step, I can continue to improve it later, especially is there is a need for these fields. Adding everything in one single PR may be a bit harder to code review.
- I'm not sure you want to be exhaustive in this dart lib or if you want to simply match flutter clients needs.
- I didn't find a documentation of all possible fields and wanted to ask you for that a bit later. Maybe I simply didn't look at the right place.
- I'm not sure how to handle the "en:XXX" strings (like the ones for labels and threatened species). Is is always english or can we sometimes have a map with several language instead?
So, do you still think I should add these two fields? If yes, I can do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Eco-Score is very new, there is no documentation for it yet (one reason is that the Eco-Score has not launched yet, so we have been asked not to make it publicly visible. That will change soon though. :) )
en:something fields are identifiers for tags, we have translations for them in different languages. But there isn't an API to retrieve the translations yet. We'll need to make one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine to have a PR with partial support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then if you agree I suggest to do that in another PR. Also, I don't fully understand how we can have only one production_system's label and only one threatened_species's ingredient, and I wonder if it shouldn't be an array. So here is another question: can we consider the current API to be stable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another PR is fine, thanks.
For the production system label, for the Eco-Score, only the most beneficial label is kept. e.g. if a product is both organic and "Label Rouge", then the bonus for organic is applied, not two bonuses.
For threatened species, at this point only palm oil is considered. There are talks to add some fishes. It might be the same as the production system: we might consider only the worse ingredient.
So, no, the current API is not 100% stable at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Hello, I took the time to pull and check the PR today. Thank you very much for the contribution @FredJul PS: I've added you to the credits of the plugin let me know if you want it in another format or removed 😉 |
Thank you @PrimaelQuemerais for the credits. FYI my current goal is to add the ecoscore to the smooth app, so I'll certainly do PRs for that as well. |
Hi,
I have added the ecoscore_score and ecoscore_data field to Product, which are currently only available when we query with ecoscore_alpha.
All data sub-fields are not handled yet.