feat: #158 - introduction to folksonomy #332
Conversation
New files: * `api_folksonomy_test.dart`: Tests around folksonomy * `folksonomy.dart`: Client calls of the Folksonomy API (Open Food Facts) * `KeyStats.dart`: Folksonomy: statistics around a tag key. * `ProductList.dart`: Folksonomy: current value for a product and a tag key. * `ProductStats.dart`: Folksonomy: statistics about the tag keys on a product. * `ProductTag.dart`: Folksonomy product tag: for this barcode, that value is set for that key. Impacted files: * `HttpHelper.dart`: added "delete" and "put" requests * `JsonHelper.dart`: added a helper method around timestamps * `OpenFoodAPIConfiguration.dart`: added reference URIs for folksonomy * `openfoodfacts.dart`: exported folksonomy files * `UriHelper.dart`: added a helper method for folksonomy URIs
Impacted files: * `api_folksonomy_test.dart`: removed unnecessary imports * `HttpHelper.dart`: rolled back "delete" and "put" requests
Impacted files: * `api_saveProduct_test.dart`: unrelated test fine-tuning
There was a problem hiding this comment.
Thanks @monsieurtanuki, the code looks really good. I have left a few comments but feel free to ignore them as they are almost all about comments.
| FolksonomyAPIClient._(); | ||
|
|
||
| /// "hello world" | ||
| static Future<void> hello({ |
There was a problem hiding this comment.
Just a void doesn't make sense, lets return the answer instead
There was a problem hiding this comment.
As for ping, returning a void does make sense. At least for me, and that's an interesting point.
If you compare to the current code in openfoodfacts, for instance https://github.com/openfoodfacts/openfoodfacts-dart/blob/master/lib/openfoodfacts.dart#L886-L890, we see that @jasmeet0817 has a problem with throwing an Exception. Should the methods handle the exception and return a clean useless value (and a step further: call Sentry) or let the developer handle the exception?
I think we should let the exceptions being thrown. We should tell the developers that our methods throw exceptions (e.g. when there's no internet, or when the server is down), and let them handle the exceptions their own way, for instance with Sentry in smoothie.
In the specific case of hello or ping, we don't give a damn about a value: we just want to know if everything was alright.
lib/folksonomy.dart
Outdated
| _checkResponse(response); | ||
| } | ||
|
|
||
| /// Returns all the [ProductStats] with that tag key, or with tag key == value |
There was a problem hiding this comment.
What does or with tag key == value mean
| final Map<String, ProductTag> result = <String, ProductTag>{}; | ||
| if (response.body == 'null') { | ||
| // not found | ||
| return result; |
There was a problem hiding this comment.
Same here, null or a mention in the comment
lib/folksonomy.dart
Outdated
|
|
||
| static Future<Map<String, KeyStats>> getKeys({ |
There was a problem hiding this comment.
No comment here.
From the docs:
Get the list of keys with statistics
The keys list can be restricted to private tags from some owner
| return result; | ||
| } | ||
|
|
||
| static Future<void> ping({ |
There was a problem hiding this comment.
What about static Future<bool> instead
Impacted files: * `api_folksonomy_test.dart`: refactored and added tests * `folksonomy.dart`: fixed side-effects; refactored the comments
|
Thank you @M123-dev for your code review! |
|
@monsieurtanuki, lets do a new release when #336 is merged, mainly because I need the new globalUser so that I can finally PR openfoodfacts/smooth-app#700 but we can either remove the folksonomy from the CHANGELOG (and introduce it when its finsihed) or add to it, that its in work and may change. |
|
@M123-dev Do what is convenient for you. folksonomy is not a top priority, smoothie is. |
|
Charles answered:
|
|
Thank you @M123-dev and Charles! |
New files:
api_folksonomy_test.dart: Tests around folksonomyfolksonomy.dart: Client calls of the Folksonomy API (Open Food Facts)KeyStats.dart: Folksonomy: statistics around a tag key.ProductList.dart: Folksonomy: current value for a product and a tag key.ProductStats.dart: Folksonomy: statistics about the tag keys on a product.ProductTag.dart: Folksonomy product tag: for this barcode, that value is set for that key.Impacted files:
HttpHelper.dart: added "delete" and "put" requestsJsonHelper.dart: added a helper method around timestampsOpenFoodAPIConfiguration.dart: added reference URIs for folksonomyopenfoodfacts.dart: exported folksonomy filesUriHelper.dart: added a helper method for folksonomy URIs