Closed monsieurtanuki closed 2 months ago
started a PR here : https://github.com/openfoodfacts/open-prices/pull/426
For now fixes
To look into
Won't "fix"
Thank you @raphodn for your first glance! I'll give you more details about the cryptic problems (e.g. the "true / false expected")
@raphodn The recent changes seem to have a limited impact on Smoothie. The only one I detected was that the "top contributors" page doesn't work anymore. That said, it looks like you're still developing live while I'm checking the errors, which makes my analysis less easy (for instance, 'Authentication credentials were not provided.' instead of 'Invalid authentication credentials') (and in that case my version is more accurate)
Following is my analysis.
[ ] line 106 (getAuthenticationToken
)
Probably no impact on Smoothie
Now we receive something like {"error":"Invalid authentication credentials"}
=> Shouldn't we receive detail
instead of error
, something like {"detail":"Invalid authentication credentials"}
?
[ ] line 117 (deleteUserSession
): expected successful response status for "delete session" was 200, is now 204
Probably no impact on Smoothie as we purposely ignored this response as we didn't want the task to fail just for not being able to delete the user session at the end.
=> Should we now expect a 204
status code for a successful "delete user session"?
[ ] line 123 (createPrice
): expected response status for "invalid bearer token" was 401, is now 403*
Probably no impact on Smoothie as we just check if it failed, we don't check the status code.
=> Should we now expect a 403
status code for a failed "create price" because the bearer token is invalid?
[ ] line 131 (getLocation
): expected response message for "no location found" was something like Location with id -1 not found
, is now No Location matches the given query.
We don't use it in Smoothie anyway
[ ] line 142 (getOSMLocation
): received unrelated HTML when failing
We don't use it in Smoothie anyway
[ ] line 165 (getPriceProductById
): : expected response message for "no product found" was something like Product with id -1 not found
, is now No Location matches the given query.
We don't use it in Smoothie anyway
[ ] line 178 (getPriceProductByCode
): received unrelated HTML when failing
We don't use it in Smoothie anyway
[ ] line 202 (uploadProof
): expected response status for "invalid bearer token" was 401, is now 403
Probably no impact on Smoothie as we just check if it failed, we don't check the status code.
=> Should we now expect a 403
status code for a failed "upload proof" because the bearer token is invalid?
[ ] line 208 (getUsers
): we expect a bool is_moderator
field for users, which is not there anymore.
=> should we consider is_moderator
an optional field, or should you populate it? Maybe both.
=> impact on Smoothie: the "top price users" page doesn't work anymore - actually it still does, but in a funny way ;)
Ok ~about to~ merged the PR, it will fix :
{"error":"Invalid authentication credentials"}
with {"detail":"Invalid authentication credentials"}
Need to look into :
User.is_moderator
field not returned (always False for now as we don't use it yet)In the wontfix :
Fixed by #971.
Description
cf. https://github.com/openfoodfacts/open-prices/issues/355 by @raphodn The Prices API has recently been refactored in Django. There are side-effects (e.g. tests now failing), cf. https://github.com/openfoodfacts/openfoodfacts-dart/actions/runs/10724138009/job/29739089653 To be solved in off-dart or in Prices. Or both.
Expected behavior
The Prices refactoring should have been transparent from off-dart. Working on this issue, we'll see how bad that is. May already have an impact on Prices for Smoothie.
cc. @teolemon @raphael0202