Handle errors for missing KBA data#68749
Conversation
aefb510 to
947b6bb
Compare
|
@mjasikowski 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] |
rafecolton
left a comment
There was a problem hiding this comment.
A few comments:
- I'm not sure if "private personal details" is a user-facing term or not (cc @jamesdeanexpensify)
- It looks like we are calling it
AccountnotSettings - I think it's a suboptimal experience to error here and require the user to go to a different page and back without either giving them a link or directing them to Profile using an RBR. Though I also don't want to expand the scope of this change. Would love some feedback from @Expensify/design on this one
|
I would say But one question - is there a specific field they need to update? Or what specifically do they need to do next? |
Ultimately they will need DOB, address, and phone number. So the updates required are one or more of those, whichever are missing. |
|
Got it - so it won't be "updating" those fields, it will be more like "adding"? |
|
Yes. For more context, for physical card activation, all those fields should be set already and hitting this error is an edge case. The user is most likely to see this when trying to activate an admin-issued virtual card |
|
Cool! I like this then, if you're good with it:
|
|
I do like it, though I'd like it even more if we could specify the fields i.e. |
|
Oh totally - I didn't know we could make it dynamic. Even better! |
|
If you can dream it, we can build it 😉 |
|
I feel like we should proactively put an RBR on Settings > Profile in this case, right? Like why wait until the user tries to reveal details, we should probably just let that user know ahead of time we need them to fill out information in their profile. Otherwise I like where you landed with copy suggestions. Other suggestions while we are here, because I've never actually seen these screens before... @MonilBhavsar can you show us a video of the whole flow? For this, we need some extra space under the headline and above the number input. I also feel like the animation is too small? For this, we could use some extra margin below the RBR message. |
|
For this copy you are suggest:
Can we make |
|
Agree with all of Shawn's comments above! Dig the copy.
How will we know when/if we should RBR profile? Once they order a card? I'm down to do something proactive if we know it's going to be an issue, just want to make sure we can know it's going to be an issue at the right time. |
|
I was thinking once they get assigned a card from the admin. |
|
Right on - that makes sense to me. I'm for it. |
I like it. Let me give it a try. @shawnborton here it is. The user has been issued a physical card and a virtual card by an admin and user is trying to activate physical card and reveal details of virtual card Screen.Recording.2025-08-21.at.10.08.46.PM.mov |
|
Would we just be adding the RBR to add personal details if they're issued a virtual card with no personal details set? Because we already collect the personal details in the flow to order a physical card, so i feel like it isn't really needed there. |
|
@DylanDylann can you do a final review please? |
rafecolton
left a comment
There was a problem hiding this comment.
@MonilBhavsar were you planning to address the lingering comments from @DylanDylann's prior review? Otherwise LGTM!
| // Update error state when error is cleared in Onyx DB | ||
| useEffect(() => { | ||
| setCardsDetailsErrors((prevErrors) => { | ||
| const clearedErrors = {...prevErrors}; | ||
| Object.keys(clearedErrors).forEach((cardID) => { | ||
| if (cardListErrors[cardID] && Object.keys(cardListErrors[cardID]).length > 0) { | ||
| return; | ||
| } | ||
| delete clearedErrors[Number(cardID)]; | ||
| }); | ||
| return clearedErrors; | ||
| }); | ||
| }, [cardListErrors]); |
There was a problem hiding this comment.
Would it benefit from a comment?
| } | ||
|
|
||
| if (response?.jsonCode === 404) { | ||
| // eslint-disable-next-line prefer-promise-reject-errors |
|
The new update looks good to me |
|
Merging as Rafe is ooo for two days |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.2.49-0 🚀
|
|
@MonilBhavsar QA team can't test this PR, we can't use EU/UK card. |
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.2.49-0 🚀
|
|
Yes, @joekaufmanexpensify has a card and should be able to help us 🙏 |
|
Yep, I will test in a bit 👍 |
|
tested on staging and worked well! |
|
@jamesdeanexpensify thank you! Can we check it off? |
|
@IuliiaHerets was that meant for me or @joekaufmanexpensify ? |
|
I think it was. I checked it off 👍 |
|
@jamesdeanexpensify sorry, tagged you by mistake |
|
All good! |
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.2.52-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.2.54-1 🚀
|
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.2.52-0 🚀
|


Explanation of Change
Fixed Issues
Part of https://github.com/Expensify/Expensify/issues/524583
PROPOSAL:
Tests
Pre requisite: An EU/UK bank account on EU/UK workspace so EU/UK virtual card can be issued
Offline tests
QA Steps
Regression testing
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./** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Screenshots/Videos
Android: Native
See MacOS: Chrome / SafariAndroid: mWeb Chrome
See MacOS: Chrome / SafariiOS: Native
iOS: mWeb Safari
See MacOS: Chrome / SafariMacOS: Chrome / Safari
Screen.Recording.2025-11-04.at.11.48.10.PM.mov
MacOS: Desktop
See MacOS: Chrome / Safari