fix: update the report primary action for when an export fails#72292
Conversation
|
@dominictb 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] |
This comment was marked as outdated.
This comment was marked as outdated.
i'll add the screenshots today |
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.
|
|
@dominictb i added the screenshots but the next steps message is still updated as expected. Can you please check on your end? |
|
Yeah the next step message is not working for me. It's
(screenshot from @nkdengineer) cc @dangrous Can you check if it's working on your side? |
|
Oh weird, it was working before? Here are the test steps I was using (and QA used) on the backend change -
Does that differ in a relevant way from the testing here? |
|
@dangrous Here's my test steps:
So it's different from your test steps because it failed right from the first place. I expect that whenever export failed, we should show the |
|
oh okay i think i see what's happening here - so if you refresh the page with your report, does it hten show the We don't send an Onyx update with the new next step when the export fails, but if you leave and come back it should be the updated message. That's what I'm seeing, at least |
@dangrous It's not working either: Screen.Recording.2025-10-29.at.19.48.38.mov |
|
|
@dominictb Updated. |
|
@dangrous I noticed that it only works when the report is auto sync. If I manually export before auto sync, the message won't show. |
|
oh interesting, I must have missed a condition somewhere. Let me try to repro and get an update out there if I can figure it out! |
|
It should be set! Feel free to test again and if we're still seeing issues let me know |
|
However, after successfully exporting the report,
The report did get exported to Intacct:
@dangrous Can you help us to quick check why? I don't see this property being handled in the FE, read only. |
|
@dangrous Can you share the progress of checking ☝️ bug? |
|
So sorry, I missed this! Looking now |
|
Sorry for the delay, I've put into review a backend PR that should fix this issue! |
|
Thank you for the prompt work. Let us know when it's ready for retesting. |
|
@dangrous Can you let us know if the BE PR had been deployed to prod? |
|
augh so sorry @dominictb - yes, BE fix is on prod! I did not follow up on this, apologies |
|
@nkdengineer Can you resolve conflicts and retest? |
Reviewer Checklist
Screenshots/VideosiOS: mWeb Safari |
|
@dangrous Not sure if this is expected behavior:
|
|
Screen.Recording.2025-12-17.at.01.00.21.movI can add your test account to this report to help with debugging because you don't have access to Sage Intacct. |
@dominictb I updated. |
So sorry I missed this; I believe this IS correct coz export is different from sync? At least, it seemed like it had similar behavior before this. Will review and merge this shortly! |
|
🚀 Deployed to staging by https://github.com/dangrous in version: 9.2.85-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.85-7 🚀
|
| const isReportApproved = isReportApprovedUtils({report}); | ||
| const isReportClosed = isClosedReportUtils(report); | ||
| const isProcessingReport = isProcessingReportUtils(report); | ||
| const isExported = isExportedUtil(reportActions); |
There was a problem hiding this comment.
This introduced #87654. isExportedUtil(reportActions) is called without report, so it falls back to scanning report actions instead of reading report.isExportedToIntegration. After a failed export, the optimistic action persists, making isExported incorrectly return true and showing Pay instead of Export. Fixed in #90154.














Explanation of Change
Fixed Issues
$ #70101
PROPOSAL: #70101 (comment)
Tests
Waiting for [preferredExporter] to export this reportOffline tests
Same as tests
QA Steps
Same as tests
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.mov
Android: mWeb Chrome
android.mov
iOS: Native
i.mov
iOS: mWeb Safari
im.mov
MacOS: Chrome / Safari
MacOS: Desktop