Hide Pex from bank options until it's implemented in ND#82836
Hide Pex from bank options until it's implemented in ND#82836carlosmiceli wants to merge 2 commits intomainfrom
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@dominictb Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a27d35e994
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (bank === CONST.COMPANY_CARDS.BANKS.PEX) { | ||
| return false; |
There was a problem hiding this comment.
Clear hidden PEX selection before rendering bank list
Filtering PEX out of data here hides the row, but it does not clear bankSelected, which is sourced from persisted Onyx state. If a user had previously selected PEX (e.g., before this rollout) and lands on this step again, no option appears selected, yet submit() still treats bankSelected as valid and advances with PEX, so the unsupported flow remains reachable despite being hidden.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Implemented in d5ac28e. Added a useEffect that checks the persisted selectedBank from Onyx — if it's PEX, it clears the selection by calling setAddNewCompanyCardStepAndData({data: {selectedBank: null}}). This ensures users who previously selected PEX won't silently advance through the unsupported flow.
If a user had previously selected PEX before it was hidden, the persisted Onyx state would still hold PEX as the selected bank. This allowed advancing through the unsupported flow despite the option being hidden from the list. Add a useEffect to clear the selection when the persisted value is PEX. Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com>
|
@carlosmiceli Please reassign this |
|
I can review as C+ |
Let's wait for @chrispader to chime in, since the fix seems like it's not as simple as originally considered. |
| const isOtherBankSelected = bankSelected === CONST.COMPANY_CARDS.BANKS.OTHER; | ||
|
|
||
| useEffect(() => { | ||
| if (addNewCard?.data.selectedBank !== CONST.COMPANY_CARDS.BANKS.PEX) { |
There was a problem hiding this comment.
Also add a comment explaining why we exclude PEX here please 🙌🏼
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
Unit Tests failing. Please merge main |
|
We should hold on reviewing/testing further, @chrispader is going through some investigation notes to fix this properly 🙇 |
|
PR with holistic fix after my recent refactoring is up here: #83192 |
|
Closing on favor of Chris' PR. |
Explanation of Change
Hiding Pex until it's properly implemented.
Context: https://expensify.slack.com/archives/C049HHMV9SM/p1771415869720979?thread_ts=1771415601.716659&cid=C049HHMV9SM
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/601495
PROPOSAL:
Tests
Offline tests
QA Steps
Go to a workspace → Company cards → Add new cards → Who's your bank?
Verify Pex is no longer an option.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari