From 9105b9629d50591cbd293769e9264e79a88daad4 Mon Sep 17 00:00:00 2001 From: Kevin Brian Bader Date: Wed, 28 Jan 2026 14:31:32 -0800 Subject: [PATCH 1/2] fix: 80708 and remove feature flag --- src/CONST/index.ts | 13 -------- .../MoneyRequestConfirmationList.tsx | 10 ++++++- src/components/TransactionItemRow/index.tsx | 3 -- src/hooks/useTransactionViolations.ts | 3 +- src/libs/AttendeeUtils.ts | 16 +++++----- src/libs/TransactionUtils/index.ts | 7 +---- src/libs/Violations/ViolationsUtils.ts | 30 +++++++------------ src/libs/Violations/types.ts | 1 + .../categories/CategoryRequiredFieldsPage.tsx | 2 +- tests/unit/ViolationUtilsTest.ts | 29 ++++++++++++++---- 10 files changed, 54 insertions(+), 60 deletions(-) diff --git a/src/CONST/index.ts b/src/CONST/index.ts index ecbadc2118b7..edab13365032 100755 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -5812,19 +5812,6 @@ const CONST = { CAROUSEL: 3, }, - /** - * Feature flag to enable the missingAttendees violation feature. - * Currently enabled only on staging for testing. - * When true: - * - Enables new missingAttendees violations to be created - * - Shows existing missingAttendees violations in transaction lists - * - Shows "Require attendees" toggle in category settings - * Note: Config?.ENVIRONMENT is undefined in local dev when .env doesn't set it, so we treat undefined as dev - */ - // We can't use nullish coalescing for boolean comparison - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - IS_ATTENDEES_REQUIRED_ENABLED: !Config?.ENVIRONMENT || Config?.ENVIRONMENT === 'staging' || Config?.ENVIRONMENT === 'development', - /** * Constants for types of violation. */ diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx index e84cc12c6de0..1b03db1ac13a 100755 --- a/src/components/MoneyRequestConfirmationList.tsx +++ b/src/components/MoneyRequestConfirmationList.tsx @@ -469,6 +469,7 @@ function MoneyRequestConfirmationList({ iouAttendees, currentUserPersonalDetails, isAttendeeTrackingEnabled: policy?.isAttendeeTrackingEnabled, + isControlPolicy: policy?.type === CONST.POLICY.TYPE.CORPORATE, }); useEffect(() => { @@ -999,7 +1000,14 @@ function MoneyRequestConfirmationList({ // Since invoices are not expense reports that need attendee tracking, this validation should not apply to invoices const isMissingAttendeesViolation = iouType !== CONST.IOU.TYPE.INVOICE && - getIsMissingAttendeesViolation(policyCategories, iouCategory, iouAttendees, currentUserPersonalDetails, policy?.isAttendeeTrackingEnabled); + getIsMissingAttendeesViolation( + policyCategories, + iouCategory, + iouAttendees, + currentUserPersonalDetails, + policy?.isAttendeeTrackingEnabled, + policy?.type === CONST.POLICY.TYPE.CORPORATE, + ); if (isMissingAttendeesViolation) { setFormError('violations.missingAttendees'); return; diff --git a/src/components/TransactionItemRow/index.tsx b/src/components/TransactionItemRow/index.tsx index 4012925a6a47..90a8c392fb4d 100644 --- a/src/components/TransactionItemRow/index.tsx +++ b/src/components/TransactionItemRow/index.tsx @@ -205,9 +205,6 @@ function TransactionItemRow({ if (!violations) { return undefined; } - if (!CONST.IS_ATTENDEES_REQUIRED_ENABLED) { - return violations.filter((violation) => violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES); - } return violations; }, [violations]); diff --git a/src/hooks/useTransactionViolations.ts b/src/hooks/useTransactionViolations.ts index e36e9f9b4afc..413d85bf6b11 100644 --- a/src/hooks/useTransactionViolations.ts +++ b/src/hooks/useTransactionViolations.ts @@ -29,8 +29,7 @@ function useTransactionViolations(transactionID?: string, shouldShowRterForSettl transactionViolations.filter( (violation: TransactionViolation) => !isViolationDismissed(transaction, violation, currentUserDetails.email ?? '', currentUserDetails.accountID, iouReport, policy) && - shouldShowViolation(iouReport, policy, violation.name, currentUserDetails.email ?? '', shouldShowRterForSettledReport, transaction) && - (CONST.IS_ATTENDEES_REQUIRED_ENABLED || violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES), + shouldShowViolation(iouReport, policy, violation.name, currentUserDetails.email ?? '', shouldShowRterForSettledReport, transaction), ), ), [transaction, transactionViolations, iouReport, policy, shouldShowRterForSettledReport, currentUserDetails.email, currentUserDetails.accountID], diff --git a/src/libs/AttendeeUtils.ts b/src/libs/AttendeeUtils.ts index bf0a076fbe1c..41cc15f05c9d 100644 --- a/src/libs/AttendeeUtils.ts +++ b/src/libs/AttendeeUtils.ts @@ -9,8 +9,7 @@ function formatRequiredFieldsTitle(translate: LocaleContextProps['translate'], p const enabledFields: string[] = []; // Attendees field should show first when both are selected and attendee tracking is enabled - // Respect feature flag - don't show attendees in title when feature is disabled - if (CONST.IS_ATTENDEES_REQUIRED_ENABLED && isAttendeeTrackingEnabled && policyCategory.areAttendeesRequired) { + if (isAttendeeTrackingEnabled && policyCategory.areAttendeesRequired) { enabledFields.push(translate('iou.attendees')); } @@ -35,9 +34,10 @@ function getIsMissingAttendeesViolation( iouAttendees: Attendee[] | string | undefined, userPersonalDetails: CurrentUserPersonalDetails, isAttendeeTrackingEnabled = false, + isControlPolicy = false, ) { - // Feature flag to quickly disable the attendees required feature - if (!CONST.IS_ATTENDEES_REQUIRED_ENABLED) { + // Only enforce attendee requirement on Control policies + if (!isControlPolicy) { return false; } @@ -78,11 +78,9 @@ function syncMissingAttendeesViolation( isControlPolicy: boolean, isInvoice = false, ): T[] { - // Feature flag to quickly disable the attendees required feature - // When disabled, remove any existing missingAttendees violations and don't add new ones - // Never add missingAttendees violation for invoices - if (!CONST.IS_ATTENDEES_REQUIRED_ENABLED || isInvoice) { - return violations.filter((v) => v.name !== CONST.VIOLATIONS.MISSING_ATTENDEES); + // No missingAttendees violation for invoices + if (isInvoice) { + return violations.filter((violation) => violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES); } const hasMissingAttendeesViolation = violations.some((v) => v.name === CONST.VIOLATIONS.MISSING_ATTENDEES); diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index afe9ad3479af..6d8d81520e5d 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -1528,10 +1528,6 @@ function getTransactionViolations( (violation) => !isViolationDismissed(transaction, violation, currentUserEmail, currentUserAccountID, iouReport, policy), ) ?? []; - if (!CONST.IS_ATTENDEES_REQUIRED_ENABLED) { - return violations.filter((violation) => violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES); - } - return violations; } @@ -1968,8 +1964,7 @@ function hasViolation( (violation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION && (showInReview === undefined || showInReview === (violation.showInReview ?? false)) && - !isViolationDismissed(transaction, violation, currentUserEmail, currentUserAccountID, iouReport, policy) && - (CONST.IS_ATTENDEES_REQUIRED_ENABLED || violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES), + !isViolationDismissed(transaction, violation, currentUserEmail, currentUserAccountID, iouReport, policy), ); } diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index ae6f496a22b5..8a06e9c04ea4 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -242,7 +242,7 @@ function extractErrorMessages(errors: Errors | ReceiptErrors, errorActions: Repo * Returns true if the violation should be cleared, false if it should persist. */ function getIsViolationFixed(violationError: string, params: ViolationFixParams): boolean { - const {category, tag, taxCode, policyCategories, policyTagLists, policyTaxRates, iouAttendees, currentUserPersonalDetails, isAttendeeTrackingEnabled} = params; + const {category, tag, taxCode, policyCategories, policyTagLists, policyTaxRates, iouAttendees, currentUserPersonalDetails, isAttendeeTrackingEnabled, isControlPolicy} = params; const violationValidators: Record boolean> = { [`${CONST.VIOLATIONS_PREFIX}${CONST.VIOLATIONS.CATEGORY_OUT_OF_POLICY}`]: () => { @@ -267,7 +267,7 @@ function getIsViolationFixed(violationError: string, params: ViolationFixParams) }, [`${CONST.VIOLATIONS_PREFIX}${CONST.VIOLATIONS.MISSING_ATTENDEES}`]: () => { // Attendees violation is fixed if getIsMissingAttendeesViolation returns false - return !getIsMissingAttendeesViolation(policyCategories, category, iouAttendees, currentUserPersonalDetails, isAttendeeTrackingEnabled); + return !getIsMissingAttendeesViolation(policyCategories, category, iouAttendees, currentUserPersonalDetails, isAttendeeTrackingEnabled, isControlPolicy); }, }; @@ -597,22 +597,15 @@ const ViolationsUtils = { newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_COMMENT}); } - const shouldProcessMissingAttendees = CONST.IS_ATTENDEES_REQUIRED_ENABLED; - - if (shouldProcessMissingAttendees) { - if (!hasMissingAttendeesViolation && shouldShowMissingAttendees) { - newTransactionViolations.push({ - name: CONST.VIOLATIONS.MISSING_ATTENDEES, - type: CONST.VIOLATION_TYPES.VIOLATION, - showInReview: true, - }); - } + if (!hasMissingAttendeesViolation && shouldShowMissingAttendees) { + newTransactionViolations.push({ + name: CONST.VIOLATIONS.MISSING_ATTENDEES, + type: CONST.VIOLATION_TYPES.VIOLATION, + showInReview: true, + }); + } - if (hasMissingAttendeesViolation && !shouldShowMissingAttendees) { - newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_ATTENDEES}); - } - } else if (hasMissingAttendeesViolation) { - // Feature flag is disabled - always remove missingAttendees violations + if (hasMissingAttendeesViolation && !shouldShowMissingAttendees) { newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_ATTENDEES}); } @@ -822,8 +815,7 @@ const ViolationsUtils = { return transactionViolations.some((violation: TransactionViolation) => { return ( !isViolationDismissed(transaction, violation, currentUserEmail, currentUserAccountID, report, policy) && - shouldShowViolation(report, policy, violation.name, currentUserEmail, true, transaction) && - (CONST.IS_ATTENDEES_REQUIRED_ENABLED || violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES) + shouldShowViolation(report, policy, violation.name, currentUserEmail, true, transaction) ); }); }); diff --git a/src/libs/Violations/types.ts b/src/libs/Violations/types.ts index 2f51b4efe44e..cd68502afb8f 100644 --- a/src/libs/Violations/types.ts +++ b/src/libs/Violations/types.ts @@ -12,6 +12,7 @@ type ViolationFixParams = { iouAttendees: Attendee[] | undefined; currentUserPersonalDetails: CurrentUserPersonalDetails; isAttendeeTrackingEnabled: boolean | undefined; + isControlPolicy?: boolean; }; export default ViolationFixParams; diff --git a/src/pages/workspace/categories/CategoryRequiredFieldsPage.tsx b/src/pages/workspace/categories/CategoryRequiredFieldsPage.tsx index db9c11fce064..728e9792dd38 100644 --- a/src/pages/workspace/categories/CategoryRequiredFieldsPage.tsx +++ b/src/pages/workspace/categories/CategoryRequiredFieldsPage.tsx @@ -76,7 +76,7 @@ function CategoryRequiredFieldsPage({ - {isAttendeeTrackingEnabled && CONST.IS_ATTENDEES_REQUIRED_ENABLED && ( + {isAttendeeTrackingEnabled && ( diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index ceae31bd5645..0f931650bfc7 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -728,13 +728,13 @@ describe('getViolationsOnyxData', () => { } as Report; }); - (!CONST.IS_ATTENDEES_REQUIRED_ENABLED ? it.skip : it)('should add missingAttendees violation when no attendees are present', () => { + it('should add missingAttendees violation when no attendees are present', () => { transaction.comment = {attendees: []}; const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false, false, false, iouReport); expect(result.value).toEqual(expect.arrayContaining([missingAttendeesViolation])); }); - (!CONST.IS_ATTENDEES_REQUIRED_ENABLED ? it.skip : it)('should add missingAttendees violation when only owner is an attendee', () => { + it('should add missingAttendees violation when only owner is an attendee', () => { transaction.comment = { attendees: [{email: 'owner@example.com', displayName: 'Owner', avatarUrl: '', accountID: ownerAccountID}], }; @@ -782,7 +782,7 @@ describe('getViolationsOnyxData', () => { describe('optimistic / offline scenarios (iouReport is undefined)', () => { // In offline scenarios, iouReport is undefined so we can't get ownerAccountID. // The code falls back to using getCurrentUserEmail() to identify the owner by login/email. - (!CONST.IS_ATTENDEES_REQUIRED_ENABLED ? it.skip : it)('should correctly calculate violation when iouReport is undefined but attendees have matching email', () => { + it('should correctly calculate violation when iouReport is undefined but attendees have matching email', () => { // When iouReport is undefined, we use getCurrentUserEmail() as fallback // If only the current user (matching MOCK_CURRENT_USER_EMAIL) is an attendee, violation should show transactionViolations = []; @@ -822,7 +822,7 @@ describe('getViolationsOnyxData', () => { expect(result.value).not.toEqual(expect.arrayContaining([missingAttendeesViolation])); }); - (!CONST.IS_ATTENDEES_REQUIRED_ENABLED ? it.skip : it)('should preserve violation when only owner attendee remains (offline)', () => { + it('should preserve violation when only owner attendee remains (offline)', () => { // If violation existed and only owner attendee remains, violation stays transactionViolations = [missingAttendeesViolation]; transaction.comment = { @@ -850,7 +850,7 @@ describe('getViolationsOnyxData', () => { jest.restoreAllMocks(); }); - (!CONST.IS_ATTENDEES_REQUIRED_ENABLED ? it.skip : it)("should add missingAttendees violation when no attendees are present (can't identify owner)", () => { + it("should add missingAttendees violation when no attendees are present (can't identify owner)", () => { transactionViolations = []; transaction.comment = {attendees: []}; const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false, false, false, undefined); @@ -858,7 +858,7 @@ describe('getViolationsOnyxData', () => { expect(result.value).toEqual(expect.arrayContaining([missingAttendeesViolation])); }); - (!CONST.IS_ATTENDEES_REQUIRED_ENABLED ? it.skip : it)('should add missingAttendees violation when only 1 attendee exists (assumed to be owner)', () => { + it('should add missingAttendees violation when only 1 attendee exists (assumed to be owner)', () => { transactionViolations = []; transaction.comment = { attendees: [{email: 'anyone@example.com', displayName: 'Someone', avatarUrl: ''}], @@ -1514,6 +1514,7 @@ describe('getIsViolationFixed', () => { const result = getIsViolationFixed('violations.missingAttendees', { ...defaultParams, isAttendeeTrackingEnabled: true, + isControlPolicy: true, category: 'Meals', policyCategories: {Meals: {name: 'Meals', enabled: true, areAttendeesRequired: true}}, iouAttendees: [], @@ -1525,6 +1526,7 @@ describe('getIsViolationFixed', () => { const result = getIsViolationFixed('violations.missingAttendees', { ...defaultParams, isAttendeeTrackingEnabled: true, + isControlPolicy: true, category: 'Meals', policyCategories: {Meals: {name: 'Meals', enabled: true, areAttendeesRequired: true}}, iouAttendees: [createAttendee('user@example.com')], @@ -1536,12 +1538,27 @@ describe('getIsViolationFixed', () => { const result = getIsViolationFixed('violations.missingAttendees', { ...defaultParams, isAttendeeTrackingEnabled: true, + isControlPolicy: true, category: 'Meals', policyCategories: {Meals: {name: 'Meals', enabled: true, areAttendeesRequired: true}}, iouAttendees: [createAttendee('user@example.com'), createAttendee('other@example.com')], }); expect(result).toBe(true); }); + + it('should return true (violation fixed) when policy is not Control type, even if category requires attendees', () => { + // This covers the downgrade scenario: after downgrading from Control to Collect, + // the category may still have areAttendeesRequired: true but we should not enforce it + const result = getIsViolationFixed('violations.missingAttendees', { + ...defaultParams, + isAttendeeTrackingEnabled: true, + isControlPolicy: false, + category: 'Meals', + policyCategories: {Meals: {name: 'Meals', enabled: true, areAttendeesRequired: true}}, + iouAttendees: [], + }); + expect(result).toBe(true); + }); }); describe('unknown violations', () => { From ddf35fb2756552420569c157a4b444fb5b137326 Mon Sep 17 00:00:00 2001 From: Kevin Brian Bader Date: Wed, 28 Jan 2026 15:10:24 -0800 Subject: [PATCH 2/2] fix: complete feature flag removal / workspace type check --- src/components/ReportActionItem/MoneyRequestView.tsx | 1 + src/hooks/useTransactionViolations.ts | 1 - src/libs/AttendeeUtils.ts | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 9d8489ed232b..aa3300981fd2 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -530,6 +530,7 @@ function MoneyRequestView({ actualAttendees, currentUserPersonalDetails, policy?.isAttendeeTrackingEnabled, + policy?.type === CONST.POLICY.TYPE.CORPORATE, ); const getErrorForField = (field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], policyHasDependentTags = false, tagValue?: string) => { diff --git a/src/hooks/useTransactionViolations.ts b/src/hooks/useTransactionViolations.ts index 413d85bf6b11..71ab0554f952 100644 --- a/src/hooks/useTransactionViolations.ts +++ b/src/hooks/useTransactionViolations.ts @@ -1,7 +1,6 @@ import {useMemo} from 'react'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import {isViolationDismissed, mergeProhibitedViolations, shouldShowViolation} from '@libs/TransactionUtils'; -import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {TransactionViolation, TransactionViolations} from '@src/types/onyx'; import getEmptyArray from '@src/types/utils/getEmptyArray'; diff --git a/src/libs/AttendeeUtils.ts b/src/libs/AttendeeUtils.ts index 41cc15f05c9d..fd8ab57a303f 100644 --- a/src/libs/AttendeeUtils.ts +++ b/src/libs/AttendeeUtils.ts @@ -78,14 +78,14 @@ function syncMissingAttendeesViolation( isControlPolicy: boolean, isInvoice = false, ): T[] { - // No missingAttendees violation for invoices + // Don't show missingAttendees violation on invoices if (isInvoice) { return violations.filter((violation) => violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES); } const hasMissingAttendeesViolation = violations.some((v) => v.name === CONST.VIOLATIONS.MISSING_ATTENDEES); const shouldShowMissingAttendees = - isControlPolicy && getIsMissingAttendeesViolation(policyCategories ?? {}, category ?? '', attendees ?? [], userPersonalDetails, isAttendeeTrackingEnabled); + isControlPolicy && getIsMissingAttendeesViolation(policyCategories ?? {}, category ?? '', attendees ?? [], userPersonalDetails, isAttendeeTrackingEnabled, isControlPolicy); if (!hasMissingAttendeesViolation && shouldShowMissingAttendees) { // Add violation when it should show but isn't present from BE