[C+ Checklist Needs Completion] [QBO Export] [$250] iOS mWeb: External links do not work from popover menus

[arosiclair]

Version Number: N/A
Reproducible in staging?: Y
Reproducible in production?: Y
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): N/A
Logs: N/A
Expensify/Expensify Issue URL:
Issue reported by: @kosmydel
Slack conversation: https://expensify.slack.com/archives/C036QM0SLJK/p1721049986353069?thread_ts=1720453976.209939&cid=C036QM0SLJK

Action Performed:

1. On iOS mWeb, log in as a policy admin with a Xero/QBO connection
2. Avatar > Workspaces > {policy} > Connections > 3 dot menu > Sync Now

Expected Result:

A window is opened that links to the integration

Actual Result:

Nothing happens and the link doesn't work

Workaround:

N/A

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

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-07-15.at.15.17.01.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d607e7cf649f3935
• Upwork Job ID: 1812925016452625460
• Last Price Increase: 2024-07-15
• Automatic offers:
• shubham1206agra | Reviewer | 103137581

Issue Owner

Current Issue Owner: @sonialiap

[melvin-bot]

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

[arosiclair]

Info from @kosmydel:

Hey, during the development of integrations reconnection, I encountered a problem with links to external pages called from the three-dot menu. On the mWeb Safari it is impossible to open external links from the PopoverMenu (ThreeDotsMenu). After some investigation, the root cause of it is that we call the onSelected function from onModalHide. The iOS safari, to open a new window, requires us to do that in the onClick event. That change would require more investigation, and it would be nice to do that in a separate PR, as it might break some things. Could we create a new GH issue for that (someone from SWM will be able to handle it), so I can proceed with my PR. Here is a minimal reproducible example.

[melvin-bot]

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

[melvin-bot]

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

[arosiclair]

@war-in can you comment so I can assign you? Thanks

[arosiclair]

@kabeer95 please do not post proposals on issue that do not have Help Wanted label

[kabeer95]

@arosiclair Okay thanks for Informing.

[war-in]

@war-in can you comment so I can assign you? Thanks

@arosiclair @zfurtak is going to take care of this issue :) Please assign her here 🙏

[zfurtak]

hello, I'm also from swm agency 😊

[melvin-bot]

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

Offer link
Upwork job

[zfurtak]

Hello there! I've been looking into this recently and I've made a few discoveries 👩🏽‍💻
Currently we're not able to open an external link, when clicking on items in the PopoverMenu because Safari browser requires a direct interaction from the user to open a popover. This is because the function is called in onHideModal.
However, when I tried to move this call to the onPress function some strange things started to happen. For example on iOS some modals open with height: 0 and width: 0 because the PopoverMenu doesn't have enough time to close, also some of the buttons don't respond.
I've been looking through all the usages of PopoverMenu in the codebase and testing different approaches, but so far I haven't found the right, universal one.
I'm still searching 🕵️‍♀️

[melvin-bot]

@arosiclair, @sonialiap, @shubham1206agra, @zfurtak Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.16-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix(PopoverMenu): fix opening external links #45762

If no regressions arise, payment will be issued on 2024-08-13. 🎊

For reference, here are some details about the assignees on this issue:

• @shubham1206agra requires payment automatic offer (Reviewer)
• @zfurtak does not require payment (Contractor)
• @kosmydel does not require payment (Contractor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR:
• [@shubham1206agra] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@shubham1206agra] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@shubham1206agra] Determine if we should create a regression test for this bug.
• [@shubham1206agra] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[arosiclair]

I think #46852 is the only regression that still needs to be addressed after these changes.

[sonialiap]

@arosiclair sounds like this is not ready for payment and we're waiting on #46852, is that right?

[arosiclair]

I don't think we need to block payment on that issue. Just account for the regressions that were caught from this one.

[sonialiap]

Payment summary:
@shubham1206agra $125 due to regression - orig offer expired, sent new offer in upwork - please complete the checklist and accept offer

[shubham1206agra]

@sonialiap Accepted offer

[sonialiap]

@shubham1206agra paid, please complete the checklist ;)

[melvin-bot]

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

[sonialiap]

@greg-schroeder I'm OOO Aug 19-30, adding leave buddy.
Status: waiting for @shubham1206agra to complete the checklist, then submit regression test to QA

[melvin-bot]

@arosiclair, @sonialiap, @greg-schroeder, @shubham1206agra, @zfurtak Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[greg-schroeder]

bump @shubham1206agra for checklist

[shubham1206agra]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR: NA
• [@shubham1206agra] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: NA
• [@shubham1206agra] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: Not Required
• [@shubham1206agra] Determine if we should create a regression test for this bug. Yes
• [@shubham1206agra] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

1. Log in as a policy admin with a Xero/QBO connection
2. Click on Avatar > Workspaces > {policy} > Connections > 3 dot menu > Sync Now.
3. Verify that an external link opens.

• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

@arosiclair, @sonialiap, @greg-schroeder, @shubham1206agra, @zfurtak Eep! 4 days overdue now. Issues have feelings too...

[greg-schroeder]

Filed regression test, closing