Repository navigation
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d06d5e51a
ℹ️ 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!
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppCleanShot.2026-09-09.at.11.38.15.1.mp4Android: mWeb ChromeCleanShot.2026-09-09.at.11.50.15.3.mp4iOS: HybridAppCleanShot.2026-09-09.at.11.58.50.5.mp4iOS: mWeb SafariCleanShot.2026-09-09.at.11.53.55.4.mp4 |
|
An issue where the primary action doesn't match the preview is also happening on the latest staging (v9.4.74-0) and is unrelated to this PR. I'll investigate and raise a separate bug report for it. CleanShot.2026-09-09.at.12.03.11.1.mp4 |
|
@FitseTLT We have a bug with invited members assigned the workspace admin role: the Export button only shows up the first time they log in/clean cache CleanShot.2026-09-09.at.12.40.06.1.mp4 |
|
@FitseTLT How about your troubleshooting for this issue? |
|
Yeah reproduced will check thx |
|
@suneox Yeah you found a gap on our implementation. As I have mentioned in my proposal this is a workaround solution and in the case of admin(which is neither owner nor preffered exporter) the BE doesn't update the errorFields of the report when it fails so the export will disappear with our current implementation. Two options we have
|
|
waiting on the above @suneox |
I'll take another look tomorrow |
|
I lost track of this one on my K2, and I'll try to troubleshoot it tomorrow |
@FitseTLT I just double check at 0:50 -> 1:04 the export action is still on the client side, and this only happens with invited members. We still receive the 'started export' and 'failed export' messages and then the export button doesn't reappear after receiving the failure message |
Yeah @suneox as you know the solution we are implementing here uses the |
@MelvinBot analyze this explanation and propose a solution. |
AnalysisRoot cause: Proposed fix: Also treat the export as finished when a new export-outcome report action shows up.
This keeps option 1 without its downside. A plain admin can only see Export in the header, and the header already has the report actions. The preview stays on the error-count check, which works there. A backend "export pending" field (option 2) would still be cleaner long term. Raise that internally. Why the preview isn't affected
Next steps: Reply with |
|
@FitseTLT Could you double-check and try to verify this approach? |
|
@suneox Updated to the report actions approach and we don't need the count approach anymore. NOw it works from both sides. |
Thanks @FitseTLT the report actions approach is a real improvement, I can confirm this issue has been fixed CleanShot.2026-10-23.at.09.42.22.1.mp4@MelvinBot @codex review PR with the updated approach |
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. |
Review of the report-actions approachThe approach works and is simpler than the count snapshot. I'd fix one regression before merge. Three other items are worth a look. 1. The More menu can now fire a duplicate export, with no confirmation (fix before merge). 2. A request-level failure keeps the button hidden until the user dismisses the error. 3. Client clock skew could strand the button (please verify). 4. Every preview now subscribes to the full report-actions collection. Test nits
|
|
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 |
Re-review after "address comments"Nearly ready. Items 1, 2, and 4 from my last review are fixed, and the new tests cover them. I'd still close item 3 before merge. All checks pass except Fixed
1. Clock skew can still hide the button for good (fix before merge). 2. A successful export ends the attempt only if the server clears Nit
|
|
@MelvinBot review |
Re-review after the second "address comments"Looks good to me. The latest commit fixes the open items from my previous review, and I found nothing new that blocks merge.
I also checked that "Mark as exported" still behaves correctly with the new outcome rule. Its pending action briefly counts as a running export. Then Every check passes except |










Explanation of Change
The export button stayed visible during an export because nothing in Onyx recorded that an export was running. The optimistic
isExportedToIntegration: truehid the button, but it also made the report look already exported, and the button could stay hidden once the real result came back.This removes that optimistic write. A new
isExportInProgressworks out the running state from the report actions: it looks for a pending "started exporting" action with no errors, stamped with server time, and checks that no result came after it. A failure message, a finished export or an approval reset all count as a result. The header, the report preview and More → Export now hide the export while one is running, and it comes back as soon as the result arrives or the request fails.Fixed Issues
$ #98025
PROPOSAL: #98025 (comment)
Tests
ETB.Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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
2026-09-03.15-46-36.mp4
Android: mWeb Chrome
2026-09-03.16-12-26.mp4
iOS: Native
iOS: mWeb Safari
2026-09-03.16-14-24.mp4
MacOS: Chrome / Safari
2026-09-03.16-14-59.mp4