From 1d650c1b79462e0559e2f7063757179f3962f728 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 7 Aug 2024 11:33:31 +0200 Subject: [PATCH 1/2] integrate pattern a --- src/libs/actions/Policy/ReportField.ts | 55 +++---------------- .../ReportFieldsListValuesPage.tsx | 3 +- tests/actions/ReportFieldTest.ts | 27 ++++----- 3 files changed, 19 insertions(+), 66 deletions(-) diff --git a/src/libs/actions/Policy/ReportField.ts b/src/libs/actions/Policy/ReportField.ts index 9217519035a2..6cf762527b2f 100644 --- a/src/libs/actions/Policy/ReportField.ts +++ b/src/libs/actions/Policy/ReportField.ts @@ -397,19 +397,7 @@ function updateReportFieldListValueEnabled(policyID: string, reportFieldID: stri onyxMethod: Onyx.METHOD.MERGE, value: { fieldList: { - [fieldKey]: {...updatedReportField, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}, - }, - errorFields: null, - }, - }, - ], - successData: [ - { - key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - onyxMethod: Onyx.METHOD.MERGE, - value: { - fieldList: { - [fieldKey]: {pendingAction: null}, + [fieldKey]: updatedReportField, }, errorFields: null, }, @@ -421,7 +409,7 @@ function updateReportFieldListValueEnabled(policyID: string, reportFieldID: stri onyxMethod: Onyx.METHOD.MERGE, value: { fieldList: { - [fieldKey]: {...reportField, pendingAction: null}, + [fieldKey]: reportField, }, errorFields: { [fieldKey]: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.reportFields.genericFailureMessage'), @@ -430,6 +418,7 @@ function updateReportFieldListValueEnabled(policyID: string, reportFieldID: stri }, ], }; + const parameters: EnableWorkspaceReportFieldListValueParams = { policyID, reportFields: JSON.stringify([updatedReportField]), @@ -457,22 +446,7 @@ function addReportFieldListValue(policyID: string, reportFieldID: string, valueN onyxMethod: Onyx.METHOD.MERGE, value: { fieldList: { - [reportFieldKey]: { - ...updatedReportField, - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, - }, - }, - errorFields: null, - }, - }, - ], - successData: [ - { - key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - onyxMethod: Onyx.METHOD.MERGE, - value: { - fieldList: { - [reportFieldKey]: {pendingAction: null}, + [reportFieldKey]: updatedReportField, }, errorFields: null, }, @@ -484,7 +458,7 @@ function addReportFieldListValue(policyID: string, reportFieldID: string, valueN onyxMethod: Onyx.METHOD.MERGE, value: { fieldList: { - [reportFieldKey]: {...reportField, pendingAction: null}, + [reportFieldKey]: reportField, }, errorFields: { [reportFieldKey]: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.reportFields.genericFailureMessage'), @@ -531,22 +505,7 @@ function removeReportFieldListValue(policyID: string, reportFieldID: string, val onyxMethod: Onyx.METHOD.MERGE, value: { fieldList: { - [reportFieldKey]: { - ...updatedReportField, - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, - }, - }, - errorFields: null, - }, - }, - ], - successData: [ - { - key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - onyxMethod: Onyx.METHOD.MERGE, - value: { - fieldList: { - [reportFieldKey]: {pendingAction: null}, + [reportFieldKey]: updatedReportField, }, errorFields: null, }, @@ -558,7 +517,7 @@ function removeReportFieldListValue(policyID: string, reportFieldID: string, val onyxMethod: Onyx.METHOD.MERGE, value: { fieldList: { - [reportFieldKey]: {...reportField, pendingAction: null}, + [reportFieldKey]: reportField, }, errorFields: { [reportFieldKey]: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.reportFields.genericFailureMessage'), diff --git a/src/pages/workspace/reportFields/ReportFieldsListValuesPage.tsx b/src/pages/workspace/reportFields/ReportFieldsListValuesPage.tsx index ac78f34ed1fb..fc0ca771cf63 100644 --- a/src/pages/workspace/reportFields/ReportFieldsListValuesPage.tsx +++ b/src/pages/workspace/reportFields/ReportFieldsListValuesPage.tsx @@ -94,7 +94,6 @@ function ReportFieldsListValuesPage({ keyForList: value, isSelected: selectedValues[value], enabled: !disabledListValues[index] ?? true, - pendingAction: reportFieldID ? policy?.fieldList?.[ReportUtils.getReportFieldKey(reportFieldID)]?.pendingAction : null, rightElement: ( localeCompare(a.value, b.value)); return [{data, isDisabled: false}]; - }, [disabledListValues, listValues, policy?.fieldList, reportFieldID, selectedValues, translate]); + }, [disabledListValues, listValues, selectedValues, translate]); const shouldShowEmptyState = Object.values(listValues ?? {}).length <= 0; const selectedValuesArray = Object.keys(selectedValues).filter((key) => selectedValues[key]); diff --git a/tests/actions/ReportFieldTest.ts b/tests/actions/ReportFieldTest.ts index 4676d187a211..5f192f5281a5 100644 --- a/tests/actions/ReportFieldTest.ts +++ b/tests/actions/ReportFieldTest.ts @@ -17,10 +17,11 @@ import * as TestHelper from '../utils/TestHelper'; import type {MockFetch} from '../utils/TestHelper'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +type PolicyReportFieldWithOfflineFeedback = Record>; +type PolicyReportFieldWithoutOfflineFeedback = Record; + OnyxUpdateManager(); describe('actions/ReportField', () => { - type PolicyReportFieldWithOfflineFeedback = Record>; - function connectToFetchPolicy(policyID: string): Promise> { return new Promise((resolve) => { const connectionID = Onyx.connect({ @@ -440,12 +441,11 @@ describe('actions/ReportField', () => { let policy: OnyxEntry = await connectToFetchPolicy(policyID); // check if the new report field was added to the policy - expect(policy?.fieldList).toStrictEqual({ + expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: { ...reportField, defaultValue: '', disabledOptions: [false, true, true], - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, }, }); @@ -492,12 +492,11 @@ describe('actions/ReportField', () => { let policy: OnyxEntry = await connectToFetchPolicy(policyID); // check if the new report field was added to the policy - expect(policy?.fieldList).toStrictEqual({ + expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: { ...reportField, defaultValue: '', disabledOptions: [false, true, true], - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, }, }); @@ -509,7 +508,7 @@ describe('actions/ReportField', () => { policy = await connectToFetchPolicy(policyID); // check if the updated report field was reset in the policy - expect(policy?.fieldList).toStrictEqual({ + expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: reportField, }); // Check if the policy errors was set @@ -550,12 +549,11 @@ describe('actions/ReportField', () => { let policy: OnyxEntry = await connectToFetchPolicy(policyID); // check if the new report field was added to the policy - expect(policy?.fieldList).toStrictEqual({ + expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: { ...reportField, values: [...reportField.values, newListValueName], disabledOptions: [...reportField.disabledOptions, false], - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, }, }); @@ -602,12 +600,11 @@ describe('actions/ReportField', () => { let policy: OnyxEntry = await connectToFetchPolicy(policyID); // check if the new report field was added to the policy - expect(policy?.fieldList).toStrictEqual({ + expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: { ...reportField, values: [...reportField.values, newListValueName], disabledOptions: [...reportField.disabledOptions, false], - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, }, }); @@ -619,7 +616,7 @@ describe('actions/ReportField', () => { policy = await connectToFetchPolicy(policyID); // check if the updated report field was reset in the policy - expect(policy?.fieldList).toStrictEqual({ + expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: reportField, }); }); @@ -658,13 +655,12 @@ describe('actions/ReportField', () => { let policy: OnyxEntry = await connectToFetchPolicy(policyID); // Check if the values were removed from the report field - expect(policy?.fieldList).toStrictEqual({ + expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: { ...reportField, defaultValue: '', values: ['Value 1'], disabledOptions: [false], - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, }, }); @@ -715,7 +711,6 @@ describe('actions/ReportField', () => { defaultValue: '', values: ['Value 1'], disabledOptions: [false], - pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, }, }); @@ -727,7 +722,7 @@ describe('actions/ReportField', () => { policy = await connectToFetchPolicy(policyID); // check if the updated report field was reset in the policy - expect(policy?.fieldList).toStrictEqual({ + expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: reportField, }); // Check if the policy errors was set From 9ff01cc2e63754ce5294281ea3f3cabdb79dc78e Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 7 Aug 2024 13:47:44 +0200 Subject: [PATCH 2/2] integrate pattern a properly --- src/libs/actions/Policy/ReportField.ts | 48 +----- tests/actions/ReportFieldTest.ts | 204 +------------------------ 2 files changed, 9 insertions(+), 243 deletions(-) diff --git a/src/libs/actions/Policy/ReportField.ts b/src/libs/actions/Policy/ReportField.ts index 6cf762527b2f..9a56c719fd73 100644 --- a/src/libs/actions/Policy/ReportField.ts +++ b/src/libs/actions/Policy/ReportField.ts @@ -390,6 +390,7 @@ function updateReportFieldListValueEnabled(policyID: string, reportFieldID: stri } }); + // We are using the offline pattern A (optimistic without feedback) const onyxData: OnyxData = { optimisticData: [ { @@ -399,21 +400,6 @@ function updateReportFieldListValueEnabled(policyID: string, reportFieldID: stri fieldList: { [fieldKey]: updatedReportField, }, - errorFields: null, - }, - }, - ], - failureData: [ - { - key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - onyxMethod: Onyx.METHOD.MERGE, - value: { - fieldList: { - [fieldKey]: reportField, - }, - errorFields: { - [fieldKey]: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.reportFields.genericFailureMessage'), - }, }, }, ], @@ -439,6 +425,7 @@ function addReportFieldListValue(policyID: string, reportFieldID: string, valueN updatedReportField.values.push(valueName); updatedReportField.disabledOptions.push(false); + // We are using the offline pattern A (optimistic without feedback) const onyxData: OnyxData = { optimisticData: [ { @@ -448,21 +435,6 @@ function addReportFieldListValue(policyID: string, reportFieldID: string, valueN fieldList: { [reportFieldKey]: updatedReportField, }, - errorFields: null, - }, - }, - ], - failureData: [ - { - key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - onyxMethod: Onyx.METHOD.MERGE, - value: { - fieldList: { - [reportFieldKey]: reportField, - }, - errorFields: { - [reportFieldKey]: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.reportFields.genericFailureMessage'), - }, }, }, ], @@ -498,6 +470,7 @@ function removeReportFieldListValue(policyID: string, reportFieldID: string, val updatedReportField.disabledOptions.splice(valueIndex, 1); }); + // We are using the offline pattern A (optimistic without feedback) const onyxData: OnyxData = { optimisticData: [ { @@ -507,21 +480,6 @@ function removeReportFieldListValue(policyID: string, reportFieldID: string, val fieldList: { [reportFieldKey]: updatedReportField, }, - errorFields: null, - }, - }, - ], - failureData: [ - { - key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, - onyxMethod: Onyx.METHOD.MERGE, - value: { - fieldList: { - [reportFieldKey]: reportField, - }, - errorFields: { - [reportFieldKey]: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.reportFields.genericFailureMessage'), - }, }, }, ], diff --git a/tests/actions/ReportFieldTest.ts b/tests/actions/ReportFieldTest.ts index 5f192f5281a5..13134e7e20fe 100644 --- a/tests/actions/ReportFieldTest.ts +++ b/tests/actions/ReportFieldTest.ts @@ -438,60 +438,9 @@ describe('actions/ReportField', () => { ReportField.updateReportFieldListValueEnabled(policyID, reportFieldID, valueIndexesTpUpdate, false); await waitForBatchedUpdates(); - let policy: OnyxEntry = await connectToFetchPolicy(policyID); - - // check if the new report field was added to the policy - expect(policy?.fieldList).toStrictEqual({ - [reportFieldKey]: { - ...reportField, - defaultValue: '', - disabledOptions: [false, true, true], - }, - }); - - // Check for success data - mockFetch.resume(); - await waitForBatchedUpdates(); - - policy = await connectToFetchPolicy(policyID); - - // Check if the policy pending action was cleared - expect(policy?.fieldList?.[reportFieldKey]?.pendingAction).toBeFalsy(); - }); - - it('updates the enabled flag of a report field list value when api returns an error', async () => { - mockFetch.pause(); - - const policyID = Policy.generatePolicyID(); - const reportFieldName = 'Test Field'; - const valueIndexesToUpdate = [1]; - const reportFieldID = generateFieldID(reportFieldName); - const reportFieldKey = ReportUtils.getReportFieldKey(reportFieldID); - const reportField: PolicyReportField = { - name: reportFieldName, - type: CONST.REPORT_FIELD_TYPES.LIST, - defaultValue: 'Value 2', - values: ['Value 1', 'Value 2', 'Value 3'], - disabledOptions: [false, false, true], - fieldID: reportFieldID, - orderWeight: 1, - deletable: false, - keys: [], - externalIDs: [], - isTax: false, - value: CONST.REPORT_FIELD_TYPES.LIST, - }; - const fakePolicy = createRandomPolicy(Number(policyID)); - - Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {...fakePolicy, fieldList: {[reportFieldKey]: reportField}}); - await waitForBatchedUpdates(); - - ReportField.updateReportFieldListValueEnabled(policyID, reportFieldID, valueIndexesToUpdate, false); - await waitForBatchedUpdates(); - - let policy: OnyxEntry = await connectToFetchPolicy(policyID); + const policy: OnyxEntry = await connectToFetchPolicy(policyID); - // check if the new report field was added to the policy + // check if the new report field was added to the policy optimistically expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: { ...reportField, @@ -499,20 +448,6 @@ describe('actions/ReportField', () => { disabledOptions: [false, true, true], }, }); - - // Check for failure data - mockFetch.fail(); - mockFetch.resume(); - await waitForBatchedUpdates(); - - policy = await connectToFetchPolicy(policyID); - - // check if the updated report field was reset in the policy - expect(policy?.fieldList).toStrictEqual({ - [reportFieldKey]: reportField, - }); - // Check if the policy errors was set - expect(policy?.errorFields?.[reportFieldKey]).toBeTruthy(); }); }); @@ -547,8 +482,8 @@ describe('actions/ReportField', () => { ReportField.addReportFieldListValue(policyID, reportFieldID, newListValueName); await waitForBatchedUpdates(); - let policy: OnyxEntry = await connectToFetchPolicy(policyID); - // check if the new report field was added to the policy + const policy: OnyxEntry = await connectToFetchPolicy(policyID); + // Check if the new report field was added to the policy optimistically expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: { ...reportField, @@ -556,69 +491,6 @@ describe('actions/ReportField', () => { disabledOptions: [...reportField.disabledOptions, false], }, }); - - // Check for success data - mockFetch.resume(); - await waitForBatchedUpdates(); - - policy = await connectToFetchPolicy(policyID); - - // Check if the policy pending action was cleared - expect(policy?.fieldList?.[reportFieldKey]?.pendingAction).toBeFalsy(); - }); - - it('adds a new value to a report field list when api returns an error', async () => { - mockFetch.pause(); - - const policyID = Policy.generatePolicyID(); - const reportFieldName = 'Test Field'; - const reportFieldID = generateFieldID(reportFieldName); - const reportFieldKey = ReportUtils.getReportFieldKey(reportFieldID); - const reportField: PolicyReportField = { - name: reportFieldName, - type: CONST.REPORT_FIELD_TYPES.LIST, - defaultValue: 'Value 2', - values: ['Value 1', 'Value 2', 'Value 3'], - disabledOptions: [false, false, true], - fieldID: reportFieldID, - orderWeight: 1, - deletable: false, - keys: [], - externalIDs: [], - isTax: false, - value: CONST.REPORT_FIELD_TYPES.LIST, - }; - const fakePolicy = createRandomPolicy(Number(policyID)); - const newListValueName = 'Value 4'; - - Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {...fakePolicy, fieldList: {[reportFieldKey]: reportField}}); - await waitForBatchedUpdates(); - - ReportField.addReportFieldListValue(policyID, reportFieldID, newListValueName); - await waitForBatchedUpdates(); - - let policy: OnyxEntry = await connectToFetchPolicy(policyID); - - // check if the new report field was added to the policy - expect(policy?.fieldList).toStrictEqual({ - [reportFieldKey]: { - ...reportField, - values: [...reportField.values, newListValueName], - disabledOptions: [...reportField.disabledOptions, false], - }, - }); - - // Check for failure data - mockFetch.fail(); - mockFetch.resume(); - await waitForBatchedUpdates(); - - policy = await connectToFetchPolicy(policyID); - - // check if the updated report field was reset in the policy - expect(policy?.fieldList).toStrictEqual({ - [reportFieldKey]: reportField, - }); }); }); @@ -652,9 +524,9 @@ describe('actions/ReportField', () => { ReportField.removeReportFieldListValue(policyID, reportFieldID, [1, 2]); await waitForBatchedUpdates(); - let policy: OnyxEntry = await connectToFetchPolicy(policyID); + const policy: OnyxEntry = await connectToFetchPolicy(policyID); - // Check if the values were removed from the report field + // Check if the values were removed from the report field optimistically expect(policy?.fieldList).toStrictEqual({ [reportFieldKey]: { ...reportField, @@ -663,70 +535,6 @@ describe('actions/ReportField', () => { disabledOptions: [false], }, }); - - // Check for success data - mockFetch.resume(); - await waitForBatchedUpdates(); - - policy = await connectToFetchPolicy(policyID); - // Check if the policy pending action was cleared - expect(policy?.fieldList?.[reportFieldKey]?.pendingAction).toBeFalsy(); - }); - - it('removes list values from a report field list when api returns an error', async () => { - mockFetch.pause(); - - const policyID = Policy.generatePolicyID(); - const reportFieldName = 'Test Field'; - const reportFieldID = generateFieldID(reportFieldName); - const reportFieldKey = ReportUtils.getReportFieldKey(reportFieldID); - const reportField: PolicyReportField = { - name: reportFieldName, - type: CONST.REPORT_FIELD_TYPES.LIST, - defaultValue: 'Value 2', - values: ['Value 1', 'Value 2', 'Value 3'], - disabledOptions: [false, false, true], - fieldID: reportFieldID, - orderWeight: 1, - deletable: false, - keys: [], - externalIDs: [], - isTax: false, - value: CONST.REPORT_FIELD_TYPES.LIST, - }; - const fakePolicy = createRandomPolicy(Number(policyID)); - - Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {...fakePolicy, fieldList: {[reportFieldKey]: reportField}}); - await waitForBatchedUpdates(); - - ReportField.removeReportFieldListValue(policyID, reportFieldID, [1, 2]); - await waitForBatchedUpdates(); - - let policy: OnyxEntry = await connectToFetchPolicy(policyID); - - // Check if the values were removed from the report field - expect(policy?.fieldList).toStrictEqual({ - [reportFieldKey]: { - ...reportField, - defaultValue: '', - values: ['Value 1'], - disabledOptions: [false], - }, - }); - - // Check for failure data - mockFetch.fail(); - mockFetch.resume(); - await waitForBatchedUpdates(); - - policy = await connectToFetchPolicy(policyID); - - // check if the updated report field was reset in the policy - expect(policy?.fieldList).toStrictEqual({ - [reportFieldKey]: reportField, - }); - // Check if the policy errors was set - expect(policy?.errorFields?.[reportFieldKey]).toBeTruthy(); }); }); });