Skip to content

[$250] IOU create action gets wrong actorAccountID when a non-owner opens a report before the owner, making auto-reported card expenses read-only for the owner #92769

Closed

Description

@melvin-bot

Coming from here.


Problem

For an auto-reported company-card expense on an OPEN expense report, the expense can become read-only for the report owner (the card holder) — no fields are editable and tags (e.g. Projects) aren't offered for editing — even though sibling transactions on the same card/feed/report are editable. The expense is correctly editable in Classic.

Root cause

NewDot gates money-request editing on isRequestor (currentUserAccountID === reportAction.actorAccountID) in canEditMoneyRequest (ReportUtils.ts#L4737-L4773). Once that returns false, canEditFieldOfMoneyRequest (ReportUtils.ts#L4941-L4948) returns false for every field, so the whole expense is read-only. (Admins/managers short-circuit to true, which is why an admin can still edit it.)

So the bug hinges on the IOU create action's actorAccountID being stamped as the wrong user.

For old transactions without a transaction thread, openReport materializes the money-request preview action and derives the actor from the parent report owner, falling back to the current user:

const parentReport =
    transactionParentReportID === optimisticSelfDMReport?.reportID
        ? optimisticSelfDMReport
        : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;

src/libs/actions/Report/index.ts

When a non-owner opens the report before the owner does, allReports may not yet have the parent report loaded client-side, so parentReport resolves to undefined. submitterAccountID then falls back to currentUserAccountID (the non-owner who opened it), and optimisticIOUAction.actorAccountID is stamped with that account. Because this optimistic action is persisted to the server (via parameters.moneyRequestPreviewReportActionID), the wrong actor becomes canonical — permanently making the expense read-only for the actual owner.

Repro (conceptual)

  1. Auto-report a company-card transaction (old/legacy, no transaction thread) to user A's (card holder/report owner) OPEN expense report.
  2. Have a different user B (e.g. an admin/copilot) open that report before user A, while the parent report isn't yet in B's local Onyx cache.
  3. User A can no longer edit that expense (no editable fields, no Projects tag); sibling transactions remain editable.

Expected

The IOU create action's actorAccountID should always be the card holder / report owner, regardless of who opens the report first.

Suggested fix direction

The intended logic (use parentReport.ownerAccountID) is already wired up but silently falls back to currentUserAccountID when the parent report isn't in the local cache. Options:

  • Ensure the parent report is fetched/available before running the legacy-transaction fallback, or
  • Resolve the owner authoritatively (server-side) instead of relying on client-side Onyx state.

Evidence

Diagnosed from production logs for a real report where two identical company-card transactions (same card, same feed, same OPEN report) differed only in the IOU create action's actorAccountID: the editable one was attributed to the report owner, the non-editable one to another user who had opened the report first.

Upwork Automation - Do Not Edit

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jun 5, 2026
  2. MelvinBot commented on Jun 5, 2026

    @MelvinBot
    Contributor

    Issue Analysis

    Root Cause: The expense goes read-only for the owner because the IOU create action's actorAccountID is stamped as a non-owner, so the owner fails the isRequestor check that gates all editing. The wrong attribution is created when a non-owner opens the report first — but the part that makes it permanent/canonical is backend-side, because the App never transmits the actor to the server (only the action ID). A pure-App change cannot fix the persisted data.

    Recommended Fix: This requires a backend change (outside the Expensify/App repo) and should be escalated to an internal engineer. The OpenReport backend command that materializes the missing money-request preview action for a legacy transaction must attribute the IOU create action's actorAccountID to the expense report owner / card holder, not to the user who happened to call OpenReport first. No App code fix is recommended (see "Why a client-only fix won't work" below).

    Evidence chain (verified in App code)

    All permalinks at App de5724c.

    1. Read-only symptom is driven by isRequestor.
    canEditMoneyRequest returns false for a non-admin/non-manager when isRequestor is false, and isRequestor is currentUserAccountID === reportAction.actorAccountID:

    Once canEditMoneyRequest is false, every field is locked because canEditFieldOfMoneyRequest returns early:

    This matches the symptoms exactly: no editable fields, no Projects tag, while siblings with the correct actor stay editable, and Classic is unaffected.

    2. The wrong actor originates in the legacy-transaction fallback in openReport.
    For old transactions with no transaction thread (transaction && !parentReportActionID), the App materializes the preview action and derives the actor from the parent report owner, falling back to the current user when the parent report isn't in local Onyx:

    When a non-owner opens first and the parent report isn't cached, submitterAccountID falls back to the non-owner → optimistic actor = non-owner.

    Why a client-only fix won't work (the key refinement to the hypothesis)

    The original write-up says the optimistic wrong actor "is persisted to the server … the wrong actor becomes canonical." The App code shows the actor itself is never sent — OpenReport only transmits the action ID:

    So the backend independently decides the actor when it creates the canonical preview action. The production evidence (canonical actor == the user who opened first) indicates the backend attributes it to the OpenReport caller rather than the report owner. Consequences:

    • The App's ?? currentUserAccountID fallback is optimistic-only, and is actually correct when the owner opens first (current user == owner). It does not transmit, and cannot fix, the persisted attribution.
    • Therefore the permanent/canonical bug is backend behavior and needs an internal fix — confirming the actor for a materialized legacy preview action should be the expense report owner.

    (Confidence note: the backend attribution is a strong inference from the App code + the issue's production logs; it is not code-verified here because backend repos are internal.)

    Possibly related: #92519 — "Admin unable to add unreported credit card transactions to a cardholder's existing report … without opening the report first." Same area (admins, card transactions, report-open ordering); worth checking whether it shares this root cause.


    Next Steps for Contributor+ team:
    To act: This is a backend/internal fix — please escalate to an internal engineer with access to the OpenReport command to correct the actorAccountID attribution for materialized legacy preview actions.
    To refine: @MelvinBot <your feedback>
    To reject: Explain why you are rejecting this analysis.

  3. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Jun 5, 2026
  4. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Jun 5, 2026
  5. David-2002065 commented on Jun 5, 2026

    @David-2002065
    Contributor

    Proposal

    What is the root cause of that problem?

    The legacy-transaction fallback in openReport stamps optimisticIOUAction.actorAccountID from parentReport?.ownerAccountID ?? currentUserAccountID, and the ?? reaches the second arm whenever the parent report isn't already in the client's allReports collection. For a non-owner opening the report first, that resolves to the non-owner's accountID, and because the optimistic action ID is sent to the server via parameters.moneyRequestPreviewReportActionID and the response doesn't overwrite the client's optimistic actorAccountID, the wrong actor becomes the canonical attribution. Downstream, canEditMoneyRequest gates on currentUserAccountID === reportAction.actorAccountID, so the actual owner sees the expense as read-only.

    The offending block, line by line:

    https://github.com/Expensify/App/blob/main/src/libs/actions/Report/index.ts#L1604-L1697

    // Some old transactions don't have a transaction thread or money request preview because they were
    // created before we started requiring them. When users open the report, we build
    // the missing pieces here so the UI works. Backend also creates them via moneyRequestPreviewReportActionID.
    // https://github.com/Expensify/App/issues/80180 - remove once all old transactions are migrated
    if (transaction && !parentReportActionID) {
        const transactionParentReportID = parentReportID ?? transaction?.reportID;
        const iouReportActionID = rand64();
    
        // Get the parent report to determine the actual submitter/owner of the expense
        // Use optimisticSelfDMReport if provided (when selfDM exists but wasn't in allReports)
        const parentReport =
            transactionParentReportID === optimisticSelfDMReport?.reportID
                ? optimisticSelfDMReport
                : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
        const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;
        ...
    
        // Override actor fields to show the submitter instead of current user.
        optimisticIOUAction.actorAccountID = submitterAccountID;
        optimisticIOUAction.person = [{style: 'strong', text: submitterPersonalDetails?.displayName ?? submitterEmail, type: 'TEXT'}];
        optimisticIOUAction.avatar = submitterPersonalDetails?.avatar;
    
        ...
        parameters.moneyRequestPreviewReportActionID = iouReportActionID;
    }

    Two reasons the parentReport lookup misses for a non-owner who arrives first:

    1. openReport is the API call that fetches the report. At the moment optimistic data is being assembled, allReports[reportID] typically isn't populated yet for a first-visit. For a card-fed legacy expense report, the owner reaches it through lastVisitedPath or LHN entry and their OpenApp payload usually includes the report; a non-owner (admin, copilot, Slack-linked viewer) often arrives without it pre-loaded.
    2. parentReport resolution has no async retry. The allReports[...] read is synchronous against the module-level snapshot at src/libs/actions/Report/index.ts:465-470. If the value isn't there, the fallback chain parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID resolves to the second arm. There's no "wait for the report to be loaded, then re-derive the owner" — the actorAccountID is baked into the optimistic action at the moment openReport runs.

    Then parameters.moneyRequestPreviewReportActionID = iouReportActionID ships the client-generated action ID to the server. The server creates the canonical action under that ID. The Onyx state already has the wrong actorAccountID attached to that ID, and the successData for this branch doesn't include an override that resets it — so the optimistic value sticks. From there, the gates in canEditMoneyRequest and canEditFieldOfMoneyRequest see the wrong actor and lock the actual owner out:

    https://github.com/Expensify/App/blob/main/src/libs/ReportUtils.ts#L4737-L4773

    https://github.com/Expensify/App/blob/main/src/libs/ReportUtils.ts#L4941-L4948

    The asymmetry the issue calls out — sibling transactions on the same card/feed/report remain editable — is consistent with this: the editable siblings had their optimistic action built when the owner (or someone with the parent report cached) opened the report; only the row whose action was first materialized by the wrong user is broken.

    A few important properties of this bug that drive the fix scope:

    • It only affects legacy transactions (the !parentReportActionID branch) — modern transactions carry their own thread/preview action and never enter this fallback.
    • It only fires once per transaction (the optimistic action gets persisted; the branch exits via parentReportActionID becoming non-empty on subsequent opens).
    • The damage is durable: once the wrong actor is stamped server-side, no client-side re-derivation can undo it. The fix has to either prevent the wrong stamp or repair past damage.

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

    Two tracks: a client-side fix that prevents new instances of the bug, and a backend remediation that repairs the existing wrong-actor rows.

    1. Don't fabricate the IOU create action client-side when we don't have authoritative owner context. The optimistic action is a UX nicety that lets the row render with the right attribution before the server responds. If we don't know who the owner is, fabricating with the wrong actor is worse than not fabricating at all — the row briefly renders with no attribution, then the server's response fills it in correctly. Update the fallback:

      if (transaction && !parentReportActionID) {
          const transactionParentReportID = parentReportID ?? transaction?.reportID;
    -     const iouReportActionID = rand64();
    -
    -     // Get the parent report to determine the actual submitter/owner of the expense
    -     // Use optimisticSelfDMReport if provided (when selfDM exists but wasn't in allReports)
    -     const parentReport =
    -         transactionParentReportID === optimisticSelfDMReport?.reportID
    -             ? optimisticSelfDMReport
    -             : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
    -     const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;
    +     // Resolve the parent report so we can stamp the correct submitter. If we can't,
    +     // skip the optimistic action entirely — the server will create it with the correct actor
    +     // via moneyRequestPreviewReportActionID and the UI will refresh on the success response.
    +     // Without this guard, the actorAccountID falls back to currentUserAccountID and gets
    +     // persisted, locking the actual owner out of editing the expense.
    +     const parentReport =
    +         transactionParentReportID === optimisticSelfDMReport?.reportID
    +             ? optimisticSelfDMReport
    +             : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
    +
    +     if (!parentReport?.ownerAccountID) {
    +         Log.alert('[Report] Skipping legacy IOU optimistic action: parent report owner unknown', {
    +             reportID,
    +             transactionParentReportID,
    +             transactionID: transaction.transactionID,
    +             currentUserAccountID,
    +         });
    +         // Still send the API call without moneyRequestPreviewReportActionID — the server
    +         // owns this case end-to-end when the client can't derive the owner.
    +     } else {
    +         const iouReportActionID = rand64();
    +         const submitterAccountID = parentReport.ownerAccountID;
    +         ...
    +         parameters.moneyRequestPreviewReportActionID = iouReportActionID;
    +     }
          ...
      }

    The Log.alert breadcrumb is important — it tracks how often this path is taken and gives the team a measurable signal that the backend-only fallback is being exercised, which de-risks the conservative branch.

    2. Add a defensive recheck in canEditMoneyRequest for legacy card-fed transactions. Even after (1), there are reports out there that already have the wrong actorAccountID baked in. The cheap, scoped repair: when the gate would return false because actorAccountID !== currentUserAccountID, check whether the report's ownerAccountID === currentUserAccountID and whether the transaction is card-fed. If yes, treat the current user as the requestor for editing purposes. This is a narrow carve-out that only fires for the exact damaged shape the bug produces:

      function canEditMoneyRequest(reportAction: OnyxInputOrEntry<ReportAction>, transaction?: OnyxInputOrEntry<Transaction>): boolean {
          const isDeleted = isDeletedAction(reportAction);
          ...
          const isRequestor = currentUserAccountID === reportAction?.actorAccountID;
    +     // Legacy-IOU repair: if the canonical actorAccountID was stamped wrong by a non-owner
    +     // opening the report first (https://github.com/Expensify/App/issues/92769), fall back
    +     // to the report's ownerAccountID. Scoped to card-fed transactions to avoid relaxing
    +     // edit rules elsewhere.
    +     const moneyRequestReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${(reportAction as ReportAction).reportID}`];
    +     const isOwnerOnCardTransaction =
    +         !!moneyRequestReport?.ownerAccountID &&
    +         moneyRequestReport.ownerAccountID === currentUserAccountID &&
    +         (transaction?.managedCard === true || !!transaction?.cardID);
    +     const effectiveIsRequestor = isRequestor || isOwnerOnCardTransaction;
          ...
    -     if (!isRequestor) {
    +     if (!effectiveIsRequestor) {
              return isAdmin || isManager;
          }
          ...
      }

    The scope (managedCard === true || !!transaction?.cardID) keeps this from affecting non-card flows where actorAccountID is the source of truth. The check is also short-circuited by the existing admin/manager branches above, so admins continue to follow their own path.

    3. Server-side repair pass. The wrong-actor rows already in production won't fix themselves through (1) or (2) — they'll just stop accumulating new ones. An Auth engineer should sweep the IOU create actions where actorAccountID !== iouReport.ownerAccountID for card-fed transactions and overwrite them to the report owner. (2) buys time for that repair to roll out without leaving customers locked out in the meantime.

    4. Add a regression test pinning the invariant. A Jest test that calls openReport with transaction set, parentReportActionID empty, and allReports not containing the parent report. Assert that either no moneyRequestPreviewReportActionID is sent OR the resulting optimisticIOUAction.actorAccountID === parentReport.ownerAccountID. Either outcome satisfies the invariant the issue is calling out; whichever the team picks for (1) is the one the test checks.

    I'd land (1) and (2) together as the user-facing fix. (3) is the durable repair the backend team owns. (4) goes with whichever of (1) lands.

    What alternative solutions did you explore? (Optional)

    I considered fetching the parent report inline when it's missing — await Report.fetchReport(transactionParentReportID) before computing submitterAccountID. That works but turns openReport into a multi-step async flow (it currently builds optimistic data and dispatches the API call synchronously), which would force a refactor of every caller that expects openReport to be fire-and-forget. The conservative branch in (1) — skip the optimistic action and let the server handle it — is the smallest behavioral change with the same correctness.

    I also looked at deriving the submitter from the transaction itself for card-fed cases (cardID → cardList[cardID].cardholderAccountID). That's correct in practice and avoids the parent-report dependency entirely, but couples the legacy-IOU-creation path to the card-list cache being loaded, which has its own pre-load timing. The check in (2) — "if the gate would deny but the user owns the report and the transaction is card-fed, allow" — captures the same intent without taking on the new dependency, and it composes cleanly with (1) for the going-forward case and with the server-side repair for the historical case.

  6. KJ21-ENG commented on Jun 5, 2026

    @KJ21-ENG
    Contributor

    Proposal

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

    For legacy auto-reported company-card expenses that do not already have a transaction thread / money request preview, the first person who opens the expense report can cause the generated IOU create action to be attributed to the wrong account. If that first opener is not the report owner / card holder, the real owner later fails the isRequestor check and the expense becomes read-only in NewDot.

    What is the root cause of that problem?

    The legacy transaction fallback in openReport() builds and persists a missing money-request preview action. It tries to set the action actor from the parent expense report owner:

    const parentReport =
        transactionParentReportID === optimisticSelfDMReport?.reportID
            ? optimisticSelfDMReport
            : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
    const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;

    The problem is the fallback to currentUserAccountID. In several entry points, createTransactionThreadReport() already receives the expense report from the route/search/list snapshot, but openReport() then re-reads the parent report from allReports. If that collection does not contain the parent report yet, parentReport is undefined, so the preview action is stamped with the opener's account instead of the expense report owner's account.

    That action ID is sent as moneyRequestPreviewReportActionID in the OpenReport request, and the backend also materializes the preview action for this legacy path. Once the wrong actorAccountID is saved, canEditMoneyRequest() treats the actual owner as not being the requestor, and canEditFieldOfMoneyRequest() returns false for every field.

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

    Remove the viewer-account fallback from this legacy preview path and make the submitter/actor source authoritative.

    On the App side, pass the known parent expense report (or at least its ownerAccountID) from createTransactionThreadReport() into openReport() when creating a transaction thread from an iouReport. Then, in the legacy transaction fallback, resolve submitterAccountID from:

    1. the explicit parent report / parent report owner passed by the caller,
    2. optimisticSelfDMReport for self-DM/unreported cases,
    3. allReports[REPORT + transactionParentReportID]?.ownerAccountID.

    If none of those sources provide an owner, do not fall back to currentUserAccountID for a reported expense. Instead, skip client creation of the money-request preview for that open, or fetch/open with enough parent report data first, so we never generate a persisted IOU action with the wrong actor.

    On the API/backend side, make OpenReport create the missing money-request preview actor from the transaction's parent report owner/card holder, not from the authenticated opener. The client can still provide moneyRequestPreviewReportActionID as the idempotency/action ID, but the server should derive actorAccountID from the transaction/report relationship before saving the action. This prevents stale or missing client Onyx state from making the wrong actor canonical.

    What alternative solutions did you explore? (Optional)

    Changing canEditMoneyRequest() to also allow the report owner when the IOU action actor is wrong would make the affected expense editable, but it would leave corrupted report-action attribution in place and could mask other permission bugs. The safer fix is to create the preview action with the correct actor and avoid persisting the current viewer as a substitute owner.

  7. moxw commented on Jun 5, 2026

    @moxw

    Proposal

    What is the root cause of that problem?

    For legacy transactions with no transaction thread / money request preview, openReport() creates the missing preview action in src/libs/actions/Report/index.ts.

    In that fallback, the submitter is derived from the parent report:

    const parentReport =
        transactionParentReportID === optimisticSelfDMReport?.reportID ? optimisticSelfDMReport : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
    
    const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;

    This is unsafe because createTransactionThreadReport() often already has the correct iouReport, but it only merges it into Onyx and then immediately calls openReport(). The module-level allReports value used by openReport() can still be stale/missing at that moment.

    When a non-owner opens the legacy card expense first, parentReport can be undefined, so the preview IOU action is created with the opener’s currentUserAccountID as actorAccountID. That wrong preview action ID is also sent as moneyRequestPreviewReportActionID, so the server can create the canonical preview with the wrong actor.

    Later, canEditMoneyRequest() in src/libs/ReportUtils.ts checks:

    const isRequestor = deprecatedCurrentUserAccountID === reportAction?.actorAccountID;

    For the real report owner/card holder this is now false, and canEditFieldOfMoneyRequest() returns false for every field. That is why the owner sees the expense as read-only while admins/managers can still edit through the separate admin/manager bypass.

    This is intermittent because it depends on who opens the old transaction first and whether the parent expense report has already hydrated into allReports.

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

    Fix the legacy preview creation path so it never derives the IOU create actor from the user who happens to open the report.

    Minimal fix:

    1. Extend the internal openReport() params to accept the resolved parent money request report, e.g. parentReport?: Report.
    2. In createTransactionThreadReport(), pass the already resolved reportToUse/iouReport into openReport().
    3. In openReport() legacy fallback, resolve the parent report in this order:
      • optimisticSelfDMReport when applicable,
      • the explicitly passed parent report when its reportID matches transactionParentReportID,
      • allReports[REPORT_${transactionParentReportID}].
    4. Only use currentUserAccountID as the actor for self-DM / tracked-expense cases where the current user is actually the submitter. For policy expense reports, require parentReport.ownerAccountID; if it is missing, do not create/send moneyRequestPreviewReportActionID yet, and log the missing parent context instead of creating a poisoned preview action.

    That keeps the existing edit permission model intact and fixes the data corruption at the source.

    Regression coverage:

    Add a test beside the existing legacy preview fallback test in tests/actions/ReportTest.ts that simulates:

    1. current user B is signed in,
    2. transaction belongs to expense report R,
    3. createTransactionThreadReport() is called with an iouReport whose ownerAccountID is user A,
    4. allReports does not yet contain R at the moment openReport() derives the preview actor.

    Assert that the created IOU action under REPORT_ACTIONS_R has actorAccountID === A, not B, and that the OpenReport API call still includes the generated moneyRequestPreviewReportActionID.

    Also keep/extend the self-DM legacy fallback test to confirm unreported/tracked expenses can still use the current user when there is no policy expense report owner.

    What alternative solutions did you explore? (Optional)

    Loosening canEditMoneyRequest() to compare against report.ownerAccountID would make this specific owner editable again, but it leaves the wrong canonical actorAccountID in the preview action and risks changing permissions for unrelated IOU actions.

    Only changing the optimistic UI action is also insufficient because the same preview action ID is sent through moneyRequestPreviewReportActionID; the server-side creation path must not be triggered with an opener-derived actor when the real parent owner is unknown.

  8. melvin-bot commented on Jun 5, 2026

    @melvin-bot
    Author

    Triggered auto assignment to Contributor-plus team member for initial proposal review - @cretadn22 (External)

  9. mukhrr commented on Jun 5, 2026

    @mukhrr
    Contributor

    Proposal

    What is the root cause of that problem?

    This is caused by the legacy transaction fallback added for old transactions that do not yet have a transaction thread or money-request preview.

    That path runs only when transaction && !parentReportActionID. It creates an optimistic IOU action, links it to the transaction thread, and passes moneyRequestPreviewReportActionID to OpenReport, so the backend creates the missing preview action with that same ID.

    Permalinks:

    • Legacy fallback creates the preview:
      // Some old transactions don't have a transaction thread or money request preview because they were
      // created before we started requiring them. When users open the report, we build
      // the missing pieces here so the UI works. Backend also creates them via moneyRequestPreviewReportActionID.
      // We log at the end of this block to track how often this path gets hit.
      // https://github.com/Expensify/App/issues/80180 - remove once all old transactions are migrated
      if (transaction && !parentReportActionID) {
      const transactionParentReportID = parentReportID ?? transaction?.reportID;
      const iouReportActionID = rand64();
      // Get the parent report to determine the actual submitter/owner of the expense
      // Use optimisticSelfDMReport if provided (when selfDM exists but wasn't in allReports)
      const parentReport =
      transactionParentReportID === optimisticSelfDMReport?.reportID ? optimisticSelfDMReport : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
      const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;
      const submitterEmail = PersonalDetailsUtils.getLoginsByAccountIDs([submitterAccountID]).at(0) ?? currentUserLogin ?? '';
      const submitterPersonalDetails = PersonalDetailsUtils.getPersonalDetailByEmail(submitterEmail);
      const optimisticIOUAction = buildOptimisticIOUReportAction({
      type: isSelfDM(parentReport) ? CONST.IOU.REPORT_ACTION_TYPE.TRACK : CONST.IOU.REPORT_ACTION_TYPE.CREATE,
      amount: Math.abs(transaction.amount),
      currency: transaction.currency,
      comment: transaction.comment?.comment ?? '',
      participants: [],
      transactionID: transaction.transactionID,
      isOwnPolicyExpenseChat: false,
      reportActionID: iouReportActionID,
      iouReportID: transactionParentReportID,
      // delegateAccountIDParam: will be threaded in PR 15; buildOptimisticIOUReportAction falls back to module-level Onyx.connect value (https://github.com/Expensify/App/issues/66425)
      delegateAccountIDParam: undefined,
      });
      // Override actor fields to show the submitter instead of current user.
      // This is needed for legacy transactions where the approver opens the transaction but it should show the submitter
      optimisticIOUAction.actorAccountID = submitterAccountID;
      optimisticIOUAction.person = [
      {
      style: 'strong',
      text: submitterPersonalDetails?.displayName ?? submitterEmail,
      type: 'TEXT',
      },
      ];
      optimisticIOUAction.avatar = submitterPersonalDetails?.avatar;
      // We have a case where the transaction data is only available from the snapshot
      // So we need to add the transaction data to Onyx so it's available when opening the report
      optimisticData.push({
      onyxMethod: Onyx.METHOD.MERGE,
      key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`,
      value: transaction,
      });
      // Add violations if they exist. This is needed when opening from search results where
      // violations are in the snapshot but not yet synced to the main collections, so we need
      // to add them to Onyx to ensure they show up in the transaction thread
      if (transactionViolations) {
      optimisticData.push({
      onyxMethod: Onyx.METHOD.MERGE,
      key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`,
      value: transactionViolations,
      });
      }
      // Attach the optimistic IOU report action created for the transaction to the transaction thread
      // Set chatReportID to link back to the parent policy expense chat for proper avatar rendering
      optimisticData.push({
      onyxMethod: Onyx.METHOD.MERGE,
      key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
      value: {
      parentReportActionID: iouReportActionID,
      chatReportID: transactionParentReportID,
      },
      });
      optimisticData.push({
      onyxMethod: Onyx.METHOD.MERGE,
      key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionParentReportID}`,
      value: {
      [iouReportActionID]: {
      ...optimisticIOUAction,
      childReportID: reportID,
      isOptimisticAction: true,
      },
      },
      });
      parameters.moneyRequestPreviewReportActionID = iouReportActionID;
    • OpenReportParams includes moneyRequestPreviewReportActionID:
      /**
      * The ID of the report action to create for an unreported transaction thread.
      * Used when we need to create a money request report action on the selfDM for an unreported expense.
      */
      moneyRequestPreviewReportActionID?: string;
    • Tracking issue confirms backend also creates these legacy previews through this parameter: Track legacy transactions missing IOU report actions #80180

    Inside that fallback we try to derive the submitter from the parent report:

    const parentReport =
        transactionParentReportID === optimisticSelfDMReport?.reportID ? optimisticSelfDMReport : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
    const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;

    If the non-owner opens first and does not have transactionParentReportID loaded in allReports, parentReport is undefined, so submitterAccountID becomes the current viewer. Then we stamp that viewer onto the preview action:

    optimisticIOUAction.actorAccountID = submitterAccountID;

    Editing later depends on that actor. canEditMoneyRequest computes isRequestor from reportAction.actorAccountID, and canEditFieldOfMoneyRequest returns false before any field-specific checks when canEditMoneyRequest is false.

    Permalinks:

    • isRequestor check:

      App/src/libs/ReportUtils.ts

      Lines 4736 to 4760 in 632ff2f

      const moneyRequestReportID = originalMessage?.IOUReportID;
      const isRequestor = deprecatedCurrentUserAccountID === reportAction?.actorAccountID;
      if (!moneyRequestReportID) {
      return actionType === CONST.IOU.REPORT_ACTION_TYPE.TRACK && isRequestor;
      }
      const moneyRequestReport = report ?? getReportOrDraftReport(String(moneyRequestReportID));
      const isSubmitted = isProcessingReport(moneyRequestReport);
      if (isIOUReport(moneyRequestReport)) {
      return isSubmitted && isRequestor;
      }
      // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
      const reportPolicy = policy ?? getPolicy(moneyRequestReport?.policyID);
      const isAdmin = reportPolicy?.role === CONST.POLICY.ROLE.ADMIN;
      const isManager = deprecatedCurrentUserAccountID === moneyRequestReport?.managerID;
      if (isInvoiceReport(moneyRequestReport) && (isManager || isChatReportArchived)) {
      return false;
      }
      // Admin & managers can always edit coding fields such as tag, category, billable, etc.
      if (isAdmin || isManager) {
      return true;
    • field edit gate:

      App/src/libs/ReportUtils.ts

      Lines 4908 to 4948 in 632ff2f

      function canEditFieldOfMoneyRequest({
      reportAction,
      fieldToEdit,
      isDeleteAction = false,
      isChatReportArchived = false,
      outstandingReportsByPolicyID,
      transaction,
      report,
      policy,
      }: {
      reportAction: OnyxInputOrEntry<ReportAction>;
      fieldToEdit: ValueOf<typeof CONST.EDIT_REQUEST_FIELD>;
      isDeleteAction?: boolean;
      isChatReportArchived?: boolean;
      outstandingReportsByPolicyID?: OutstandingReportsByPolicyIDDerivedValue;
      transaction: OnyxEntry<Transaction>;
      report?: OnyxInputOrEntry<Report>;
      policy?: OnyxEntry<Policy>;
      }): boolean {
      // A list of fields that cannot be edited by anyone, once an expense has been settled
      const restrictedFields: string[] = [
      CONST.EDIT_REQUEST_FIELD.AMOUNT,
      CONST.EDIT_REQUEST_FIELD.CURRENCY,
      CONST.EDIT_REQUEST_FIELD.MERCHANT,
      CONST.EDIT_REQUEST_FIELD.DATE,
      CONST.EDIT_REQUEST_FIELD.RECEIPT,
      CONST.EDIT_REQUEST_FIELD.DISTANCE,
      CONST.EDIT_REQUEST_FIELD.DISTANCE_RATE,
      CONST.EDIT_REQUEST_FIELD.REIMBURSABLE,
      CONST.EDIT_REQUEST_FIELD.REPORT,
      CONST.EDIT_REQUEST_FIELD.BILLABLE,
      ];
      if (!isMoneyRequestAction(reportAction) || !canEditMoneyRequest(reportAction, transaction, isChatReportArchived, report, policy)) {
      return false;
      }
      // If we're editing fields such as category, tag, description, etc. the check above should be enough for handling the permission
      if (!restrictedFields.includes(fieldToEdit)) {
      return true;
      }

    Similar cases do not have this exact risk:

    • Search hydration uses from?.accountID from the search result instead of the current viewer:

      App/src/libs/actions/Search.ts

      Lines 1414 to 1432 in 632ff2f

      // Set optimistic parent report action
      if (!hasParentReportAction && moneyRequestReportActionID && moneyRequestReportActionID !== '0') {
      const optimisticIOUAction = buildOptimisticIOUReportAction({
      type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
      amount,
      currency,
      comment: '',
      participants: [],
      transactionID,
      iouReportID: reportID,
      created,
      reportActionID: moneyRequestReportActionID,
      linkedExpenseReportAction: {childReportID: IOUTransactionID} as ReportAction,
      // delegateAccountIDParam: will be threaded in PR 15; buildOptimisticIOUReportAction falls back to module-level Onyx.connect value (https://github.com/Expensify/App/issues/66425)
      delegateAccountIDParam: undefined,
      });
      optimisticIOUAction.pendingAction = undefined;
      optimisticIOUAction.actorAccountID = from?.accountID;
      onyxUpdates.push({
    • Report-policy conversion uses iouReport.ownerAccountID as submitter, not the viewer:
      const reportID = iouReport.reportID;
      const isPolicyAdmin = isPolicyAdminPolicyUtils(policy);
      const submitterAccountID = iouReport.ownerAccountID;
      const policyID = policy.id;
      // This flow only works for admins moving an IOU report to a policy where the submitter is NOT yet a member of the policy
      if (!isPolicyAdmin || !isIOUReportUsingReport(iouReport) || !submitterAccountID || !submitterLogin || isPolicyMember(policy, submitterLogin)) {
    • Normal money-request creation is initiated by the requester, so buildOptimisticIOUReportAction defaulting to the current user is correct there.

    Git history also points to this being introduced with the legacy transaction recovery work for issue 80180. The fallback and moneyRequestPreviewReportActionID were added together, and issue 80180 explicitly says this code exists only to materialize old missing previews until legacy transactions are migrated.

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

    We should stop using the current viewer as the fallback author for this legacy preview path.

    The backend OpenReport handling for moneyRequestPreviewReportActionID should resolve the preview action actor from authoritative server data: the transaction’s parent report owner/card holder. That prevents whichever user opens first from determining ownership.

    On the client, keep the existing optimistic preview only when we can prove the owner from local data. If the parent report is missing, still allow the backend to create/return the preview, but do not stamp the viewer as the submitter.

    Small client-side shape:

     const parentReport =
         transactionParentReportID === optimisticSelfDMReport?.reportID
             ? optimisticSelfDMReport
             : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
    
    -const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;
    +const submitterAccountID = parentReport?.ownerAccountID;
    +const canBuildAuthoritativeOptimisticPreview = !!submitterAccountID;

    Then only override actor/person/avatar when canBuildAuthoritativeOptimisticPreview is true:

    -optimisticIOUAction.actorAccountID = submitterAccountID;
    -optimisticIOUAction.person = [...]
    -optimisticIOUAction.avatar = submitterPersonalDetails?.avatar;
    +if (canBuildAuthoritativeOptimisticPreview) {
    +    optimisticIOUAction.actorAccountID = submitterAccountID;
    +    optimisticIOUAction.person = [...]
    +    optimisticIOUAction.avatar = submitterPersonalDetails?.avatar;
    +}

    To avoid regressions:

    • Do not change canEditMoneyRequest; the permission model is correct when actorAccountID is correct.
    • Do not remove the legacy fallback entirely, because issue 80180 confirms old transactions still hit it.
    • Preserve optimistic behavior when parentReport.ownerAccountID is available.
    • Add/adjust backend coverage for OpenReport legacy preview creation where the authenticated opener is not the report owner, verifying the created IOU action actor is the report owner.
    • Add a frontend unit test for the fallback where parentReport is missing, verifying we do not set actorAccountID to currentUserAccountID.

    What alternative solutions did you explore? (Optional)

    n/a

  10. abbasifaizan70 commented on Jun 5, 2026

    @abbasifaizan70
    Contributor

    Proposal

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

    An auto-reported company-card expense on an OPEN expense report can become permanently read-only for the report owner (card holder) in NewDot — no fields editable, no tags offered — while sibling transactions on the same card/feed/report remain editable. The same expense is correctly editable in Classic, and admins can still edit it.

    What is the root cause of that problem?

    NewDot gates money-request editing on isRequestor (currentUserAccountID === reportAction.actorAccountID) in canEditMoneyRequest

    App/src/libs/ReportUtils.ts

    Lines 4676 to 4712 in 3d76089

    function canEditMoneyRequest(
    reportAction: OnyxInputOrEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>>,
    linkedTransaction: OnyxEntry<Transaction>,
    isChatReportArchived = false,
    report?: OnyxInputOrEntry<Report>,
    policy?: OnyxEntry<Policy>,
    ): boolean {
    const isDeleted = isDeletedAction(reportAction);
    if (isDeleted) {
    return false;
    }
    const allowedReportActionType: Array<ValueOf<typeof CONST.IOU.REPORT_ACTION_TYPE>> = [CONST.IOU.REPORT_ACTION_TYPE.TRACK, CONST.IOU.REPORT_ACTION_TYPE.CREATE];
    const originalMessage = getOriginalMessage(reportAction);
    const actionType = originalMessage?.type;
    if (!actionType || !(allowedReportActionType.includes(actionType) || (actionType === CONST.IOU.REPORT_ACTION_TYPE.PAY && !!originalMessage.IOUDetails))) {
    return false;
    }
    if (!linkedTransaction) {
    return false;
    }
    // In case the transaction is failed to be created, we should disable editing the money request
    if (!linkedTransaction.transactionID || (linkedTransaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD && !isEmptyObject(linkedTransaction.errors))) {
    return false;
    }
    // Domain admins can report unreported managed card transactions
    if (linkedTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID && isManagedCardTransaction(linkedTransaction)) {
    return true;
    }
    const moneyRequestReportID = originalMessage?.IOUReportID;
    const isRequestor = deprecatedCurrentUserAccountID === reportAction?.actorAccountID;

    Once that returns false, canEditFieldOfMoneyRequest returns false for every field

    App/src/libs/ReportUtils.ts

    Lines 4883 to 4951 in 3d76089

    function canEditFieldOfMoneyRequest({
    reportAction,
    fieldToEdit,
    isDeleteAction = false,
    isChatReportArchived = false,
    outstandingReportsByPolicyID,
    transaction,
    report,
    policy,
    archivedReportsIDSet,
    }: {
    reportAction: OnyxInputOrEntry<ReportAction>;
    fieldToEdit: ValueOf<typeof CONST.EDIT_REQUEST_FIELD>;
    isDeleteAction?: boolean;
    isChatReportArchived?: boolean;
    outstandingReportsByPolicyID?: OutstandingReportsByPolicyIDDerivedValue;
    transaction: OnyxEntry<Transaction>;
    report?: OnyxInputOrEntry<Report>;
    policy?: OnyxEntry<Policy>;
    // Temporarily optional while archived report checks are migrated in smaller PRs. Remove this fallback as part of https://github.com/Expensify/App/issues/66422.
    archivedReportsIDSet?: ArchivedReportsIDSet;
    }): boolean {
    // A list of fields that cannot be edited by anyone, once an expense has been settled
    const restrictedFields: string[] = [
    CONST.EDIT_REQUEST_FIELD.AMOUNT,
    CONST.EDIT_REQUEST_FIELD.CURRENCY,
    CONST.EDIT_REQUEST_FIELD.MERCHANT,
    CONST.EDIT_REQUEST_FIELD.DATE,
    CONST.EDIT_REQUEST_FIELD.RECEIPT,
    CONST.EDIT_REQUEST_FIELD.DISTANCE,
    CONST.EDIT_REQUEST_FIELD.DISTANCE_RATE,
    CONST.EDIT_REQUEST_FIELD.REIMBURSABLE,
    CONST.EDIT_REQUEST_FIELD.REPORT,
    CONST.EDIT_REQUEST_FIELD.BILLABLE,
    ];
    if (!isMoneyRequestAction(reportAction) || !canEditMoneyRequest(reportAction, transaction, isChatReportArchived, report, policy)) {
    return false;
    }
    // If we're editing fields such as category, tag, description, etc. the check above should be enough for handling the permission
    if (!restrictedFields.includes(fieldToEdit)) {
    return true;
    }
    const iouMessage = getOriginalMessage(reportAction);
    const moneyRequestReport = report ?? (iouMessage?.IOUReportID ? (getReport(iouMessage?.IOUReportID, deprecatedAllReports) ?? ({} as Report)) : ({} as Report));
    if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.BILLABLE && isInvoiceReport(moneyRequestReport) && isReportApproved({report: moneyRequestReport})) {
    return false;
    }
    if (isSettled(moneyRequestReport) || isReportApproved({report: moneyRequestReport})) {
    return false;
    }
    if ((fieldToEdit === CONST.EDIT_REQUEST_FIELD.AMOUNT || fieldToEdit === CONST.EDIT_REQUEST_FIELD.CURRENCY) && isCardTransactionTransactionUtils(transaction)) {
    return false;
    }
    if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.REIMBURSABLE && isClosedReport(moneyRequestReport)) {
    return false;
    }
    // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
    const reportPolicy = policy ?? getPolicy(moneyRequestReport?.policyID);
    const isAdmin = isExpenseReport(moneyRequestReport) && reportPolicy?.role === CONST.POLICY.ROLE.ADMIN;
    const isManager = isExpenseReport(moneyRequestReport) && deprecatedCurrentUserAccountID === moneyRequestReport?.managerID;
    const isRequestor = deprecatedCurrentUserAccountID === reportAction?.actorAccountID;

    The wrong actorAccountID gets stamped by the legacy-transaction fallback in openReport

    if (transaction && !parentReportActionID) {
    const transactionParentReportID = parentReportID ?? transaction?.reportID;
    const iouReportActionID = rand64();
    // Get the parent report to determine the actual submitter/owner of the expense
    // Use optimisticSelfDMReport if provided (when selfDM exists but wasn't in allReports)
    const parentReport =
    transactionParentReportID === optimisticSelfDMReport?.reportID ? optimisticSelfDMReport : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
    const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;
    const submitterEmail = PersonalDetailsUtils.getLoginsByAccountIDs([submitterAccountID]).at(0) ?? currentUserLogin ?? '';
    const submitterPersonalDetails = PersonalDetailsUtils.getPersonalDetailByEmail(submitterEmail);
    const optimisticIOUAction = buildOptimisticIOUReportAction({
    type: isSelfDM(parentReport) ? CONST.IOU.REPORT_ACTION_TYPE.TRACK : CONST.IOU.REPORT_ACTION_TYPE.CREATE,
    amount: Math.abs(transaction.amount),
    currency: transaction.currency,
    comment: transaction.comment?.comment ?? '',
    participants: [],
    transactionID: transaction.transactionID,
    isOwnPolicyExpenseChat: false,
    reportActionID: iouReportActionID,
    iouReportID: transactionParentReportID,
    // delegateAccountIDParam: will be threaded in PR 15; buildOptimisticIOUReportAction falls back to module-level Onyx.connect value (https://github.com/Expensify/App/issues/66425)
    delegateAccountIDParam: undefined,
    });
    // Override actor fields to show the submitter instead of current user.
    // This is needed for legacy transactions where the approver opens the transaction but it should show the submitter
    optimisticIOUAction.actorAccountID = submitterAccountID;
    optimisticIOUAction.person = [
    {
    style: 'strong',
    text: submitterPersonalDetails?.displayName ?? submitterEmail,
    type: 'TEXT',
    },
    ];
    optimisticIOUAction.avatar = submitterPersonalDetails?.avatar;
    // We have a case where the transaction data is only available from the snapshot
    // So we need to add the transaction data to Onyx so it's available when opening the report
    optimisticData.push({
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`,
    value: transaction,
    });
    // Add violations if they exist. This is needed when opening from search results where
    // violations are in the snapshot but not yet synced to the main collections, so we need
    // to add them to Onyx to ensure they show up in the transaction thread
    if (transactionViolations) {
    optimisticData.push({
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`,
    value: transactionViolations,
    });
    }
    // Attach the optimistic IOU report action created for the transaction to the transaction thread
    // Set chatReportID to link back to the parent policy expense chat for proper avatar rendering
    optimisticData.push({
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
    value: {
    parentReportActionID: iouReportActionID,
    chatReportID: transactionParentReportID,
    },
    });
    optimisticData.push({
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionParentReportID}`,
    value: {
    [iouReportActionID]: {
    ...optimisticIOUAction,
    childReportID: reportID,
    isOptimisticAction: true,
    },
    },
    });
    parameters.moneyRequestPreviewReportActionID = iouReportActionID;
    // Log how often the legacy transaction fallback path is taken
    Log.info('[Report] Legacy transaction fallback: creating money request preview', false, {
    transactionParentReportID,
    transactionID: transaction.transactionID,
    reportID,
    });
    }

    For old transactions without a transaction thread, it materializes the money-request preview action and derives the actor from the parent report owner, falling back to the current user
    const parentReport =
    transactionParentReportID === optimisticSelfDMReport?.reportID ? optimisticSelfDMReport : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
    const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;

    There are two gaps:

    1. createTransactionThreadReport (Report/index.ts#L2150) already holds the parent report (iouReport, e.g. from a search snapshot) and merges it into Onyx (Report/index.ts#L2188-L2193) — but Onyx.merge is async and openReport is called synchronously right after (Report/index.ts#L2204), so the module-level allReports cache hasn't been updated yet. The owner information is available but silently lost.
    2. When the owner can't be resolved, the wrong actor is stamped on the optimistic action (Report/index.ts#L1641) and still persisted to the server via parameters.moneyRequestPreviewReportActionID (Report/index.ts#L1693) with actorAccountID = currentUserAccountID. If a non-owner (admin/copilot) opens the report first, the wrong actor becomes canonical — permanently making the expense read-only for the actual owner.

    Confirmed via a failing Jest test: a non-owner calling createTransactionThreadReport for a legacy transaction whose parent report is not yet in Onyx produces an IOU CREATE action with the non-owner's actorAccountID (expected owner 1, received 2).

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

    Two changes in src/libs/actions/Report/index.ts:

    1. Thread the parent report through to openReport

    createTransactionThreadReport already has the parent report (iouReport from its caller / search snapshot) — pass it down instead of losing it. Add an optional param to OpenReportActionParams:

    /** The parent (money request) report of a legacy transaction, used to resolve the submitter when the report is not yet in Onyx */
    parentReport?: OnyxEntry<Report>;

    In createTransactionThreadReport's openReport call:

    openReport({
        // ...existing params
        parentReportID: selfDMReportID,
        // Pass the parent report down so the legacy-transaction fallback can resolve the actual submitter
        // even when the report isn't in allReports yet (e.g. when opening from a search snapshot)
        parentReport: reportToUse,
        // ...
    });

    In the legacy fallback, resolve the submitter as optimisticSelfDMReport → allReports → the passed parentReport (when its reportID matches):

    let resolvedParentReport: OnyxEntry<Report>;
    if (transactionParentReportID === optimisticSelfDMReport?.reportID) {
        resolvedParentReport = optimisticSelfDMReport;
    } else {
        resolvedParentReport =
            allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`] ?? (parentReport?.reportID === transactionParentReportID ? parentReport : undefined);
    }

    This fixes the common case (opening from search snapshot / fresh cache) so the actor is always the report owner.

    2. Never persist an unverified actor

    Gate the materialization of the preview action on having actually resolved the owner. If we can't, skip it (and skip parameters.moneyRequestPreviewReportActionID) instead of falling back to the current user:

    // Only materialize the money request preview when we can resolve the actual submitter/owner.
    // The optimistic action is persisted to the server (via moneyRequestPreviewReportActionID), so if we
    // fell back to the current user here, a non-owner opening the report first would permanently stamp
    // the wrong actorAccountID on the IOU action, making the expense read-only for the real owner
    // (canEditMoneyRequest gates editing on the action's actorAccountID).
    const submitterAccountID = resolvedParentReport?.ownerAccountID;
    if (submitterAccountID !== undefined || isSelfDM(resolvedParentReport)) {
        // ...existing logic: buildOptimisticIOUReportAction, override actor fields with the submitter,
        // push optimisticData, and set parameters.moneyRequestPreviewReportActionID = iouReportActionID;
    } else {
        // The parent report isn't available client-side, so we can't know the submitter. Skip creating the
        // preview action rather than persisting one with the wrong actor; it will be materialized on a
        // subsequent open once the parent report is available.
        Log.warn('[Report] Legacy transaction fallback: skipping money request preview creation because the parent report owner could not be resolved', {
            transactionParentReportID,
            transactionID: transaction.transactionID,
            reportID,
        });
    }

    The transaction (and violations) Onyx merges are kept unconditionally, so search-snapshot data still syncs. The selfDM/track-expense case is unaffected (selfDM reports carry ownerAccountID = current user, which is correct there). Together: fix 1 makes the owner almost always resolvable; fix 2 guarantees that when it isn't, nothing wrong is persisted.

    Regression tests added to tests/actions/ReportTest.ts:

    • openReport does not persist a money request preview when the parent report owner cannot be resolved (no IOU action attributed to the opener; no moneyRequestPreviewReportActionID in the API call).
    • openReport resolves the submitter from the new parentReport param when the report isn't in Onyx (actor = owner; preview persisted).
    • End-to-end: createTransactionThreadReport attributes the legacy preview to the report owner even when the parent report is not yet in Onyx (the exact production scenario).

    All three fail on main (received actor = current user) and pass with the fix; the pre-existing legacy-fallback test still passes.

    What alternative solutions did you explore? (Optional)

    • Resolve the owner server-side: have the backend ignore/override the actor of the client-materialized preview action and stamp the report owner authoritatively. More robust long-term, but requires Web-E changes; the client fix above closes the corruption path immediately and is contained to the App repo.
    • Block the fallback until the parent report is fetched: await an OpenReport/GetReport for the parent before materializing. Adds latency and complexity to a hot path for a rare race; passing the already-available report down achieves the same without extra requests.
    • Data fix for already-corrupted actions: existing affected actions need a backend backfill (re-stamp actorAccountID from the parent report's ownerAccountID for IOU CREATE actions on company-card expenses). Out of scope for this PR but worth a follow-up.
  11. wildan-m commented on Jun 5, 2026

    @wildan-m
    Contributor

    🚫 Duplicated proposal withdrawn by 🤖 ProposalPolice.

  12. 21 remaining items

  13. cretadn22 commented on Jun 9, 2026

    @cretadn22
    Contributor

    Before we review proposals, has anyone been able to reproduce this bug using a real account? we need to verify the root cause using actual data from an affected account to ensure the proposed fixes are accurate

  14. David-2002065 commented on Jun 9, 2026

    @David-2002065
    Contributor

    @cretadn22 thanks for taking a look. Confirming the bug is verified from two independent angles — production data already in the issue body, plus a deterministic code path. Sharing the trace so it can be audited without a fresh repro account.

    Production-data verification (already in the issue body): the reporter found two identical company-card transactions on the same card, same feed, same OPEN report, where the only field that differed on the IOU create action was actorAccountID — the editable one was attributed to the report owner, the non-editable one to a different user who had opened the report first. Nothing else in the code stamps actorAccountID on a legacy IOU create action from non-server-derived state, so that asymmetry isolates the source to one path: the legacy fallback in openReport.

    Deterministic code path (not a hard-to-hit race): createTransactionThreadReport calls Onyx.merge(...) on the parent report, then on the next line calls openReport(...) without forwarding that parent.

    https://github.com/Expensify/App/blob/main/src/libs/actions/Report/index.ts#L2192-L2223

    openReport then reads the parent synchronously from the module-level allReports snapshot.

    https://github.com/Expensify/App/blob/main/src/libs/actions/Report/index.ts#L1604-L1697

    That snapshot is only kept in sync via an asynchronous Onyx.connect(...) callback.

    https://github.com/Expensify/App/blob/main/src/libs/actions/Report/index.ts#L465-L470

    So the merge does not land in allReports before the read on the next line. For a non-owner opening a legacy card expense for the first time (LHN / search / copilot / direct link — anywhere OpenApp didn't pre-load the parent), parentReport === undefined is the deterministic outcome, the ?? currentUserAccountID resolves to the opener, and parameters.moneyRequestPreviewReportActionID ships that wrong-actor action ID to the server — where it becomes canonical and locks the owner via isRequestor in canEditMoneyRequest.

    Precedent — the team has already fixed this exact class of bug, just for a different consumer. The optimisticSelfDMReport parameter in openReport exists only because this same stale-allReports race was hitting self-DM access. The fix in #88799 ("fix: recover stale DM access when opening profile message", issue #85793) was to pass the freshly-merged report explicitly so the legacy fallback resolves the owner from authoritative caller state instead of the async cache. Earlier actorAccountID corrections to the IOU preview path follow the same pattern: PR #51534 ("fix report preview optimistic actorAccountID") and commit be7a4d1 ("set the correct actorAccountID of the report preview of expense report").

    This is the third time ?? currentUserAccountID in this fallback has bitten — once for DMs, once for IOU preview attribution, and now for legacy company-card expense reports. The fix in my earlier proposal applies the same pattern that already shipped for DMs in #88799: thread the parent expense report (or its ownerAccountID) through createTransactionThreadReport → openReport, and stop silently substituting the opener.

    I can add a Jest unit test that pins the invariant by mocking the stale-allReports window — same shape as the regression test added with #88799 — if that helps lock the bar before we proceed.

  15. heroking777 commented on Jun 9, 2026

    @heroking777

    Thank you for providing such detailed information about the bug and proposed solution. I'll review it step-by-step to ensure clarity and accuracy.

    Root Cause Analysis

    The root cause of the problem is that in the openReport function's legacy fallback, the actorAccountID for an IOU create action is incorrectly derived from either the parent report's owner or the current user if the parent report isn't available in the stale module-level allReports cache. This can happen when a non-owner opens the report first before the owner does, leading to the wrong actor being persisted. Consequently, the canEditMoneyRequest function locks out the actual card holder because it relies on the isRequestor check.

    Proposed Solution

    The proposed solution is to add a check in the ReportUtils.ts file to treat the current user as the rightful editor if they own the report and the transaction is a managed (company) card expense, even if the persisted actorAccountID is wrong. This would be done by modifying the canEditMoneyRequest function as follows:

    const isManagedCardExpenseOwner = isManagedCardTransaction(linkedTransaction) && isCurrentUserSubmitter(moneyRequestReport);
    
    return !isReportApproved({report: moneyRequestReport}) && !isSettled(moneyRequestReport) && !isClosedReport(moneyRequestReport) && (isRequestor || isManagedCardExpenseOwner);

    Alternative Solutions Explored

    The alternative solution explored involves modifying the openReport function to pass the parent expense report or its ownerAccountID explicitly through createTransactionThreadReport → openReport. This would prevent the fallback from using the current user's account ID when the parent report isn't available in the cache. The precedent for this approach is already established with similar fixes for DM access and IOU preview attribution.

    Verification

    The bug has been verified using both production data and a deterministic code path. The production data shows two identical company-card transactions on the same card, same feed, same OPEN report, where the only difference in the IOU create action is the actorAccountID. The deterministic code path confirms that the issue arises from the stale-allReports race condition.

    Testing

    A Jest unit test can be added to pin the invariant by mocking the stale-allReports window. This would ensure that the fix is robust and prevents future regressions.

    Conclusion

    The proposed solution is

  16. abbasifaizan70 commented on Jun 11, 2026

    @abbasifaizan70
    Contributor

    Before we review proposals, has anyone been able to reproduce this bug using a real account? we need to verify the root cause using actual data from an affected account to ensure the proposed fixes are accurate

    @cretadn22
    Yes — verified at three levels:

    1. Real affected data (production). The root cause was diagnosed from an actual affected report: two identical company-card transactions (same card, same feed, same OPEN report) differing only in the IOU CREATE action's actorAccountID — the editable one attributed to the report owner, the read-only one to another user who opened the report first. This is verifiable on any affected account: on a company-card expense, if the IOU CREATE action's actorAccountID ≠ the parent report's ownerAccountID, the expense is read-only for the owner, because canEditMoneyRequest gates editing on isRequestor. I'm happy to run this check against any affected report ID you can share.

    2. The mechanism, reproduced and proven with tests. The corruption requires a race: a legacy transaction (no transaction thread) opened by a non-owner while the parent report is absent from their Onyx cache at the moment openReport's fallback runs — it then falls back to currentUserAccountID and persists it via moneyRequestPreviewReportActionID. That window can't be staged by hand on a live account (and succeeding would permanently corrupt the expense), so I wrote regression tests that stage exactly that state:Against current main: 3 tests fail — the materialized IOU action is stamped with the opener's accountID (Expected: 1, Received: 2). Full output: proof_before_fix.txt (attached).
      Same tests with the proposed fix: all pass. Full output: proof_after_fix.txt (attached).

    Anyone can verify: npx jest tests/actions/ReportTest.ts -t "legacy" with and without the fix applied.
    3. The code path, verified on a real account (dev). Two-account workspace, signed in as the admin (22381781): simulating a legacy transaction (removing the IOU action while offline) triggers the fallback, which materializes a new optimistic IOU action whose actor determines the owner's edit access — exactly the path above. Screenshot attached (actor resolves to the owner 22250246 here because the parent report was cached; the bug is the cache-miss case the tests cover).

    proof_after_fix.txt
    proof_before_fix.txt

  17. melvin-bot commented on Jun 12, 2026

    @melvin-bot
    Author

    📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

  18. melvin-bot commented on Jun 12, 2026

    @melvin-bot
    Author

    @mollfpr, @cretadn22 Whoops! This issue is 2 days overdue. Let's get this updated quick!

  19. Tony-MK commented on Jun 13, 2026

    @Tony-MK
    Contributor

    Proposal

    Not a duplicate of @dilshodmackbook-sketch. Dilshodmackbook-sketch's Part 1 guards inside openReport and skips the optimistic action entirely when parentReport?.ownerAccountID is unknown — the preview row is blank until the server responds. This proposal instead threads iouReport?.ownerAccountID from the caller (createTransactionThreadReport already has iouReport in scope at the call site) so the optimistic preview shows immediately with the correct actor. No blank state, no skip — just the right attribution stamped from the start.


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

    For a legacy auto-reported company-card expense on an OPEN expense report, the expense becomes permanently read-only for the report owner (the card holder) in NewDot when a different user (admin, copilot, or manager) opens the report first. The IOU create action is stamped with the opener's actorAccountID instead of the card holder's, causing canEditMoneyRequest to return false for the real owner and locking every field.


    What is the root cause of that problem?

    The editing gate — canEditMoneyRequest in src/libs/ReportUtils.ts line 4717:

    const isRequestor = deprecatedCurrentUserAccountID === reportAction?.actorAccountID;

    For a non-admin, non-manager submitter on an OPEN expense report the final return path (line 4753) is:

    return !isReportApproved({report: moneyRequestReport}) && !isSettled(moneyRequestReport) && !isClosedReport(moneyRequestReport) && isRequestor;

    When isRequestor is false because actorAccountID was stamped with a different user, every field is locked for the real card holder.

    The wrong stamp — legacy fallback in src/libs/actions/Report/index.ts lines 1620-1622:

    const parentReport =
        transactionParentReportID === optimisticSelfDMReport?.reportID
            ? optimisticSelfDMReport
            : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`];
    const submitterAccountID = parentReport?.ownerAccountID ?? currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID;

    allReports is a module-level Onyx.connect snapshot updated asynchronously. When createTransactionThreadReport (line 2193) calls Onyx.merge(…, iouReport) and immediately calls openReport on line 2205, the merge has not yet propagated. For a non-owner whose OpenApp payload didn't pre-load the parent report, parentReport is undefined — so submitterAccountID resolves to currentUserAccountID (the opener). That ID is stamped on the optimistic action and sent to the backend via parameters.moneyRequestPreviewReportActionID. The bad actorAccountID is now a permanent server-side record.


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

    Two-part fix. The key difference from @dilshodmackbook-sketch is in Part 1.

    Part 1 — Thread the owner from the caller (no blank state)

    The openReport function cannot reliably resolve parentReport?.ownerAccountID because allReports is stale at call time. But the caller already has the answer: createTransactionThreadReport holds iouReport in scope (it just created it on line 2193). Pass the owner explicitly rather than re-deriving it from the stale cache.

    Why this is better than skipping the optimistic action: Dilshodmackbook-sketch's guard skips the entire optimistic preview when the owner is unknown — the preview row is blank for the non-owner opener until the server responds. Threading from the caller avoids that blank state: the optimistic preview renders immediately with the correct actorAccountID, indistinguishable from the happy path.

    src/libs/actions/Report/index.ts — add to OpenReportActionParams type (~line 291):

    +/** ownerAccountID of the parent expense report — passed explicitly to avoid stale allReports cache */
    +parentReportOwnerAccountID?: number;

    In createTransactionThreadReport (~line 2205), pass iouReport?.ownerAccountID:

     openReport({
         reportID: optimisticTransactionThreadReportID,
         // ...existing fields...
    +    parentReportOwnerAccountID: iouReport?.ownerAccountID,
     });

    In openReport's legacy fallback (~line 1620-1622), prefer the explicitly-passed value and remove the ?? currentUserAccountID fallback for reported expenses:

     const submitterAccountID =
    +    parentReportOwnerAccountID ??
         (transactionParentReportID === optimisticSelfDMReport?.reportID
             ? optimisticSelfDMReport?.ownerAccountID
             : allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionParentReportID}`]?.ownerAccountID) ??
    -    currentUserAccountID ??
         CONST.DEFAULT_NUMBER_ID;

    CONST.DEFAULT_NUMBER_ID is a safe sentinel for cases where neither the explicit param nor the cache has the owner — the server creates its own canonical version. Removing ?? currentUserAccountID ensures we never silently stamp the opener's ID.

    Part 2 — Heal already-corrupted records (permission-gate fix)

    Records already persisted with the wrong actorAccountID remain locked for the real card holder regardless of Part 1. In canEditMoneyRequest (src/libs/ReportUtils.ts), add a guard after the existing admin/manager bypass and before the final isRequestor return:

    +// For managed card expenses, report.ownerAccountID is always the rightful submitter.
    +// When actorAccountID was corrupted (opener's ID stamped instead of card holder's),
    +// fall back to ownerAccountID — which cannot be poisoned by the stale-cache race.
    +if (isManagedCardTransaction(linkedTransaction) && isCurrentUserSubmitter(moneyRequestReport)) {
    +    return (
    +        !isReportApproved({report: moneyRequestReport}) &&
    +        !isSettled(moneyRequestReport) &&
    +        !isClosedReport(moneyRequestReport)
    +    );
    +}

    isManagedCardTransaction (TransactionUtils/index.ts:1313) checks !!transaction?.managedCard. isCurrentUserSubmitter (ReportUtils.ts:1598) checks report.ownerAccountID === deprecatedCurrentUserAccountID. Both use authoritative server-provided fields that cannot be poisoned by the race.


    How does this differ from existing proposals?

    @dilshodmackbook-sketch Part 1 This proposal Part 1
    Mechanism Guard inside openReport: skip optimistic action if owner unknown Thread iouReport.ownerAccountID from caller to openReport
    UX when non-owner opens first Preview row is blank until server responds Preview row shows immediately with correct actor
    Where owner is resolved Re-derived from stale allReports cache (then skipped if missing) Read from iouReport in caller scope — always present
    New param needed No Yes — parentReportOwnerAccountID?: number on OpenReportActionParams

    Both proposals share Part 2 (the canEditMoneyRequest permission-gate carve-out for card-fed transactions). The difference is purely in Part 1: skip-if-unknown (blank state) vs. thread-from-caller (no blank state).

  20. github-actions commented on Jun 13, 2026

    @github-actions
    Contributor

    ⚠️ @Tony-MK Your proposal is a duplicate of an already existing proposal and has been automatically withdrawn to prevent spam. Please review the existing proposals before submitting a new one.

  21. mollfpr commented on Jun 15, 2026

    @mollfpr
    Contributor

    Auth PR is deployed, and CQ is run. Waiting for confirmation on the user.

  22. mollfpr commented on Jun 15, 2026

    @mollfpr
    Contributor

    Confirmed issue is fixed!

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

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributorHelp WantedApply this label when an issue is open to proposals by contributors

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions