diff --git a/src/components/MoneyRequestReportView/MoneyRequestViewReportFields.tsx b/src/components/MoneyRequestReportView/MoneyRequestViewReportFields.tsx index f7862af150c7..29eb587a9650 100644 --- a/src/components/MoneyRequestReportView/MoneyRequestViewReportFields.tsx +++ b/src/components/MoneyRequestReportView/MoneyRequestViewReportFields.tsx @@ -18,6 +18,7 @@ import { getFieldViolationTranslation, getReportFieldKey, getReportFieldMaps, + getReportFieldsWithoutFailedAdds, isReportFieldDisabledForUser, isReportFieldTargetMatchingReport, shouldDisplayReportFields as shouldDisplayReportFieldsUtils, @@ -97,7 +98,7 @@ function ReportFieldView( value={reportField.fieldValue} isDisabled={reportField.isFieldDisabled} errorText={reportField.violationTranslation} - fieldList={policy?.fieldList} + fieldList={getReportFieldsWithoutFailedAdds(policy?.fieldList, policy?.errorFields)} onSaveValue={(value) => onSaveValue(reportField, value)} onDelete={ reportField.isFieldDeletable @@ -140,7 +141,7 @@ function MoneyRequestViewReportFields({report, policy, pendingAction, style, sho deleteReportField(report.reportID, report.fieldList?.[reportField.fieldKey] ?? reportField); }; - const {fieldValues, fieldsByName} = getReportFieldMaps(report, policy?.fieldList ?? {}, reportNameValuePairs); + const {fieldValues, fieldsByName} = getReportFieldMaps(report, policy?.fieldList ?? {}, reportNameValuePairs, policy?.errorFields); const fields = Object.values(fieldsByName); const sortedPolicyReportFields: EnrichedPolicyReportField[] = fields .filter((field) => isReportFieldTargetMatchingReport(report, field)) diff --git a/src/components/ReportActionItem/MoneyReportView.tsx b/src/components/ReportActionItem/MoneyReportView.tsx index 4c6fb1b18e14..ce51122e0906 100644 --- a/src/components/ReportActionItem/MoneyReportView.tsx +++ b/src/components/ReportActionItem/MoneyReportView.tsx @@ -134,7 +134,7 @@ function MoneyReportView({ StyleUtils.getColorStyle(theme.textSupporting), ]; - const {fieldsByName} = getReportFieldMaps(report, policy?.fieldList ?? {}, reportNameValuePairs); + const {fieldsByName} = getReportFieldMaps(report, policy?.fieldList ?? {}, reportNameValuePairs, policy?.errorFields); const policyReportFields = Object.values(fieldsByName).filter((field) => isReportFieldTargetMatchingReport(report, field)); const isOnlyTitleFieldEnabled = policyReportFields.every(shouldHideSingleReportField); diff --git a/src/hooks/useAdvancedSearchFilters.ts b/src/hooks/useAdvancedSearchFilters.ts index cb76d36a79b9..efe74fdebd54 100644 --- a/src/hooks/useAdvancedSearchFilters.ts +++ b/src/hooks/useAdvancedSearchFilters.ts @@ -1,5 +1,6 @@ import {isFilterableBankAccount} from '@libs/BankAccountUtils'; import {hasVendorFeatureOnAnyPolicy, isPolicyFeatureEnabled} from '@libs/PolicyUtils'; +import {getReportFieldsWithoutFailedAdds} from '@libs/ReportUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -246,7 +247,7 @@ const policyDerivedSelector = (policies: OnyxCollection) => { isAttendeeTrackingEnabled = isPolicyFeatureEnabled(policy, CONST.POLICY.MORE_FEATURES.IS_ATTENDEE_TRACKING_ENABLED); } if (!hasReportFields) { - hasReportFields = Object.values(policy.fieldList ?? {}).some((val) => val.type !== CONST.POLICY.DEFAULT_FIELD_LIST_TYPE); + hasReportFields = Object.values(getReportFieldsWithoutFailedAdds(policy.fieldList, policy.errorFields)).some((val) => val.type !== CONST.POLICY.DEFAULT_FIELD_LIST_TYPE); } if (!hasAnyTaxRates && policy.taxRates?.taxes && Object.keys(policy.taxRates.taxes).length > 0) { hasAnyTaxRates = true; diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index dd12bfd49699..3e9f9779cb6c 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -5624,7 +5624,8 @@ function isReportFieldDisabledForUser( */ function canEditReportTitle(report: OnyxEntry, policy: OnyxEntry, currentUserAccountID: number | undefined, rules: OnyxCollection): boolean { const titleField = - getAvailableReportFields(report, Object.values(policy?.fieldList ?? {})).find((reportField) => isReportFieldOfTypeTitle(reportField)) ?? getTitleFieldWithFallback(policy); + getAvailableReportFields(report, Object.values(policy?.fieldList ?? {}), policy?.errorFields).find((reportField) => isReportFieldOfTypeTitle(reportField)) ?? + getTitleFieldWithFallback(policy); const isFieldDisabled = isReportFieldDisabled(report, titleField, policy, rules); return !isFieldDisabled && isAdminOwnerApproverOrReportOwner(report, policy, currentUserAccountID) && isExpenseReport(report) && isGroupPolicyPolicyUtils(policy); @@ -5666,6 +5667,24 @@ function getReportFieldKey(reportFieldId: string | undefined) { return `expensify_${reportFieldId}`; } +/** + * Whether a policy report field failed to be created. It stays on the policy, pending ADD with an error, until the error + * is dismissed. Fields still pending ADD without an error (for example, created offline) haven't failed. + */ +function isFailedReportFieldAdd(reportField: (PolicyReportField & {pendingAction?: PendingAction | null}) | undefined, policyErrorFields: ErrorFields | undefined): boolean { + return reportField?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD && !!policyErrorFields?.fieldList?.[getReportFieldKey(reportField.fieldID)]; +} + +/** + * Get the policy report fields without the ones that failed to be created + */ +function getReportFieldsWithoutFailedAdds( + fieldList: Record | undefined, + policyErrorFields: ErrorFields | undefined, +): Record { + return Object.fromEntries(Object.entries(fieldList ?? {}).filter(([, reportField]) => !isFailedReportFieldAdd(reportField, policyErrorFields))); +} + /** * Get the report fields attached to the policy given policyID */ @@ -5689,7 +5708,11 @@ function getReportFieldsByPolicyID(policy: OnyxEntry): Policy['fieldList * Get the report fields that we should display a MoneyReportView gets opened */ -function getAvailableReportFields(report: OnyxEntry, policyReportFields: PolicyReportField[]): PolicyReportField[] { +function getAvailableReportFields( + report: OnyxEntry, + policyReportFields: Array, + policyErrorFields?: ErrorFields, +): PolicyReportField[] { // Get the report fields that are attached to a report. These will persist even if a field is deleted from the policy. const reportFields = Object.values(report?.fieldList ?? {}); const reportIsSettled = isSettled(report?.reportID); @@ -5701,7 +5724,10 @@ function getAvailableReportFields(report: OnyxEntry, policyReportFields: // If the report is unsettled, we want to merge the new fields that get added to the policy with the fields that // are attached to the report. - const mergedFieldIds = Array.from(new Set([...policyReportFields.map(({fieldID}) => fieldID), ...reportFields.map(({fieldID}) => fieldID)])); + // Policy fields that failed to be created are skipped. + const reportFieldIDs = new Set(reportFields.map(({fieldID}) => fieldID)); + const availablePolicyReportFields = policyReportFields.filter((policyReportField) => !isFailedReportFieldAdd(policyReportField, policyErrorFields)); + const mergedFieldIds = Array.from(new Set([...availablePolicyReportFields.map(({fieldID}) => fieldID), ...reportFieldIDs])); const fields = mergedFieldIds.map((id) => { const field = report?.fieldList?.[getReportFieldKey(id)] ?? report?.fieldList?.[id]; @@ -8002,7 +8028,7 @@ function buildOptimisticIOUReport( parentReportID: chatReportID, created, lastVisibleActionCreated: created, - fieldList: policy?.fieldList, + fieldList: policy?.fieldList ? getReportFieldsWithoutFailedAdds(policy.fieldList, policy.errorFields) : undefined, parentReportActionID, }; } @@ -8227,7 +8253,7 @@ function buildOptimisticExpenseReport({ } // Otherwise, keep the default format: `${policyName} owes ${formattedTotal}` - expenseReport.fieldList = policy?.fieldList; + expenseReport.fieldList = policy?.fieldList ? getReportFieldsWithoutFailedAdds(policy.fieldList, policy.errorFields) : undefined; return expenseReport; } @@ -10869,7 +10895,7 @@ function hasVisibleReportFieldViolations( return false; } - const {fieldsByName} = getReportFieldMaps(report, policy.fieldList, reportNameValuePairs); + const {fieldsByName} = getReportFieldMaps(report, policy.fieldList, reportNameValuePairs, policy.errorFields); return Object.values(fieldsByName).some((field) => { if (!isReportFieldTargetMatchingReport(report, field)) { @@ -15116,8 +15142,9 @@ function getReportFieldMaps( report: OnyxEntry, fieldList: Record, reportNameValuePairs: OnyxEntry, + policyErrorFields?: ErrorFields, ): {fieldValues: Record; fieldsByName: Record} { - const availableFields = getAvailableReportFields(report, Object.values(fieldList ?? {})); + const availableFields = getAvailableReportFields(report, Object.values(fieldList ?? {}), policyErrorFields); const invoiceReportFields = report?.type === CONST.REPORT.TYPE.INVOICE ? getInvoiceReportFieldsFromReportNameValuePairs(reportNameValuePairs) : []; const fieldsMatchingReport = [...availableFields, ...invoiceReportFields].filter((field) => isReportFieldTargetMatchingReport(report, field)); const fields = [...new Map(fieldsMatchingReport.map((field) => [field.fieldID, field])).values()]; @@ -15518,6 +15545,8 @@ export { getReimbursementQueuedActionMessage, getReportDescription, getReportFieldKey, + getReportFieldsWithoutFailedAdds, + isFailedReportFieldAdd, getReportFieldMaps, getReportIDFromLink, getReportTransactions, diff --git a/src/libs/actions/Policy/ReportField.ts b/src/libs/actions/Policy/ReportField.ts index 01cd2864f232..e9db25c08ae1 100644 --- a/src/libs/actions/Policy/ReportField.ts +++ b/src/libs/actions/Policy/ReportField.ts @@ -25,7 +25,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {WorkspaceReportFieldForm} from '@src/types/form/WorkspaceReportFieldForm'; import INPUT_IDS from '@src/types/form/WorkspaceReportFieldForm'; import type {Policy, PolicyReportField} from '@src/types/onyx'; -import type {OnyxValueWithOfflineFeedback} from '@src/types/onyx/OnyxCommon'; +import type {ErrorFields, OnyxValueWithOfflineFeedback} from '@src/types/onyx/OnyxCommon'; import type {OnyxData} from '@src/types/onyx/Request'; import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx'; @@ -95,6 +95,31 @@ type UpdateReportFieldInitialValueParams = { policy: OnyxEntry; }; +/** + * Returns the errorFields update that clears a single report field's errors without wiping other policy errors (such as the report title error). + */ +function getReportFieldErrorFieldsToClear(fieldKey: string) { + return { + fieldList: { + [fieldKey]: null, + }, + }; +} + +/** + * Returns the errorFields update that sets a report field's generic failure error at errorFields.fieldList..defaultValue, where the server also returns its errors. + */ +function getReportFieldFailureErrorFields(fieldKey: string) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- ErrorFields doesn't model the nested fieldList..defaultValue shape the server uses, same as the report title error + return { + fieldList: { + [fieldKey]: { + defaultValue: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.reportFields.genericFailureMessage'), + }, + }, + } as unknown as ErrorFields; +} + function openPolicyReportFieldsPage(policyID: string) { if (!policyID) { Log.warn('openPolicyReportFieldsPage invalid params', {policyID}); @@ -237,7 +262,7 @@ function createReportField({name, type, initialValue, listValues, disabledListVa fieldList: { [fieldKey]: {...optimisticReportFieldDataForPolicy, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}, }, - errorFields: null, + errorFields: getReportFieldErrorFieldsToClear(fieldKey), }, }, ...(policyReportIDs ?? []).map( @@ -258,12 +283,7 @@ function createReportField({name, type, initialValue, listValues, disabledListVa key: `${ONYXKEYS.COLLECTION.POLICY}${policy?.id}`, onyxMethod: Onyx.METHOD.MERGE, value: { - fieldList: { - [fieldKey]: null, - }, - errorFields: { - [fieldKey]: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.reportFields.genericFailureMessage'), - }, + errorFields: getReportFieldFailureErrorFields(fieldKey), }, }, ...(policyReportIDs ?? []).map( @@ -289,7 +309,7 @@ function createReportField({name, type, initialValue, listValues, disabledListVa fieldList: { [fieldKey]: {pendingAction: null}, }, - errorFields: null, + errorFields: getReportFieldErrorFieldsToClear(fieldKey), }, }, ], @@ -323,7 +343,10 @@ function deleteReportFields({policy, reportFieldsToUpdate}: DeleteReportFieldsPa return; } - const updatedReportFields = Object.fromEntries(Object.entries(allReportFields).filter(([key]) => !resolvedReportFieldKeys.includes(key))); + // Fields that failed to be created don't exist on the server, so they're left out of the remaining fields we send + const updatedReportFields = Object.fromEntries( + Object.entries(ReportUtils.getReportFieldsWithoutFailedAdds(allReportFields, policy.errorFields)).filter(([key]) => !resolvedReportFieldKeys.includes(key)), + ); const optimisticReportFields = resolvedReportFieldKeys.reduce>>>((acc, reportFieldKey) => { acc[reportFieldKey] = {pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE}; return acc; @@ -334,6 +357,9 @@ function deleteReportFields({policy, reportFieldsToUpdate}: DeleteReportFieldsPa return acc; }, {}); + // Only the deleted fields' errors are cleared, so other fields keep theirs (for example, a field that failed to be created) + const successErrorFields = {fieldList: successReportFields}; + const failureReportFields = resolvedReportFieldKeys.reduce>>>((acc, reportFieldKey) => { acc[reportFieldKey] = {pendingAction: null}; return acc; @@ -355,7 +381,7 @@ function deleteReportFields({policy, reportFieldsToUpdate}: DeleteReportFieldsPa key: `${ONYXKEYS.COLLECTION.POLICY}${policy?.id}`, value: { fieldList: successReportFields, - errorFields: null, + errorFields: successErrorFields, }, }, ], @@ -427,7 +453,7 @@ function updateReportFieldInitialValue({policy, reportFieldID, newInitialValue}: fieldList: { [fieldKey]: {...updatedReportField, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}, }, - errorFields: null, + errorFields: getReportFieldErrorFieldsToClear(fieldKey), }, }, ], @@ -439,7 +465,7 @@ function updateReportFieldInitialValue({policy, reportFieldID, newInitialValue}: fieldList: { [fieldKey]: {pendingAction: null}, }, - errorFields: null, + errorFields: getReportFieldErrorFieldsToClear(fieldKey), }, }, ], @@ -451,9 +477,7 @@ function updateReportFieldInitialValue({policy, reportFieldID, newInitialValue}: fieldList: { [fieldKey]: {...previousFieldList[fieldKey], pendingAction: null}, }, - errorFields: { - [fieldKey]: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.reportFields.genericFailureMessage'), - }, + errorFields: getReportFieldFailureErrorFields(fieldKey), }, }, ], @@ -626,9 +650,36 @@ function removeReportFieldListValue({policy, reportFieldID, valueIndexes}: Remov API.write(removeCommand, parameters, onyxData); } +/** + * Clears a report field's errors. A field that failed to be created is removed from the list. + */ +function clearReportFieldErrors(policy: OnyxEntry, reportFieldID: string) { + if (!policy?.id) { + return; + } + + const fieldKey = ReportUtils.getReportFieldKey(reportFieldID); + const reportField = policy.fieldList?.[fieldKey]; + + if (reportField?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) { + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`, { + fieldList: { + [fieldKey]: null, + }, + errorFields: getReportFieldErrorFieldsToClear(fieldKey), + }); + return; + } + + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`, { + errorFields: getReportFieldErrorFieldsToClear(fieldKey), + }); +} + export type {CreateReportFieldParams}; export { + clearReportFieldErrors, setInitialCreateReportFieldsForm, createReportFieldsListValue, renameReportFieldsListValue, diff --git a/src/pages/DynamicEditReportFieldPage.tsx b/src/pages/DynamicEditReportFieldPage.tsx index f24e212352c5..650e610aaed7 100644 --- a/src/pages/DynamicEditReportFieldPage.tsx +++ b/src/pages/DynamicEditReportFieldPage.tsx @@ -23,6 +23,7 @@ import {getReportName} from '@libs/ReportNameUtils'; import { getReportFieldFromReportNameValuePairs, getReportFieldKey, + getReportFieldsWithoutFailedAdds, getTitleFieldWithFallback, isGroupPolicyExpenseReport, isInvoiceReport, @@ -134,6 +135,7 @@ function DynamicEditReportFieldPage({route}: DynamicEditReportFieldPageProps) { const menuItems: PopoverMenuItem[] = []; const isReportFieldDeletable = reportField.deletable && reportField?.fieldID !== CONST.REPORT_FIELD_TITLE_FIELD_ID; + const policyFieldList = getReportFieldsWithoutFailedAdds(policy?.fieldList, policy?.errorFields); if (isReportFieldDeletable) { menuItems.push({ @@ -168,7 +170,7 @@ function DynamicEditReportFieldPage({route}: DynamicEditReportFieldPageProps) { fieldValue={fieldValue} isRequired={!isReportFieldDeletable} onSubmit={handleReportFieldChange} - fieldList={policy?.fieldList} + fieldList={policyFieldList} /> )} @@ -198,7 +200,7 @@ function DynamicEditReportFieldPage({route}: DynamicEditReportFieldPageProps) { fieldValue={fieldValue} isRequired={!isReportFieldDeletable} onSubmit={handleReportFieldChange} - fieldList={policy?.fieldList} + fieldList={policyFieldList} disabled /> )} diff --git a/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx b/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx index 45942fe1a3b5..3a9016856564 100644 --- a/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx +++ b/src/pages/DynamicReportDetailsPage/ReportDetailsTitleSection.tsx @@ -76,7 +76,7 @@ function ReportDetailsTitleSectionContent({report}: ReportDetailsTitleSectionCon const parentNavigationSubtitleData = getParentNavigationSubtitle(report, policy, conciergeReportID, translate, formatPhoneNumber, derivedParentReportName, isParentReportArchived); const titleField: OnyxTypes.PolicyReportField | undefined = (() => { - const fields = getAvailableReportFields(report, Object.values(policy?.fieldList ?? {})); + const fields = getAvailableReportFields(report, Object.values(policy?.fieldList ?? {}), policy?.errorFields); return fields.find((reportField) => isReportFieldOfTypeTitle(reportField)); })(); const fieldKey = getReportFieldKey(titleField?.fieldID); diff --git a/src/pages/workspace/fields/CreateFieldsPage.tsx b/src/pages/workspace/fields/CreateFieldsPage.tsx index 36ae2013625e..8ffcd20eeb60 100644 --- a/src/pages/workspace/fields/CreateFieldsPage.tsx +++ b/src/pages/workspace/fields/CreateFieldsPage.tsx @@ -15,6 +15,7 @@ import {setDraftValues} from '@libs/actions/FormActions'; import {addErrorMessage} from '@libs/ErrorUtils'; import {hasCircularReferences} from '@libs/Formula'; import Navigation from '@libs/Navigation/Navigation'; +import {getReportFieldsWithoutFailedAdds} from '@libs/ReportUtils'; import {isRequiredFulfilled} from '@libs/ValidationUtils'; import {getExistingReportFieldByName, getReportFieldsForTarget, getUnsupportedReportFieldFormulaParts, hasFormulaPartsInInitialValue} from '@libs/WorkspaceReportFieldUtils'; @@ -56,6 +57,8 @@ function CreateFieldsPage({policy, policyID, isInvoiceField, listValuesRoute, ge const reportTypeForTarget = isInvoiceField ? CONST.REPORT.TYPE.INVOICE : CONST.REPORT.TYPE.EXPENSE; const fieldTarget = isInvoiceField ? CONST.REPORT_FIELD_TARGETS.INVOICE : CONST.REPORT_FIELD_TARGETS.EXPENSE; + // Fields that failed to be created are left out, so the user can retry with the same name without dismissing the error first. + const fieldList = getReportFieldsWithoutFailedAdds(policy?.fieldList, policy?.errorFields); const policyReportIDsSelector = (reports: OnyxCollection) => Object.values(reports ?? {}) @@ -83,7 +86,7 @@ function CreateFieldsPage({policy, policyID, isInvoiceField, listValuesRoute, ge }; const getExistingFieldNameError = (name: string) => { - const existingField = getExistingReportFieldByName(policy?.fieldList, name); + const existingField = getExistingReportFieldByName(fieldList, name); if (!existingField) { return undefined; } @@ -117,7 +120,7 @@ function CreateFieldsPage({policy, policyID, isInvoiceField, listValuesRoute, ge if ( (type === CONST.REPORT_FIELD_TYPES.TEXT || type === CONST.REPORT_FIELD_TYPES.FORMULA) && - hasCircularReferences(formInitialValue, name, getReportFieldsForTarget(policy?.fieldList, fieldTarget)) + hasCircularReferences(formInitialValue, name, getReportFieldsForTarget(fieldList, fieldTarget)) ) { errors[INPUT_IDS.INITIAL_VALUE] = translate('workspace.reportFields.circularReferenceError'); } diff --git a/src/pages/workspace/fields/FieldsInitialValuePage.tsx b/src/pages/workspace/fields/FieldsInitialValuePage.tsx index fc41a46e2bf1..2ed8a5b6759c 100644 --- a/src/pages/workspace/fields/FieldsInitialValuePage.tsx +++ b/src/pages/workspace/fields/FieldsInitialValuePage.tsx @@ -12,7 +12,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import {hasCircularReferences} from '@libs/Formula'; import Navigation from '@libs/Navigation/Navigation'; -import {getReportFieldKey, hasEnabledListValue} from '@libs/ReportUtils'; +import {getReportFieldKey, getReportFieldsWithoutFailedAdds, hasEnabledListValue} from '@libs/ReportUtils'; import {isRequiredFulfilled} from '@libs/ValidationUtils'; import {getReportFieldInitialValue, getReportFieldsForTarget, getUnsupportedReportFieldFormulaParts, isReportFieldTargetValid} from '@libs/WorkspaceReportFieldUtils'; @@ -75,7 +75,7 @@ function FieldsInitialValuePage({policy, policyID, reportFieldID, featureName, e if ( (reportField?.type === CONST.REPORT_FIELD_TYPES.TEXT || reportField?.type === CONST.REPORT_FIELD_TYPES.FORMULA) && - hasCircularReferences(formInitialValue, reportField?.name, getReportFieldsForTarget(policy?.fieldList, expectedTarget)) + hasCircularReferences(formInitialValue, reportField?.name, getReportFieldsForTarget(getReportFieldsWithoutFailedAdds(policy?.fieldList, policy?.errorFields), expectedTarget)) ) { errors[INPUT_IDS.INITIAL_VALUE] = translate('workspace.reportFields.circularReferenceError'); } diff --git a/src/pages/workspace/fields/WorkspaceFieldsSection.tsx b/src/pages/workspace/fields/WorkspaceFieldsSection.tsx index ca80432391cc..14244a478cec 100644 --- a/src/pages/workspace/fields/WorkspaceFieldsSection.tsx +++ b/src/pages/workspace/fields/WorkspaceFieldsSection.tsx @@ -16,15 +16,17 @@ import usePolicyFeatureWriteAccess from '@hooks/usePolicyFeatureWriteAccess'; import useThemeStyles from '@hooks/useThemeStyles'; import {isConnectionInProgress, isConnectionUnverified} from '@libs/actions/connections'; +import {getLatestErrorField} from '@libs/ErrorUtils'; import Navigation from '@libs/Navigation/Navigation'; import {getConnectedIntegration, isControlPolicy, shouldShowSyncError} from '@libs/PolicyUtils'; import type {PolicyFeature} from '@libs/PolicyUtils'; +import {getReportFieldKey, isFailedReportFieldAdd} from '@libs/ReportUtils'; import {getReportFieldTypeTranslationKey, isReportFieldImportedFromIntegration} from '@libs/WorkspaceReportFieldUtils'; import {getCurrentAccountingIntegrationName} from '@pages/workspace/accounting/utils'; import ToggleSettingOptionRow from '@pages/workspace/workflows/ToggleSettingsOptionRow'; -import {setInitialCreateReportFieldsForm} from '@userActions/Policy/ReportField'; +import {clearReportFieldErrors, setInitialCreateReportFieldsForm} from '@userActions/Policy/ReportField'; import CONST from '@src/CONST'; import type {TranslationPaths} from '@src/languages/types'; @@ -32,7 +34,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type {Route} from '@src/ROUTES'; import type {Policy, PolicyConnectionName} from '@src/types/onyx'; -import type {PendingAction} from '@src/types/onyx/OnyxCommon'; +import type {Errors, PendingAction} from '@src/types/onyx/OnyxCommon'; import type {ListRenderItemInfo} from '@shopify/flash-list'; import type {OnyxEntry} from 'react-native-onyx'; @@ -46,7 +48,9 @@ type FieldListValue = NonNullable[string]; type FieldListItem = { fieldID: string; + errors: Errors; isDisabled: boolean; + isFailedAdd: boolean; keyForList: string; pendingAction?: PendingAction; rightLabel: string; @@ -117,6 +121,7 @@ function WorkspaceFieldsSection({ const isConnectionVerified = connectedIntegration && !isConnectionUnverified(policy, connectedIntegration); const currentConnectionName = getCurrentAccountingIntegrationName(policy, translate); const fieldList = policy?.fieldList; + const errorFields = policy?.errorFields; const hasImportedField = Object.values(fieldList ?? {}).some((field) => fieldFilter(field) && isReportFieldImportedFromIntegration(field, policy)); const {canWrite, withReadOnlyFallback} = usePolicyFeatureWriteAccess(policy, policyFeature); @@ -135,17 +140,24 @@ function WorkspaceFieldsSection({ : Object.values(fieldList) .filter(fieldFilter) .sort((a, b) => localeCompare(a.name, b.name)) - .map((field) => ({ - text: field.name, - keyForList: String(field.fieldID), - fieldID: field.fieldID, - pendingAction: field.pendingAction, - isDisabled: field.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, - rightLabel: Str.recapitalize(translate(getReportFieldTypeTranslationKey(field.type ?? CONST.REPORT_FIELD_TYPES.TEXT))), - })); + .map((field) => { + const fieldKey = getReportFieldKey(field.fieldID); + + return { + text: field.name, + keyForList: String(field.fieldID), + fieldID: field.fieldID, + pendingAction: field.pendingAction, + errors: getLatestErrorField({errorFields: errorFields?.fieldList?.[fieldKey] ?? {}}, 'defaultValue'), + isDisabled: field.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, + isFailedAdd: isFailedReportFieldAdd(field, errorFields), + rightLabel: Str.recapitalize(translate(getReportFieldTypeTranslationKey(field.type ?? CONST.REPORT_FIELD_TYPES.TEXT))), + }; + }); const navigateToFieldSettings = (item: FieldListItem) => { - if (!canWrite) { + // A field that failed to be created doesn't exist on the server, so its settings can't be saved. The user dismisses the error instead. + if (!canWrite || item.isFailedAdd) { return; } @@ -153,14 +165,20 @@ function WorkspaceFieldsSection({ }; const renderItem = ({item}: ListRenderItemInfo) => ( - + clearReportFieldErrors(policy, item.fieldID)} + > navigateToFieldSettings(item)} description={item.text} disabled={item.isDisabled} - shouldShowRightIcon={!item.isDisabled && canWrite} - interactive={!item.isDisabled && canWrite} + shouldShowRightIcon={!item.isDisabled && !item.isFailedAdd && canWrite} + interactive={!item.isDisabled && !item.isFailedAdd && canWrite} rightLabel={item.rightLabel} descriptionTextStyle={[styles.popoverMenuText, styles.textStrong]} /> diff --git a/src/selectors/Policy.ts b/src/selectors/Policy.ts index 5469c1a96a57..21d64f36dddd 100644 --- a/src/selectors/Policy.ts +++ b/src/selectors/Policy.ts @@ -23,6 +23,7 @@ import { isTimeTrackingEnabled, shouldShowPolicy, } from '@libs/PolicyUtils'; +import {getReportFieldsWithoutFailedAdds} from '@libs/ReportUtils'; import type {BillingRestrictionPolicy} from '@libs/SubscriptionUtils'; import {getDefaultAvatarURL} from '@libs/UserAvatarUtils'; @@ -267,12 +268,12 @@ const createHasWorkspaceToSubmitToSelector = getActivePoliciesWithExpenseChat(policies, currentUserLogin).length > 0; /** - * Creates a selector that aggregates all non-formula policy report fields from all policies, + * Creates a selector that aggregates all non-formula policy report fields from all policies, except the ones that failed to be created, * sorted alphabetically by field key using the provided locale compare function */ const createAllPolicyReportFieldsSelector = (policies: OnyxCollection, localeCompare: (a: string, b: string) => number) => { const allPolicyReportFields = Object.values(policies ?? {}).reduce>((acc, policy) => { - Object.assign(acc, policy?.fieldList ?? {}); + Object.assign(acc, getReportFieldsWithoutFailedAdds(policy?.fieldList, policy?.errorFields)); return acc; }, {}); diff --git a/tests/actions/ReportFieldTest.ts b/tests/actions/ReportFieldTest.ts index 1e8639df010c..398a24089823 100644 --- a/tests/actions/ReportFieldTest.ts +++ b/tests/actions/ReportFieldTest.ts @@ -1,3 +1,4 @@ +import {WRITE_COMMANDS} from '@libs/API/types'; import DateUtils from '@libs/DateUtils'; import {generateFieldID} from '@libs/WorkspaceReportFieldUtils'; @@ -13,7 +14,7 @@ import * as ReportUtils from '@src/libs/ReportUtils'; import ONYXKEYS from '@src/ONYXKEYS'; import INPUT_IDS from '@src/types/form/WorkspaceReportFieldForm'; import type {PolicyReportField, Policy as PolicyType} from '@src/types/onyx'; -import type {OnyxValueWithOfflineFeedback} from '@src/types/onyx/OnyxCommon'; +import type {ErrorFields, OnyxValueWithOfflineFeedback} from '@src/types/onyx/OnyxCommon'; import type {OnyxEntry} from 'react-native-onyx'; @@ -230,6 +231,93 @@ describe('actions/ReportField', () => { // Check if the policy pending action was cleared expect(policy?.fieldList?.[reportFieldKey].pendingAction).toBeFalsy(); }); + + it('keeps the failed report field with an error until the error is dismissed', async () => { + // Given a workspace with a report title error + const reportFieldName = 'Test Field'; + const reportFieldID = generateFieldID(reportFieldName); + const reportFieldKey = ReportUtils.getReportFieldKey(reportFieldID); + const titleErrors = {[CONST.POLICY.FIELDS.FIELD_LIST_TITLE]: 'Title error'}; + const fakePolicy: PolicyType = { + ...createRandomPolicy(0), + fieldList: {}, + errorFields: {fieldList: titleErrors}, + }; + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + + // When creating a report field fails on the server + mockFetch.pause(); + ReportField.createReportField({ + policy: fakePolicy, + name: reportFieldName, + type: CONST.REPORT_FIELD_TYPES.TEXT, + initialValue: '{field:DoesNotExist}', + listValues: [], + disabledListValues: [], + policyReportIDs: [], + }); + await waitForBatchedUpdates(); + mockFetch.fail(); + mockFetch.resume(); + await waitForBatchedUpdates(); + + // Then the field stays in the list with its error so the user can see which field failed, and the title error is kept + let policy = await connectToFetchPolicy(fakePolicy.id); + expect(policy?.fieldList?.[reportFieldKey]?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); + expect(policy?.errorFields?.fieldList?.[reportFieldKey]).toBeTruthy(); + expect(policy?.errorFields?.fieldList?.[CONST.POLICY.FIELDS.FIELD_LIST_TITLE]).toBe(titleErrors[CONST.POLICY.FIELDS.FIELD_LIST_TITLE]); + + // When the user dismisses the error + ReportField.clearReportFieldErrors(policy, reportFieldID); + await waitForBatchedUpdates(); + + // Then the failed field and its errors are removed, and the title error is untouched + policy = await connectToFetchPolicy(fakePolicy.id); + expect(policy?.fieldList?.[reportFieldKey]).toBeUndefined(); + expect(policy?.errorFields?.fieldList).toEqual(titleErrors); + }); + }); + + describe('clearReportFieldErrors', () => { + it('clears the errors of an existing report field without removing it', async () => { + // Given an existing report field whose initial value update was rejected by the server + const reportFieldName = 'Test Field'; + const reportFieldID = generateFieldID(reportFieldName); + const reportFieldKey = ReportUtils.getReportFieldKey(reportFieldID); + const reportField: PolicyReportField = { + name: reportFieldName, + type: CONST.REPORT_FIELD_TYPES.TEXT, + defaultValue: 'Default Value', + values: [], + disabledOptions: [], + fieldID: reportFieldID, + orderWeight: 1, + deletable: false, + keys: [], + externalIDs: [], + isTax: false, + }; + const errorKey = DateUtils.getMicroseconds(); + const fakePolicy: PolicyType = { + ...createRandomPolicy(0), + fieldList: {[reportFieldKey]: reportField}, + // The server returns report field errors at errorFields.fieldList..defaultValue, which ErrorFields doesn't model + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + errorFields: { + fieldList: {[reportFieldKey]: {defaultValue: {[errorKey]: 'Formula field {field:DoesNotExist} not recognized'}}}, + } as unknown as ErrorFields, + }; + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + + // When the user dismisses the error + ReportField.clearReportFieldErrors(fakePolicy, reportFieldID); + await waitForBatchedUpdates(); + + // Then the error is cleared and the field is kept, because it already exists on the server + const policy = await connectToFetchPolicy(fakePolicy.id); + expect(policy?.fieldList?.[reportFieldKey]).toEqual(reportField); + expect(policy?.errorFields?.fieldList?.[reportFieldKey]).toBeUndefined(); + }); }); it('optimistically treats Text type with formula-like initial value as Formula', async () => { @@ -372,6 +460,61 @@ describe('actions/ReportField', () => { [reportFieldKey]: fakeReportField, }); }); + + it('leaves a report field that failed to be created out of the request and keeps its error', async () => { + // Given a workspace with a field to delete and a field that failed to be created, which is still pending ADD with its error + const reportFieldToDelete: PolicyReportField = { + name: 'Field To Delete', + type: CONST.REPORT_FIELD_TYPES.TEXT, + defaultValue: 'Default Value', + values: [], + disabledOptions: [], + fieldID: generateFieldID('Field To Delete'), + orderWeight: 1, + deletable: true, + keys: [], + externalIDs: [], + isTax: false, + }; + const failedReportField: OnyxValueWithOfflineFeedback = { + ...reportFieldToDelete, + name: 'Failed Field', + defaultValue: '{field:DoesNotExist}', + fieldID: generateFieldID('Failed Field'), + orderWeight: 2, + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + }; + const reportFieldToDeleteKey = ReportUtils.getReportFieldKey(reportFieldToDelete.fieldID); + const failedReportFieldKey = ReportUtils.getReportFieldKey(failedReportField.fieldID); + const failedReportFieldErrors = {defaultValue: {[DateUtils.getMicroseconds()]: 'Formula field {field:DoesNotExist} not recognized'}}; + const fakePolicy: PolicyType = { + ...createRandomPolicy(0), + fieldList: { + [reportFieldToDeleteKey]: reportFieldToDelete, + [failedReportFieldKey]: failedReportField, + }, + // The server returns report field errors at errorFields.fieldList..defaultValue, which ErrorFields doesn't model + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + errorFields: {fieldList: {[failedReportFieldKey]: failedReportFieldErrors}} as unknown as ErrorFields, + }; + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + + // When the user deletes the other field before dismissing the error + ReportField.deleteReportFields({policy: fakePolicy, reportFieldsToUpdate: [reportFieldToDeleteKey]}); + await waitForBatchedUpdates(); + + // Then the failed field isn't sent to the server as a remaining field, because it was never created there + const body = TestHelper.getFetchMockCalls(WRITE_COMMANDS.DELETE_POLICY_REPORT_FIELD).at(0)?.[1]?.body; + const reportFields = body instanceof FormData ? body.get('reportFields') : null; + expect(reportFields).toBe(JSON.stringify([])); + + // Then the failed field keeps its error, so it's still treated as failed until the error is dismissed + const policy = await connectToFetchPolicy(fakePolicy.id); + expect(policy?.fieldList?.[reportFieldToDeleteKey]).toBeUndefined(); + expect(policy?.fieldList?.[failedReportFieldKey]?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); + expect(policy?.errorFields?.fieldList?.[failedReportFieldKey]).toEqual(failedReportFieldErrors); + expect(ReportUtils.isFailedReportFieldAdd(policy?.fieldList?.[failedReportFieldKey], policy?.errorFields)).toBe(true); + }); }); describe('updateReportFieldInitialValue', () => { @@ -480,7 +623,7 @@ describe('actions/ReportField', () => { [reportFieldKey]: reportField, }); // Check if the policy errors was set - expect(policy?.errorFields?.[reportFieldKey]).toBeTruthy(); + expect(policy?.errorFields?.fieldList?.[reportFieldKey]).toBeTruthy(); }); it('reverts optimistic type to text when new initial value is non-formula', async () => { diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 43f7c1f81f1e..55c902f79dfe 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -146,6 +146,7 @@ import { getReportActionWithSmartscanError, getReportFieldMaps, getReportFieldsByPolicyID, + getReportFieldsWithoutFailedAdds, getReportForHeader, getReportIDFromLink, getReportNotificationPreference, @@ -275,7 +276,7 @@ import type { Transaction, TransactionViolation, } from '@src/types/onyx'; -import type {Icon, OnyxValueWithOfflineFeedback} from '@src/types/onyx/OnyxCommon'; +import type {ErrorFields, Icon, OnyxValueWithOfflineFeedback} from '@src/types/onyx/OnyxCommon'; import type {ACHAccount, PolicyReportField} from '@src/types/onyx/Policy'; import type {Participant, Participants, ReportCollectionDataSet} from '@src/types/onyx/Report'; import type {ReportActionsCollectionDataSet} from '@src/types/onyx/ReportAction'; @@ -19505,6 +19506,36 @@ describe('ReportUtils', () => { expect(expenseReport.reportName).toBe(`${fakePolicy.name} owes ${convertToDisplayString(-total, currency)}`); }); + it('should leave report fields that failed to be created out of the report field list', async () => { + // Given a policy with a saved field and a field that failed to be created, which is still pending ADD with its error + const policyID = '3'; + const baseField = createMock({type: CONST.REPORT_FIELD_TYPES.TEXT, values: [], disabledOptions: [], defaultValue: ''}); + const savedField = {...baseField, fieldID: 'field_id_SAVED', name: 'Saved'}; + const failedField = {...baseField, fieldID: 'field_id_FAILED', name: 'Failed', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}; + const fakePolicy: Policy = { + ...createRandomPolicy(Number(policyID)), + fieldList: {expensify_field_id_SAVED: savedField, expensify_field_id_FAILED: failedField}, + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- ErrorFields doesn't model the nested fieldList..defaultValue shape the server uses + errorFields: {fieldList: {expensify_field_id_FAILED: {defaultValue: {1: 'Error'}}}} as unknown as ErrorFields, + }; + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, fakePolicy); + + // When an expense report is built before the error is dismissed + const expenseReport = buildOptimisticExpenseReport({ + rules: undefined, + chatReportID: undefined, + policyID, + payeeAccountID: 1, + total: 100, + currency: CONST.CURRENCY.USD, + isASAPSubmitBetaEnabled: true, + getCurrencyDecimals: getCurrencyDecimalsLocal, + }); + + // Then only the saved field is copied to the report, because the failed field doesn't exist on the server + expect(Object.keys(expenseReport.fieldList ?? {})).toEqual(['expensify_field_id_SAVED']); + }); + it('should keep the English workspace fallback in the report name when another locale is loaded', async () => { // Given a chat report that cannot be resolved, so the report name falls back to the "unavailable workspace" copy const chatReportID = '9999'; @@ -22936,6 +22967,100 @@ describe('ReportUtils', () => { ]); expect(getAvailableReportFields(report, policyFieldList)).toEqual(expectedFieldList); }); + + it('should skip policy fields that failed to be created', async () => { + // Given an unsettled report and a policy field that failed to be created, so it is still pending ADD on the policy with an error and was removed from the report + const report = createMock({ + reportID: '3', + policyID: '1', + fieldList: fieldList1, + }); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); + const failedPolicyField = {...fieldList2.expensify_field_id_LIST_POLICY, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- ErrorFields doesn't model the nested fieldList..defaultValue shape the server uses + const policyErrorFields = {fieldList: {expensify_field_id_LIST_POLICY: {defaultValue: {1: 'Error'}}}} as unknown as ErrorFields; + + // When getting the available report fields + const availableFields = getAvailableReportFields(report, [failedPolicyField], policyErrorFields); + + // Then the failed field is not shown on the report + expect(availableFields).toEqual(Object.values(fieldList1)); + }); + + it('should keep policy fields pending ADD without an error that are not on the report', async () => { + // Given a report created after a field was added offline, so the field is pending ADD on the policy, has no error, and was never written to this report + const report = createMock({ + reportID: 'reportCreatedAfterOfflineFieldAdd', + policyID: '1', + fieldList: {}, + }); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); + const pendingPolicyField = {...fieldList2.expensify_field_id_LIST_POLICY, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}; + + // When getting the available report fields + const availableFields = getAvailableReportFields(report, [pendingPolicyField], {}); + + // Then the pending field is still shown, because it hasn't failed + expect(availableFields.map(({fieldID}) => fieldID)).toEqual(['field_id_LIST_POLICY']); + }); + + it('should keep policy fields pending ADD that are already on the report', async () => { + // Given an unsettled report that has an optimistically added field, which is also pending ADD on the policy + const report = createMock({ + reportID: '4', + policyID: '1', + fieldList: fieldList1, + }); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report); + const pendingPolicyField = {...fieldList1.expensify_field_id_LIST, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}; + + // When getting the available report fields + const availableFields = getAvailableReportFields(report, [pendingPolicyField]); + + // Then the optimistic field is still shown on the report + expect(availableFields.map(({fieldID}) => fieldID)).toEqual(['field_id_LIST']); + }); + }); + + describe('getReportFieldsWithoutFailedAdds', () => { + const baseField = createMock({type: 'text', values: [], disabledOptions: [], defaultValue: ''}); + const savedField = {...baseField, fieldID: 'field_id_SAVED', name: 'Saved'}; + const offlineField = {...baseField, fieldID: 'field_id_OFFLINE', name: 'Offline', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}; + const failedField = {...baseField, fieldID: 'field_id_FAILED', name: 'Failed', pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}; + const updateFailedField = {...baseField, fieldID: 'field_id_UPDATE_FAILED', name: 'Update failed'}; + const fieldList = { + expensify_field_id_SAVED: savedField, + expensify_field_id_OFFLINE: offlineField, + expensify_field_id_FAILED: failedField, + expensify_field_id_UPDATE_FAILED: updateFailedField, + }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- ErrorFields doesn't model the nested fieldList..defaultValue shape the server uses + const policyErrorFields = { + fieldList: { + expensify_field_id_FAILED: {defaultValue: {1: 'Error'}}, + expensify_field_id_UPDATE_FAILED: {defaultValue: {2: 'Error'}}, + }, + } as unknown as ErrorFields; + + it('should leave out only the fields that failed to be created', () => { + // Given a policy with a saved field, a field added offline, a field whose creation failed, and an existing field whose update failed + + // When removing the failed adds + const fields = getReportFieldsWithoutFailedAdds(fieldList, policyErrorFields); + + // Then only the field whose creation failed is left out, because it doesn't exist on the server. The others do, or are still being created. + expect(Object.keys(fields)).toEqual(['expensify_field_id_SAVED', 'expensify_field_id_OFFLINE', 'expensify_field_id_UPDATE_FAILED']); + }); + + it('should keep every field when the policy has no errors', () => { + // Given a policy without report field errors + + // When removing the failed adds + const fields = getReportFieldsWithoutFailedAdds(fieldList, undefined); + + // Then no field is left out, because a pending ADD without an error hasn't failed + expect(fields).toEqual(fieldList); + }); }); describe('getReportFieldMaps', () => {