Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
1 similar comment
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@MelvinBot review |
Code reviewI found no blocking bugs. The selection, exclusion and checkbox logic holds up. There is one gap to fix and one edge case to check. 1. Fix: "Download receipts" still shows when a partly loaded group is checked. The merged selection swaps the group's rows for its To match the "only whole-group actions" rule, gate this option on 2. Check: deleting an expense from a header-checked, partly loaded group.
What I checked and found correct
This review is from reading the code at |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
…election-exclusions # Conflicts: # src/components/Search/SearchBulkActionsButton.tsx # tests/unit/Search/SearchSelectionFooterTest.tsx
…ep row actions next to a cash back row
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
|
All yours @situchan |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safaritest1.2.movtest3.4.movtest5.6.movtest7.mov |
|
Please fix conflict |
This is failing. I don't see "Select all" option when list grouped by category Screen.Recording.2026-10-18.at.5.08.00.PM.movPlease make sure to add screen recordings for all 10 test cases |
This test is failing too. Screen.Recording.2026-10-18.at.5.10.37.PM.movPlease make sure to validate test cases yourself (if wrong, correct them). Otherwise this PR will be fail QA. |
|
Bug: Fully loaded group is treated as still loading when its count is stale Precondition: Report A has 2 expenses and Report B has 1.
Expected: "2 selected" Screen.Recording.2026-10-18.at.5.26.48.PM.mov |
This is failing. Screen.Recording.2026-10-18.at.5.40.28.PM.mov |
This is failing because of the bug happening on main. Shows skeleton and then disappears. New expense is not added to the group until collapse and expand again test5.6.mov |
I remember testing all 10 before the account ban. Have you merged main when testing I think that my have led to inconsistencies Anyways will review all tomorrow thanks |
If so, why not uploaded in checklist
yes, I merged main. Fixed conflicts locally. |
Because I didn't record vid formally. The account got rate limited and banned before recording. |
Explanation of Change
This is part 4 of #90539 (shift+click range selection). While building ranges in Search, I found that when a group hadn't loaded all of its expenses, its checkbox, the selected count and total, and the bulk actions disagreed about what was selected. This PR makes them agree.
In grouped searches (by category, tag and so on), a group loads its expenses a page at a time. With this change:
With Select all on:
Fixed Issues
$ #90539
PROPOSAL:
Tests
Prerequisites: use a workspace with its default categories and add these expenses:
Test 1. Checking a group that hasn't fully loaded selects all of it
Test 2. A group shows partly checked when only some of it is selected
Test 3. Expenses that load later come in checked
Test 4. Expenses checked lower down stay checked after you open an expense
Test 5. Checking every expense of a group one by one shows a full check
Test 6. A new expense added to a fully checked group stays unchecked
Test 7. Deleting an expense keeps its group checked
Test 8. An expense moved to another category goes with it
Test 9. Select all: unchecking a group that hasn't fully loaded
This needs a grouped list long enough that, with nothing selected, clicking the checkbox at the top of the list opens a menu with "Select all on this page" and "Select all". If it selects the page straight away, the list is too short for this test.
Test 10. Select all, then select the page again
This needs an ungrouped Expenses list with more than 50 expenses.
Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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.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