[Internal QA]: Fix broken connection visibility for Cards#76620
[Internal QA]: Fix broken connection visibility for Cards#76620narefyev91 wants to merge 7 commits intoExpensify:mainfrom
Conversation
melvin-bot
bot
requested review from
Krishna2323
and removed request for
a team
|
@Krishna2323 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] |
melvin-bot
bot
requested review from
heyjennahay
and removed request for
a team
narefyev91
changed the title
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@joekaufmanexpensify could you please run ad-hocs and test this one? all UI issues should be fixed |
|
Yep, will test today 👍 |
|
🚧 @joekaufmanexpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Reviewing... |
|
Hi @narefyev91 — as an external contributor, I don’t have access to a company card. I simulated a broken connection by setting Monosnap.screencast.2025-12-06.00-45-31.mp4Code used to Seed Mock Broken CompanyCard/**
* DEV-ONLY: Seed a mock company card feed and a broken connection card into Onyx to enable local UI testing
* without needing a real bank connection.
*
* This will:
* - Create a feed entry in SHARED_NVP_PRIVATE_DOMAIN_MEMBER for the workspace
* - Set the LAST_SELECTED_FEED for the given policy
* - Create one card under WORKSPACE_CARDS_LIST with lastScrapeResult set to a non-ignored error (401)
* - Mirror the card into CARD_LIST for global consumers
*/
function seedMockBrokenCompanyCard(policyID: string) {
// Only allow seeding in dev
// eslint-disable-next-line no-undef
if (typeof __DEV__ !== 'undefined' && !__DEV__) {
return;
}
const workspaceAccountID = PolicyUtils.getWorkspaceAccountID(policyID);
if (!workspaceAccountID) {
return;
}
const bankName = CONST.COMPANY_CARD.FEED_BANK_NAME.CAPITAL_ONE;
const domainOrWorkspaceAccountID = workspaceAccountID;
const cardID = '99990001';
const lastScrapeResult = 401; // not in BROKEN_CONNECTION_IGNORED_STATUSES, so treated as broken
const mockDomainName = 'Mock Workspace';
const mockCardName = 'Business Gold Card - 3333';
const mockCardState = CONST.EXPENSIFY_CARD.STATE.OPEN;
const mockErrors = {
connection: "There's an issue with this card's connection, which may result in you not seeing transactions. Please contact your administrator to resolve.",
};
const feedWithDomainID = CardUtils.getCompanyCardFeedWithDomainID(bankName, domainOrWorkspaceAccountID);
const updates: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER}${domainOrWorkspaceAccountID}`,
value: {
settings: {
companyCards: {
[bankName]: {
preferredPolicy: policyID,
},
},
},
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.LAST_SELECTED_FEED}${policyID}`,
value: feedWithDomainID,
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${domainOrWorkspaceAccountID}_${bankName}`,
value: {
[cardID]: {
cardID: Number(cardID),
bank: bankName,
domainName: mockDomainName,
cardName: mockCardName,
lastScrapeResult,
state: mockCardState,
errors: mockErrors,
},
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.CARD_LIST,
value: {
[cardID]: {
cardID: Number(cardID),
bank: bankName,
domainName: mockDomainName,
cardName: mockCardName,
lastScrapeResult,
state: mockCardState,
errors: mockErrors,
},
},
},
];
Onyx.update(updates);
} |
|
@narefyev91 Minor issue. We are showing the red dot twice on the card row on wallet. If you look at OP in the issue, we previously only showed it alongside the error. Can we do that we did before? 
|
|
I also am not seeing the new message in #admins, is that expected? 
|
|
Also, when I fixed the broken connection, we removed the error on the card feed, but the one on wallet remained. Both should be removed when the connection is fixed. 
|
@joekaufmanexpensify could you please send Onyx data for "reportActions_ID_OF_YOUR_ADMIN_CHAT" - let's see if we get from Onyx that message. |
I could be wrong - but could you also check |
joekaufmanexpensify
requested review from
joekaufmanexpensify
and removed request for
heyjennahay
|
Sure. I will retest this morning to get the error state back to send both of those onyx details to you 👍 |
I DM'ed this to you 👍
The card in onyx has this error, even after I fix the broken connection and the error is removed from the
|
|
🚧 @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! 🧪🧪
|
|
@narefyev91 One other thought. Would it be easy to change the error message on the card row of
|
Well error message is taken from API response - i think based on the logged user - API could return correct error message in Onyx or just return 2 error messages - one for regular user - one for admin. And on FE side i could pick correct one to show |
|
Ah. I see. So doing that change correctly would require a backend change? |
yeah you are 100% right |
|
PR to fix #76620 (comment) is up: https://github.com/Expensify/Auth/pull/18719 |
|
🚧 @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! 🧪🧪
|
|
Nice, thanks @NikkiWines! I see that PR is merged, so I ran an adhoc and will test the fix. Are you able to help with the other backend changes we were looking to make:
|
|
Chatted with @NikkiWines and confirmed she doesn't have bandwidth to help with the other backend changes right now, so looking for another volunteer. |
Retested this and still one minor issue. Discussing in the BE PR. |
BUG: We're incorrectly showing broken connection across multiple feedsSteps
Expected behaviorWe only show the broken connection error for the direct feed, not the commercial feed. Commercial feed connections can't break. Actual behaviorWe show the broken connection error for both feeds. 
|
|
New backend PR is up to fix #76620 (comment). It's merged and awaiting deploy |
|
@joekaufmanexpensify updated FE part as well - now should show correctly error for the selected feed |
|
🚧 @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! 🧪🧪
|
|
Retested #76620 (comment) and mostly working now. Only minor feedback @narefyev91 is we should still show a red dot next to the Visa feed name when the Visa feed is open, which is meant to guide the user to open the feed selector where they can select the feed with the error. LMK if you have any questions on what I mean by that. 
|
|
🚧 @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! 🧪🧪
|
|
Retested #76620 (comment) and the red dot fix looks good. TY! |
|
@nkuoch volunteered to handle these remaining backend changes. We'll tackle them in this internal issue. |
|
This other backend PR is still pending deploy. Will retest that bug as soon as it's out. |
# Conflicts: # src/components/FeedSelector.tsx # src/libs/CardUtils.ts # src/pages/settings/Wallet/PaymentMethodListItem.tsx # src/pages/workspace/companyCards/WorkspaceCompanyCardsListHeaderButtons.tsx
|
@joekaufmanexpensify my PR and correct changes were fully broke down by this PR https://github.com/Expensify/App/pull/78178/files |
|
It is a new feature and I agree there are some considerations related to it. Let's discuss here. |
Partially based on changes by @narefyev91 in Expensify#76620
4 participants
Explanation of Change
Visual fix for Company card broken connection and in wallet cards
Fixed Issues
$ #76438
PROPOSAL:
Tests
Offline tests
No changes
QA Steps
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