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
15 changes: 9 additions & 6 deletions src/CONST/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5772,14 +5772,17 @@ const CONST = {
},

/**
* Feature flag to disable the missingAttendees violation feature.
* Set to true to disable the feature if regressions are found.
* Feature flag to enable the missingAttendees violation feature.
* Currently enabled only on staging for testing.
* When true:
* - Prevents new missingAttendees violations from being created
* - Removes existing missingAttendees violations from transaction lists
* - Hides "Require attendees" toggle in category settings
* - 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
*/
IS_ATTENDEES_REQUIRED_FEATURE_DISABLED: true,
// 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
24 changes: 19 additions & 5 deletions src/components/MoneyRequestConfirmationList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
isMerchantMissing,
isScanRequest as isScanRequestUtil,
} from '@libs/TransactionUtils';
import {getIsViolationFixed} from '@libs/Violations/ViolationsUtils';
import {hasInvoicingDetails} from '@userActions/Policy/Policy';
import type {IOUAction, IOUType} from '@src/CONST';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -457,6 +458,18 @@ function MoneyRequestConfirmationList({
[iouCategory, policyCategories, policy?.areRulesEnabled],
);

const isViolationFixed = getIsViolationFixed(formError, {
category: iouCategory,
tag: getTag(transaction),
taxCode: transaction?.taxCode,
policyCategories,
policyTagLists: policyTags,
policyTaxRates: policy?.taxRates?.taxes,
iouAttendees,
currentUserPersonalDetails,
isAttendeeTrackingEnabled: policy?.isAttendeeTrackingEnabled,
});

useEffect(() => {
if (shouldDisplayFieldError && didConfirmSplit) {
setFormError('iou.error.genericSmartscanFailureMessage');
Expand All @@ -466,20 +479,21 @@ function MoneyRequestConfirmationList({
setFormError('iou.receiptScanningFailed');
return;
}
// Check 1: If formError does NOT start with "violations.", clear it and return
// Reset the form error whenever the screen gains or loses focus
// but preserve violation-related errors since those represent real validation issues
// that can only be fixed by changing the underlying data
// that can only be resolved by fixing the underlying issue
if (!formError.startsWith(CONST.VIOLATIONS_PREFIX)) {
setFormError('');
return;
}
// Clear missingAttendees violation if user fixed it by changing category or attendees
const isMissingAttendeesViolation = getIsMissingAttendeesViolation(policyCategories, iouCategory, iouAttendees, currentUserPersonalDetails, policy?.isAttendeeTrackingEnabled);
if (formError === 'violations.missingAttendees' && !isMissingAttendeesViolation) {
// Check 2: Only reached if formError STARTS with "violations."
// Clear any violation error if the user has fixed the underlying issue
if (isViolationFixed) {
setFormError('');
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- we don't want this effect to run if it's just setFormError that changes
}, [isFocused, shouldDisplayFieldError, hasSmartScanFailed, didConfirmSplit, iouCategory, iouAttendees, policyCategories, currentUserPersonalDetails, policy?.isAttendeeTrackingEnabled]);
}, [isFocused, shouldDisplayFieldError, hasSmartScanFailed, didConfirmSplit, isViolationFixed]);

useEffect(() => {
// We want this effect to run only when the transaction is moving from Self DM to a expense chat
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
import React, {useCallback, useContext, useMemo} from 'react';
import {View} from 'react-native';
// We need direct access to useOnyx to fetch live policy data at render time
// without triggering the wrapper's additional logic, ensuring violations
// sync immediately when category settings change
// eslint-disable-next-line no-restricted-imports
import {useOnyx as originalUseOnyx} from 'react-native-onyx';
import {DelegateNoAccessContext} from '@components/DelegateNoAccessModalProvider';
import Icon from '@components/Icon';
import {useSearchContext} from '@components/Search/SearchContext';
import BaseListItem from '@components/SelectionListWithSections/BaseListItem';
import type {ExpenseReportListItemProps, ExpenseReportListItemType, ListItem} from '@components/SelectionListWithSections/types';
import Text from '@components/Text';
import useAnimatedHighlightStyle from '@hooks/useAnimatedHighlightStyle';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import {handleActionButtonPress} from '@libs/actions/Search';
import {isOpenExpenseReport, isProcessingReport} from '@libs/ReportUtils';
import {syncMissingAttendeesViolation} from '@libs/AttendeeUtils';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import {isInvoiceReport, isOpenExpenseReport, isProcessingReport} from '@libs/ReportUtils';
import {isViolationDismissed, shouldShowViolation} from '@libs/TransactionUtils';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import {isActionLoadingSelector} from '@src/selectors/ReportMetaData';
import type {Policy, Report} from '@src/types/onyx';
Expand Down Expand Up @@ -46,6 +56,12 @@ function ExpenseReportListItem<TItem extends ListItem>({
const [personalPolicyID] = useOnyx(ONYXKEYS.PERSONAL_POLICY_ID, {canBeMissing: true});
const [isActionLoading] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportItem.reportID}`, {canBeMissing: true, selector: isActionLoadingSelector});
const expensifyIcons = useMemoizedLazyExpensifyIcons(['DotIndicator']);
const currentUserDetails = useCurrentUserPersonalDetails();

// Fetch live policy categories from Onyx to sync violations at render time
const [parentPolicy] = originalUseOnyx(`${ONYXKEYS.COLLECTION.POLICY}${getNonEmptyStringOnyxID(reportItem.policyID)}`, {canBeMissing: true});
const [parentReport] = originalUseOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(reportItem.reportID)}`, {canBeMissing: true});
const [policyCategories] = originalUseOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${getNonEmptyStringOnyxID(reportItem.policyID)}`, {canBeMissing: true});

const searchData = currentSearchResults?.data;

Expand All @@ -66,6 +82,42 @@ function ExpenseReportListItem<TItem extends ListItem>({
return reportItem.isDisabled ?? reportItem.isDisabledCheckbox;
}, [reportItem.isDisabled, reportItem.isDisabledCheckbox]);

// Prefer live Onyx policy data over snapshot to ensure fresh policy settings
// like isAttendeeTrackingEnabled is not missing
// Use snapshotReport/snapshotPolicy as fallbacks to fix offline issues where
// newly created reports aren't in the search snapshot yet
const policyForViolations = parentPolicy ?? snapshotPolicy;
const reportForViolations = parentReport ?? snapshotReport;

// Sync missingAttendees violation at render time for each transaction in the report
// This ensures violations show immediately when category settings change, without needing to click the row
const hasSyncedMissingAttendeesViolation = useMemo(() => {
if (!policyForViolations?.isAttendeeTrackingEnabled || policyForViolations?.type !== CONST.POLICY.TYPE.CORPORATE) {
return false;
}

const isInvoice = isInvoiceReport(reportItem) || reportItem.type === CONST.REPORT.TYPE.INVOICE;
return reportItem?.transactions?.some((transaction) => {
const relevantViolations = (transaction.violations ?? []).filter(
(violation) =>
!isViolationDismissed(transaction, violation, currentUserDetails.email ?? '', currentUserDetails.accountID, reportForViolations, policyForViolations) &&
shouldShowViolation(reportForViolations, policyForViolations, violation.name, currentUserDetails.email ?? '', false, transaction),
);

const violations = syncMissingAttendeesViolation(
relevantViolations,
policyCategories,
transaction.category ?? '',
transaction.attendees,
currentUserDetails,
policyForViolations.isAttendeeTrackingEnabled ?? false,
policyForViolations.type === CONST.POLICY.TYPE.CORPORATE,
isInvoice,
);
return violations.some((violation) => violation.name === CONST.VIOLATIONS.MISSING_ATTENDEES);
});
}, [reportItem, policyCategories, policyForViolations, reportForViolations, currentUserDetails]);

const {isDelegateAccessRestricted, showDelegateNoAccessModal} = useContext(DelegateNoAccessContext);

const handleOnButtonPress = useCallback(() => {
Expand Down Expand Up @@ -133,8 +185,15 @@ function ExpenseReportListItem<TItem extends ListItem>({

const shouldShowViolationDescription = isOpenExpenseReport(reportItem) || isProcessingReport(reportItem);

// Show violation description if either:
// 1. Pre-computed hasVisibleViolations from search data, OR
// 2. Synced missingAttendees violation computed at render time (for stale data)
// We're using || instead of ?? because the variables are boolean
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const hasAnyVisibleViolations = reportItem?.hasVisibleViolations || hasSyncedMissingAttendeesViolation;

const getDescription = useMemo(() => {
if (!reportItem?.hasVisibleViolations || !shouldShowViolationDescription) {
if (!hasAnyVisibleViolations || !shouldShowViolationDescription) {
return;
}
return (
Expand All @@ -150,7 +209,7 @@ function ExpenseReportListItem<TItem extends ListItem>({
</View>
);
}, [
reportItem?.hasVisibleViolations,
hasAnyVisibleViolations,
shouldShowViolationDescription,
styles.flexRow,
styles.alignItemsCenter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type {TransactionPreviewData} from '@libs/actions/Search';
import {handleActionButtonPress as handleActionButtonPressUtil} from '@libs/actions/Search';
import {syncMissingAttendeesViolation} from '@libs/AttendeeUtils';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import {isInvoiceReport} from '@libs/ReportUtils';
import {isViolationDismissed, mergeProhibitedViolations, shouldShowViolation} from '@libs/TransactionUtils';
import variables from '@styles/variables';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -135,6 +136,7 @@ function TransactionListItem<TItem extends ListItem>({
shouldShowViolation(reportForViolations, policyForViolations, violation.name, currentUserDetails.email ?? '', false, transactionItem),
);

const isInvoice = isInvoiceReport(reportForViolations) || reportForViolations.type === CONST.REPORT.TYPE.INVOICE;
// Sync missingAttendees violation with current policy category settings (can be removed later when BE handles this)
// Use live transaction data (attendees, category) to ensure we check against current state, not stale snapshot
const attendeeOnyxViolations = syncMissingAttendeesViolation(
Expand All @@ -145,6 +147,7 @@ function TransactionListItem<TItem extends ListItem>({
currentUserDetails,
policyForViolations?.isAttendeeTrackingEnabled ?? false,
policyForViolations?.type === CONST.POLICY.TYPE.CORPORATE,
isInvoice,
);

const transactionViolations = mergeProhibitedViolations(attendeeOnyxViolations);
Expand Down
2 changes: 1 addition & 1 deletion src/components/TransactionItemRow/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ function TransactionItemRow({
if (!violations) {
return undefined;
}
if (CONST.IS_ATTENDEES_REQUIRED_FEATURE_DISABLED) {
if (!CONST.IS_ATTENDEES_REQUIRED_ENABLED) {
return violations.filter((violation) => violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES);
}
return violations;
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useTransactionViolations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function useTransactionViolations(transactionID?: string, shouldShowRterForSettl
(violation: TransactionViolation) =>
!isViolationDismissed(transaction, violation, currentUserDetails.email ?? '', currentUserDetails.accountID, iouReport, policy) &&
shouldShowViolation(iouReport, policy, violation.name, currentUserDetails.email ?? '', shouldShowRterForSettledReport, transaction) &&
(!CONST.IS_ATTENDEES_REQUIRED_FEATURE_DISABLED || violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES),
(CONST.IS_ATTENDEES_REQUIRED_ENABLED || violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES),
),
),
[transaction, transactionViolations, iouReport, policy, shouldShowRterForSettledReport, currentUserDetails.email, currentUserDetails.accountID],
Expand Down
8 changes: 5 additions & 3 deletions src/libs/AttendeeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function formatRequiredFieldsTitle(translate: LocaleContextProps['translate'], p

// 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_FEATURE_DISABLED && isAttendeeTrackingEnabled && policyCategory.areAttendeesRequired) {
if (CONST.IS_ATTENDEES_REQUIRED_ENABLED && isAttendeeTrackingEnabled && policyCategory.areAttendeesRequired) {
enabledFields.push(translate('iou.attendees'));
}

Expand All @@ -37,7 +37,7 @@ function getIsMissingAttendeesViolation(
isAttendeeTrackingEnabled = false,
) {
// Feature flag to quickly disable the attendees required feature
if (CONST.IS_ATTENDEES_REQUIRED_FEATURE_DISABLED) {
if (!CONST.IS_ATTENDEES_REQUIRED_ENABLED) {
return false;
}

Expand Down Expand Up @@ -76,10 +76,12 @@ function syncMissingAttendeesViolation<T extends {name: string}>(
userPersonalDetails: CurrentUserPersonalDetails,
isAttendeeTrackingEnabled: boolean,
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
if (CONST.IS_ATTENDEES_REQUIRED_FEATURE_DISABLED) {
// Never add missingAttendees violation for invoices
if (!CONST.IS_ATTENDEES_REQUIRED_ENABLED || isInvoice) {
return violations.filter((v) => v.name !== CONST.VIOLATIONS.MISSING_ATTENDEES);
}

Expand Down
4 changes: 2 additions & 2 deletions src/libs/TransactionUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1506,7 +1506,7 @@ function getTransactionViolations(
(violation) => !isViolationDismissed(transaction, violation, currentUserEmail, currentUserAccountID, iouReport, policy),
) ?? [];

if (CONST.IS_ATTENDEES_REQUIRED_FEATURE_DISABLED) {
if (!CONST.IS_ATTENDEES_REQUIRED_ENABLED) {
return violations.filter((violation) => violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES);
}

Expand Down Expand Up @@ -1947,7 +1947,7 @@ function hasViolation(
violation.type === CONST.VIOLATION_TYPES.VIOLATION &&
(showInReview === undefined || showInReview === (violation.showInReview ?? false)) &&
!isViolationDismissed(transaction, violation, currentUserEmail, currentUserAccountID, iouReport, policy) &&
(!CONST.IS_ATTENDEES_REQUIRED_FEATURE_DISABLED || violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES),
(CONST.IS_ATTENDEES_REQUIRED_ENABLED || violation.name !== CONST.VIOLATIONS.MISSING_ATTENDEES),
);
}

Expand Down
Loading
Loading