-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
feat: 831 - new price methods like add price and upload proof #884
feat: 831 - new price methods like add price and upload proof #884
Conversation
New files: * `maybe_error.dart`: Contains a successful value OR an error. * `price_per.dart`: Price Per. * `proof.dart`: Proof of a price. * `proof.g.dart`: generated * `proof_type.dart`: Type of a Proof. * `session.dart`: Price session. * `session.g.dart`: generated Impacted files: * `api_get_save_product_test.dart`: unrelated minor fix * `api_prices_test.dart`: added tests for the new methods * `http_helper.dart`: added optional "bearerToken" parameter to GET; new `doDeleteRequest`, `doPostJsonRequest` and `imagineMediaType` methods * `open_prices_api_client.dart`: added parameters to `getPrices`; new methods `getAuthenticationToken`, `getUserSession`, `deleteUserSession`, `createPrice`, `deletePrice`, `uploadProof` and `deleteProof`; minor refactoring * `openfoodfacts.dart`: added 5 new files to export * `price.dart`: added fields. * `price.g.dart`: generated * `pubspec.yaml`: added package `http_parser` for `MediaType` * `test_constants.dart`: unrelated minor fix
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #884 +/- ##
==========================================
+ Coverage 75.64% 76.01% +0.36%
==========================================
Files 231 236 +5
Lines 8085 8391 +306
==========================================
+ Hits 6116 6378 +262
- Misses 1969 2013 +44 ☔ View full report in Codecov by Sentry. |
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 a lot @monsieurtanuki, added some minor comments 👍🏼
if (bearerToken != null) { | ||
return _getBearerHeader(bearerToken); | ||
} |
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.
Don't we want a user agent and a "From" header. Just because prices does not use them yet it could be interesting
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.
Don't we want a user agent and a "From" header. Just because prices does not use them yet it could be interesting
Could be done later if needed, for stats I guess.
I don't know if prices
plan to do something with it.
For the moment the code is clearer to read that way: "token? just return this! else do as usual".
Co-authored-by: Marvin Möltgen <39344769+M123-dev@users.noreply.github.com>
Co-authored-by: Marvin Möltgen <39344769+M123-dev@users.noreply.github.com>
Thank you very much @M123-dev for your quick (but detailed) review! Regarding verbose comments, I don't know the best practice: is it relevant to copy the API comments here, as they can be outdated? Is it relevant to just point to the API docs? |
Great 🎉 |
@raphael0202 Today I'll PR a bigger I'm still a bit stuck with the "get location" feature, that we need too:
Perhaps we would need the prices for a product within 5km. |
Currently we use OSM Nominatim, which does the job (but is far from perfect). Can you use it as well here?
Hum I'm not sure of this, maybe @raphodn will know better |
What
Now we're getting serious about prices with the following new methods:
getAuthenticationToken
to start a session with authenticationgetUserSession
to check if a session is runningdeleteUserSession
to close a sessioncreatePrice
to add a pricedeletePrice
to delete a priceuploadProof
to upload a proofdeleteProof
to delete a proofPart of
Files
New files:
maybe_error.dart
: Contains a successful value OR an error.price_per.dart
: Price Per.proof.dart
: Proof of a price.proof.g.dart
: generatedproof_type.dart
: Type of a Proof.session.dart
: Price session.session.g.dart
: generatedImpacted files:
api_get_save_product_test.dart
: unrelated minor fixapi_prices_test.dart
: added tests for the new methodshttp_helper.dart
: added optional "bearerToken" parameter to GET; newdoDeleteRequest
,doPostJsonRequest
andimagineMediaType
methodsopen_prices_api_client.dart
: added parameters togetPrices
; new methodsgetAuthenticationToken
,getUserSession
,deleteUserSession
,createPrice
,deletePrice
,uploadProof
anddeleteProof
; minor refactoringopenfoodfacts.dart
: added 5 new files to exportprice.dart
: added fields.price.g.dart
: generatedpubspec.yaml
: added packagehttp_parser
forMediaType
test_constants.dart
: unrelated minor fix