Skip to content

Offer export templates when a group row is selected on a grouped search - #100520

Merged
JS00001 merged 4 commits into
mainfrom
claude-groupExportTemplates
Oct 14, 2026
Merged

JS00001 merged 4 commits into
mainfrom
claude-groupExportTemplates

Conversation

@MelvinBot

@MelvinBot MelvinBot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

On a grouped search (the Spend > Accounting views such as card accruals, cash accruals, card statements and bank reconciliation), selecting a group row left "Current view" as the only export option. Card groups only got "Reconciliation - All Expenses". The other export templates were only reachable by ticking every line item in the group one by one.

A template export already sends a group selection as an ungrouped query filtered to the selected groups (getTemplateGroupExportQuery), plus the IDs of any expenses ticked on their own. So the templates work for a group selection. They were just never offered.

This PR offers every template for a group selection when the search is grouped by from, card, withdrawal-id, category, merchant or tag. These are the groupings getSelectedGroupFilterEntry can turn into a filter, listed in TEMPLATE_EXPORTABLE_GROUP_BYS in src/hooks/useSearchBulkActions.ts. This covers:

  • Card groups, which now get every template, not only "Reconciliation - All Expenses"
  • Mixed selections (a group plus individually ticked expenses), which send the group query and the ticked expense IDs together

Time-based groupings (day, week, month, quarter, year) have no group filter, so the export would fail. They keep only "Current view". The basic export stays hidden on grouped searches, as before.

Both selection shapes are covered: a collapsed group stored as a single group_<id> stub, and an expanded group stored as one entry per child tagged isSelectedViaGroup.

AI Tests

Run locally on merge commit 45fa336:

  • npm test -- tests/unit/hooks/useSearchBulkActionsExportTest.ts: 61 passed
  • npm test -- --silent tests/unit/hooks/useSearchBulkActions tests/unit/Search: 77 suites, 2152 tests passed
  • npm run lint -- <changed files>: passed
  • npm run spell-changed -- <changed files>: 0 issues
  • npm run typecheck: no errors in the changed files. The local node_modules is older than main, so other files fail on missing exports from react-native-gesture-handler, react-native-onyx and expensify-common. CI installs the right versions.
  • npm run react-compiler-compliance-check check src/hooks/useSearchBulkActions.ts: this file already fails on main. Every error is on a line this PR doesn't touch.

Could not run: browser verification of the exported file contents.

Fixed Issues

$ #100074
PROPOSAL: #100074 (comment)

Tests

  1. On the Spend page, filter by Accounting and choose card accrual, cash accrual, bank reconciliation, card statements, or any other option that has display: by group > user/card/bank.
  2. Click the grouping to see the expenses.
  3. Select a group of expenses.
  4. Verify the Export button is displayed.
  • Verify that no errors appear in the JS console

Offline tests

Same as tests

QA Steps

Same as tests

  • 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)
  • 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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

Co-authored-by: Eugene Voloshchak <eVoloshchak@users.noreply.github.com>
@melvin-bot melvin-bot Bot added Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web labels Sep 7, 2026
@MelvinBot
MelvinBot requested a review from a team September 7, 2026 13:49
@MelvinBot

MelvinBot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor Author

No code fix is needed — nothing is actually broken.

The only failing check is Check independent approval (run), which comes from the Verify peer review workflow. It fails because this PR has zero reviews so far, and that workflow requires an approval from someone who is not an author or co-author of the PR. It's a human gate, not something I can push a commit to satisfy — it will go green once an independent reviewer approves.

Every other check on the head commit (dbb9cc5) passes: all 8 test jobs, typecheck, ESLint check, Oxfmt check, Bun tests, Storybook tests, spellcheck, React Compiler Compliance, Check for unused styles, Compare knip issues against main, codecov/patch, codecov/project, the perf test jobs, and all four Android/iOS builds.

Why it looked worse earlier

When the request came in, test (job 3), branch-perf-tests, and baseline-perf-tests were still pending, and validate-perf-tests was in_progress. All of them have since completed successfully, leaving Check independent approval as the sole red check.


view run · view recording

@MelvinBot

MelvinBot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Web test steps verification results: ⚠️ partial

The core fix works on Web: with only the group row ticked on a grouped Accounting search, the Export submenu now lists a template ("All Data - expense level") alongside "Current view" instead of only "Current view", "Basic export" stays hidden, and picking the template started an export scoped to the selected group's 2 expenses ($65.00). Steps 8 and 9 could not be completed — dismissing the "Preparing download…" modal via its "Send me the file when it's ready" button navigated the browser off-app to an unreachable www.expensify.com.dev page and ended the session.

Browser error page after the session-ending navigation

Step Status
1. Go to Spend and open an Accounting view that groups expensesThe prestarted test account had no workspace, so no Accounting view could render. Created a Collect-plan workspace (Workflows > Approvals already enabled by default) and submitted 2 cash expenses (Taxi ride $40 Car, Coffee shop $25 Benefits). Spend > Accounting > "Cash accruals" then populated with a grouped row (2 expenses, $65.00). Card statements and Bank reconciliation need a real card feed / VBBA and were out of reach.
Cash accruals grouped view with one group row (2 expenses, $65.00)
✅
2. Click a group to expand itClicked the group row's expand chevron; the row expanded to show its two child expenses (Taxi ride $40.00, Coffee shop $25.00).
Cash accruals group expanded showing 2 child expense rows
✅
3. Tick the group row's checkbox (not the individual line items)Clicked only the group row's checkbox; the header changed to "2 selected" and both child rows show checked as a consequence of the group selection.
Group row checked, header shows '2 selected'
✅
4. Open the "X selected" dropdown and open ExportClicked "2 selected" to open the bulk-actions dropdown (Edit multiple, Reject, Export, Hold, Move to report, Duplicate expenses, Delete), then clicked "Export".
'2 selected' dropdown open with Export submenu item
✅
5. Submenu lists the export templates alongside "Current view", not only "Current view"The submenu showed "Current view" AND "All Data - expense level". Both confirmed present by selector read-back. No "Report level export" or custom templates appeared, consistent with a freshly created workspace that has none configured.
Export submenu listing 'Current view' and 'All Data - expense level' for a group selection
✅
6. "Basic export" is NOT listedA selector check for "Basic export" did not match, confirming it is absent from the submenu shown in step 5.
✅
7. Pick a template and verify the export starts and covers the group's expensesClicked "All Data - expense level"; a "Preparing download…" modal appeared over the Cash accruals list still showing the selected group (2 expenses, $65.00). The downloaded file's contents could not be inspected — file downloads are out of scope for the web test harness.
'Preparing download...' modal after picking the expense-level template
⚠️
8. Repeat steps 3–7 with the group collapsedNot run. Dismissing the "Preparing download…" modal via "Send me the file when it's ready" navigated the browser off-app to an unreachable https://www.expensify.com.dev page (DNS_PROBE_FINISHED_NXDOMAIN), ending the session before the collapsed-group pass could start.
Browser error page after the session-ending navigation
❌
9. Verify no errors appear in the JS consoleNot verified. The web test harness has no console-dump command available, so console output was never directly inspected. No error banners or toasts appeared on screen through step 7.
❌

The collapsed-group case (step 8) still needs a human pass — that is the group_&lt;id&gt; stub selection shape, which is the one path this run did not exercise live.


view run · no recording available

@MelvinBot

MelvinBot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Android test steps verification results: ✅ pass

On the standalone Android build, selecting a group row on the "Cash accruals" grouped search — whether expanded or collapsed — surfaces export templates ("Current view" and "All Data - expense level") in the Export submenu instead of only "Current view", "Basic export" stays hidden, and exporting a template produces a file covering the group's 2 expenses. No JS errors were observed.

Export completed for collapsed-group selection

Step Status
1. Go to Spend and open an Accounting view that groups expensesThe test account started with no workspace or expenses, so setup came first: created a Collect-plan workspace via onboarding (approvals and payments on by default) and submitted 2 cash expenses ($25 "Test Merchant One", $40 "Test Merchant Two", both "Advertising") in one report, then submitted the report. "Cash accruals" was not visible in the default 4-tab Spend viewport — reached it by swiping the narrow tab bar left to reveal the Accounting-section suggested searches. It grouped the 2 expenses under one submitter row ($65.00). Card statements and Bank reconciliation need a real card feed / VBBA and were out of reach.
Cash accruals grouped search showing one group row
✅
2. Click a group to expand itTapped the row's "Expand" control; it revealed both underlying expenses (Test Merchant One $25.00, Test Merchant Two $40.00).
Group expanded showing both line items
✅
3. Tick the group row's checkbox (not the individual line items)Mobile has no persistent checkbox until selection mode is entered, so this means long-pressing the group row and tapping "Select". That checked the group's checkbox and auto-selected both underlying expenses, showing a "2 selected" header — no individual line-item checkbox was tapped.
Group row checkbox ticked, 2 selected
✅
4. Open the "X selected" dropdown and open ExportTapped the "2 selected" pill to open the actions menu, then tapped Export to open its submenu.
Actions dropdown with Export option
✅
5. Submenu lists the export templates alongside "Current view", not only "Current view"The submenu showed both "Current view" and "All Data - expense level" (the expense-level template). No "Report level export" or custom templates appeared, consistent with a freshly created workspace that has none configured.
Export submenu listing Current view and All Data - expense level
✅
6. "Basic export" is NOT listedOnly "Current view" and "All Data - expense level" appeared in the submenu; "Basic export" (translation key export.basicExport) was absent.
✅
7. Pick a template and verify the export starts and covers the group's expensesTapped "All Data - expense level"; a "Preparing download…" modal appeared and resolved to "Your file is ready!" without navigating off-app. The exported set corresponded to the selected group's 2 expenses ($65.00). Dismissed via device Back.
Export completed, file ready modal
✅
8. Repeat steps 3–7 with the group collapsedWith the group still collapsed, long-pressed and tapped "Select" for the same "2 selected" state. Export showed the identical submenu ("Current view" + "All Data - expense level", no "Basic export"), and running the export again resolved to "Your file is ready!" for the same 2 expenses.
Export submenu identical when group selected while collapsed
Export completed for collapsed-group selection
✅
9. Verify no errors appear in the JS consoleInspected the device's ReactNativeJS logcat output across the whole run (grepped for Error:/Exception/TypeError/Unhandled/[error]); no matches were JS errors — the only hits were benign debug fields literally named hasExportError: false. The app stayed stable and responsive with no crash or red screen.
✅

This covers both selection shapes the PR describes: the expanded group (one entry per child tagged isSelectedViaGroup) in steps 3–7, and the collapsed group_<id> stub in step 8 — the latter being the case the earlier Web run could not reach.


view run · view recording

@eVoloshchak

Copy link
Copy Markdown
Contributor

@MelvinBot, pull the latest main

…plates

Co-authored-by: Eugene Voloshchak <eVoloshchak@users.noreply.github.com>
@eVoloshchak

Copy link
Copy Markdown
Contributor

@MelvinBot, isn't this redundant after #98779 and #99217 was implemented?

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Merged the latest main into claude-groupExportTemplates — no conflicts, and all CI checks pass.

Merge commit: 5f498e4 (a real merge commit — nothing was rebased, squashed or force-pushed).

Worth knowing: both files this PR touches moved a lot on main over the last two weeks, so the merge was not a no-op. It still auto-merged cleanly, and the PR's own diff is unchanged — still just the includesGroupExport removal in src/hooks/useSearchBulkActions.ts:2219 plus its test updates. The !isGroupedSearch guard that keeps "Basic export" hidden on grouped searches survived intact at src/hooks/useSearchBulkActions.ts:2249.

Verification after the merge

Run locally on the merged branch:

  • npm test -- tests/unit/hooks/useSearchBulkActionsExportTest.ts — 39 passed (up from 34; main added cases to the same file and they merged in alongside this PR's)
  • npm run typecheck — passed
  • npm run lint-changed — passed

On CI, every check on the merge commit is green except Check independent approval, which is the standard approval gate and not a test failure.

I could not re-verify in the browser this run — the web test environment failed to come up, so no NewDot session was available. The earlier Web and Android test runs on this PR still stand, and the main changes to useSearchBulkActions.ts were in neighbouring bulk-action code rather than the export-template branch this PR edits, so I would not expect behavioural drift. If you want a fresh pass, re-applying the Melvin-Test-Web label will kick one off.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No fix to push — nothing is broken.

After the merge of main into this branch (5f498e4), every check re-ran and the only red one is Check independent approval (run). Its log says:

Expensify/App#100520 does not have enough independent eligible reviewer approvals.

That's a human gate, not a code failure — the PR currently has zero reviews, and the workflow requires an approval from someone who isn't an author or co-author. It goes green once an independent reviewer approves.

Full check status on 5f498e4

Passing: Jest Unit Tests (all 8 jobs + Storybook), TypeScript Checks, ESLint check, Oxfmt check, Bun tests, Spell check, Knip check, Check unused styles, React Compiler Compliance Check, Check validateCode terminology, Reassure Performance Tests, Verify signed commits, CLA Assistant, Remote Build Android, Remote Build iOS.

Skipped (not applicable): Validate Github Actions and Workflows, imgbot Image Optimization, PR Author Checklist, Onyx.connectWithoutView reviewers.

Failing: Check independent approval only.

When the request came in, several jobs were still pending because the main merge had just landed — all of them have since completed successfully. Notably Jest Unit Tests passed against the freshly merged main, so the merge introduced no regressions.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 No — they don't overlap. Always open the Export submenu on Search, even with a single export option changed how many clicks it takes to reach the export option list. This PR changes what is in that list. After #99217, a group selection still reaches a one-item menu containing only "Current view".

The includesGroupExport flag this PR removes is still live on main today, at src/hooks/useSearchBulkActions.ts:1960 and :2219. #99217 carried that line through verbatim — it appears as unchanged context in its own diff.

The clearest evidence is #99217's own test, still passing on main: tests/unit/hooks/useSearchBulkActionsExportTest.ts:1222 — "hides templates when a full group is selected in an explicitly grouped search" — asserts the options are exactly ['export.currentView']. That is the bug reported in Unable to see "export To" when selecting group of expenses, and it is still asserted as expected behaviour on main.

Why the two are orthogonal, and the one place they do touch

What #99217 changed (diff): it deleted the single-option collapse and replaced it with openExportOptionsDirectlyIfSoleAction, plus added a value to each option and an ExportMenuItem type. All of that operates on subMenuItems after getExportOptions() has already returned. It never touches which options get built.

What this PR changes: it removes the guard that stops the template options from being built in the first place. A group selection on a grouped search currently fails both branches — includesGroupExport is true so it skips the templates at :2219, and isGroupedSearch is true so it also skips the basic-export fallback at :2249. It gets "Current view" and nothing else.

Nothing else surfaces the templates. useSearchBulkActions has exactly one consumer, SearchBulkActionsButton. The other template builders (useExportActions, useSelectedTransactionsActions) are report-scoped, not search-scoped, and useCombinedExportTemplates only feeds the "Exported to" filter picker.

The suppression was always cosmetic. beginExportWithTemplate at src/hooks/useSearchBulkActions.ts:1107-1122 is already group-aware — it strips the group stubs from the ID list and rewrites the query through addSelectedGroupsFilter. The templates work for a group selection; they were just never offered.

Where the two PRs do touch: this PR edits two tests #99217 added. It does not delete their behaviour — it re-fixtures them onto a deleted-selection case (CONST.REPORT.TRASH_REPORT_ID) so they still exercise a genuine single-export-option path once group selections gain templates. #99217's assertions are preserved, and coverage is added for the collapsed group_<id> stub shape, which had none.

A useful contrast that isolates the cause: :1205 "shows templates when reports are selected through their report groups" uses the same isSelectedViaGroup selection shape on the Reports tab (no groupBy) — and the templates do appear. The suppression is specific to an explicitly grouped search.

I could not verify this in a browser — the web test environment failed to start this run. The conclusion rests on the code and on the checked-in tests, which assert the hook's output directly. The earlier Web and Android test passes on this PR still stand.

@eVoloshchak

eVoloshchak commented Sep 22, 2026 •

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 image
Android: mWeb Chrome Screenshot_1790102005
iOS: HybridApp
Screen.Recording.2026-09-22.at.20.38.21.mov
iOS: mWeb Safari Simulator Screenshot - iPhone 17 - 2026-09-22 at 20 30 59
MacOS: Chrome / Safari
Screen.Recording.2026-10-12.at.14.19.28.mov

@eVoloshchak

Copy link
Copy Markdown
Contributor

@MelvinBot, please fill out the Tests sections with the following tests I've performed:

  1. On the spend page, filter by Accounting and choose card accrual, cash accrual, bank reconciliation, card statements, or any other option that has display: by group > user/card/bank
  2. Click the grouping to see the expenses
  3. Select a group of expenses
  4. Verify the Export button is displayed

@eVoloshchak eVoloshchak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@MelvinBot, makr this as ready for review

@melvin-bot

melvin-bot Bot commented Sep 22, 2026

Copy link
Copy Markdown

We did not find an internal engineer to review this PR, trying to assign a random engineer to #100074 as well as to this PR... Please reach out for help on Slack if no one gets assigned!

@melvin-bot
melvin-bot Bot requested a review from JS00001 September 22, 2026 18:46
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Filled out the Tests section with the steps you performed:

  1. On the Spend page, filter by Accounting and choose card accrual, cash accrual, bank reconciliation, card statements, or any other option that has display: by group > user/card/bank.
  2. Click the grouping to see the expenses.
  3. Select a group of expenses.
  4. Verify the Export button is displayed.

This replaced the // TODO placeholder and the suggested starting point I had left there. I diffed before and after — the Tests section is the only part of the body that changed.

Two sections are still open and need you, since I shouldn't fill them in on your behalf:

  • QA Steps — still has the // TODO placeholder. Your steps look runnable on staging as-is, so Same as tests would cover it. Say the word and I'll set that.
  • Offline tests — still empty.

I couldn't verify the steps in a browser this run — the web test environment failed to come up, so no NewDot session was available. The Web and Android runs recorded earlier in this PR still stand.

@MelvinBot
MelvinBot marked this pull request as ready for review September 22, 2026 18:51
@MelvinBot
MelvinBot requested a review from a team as a code owner September 22, 2026 18:51
@melvin-bot
melvin-bot Bot requested review from eVoloshchak and garrettmknight and removed request for a team September 22, 2026 18:51
@melvin-bot

melvin-bot Bot commented Sep 22, 2026

Copy link
Copy Markdown

@eVoloshchak 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 September 22, 2026 18:52
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Done — this PR is out of draft and marked ready for review. Reviewers have been auto-requested.

One thing to fix before it can merge: the QA Steps section still has the // TODO placeholder. That is a human-only section, so I left it alone rather than writing it for you. Either fill it in (writing "Same as tests" is fine if QA can run the steps in the Tests section) or add [No QA] to the PR title.

The Tests section is filled in, and every CI check on the head commit passes — the only red one is Check independent approval, which is the approval gate rather than a test failure.

The PR Author Checklist is also still unchecked; that is yours to complete.

I could not verify the change in the browser this run — the web test environment failed to come up, so no NewDot session was available. The earlier Web and Android test passes on this PR still stand.

@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: 5f498e4bba

ℹ️ 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/hooks/useSearchBulkActions.ts Outdated

if (!allSelectedAreDeleted && !includesGroupExport) {
// A group selection is exported through a query filter (see `addSelectedGroupsFilter`) rather than a list of IDs, so the templates work for it just like they do for an individual selection.
if (!allSelectedAreDeleted) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Route expanded group template exports through the group query

When an expanded group row is selected, its entries have transaction keys such as tx1 and carry isSelectedViaGroup, as exercised by the new test, but beginExportWithTemplate recognizes a group export only when a selected key starts with group_. Exposing templates here therefore sends {} as jsonQuery and only the currently loaded transaction IDs; if limit: leaves some group children unloaded, the resulting accounting export silently omits them. Use the same isGroupSelection detection and group-query filtering already used by handleCSVExport before making these templates available.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@MelvinBot, apply this feedback

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.

🤖 Applied in 88dacc9. The feedback was right.

  • Fix: beginExportWithTemplate now uses isGroupSelection, the same check as handleCSVExport (src/hooks/useSearchBulkActions.ts:964). An expanded group now exports through the group query, so children that limit: left unloaded are included.
  • Related gap: A template export sends either the group query or an ID list, never both. So if you select a group and also pick single expenses from elsewhere, the menu now shows only "Current view" (:2223-2227). This stops one side of the selection from being dropped silently.
Tests

I added two tests (useSearchBulkActionsExportTest.ts:1261):

  • One test checks that an expanded group with a template picked sends empty ID lists plus the grouped query. It fails with the old group_-prefix check, which received ["tx1"].
  • One test checks that a mixed group-plus-single selection offers only "Current view".

Checks I ran:

  • useSearchBulkActionsExportTest.ts: 41 passed.
  • tests/unit/hooks/useSearchBulkActions + tests/unit/Search: 65 suites and 1902 tests passed.
  • typecheck, lint on the changed files, and spell-changed: all passed.
  • React Compiler: 10 babel / 9 oxc errors, the same as before this commit.

I could not run oxfmt in this environment, so CI's Oxfmt check will confirm formatting. I did not run a browser test for this change.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

I removed [hold] from the PR title. The title is now "Offer export templates when a group row is selected on a grouped search".


view run

@eVoloshchak

Copy link
Copy Markdown
Contributor

@MelvinBot, is the updated backend request supposed to filter out some expenses? I.e. if the app says "6 selected" during the export and there are 6 expenses visible, the exported CSV is always supposed to have 6 rows? Or is there some filtering/consolidation happening on the BE?

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 For an expense-level template, yes: "6 selected" with 6 visible expenses should export 6 rows. The App side of this PR applies no extra filtering. The row count can legitimately differ in two cases:

  • "All Data - Report Level Export" gives one row per report, not one per expense. So 6 expenses on 2 reports give 2 rows. I'm inferring this from the template, not from backend code.
  • Reconciliation views send a searchKey that can add rows that aren't regular expenses, such as Expensify Card cash back. That export can have more rows than the visible list.

If you saw a mismatch with "All Data - Expense Level Export" on a non-reconciliation view, treat it as a bug. Please share the template, view, and selected/exported counts.

What the request sends for a group selection
  • jsonQuery is the view's query with groupBy and limit removed and a filter for the selected group added, plus the view's searchKey (useSearchBulkActions.ts:362-393, :1080-1101). Removing groupBy is what stops the one-row-per-group output trjExpensify saw.
  • transactionIDList holds only expenses ticked on their own, outside the group. reportIDList is empty.

I couldn't confirm the backend side from here. That includes how it combines jsonQuery with transactionIDList and whether it drops any expense types, such as pending card transactions or deleted expenses. An internal engineer would need to confirm it if the expense-level counts don't match.


view run

@eVoloshchak

Copy link
Copy Markdown
Contributor

Ah, turns out it filters out transactions with "Unreported" status when exporting; that's why there's a discrepancy.
Works well then

@eVoloshchak eVoloshchak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@melvin-bot
melvin-bot Bot requested a review from JS00001 October 12, 2026 12:59
@JS00001

JS00001 commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Ah, turns out it filters out transactions with "Unreported" status when exporting; that's why there's a discrepancy.
Works well then

@trjExpensify is this expected?

@trjExpensify

Copy link
Copy Markdown
Contributor

No. For example, "Card accruals" includes Unreported card transactions because they still need to be accrued.

@JS00001

JS00001 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

@dukenv0307 we probably need to fix that then ^

@dukenv0307

Copy link
Copy Markdown
Contributor

@MelvinBot investigate #100520 (comment), #100520 (comment). Is it BE bug?

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Yes, this looks like a backend bug. The App asks for unreported expenses, but the template export leaves them out.

For a Card accruals group, the App sends QueueExportSearchWithTemplate with this jsonQuery:

type:expense status:unreported,drafts,outstanding feed:<feed> cardID:<id> plus searchKey: unapprovedCard

This PR didn't cause it. main already sends the same query for "Reconciliation - All Expenses" on a card group. This PR just offers more templates, so the gap is easier to see. I'd track the backend fix separately so it doesn't block this PR. This appears to be a backend/internal issue. A separate internal investigation is needed.

Notes
  • I couldn't read the backend code, so I can't say where the rows are dropped.
  • Cash accruals is different. Its query is status:drafts,outstanding with no unreported, so it hides unreported expenses in the list too. That's expected.
  • When you tick single expenses, the App sends unreported IDs in transactionIDList and leaves report 0 out of reportIDList (:753-761). I didn't check whether that path keeps them.

view run

@dukenv0307

Copy link
Copy Markdown
Contributor

@MelvinBot create an E/E issue then assign me

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 I created an internal Expensify/Expensify issue for the backend bug and assigned you. I'm not linking it here because this repo is public. You'll find it in your assigned issues.

The issue covers template exports that leave out unreported expenses, such as unreported card expenses in Card accruals. It includes the request the App sends and the steps to reproduce.


view run

@dukenv0307

Copy link
Copy Markdown
Contributor

@MelvinBot I think it shouldn't block this PR since it's a pre-existing issue, right?

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Yes, I agree it shouldn't block this PR. The backend drops unreported expenses for the same request on main. This PR doesn't change the export query.

  • Same request on main: main already offers "Reconciliation - All Expenses" for a card group. It sends the same jsonQuery, with status:unreported,drafts,outstanding and the card filter. The query-building code (useSearchBulkActions.ts:367-393) and the export call (:1081-1101) match main.
  • What this PR changes: It offers more templates for a group selection. That makes the backend gap easier to hit, but it doesn't create it.
  • Tracking: The backend fix has its own internal issue, which is assigned to you.

trjExpensify said Card accruals exports should include unreported expenses. Before merging, confirm with them that shipping these templates before the backend fix is acceptable.


view run

@dukenv0307

Copy link
Copy Markdown
Contributor

@trjExpensify I think we can merge it and handle the issue above separately. Is it ok cc @JS00001

@JS00001

JS00001 commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Yeah we can handle it separately, that sounds good

@JS00001
JS00001 merged commit ae3538e into main Oct 14, 2026
45 of 48 checks passed
@JS00001
JS00001 deleted the claude-groupExportTemplates branch October 14, 2026 13:14
@github-actions

Copy link
Copy Markdown
Contributor

🚧 JS00001 has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/JS00001 in version: 9.5.10-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No help site update needed. This PR makes the existing export templates available when you select a group row, which is what users would already expect. No article says that only Current view is offered for grouped selections, and the Expensify Card reconciliation article already tells users to "Choose an export template," which is now more accurate.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/JS00001 in version: 9.5.12-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No help site update needed. This PR makes the existing export templates available when you select a group row, which is what users would already expect. No article says that only Current view is offered for grouped selections, and the Expensify Card reconciliation article already tells users to "Choose an export template," which is now more accurate.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/cristipaval in version: 9.5.12-1 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

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

Labels

Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants