-
-
Notifications
You must be signed in to change notification settings - Fork 14
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(proof): fetch and insert proof data to 'add single price' page when using 'add the price' button #584
Conversation
…hen using 'add the price' button
this.addPriceSingleForm.proof_id = proofId | ||
handleProofSelected(proof) { | ||
this.addPriceSingleForm.proof_id = proof.id | ||
this.addPriceSingleForm.date = new Date(proof.created).toISOString().split('T')[0] |
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.
I'd rather keep it empty and let the user fill it in, no ? Or some kind of warning ?
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.
When setting a recent proof or proof from URL parameter ID I definitely intended the date to be automatically set like it already does when uploading a new proof, but maybe not to the proof upload date like it does here. Preferably it should set the date to when the picture was taken instead of today's date like the current running version of the site does when selecting a proof right now. However, without adding a column for file creation time in the database, it probably have to use some potentially expensive sql queries to determine the date based on other prices uploaded to the proof already, if it even has any price entries at all. I've tried to download proof images I uploaded with exif data that has been compressed and process into mebp files, but I was not able to find any of the original exif data or dates. Only the modification date of when I uploaded the file to Open Prices. It could be that the file's exif date is stripped when serving the file through the URL (and still available internally), but it appears that maybe that data is lost in the compression process.
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 for your detailed analysis.
the compression/conversion (to .webp) is done on the frontend side.
for the metadata you found in an already-uploaded proof image, you mention the modification date, is the date in the json returned by the server, or a date present in the metadata ?
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.
The date in the json response makes even less sense. It's before I even took the image. I'm assuming it's using a different time zone and it's actually 17:00 reflecting the date of the downloaded image:
{
"id": 6644,
"file_path": "0007/2rO9xlT7Nn.webp",
"mimetype": "image/webp",
"type": "PRICE_TAG",
"price_count": 1,
"owner": "odinh",
"created": "2024-05-21T15:00:46.692372Z"
}
Here's a screenshot showing the metadata of both the downloaded image and the original:
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.
yeah I think the server is on a strange timezone or something.
maybe we could add a field "date_taken" ? though sometimes we might not have this info, if an image is uploaded without the related metadata
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.
Maybe changing Disregard that, apparently the created
in the json response from a offset-naive to a offset-aware timestamp would be a good idea to do instead, so it includes the timezone offset?Z
at the end indicates UTC
.
Regarding the file metadata, could maybe make it default to the file's creation date instead, if date_taken
is not available? Most files usually have a creation date even if they don't have the date_taken
EXIF value. Another alternative would be to extract it from the filename like you could for the file in my example:
20240521_162617.jpg
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.
linked issues in the backend :
@@ -412,18 +412,28 @@ export default { | |||
showUserRecentProofs() { | |||
this.userRecentProofsDialog = true | |||
}, | |||
handleProofSelected(proofId) { | |||
this.addPriceSingleForm.proof_id = proofId | |||
handleProofSelected(proof) { |
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.
the problem here is that above in mounted
we only have the proof_id, so this will not work...
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.
can be fixed by keeping proofId
, and removing the date init.
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.
and add an extra param to fetch or not the proof image ? because if coming from the UserRecentProofsDialog
then it we be called even though we already have the image (line 424)
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.
Oh, I forgot to change the function used in mounted
to getProofById
when copying the code over to GitHub. Did that now.
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.
Not sure what you mean by it being called twice when using the UserRecentProofsDialog
though. It's only calling it once for me. Maybe I'm misunderstanding you or something is wrong with my testing environment. I couldn't get image uploading to work, uploaded pics just give me 404 error when I try to access the upload path, so I had to do a few tweaks to work around that by using remote URLs instead.
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.
my bad there doesn't seem to be duplicate loading 👌
ok i pushed a quick commit for linting/cleanup, will merge 💯 |
What
It uses the date the proof was uploaded, so might not always be the correct date for when the picture was taken
Fixes bug(s)
Part of