[Awaiting Payment 26th Jane] [$500] PDF receipt doesn't load in the receipt modal

[trjExpensify]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

This was fixed here, but broke again as per here.

As for the PDF receipt thumbnails, that's being worked on in this issue.

Version Number: v1.4.15-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @trjExpensify
Slack conversation:

Action Performed:

1. Request Money > Scan
2. Upload a PDF receipt > confirm
3. Click the report preview component to navigate to the expense report
4. Click the request preview component to navigate to the transaction thread
5. Click the thumbnail of the receipt to access the receipt modal

Expected Result:

PDF image should load

Actual Result:

We get a "failed to load" error

Workaround:

None (well, go to OldDot and you can see it if it's an expenseReport

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015df189659b56a1d4
• Upwork Job ID: 1737830160739762176
• Last Price Increase: 2023-12-21
• Automatic offers:
• namhihi237 | Contributor | 28070111

[melvin-bot]

Current assignee @trjExpensify is eligible for the Bug assigner, not assigning anyone new.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015df189659b56a1d4

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Current assignee @sobitneupane is eligible for the External assigner, not assigning anyone new.

[trjExpensify]

CC: @Talha345 from here: #27680 (comment)

[Talha345]

@trjExpensify Should I open a PR?

[trjExpensify]

Sure! Can you quickly recap the plan of action though on this issue and @sobitneupane can give it a quick 👀

[Talha345]

@trjExpensify Sure! I will do that in a few hours.

Can you please assign the issue to me in the meantime?

[trjExpensify]

We'll need to see the proposal first before assigning out the issue sorry!

[namhihi237]

Proposal

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

PDF image should load when open the receipt modal

What is the root cause of that problem?

Currently we isLocalFile has true while we load the receipt from the S3, so the isAuthTokenRequired is false

App/src/components/ReportActionItem/ReportActionItemImage.js

Line 87 in b966bd4

isAuthTokenRequired={!isLocalFile}

We set true here

App/src/libs/ReceiptUtils.ts

Line 69 in b966bd4

return {thumbnail: image, image: path, isLocalFile: true};

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

We should update to check if is local file use

  const isLocalFile = path.startsWith('blob:') || path.startsWith('file:');
    return {thumbnail: image , image: path, isLocalFile};

What alternative solutions did you explore? (Optional)

N/A

Result:

Screen.Recording.2023-12-21.at.21.00.52.mov

[sobitneupane]

@namhihi237 I don't think the change you proposed will be enough. Instead of thumbnail, loading Indicator is displayed in money request view and preview.

[Talha345]

Proposal

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

PDF image is not loading in the receipt modal.

What is the root cause of that problem?

In getThumbnailAndImageURIs method, we are passing isLocalFile: true always.

App/src/libs/ReceiptUtils.ts

Line 69 in 880254c

return {thumbnail: image, image: path, isLocalFile: true};

While this works till the time the PDF file is being uploaded to the server, once uploaded the file is no longer a local file and therefore requires authentication to get the file from the server which can be seen as follows:

App/src/components/ReportActionItem/ReportActionItemImage.js

Line 87 in 880254c

isAuthTokenRequired={!isLocalFile}

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

We can simply check if a file is local or not by making the following change:

return {thumbnail: image, image: path, isLocalFile: path.startsWith('blob:') || path.startsWith('file:')};

What alternative solutions did you explore? (Optional)

N/A

Result:

Screen.Recording.2023-12-21.at.19.46.33.mp4

[namhihi237]

@sobitneupane what do you mean, currently, the thumbnail always shows with PDF

[sobitneupane]

@Talha345 Your proposal is not much different from initial proposal (before edit) by @namhihi237.

@namhihi237 Please add a comment following the guideline after updating proposal.

Sorry for the confusion here. Loading Indicator was being displayed instead of Thumbnail, previously. But, after clearing the cache, the issue is not reproducible any longer.

Proposal from @namhihi237 looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @MonilBhavsar, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Talha345]

@sobitneupane During the proposal review for #27680, I had the same exact proposal as the one that was chosen but you told me that you chose the other proposal because I incorporated a fix later on in my proposal (which was kind of like a small regression fix that occurred after implementing the proposed solution) and therefore my proposal even though it came early was incomplete.

Now, here you mention that my proposal is not much different from @namhihi237's original proposal but obviously when that proposal was posted it was incomplete and that's why I posted a complete solution to avoid any missing case.

There should be some standardisation in this process TBH. Also, I think in this case I volunteered earlier to work on this issue because as I was already familiar with these changes.

Also, apologies for being a bit blunt but the selected proposal was edited multiple (6) times so I do not understand to which version my proposal is not much different from. If it was not much different, there should not have the need for such multiple edits.

[sobitneupane]

During the proposal review for #27680, I had the same exact proposal as the one that was chosen but you told me that you chose the other proposal because I incorporated a fix later on in my proposal (which was kind of like a small regression fix that occurred after implementing the proposed solution) and therefore my proposal even though it came early was incomplete.

@Talha345 Regarding previous issue, the difference between the two proposals was significant enough to go with the other.

Now, here you mention that my proposal is not much different from @namhihi237's original proposal but obviously when that proposal was posted it was incomplete and that's why I posted a complete solution to avoid any missing case.

Regarding this issue, two proposals doesn't look significantly different to me. If @MonilBhavsar thinks other wise, I am happy to go with your proposal.

Also, apologies for being a bit blunt but the selected proposal was edited multiple (6) times so I do not understand to which version my proposal is not much different from. If it was not much different, there should not have the need for such multiple edits.

This was the proposal I initially reviewed

[Talha345]

@sobitneupane Okay, I would be happy to accept what @MonilBhavsar's judgement here, but it is also important to note that the Expensify docs mention that you are supposed to put an "Updated Proposal" comment whenever you make a change to an existing proposal which was also not followed in this case.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[sobitneupane]

@namhihi237 If you are around, can you please handle above cases?

[namhihi237]

Sure I will raise this PR

[sobitneupane]

Let's make sure to add tests covering all the Attachment Views.

[namhihi237]

@sobitneupane PR already here #34030

[parasharrajat]

PR of this issue caused a regression #34049 cc @trjExpensify

[trjExpensify]

Ah, is it the same root being fixed here?

[sobitneupane]

Yup @trjExpensify. The issue has been addressed in #34030.

[trjExpensify]

Cool, so we'll count that as the one then!

[dylanexpensify]

Nice, should we close this?

[trjExpensify]

Going to close, if someone wants to let me know otherwise -- great!

[namhihi237]

@trjExpensify seem we missing payment

[trjExpensify]

Oh sorry, I totally thought there was another issue for this one!

[trjExpensify]

Okay, so payments..

1 regression to account for, as the second regression had the same root cause.

• @sobitneupane $250 for the C+ review
• @namhihi237 $250 for the fix

Correct?

[MonilBhavsar]

Looks good! Let's pay

[trjExpensify]

Cool, thanks for confirming!

• @namhihi237 $250 for the fix - paid!

[trjExpensify]

@sobitneupane you're free to request $250 as per here. Closing!

[JmillsExpensify]

$250 approved for @sobitneupane based on summary above.