Fix import/no-cycle - part 15 by LukasMod · Pull Request #101721 · Expensify/App

LukasMod · 2026-09-21T10:24:45Z

Explanation of Change

The read-only IOU workflow checks (canApproveIOU, canIOUBePaid, canSubmitReport, getBadgeFromIOUReport, getIOUReportActionWithBadge) moved from actions/IOU/ReportWorkflow.ts into ReportUtils.ts. ReportUtils no longer imports ReportWorkflow, which clears the import loop. ReportWorkflow keeps only the write actions (submit, approve, pay, reopen, retract).
changeMoneyRequestHoldStatus moved from ReportUtils into actions/IOU/Hold.ts, so ReportUtils no longer imports unholdRequest. It now takes policy from the caller instead of looking it up in a global cache.
The badge checks no longer read the deprecated global violations cache. Callers pass violations in, down through the LHN derived value, the option list, the debug page and useOptimisticNextStep. The "every expense auto-rejected" rule that hides the Submit badge depends on this.
canIOUBePaid now reads the chat's archived flag from ReportUtils' own copy of the report name-value pairs. It is the same Onyx collection, read through a different module.
The unread indicator reuses the pre-computed requiresAttention from the report attributes instead of recomputing it.
Cleanup: isPayAtEndExpenseReport and isReportExcludedForHeldExpenses are no longer exported, and isPaidGroupPolicy(policy) became isPaidGroupPolicyByType(policy.type), which has the same body.
The module caches are the blocker for moving this into totally new module, all other options recreate no-cycle issue

Covered by automated tests

Auto-rejected expenses hide the Submit badge, the attention dot and the option-list GBR: tests/actions/IOUTest/ReportWorkflowTest.ts:5100.
The debug page shows no GBR for auto-rejected reports: tests/unit/DebugUtilsTest.ts:1154 and :1385.
changeMoneyRequestHoldStatus navigates to the hold reason or unholds: tests/actions/IOUTest/HoldTest.ts:513 and :526.
The context menu passes the expense report's policy when unholding: tests/ui/components/BaseReportActionContextMenuTest.tsx:408.
The moved Approve, Pay and Submit logic: the existing ReportWorkflowTest cases, now importing from ReportUtils.

import/no-cycle cleared (whole repo, 120 → 99)

Fixed Issues

$ #99650
PROPOSAL:

Tests

Verify that no errors appear in the JS console

Prerequisites: a team workspace with approvals on; account A approves and pays, account S submits.

Test 1: Smoke test for workflow buttons, badges and hold

As A, open a report awaiting approval, tap Approve, and confirm it moves to Approved.
As A, open a report where A is the payer and confirm Pay opens the pay flow.
As S, open an open report and confirm Submit submits it.
Confirm each LHN row shows the matching Pay, Approve or Submit badge.
On an expense, choose Hold from the header, then Unhold , confirm both work

Test 2: Auto-rejected expenses hide the Submit badge in the LHN

Prerequisite: a report where every one of S's expenses was auto-rejected and A is the report manager.

As A, find that workspace chat in the LHN.
Confirm the row shows no Submit badge and no green dot.
Open the chat and confirm the report preview offers no Submit action.

Test 3: No Pay option in an archived chat

Archiving is automatic, there is no manual "Archive" button. A chat is archived
when its workspace/account relationship ends (member removed, workspace deleted,
account closed/merged). Archived chats appear under the "Archived" header in the LHN.

Prerequisite: you are the invoice receiver/payer of an unpaid invoice; the invoice chat then gets
archived (e.g. the sending workspace is deleted).

As the payer, expand the "Archived" section and open that invoice chat.
Confirm the LHN row shows no Pay badge.
Confirm the report preview shows no Pay button.

Test 4: Unread count still includes reports that need attention

Prerequisite: account A has an unread chat whose report is waiting on A to Approve or Pay.

As A, stay on a different chat.
Confirm the unread count (browser tab title or app icon badge) includes that chat.
Open the chat and mark it read.
Confirm the unread count drops by one.

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

android.native.mov

Android: mWeb Chrome

android.web.mov

iOS: Native

ios.native.1-2.mov

ios.native.3.mov.mov

ios.native.4.mov

iOS: mWeb Safari

ios.web.1-4.mov

MacOS: Chrome / Safari

web-1.mov

web-2.mov

web.4.mov

canIOUBePaid, canApproveIOU, getBadgeFromIOUReport and getIOUReportActionWithBadge are pure read-side predicates, but they lived in libs/actions/IOU/ReportWorkflow. Every component that needed them had to import an actions module, which is the edge keeping the ReportUtils <-> IOU actions cycle alive. Move them into ReportUtils and update the import sites. ReportWorkflow keeps only the write-side actions. ReportUtils gains a module-scoped TRANSACTION_VIOLATIONS cache for the SUBMIT badge, which is computed inside the reportAttributes derived value; the deprecated getAllTransactionViolations() getter in IOU/index.ts is not importable from here without recreating the cycle.

Drop the new Onyx.connectWithoutView violations cache in ReportUtils and pass the collection from the reportAttributes derived value instead. Thread it through getReasonAndReportActionThatRequiresAttention into getIOUReportActionWithBadge/getBadgeFromIOUReport down to canSubmitAndIsAwaitingForCurrentUser. generateReportAttributes already had the collection in scope, so no new module-level Onyx state is needed.

codecov · 2026-09-21T10:28:09Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
...eaderActions/MoneyReportHeaderSecondaryActions.tsx	`0.62% <ø> (ø)`
...aderActions/MoneyReportHeaderSelectionDropdown.tsx	`0.86% <ø> (ø)`
src/components/MoneyReportHeaderModals.tsx	`0.00% <ø> (ø)`
...ReportHeaderPrimaryAction/ApprovePrimaryAction.tsx	`0.00% <ø> (ø)`
...oneyReportHeaderPrimaryAction/PayPrimaryAction.tsx	`0.00% <ø> (ø)`
...ortHeaderPrimaryAction/RemoveHoldPrimaryAction.tsx	`0.00% <ø> (ø)`
src/components/MoneyRequestHeaderPrimaryAction.tsx	`1.56% <ø> (ø)`
.../components/MoneyRequestHeaderSecondaryActions.tsx	`66.81% <ø> (ø)`
...estReportPreview/useReportPreviewActionDecision.ts	`100.00% <100.00%> (ø)`
...h/SearchList/ListItem/ActionCell/PayActionCell.tsx	`94.64% <ø> (ø)`
... and 16 more
... and 12 files with indirect coverage changes

Nothing outside ReportUtils imports it now that the workflow predicates live here.

isPaidGroupPolicy and isPaidGroupPolicyByType are the identical TEAM/CORPORATE check, but only isPaidGroupPolicy is a restricted import name. ReportUtils already uses the type-based sibling isGroupPolicyByType(policy?.type), so switch canApproveIOU/canIOUBePaid to isPaidGroupPolicyByType(policy.type) and drop the no-restricted-imports disable. No behavior change.

ReportWorkflow keeps the deletion of the moved predicates, and upstream's getReimbursementChoice reads are ported into canIOUBePaid in ReportUtils. Dropped the now-unused getReimbursementChoice import in ReportWorkflow.

The only callers moved to ReportUtils with canIOUBePaid.

MelvinBot · 2026-09-21T15:30:48Z

🤖 Code review — [No QA] Fix import/no-cycle - part 15

The refactor is sound: ReportUtils no longer imports anything from actions/IOU (verified — zero matches for from './actions/IOU), the moved bodies are byte-equivalent apart from alias renames, and isPaidGroupPolicy(policy) → isPaidGroupPolicyByType(policy.type) is a true no-op (PolicyUtils.ts:1450-1456). ESLint, typecheck, knip and all 8 Jest shards are green.

One real regression, and it's the one you flagged yourself. Dropping the deprecated global violations read from getBadgeFromIOUReport breaks the chain at requiresAttentionFromCurrentUser, which has no way to supply violations. Please fix before merge.

1. `requiresAttentionFromCurrentUser` loses the auto-rejected suppression 🔴

ReportUtils.ts:4966-4974 calls getReasonAndReportActionThatRequiresAttention without the new transactionViolations argument, so allViolations arrives as undefined at getBadgeFromIOUReport. Inside canSubmitAndIsAwaitingForCurrentUser the violation list then resolves to [], hasAutoRejectedTransactionsForManager becomes false, and the SUBMIT badge is returned where main suppressed it (ReportUtils.ts:3130-3141).

That makes reasonForReportToBeInOptionList return HAS_GBR for a report where every expense was auto-rejected and the current user is the manager.

The LHN is safe — generateReportAttributes threads violations and passes requiresAttention precomputed (ReportUtils.ts:13851). These two callers do not, so they hit the broken fallback:

OptionsListUtils/index.ts:1861 — search, new-chat and mention option lists
UnreadIndicatorUpdater/index.ts:110 — unread count and browser title

Suggested fix: add an optional transactionViolations param to requiresAttentionFromCurrentUser and to ShouldReportBeInOptionListParams, then forward it from both call sites. Alternatively give ReportUtils its own module-level Onyx.connect on COLLECTION.TRANSACTION_VIOLATIONS and default allViolations ?? allTransactionViolations — that restores the old behavior exactly without reintroducing the cycle, at the cost of one more global cache.

Your two other named callers turn out to be harmless, so no action needed there:

Why useOptimisticNextStep and DebugUtils are fine

useOptimisticNextStep.ts:78 only reads gbrResult.reason === HAS_DEW_APPROVE_FAILED, and only on STATUS_NUM.SUBMITTED reports. The auto-rejected suppression only affects the SUBMIT badge, which requires an open report — unreachable here.
DebugUtils.ts:1601 is debug-only tooling.

2. The new test doesn't cover the regression

should not return SUBMIT actionBadge when every transaction of the report was auto-rejected (tests/actions/IOUTest/ReportWorkflowTest.ts:5167) passes allViolations explicitly, so it is green on both main and this branch. Add a case that calls requiresAttentionFromCurrentUser (or shouldReportBeInOptionList without requiresAttention) on that same fixture — that one fails today and would have caught this.

3. Archived-chat pay gate — verified equivalent

canIOUBePaid:3227 now reads allReportNameValuePair instead of getAllReportNameValuePairs(). Both subscribe to the same collection key with a plain Onyx.connect and no waitForCollectionCallback (which no longer exists in react-native-onyx@3.0.111), so both hold the full collection. The only difference is the if (!value) return; guard at ReportUtils.ts:1210-1219, which keeps the last non-empty snapshot where actions/IOU/index.ts:61-67 would clear it. That only diverges on a full Onyx.clear(), so I'd treat your Test 3 as nice-to-have rather than required.

4. `changeMoneyRequestHoldStatus` policy threading — verified correct

Hoisting the policy lookup out of Hold.ts:1061 into six callers is the riskiest mechanical change here, and all six resolve to the money-request report's policy, matching the old internal lookup:

Per-call-site check

Call site	Policy source	Matches old lookup
`MoneyReportHeaderEducationalModals.tsx`	`POLICY[moneyRequestReport.policyID]`	✅
`RemoveHoldPrimaryAction.tsx:34`	`POLICY[moneyRequestReport.policyID]`	✅
`useHoldRejectActions.ts`	`POLICY[moneyRequestReport.policyID]`	✅
`BaseReportActionContextMenu.tsx:239`	`POLICY[REPORT[moneyRequestAction.reportID].policyID]`	✅
`MoneyRequestHeaderPrimaryAction.tsx:71`	`POLICY[transactionThread.policyID]`	✅ — `buildTransactionThread` copies `policyID` from the money request report (`ReportUtils.ts:9843`)
`MoneyRequestHeaderSecondaryActions.tsx:151`	same as above	✅

The BaseReportActionContextMenuTest change is a good trade: it stops asserting unholdRequest args but now pins the new policy argument, and HoldTest's two new cases cover the hold/unhold branches directly. Net coverage is up.

5. Nits

Stale aliases. canIOUBePaid as canIOUBePaidAction and canIOUBePaidIOUActions (useReportPreviewActionDecision.ts:8, useLifecycleActions.tsx:13) now name a ReportUtils read helper "Action". Dropping the alias in this PR avoids another rename pass later.
Mixed module alias. This PR adds both @libs/actions/IOU/Hold and @userActions/IOU/Hold for the same module. Pick one.
Lost TODO. The // TODO: https://github.com/Expensify/App/issues/66512 that documented the deprecated global read is gone, but ReportUtils.ts:3371 still suppresses no-deprecated for getReportTransactions. Re-add the link so that call stays tracked.
Import member order. canIOUBePaid lands before canEditMultipleTransactions in useSearchBulkActions.ts and hasAnyTransactionWithoutRTERViolation after hasNoticeTypeViolation in ReportUtils.ts. ESLint doesn't enforce it, so cosmetic only.
Removing isPayAtEndExpenseReport from the export list is correct — the only three references are inside ReportUtils itself.

Reply with @MelvinBot <feedback> to refine this review.

…eview nits

ReportWorkflow keeps the deletion of the moved predicates. Upstream's held-expense exclusion and the report-actions parameter for getBadgeFromIOUReport land in the moved copies in ReportUtils, where the collection param is named allReportActionsParam to avoid shadowing the module-level cache.

…eportutils-workflow-reads-v1

Its only outside caller was the canIOUBePaid/getBadgeFromIOUReport pair that now lives here, so the export is dead. Knip flagged it as a new unused export on this branch.

…eportutils-workflow-reads-v1

…ext step

LukasMod · 2026-09-23T11:21:14Z

@MelvinBot review

MelvinBot · 2026-09-23T11:31:28Z

🤖 Re-review — [No QA] Fix import/no-cycle - part 15 (head b0c7b2a)

Every finding from my last pass is fixed, and I found nothing new that blocks merge. All checks green except Check independent approval, which just needs a human reviewer.

Prior findings — all resolved ✅

1. requiresAttentionFromCurrentUser losing the auto-rejected suppression — fixed. requiresAttentionFromCurrentUser now takes transactionViolations and forwards it as the 10th positional arg, and ShouldReportBeInOptionListParams carries it through to the fallback (ReportUtils.ts:10624). Both broken callers are covered, by two different means:

OptionsListUtils/index.ts:1896 threads the violations it already had in scope.
UnreadIndicatorUpdater/index.ts:104 passes the precomputed requiresAttention instead, matching what SidebarUtils already does.

I verified the positional-argument threading at all four call sites — the new param lands in the right slot every time.

2. Missing regression test — fixed, twice. ReportWorkflowTest.ts:5168 now asserts requiresAttentionFromCurrentUser returns false on the auto-rejected fixture, and DebugUtilsTest.ts:1302 covers the GBR-reason path. Both fail without the threading.

3–5. Nits — all addressed. The canIOUBePaidAction / canIOUBePaidIOUActions aliases are gone, every changeMoneyRequestHoldStatus import now uses @libs/actions/IOU/Hold (no more mixed @userActions spelling), the 66512 TODO link is back at ReportUtils.ts:3346, and the useSearchBulkActions import order is sorted.

What I re-checked on the delta

Verification details

Moved bodies survived both main merges. Diffed all six helpers against main at the current merge base (53f9fb1). Still equivalent modulo local-variable renames that avoid shadowing the now-local function names (isOpenExpenseReport → isReportOpen, isClosedReport → isReportClosed, isPayAtEndExpenseReport → isReportPayAtEnd).
isPaidGroupPolicyByType(policy?.type) ≡ isPaidGroupPolicy(policy) still holds at PolicyUtils.ts:1457-1463, including the policy === undefined case in canIOUBePaid.
getIOUReportActionWithBadge param rename is required, not cosmetic. ReportUtils has a module-level allReportActions; renaming the param to allReportActionsParam keeps the no-fallback lookup the function always had.
Unexporting isReportExcludedForHeldExpenses is safe — only three references remain, all inside ReportUtils.
changeMoneyRequestHoldStatus move to Hold.ts:1061. moneyRequestPolicy in BaseReportActionContextMenu already existed for shouldShow; it resolves to POLICY[REPORT[(moneyRequestAction ?? reportAction).reportID].policyID], matching the old internal lookup. That component is the only builder of the onPress payload, so the optional param is always supplied.
Tests relocated, not dropped. The two changeMoneyRequestHoldStatus cases deleted from ReportUtilsTest land in HoldTest, and the unhold case now asserts on the resulting Onyx report action instead of a spy — a stronger assertion than before.

One note, not a blocker

UnreadIndicatorUpdater now sources requiresAttention from the reportAttributes derived value. triggerUnreadUpdate only fires on COLLECTION.REPORT changes, and memoizedGetUnreadReportsForUnreadIndicator keys on allReports, so if the derived value ever lags a report write by more than the 300 ms debounce, the unread count and tab title hold the stale value until the next report change. This is the same dependency derivedIsEmptyReport already has in that file, and reading the derived value actually makes the count more consistent with the LHN (it is computed with the real parentReportAction, which the fallback passes as undefined). I'd merge as-is.

Also cosmetic: hasAnyTransactionWithoutRTERViolation sits after hasMissingSmartscanFieldsForRBR at ReportUtils.ts:276 rather than alphabetically before it. ESLint doesn't enforce it.

This review is static analysis only — the browser environment was unavailable for this run, so I could not exercise Tests 1–4 in NewDot. Your Test 2 (auto-rejected suppression) is now covered by unit tests; Tests 1, 3, and 4 still want a manual pass.

Reply with @MelvinBot <feedback> to refine this review.

…list and debug LHN reasons

…ycle-part15-reportutils-workflow-reads-v1

melvin-bot · 2026-09-24T13:51:31Z

@thelullabyy Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

thelullabyy

[lullabyy agent] Reviewed the move of the read-only workflow checks into ReportUtils and of changeMoneyRequestHoldStatus into Hold.ts. The moved logic matches the original, and every hold caller passes the expense report's policy, the same one the old global lookup returned. I left a few inline comments: one about the API, one about lost test coverage, and one about test docs.

thelullabyy · 2026-10-04T13:16:05Z

+    currentUserLogin: string,
+    currentUserAccountID: number,
+    iouReportActions: OnyxEntry<ReportActions>,
+    allViolations?: OnyxCollection<TransactionViolations>,


[lullabyy agent] allViolations is optional here and in getIOUReportActionWithBadge (line 3443). The old code always read the global violations cache. If a caller leaves this argument out, canSubmitAndIsAwaitingForCurrentUser gets undefined, the "every expense auto-rejected" check silently stops working, and the SUBMIT badge comes back with no type error. This PR is meant to fix exactly that case, so could we make the parameter required (allViolations: OnyxCollection<TransactionViolations> | undefined)? Then every current and future caller has to choose what to pass. The same applies to the new transactionViolations? parameter on getReasonAndReportActionThatRequiresAttention and requiresAttentionFromCurrentUser.

thelullabyy · 2026-10-04T13:16:05Z

-        mockedCanSubmitReport.mockReturnValue(true);
        const result = canSubmitAndIsAwaitingForCurrentUser(iouReport, chatReport, basePolicy, transactions, {}, 'user@test.com', CURRENT_USER_ACCOUNT_ID, undefined);
        expect(result).toBe(true);
    });


[lullabyy agent] This PR removes the forwards the passed iouReportOwnerLogin instead of looking it up in the personal details list test because canSubmitReport now lives in the same module and can't be mocked. That leaves the iouReportOwnerLogin ?? getLoginByAccountID(...) branch in canSubmitAndIsAwaitingForCurrentUser with no tests. Could we add a behavior-based test instead? For example, seed no personal details, add a transaction with a pending RTER violation that belongs to 'threaded.owner@test.com', and assert that the result changes depending on whether the owner login is passed.

canSubmitAndIsAwaitingForCurrentUser passes undefined violations to canSubmitReport, and main does the same. The owner login only matters when a dismissed violation gets checked, so on this path the result is the same with or without the fallback, and a behavior test can’t see it. I added a test on canSubmitReport instead, since that’s where the login decides whether a dismissal counts. Passing the violations through would change the SUBMIT badge, so I don't want to touch it in this PR

thelullabyy · 2026-10-04T13:16:05Z

            expect(result.actionBadge).toBe(CONST.REPORT.ACTION_BADGE.SUBMIT);
        });

+        it('should not return SUBMIT actionBadge when every transaction of the report was auto-rejected', async () => {


[lullabyy agent] This new test has // Then comments but no // Given or // When comments. Per tests/README.md#documenting-tests, every test case should explain why it does what it does. Please add a // Given comment above the setup (an open report whose only expense is auto-rejected, with the current user as owner and manager) and a // When comment above the getIOUReportActionWithBadge call. The first two expects also need a // Then comment.

thelullabyy · 2026-10-04T13:16:05Z

    });

-    it('calls unholdRequest when pressing the unhold action', async () => {
+    it('calls changeMoneyRequestHoldStatus with the money request policy when pressing the unhold action', async () => {


[lullabyy agent] This test was renamed and rewritten to check the new changeMoneyRequestHoldStatus contract, so it would be a good time to add the required // Given / // When / // Then comments. For example: Given an on-hold expense whose report belongs to policyID; When the Unhold menu item is pressed; Then the hold action gets the money request report's policy rather than the chat's.

thelullabyy · 2026-10-04T13:32:19Z

@LukasMod Overall PR LGTM, I left some comments from my agent. Please check and resolve conflicts as well so that I can approve the PR

…eportutils-workflow-reads-v1 Keep the workflow predicates (canApproveIOU, canIOUBePaid, canSubmitReport, getBadgeFromIOUReport) in ReportUtils, where this branch moved them, and port upstream's canAdminPayReport() helper into canIOUBePaid.

Optional violations parameters let a caller silently drop the auto-rejected-expense check and resurrect the SUBMIT badge with no type error. Make allViolations/transactionViolations required (OnyxCollection | undefined) on getBadgeFromIOUReport, getIOUReportActionWithBadge, getReasonAndReportActionThatRequiresAttention and requiresAttentionFromCurrentUser, hoisting each to the last required position since TS forbids a required parameter after an optional one.

LukasMod · 2026-10-05T09:40:27Z

@thelullabyy thanks for review, I applied feedback and answered 👍 conflicts resolved

thelullabyy · 2026-10-05T17:21:26Z

@LukasMod Thank you for the update. Changes look good, however, it got conflicts again :((

…eportutils-workflow-reads-v1 # Conflicts: # src/libs/ReportUtils.ts # src/libs/actions/IOU/ReportWorkflow.ts

LukasMod · 2026-10-05T18:10:58Z

@thelullabyy resolved! :)

…eportutils-workflow-reads-v1 # Conflicts: # tests/unit/ReportUtilsTest.ts

LukasMod · 2026-10-07T13:08:34Z

I updated getReasonAndReportActionThatRequiresAttention to pass eslint max params issue after last merge

…eportutils-workflow-reads-v1 # Conflicts: # src/libs/ReportUtils.ts # tests/ui/components/BaseReportActionContextMenuTest.tsx # tests/unit/ReportUtilsTest.ts

…eportutils-workflow-reads-v1 # Conflicts: # src/libs/actions/IOU/ReportWorkflow.ts

LukasMod · 2026-10-08T09:26:13Z

Conflicts each 2 hours 😭

OSBotify · 2026-10-08T16:27:39Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-10-08T16:34:25Z

🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-10-08T16:39:48Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #101721.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-abd8011-b8010af-63fae9e5619642ac74506825a462da306ad58ec4/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-abd8011-b8010af-e6ab656984e1fbacb21d083703336c1491d7dfcd/index.html

Web 🕸️
https://101721.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-10-08T19:44:04Z

🚀 Deployed to staging by https://github.com/roryabraham in version: 9.5.6-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

MelvinBot · 2026-10-08T19:48:42Z

No help site update needed. This PR moves IOU workflow and hold logic between modules to clear import cycles, so no existing article sentence becomes incorrect.

view run

OSBotify · 2026-10-09T20:29:41Z

🚀 Deployed to production by https://github.com/puneetlath in version: 9.5.6-6 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

LukasMod added 2 commits September 18, 2026 13:09

melvin-bot Bot assigned LukasMod Sep 21, 2026

Drop unused isPayAtEndExpenseReport export

ec3474b

Nothing outside ReportUtils imports it now that the workflow predicates live here.

LukasMod force-pushed the fix/no-cycle-part15-reportutils-workflow-reads-v1 branch from e11875e to ec3474b Compare September 21, 2026 10:59

LukasMod added 4 commits September 21, 2026 13:46

Drop unused getReimbursementChoice import in ReportWorkflow

a4832a7

The only callers moved to ReportUtils with canIOUBePaid.

Move changeMoneyRequestHoldStatus into actions/IOU/Hold

21b008b

LukasMod added 6 commits September 21, 2026 18:24

Thread violations into requiresAttentionFromCurrentUser and address r…

ae60ccb

…eview nits

Merge remote-tracking branch 'origin/main' into fix/no-cycle-part15-r…

08852bf

…eportutils-workflow-reads-v1

Stop exporting isReportExcludedForHeldExpenses

c7cc9f2

Its only outside caller was the canIOUBePaid/getBadgeFromIOUReport pair that now lives here, so the export is dead. Knip flagged it as a new unused export on this branch.

Merge remote-tracking branch 'origin/main' into fix/no-cycle-part15-r…

fe2a349

…eportutils-workflow-reads-v1

Thread transaction violations into debug GBR reasons and optimistic n…

b0c7b2a

…ext step

LukasMod added 2 commits September 23, 2026 13:32

Move transactionViolations next to isReportArchived and cover option …

901945a

…list and debug LHN reasons

Merge remote-tracking branch 'refs/remotes/origin/main' into fix/no-c…

2b26414

…ycle-part15-reportutils-workflow-reads-v1

LukasMod changed the title ~~[No QA] Fix import/no-cycle - part 15~~ Fix import/no-cycle - part 15 Sep 24, 2026

LukasMod marked this pull request as ready for review September 24, 2026 13:51

LukasMod requested review from a team as code owners September 24, 2026 13:51

melvin-bot Bot requested review from JmillsExpensify and thelullabyy and removed request for a team September 24, 2026 13:51

melvin-bot Bot removed the request for review from a team September 24, 2026 13:51

thelullabyy reviewed Oct 4, 2026

View reviewed changes

LukasMod added 3 commits October 5, 2026 09:21

Add owner login dismissal test and Given/When/Then comments

de35d1a

LukasMod requested a review from thelullabyy October 5, 2026 13:35

Merge remote-tracking branch 'origin/main' into fix/no-cycle-part15-r…

f334cdf

…eportutils-workflow-reads-v1 # Conflicts: # src/libs/ReportUtils.ts # src/libs/actions/IOU/ReportWorkflow.ts

LukasMod added 2 commits October 7, 2026 08:48

Merge remote-tracking branch 'origin/main' into fix/no-cycle-part15-r…

2da8272

…eportutils-workflow-reads-v1 # Conflicts: # tests/unit/ReportUtilsTest.ts

Pass a params object to getReasonAndReportActionThatRequiresAttention

780d24d

Merge remote-tracking branch 'origin/main' into fix/no-cycle-part15-r…

3486f97

…eportutils-workflow-reads-v1 # Conflicts: # src/libs/ReportUtils.ts # tests/ui/components/BaseReportActionContextMenuTest.tsx # tests/unit/ReportUtilsTest.ts

thelullabyy approved these changes Oct 7, 2026

View reviewed changes

melvin-bot Bot requested a review from roryabraham October 7, 2026 17:54

Merge remote-tracking branch 'origin/main' into fix/no-cycle-part15-r…

5d8fb71

…eportutils-workflow-reads-v1 # Conflicts: # src/libs/actions/IOU/ReportWorkflow.ts

roryabraham approved these changes Oct 8, 2026

View reviewed changes

roryabraham merged commit abd8011 into Expensify:main Oct 8, 2026
34 of 35 checks passed

OSBotify mentioned this pull request Oct 8, 2026

Deploy Checklist: New Expensify 2026-10-08 #103502

Closed

90 tasks done

MelvinBot mentioned this pull request Oct 8, 2026

[Due for payment 2026-10-16] Expense - Error shows up when guest member submits report #103525

Closed

2 of 7 tasks

melvin-bot Bot mentioned this pull request Oct 9, 2026

Fix import/no-cycle that are currently silent #99650

Open

mkhutornyi mentioned this pull request Oct 10, 2026

[Due for payment 2026-10-16] [$250] Support Pay > Mark as paid on reports with a $0 total #102215

Open

Conversation

LukasMod commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Covered by automated tests

Fixed Issues

Tests

Test 1: Smoke test for workflow buttons, badges and hold

Test 2: Auto-rejected expenses hide the Submit badge in the LHN

Test 3: No Pay option in an archived chat

Test 4: Unread count still includes reports that need attention

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov Bot commented Sep 21, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MelvinBot commented Sep 21, 2026

1. requiresAttentionFromCurrentUser loses the auto-rejected suppression 🔴

2. The new test doesn't cover the regression

3. Archived-chat pay gate — verified equivalent

4. changeMoneyRequestHoldStatus policy threading — verified correct

5. Nits

Uh oh!

LukasMod commented Sep 23, 2026

Uh oh!

MelvinBot commented Sep 23, 2026

Prior findings — all resolved ✅

What I re-checked on the delta

One note, not a blocker

Uh oh!

melvin-bot Bot commented Sep 24, 2026

Uh oh!

thelullabyy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thelullabyy commented Oct 4, 2026

Uh oh!

LukasMod commented Oct 5, 2026

Uh oh!

thelullabyy commented Oct 5, 2026

Uh oh!

LukasMod commented Oct 5, 2026

Uh oh!

LukasMod commented Oct 7, 2026

Uh oh!

LukasMod commented Oct 8, 2026

Uh oh!

Uh oh!

OSBotify commented Oct 8, 2026

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

OSBotify commented Oct 8, 2026

Uh oh!

MelvinBot commented Oct 8, 2026

Uh oh!

OSBotify commented Oct 9, 2026

Uh oh!

LukasMod commented Sep 21, 2026 •

edited

Loading

codecov Bot commented Sep 21, 2026 •

edited by github-actions Bot

Loading

1. `requiresAttentionFromCurrentUser` loses the auto-rejected suppression 🔴

4. `changeMoneyRequestHoldStatus` policy threading — verified correct