-
Notifications
You must be signed in to change notification settings - Fork 7
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
[Issue] Only iOS - user could upload the duplicate photo #2715
Comments
➤ Sam commented: Kenny Hung, just to double check Android show wrong error message you mean this message? https://app.asana.com/app/asana/-/get_asset?asset_id=1204361548656097 ( https://app.asana.com/app/asana/-/get_asset?asset_id=1204361548656097 ) |
➤ Kenny Hung commented: Sam Yes, like the similar error messge, but I didn't see it before. I suggest the error message could be more simple, like (Need Tammy Yang's confirmation.) "The asset you are trying to upload already exists on the blockchain. Please upload a different asset." About Chinese, need Sofia Yan's help. |
➤ Kenny Hung commented: But this task is about avoiding user could upload the same photo file more 1 time. (Error message content is another task. [Issue] Error handling for uploading duplicate photo ( https://app.asana.com/0/1201016280880500/1204361832181061/f )) |
➤ Sam commented: Kenny Hung thank you for your feedback. |
➤ Sam commented: Kenny Hung, can you try again on iOS same version.
I think you will get Internet connection ... error this time. |
➤ Kenny Hung commented: Sam Yes, but this issue can’t reproduce every time, the ratio is about 20%. when you try more times, almost 5 times, user could upload 2 times successfully. |
➤ Tammy Yang commented: James ChienOlgaBofu Chen need your help We encounter the problem that on iOS duplicate photos are allowed to upload. Steps to reproduce:
Example for the issue:
Debugging: I do notice that XMP injection of the two files are different:
As you can see, the image filename is different and the extension is wrong. My question: Do you know if XMP injection may affect the IPFS cid? I am trying to figure out is it because of any injection done by the mobile OS which causes the issue. Any other suggestions are also welcome. |
➤ Tammy Yang commented: Additional debugging, I downloaded the two images from Kenny's example and does the pixel-by-pixel comparison (source code see https://pastebin.com/a6Zre8Zs ( https://pastebin.com/a6Zre8Zs )), they are identical (i.e. the difference is must from header) |
➤ Tammy Yang commented: Exif are all the same, too (source code to check see https://pastebin.com/8aShmRN2 ( https://pastebin.com/8aShmRN2 ))
|
➤ Tammy Yang commented: However, the interesting part is, if I upload the two photos again to our IPFS gateway using this API ( https://docs.numbersprotocol.io/developers/more-tools/ipfs-pin-file-to-ipfs ) (which look identical from every aspect except the filename in XMP) again, the Nid are indeed different 😅. |
➤ Sam commented: Tammy Yang, regarding CAI injection I found this in changelog ( https://github.com/numbersprotocol/capture-lite/blob/master/CHANGELOG.md#changed-32 ). And this is related commit ( f5803fd ) in 0.39.0. I think there is no CAI injection logic from the capture app. |
➤ Tammy Yang commented: Well... their header is the IDENTICAL XD Source code: https://pastebin.com/JzNGVXj3 ( https://pastebin.com/JzNGVXj3 ) Result printed:
|
➤ Bofu Chen commented: Extract and save the headers of the two photos
Comparing the difference between the headers, the only difference is
|
➤ Sam commented: I think I narrow down bug surface. In capture app there is function called setAsset (
Then in capture app we check ( capture-lite/src/app/shared/database/table/capacitor-filesystem-table/capacitor-filesystem-table.ts Line 106 in d9f8764
Here is what I did iOS 0.77.2
And it happened because for the same base64 (image 5) capture app generated (
Here are the logs from capture app iOS logs: |
➤ Bofu Chen commented: Kenny HungSamTammy Yang If the failure rate is medium (1/30) and only happens on iOS, I recommend that we release first and ensure to fix this issue in this sprint. |
➤ Tammy Yang commented: SamKenny Hung Let's do this. Which ever with lowest fail rate and highest quality, let's go with it and set conditional pass. This issue may need a bit more efforts and I will ask James Chien's support to check together in the next sprint. |
➤ Sam commented: Kenny Hung, so 0.77.2 (1) firebase release is ok? If yes I will start prepearing code review We choose 0.77.2(1) firebase release as candidate right? |
➤ Kenny Hung commented: Sam Yes! Please submit code reivew v230321-ionic based on 0.77.2 You could refer Comment by @kenny Hung on v230310-capture-app-ionic(merge v230223) ( https://app.asana.com/0/0/1204010561520024/1204419595745090/f ). |
➤ Kenny Hung commented: Sam Another thing, after you prepare code, and will start v230413-capture-app-ionic ( https://app.asana.com/0/0/1204223578973623 ) please also make sure the firebase release includes the 0.77.2 implemented. |
➤ Tammy Yang commented: If [CFM] Update upload review mechanism ( https://app.asana.com/0/0/1204451854984158 ) is fixed, I believe this issue will no longer exist. Though, still good to find out what happens in the App side to change the image. |
User story
As a user of the asset upload feature on the capture app, I want the app to prevent me from uploading duplicate photos, so that I can avoid creating unnecessary duplicates and efficiently manage my assets.
Reproduce step
Example:
﹍https://nftsearch.site/asset-profile?cid=bafybeiafqrzdvtf44rmt2zngqvqr6mommtpxvyctvvzdmkqp3gc6tccq5a﹍
﹍https://nftsearch.site/asset-profile?cid=bafybeiclmlojwqypwpuhz2akzd3pmfi3pl3poql24qdvxjzausanti4pie﹍
Expectation
Additional information:
┆Issue is synchronized with this Asana task by Unito
┆Created By: Kenny Hung
The text was updated successfully, but these errors were encountered: