Skip to content

Fix false missingAttendees violation in Search rows #104540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mountiny merged 2 commits into from
Oct 21, 2026
Merged

Fix false missingAttendees violation in Search rows #104540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc24df7
Fix false missingAttendees violation in Search rows
VickyStash Oct 21, 2026
fb778b3
Correct the type
VickyStash Oct 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/components/Search/SearchList/ListItem/ExpenseReportListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import {isAttendeeTrackingEnabled} from '@libs/PolicyUtils';
import {getNonHeldAndFullAmount, isInvoiceReport, isOpenExpenseReport, isProcessingReport, isReportPendingDelete, shouldShowMarkAsDone} from '@libs/ReportUtils';
import {hasVisibleViolations} from '@libs/SearchUIUtils';
import shouldBreakAccessibilityGrouping from '@libs/shouldBreakAccessibilityGrouping';
import {isOnHold, isViolationDismissed, shouldShowViolation, showHeldExpensesBlockModal, showPendingCardTransactionsBlockModal} from '@libs/TransactionUtils';
import {getAttendees, isOnHold, isViolationDismissed, shouldShowViolation, showHeldExpensesBlockModal, showPendingCardTransactionsBlockModal} from '@libs/TransactionUtils';

import variables from '@styles/variables';

Expand Down Expand Up @@ -205,7 +205,7 @@ function ExpenseReportListItemInner<TItem extends ListItem>({
relevantViolations,
policyCategories,
transaction.category ?? '',
transaction.attendees,
getAttendees(transaction),
currentUserDetails,
isAttendeeTrackingEnabled(policyForViolations),
policyForViolations.type === CONST.POLICY.TYPE.CORPORATE,
Expand Down
3 changes: 2 additions & 1 deletion src/components/Search/SearchList/ListItem/TransactionListItem/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import {isAttendeeTrackingEnabled} from '@libs/PolicyUtils';
import {isInvoiceReport, shouldShowMarkAsDone} from '@libs/ReportUtils';
import {
getAttendees,
isDeletedTransaction as isDeletedTransactionUtil,
isViolationDismissed,
mergeProhibitedViolations,
Expand Down Expand Up @@ -212,7 +213,7 @@ function TransactionListItemInner<TItem extends ListItem>({
onyxViolations,
policyCategories,
transaction?.category ?? transactionItem.category ?? '',
transaction?.comment?.attendees ?? transactionItem.attendees,
getAttendees(transaction ?? transactionItem),

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Compare required attendees with the report owner

When a manager or approver views a Control-workspace expense in a category that requires attendees but has no non-owner attendee, getTransactionsSections inserts the report owner as the default snapshot attendee, and this new fallback passes that owner into syncMissingAttendeesViolation while the next argument remains the viewer's currentUserDetails. The helper therefore counts the submitter as a valid non-owner attendee and fails to add the real violation; the new getAttendees(transaction) call in ExpenseReportListItem.tsx:208 has the same owner/viewer mismatch and can remove the snapshot violation. Use the report owner/submitterLogin for the owner exclusion in both rows.

Useful? React with 👍 / 👎.

This comment was marked as outdated.

Sign in to view

@lakchote lakchote Oct 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VickyStash I think the owner vs viewer check needs fixing in this PR.

Say Alice submits an expense and is the only attendee. The backend flags it, because it leaves out the owner when it counts attendees. But syncMissingAttendeesViolation leaves out the viewer. So when Bob, the approver, looks at the row in Search, the check leaves out Bob instead of Alice, Alice counts as the extra attendee, and the violation is hidden.

Can we leave out the report owner's login instead, like the backend does, in both Search rows, with the current user as the fallback for unreported expenses?

@nkdengineer nkdengineer Oct 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I misunderstood the codex comment. @lakchote is right.

@VickyStash VickyStash Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The backend flags it, because it leaves out the owner when it counts attendees. But syncMissingAttendeesViolation leaves out the viewer.

Yeah, and other places in the app pass the current user instead of the owner in other places too:

I've mentioned it here: #104540 (comment)

So if we do this:

Can we leave out the report owner's login instead, like the backend does, in both Search rows, with the current user as the fallback for unreported expenses?

we should change it everywhere, right?

@nkdengineer nkdengineer Oct 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Here is the result from creator
Screenshot 2026-10-21 at 20 12 22
  • Here is the result from approver
Screenshot 2026-10-21 at 20 12 29

@VickyStash VickyStash Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkdengineer I guess it's the same on staging?

@lakchote lakchote Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving see https://expensify.slack.com/archives/C05LX9D6E07/p1792592889779979?thread_ts=1792563208.522159&cid=C05LX9D6E07

currentUserDetails,
isAttendeeTrackingEnabled(policyForViolations),
policyForViolations?.type === CONST.POLICY.TYPE.CORPORATE,
Expand Down
4 changes: 0 additions & 4 deletions src/components/Search/SearchList/ListItem/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import type {
TransactionViolation,
TransactionViolations,
} from '@src/types/onyx';
import type {Attendee} from '@src/types/onyx/IOU';
import type {Errors, Icon} from '@src/types/onyx/OnyxCommon';
import type {
SearchCardGroup,
Expand Down Expand Up @@ -173,9 +172,6 @@ type TransactionListItemType = ListItem &
/** The name of the file used for a receipt */
filename?: string;

/** Attendees in the transaction */
attendees?: Attendee[];

/** Precomputed violations */
violations?: TransactionViolation[];

Expand Down
Loading