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-12-20] [$250] Screen briefly flashes Chase feed page after connecting Amex feed and back to expensify #53510

Closed
2 of 8 tasks
m-natarajan opened this issue Dec 4, 2024 · 18 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Dec 4, 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:
Reproducible in staging?:
Reproducible in production?:
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @joekaufmanexpensify
Slack conversation (hyperlinked to channel name): ts_external_expensify_expense

Action Performed:

  1. Enable company card in workspace
  2. Click Add card and choose Amex bank
  3. Connect to Amex using Direct feed
  4. Click Return to Expensify

Expected Result:

User returned to company cards page

Actual Result:

Screen briefly flashes Chase feed page

Workaround:

Unknown

Platforms:

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

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
2024-12-02_16-35-27.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021866127944349300492
  • Upwork Job ID: 1866127944349300492
  • Last Price Increase: 2024-12-09
Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @joekaufmanexpensify
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 4, 2024
Copy link

melvin-bot bot commented Dec 4, 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.

@dukenv0307
Copy link
Contributor

I already worked on company card project so I would like to take it as C+

@joekaufmanexpensify
Copy link
Contributor

We're discussing in the thread that maybe this will be fixed elsewhere

@DylanDylann
Copy link
Contributor

@mountiny @joekaufmanexpensify I tried to reproduce this bug from my side. I used a mock response from here. Let's see this video

Screen.Recording.2024-12-06.at.23.45.19.mov

When clicking mock data (it means that when BE returns data) the App opens the bank connection URL again. The RCA of this problem:

useEffect(() => {
if (!url) {
return;
}
if (isNewFeedConnected) {
customWindow?.close();
if (newFeed) {
Card.updateSelectedFeed(newFeed, policyID ?? '-1');
}
Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policyID ?? '-1'));
return;
}
if (isBankConnectionCompleteRoute) {
customWindow?.close();
return;
}
customWindow = openBankConnection(url);
}, [isNewFeedConnected, newFeed, isBankConnectionCompleteRoute, policyID, url]);
return (
<ScreenWrapper testID={BankConnection.displayName}>

When isNewFeedConnected is true, we close the connection URL and navigate to the company card page. But this useEffect is triggered again before navigating --> the URL is opened again. You guys can see my log on the above video for more detail

I suspect that this was caused by this PR
cc @allgandalf @VickyStash

@VickyStash
Copy link
Contributor

Hi, I'm Viktoryia from Callstack - expert contributor group - and I would like to work on this issue.

@VickyStash
Copy link
Contributor

Not overdue, staring to work on this one.

@melvin-bot melvin-bot bot removed the Overdue label Dec 9, 2024
@VickyStash
Copy link
Contributor

When clicking mock data (it means that when BE returns data) the App opens the bank connection URL again.

@DylanDylann I've tested your flow and I guess in your case the window with URL is opened again cause this check in the useEffect doesn't work for you, because you used the mocked data:

if (isBankConnectionCompleteRoute) {
customWindow?.close();
return;
}

const isBankConnectionCompleteRoute = currentUrl.includes(ROUTES.BANK_CONNECTION_COMPLETE);

So there should be no problem like this for the real users, as they will have isBankConnectionCompleteRoute true. Let me know what you think!

But let me try to check what can cause the bug initially reported in this issue.

@VickyStash
Copy link
Contributor

I was able to reproduce it on ios without even connecting to the banks, with the next steps:

  1. start the feed setup -> select Chase bank -> wait for webview open -> go back
  2. start the feed setup -> select Amex bank -> wait for webview open -> go back -> for a ms (really quickly, watch the attached video in slow motion) you will see the Chase login page
shortened_video.mp4

@joekaufmanexpensify joekaufmanexpensify added the External Added to denote the issue can be worked on by a contributor label Dec 9, 2024
@melvin-bot melvin-bot bot changed the title Screen briefly flashes Chase feed page after connecting Amex feed and back to expensify [$250] Screen briefly flashes Chase feed page after connecting Amex feed and back to expensify Dec 9, 2024
Copy link

melvin-bot bot commented Dec 9, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 9, 2024
Copy link

melvin-bot bot commented Dec 9, 2024

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

@joekaufmanexpensify joekaufmanexpensify removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 9, 2024
@DylanDylann
Copy link
Contributor

@VickyStash Thanks for your information. Just want to confirm this point:

So there should be no problem like this for the real users, as they will have isBankConnectionCompleteRoute true. Let me know what you think!

I think currentUrl will not change when connecting successful. isBankConnectionCompleteRoute seems outdated now. More detail here

@VickyStash
Copy link
Contributor

I think I was able to get rid of the screen flashing if I replace Modal with ScreenWrapper here, but trying to know why we needed Modal here:

<Modal
onClose={handleBackButtonPress}
fullscreen
isVisible={isWebViewOpen}
type={CONST.MODAL.MODAL_TYPE.CENTERED_UNSWIPEABLE}
>

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Dec 10, 2024
@VickyStash
Copy link
Contributor

The PR has been opened for the review!

@joekaufmanexpensify
Copy link
Contributor

Woooo, TY!

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 13, 2024
@melvin-bot melvin-bot bot changed the title [$250] Screen briefly flashes Chase feed page after connecting Amex feed and back to expensify [HOLD for payment 2024-12-20] [$250] Screen briefly flashes Chase feed page after connecting Amex feed and back to expensify Dec 13, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 13, 2024
Copy link

melvin-bot bot commented Dec 13, 2024

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

Copy link

melvin-bot bot commented Dec 13, 2024

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

If no regressions arise, payment will be issued on 2024-12-20. 🎊

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

Copy link

melvin-bot bot commented Dec 13, 2024

@DylanDylann @joekaufmanexpensify @DylanDylann The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@mountiny
Copy link
Contributor

No regression test is required imho, there was not a specific PR that would cause this issue its was a design fault. pyaments will be in total for the project

@github-project-automation github-project-automation bot moved this from Bugs and Follow Up Issues to Done in [#whatsnext] #expense Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
Status: Done
Development

No branches or pull requests

6 participants