[Payment due @ahmedGaber93] Enable group exports#88577
Conversation
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.
|
|
@MelvinBot what exactly is causing the ESLint check to fail that's related to this PR? |
|
The full-repo ESLint check fails because your changes in
The "Changed files ESLint check" passed because it only lints the diff hunks, but the full-repo ESLint check has a max-warnings threshold that these new warnings push over. Fix: Add |
|
@MelvinBot can you write comprehensive testing steps for grouped exports and also ensure that existing export functionalities continue to work correctly? |
This comment has been minimized.
This comment has been minimized.
|
@MelvinBot can you hide this comment as "Low Quality"? |
|
@ahmedGaber93 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a4bd07816
ℹ️ 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".
|
All The possible Additionally, Why any groupBy can be emptyGroups are created upfront from backend metadata before their individual transactions are fetched. A group can have
The group creation functions (e.g., |
|
Pushed the fix @ahmedGaber93. let me know if we're missing anything |
|
@getusha The export scenarios worked as expected in the latest test. The remaining issue only affects small screens; everything works correctly on larger screens.
I think we can move forward once this bug is fixed. Screen.Recording.2026-05-31.at.8.37.25.PM.mov |
|
@ahmedGaber93 done! |
|
We have a production issue with grouping by merchant. The merchant grouping logic uses a "includes" check instead of strict equality, which leads to inconsistent expense counts and row counts. Screen.Recording.2026-06-01.at.2.07.56.PM.movThis affects the consistency between the selected groups and the exported output (merchant only). However, I believe the export behavior here is correct. The root cause appears to be the merchant grouping logic, which uses a "includes" check instead of strict equality Screen.Recording.2026-06-01.at.2.04.05.PM.movMay be we can handle this with the follow-ups or report it on slack. |
|
🎯 @ahmedGaber93, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
|
Follow-ups:
|
|
🚧 @JS00001 has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@MelvinBot can you create issues for the follow up issues listed here #88577 (comment) and assign me |
|
Created the 4 follow-up issues from this comment and assigned them to I created them in
|
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.3.91-0 🚀
Bundle Size Analysis (Sentry): |
Help site review: changes required ✅This PR enables group exports — when expenses on the Spend page are grouped (via Display > Group by) and the groups are selected, choosing Basic export now produces an export that preserves the group structure (each group is its own section with a header row, with its expenses listed below). This user-facing behavior wasn't documented anywhere in the help site. The closest article, Export Expenses and Reports, covered CSV/PDF export but said nothing about exporting grouped results. Draft help site PR: #92318 What it adds to
How the UI labels were verifiedI verified the flow against the live app and used exact labels per
Note: the existing article still uses some older labels (e.g. "Selected", "Basic Export", "All Data – Expense Level Export"). I scoped this PR to the grouped-export addition only and did not rewrite those, to keep the change focused — flagging in case you want a follow-up. @getusha, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
Hi @getusha @JS00001 I used these fake_transactions-2026-01-16.10_56_55.629.csv in wallet and have this result. Is it pass, could you please check and let me know? Thank you in advance Recording.3799.mp4 |
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.3.94-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.3.94-0 🚀
|
|
🤖 Payment issue created: #92451 |
|
@izarutskaya Yes, it passed. The result shown in your video is expected and matches the test steps' expected outcome. |
Explanation of Change
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/596490
PROPOSAL:
Tests
Offline tests
N/a
QA Steps
same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari