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

[Hold for payment 2024-09-23] [$250] Search - Receipt thumbnail becomes gray & blank after changing receipt from JPG to PDF file #47387

Closed
6 tasks done
IuliiaHerets opened this issue Aug 14, 2024 · 48 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Aug 14, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: v9.0.20-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to FAB > Submit expense > Manual.
  3. Submit a manual expense with JPG receipt.
  4. Go to Search.
  5. Click on the expense in Step 3.
  6. Click on the receipt.
  7. Replace the receipt with a PDF file.
  8. Close the RHP.

Expected Result:

The receipt thumbnail in search expense list will update to show the pdf receipt.

Actual Result:

The receipt thumbnail in search expense list shows blank gray thumbnail after changing to a pdf receipt.

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

20240814_141831.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0130e8b27688b14ba0
  • Upwork Job ID: 1823720055329371979
  • Last Price Increase: 2024-09-04
Issue OwnerCurrent Issue Owner: @Pujan92
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 14, 2024
Copy link

melvin-bot bot commented Aug 14, 2024

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@IuliiaHerets
Copy link
Author

We think that this bug might be related to #wave-collect - Release 1

@IuliiaHerets
Copy link
Author

@joekaufmanexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@joekaufmanexpensify joekaufmanexpensify added the External Added to denote the issue can be worked on by a contributor label Aug 14, 2024
Copy link

melvin-bot bot commented Aug 14, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0130e8b27688b14ba0

@melvin-bot melvin-bot bot changed the title Search - Receipt thumbnail becomes gray & blank after changing receipt from JPG to PDF file [$250] Search - Receipt thumbnail becomes gray & blank after changing receipt from JPG to PDF file Aug 14, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 14, 2024
@joekaufmanexpensify
Copy link
Contributor

cc @luacmartins

Copy link

melvin-bot bot commented Aug 14, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Pujan92 (External)

@luacmartins luacmartins self-assigned this Aug 14, 2024
@joekaufmanexpensify
Copy link
Contributor

Waiting for proposals

@NJ-2020
Copy link
Contributor

NJ-2020 commented Aug 15, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Search - Receipt thumbnail becomes gray & blank after changing receipt from JPG to PDF file

What is the root cause of that problem?

When we change the receipt to pdf file, the PDFThumbnail file doesn't handle the thumbnail image properly and seems not working, in other file like DistanceEReceipt file we use the image thumbnail for PDF provided by the BE, by invoking the
ReceiptUtils.getThumbnailAndImageURIs function

const thumbnail = TransactionUtils.hasReceipt(transaction) ? ReceiptUtils.getThumbnailAndImageURIs(transaction).thumbnail : null;

const thumbnailSource = tryResolveUrlFromApiRoot(thumbnail ?? '');

But in our case we not we use the PDF file link url
source={tryResolveUrlFromApiRoot(transactionItem?.receipt?.source ?? '')}

What changes do you think we should make in order to solve the problem?

We can use the same way used in other file i.e DistanceEReceipt
But in our case the BE for Search API doesn't return filename inside the receipt object which will be used to check if the file pdf we will get from BE for the thumbnail inside ReceiptUtils.getThumbnailAndImageURIs function.

So inside the ReceiptUtils.getThumbnailAndImageURIs function we can retrieve the filename receiptFilename from the url

What alternative solutions did you explore? (Optional)

Inside the TransactionListItemRow file we can retrieve the new transaction value from the Onyx by using the current transaction id i.e transaction_id, because the current transaction value from BE (Search API) that being pass to this function doesn't return with filename value inside receipt object

@luacmartins
Copy link
Contributor

PDFThumbnail file doesn't handle the thumbnail image properly and seems not working

Why is this the case?

@NJ-2020
Copy link
Contributor

NJ-2020 commented Aug 15, 2024

@luacmartins I don't know, when i see on other page like report page it showing white background seems not working

@AlfredoAlc
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Search - Receipt thumbnail becomes gray & blank after changing receipt from JPG to PDF file

What is the root cause of that problem?

The component that renders the thumbnail is Image from React Native, which doesn't support displaying PDF files.

What changes do you think we should make in order to solve the problem?

We'll need to modify this source prop to send the correct uri using the getThumbnailAndImageURIs() function.

source={tryResolveUrlFromApiRoot(transactionItem?.receipt?.source ?? '')}

We can apply the following logic, to check whether the file is PDF or not and pass the correct uri.

const filename = getFileName(transactionItem?.receipt?.source ?? '');
const receiptURIs = getThumbnailAndImageURIs(transactionItem, null, filename);
const isReceiptPDF = Str.isPDF(filename);
const source = tryResolveUrlFromApiRoot(isReceiptPDF ? receiptURIs.thumbnail ?? '' : receiptURIs.image ?? '');

@melvin-bot melvin-bot bot added the Overdue label Aug 17, 2024
@joekaufmanexpensify
Copy link
Contributor

@Pujan92 do you expect to be able to review the new proposal today?

Copy link

melvin-bot bot commented Aug 19, 2024

@Pujan92, @luacmartins, @joekaufmanexpensify Eep! 4 days overdue now. Issues have feelings too...

@joekaufmanexpensify
Copy link
Contributor

Bump @Pujan92 when you have a sec

@Pujan92
Copy link
Contributor

Pujan92 commented Aug 20, 2024

Sorry for the delay @joekaufmanexpensify, I will review it tomorrow.

@melvin-bot melvin-bot bot removed the Overdue label Aug 20, 2024
@Pujan92
Copy link
Contributor

Pujan92 commented Aug 21, 2024

@NJ-2020 @AlfredoAlc

I see the pdf thumbnail image correctly when I revisit the page, I think TransactionListItem isn't rerendering when the pdf gets uploaded. Could you plz confirm and update your proposal accordingly if that is the case?

Screen.Recording.2024-08-21.at.12.29.14.mov

@joekaufmanexpensify
Copy link
Contributor

As an fyi, going to be OOO from tomorrow - Monday. Not adding another BZ since should be nothing BZ-related till then. Please ask in slack if something comes up!

Copy link

melvin-bot bot commented Aug 21, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@AlfredoAlc
Copy link
Contributor

@Pujan92 Yes, when the receipt gets uploaded the file for that transaction gets updated with the new file which is a pdf. Pdf files are not rendered correctly from that Image component.

In that page we get the data via a snapshot, so when you revisit that page it gets a new snapshot which internally seems to apply a similar logic(as the proposal) to pdf files, and returns the transaction with a valid uri for the thumbnail.

Copy link

melvin-bot bot commented Sep 3, 2024

@Pujan92, @luacmartins, @joekaufmanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented Sep 4, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@joekaufmanexpensify
Copy link
Contributor

@Pujan92 @luacmartins Is any more discussion needed here before we can evaluate the proposals on the issue?

@luacmartins
Copy link
Contributor

I think my latest comment should resolve the issue, maybe we need some updated proposals to account for that.

Copy link

melvin-bot bot commented Sep 5, 2024

@Pujan92, @luacmartins, @joekaufmanexpensify Eep! 4 days overdue now. Issues have feelings too...

@joekaufmanexpensify
Copy link
Contributor

Got it. Sounds good! @AlfredoAlc @NJ-2020 could you please update your proposals?

@Pujan92
Copy link
Contributor

Pujan92 commented Sep 6, 2024

Thanks @luacmartins.

RCA from the @NJ-2020 seems incorrect as we aren't using PDFThumbnail for the TransactionListItemRow.
@AlfredoAlc's RCA is correct and the solution makes sense to retrieve filename from the URL and based on filetype replace the extension as per the requirement. We can proceed with @AlfredoAlc's proposal.

🎀👀🎀 C+ reviewed

@melvin-bot melvin-bot bot removed the Overdue label Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

Current assignee @luacmartins is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

📣 @AlfredoAlc You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 7, 2024
@joekaufmanexpensify
Copy link
Contributor

PR in review

@joekaufmanexpensify
Copy link
Contributor

PR merged

@luacmartins luacmartins mentioned this issue Sep 19, 2024
50 tasks
@joekaufmanexpensify
Copy link
Contributor

Looks like this one is due for payment but automation did not work.

@joekaufmanexpensify
Copy link
Contributor

I do see the second PR. Seems like that second PR was not a regression, is that right @luacmartins

@joekaufmanexpensify joekaufmanexpensify changed the title [$250] Search - Receipt thumbnail becomes gray & blank after changing receipt from JPG to PDF file [Hold for payment 2024-09-23] [$250] Search - Receipt thumbnail becomes gray & blank after changing receipt from JPG to PDF file Sep 23, 2024
@luacmartins
Copy link
Contributor

Yea, it was a side effect but didn't cause a blocker

@joekaufmanexpensify
Copy link
Contributor

Got it. TY!

@joekaufmanexpensify
Copy link
Contributor

No regression penalty here then. Payment we need to issue here is:

@joekaufmanexpensify
Copy link
Contributor

@AlfredoAlc $250 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

Closing as all that's left is for @Pujan92 to request payment via NewDot. TY everyone!

@garrettmknight
Copy link
Contributor

$250 approved for @Pujan92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Archived in project
Development

No branches or pull requests

7 participants