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

Bank Account - User does not land back on the country selector after closing the plaid UI. #54737

Closed
4 of 8 tasks
mitarachim opened this issue Jan 2, 2025 · 17 comments
Closed
4 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering

Comments

@mitarachim
Copy link

mitarachim commented Jan 2, 2025

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: v9.0.80-2
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: #52322
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Mac 14.5/ Chrome
App Component: Other

Action Performed:

  1. Login to NewDot.
  2. Go to Settings > Wallet > Add bank account.
  3. Click Add bank account.
  4. Select United States.
  5. Click Next.
  6. Click the X on the Plaid UI.

Expected Result:

User land back on the country selector with United States selected.

Actual Result:

User lands on wallet page

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6704913_1735779064823.Screen_Recording_2025-01-02_at_3.28.24_at_night.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @slafortune
@mitarachim mitarachim added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 2, 2025
Copy link

melvin-bot bot commented Jan 2, 2025

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

Copy link

melvin-bot bot commented Jan 2, 2025

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

Copy link

melvin-bot bot commented Jan 2, 2025

💬 A slack conversation has been started in #expensify-open-source

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Jan 2, 2025
Copy link
Contributor

github-actions bot commented Jan 2, 2025

👋 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.

@shubham1206agra
Copy link
Contributor

@MonilBhavsar Assign this to me for follow-up fix. Bad merge happened from #50228

@daledah
Copy link
Contributor

daledah commented Jan 2, 2025

Edited by proposal-police: This proposal was edited at 2025-01-02 08:24:55 UTC.

Proposal

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

User lands on wallet page

What is the root cause of that problem?

After closing the plaid UI in this case => topMostCentralPane?.name will be Settings_Wallet => User lands on wallet page

const goBack = useCallback(() => {
switch (topMostCentralPane?.name) {
case SCREENS.SETTINGS.WALLET.ROOT:
Navigation.goBack(ROUTES.SETTINGS_WALLET, true);
break;
case SCREENS.REPORT:
Navigation.closeRHPFlow();
break;
default:
Navigation.goBack();
break;
}
}, [topMostCentralPane]);

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

We can remove this condition here to run default function navigate.goBack() when closing the plaid UI

case SCREENS.SETTINGS.WALLET.ROOT:
Navigation.goBack(ROUTES.SETTINGS_WALLET, true);
break;

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

NA

What alternative solutions did you explore? (Optional)

OR we can change goBack function here to navigate.goBack()

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@shubham1206agra
Copy link
Contributor

@Beamanator Please assign @daledah here.

@daledah
Copy link
Contributor

daledah commented Jan 2, 2025

I'll raise PR soon

@isagoico
Copy link

isagoico commented Jan 6, 2025

We can't try to reproduce anymore with the revert #54729 as the same flow is not available.

@shubham1206agra
Copy link
Contributor

@isagoico Please close this issue.

@puneetlath puneetlath removed the DeployBlockerCash This issue or pull request should block deployment label Jan 6, 2025
@daledah
Copy link
Contributor

daledah commented Jan 8, 2025

@Beamanator @puneetlath should we process payment here, I was assigned to this issue and have a PR here.

@puneetlath puneetlath reopened this Jan 9, 2025
@Beamanator
Copy link
Contributor

Mmmm I haven't dealt with this case in a while - maybe @mallenexpensify can help - while we did do a revert that replaced your PR, we possibly should still compensate at least something for your time working on the PR - because it looks like you did fill out the whole checklist and do the videos before we decided to revert the main PR which made your PR null 🤔

@Beamanator Beamanator removed the Reviewing Has a PR in review label Jan 10, 2025
@mallenexpensify
Copy link
Contributor

From what I gather (there was a big PR we reverted, and by reverting that PR then the PR that @daledah worked on isn't needed). Assuming that's correct, @daledah is due compensation cuz they were hired, did the work and it was our choice that we didn't want the PR anymore). Per our main payment SO

  • 50% payment to contributor and C+ if the contributor was 🎀👀🎀 by a C+ or hired/assigned.
  • 100% due if contributor drafted a PR and requested a review.
  • 100% due to C+ if they reviewed the PR.

@daledah requested a review of the PR so they're due 100%
@shubham1206agra had yet to review the PR so they're due 50%

@slafortune can you manage payments plz? Thx

@mallenexpensify mallenexpensify added Daily KSv2 and removed Weekly KSv2 labels Jan 11, 2025
@slafortune
Copy link
Contributor

@daledah offer sent https://www.upwork.com/nx/wm/offer/105714004
@shubham1206agra offer sent - https://www.upwork.com/nx/wm/offer/105714017

Copy link

melvin-bot bot commented Jan 16, 2025

@Beamanator @slafortune @shubham1206agra @daledah this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@slafortune
Copy link
Contributor

@shubham1206agra Paid 👍

@slafortune
Copy link
Contributor

@daledah paid too 👍

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. Daily KSv2 Engineering
Projects
None yet
Development

No branches or pull requests

8 participants