Support displaying a pending state in the report status for To-Do search results when an action is pending#79736
Conversation
…ing while offline Signed-off-by: Tsaqif <tsaiinkwa@yahoo.com>
Signed-off-by: Tsaqif <tsaiinkwa@yahoo.com>
Signed-off-by: Tsaqif <tsaiinkwa@yahoo.com>
Signed-off-by: Tsaqif <tsaiinkwa@yahoo.com>
Signed-off-by: Tsaqif <tsaiinkwa@yahoo.com>
|
@dukenv0307 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
Lines 1769 to 1772 in c882930
The getReportSections filtering now only checks status for EXPENSE queries and no longer considers the action filter that powers the Submit/Approve/Pay/Export canned searches. This means when results are built locally (e.g., offline or when using cached snapshot data), an action:approve/action:submit query can show reports in the wrong state because the action predicate is never applied. That’s a regression in the search semantics (the query still includes action), so those tabs/filters will be inconsistent with the intended results unless action filtering is restored or scoped to allow the pending exception explicitly.
ℹ️ 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".
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
src/components/Search/index.tsx
Outdated
| return [filteredData1, filteredData1.length, allLength]; | ||
| }, [ | ||
| searchKey, | ||
| isFocused, |
There was a problem hiding this comment.
do we need this change?
There was a problem hiding this comment.
@dukenv0307 Yes, it’s needed so the data gets updated when the user goes back to the search list after viewing the report.
If I don’t include this, the pending data won’t update properly.
| archivedReportsIDList: archivedReportsIdSet, | ||
| isActionLoadingSet, | ||
| cardFeeds, | ||
| shouldSkipActionFiltering: true, |
There was a problem hiding this comment.
Why do we need to remove shouldSkipActionFiltering? I don't see you mentioning it in the proposal.
There was a problem hiding this comment.
This was added to fix a regression caused by the offending PR. Since I am reverting the original PR, this is no longer needed.
| type ReportSearchHeaderProps = { | ||
| /** Report, if we're showing the details for one and using AvatarWithDisplay */ | ||
| report?: OnyxEntry<Report>; | ||
| report?: ExpenseReportListItemType; |
There was a problem hiding this comment.
Actually, the data type passed to ReportSearchHeader is ExpenseReportListItemType.
and OnyxEntry is only part of it. Since isReportStatePending is not part of OnyxEntry<Report>, I need to use the full type.
| @@ -1795,11 +1779,6 @@ | |||
| shouldShow = isValidExpenseStatus(status) ? expenseStatusActionMapping[status](reportItem) : false; | |||
There was a problem hiding this comment.
Please add a comment for this change.
There was a problem hiding this comment.
I didn’t modify these lines—below this, I’m only reverting the offending PR. What comment are you referring to?
|
Offending PR is being reverted. Please hold this for #79714 |
|
@dukenv0307 I’ve replied to your comments. Should we tag or loop in the design team? |
|
#79714 is merged |
|
@tsa321 can you please merge main? |
|
@dukenv0307 I have merged main |
|
@tsa321 Thank you, can you please confirm with @Expensify/design team about showing the offline indicator? |
|
Sounds good! |
|
@trjExpensify Since PR #79506 has been merged, reports are now listed under the correct action tab when offline. Do we still want to show a pending state on the status field for reports that have been submitted or approved while offline? It would look something like this: Screen.Recording.2026-01-24.at.07.53.37.mov |
|
@trjExpensify @luacmartins can you please take a look at the comment above ^? |
|
Not sure if we need the pending state, but the main issue is fixed now. cc @trjExpensify |
|
Mhm... I think we probably should still do the pending state change as it can be rolled back when coming back online if it fails. 👍 |
Signed-off-by: Tsaqif <tsaiinkwa@yahoo.com>
|
@dukenv0307 I’ve updated the code. |
| const {isLargeScreenWidth} = useResponsiveLayout(); | ||
|
|
||
| const statusContainerStyle = useMemo(() => { | ||
| return [isLargeScreenWidth ? styles.mt1 : styles.mt0Half, report?.isReportStatePending && styles.offlineFeedbackPending]; |
There was a problem hiding this comment.
Should apply offlineFeedbackPending if report?.isReportStatePending and isOffline?
Signed-off-by: Tsaqif <tsaiinkwa@yahoo.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-01-29.at.10.06.16.movAndroid: mWeb ChromeScreen.Recording.2026-01-29.at.10.01.53.moviOS: HybridAppScreen.Recording.2026-01-29.at.10.06.45.moviOS: mWeb SafariScreen.Recording.2026-01-29.at.10.02.16.movMacOS: Chrome / SafariScreen.Recording.2026-01-29.at.09.56.24.mov |
|
Woo |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/robertjchen in version: 9.3.11-16 🚀
|
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.3.12-1 🚀
|
Explanation of Change
This PR displays a lower-opacity (pending) state for the report’s status field when an action is pending in the To-Do search results.
Fixed Issues
$ #78599
PROPOSAL: #78599 (comment)
Tests
Precondition: Have some draft reports (two is sufficient).
Offline tests
QA Steps
Precondition: Have some draft reports (two is sufficient).
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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_native_d.mp4
Android: mWeb Chrome
android_mweb_d.mp4
iOS: Native
ios_native_d.mp4
iOS: mWeb Safari
ios_msafari_d.mp4
MacOS: Chrome / Safari
macos_web_d.mp4