Skip to content
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: 891 - new "uploaded timestamp" field for raw images #892

Merged
merged 3 commits into from
Mar 30, 2024

Conversation

monsieurtanuki
Copy link
Contributor

What

  • Now we use the "uploaded timestamp" field that the server provided for raw images, but that we just didn't retrieve.
  • With this field we'll be able to say if an uploaded image is "too old", e.g. if it was uploaded more than a year ago.

Fixes bug(s)

Part of

Impacted files:

  • api_json_to_from_test.dart: added tests around new field uploaded
  • json_helper.dart: added the uploaded field to from/to json conversion methods; refactored
  • product_image.dart: added field uploaded, for uploaded images only

Impacted files:
* `api_json_to_from_test.dart`: added tests around new field `uploaded`
* `json_helper.dart`: added the `uploaded` field to from/to json conversion methods; refactored
* `product_image.dart`: added field `uploaded`, for uploaded images only
@@ -130,6 +130,7 @@ class ProductImage {
required String this.imgid,
this.width,
this.height,
this.uploaded,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can have a better variable name, maybe something like uploaded_on or created_at. Or is it the same var name in other places?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @AshAman999!

In this PR we have an 'uploaded_t' JSON tag and an int? uploaded field.

If we compare to similar fields:

  • we also have a 'created_t' JSON tag and a created field (cf. in Product)
  • but instead of int? in this PR, we do have a much more explicit DateTime?

Would you be OK with DateTime? uploaded? That would be consistent with the rest of the timestamp fields in the package, and wouldn't be confusing with a DateTime? type.

Copy link
Member

@teolemon teolemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per @AshAman999 's comment

@monsieurtanuki
Copy link
Contributor Author

Thank you @teolemon @AshAman999 for your comments!
I don't merge right now as DateTime? uploaded was indeed a good idea instead of int?. I'll implement that by tomorrow.

@monsieurtanuki monsieurtanuki merged commit 56446da into openfoodfacts:master Mar 30, 2024
2 of 3 checks passed
@monsieurtanuki
Copy link
Contributor Author

Thank you @teolemon @AshAman999: I've just merged the code with DateTime? uploaded field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Add field "uploaded timestamp" for "raw" images
3 participants