Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 0 additions & 13 deletions src/CONST/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
10 changes: 9 additions & 1 deletion src/components/MoneyRequestConfirmationList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ function MoneyRequestConfirmationList({
iouAttendees,
currentUserPersonalDetails,
isAttendeeTrackingEnabled: policy?.isAttendeeTrackingEnabled,
isControlPolicy: policy?.type === CONST.POLICY.TYPE.CORPORATE,
});

useEffect(() => {
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
3 changes: 0 additions & 3 deletions src/components/TransactionItemRow/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]);

Expand Down
4 changes: 1 addition & 3 deletions src/hooks/useTransactionViolations.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -29,8 +28,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],
Expand Down
18 changes: 8 additions & 10 deletions src/libs/AttendeeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -78,16 +78,14 @@ function syncMissingAttendeesViolation<T extends {name: string}>(
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);
// 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
Expand Down
7 changes: 1 addition & 6 deletions src/libs/TransactionUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@
});

let allPolicyTags: OnyxCollection<PolicyTagLists> = {};
Onyx.connect({

Check warning on line 143 in src/libs/TransactionUtils/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY_TAGS,
waitForCollectionCallback: true,
callback: (value) => {
Expand Down Expand Up @@ -1528,10 +1528,6 @@
(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;
}

Expand Down Expand Up @@ -1968,8 +1964,7 @@
(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),
Comment thread
ikevin127 marked this conversation as resolved.
);
}

Expand Down
30 changes: 11 additions & 19 deletions src/libs/Violations/ViolationsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, () => boolean> = {
[`${CONST.VIOLATIONS_PREFIX}${CONST.VIOLATIONS.CATEGORY_OUT_OF_POLICY}`]: () => {
Expand All @@ -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);
},
};

Expand Down Expand Up @@ -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});
}

Expand Down Expand Up @@ -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)
);
});
});
Expand Down
1 change: 1 addition & 0 deletions src/libs/Violations/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type ViolationFixParams = {
iouAttendees: Attendee[] | undefined;
currentUserPersonalDetails: CurrentUserPersonalDetails;
isAttendeeTrackingEnabled: boolean | undefined;
isControlPolicy?: boolean;
};

export default ViolationFixParams;
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function CategoryRequiredFieldsPage({
</View>
</View>
</OfflineWithFeedback>
{isAttendeeTrackingEnabled && CONST.IS_ATTENDEES_REQUIRED_ENABLED && (
{isAttendeeTrackingEnabled && (
<OfflineWithFeedback pendingAction={policyCategory?.pendingFields?.areAttendeesRequired}>
<View style={[styles.mh5]}>
<View style={[styles.flexRow, styles.mv5, styles.mr2, styles.alignItemsCenter, styles.justifyContentBetween]}>
Expand Down
29 changes: 23 additions & 6 deletions tests/unit/ViolationUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}],
};
Expand Down Expand Up @@ -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 = [];
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -850,15 +850,15 @@ 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);
// With 0 attendees, attendeesMinusOwnerCount = Math.max(0, 0 - 1) = 0, violation should be added
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: ''}],
Expand Down Expand Up @@ -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: [],
Expand All @@ -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')],
Expand All @@ -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', () => {
Expand Down
Loading