[No QA] Report managed card transactions to employee's default policy#94768
[No QA] Report managed card transactions to employee's default policy#94768s77rt wants to merge 11 commits into
Conversation
…ses as source of truth
…y policyID to backend
melvin-bot
Bot
requested review from
flaviadefaria and
marcochavezf
and removed request for
a team
|
@marcochavezf 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] |
|
Does this need C+ review? |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const [betas] = useOnyx(ONYXKEYS.BETAS); | ||
| const [allPolicyTags] = useOnyx(ONYXKEYS.COLLECTION.POLICY_TAGS); | ||
| const hasPerDiemTransactions = useHasPerDiemTransactions(selectedTransactionsKeys); | ||
| const hasUnreportedManagedCardTransactions = transactions.some((transaction) => isExpenseUnreported(transaction) && isManagedCardTransaction(transaction)); |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The predicate isExpenseUnreported(transaction) && isManagedCardTransaction(transaction) is duplicated verbatim across three files in this PR (SearchTransactionsChangeReport.tsx, IOURequestEditReport.tsx, and IOURequestStepReport.tsx). It expresses a single named concept ("unreported managed card transaction") that is now spread across the codebase, so any change to the definition has to be made in multiple places.
Extract a reusable helper in TransactionUtils and call it from each site:
// src/libs/TransactionUtils/index.ts
function isUnreportedManagedCardTransaction(transaction?: OnyxEntry<Transaction>): boolean {
return isExpenseUnreported(transaction) && isManagedCardTransaction(transaction);
}// callers
const hasUnreportedManagedCardTransactions = transactions.some(isUnreportedManagedCardTransaction);Reviewed at: e280d03 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| const policyForMovingExpenses = policyForMovingExpensesID ? allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyForMovingExpensesID}`] : undefined; | ||
| const [betas] = useOnyx(ONYXKEYS.BETAS); | ||
| const [transactions] = useTransactionsByID(transactionIDs); | ||
| const hasUnreportedManagedCardTransactions = transactions.some((transaction) => isExpenseUnreported(transaction) && isManagedCardTransaction(transaction)); |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The predicate isExpenseUnreported(transaction) && isManagedCardTransaction(transaction) is duplicated here and in SearchTransactionsChangeReport.tsx and IOURequestStepReport.tsx. Consolidate this single concept into one util so it stays consistent.
Add isUnreportedManagedCardTransaction(transaction) to TransactionUtils and reuse it:
const hasUnreportedManagedCardTransactions = transactions.some(isUnreportedManagedCardTransaction);Reviewed at: e280d03 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| : selectedReport?.ownerAccountID; | ||
| const ownerPersonalDetails = getPersonalDetailsForAccountID(ownerAccountID, personalDetails) as PersonalDetails; | ||
| const isPerDiemTransaction = isPerDiemRequest(transaction); | ||
| const isUnreportedManagedCardTransaction = isExpenseUnreported(transaction) && isManagedCardTransaction(transaction); |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The predicate isExpenseUnreported(transaction) && isManagedCardTransaction(transaction) is duplicated here and in SearchTransactionsChangeReport.tsx and IOURequestEditReport.tsx. Extract it into a single TransactionUtils helper and call that helper here:
const isUnreportedManagedCardTransaction = isUnreportedManagedCardTransactionUtil(transaction);Reviewed at: e280d03 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e280d034aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const policyForMovingExpenses = policyForMovingExpensesID ? allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyForMovingExpensesID}`] : undefined; | ||
| const [betas] = useOnyx(ONYXKEYS.BETAS); | ||
| const [transactions] = useTransactionsByID(transactionIDs); | ||
| const hasUnreportedManagedCardTransactions = transactions.some((transaction) => isExpenseUnreported(transaction) && isManagedCardTransaction(transaction)); |
There was a problem hiding this comment.
When this edit-report page is used for an unreported managed-card transaction, this flag is only passed to IOURequestEditReportCommon for display; the usePolicyForMovingExpenses(...) call above still runs without it, and createReportForPolicy uses that resolved policy in createNewReport. If the admin has an active/default workspace, pressing “Create report” from this flow still sends the admin's policyID instead of omitting policyID so the backend can choose the employee's default policy.
Useful? React with 👍 / 👎.
| // If this is an employee's card transaction that we manage, then we should report it to their default policy | ||
| // which we don't know. Sending an empty `policyID` instructs the backend to auto-select the preferred policy. | ||
| if (isUnreportedManagedCardTransaction) { | ||
| return {policyForMovingExpensesID: undefined, policyForMovingExpenses: undefined, shouldSelectPolicy: false, shouldNavigateToUpgradePath: false}; |
There was a problem hiding this comment.
This managed-card branch intentionally returns no policy, but the create-report callers still pass that undefined policy into createNewReport. That function builds optimistic data through getPolicyExpenseChat(accountID, policy?.id) and then merges into keys based on parentReport?.reportID, so this path writes partial report_undefined/reportActions_undefined entries and an optimistic report with no policy/parent until the server responds. The backend can auto-select the policy, but the frontend should not build policy-chat optimistic updates when policyID is omitted.
Useful? React with 👍 / 👎.
1 participant
Explanation of Change
Modified the
usePolicyForMovingExpenseshook that we use to select the targetpolicyIDwhen moving expenses, if we are moving an unreported managed card transaction then it should be moved to the employee's default policy (and not ours) and since we don't know that info, we will returnundefinedand this will omit sending thepolicyIDto the backend.In the backend, when no explicit
policyIDis provided, it will automatically find and use the preferred policy.Fixed Issues
#87954
PROPOSAL:
Tests
Create reportoption does not display any workspace name under it i.e. no supporting textScreen.Recording.2026-06-27.at.11.46.24.AM.mov
Offline tests
n/a
QA Steps
n/a
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari