Skip to content

Show cash back credits in Bank Reconciliation - #97958

Open
mukhrr wants to merge 10 commits into
Expensify:mainfrom
mukhrr:fix/94133
Open

Show cash back credits in Bank Reconciliation#97958
mukhrr wants to merge 10 commits into
Expensify:mainfrom
mukhrr:fix/94133

Conversation

@mukhrr

@mukhrr mukhrr commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

Shows ACH cash back credits as their own row in the Expensify Card Bank Reconciliation view, matching Expensify Classic.

  • Backend flags these groups with isCashBack and already returns total signed negative, so there is no client-side negation
  • isCashBack is a flag rather than a SETTLEMENT_STATUS value, because cash back is orthogonal to settlement state and adding it to the enum would leak it into the withdrawal-status: filter
  • The row renders a green Cash back badge, a blank Expenses cell, and the credit amount
  • Not expandable in either render path: TransactionGroupListItem on narrow and native, GroupHeader on wide web
  • Selection sets an explicit groupAmount, since the footer otherwise falls back to -Math.abs(amount) and would add the credit instead of subtracting it

Fixed Issues

$ #94133
PROPOSAL: #94133 (comment)

Tests

Requires an account on the cashBackReconciliation beta with an Expensify Card and at least one settled ACH cash back credit.

  1. Go to Spend > Expenses and switch to the Bank reconciliation view (type:expense groupBy:withdrawal-id withdrawalType:expensify-card)
  2. Verify a cash back row appears alongside the normal settlement rows
  3. Verify its Withdrawal status column shows a green Cash back badge, not Pending / Cleared / Failed
  4. Verify its Expenses column is blank, not 0
  5. Verify its Total shows a negative amount
  6. Verify the row has no expand arrow and does not expand when clicked, on both a wide browser window and a narrow one
  7. Select the cash back row together with a normal settlement row and verify the footer total subtracts the cash back amount
  8. Verify normal settlement rows are unchanged: badge, positive total, expense count, and expanding to show child transactions all still work
  • Verify that no errors appear in the JS console

Offline tests

The reconciliation view is read-only, so there is no optimistic write to verify. Going offline with the view open keeps the last fetched rows rendered, including the cash back row, and no new Search request is issued until the connection returns.

QA Steps

Same as tests. Note this needs an account enrolled in the cashBackReconciliation beta with real ACH cash back activity, since the backend only returns these groups to beta accounts.

  • 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 verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • 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_app.mp4
Android: mWeb Chrome
android_web.mp4
iOS: Native
IOS_app.mp4
iOS: mWeb Safari
IOS-mWeb.mp4
MacOS: Chrome / Safari
94133-cashback-reconciliation-web.mp4

Surface ACH cash back credits as their own non-expandable row in the
Expensify Card Bank Reconciliation view, matching Expensify Classic.

The backend flags these groups with isCashBack and returns the total
already signed negative, so the row renders a Cash back badge, a blank
Expenses cell and the credit amount with no client-side negation. The
row is not expandable in either render path: TransactionGroupListItem
on narrow and native, GroupHeader on wide web.

Selection needs an explicit groupAmount because the footer falls back
to -Math.abs(amount) when it is absent, which would make a credit add
to the selected total instead of reducing it.
@melvin-bot

melvin-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

Hey, I noticed you changed src/languages/en.ts in a PR from a fork. For security reasons, translations are not generated automatically for PRs from forks.

If you want to automatically generate translations for other locales, an Expensify employee will have to:

  1. Look at the code and make sure there are no malicious changes.
  2. Run the Generate static translations GitHub workflow. If you have write access and the K2 extension, you can simply click: [this button]

Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running:

npx ts-node ./scripts/generateTranslations.ts --help

Typically, you'd want to translate only what you changed by running npx ts-node ./scripts/generateTranslations.ts --compare-ref main

@mukhrr

mukhrr commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running:

@Julesssss could you pls help generating translations? thanks

@mukhrr
mukhrr marked this pull request as ready for review August 6, 2026 12:18
@mukhrr
mukhrr requested review from a team as code owners August 6, 2026 12:18
@melvin-bot
melvin-bot Bot requested review from heyjennahay and situchan and removed request for a team August 6, 2026 12:19
@melvin-bot

melvin-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

@situchan 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
melvin-bot Bot removed the request for review from a team August 6, 2026 12:19
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

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/components/Search/SearchBulkActionsButton.tsx 93.82% <100.00%> (+2.46%) ⬆️
...SearchList/ListItem/WithdrawalIDListItemHeader.tsx 94.20% <100.00%> (+94.20%) ⬆️
src/components/Search/SearchList/ListItem/types.ts 100.00% <ø> (ø)
src/components/Search/SearchSelectionFooter.tsx 77.35% <100.00%> (+3.54%) ⬆️
src/components/Search/selectionBuilders.ts 60.71% <100.00%> (+1.45%) ⬆️
src/libs/SearchUIUtils.ts 66.42% <88.88%> (+0.42%) ⬆️
...h/SearchList/ListItem/TransactionGroupListItem.tsx 76.04% <70.00%> (-0.54%) ⬇️
...ponents/Search/SearchList/ListItem/GroupHeader.tsx 0.00% <0.00%> (ø)
... and 88 files with indirect coverage changes

Comment thread src/components/Search/SearchList/ListItem/GroupHeader.tsx Outdated
Comment thread src/components/Search/SearchList/ListItem/WithdrawalIDListItemHeader.tsx Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cce3db43f5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/libs/SearchUIUtils.ts Outdated
queryJSON && withdrawalIDGroup.entryID ? buildSpecificGroupQuery(queryJSON, CONST.SEARCH.SYNTAX_FILTER_KEYS.WITHDRAWAL_ID, withdrawalIDGroup.entryID) : undefined;

if (!withdrawalIDGroup.accountNumber) {
if (!withdrawalIDGroup.accountNumber && !withdrawalIDGroup.isCashBack) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve account-number guard for mobile cash-back rows

When the backend sends a cash-back group without accountNumber, this new exception lets the row reach WithdrawalIDListItemHeader, but the narrow/mobile rendering still evaluates withdrawalIDItem.accountNumber.slice(-4), which will throw before the cash-back row can render. Either keep filtering rows without an account number or make the mobile label use the already-safe maskedNumber/fallback path as the large-screen path does.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accountNumber comes from BE. #94133 (comment)

mukhrr added 2 commits August 6, 2026 17:42
…unt number guard

The cash back spacer passed an inline object literal to style, which allocates
a new object each render and bypasses the shared style system. Use
StyleUtils.getWidthAndHeightStyle instead, in both render paths.

Also drop the isCashBack exception to the account number guard. The backend
sends accountNumber on cash back groups, so the exception never fired, and
relaxing the guard would have let a row through to a narrow-screen label that
dereferences accountNumber without a fallback.
WithdrawalIDListItemHeader had no test file at all, so the cash back badge,
the blank Expenses cell and the negative total were uncovered. Each case is
paired with a settlement control so a regression that drops the cash back
branch fails rather than silently rendering a plausible "Cleared" row.

The expand-arrow cases run under narrow layout because that is the only
layout where this component owns the arrow; on wide screens the parent
supplies the toggle.

Also covers isCashBackWithdrawalGroup directly, including a group of another
type carrying the flag, which must not opt into cash back rendering.
@OSBotify

OSBotify commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

🦜 Polyglot Parrot! 🦜

Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues:

View the translation diff
diff --git a/src/languages/de.ts b/src/languages/de.ts
index a89d7c1f88f..4da1d962d7e 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -9181,13 +9181,7 @@ Fügen Sie weitere Ausgabelimits hinzu, um den Cashflow Ihres Unternehmens zu sc
         },
     },
     settlement: {
-        status: {
-            pending: 'Ausstehend',
-            cleared: 'Ausgeglichen',
-            failed: 'Fehlgeschlagen',
-            never: 'Nie',
-            cashBack: 'Cashback',
-        },
+        status: {pending: 'Ausstehend', cleared: 'Ausgeglichen', failed: 'Fehlgeschlagen', never: 'Nie', cashBack: 'Cashback'},
         failedError: ({link}: {link: string}) => `Wir versuchen diese Abrechnung erneut, sobald du <a href="${link}">dein Konto entsperrst</a>.`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date} • Auszahlungs-ID: ${withdrawalID}`,
     },
diff --git a/src/languages/el.ts b/src/languages/el.ts
index 7d4818ab075..fb0779a9f5e 100644
--- a/src/languages/el.ts
+++ b/src/languages/el.ts
@@ -9400,13 +9400,7 @@ ${reportName}`,
         },
     },
     settlement: {
-        status: {
-            pending: 'Σε εκκρεμότητα',
-            cleared: 'Εκκαθαρισμένο',
-            failed: 'Απέτυχε',
-            never: 'Ποτέ',
-            cashBack: 'Επιστροφή χρημάτων',
-        },
+        status: {pending: 'Σε εκκρεμότητα', cleared: 'Εκκαθαρισμένο', failed: 'Απέτυχε', never: 'Ποτέ', cashBack: 'Επιστροφή μετρητών'},
         failedError: ({link}: {link: string}) => `Θα προσπαθήσουμε ξανά για αυτόν τον διακανονισμό όταν <a href="${link}">ξεκλειδώσετε τον λογαριασμό σας</a>.`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date} • Αναγνωριστικό ανάληψης: ${withdrawalID}`,
     },
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 3ad482bc841..d2cd57fa964 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -8974,13 +8974,7 @@ El plan Controlar empieza en 9 $ por miembro activo al mes.`,
         },
     },
     settlement: {
-        status: {
-            pending: 'Pendiente',
-            cleared: 'Liquidado',
-            failed: 'Fallido',
-            never: 'Nunca',
-            cashBack: 'Reembolso',
-        },
+        status: {pending: 'Pendiente', cleared: 'Liquidado', failed: 'Fallido', never: 'Nunca', cashBack: 'Devolución de dinero'},
         failedError: ({link}: {link: string}) => `Reintentaremos esta liquidación cuando <a href="${link}">desbloquees tu cuenta</a>.`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date} • ID de retiro: ${withdrawalID}`,
     },
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 4514860ba7d..f93be460763 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -9216,13 +9216,7 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e
         },
     },
     settlement: {
-        status: {
-            pending: 'En attente',
-            cleared: 'Compensé',
-            failed: 'Échec',
-            never: 'Jamais',
-            cashBack: 'Remise en argent',
-        },
+        status: {pending: 'En attente', cleared: 'Compensé', failed: 'Échec', never: 'Jamais', cashBack: 'Remboursement en espèces'},
         failedError: ({link}: {link: string}) => `Nous réessaierons ce règlement lorsque vous <a href="${link}">déverrouillerez votre compte</a>.`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date} • ID de retrait : ${withdrawalID}`,
     },
diff --git a/src/languages/it.ts b/src/languages/it.ts
index ee8d06156c2..95ce99842a2 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -9155,13 +9155,7 @@ Aggiungi altre regole di spesa per proteggere il flusso di cassa aziendale.`,
         },
     },
     settlement: {
-        status: {
-            pending: 'In sospeso',
-            cleared: 'Compensato',
-            failed: 'Non riuscito',
-            never: 'Mai',
-            cashBack: 'Cashback',
-        },
+        status: {pending: 'In sospeso', cleared: 'Compensato', failed: 'Non riuscito', never: 'Mai', cashBack: 'Cashback'},
         failedError: ({link}: {link: string}) => `Riproveremo a effettuare questa liquidazione quando <a href="${link}">sblocchi il tuo conto</a>.`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date} • ID prelievo: ${withdrawalID}`,
     },
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index de583f58b81..991e534662f 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -9031,13 +9031,7 @@ ${reportName}`,
         },
     },
     settlement: {
-        status: {
-            pending: '保留中',
-            cleared: '支払済み',
-            failed: '失敗しました',
-            never: 'なし',
-            cashBack: 'キャッシュバック',
-        },
+        status: {pending: '保留中', cleared: '支払済み', failed: '失敗しました', never: 'なし', cashBack: 'キャッシュバック'},
         failedError: ({link}: {link: string}) => `<a href="${link}">アカウントのロックを解除</a>すると、この精算を再試行します。`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date}・出金 ID:${withdrawalID}`,
     },
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index ad1a029c736..a86acd078af 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -9127,13 +9127,7 @@ er bestedingsregels toe om de kasstroom van het bedrijf te beschermen.`,
         },
     },
     settlement: {
-        status: {
-            pending: 'In behandeling',
-            cleared: 'Verrekend',
-            failed: 'Mislukt',
-            never: 'Nooit',
-            cashBack: 'Cashback',
-        },
+        status: {pending: 'In behandeling', cleared: 'Verrekend', failed: 'Mislukt', never: 'Nooit', cashBack: 'Cashback'},
         failedError: ({link}: {link: string}) => `We proberen deze afrekening opnieuw zodra je <a href="${link}">je account ontgrendelt</a>.`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date} • Opname-ID: ${withdrawalID}`,
     },
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index f3ee77caba2..eff8ee13ec3 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -9105,13 +9105,7 @@ Dodaj więcej zasad wydatków, żeby chronić płynność finansową firmy.`,
         },
     },
     settlement: {
-        status: {
-            pending: 'Oczekujące',
-            cleared: 'Wyczyszczono',
-            failed: 'Niepowodzenie',
-            never: 'Nigdy',
-            cashBack: 'Zwrot gotówki',
-        },
+        status: {pending: 'Oczekujące', cleared: 'Wyczyszczono', failed: 'Niepowodzenie', never: 'Nigdy', cashBack: 'Zwrot gotówki'},
         failedError: ({link}: {link: string}) => `Spróbujemy ponownie rozliczyć tę płatność, gdy <a href="${link}">odblokujesz swoje konto</a>.`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date} • ID wypłaty: ${withdrawalID}`,
     },
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index c0922593104..cb2199eba92 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -9118,13 +9118,7 @@ Adicione mais regras de gasto para proteger o fluxo de caixa da empresa.`,
         },
     },
     settlement: {
-        status: {
-            pending: 'Pendente',
-            cleared: 'Compensado',
-            failed: 'Falhou',
-            never: 'Nunca',
-            cashBack: 'Cashback',
-        },
+        status: {pending: 'Pendente', cleared: 'Compensado', failed: 'Falhou', never: 'Nunca', cashBack: 'Cashback'},
         failedError: ({link}: {link: string}) => `Tentaremos processar este acerto novamente quando você <a href="${link}">desbloquear sua conta</a>.`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date} • ID de saque: ${withdrawalID}`,
     },
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 475bc865c39..cbbe39e9db1 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -8802,13 +8802,7 @@ ${reportName}`,
         },
     },
     settlement: {
-        status: {
-            pending: '待处理',
-            cleared: '已入账',
-            failed: '失败',
-            never: '从不',
-            cashBack: '返现',
-        },
+        status: {pending: '待处理', cleared: '已入账', failed: '失败', never: '从不', cashBack: '返现'},
         failedError: ({link}: {link: string}) => `当你<a href="${link}">解锁你的账户</a>后,我们会重试此结算。`,
         withdrawalInfo: ({date, withdrawalID}: {date: string; withdrawalID: number}) => `${date} • 提现 ID:${withdrawalID}`,
     },

Note

You can apply these changes to your branch by copying the patch to your clipboard, then running pbpaste | git apply 😉

View workflow run

@mukhrr

mukhrr commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@situchan we are good to move now

@mukhrr

mukhrr commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@situchan kindly bump

# Conflicts:
#	tests/unit/TransactionGroupListItemTest.tsx
@trjExpensify

Copy link
Copy Markdown
Contributor

@mukhrr couple of conflicts now unfortunately. I've bumped @situchan to complete this review today or I will find someone else.

@situchan

Copy link
Copy Markdown
Contributor

It's odd to me that cash back row is hoverable, clickable but no action when clicked

Screen.Recording.2026-08-12.at.1.26.03.PM.mov

@mukhrr

mukhrr commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@situchan I guess "not clicked" as expected. so we want no hoverable, no clickable? cc @trjExpensify @shawnborton

@trjExpensify

trjExpensify commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Fair. I think we can make it such that just the checkbox part of the row shows the "clickable" cursor. As that's all that is for these cashback rows.

@trjExpensify

Copy link
Copy Markdown
Contributor

Curious for @shawnborton's take on that.

Comment thread src/libs/SearchUIUtils.ts
mukhrr added 2 commits August 12, 2026 21:16
# Conflicts:
#	src/components/Search/SearchList/ListItem/TransactionGroupListItem.tsx
#	tests/unit/TransactionGroupListItemTest.tsx
The withdrawal status column sorted on the raw settlement state, and a cash
back row carries state 8 while showing a badge that is not a settlement
status. It tied with the cleared settlements and interleaved among them.

Sort on a derived rank instead, so cash back ranks past every settlement
state. Also drop the pointer cursor and hover highlight from a cash back
row, which has nothing to open; its checkbox stays interactive.
@mukhrr

mukhrr commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@situchan could you pls have another look here? thanks

@mukhrr
mukhrr requested a review from situchan August 12, 2026 16:51
@situchan

Copy link
Copy Markdown
Contributor

Looks good. Just waiting for design feedback on #97958 (comment)

@shawnborton

Copy link
Copy Markdown
Contributor

Sorry for the delay - yes, I agree with what Tom is saying - only use a pointer cursor over the checkbox of those rows where the entire row cannot be clicked.

@trjExpensify

Copy link
Copy Markdown
Contributor

Perf, let's do it. 👍

@mukhrr

mukhrr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

cool, we already have it on 7fb025a commit. so we are good to move @situchan

@situchan

Copy link
Copy Markdown
Contributor

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible 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 checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: HybridApp
    • Android: mWeb Chrome
    • iOS: HybridApp
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this 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.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

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

@situchan

Copy link
Copy Markdown
Contributor

Bug: 0 selected

Screen.Recording.2026-08-13.at.4.06.57.AM.mov

@quinthar quinthar removed the #migrate label Aug 13, 2026
The header sums how many expenses each selected group holds. A cash back
row holds none, so ticking it read "0 selected" with a dropdown that
opened nothing.

Count a cash back group as one item. The footer still reads 0 there,
which is correct: it reports expenses, not selected rows.
@mukhrr

mukhrr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Bug: 0 selected

Screen.Recording.2026-08-13.at.4.06.57.AM.mov

@situchan fixed it

@mukhrr

mukhrr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

oh wait..

@mukhrr

mukhrr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@shawnborton @trjExpensify a cash back row has 0 expenses, so bulk actions dropdown opens with only "Current View" which doesn't work as well. should the row be selectable at all? it does contribute its credit to Total spend, which feels like the point on a reconciliation page, but there's no action you can take on it.

@mukhrr

mukhrr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@ShridharGoel mind helping here too?

  1. when the footer currency is switched off the default, what sign does GetTransactionsConvertedAmount return for a cash back group? sign-preserved (positive, an inflow), or force-negated like -Math.abs()?

the footer sums acc - convertedAmount, so if it comes back negative the -$25.00 credit gets added as +$25.00 instead of subtracted. default currency is fine, it only breaks after a currency switch.

  1. Cashback carries state 8, the same state as a cleared settlement, so it was interleaving with the Cleared rows. i now rank it past every settlement state on the FE.

sortBy is sent to the server, so the server still picks page membership by raw state. can the ranking match server side, or do you want me to leave the FE sort diverging and accept rows shifting as you paginate?

Every loaded group was stamped -Math.abs(total), but a selected cash back
row's live source is a positive groupAmount. The freshness check compares
the two, so a cash back group stamped by the bulk path never went fresh
and its cached conversion was refetched every time.
@shawnborton

Copy link
Copy Markdown
Contributor

Hmm so if you select the single row, there is no action you could take from the "1 selected" button? If that's the case, I guess it would make sense to have it be unselectable (just keep the checkbox disabled?)

@mukhrr

mukhrr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@shawnborton okay, so if user selects all from top, we skip cashback row, right?

@shawnborton

Copy link
Copy Markdown
Contributor

That makes sense. Let's see what Tom thinks though too.

@trjExpensify

Copy link
Copy Markdown
Contributor

Mhm, I wouldn't think we should skip the cashback and it would be included in the CSV export as it forms part of the rec. @joekaufmanexpensify do you have a test account with cashback to download from Classic to doublecheck that?

@mukhrr

mukhrr commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

so bulk actions dropdown opens with only "Current View" which doesn't work as well.

hmm maybe that's why it didn't work for me. I mean on my account

@ShridharGoel

Copy link
Copy Markdown
Contributor

when the footer currency is switched off the default, what sign does GetTransactionsConvertedAmount return for a cash back group? sign-preserved (positive, an inflow), or force-negated like -Math.abs()?
the footer sums acc - convertedAmount, so if it comes back negative the -$25.00 credit gets added as +$25.00 instead of subtracted. default currency is fine, it only breaks after a currency switch.

GetTransactionsConvertedAmount should be returning cash back as a positive expense-signed amount so footer would show it correctly. Is that not happening? Can you check it?

sortBy is sent to the server, so the server still picks page membership by raw state. can the ranking match server side, or do you want me to leave the FE sort diverging and accept rows shifting as you paginate?

Backend sorts by state, so same can be done on FE. Can you write an example of the behaviour so that @trjExpensify and others can confirm the expectations?

@joekaufmanexpensify

Copy link
Copy Markdown
Contributor

Mhm, I wouldn't think we should skip the cashback and it would be included in the CSV export as it forms part of the rec. @joekaufmanexpensify do you have a test account with cashback to download from Classic to doublecheck that?

Yep, I just confirmed we include cashback rows in both the CSV and statement downloads in OldDot. I would assume we would want the row to be selectable so it can be included in either a NewDot as well.

@trjExpensify

Copy link
Copy Markdown
Contributor

Thanks, Joe!

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.

8 participants