feat: 617 - new "save packagings V3" feature #640
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
New files: * `api_saveProductV3_test.dart`: Integration tests around the "save packagings V3" feature. * `ProductPackaging.dart`: Packaging component for a product, e.g. recyclable bottle made of glass. * `ProductPackaging.g.dart`: generated Impacted files: * `AbstractQueryConfiguration.dart`: populated parameter `tags_lc` * `api_getProduct_test.dart`: removed references to now deprecated field `packaging`; added a test about the new packagings field * `api_saveProduct_test.dart`: skipped one 504 test; removed references to deprecated field `packaging` * `HttpHelper.dart`: new method `doPatchRequest`; minor refactoring * `openfoodfacts.dart`: new method `temporarySaveProductV3` * `Product.dart`: deprecated field `packaging`; added field `packagings` * `Product.g.dart`: generated * `ProductFields.dart`: deprecated `PACKAGING`; added `PACKAGINGS` and `RAW` * `ProductSearchQueryConfiguration.dart`: added api version parameter * `UriHelper.dart`: new method `getPatchUri`
M123-dev
approved these changes
Dec 16, 2022
There was a problem hiding this comment.
Looks good, thanks @monsieurtanuki just some minor comments
Comment on lines
+315
to
+316
| // TODO: deprecated from 2022-12-16; remove when old enough | ||
| @Deprecated('User packagingS instead') |
There was a problem hiding this comment.
Suggested change
| // TODO: deprecated from 2022-12-16; remove when old enough | |
| @Deprecated('User packagingS instead') | |
| // TODO: deprecated from 2022-12-16; remove when old enough | |
| @Deprecated('Use [packagings] instead') |
| @@ -468,7 +475,8 @@ class Product extends JsonObject { | |||
| this.labels, | |||
| this.labelsTags, | |||
| this.labelsTagsInLanguages, | |||
| this.packaging, | |||
| // TODO: deprecated from 2022-12-16; remove when old enough | |||
| @Deprecated('Use packagingS field instead') this.packaging, | |||
There was a problem hiding this comment.
Suggested change
| @Deprecated('Use packagingS field instead') this.packaging, | |
| @Deprecated('Use [packagings] field instead') this.packaging, |
Feel free to leave it with the capital S
There was a problem hiding this comment.
The square bracket syntax is unfortunately not detected in strings, only in /// comments.
| /// | ||
| /// For the moment that's the only field supported in WRITE by API V3. | ||
| /// Long term target is of course more something like [saveProduct]. | ||
| static Future<ProductResultV3> temporarySaveProductV3( |
There was a problem hiding this comment.
Suggested change
| static Future<ProductResultV3> temporarySaveProductV3( | |
| static Future<ProductResultV3> temporarySaveProductPackagingsV3( |
Just a suggestion
There was a problem hiding this comment.
Could make sense but in the long run I expect more fields to match V3 (e.g. one by month) : I'm not going to create a different method for each.
Comment on lines
+32
to
+35
| static Map<String, dynamic>? addUserAgentParameters( | ||
| Map<String, dynamic>? map, | ||
| ) { | ||
| map ??= <String, String>{}; | ||
| map ??= <String, dynamic>{}; |
There was a problem hiding this comment.
With the new method doPatchRequest the input can be Map<String, dynamic>, and not Map<String, String>.
More specifically in that case:
parameterMap['product']['packagings'] = packagings;
|
Thank you @M123-dev for your fast review! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New files:
api_saveProductV3_test.dart: Integration tests around the "save packagings V3" feature.ProductPackaging.dart: Packaging component for a product, e.g. recyclable bottle made of glass.ProductPackaging.g.dart: generatedImpacted files:
AbstractQueryConfiguration.dart: populated parametertags_lcapi_getProduct_test.dart: removed references to now deprecated fieldpackaging; added a test about the new packagings fieldapi_saveProduct_test.dart: skipped one 504 test; removed references to deprecated fieldpackagingHttpHelper.dart: new methoddoPatchRequest; minor refactoringopenfoodfacts.dart: new methodtemporarySaveProductV3Product.dart: deprecated fieldpackaging; added fieldpackagingsProduct.g.dart: generatedProductFields.dart: deprecatedPACKAGING; addedPACKAGINGSandRAWProductSearchQueryConfiguration.dart: added api version parameterUriHelper.dart: new methodgetPatchUriWhat
packagingsproduct field provides detailed information about the packaging, cf. new classProductPackagingpackagingsis supported with save/V3. That means that packagings should be saved with a different call from the other fields.Fixes bug(s)