[HOLD for payment 2024-12-07] Amex direct feeds polish

[joekaufmanexpensify]

Connecting to Amex is now working, but there are some quirks we need to iron out. Namely:

• We aren't auto-closing the RHP after successfully connecting via oAuth (This also happened with Wells Fargo, so let's fix it everywhere if we can).
• The feed name is undefined cards (should be American Express cards.)
• Card number field on the card list is incorrectly blank (but appears correctly on the card details page, workspace member profile, and wallet page).

cc @mountiny @VickyStash

Issue Owner

Current Issue Owner: @isabelastisser

[melvin-bot]

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

[joekaufmanexpensify]

@VickyStash please comment when you have a sec and I'll assign you

[ChavdaSachin]

I would like to work on this issue, and would raise a quick pr if needed.

Thank you.

[joekaufmanexpensify]

@ChavdaSachin Appreciate your interest! This one is not looking for volunteers right now. You can find issues that are looking for volunteers by searching for the Help wanted label, per our contributing guidelines.

[VickyStash]

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

[VickyStash]

Note: The last 4 numbers are displayed now, so the card number issue is not relevant.

[VickyStash]

I've fixed the undefined in the name issue and found the reason for RHP not closing.

Reason of RHP not closing:
When the connection is successful, the app tries to set the new connection feed as the last selected one and navigates the user back to the cards list (closes RHP).

App/src/pages/workspace/companyCards/addNew/BankConnection/index.tsx

Lines 76 to 81 in bb4c89d

	if (feedName && connectedBank && !isEmptyObject(connectedBank)) {
	customWindow?.close();
	Card.updateSelectedFeed(feedName, policyID ?? '-1');
	Navigation.navigate(ROUTES.WORKSPACE_COMPANY_CARDS.getRoute(policyID ?? '-1'));
	return;
	}

But as you can see there is a check for the feedName - it's actually just feed (ex. oauth.citibank.com), and a check for connected bank which relays on the feed value as well.

The thing is the app tries to predict the feed and renavigate user to that feed after successful connection. So for example if user selected American Express the app will think that the feed which should be set as selected is oauth.americanexpressfdx.com. But it's turned out it also can be oauth.americanexpressfdx.com 2003 for example. So the checks doesn't work and RHP is not closing.

In the video provided by Joe we see the the new feed turned out to be selected, but it's just because it's the only option, and the app selects first feed if the lastSelectedFeed key is empty.

Possible solution:

1. After the successful connection navigate to the first related feed. For example if user connecting to American Express navigate to the first feed in the list which starts with oauth.americanexpressfdx.com. Cons: it can select the wrong one if there are several connections to the same bank.
2. Store the lastSelectedFeed onyx key in the BE and return it inside the Onyx data after the successful connection. This way the app shouldn't predict the feed and will know it for sure. But this approach will require more updates.

@mountiny @allgandalf Please, let me know if you have any other ideas or any thoughts.

[mountiny]

Yeah I would do:

After the successful connection navigate to the first related feed. For example if user connecting to American Express navigate to the first feed in the list which starts with oauth.americanexpressfdx.com. Cons: it can select the wrong one if there are several connections to the same bank.

For now and we are going to work on adding a new special onyx key that will indicate the feed being added similar to accounting connection being made. cc @DylanDylann but we would add that later

[allgandalf]

Agree with what @mountiny said ^

[VickyStash]

The PR has been opened for the review! @mountiny @allgandalf
I've changed a little bit the approach with re-navigation, cause getting the first feed in the list which starts with expected feed had some side effects as well. It would be great to test the PR with the ad-hoc builds.

[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.68-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Amex direct feeds polish #53122

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

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

• @VickyStash does not require payment (Contractor)
• @allgandalf requires payment (Needs manual offer from BZ)

[melvin-bot]

@allgandalf @isabelastisser @allgandalf 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]

[allgandalf]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: https://github.com/Expensify/App/pull/49928/files#r1865473583

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

No regression test here, this issue is part of a project, Tests will be added at the end of it.

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

Test:

Do we agree 👍 or 👎

[allgandalf]

Lets close this one @isabelastisser no payments due here, I will be paid in bulk at the end of the project