[Internal QA]: More Plaid import follow ups#64741
Conversation
|
@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] |
|
@narefyev91 Could you kindly merge |
Reviewer Checklist
Screenshots/Videos |
|
@joekaufmanexpensify One small question: If I was assigned a card by another workspace which belongs to another feed, should I show that feed option in card filters page (i.e., the |
|
@joekaufmanexpensify One small suggestion is that we could show the card name and card feed name following the same format with Expensify card: |
|
@dominictb fixed: |
|
Code looks good and tests well for now. |
Is there a reason we wouldn't? Doesn't the filter show the feed in the "Card feeds" section whenever you're assigned at least one card on a feed? And tapping it selects all your assigned cards from that feed (which can be just one)? |
I'm not opposed to putting the card name there for third party cards. Though, we'd probably need to truncate if it is long. There are some situations where we don't use card name for Expensify Cards, like for domain cards from OldDot. But we do always seem to for workspace Expensify Cards. So adding that here would be consistent. WDYT @Expensify/design ? The second item you're referencing for Expensify Cards is not the feed name. It looks like it is the card type, ie if it's virtual or physical. That is N/A for third party cards. |
|
Also, once this one is reviewed, let's test it on an ad-hoc build before we merge 👍 |
Always down for consistency! |
|
Cool cool. @narefyev91 @dominictb could we go ahead and add the card name for third party cards here then? |
Yeah will add today |
|
|
@koko57 Ideally, no. In what context are we showing the feed name like that? |
Circling back to this, I chatted with @nkuoch and confirmed there is actually a way to test broken connection on staging/ad-hoc. So once the PR is otherwise ready to go, let me know and I'll spin up an ad-hoc build to test this. |
@joekaufmanexpensify after choosing All Visa Cards in the search filter. Now after my fix the feed name will be displayed correctly for all the feeds
|
|
Sounds good! So we're just displaying the feed name (I assume for the second example, the feed name is simply |
|
@joekaufmanexpensify yep, exactly! |
|
@koko57 About this bug #64741 (comment), steps to reproduce:
This is happening on staging and production as well and seems like something broken with |
|
Discussing more here |
|
@dominictb still, couldn't repro locally. Could repro on staging. @joekaufmanexpensify I saw the discussion - so should we fix it here or it will be handled separately? Is it still a blocker for this PR? |
dominictb
left a comment
There was a problem hiding this comment.
That bug is handled separately.
|
Initiated adhoc build to try and test broken connection fix there 👍 |
|
🚧 @joekaufmanexpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
Talked with Nathalie more and we're not sure that the broken connection can be simulated in staging/adhoc after all. So we're just going to test that piece on prod 👍 |
|
🚀 Deployed to staging by https://github.com/nkuoch in version: 9.1.83-0 🚀
|
|
Tested everything I could on staging, looks good 👍 |
|
I will test the Wells Fargo fix and broken connection fix on Prod |
|
Checked off as that is very minor 👍 |
|
@joekaufmanexpensify the svg added for the card has a border - but as it's the same/very similar color it's not noticeable for the highlighted item |
|
@joekaufmanexpensify Yep, what Agata said—the highlight background uses our same border color, so it doesn't "show up" on the selected row. I think it's totally fine! And I think the @Expensify/design is proposing a change to that selected state soon anyways. |
|
Yup, what Danny said! |
|
Sweet, thank you all for confirming! |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.1.83-5 🚀
|
|
A few issues when testing in prod. Discussing here. |
| getDomainNameForPolicy(policyID), | ||
| JSON.stringify(metadata?.accounts), | ||
| ); | ||
| InteractionManager.runAfterInteractions(() => { |
There was a problem hiding this comment.
@narefyev91 why was this InteractionManager necessary here? (I'm working on InteracionManager migration and I need to understand the use case)
Thank you in advance!


































Explanation of Change
More Plaid import follow ups
Fixed Issues
$ #64659, #63516
PROPOSAL:
Tests
Bug 1: Plaid import card icons are too hard to see in light mode
Bug 2: Missing card feed name and custom icons on card filer on reports page
Bug 3: Card filter on reports page not working to filter transactions
Bug 4: Missing last 4 digits for some cards on card feed
Offline tests
No changes
QA Steps
Bug 1: Plaid import card icons are too hard to see in light mode
Bug 2: Missing card feed name and custom icons on card filer on reports page
Bug 3: Card filter on reports page not working to filter transactions
Bug 4: Missing last 4 digits for some cards on card feed
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))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
Bug 2:
Android: mWeb Chrome
Bug 2:
iOS: Native
Bug 2:
iOS: mWeb Safari
Bug 2:
MacOS: Chrome / Safari
Bug 2:
MacOS: Desktop
Bug 2: