Skip to content

[Due for payment 2025-10-06] [$250] Expense - After exporting to CSV, "Unapprove report" modal shows up when unapproving report #70011

Closed

Description

@jponikarchuk

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.2.2-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: https://test-management.browserstack.com/projects/2219752/test-runs/TR-701/folder/14961849/44221364/881813355
Email or phone of affected tester (no customers): sdjoinsdjosjisd@gmail.com
Issue reported by: Applause Internal Team
Device used: Mac 15.5 / Chrome
App Component: Money Requests

Action Performed:

Precondition:

  • Workspace is not connected to accounting system.
  1. Go to staging.new.expensify.com
  2. Go to workspace chat.
  3. Create a manual expense in workspace chat.
  4. Open the expense report.
  5. Click Submit and Approve.
  6. Click More > Export > All Data - report level.
  7. Wait for download message from Concierge to show up.
  8. Click More > Unapprove.

Expected Result:

The report will be unapproved without "Unapprove report" confirm modal because the workspace is not connected to accounting system.

Actual Result:

After exporting to All Data - report level, "Unapprove report" confirm modal shows up when unapproving the report.
"Heads up! This report has already been exported to ." is incomplete.

Workaround:

Unknown

Platforms:

  • Android: App
  • Android: mWeb Chrome
  • iOS: App
  • iOS: mWeb Safari
  • iOS: mWeb Chrome
  • Windows: Chrome
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6936697_1757147455588.Screen_Recording_2025-09-06_at_16.29.03.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021965413328973230319
  • Upwork Job ID: 1965413328973230319
  • Last Price Increase: 2025-09-16
  • Automatic offers:
    • nkdengineer | Contributor | 108865660
Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @nkdengineer

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Sep 6, 2025
  2. melvin-bot commented on Sep 6, 2025

    @melvin-bot

    Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

  3. samranahm commented on Sep 6, 2025

    @samranahm
    Contributor

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    Expense - After exporting to CSV, "Unapprove report" modal shows up when unapproving report

    What is the root cause of that problem?

    we are not checking for connected integration here

    [CONST.REPORT.SECONDARY_ACTIONS.UNAPPROVE]: {
    text: translate('iou.unapprove'),
    icon: Expensicons.CircularArrowBackwards,
    value: CONST.REPORT.SECONDARY_ACTIONS.UNAPPROVE,
    onSelected: () => {
    if (isDelegateAccessRestricted) {
    showDelegateNoAccessModal();
    return;
    }
    if (isExported) {
    setIsUnapproveModalVisible(true);
    return;
    }
    unapproveExpenseReport(moneyRequestReport);
    },
    },

    What changes do you think we should make in order to solve the problem?

    we should check if we have connected integration before setting UnapproveModalVisible true here

    [CONST.REPORT.SECONDARY_ACTIONS.UNAPPROVE]: {
    text: translate('iou.unapprove'),
    icon: Expensicons.CircularArrowBackwards,
    value: CONST.REPORT.SECONDARY_ACTIONS.UNAPPROVE,
    onSelected: () => {
    if (isDelegateAccessRestricted) {
    showDelegateNoAccessModal();
    return;
    }
    if (isExported) {
    setIsUnapproveModalVisible(true);
    return;
    }
    unapproveExpenseReport(moneyRequestReport);
    },
    },

    const connectedIntegration = getValidConnectedIntegration(policy);
    if (isExported && connectedIntegration) { 

    What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

    N/A

    What alternative solutions did you explore? (Optional)

    we should explicitly check connectedIntegration in isExported function and return false if there is no connected integration
    Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

  4. aadil42 commented on Sep 6, 2025

    @aadil42
    Contributor

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    "Unapprove report" modal shows up after unapproving the report even when no Accounting system was connected.

    What is the root cause of that problem?

    This is the root cause.

    if (isExported) {

    We're only checking if the data was exported. We should also check if an Accounting system is setup.

    What changes do you think we should make in order to solve the problem?

    We should simply add for integrationNameFromExportMessage a check in the existing if condition here.

    We already get integrationNameFromExportMessage here:

    const integrationNameFromExportMessage = useMemo(() => {
    if (!isExported) {

    if (isExported) {

    if (isExported && integrationNameFromExportMessage) {
                        setIsUnapproveModalVisible(true);
                        return;
                    }

    What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

    N/A

    What alternative solutions did you explore? (Optional)

    Or we can have the condition as:

    if (isExported && (connectedIntegration || integrationNameFromExportMessage) ) {
            setIsUnapproveModalVisible(true);
            return;
    }
  5. lorretheboy commented on Sep 6, 2025

    @lorretheboy
    Contributor

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    After exporting to All Data - report level, "Unapprove report" confirm modal shows up when unapproving the report.
    "Heads up! This report has already been exported to ." is incomplete

    What is the root cause of that problem?

    We don't check if user connected integration before showing modal here

    if (isExported) {

    What changes do you think we should make in order to solve the problem?

    The easiest way is update that condition to
    if (isExported && connectedIntegration) {

    What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

    NA

    What alternative solutions did you explore? (Optional)

    Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

  6. lorretheboy commented on Sep 6, 2025

    @lorretheboy
    Contributor

    Note for C+ review here:

    I don't try to post a duplicated version. When I post my proposal, his proposal looks like below picture and I see it is different with my proposal (diff in code changes and place to change). That's why I posted here. However, after the first proposal updated their solution (He didnt notice), I found somehow is 2 proposals are pretty the same. So I will let u decide, if u think my proposal is duplicated from beginning, I will remove it

    Here is the first proposal's original version, it uses the function to get connectedIntegration while mine version doesn't use it. And the change scope my version is outside his proposal (This one looks like due to different code version in git)

    The first propoosal original version:
    Image

    My proposal:
    Image

  7. samranahm commented on Sep 6, 2025

    @samranahm
    Contributor

    @lorretheboy mind telling us what was different in your proposal that you think your proposal was not duplicated??

    This issue was related to not having the check for connectedIntegration and I already mentioned that. I just make sure to keep the isExported check there while adding connectedIntegration check, in update you mentioned.

    Please take a look:

    • Note: Before submitting a proposal on an issue, be sure to read any other existing proposals. ALL NEW PROPOSALS MUST BE DIFFERENT FROM EXISTING PROPOSALS. The difference should be important, meaningful or considerable.
  8. aadil42 commented on Sep 6, 2025

    @aadil42
    Contributor

    @samranahm, Hello. In your solution I think you suggested to create connectedIntegration while in my suggestion and in @lorretheboy suggestion we were using existing variables. This was the diff I think.

  9. lorretheboy commented on Sep 6, 2025

    @lorretheboy
    Contributor

    Hi, I have mentioned here to compare your original proposal with my proposal. It is clearly different (I am not C+, so I don't check the details how u handle it but for the first glance, it is different both in places to change and how we change). Please note that I'm not reviewer, so I can't go to check every single proposal details implementation to view how many percent it is duplicated

    Here is the comparision of our two proposals. Your first proposal is creating connectedIntegration while mine is not

    Image

    .
    .
    .
    .

    So I will let u decide, if u think my proposal is duplicated from beginning, I will remove it

    Lastly, I have also mentioned here. If C+ think it is really duplicated, then I'm happy to remove, I don't try to post the duplicated version

  10. nkdengineer commented on Sep 6, 2025

    @nkdengineer
    Contributor

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    After exporting to All Data - report level, "Unapprove report" confirm modal shows up when unapproving the report.
    "Heads up! This report has already been exported to ." is incomplete.

    What is the root cause of that problem?

    We only pass reportActions to check the isExported here, then we skip report?.isExportedToIntegration check because the report is undefined. When we export CSV, a report action exported to All Data - report level is returned and it's a EXPORTINTEGRATION. So the isExported function returns true, and the warning modal appears

    const isExported = useMemo(() => isExportedUtils(reportActions), [reportActions]);

    App/src/libs/ReportUtils.ts

    Lines 11039 to 11060 in 34c2f92

    function isExported(reportActions: OnyxEntry<ReportActions> | ReportAction[], report?: OnyxEntry<Report>): boolean {
    // If report object is provided and has the property, use it directly
    if (report?.isExportedToIntegration !== undefined) {
    return report.isExportedToIntegration;
    }
    // Fallback to checking actions for backward compatibility
    if (!reportActions) {
    return false;
    }
    let exportIntegrationActionsCount = 0;
    let integrationMessageActionsCount = 0;
    const reportActionList = Array.isArray(reportActions) ? reportActions : Object.values(reportActions);
    for (const action of reportActionList) {
    if (isExportIntegrationAction(action)) {
    // We consider any reports marked manually as exported to be exported, so we shortcut here.
    if (getOriginalMessage(action)?.markedManually) {
    return true;
    }
    exportIntegrationActionsCount++;

    What changes do you think we should make in order to solve the problem?

    1. We can pass the expense report to isExportedUtils function here to also check the isExportedToIntegration if it exists. If we want to do that we need to ensure update isExportedToIntegration in the optimistic data when we export to an integration or manually export.

    const isExported = useMemo(() => isExportedUtils(reportActions), [reportActions]);

    1. We should check the label/type in the original message of EXPORTINTEGRATION action to consider whether it's exported to integration or not when calculating exportIntegrationActionsCount here

    App/src/libs/ReportUtils.ts

    Lines 11039 to 11060 in 34c2f92

    function isExported(reportActions: OnyxEntry<ReportActions> | ReportAction[], report?: OnyxEntry<Report>): boolean {
    // If report object is provided and has the property, use it directly
    if (report?.isExportedToIntegration !== undefined) {
    return report.isExportedToIntegration;
    }
    // Fallback to checking actions for backward compatibility
    if (!reportActions) {
    return false;
    }
    let exportIntegrationActionsCount = 0;
    let integrationMessageActionsCount = 0;
    const reportActionList = Array.isArray(reportActions) ? reportActions : Object.values(reportActions);
    for (const action of reportActionList) {
    if (isExportIntegrationAction(action)) {
    // We consider any reports marked manually as exported to be exported, so we shortcut here.
    if (getOriginalMessage(action)?.markedManually) {
    return true;
    }
    exportIntegrationActionsCount++;

    OPTIONAL: We can also rename this function to isExportedToIntegration

    What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

    Add a test case to isExported function with an exported to All Data - report level action and verify that this function returns false

    What alternative solutions did you explore? (Optional)

    NA

    Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

  11. samranahm commented on Sep 6, 2025

    @samranahm
    Contributor

    I can't go to check every single proposal details implementation to view how many percent it is duplicated

    @lorretheboy No one is 100% percent perfect and can't expect someone to be. But I believe that It does not took a lot of time to check what other developers has posted and is there any proposal with the same RCA you found. Even if there's is, It actually make us more confident that we found the correct RCA.

    Next we need to make sure our solution does not contain the same logic or same approach others have already posted. No one can post the whole implementation in proposal but we get the complete idea what they are proposing. So we should make sure to post something different instead to altering and adjusting the already posted proposal to make yours look like different one.

  12. lorretheboy commented on Sep 6, 2025

    @lorretheboy
    Contributor

    Btw I dont really agree that it is duplicated with your original version, thats why I left the comment for C+ decision

  13. samranahm commented on Sep 6, 2025

    @samranahm
    Contributor

    Let them handle that.

  14. 51 remaining items

  15. changed the title [-][$250] Expense - After exporting to CSV, "Unapprove report" modal shows up when unapproving report[/-] [+][Due for payment 2025-10-06] [$250] Expense - After exporting to CSV, "Unapprove report" modal shows up when unapproving report[/+] on Sep 29, 2025
  16. melvin-bot commented on Sep 29, 2025

    @melvin-bot

    Reviewing label has been removed, please complete the "BugZero Checklist".

  17. melvin-bot commented on Sep 29, 2025

    @melvin-bot

    The solution for this issue has been 🚀 deployed to production 🚀 in version 9.2.19-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

    If no regressions arise, payment will be issued on 2025-10-06. 🎊

    For reference, here are some details about the assignees on this issue:

  18. melvin-bot commented on Sep 29, 2025

    @melvin-bot

    @dukenv0307 @twisterdotcom The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

  19. dukenv0307 commented on Sep 30, 2025

    @dukenv0307
    Contributor

    BugZero Checklist:

    • [Contributor] Classify the bug:
    Bug classification

    Source of bug:

    • 1a. Result of the original design (eg. a case wasn't considered)
    • 1b. Mistake during implementation
    • 1c. Backend bug
    • 1z. Other:

    Where bug was reported:

    • 2a. Reported on production
    • 2b. Reported on staging (deploy blocker)
    • 2c. Reported on a PR
    • 2z. Other:

    Who reported the bug:

    • 3a. Expensify user
    • 3b. Expensify employee
    • 3c. Contributor
    • 3d. QA
    • 3z. Other:
    • [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

      Link to comment: https://github.com/Expensify/App/pull/61704/files#r2390917533

    • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

      Link to discussion: N/A

    • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again. Yes

    Regression Test Proposal Template

    Regression Test Proposal

    Test:

    1. Go to workspace chat.
    2. Create a manual expense in workspace chat.
    3. Open the expense report.
    4. Click Submit and Approve.
    5. Click More > Export > All Data - report level.
    6. Wait for download message from Concierge to show up.
    7. Click More > Unapprove.
    8. Verify that the report is unapproved without a warning modal

    Do we agree 👍 or 👎

  20. twisterdotcom commented on Oct 9, 2025

    @twisterdotcom
    Contributor

    Payment Summary:

  21. nkdengineer commented on Oct 16, 2025

    @nkdengineer
    Contributor

    @twisterdotcom I will request payment via NewDot

  22. trjExpensify commented on Oct 17, 2025

    @trjExpensify
    Contributor

    approved $250 for @dukenv0307.

  23. JmillsExpensify commented on Nov 3, 2025

    @JmillsExpensify
    Contributor

    $250 approved for @nkdengineer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.ExternalAdded to denote the issue can be worked on by a contributorWeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions