[HOLD for payment Oct 4][$250] Attachment - Error not displayed when trying to upload corrupted pdf file to chat

[lanitochka17]

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

---

Version Number: 9.0.26-1
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

Precondition: user should be Signed In

1. Open app
2. Navigate to any chat
3. Tap on plus button
4. Select Add attachment option
5. Upload this corrupted pdf file corrupted.pdf

Expected Result:

"Attachment error" pop-up should be displayed

Actual Result:

Keyboard overlaps the attachment error pop-up and error pop-up is not visible

Workaround:

Unknown

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

Bug6586335_1724924980382.RPReplay_Final1724924944.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a722d0e57042bb6b
• Upwork Job ID: 1829184455392581969
• Last Price Increase: 2024-09-12
• Automatic offers:
• dominictb | Contributor | 103991859

Issue Owner

Current Issue Owner: @Christinadobrzyn

[melvin-bot]

Triggered auto assignment to @srikarparsi (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[srikarparsi]

Making this external to get some eyes

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a722d0e57042bb6b

[melvin-bot]

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

[srikarparsi]

This PR is related to the attachment modal

[srikarparsi]

This seems to only be happening on iOS, tested mWeb chrome and safari and it's not reproducible

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (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.

[muratti32]

@srikarparsi It worked correctly on iOS 17.4. Could you please specify the iOS version you encountered issues with?

Screen.Recording.2024-08-29.at.23.43.38.mov

[melvin-bot]

📣 @muratti32! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[muratti32]

Contributor details
Your Expensify account email:[email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~0133345a239098ea13

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[dominictb]

Edited by proposal-police: This proposal was edited at 2024-08-29 23:59:32 UTC.

Proposal

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

Keyboard overlaps the attachment error pop-up and error pop-up is not visible

What is the root cause of that problem?

• When the PDF file encountered an error because it is corrupted file, we close the modal:
  

  App/src/components/AttachmentModal.tsx

  Line 566 in 6d70b10

  setIsModalOpen(false);

hence its onModalHide is called, which call:

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx

Line 301 in 6d70b10

restoreKeyboardState();

hence the keyboard is displayed.

• Then we show the error modal:
  

  App/src/components/AttachmentModal.tsx

  Line 499 in 6d70b10

  setIsAttachmentInvalid(true);

  
  but now that modal is displayed behind the keyboard.

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

• We need to make sure when the pdf file encountered an error, it will:

Close attachment modal > Open error modal > Call onModalHide function

currently, the order is:

Close attachment modal > Call onModalHide function > Open error modal

• To do it:

App/src/components/AttachmentModal.tsx

Line 495 in 6d70b10

onModalHide();

should be:

                    if (!isPDFLoadError.current) {
                        onModalHide();
                    }

It will make sure if there is a pdf error, we don't call onModalHide, in other words, don't call the function to focus on the composer input restoreKeyboardState().

• And in here:
  

  App/src/components/AttachmentModal.tsx

  Line 614 in 6d70b10

  <ConfirmModal

  
  add a new prop

                    onModalHide={() => {
                        if (isPDFLoadError.current) {
                            isPDFLoadError.current = false;
                            onModalHide?.();
                        }
                    }}

It will call onModalHide?.(), in other words, call the function to focus on the composer restoreKeyboardState().

• Finally, remove:
  

  App/src/components/AttachmentModal.tsx

  Line 498 in 6d70b10

  isPDFLoadError.current = false;

• Because the AttachmentModal is used in a lot of positions in app, so we need to create a new flag prop and only apply the above changes if that flag is true to make sure it does not affect others.

What alternative solutions did you explore? (Optional)

[thesahindia]

@dominictb's proposal looks good to me!

There is only one issue that the keyboard appears on the screen for a brief moment just after the attachment modal closes. I think it will be hard to fix that (not sure).

Screen.Recording.2024-09-13.at.12.45.52.AM.mov

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[thesahindia]

There is only one issue that the keyboard appears on the screen for a brief moment just after the attachment modal closes. I think it will be hard to fix that (not sure).

@dominictb, could you look into it and share your thoughts?

[dominictb]

There is only one issue that the keyboard appears on the screen for a brief moment just after the attachment modal closes. I think it will be hard to fix that (not sure).

@thesahindia I cannot reproduce this behavior when applying my solution. However, will try to reproduce it and fix in PR phase.

Screen.Recording.2024-09-13.at.16.02.39.mov

[thesahindia]

Let's assign @dominictb.

[alexpensify]

@srikarparsi, we need your review; thanks!

[Christinadobrzyn]

Nudged @srikarparsi for a review when online!

[srikarparsi]

Sorry for the delay, assigning @dominictb

[melvin-bot]

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[Christinadobrzyn]

just a heads up that I'm going to be ooo until 9/30. I'm not going to assign a BZ teammate since we're working on the PR/discussing the issue. If you need someone before Monday please reach out in Slack for a volunteer

[dominictb]

@Christinadobrzyn PR on production 2 days ago. Should add [HOLD for payment Oct 4].

[Christinadobrzyn]

Payouts due:

• Contributor: $250 @dominictb (Paid in Upwork - https://www.upwork.com/nx/wm/workroom/38495529/overview)
• Contributor+: $250 @thesahindia (you're paid in NewDot, yes?)

Do we need a regression test?

[thesahindia]

(you're paid in NewDot, yes?)

Yep!

[thesahindia]

Here are the steps for the test case:-

1. On native app go to any chat
2. Tap on composer to open the keyboard
3. Tap on the "+" icon > "Add attachment"
4. Select a corrupt pdf
5. Verify you see the error modal and it's not hidden behind the keyboard

[Christinadobrzyn]

• Regression test - https://github.com/Expensify/Expensify/issues/433275

• Payment summary

Thanks @thesahindia - please submit a payment via NewDot. TY!
@dominictb I've paid you out in Upwork

[Christinadobrzyn]

I DMd @thesahindia reminding them to request payment. We'll keep it open for a bit longer to make sure @thesahindia sees it.

[thesahindia]

Thanks for the reminder! I have added it to my list. Feel free to close it.

[garrettmknight]

$250 approved for @thesahindia