Skip to content

Fix: show "Personal card" for personal cards in group-by-card feed name#92631

Draft
MelvinBot wants to merge 2 commits into
mainfrom
claude-personalCardFeedNameGroupByCard
Draft

Fix: show "Personal card" for personal cards in group-by-card feed name#92631
MelvinBot wants to merge 2 commits into
mainfrom
claude-personalCardFeedNameGroupByCard

Conversation

@MelvinBot
Copy link
Copy Markdown
Contributor

Explanation of Change

On the Spend page with Group-by: Card, a personal card group rendered its feed label as "Deleted feed" instead of "Personal card".

getCardSections resolved every group's formattedFeedName through getFeedNameForDisplay, a company-card-feed helper. A personal card's bank is never present in the company cardFeeds collection, so doesCardFeedExist returned false and the helper short-circuited to the workspace.companyCards.deletedFeed label.

The only reliable signal for "this is a personal card" is the card's missing fundID (isPersonalCard), which lives on the Card record — not on the backend group row. The card list threaded into getCardSections was NON_PERSONAL_AND_WORKSPACE_CARD_LIST, which filters personal cards out, so a naive isPersonalCard(cardList?.[cardGroup.cardID]) check could not work: the lookup is undefined for personal cards, and isPersonalCard(undefined) returns true (which would also mislabel genuinely-deleted company feeds whose card record is gone).

This PR therefore makes two coordinated changes:

  1. Feed the grouped-section builders the unfiltered PERSONAL_AND_WORKSPACE_CARD_LIST (in src/components/Search/index.tsx and src/hooks/useSearchSections.ts) so personal cards are resolvable by cardGroup.cardID. The existing per-row nonPersonalAndWorkspaceCards prop is left untouched.
  2. In getCardSections, look up const card = cardList?.[cardGroup.cardID] and, when card && isPersonalCard(card), use translate('cardTransactions.personalCard'). Otherwise fall through to the existing getFeedNameForDisplay(...) call.

The card && guard keeps the legitimate "Deleted feed" message intact for removed company feeds (with or without a card record), while valid personal-card groups now show "Personal card".

🤖 AI Tests run locally by MelvinBot
  • npm test -- tests/unit/Search/SearchUIUtilsTest.ts361 passed (includes a new regression test asserting personal cards → "Personal card", CSV personal cards → "Personal card", and removed company feeds → "Deleted feed")
  • npm test -- tests/unit/hooks/useSearchSections.test.ts3 passed
  • npm run typecheck-tsgo — passed
  • npm run lint-changed — passed
  • npm run prettier-changed — no changes
  • npm run react-compiler-compliance-check check ...useSearchSections.ts compiles; Search/index.tsx reports the same 9 pre-existing errors on origin/main (no regression introduced)

Fixed Issues

$ #92523
PROPOSAL:

Tests

// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"

  1. Connect a personal card with transactions
  2. Open the Spend page
  3. Change the grouping to Group-by: Card
  4. Verify the personal card feed displays as "Personal card" (not "Deleted feed")
  5. Verify company card feeds still display their correct feed name, and a removed company feed still shows "Deleted feed"
  • Verify that no errors appear in the JS console

Offline tests

This is a pure display derivation over already-synced Onyx data, so behavior is unchanged offline.

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

  1. Connect a personal card with transactions
  2. Open the Spend page
  3. Change the grouping to Group-by: Card
  4. Verify the personal card feed displays as "Personal card"
  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

Personal card groups on the Spend page rendered as "Deleted feed" because
getCardSections resolved every group's feed via getFeedNameForDisplay, a
company-feed helper. Detection requires the card's fundID, so the grouped
section builders now receive the unfiltered PERSONAL_AND_WORKSPACE_CARD_LIST
and label personal cards via isPersonalCard.

Co-authored-by: daledah <daledah@users.noreply.github.com>
@MelvinBot MelvinBot requested a review from a team June 4, 2026 08:00
@MelvinBot
Copy link
Copy Markdown
Contributor Author

I created this PR from an upstream branch since I don't have push access to your fork.

To take ownership of this branch and be able to push updates, run:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-personalCardFeedNameGroupByCard
git checkout -b claude-personalCardFeedNameGroupByCard upstream/claude-personalCardFeedNameGroupByCard
git push -u origin claude-personalCardFeedNameGroupByCard

Then you can close this PR and open a new one from your fork.

@MelvinBot
Copy link
Copy Markdown
Contributor Author

The failing checks are not related to this PR's changes — no code fix is needed.

1. test (job 8) — flaky timeout (unrelated)

The failure is a Jest timeout, not an assertion failure:

thrown: Exceeded timeout of 120000 ms for a test.

It occurred in tests/ui/SessionTest.tsx:121"Deep linking › should not remember the report path of the last deep link login after signing out and in again". This is a session/deep-linking UI test.

This PR only touches Search card-feed code (src/components/Search/index.tsx, src/hooks/useSearchSections.ts, src/libs/SearchUIUtils.ts) and its unit test (tests/unit/Search/SearchUIUtilsTest.ts). SessionTest.tsx imports none of these, and a 120s timeout in an unchanged test is a known flaky-CI pattern. The PR's own unit suite (SearchUIUtilsTest, 361 passed) is green.

Recommended action: re-run the failed job (gh run rerun 26938939129 --repo Expensify/App --failed, or Re-run failed jobs in the Actions UI). I attempted the re-run automatically but it required manual approval.

2. PR Author Checklist / checklist (unrelated)

This fails because the human-only Tests / QA Steps sections and the author checklist are still unchecked. I deliberately don't fill those out on a person's behalf — the human co-author should complete them before marking the PR ready for review.

@MelvinBot
Copy link
Copy Markdown
Contributor Author

Re-checked — the state is unchanged from my previous comment, and there is still no code fix to push. Both failing checks are unrelated to this PR's Search card-feed changes:

  • test (job 8) — the same flaky Jest timeout (Exceeded timeout of 120000 ms) in the unchanged tests/ui/SessionTest.tsx:121 (deep-linking/session). It has not been re-run since.
  • PR Author Checklist / checklist — fails only because the human-only Tests/QA sections are unchecked.

The single blocker is the flaky timeout, which just needs a CI re-run. I tried to trigger it automatically twice (gh run rerun 26938939129 --failed) but it requires interactive approval that I can't grant from here. A maintainer should click Re-run failed jobs on the run (or push any new commit) to clear it.

@daledah
Copy link
Copy Markdown
Contributor

daledah commented Jun 5, 2026

@MelvinBot please resovle conflict

…edNameGroupByCard

Co-authored-by: daledah <daledah@users.noreply.github.com>

# Conflicts:
#	src/hooks/useSearchSections.ts
@MelvinBot
Copy link
Copy Markdown
Contributor Author

🤖 Resolved the merge conflict by merging the latest main into the branch.

Conflict: src/hooks/useSearchSections.ts — the getSections({...}) argument list. This branch changed cardList to personalAndWorkspaceCards (the core of this PR), while main removed the allReportMetadata param from GetSectionsParams entirely.

Resolution: Kept this PR's cardList: personalAndWorkspaceCards and dropped allReportMetadata (its useOnyx declaration and usage no longer exist after main's change). I also removed the now-invalid allReportMetadata: {} from the new regression test in tests/unit/Search/SearchUIUtilsTest.ts, which typecheck flagged once main was merged.

Verification
  • npm run typecheck-tsgo — passed (clean)
  • npm test -- tests/unit/Search/SearchUIUtilsTest.ts342 passed
  • npx prettier --check on both changed files — no changes

PR is now mergeable (no conflicts). Please re-test if needed before marking ready for review.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

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.

Files with missing lines Coverage Δ
src/hooks/useSearchSections.ts 100.00% <100.00%> (ø)
src/libs/SearchUIUtils.ts 63.43% <100.00%> (+0.14%) ⬆️
src/components/Search/index.tsx 0.59% <0.00%> (-0.01%) ⬇️
... and 16 files with indirect coverage changes

@MelvinBot
Copy link
Copy Markdown
Contributor Author

🤖 I looked into the failing checks. No code fix is needed — every substantive check passes (typecheck, all 8 test jobs, ESLint, Prettier, Storybook, React Compiler, all builds, and the perf tests).

The only red check is PR Author Checklist / checklist, which fails with:

PR Author Checklist is not completely filled out. Please check every box to verify you've thought about the item.

This isn't a code failure — it's gating on the human-authored sections of the PR. To make it pass, the human co-author needs to:

  1. Fill out the Tests and QA Steps sections (currently // TODO placeholders).
  2. Check every box in the PR Author Checklist.

I can't tick those boxes or fill out those sections on your behalf — they're a human attestation. Once they're completed, this check will go green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants