Skip to content

[Due for payment 2026-10-28] [$175] Migrate the Expensify Card statement PDF modal to the global modal stack #103508

New issue
New issue
Closed
Task
Closed
[Due for payment 2026-10-28] [$175] Migrate the Expensify Card statement PDF modal to the global modal stack#103508
Task
Assignees
mallenexpensifyroryabrahamdaledah
Labels
Awaiting PaymentAuto-added when associated PR is deployed to productionDailyKSv2

Description

@roryabraham
roryabraham
opened on Oct 8, 2026

Problem

The Expensify Card statement PDF flow mounts ExpensifyCardStatementPDFDownloadModal inline from Search bulk actions and owns its visibility and cleanup state in the caller. Move it to the imperative/global modal stack.

Scope

  • Migrate ExpensifyCardStatementPDFDownloadModal and the Search bulk-action flow.
  • Reuse the global PDF modal pattern established by the report PDF migration.
  • Pass statement parameters through the show API while preserving the asynchronously populated statement key.
  • Remove obsolete visibility state, inline mounting, and modal-hide plumbing from Search.

Acceptance criteria

  • Card statement PDF downloads open through the global modal stack.
  • The modal remains reactive while statement generation completes and downloads the correct file.
  • Selection cleanup still occurs after the modal has finished hiding.
  • The one-feed-at-a-time alert and existing error/cancel behavior remain unchanged.
  • Relevant Search and statement PDF tests are updated or added.

Reference

This work was identified in the Slack discussion: https://expensify.slack.com/archives/D0C6V1Z9C03/p1791482470733119

Issue OwnerCurrent Issue Owner: @mallenexpensify
Upwork Automation - Do Not Edit

Activity

  1. roryabraham
    assigned
    daledah
    and
    roryabraham
    on Oct 8, 2026
  2. roryabraham
    added the
    Task
    issue type on Oct 8, 2026
  3. roryabraham
    added
    WeeklyKSv2
    on Oct 8, 2026

  4. daledah commented on Oct 12, 2026

    @daledah
    daledah
    on Oct 12, 2026
    Contributor

    @MelvinBot please post a proposal to this issue

  5. MelvinBot commented on Oct 12, 2026

    @MelvinBot
    MelvinBot
    on Oct 12, 2026
    Contributor

    Proposal

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

    The Expensify Card statement PDF modal is mounted inline in the Search bulk-action bar. Search owns its visibility, params and hide cleanup. It should open through the global modal stack, like the report PDF modal does now.

    What is the root cause of that problem?

    This flow was not part of the report PDF migration in Clear Search selection immediately when downloading a single report PDF. So it still uses the old pattern:

    • useSearchBulkActions keeps isExpensifyCardStatementPDFModalVisible and expensifyCardStatementPDFParams. It merges statementKey into the params after GetExpensifyCardStatementPDF returns.
    • SearchBulkActionsButton renders <ExpensifyCardStatementPDFDownloadModal> inline.
    • Selection is cleared in handleExpensifyCardStatementPDFModalHide. The cleanup waits for hide because the modal lives inside the selection-gated bar and would unmount if the selection cleared first.

    statementKey only comes from the API response, not from Onyx. So the modal can't just read it after it opens, the way ReportPDFDownloadModal reads its filename.

    Code references

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

    1. Add a wrapper. Create src/components/Modal/Global/ExpensifyCardStatementPDFDownloadModalWrapper.tsx, modeled on ReportPDFDownloadModalWrapper. It takes statementParams and keeps a local isVisible. onClose hides it, and onModalHide calls closeModal() only after it is hidden. The promise therefore resolves after the hide animation ends.
    2. Add a show hook. Create useExpensifyCardStatementPDFDownloadModal returning showExpensifyCardStatementPDFDownloadModal({statementParams}) and closeExpensifyCardStatementPDFDownloadModal(). It calls showModal with a fixed id, CONST.MODAL.EXPENSIFY_CARD_STATEMENT_PDF or similar.
    3. Push statementKey in place. In exportExpensifyCardStatementPDF, show the modal with the initial params. When the response returns, call the show function again with {...statementParams, statementKey}. showModal updates the props of an open modal with the same id instead of stacking a new one (ModalContext.tsx:81-91). The modal doesn't remount, so the Onyx-driven auto-download still fires once the file is ready.
    4. Guard stale responses. Keep the existing requestID check. Also bump the request id when the modal's promise resolves. A late response then can't reopen a modal the user already closed, because a same-id call after close would push a new modal.
    5. Clean up after hide. Chain .then(() => clearSelectedTransactions()) on the show promise. Selection still clears only after the modal has finished hiding.
    6. Errors. In showStatementError, call closeModalByID(id) instead of the visibility setter, then show the existing download-failed modal. The one-feed-at-a-time alert and the offline check are unchanged.
    7. Remove the old plumbing. Delete the visibility and params state, setIsExpensifyCardStatementPDFModalVisible, handleExpensifyCardStatementPDFModalHide, their return values, and the inline mount in SearchBulkActionsButton.
    8. Tests.
      • Update the statement cases in tests/unit/hooks/useSearchBulkActionsDownloadPDFTest.ts to mock the new hook. Assert the first show args, the in-place update with statementKey, closeModalByID on error, stale-request handling, and that clearSelectedTransactions runs only after the show promise resolves.
      • Add a small wrapper test: closeModal must not run until onModalHide fires after close.
    Note on a related open PR

    Use useDecisionModal for Search bulk actions and SelectionToolbar alerts is open. It edits the same files and moves the one-feed-at-a-time and download-failed alerts to useDecisionModal(). This change should be based on it once it merges, so step 6 would call its error helper.

    What alternative solutions did you explore? (Optional)

    The wrapper could start GetExpensifyCardStatementPDF itself on mount and keep statementKey in local state. That removes the need for in-place updates and the request-id ref. However, it moves the API call and error handling out of Search and changes more behavior. The issue also asks to pass the statement params through the show API, so I prefer the approach above.

    Next Steps for Contributor+ team:
    To accept: @MelvinBot implement [this](https://github.com/Expensify/App/issues/103508) to create a draft PR.
    To refine: @MelvinBot <your feedback>
    To reject: Explain why you are rejecting Melvin's proposal.

    view run

  6. melvin-bot
    added
    DailyKSv2
    and removed
    WeeklyKSv2
    on Oct 12, 2026

  7. daledah commented on Oct 12, 2026

    @daledah
    daledah
    on Oct 12, 2026
    Contributor

    @MelvinBot implement this

  8. MelvinBot commented on Oct 12, 2026

    @MelvinBot
    MelvinBot
    on Oct 12, 2026
    Contributor

    🤖 Draft PR created: #103708
    Please mark it as "Ready for review" or ask me to do that for you when it is ready for review.

    The PR follows the proposal. The card statement PDF modal now opens through the global modal stack. The statementKey is pushed into the open modal in place, and the selection clears after the modal hides. On failure the selection is kept, as before.

    I couldn't push to your fork, so the branch is on upstream. Unit tests, typecheck and lint pass. No browser test was run, because the test account has no Expensify Card settlements to export. Please fill in the Tests and QA sections.

    Use useDecisionModal for Search bulk actions and SelectionToolbar alerts edits the same files. Whichever PR merges second will need a small merge.

    view run

  9. melvin-bot
    added
    ReviewingHas a PR in review
    WeeklyKSv2
    and removed
    DailyKSv2
    on Oct 15, 2026
  10. melvin-bot
    changed the title [-]Migrate the Expensify Card statement PDF modal to the global modal stack[/-] [+][$175] Migrate the Expensify Card statement PDF modal to the global modal stack[/+] on Oct 15, 2026

  11. melvin-bot commented on Oct 15, 2026

    @melvin-bot
    melvin-botBot
    on Oct 15, 2026 – with Melvin Bot

    Job added to Upwork: https://www.upwork.com/jobs/~022110638070580322345

  12. 7 remaining items

  13. daledah commented on Oct 23, 2026

    @daledah
    daledah
    on Oct 23, 2026
    Contributor

    Contributor+ Checklist:

    • [Contributor] The offending PR and associated issue have been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

      Link to the comment on the PR: N/A

    • [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:

    • [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.

    Regression Test Proposal

    Precondition:

    • sign in as an admin of a workspace with the Expensify Card enabled and at least two Expensify Card settlements.

    Test:

    Download a statement

    1. Go to Reports and search for type:expense group-by:withdrawal-id withdrawal-type:expensify-card.
    2. Select one settlement.
    3. Open the bulk actions menu and click Download statement.
    4. Verify the "Download statement" modal opens and shows "Please wait while we generate the PDF."
    5. Verify the message changes to "Your PDF has been generated!" and the PDF downloads automatically.
    6. Close the modal.
    7. Verify the modal closes and the settlement selection is cleared.

    Close while generating

    1. Select one settlement and click Download statement.
    2. Close the modal right away, while it still says "Please wait while we generate the PDF."
    3. Wait a few seconds.
    4. Verify the modal does not reopen.
    5. Select the settlement again and click Download statement.
    6. Verify the modal opens and the PDF downloads.

    Multiple feeds

    1. Select settlements from two different Expensify Card feeds. Skip this test if the account has only one feed.
    2. Click Download statement.
    3. Verify the "Please select settlements from one Expensify Card feed at a time." alert appears and no statement modal opens.

    Regression: report PDF

    1. Search for type:expense-report, select one report, and click Download report in the bulk actions menu.
    2. Verify the report PDF modal still opens and downloads the PDF.

    Do we agree 👍 or 👎

  14. melvin-bot
    added
    WeeklyKSv2
    DailyKSv2
    Awaiting PaymentAuto-added when associated PR is deployed to production
    Overdue
    and removed
    DailyKSv2
    WeeklyKSv2
    Overdue
    on Oct 23, 2026

  15. melvin-bot commented on Oct 27, 2026

    @melvin-bot
    melvin-botBot
    on Oct 27, 2026 – with Melvin Bot

    Triggered auto assignment to @mallenexpensify (Awaiting Payment)

  16. melvin-bot
    assigned
    mallenexpensify
    on Oct 27, 2026

  17. melvin-bot commented on Oct 27, 2026

    @melvin-bot
    melvin-botBot
    on Oct 27, 2026 – with Melvin Bot

    Payment Summary

    Resolving PRs:

    BugZero Checklist (@mallenexpensify)

    • I have confirmed assignees, roles, and Upwork contracts look correct
    • I have paid out Upwork contracts / manual NewDot requests
    • [BugZero Assignee] I have created a GH issue for creating/updating the regression test once above steps have been agreed upon
  18. melvin-bot
    added
    Overdue
    on Oct 29, 2026

  19. mallenexpensify commented on Oct 30, 2026

    @mallenexpensify
    mallenexpensify
    on Oct 30, 2026
    Contributor

    Payment Summary

    Contributor+: @daledah due $175 via NewDot

  20. mallenexpensify
    closed this as completedon Oct 30, 2026
  21. melvin-bot
    removed
    Overdue
    on Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionDailyKSv2

Type

Task

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions