Merge pull request #89867 from wildan-m/wildan/89865-restore-non-default-distance-rates

Valforte · web-flow · commit 7b2887de5512 · 2026-05-07T10:35:22.000-03:00
Fix Distance expense - Undefault distance rate is not displayed when duplicate wp offline
diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts
@@ -298,21 +298,41 @@ function hasEligibleActiveAdminFromWorkspaces(policies: OnyxCollection<Policy> |
     return false;
 }
 
+/**
+ * Pick the rate that the expense flow will treat as default for a distance custom unit's rates
+ * dictionary: filter enabled rates, sort by `index` (treating missing index as CONST.DEFAULT_NUMBER_ID),
+ * and take the first. Mirrors `getDefaultMileageRate`'s selection so optimistic state stays aligned
+ * with the rate the expense flow later picks.
+ */
+function getDefaultDistanceRate(rates: Record<string, Rate> | undefined): Rate | undefined {
+    if (!rates) {
+        return undefined;
+    }
+    return Object.values(rates)
+        .filter((rate) => rate.enabled !== false)
+        .sort((a, b) => (a.index ?? CONST.DEFAULT_NUMBER_ID) - (b.index ?? CONST.DEFAULT_NUMBER_ID))
+        .at(0);
+}
+
 function cloneCustomUnitWithNewIDs(unit: CustomUnit, newCustomUnitID: string, newDefaultRateID?: string): CustomUnit {
     if (newDefaultRateID) {
-        // The server-side DUPLICATE_POLICY assigns newDefaultRateID to the source's default rate.
-        // Mirror getDefaultMileageRate's selection (enabled rates, sorted by index with
-        // CONST.DEFAULT_NUMBER_ID for missing indexes) so the optimistic clone aligns with the
-        // rate the expense flow will later treat as default. Other source rates get fresh server
-        // IDs, so we drop them from the optimistic state to avoid stale duplicates.
-        const defaultRate = Object.values(unit.rates)
-            .filter((rate) => rate.enabled !== false)
-            .sort((a, b) => (a.index ?? CONST.DEFAULT_NUMBER_ID) - (b.index ?? CONST.DEFAULT_NUMBER_ID))
-            .at(0);
+        // Mirror DUPLICATE_POLICY: rebind the source default to newDefaultRateID, keep other rates
+        // under their source IDs (the server preserves them). Preserve source iteration order so
+        // getDefaultMileageRate's stable sort still picks the original default when rates tie on
+        // index (e.g. a rate with no index falls back to CONST.DEFAULT_NUMBER_ID).
+        const defaultRate = getDefaultDistanceRate(unit.rates);
+        const rates: Record<string, Rate> = {};
+        for (const rate of Object.values(unit.rates)) {
+            if (rate.customUnitRateID === defaultRate?.customUnitRateID) {
+                rates[newDefaultRateID] = {...rate, customUnitRateID: newDefaultRateID};
+            } else {
+                rates[rate.customUnitRateID] = rate;
+            }
+        }
         return {
             ...unit,
             customUnitID: newCustomUnitID,
-            rates: defaultRate ? {[newDefaultRateID]: {...defaultRate, customUnitRateID: newDefaultRateID}} : {},
+            rates,
         };
     }
 
@@ -2268,6 +2288,7 @@ export {
     getManagerAccountID,
     isPreferredExporter,
     getCustomUnitsForDuplication,
+    getDefaultDistanceRate,
     getCountOfRequiredTagLists,
     getActiveEmployeeWorkspaces,
     getPolicyRole,
diff --git a/tests/actions/PolicyTest.ts b/tests/actions/PolicyTest.ts
@@ -773,6 +773,107 @@ describe('actions/Policy', () => {
             expect(messages?.at(0)?.text).toBe(callerEmail);
         });
 
+        it('duplicate workspace with distance rates clones all source rates in optimistic data with the default rebound to the API-known rate ID', async () => {
+            await Onyx.set(ONYXKEYS.SESSION, {email: ESH_EMAIL, accountID: ESH_ACCOUNT_ID});
+
+            const sourceDistanceUnitID = 'srcDistUnit';
+            const sourceDefaultRateID = 'srcDefaultRate';
+            const sourceExtraRateID = 'srcExtraRate';
+            const fakePolicy: PolicyType = {
+                ...createRandomPolicy(19, CONST.POLICY.TYPE.TEAM),
+                customUnits: {
+                    [sourceDistanceUnitID]: {
+                        customUnitID: sourceDistanceUnitID,
+                        name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
+                        enabled: true,
+                        attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES},
+                        rates: {
+                            [sourceDefaultRateID]: {customUnitRateID: sourceDefaultRateID, name: 'Default Rate', rate: 72.5, currency: 'USD', enabled: true, index: 0},
+                            [sourceExtraRateID]: {customUnitRateID: sourceExtraRateID, name: 'Extra Rate', rate: 100, currency: 'USD', enabled: true, index: 1},
+                        },
+                    },
+                },
+            };
+            await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy);
+            await waitForBatchedUpdates();
+
+            const policyID = Policy.generatePolicyID();
+            const options = {
+                currentUserAccountID: ESH_ACCOUNT_ID,
+                currentUserEmail: ESH_EMAIL,
+                policyName: 'Distance Rates Duplicate',
+                policyID: fakePolicy.id,
+                targetPolicyID: policyID,
+                welcomeNote: 'Join my policy',
+                parts: {
+                    people: false,
+                    reports: false,
+                    connections: false,
+                    categories: false,
+                    tags: false,
+                    taxes: false,
+                    perDiem: false,
+                    reimbursements: false,
+                    expenses: false,
+                    distance: true,
+                    invoices: false,
+                    exportLayouts: false,
+                },
+                localCurrency: 'USD',
+            };
+
+            // Pause the API call so successData (which clears the optimistic source rate IDs) does not
+            // run yet — this lets us inspect the optimistic state the user sees offline.
+            mockFetch.pause();
+            Policy.duplicateWorkspace(fakePolicy, options);
+            await waitForBatchedUpdates();
+
+            const duplicatePolicy: OnyxEntry<PolicyType> = await new Promise((resolve) => {
+                const connection = Onyx.connect({
+                    key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
+                    callback: (workspace) => {
+                        Onyx.disconnect(connection);
+                        resolve(workspace);
+                    },
+                });
+            });
+
+            expect(duplicatePolicy?.areDistanceRatesEnabled).toBe(true);
+            const distanceUnit = Object.values(duplicatePolicy?.customUnits ?? {}).find((unit) => unit.name === CONST.CUSTOM_UNITS.NAME_DISTANCE);
+            if (!distanceUnit) {
+                throw new Error('Expected duplicated distance unit');
+            }
+            // The duplicated unit's customUnitID should differ from the source's (fresh ID).
+            expect(distanceUnit.customUnitID).not.toBe(sourceDistanceUnitID);
+            // Both rates should be present in optimistic data: the extra rate keeps its source ID,
+            // and the default is rebound to a fresh API-known customUnitRateID.
+            const rateKeys = Object.keys(distanceUnit.rates);
+            expect(rateKeys).toContain(sourceExtraRateID);
+            expect(rateKeys).not.toContain(sourceDefaultRateID);
+            expect(rateKeys).toHaveLength(2);
+            // The Default Rate should be the one picked by getDefaultDistanceRate (lowest enabled index).
+            const defaultRate = Object.values(distanceUnit.rates)
+                .filter((rate) => rate.enabled !== false)
+                .sort((a, b) => (a.index ?? CONST.DEFAULT_NUMBER_ID) - (b.index ?? CONST.DEFAULT_NUMBER_ID))
+                .at(0);
+            expect(defaultRate?.name).toBe('Default Rate');
+            expect(defaultRate?.rate).toBe(72.5);
+            // The default rate's customUnitRateID matches its dictionary key (rebound).
+            expect(defaultRate?.customUnitRateID).not.toBe(sourceDefaultRateID);
+            const defaultRateID = defaultRate?.customUnitRateID;
+            if (!defaultRateID) {
+                throw new Error('Expected default rate to have a customUnitRateID');
+            }
+            expect(distanceUnit.rates[defaultRateID]).toBe(defaultRate);
+
+            // Resume the mocked fetch so the API call resolves; the duplicated workspace's optimistic
+            // rates remain visible until a Pusher response (not mocked here) repopulates them with
+            // the server's rate set. We rely on the server preserving the source's non-default rate
+            // IDs in the duplicate, so the optimistic state and the eventual server state line up.
+            await mockFetch.resume?.();
+            await waitForBatchedUpdates();
+        });
+
         it('creates a new workspace with BASIC approval mode if the introSelected is MANAGE_TEAM', async () => {
             const policyID = Policy.generatePolicyID();
             // When a new workspace is created with introSelected set to MANAGE_TEAM
diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts
@@ -352,7 +352,7 @@ describe('PolicyUtils', () => {
             ).toBeUndefined();
         });
 
-        it('clones the source default rate (lowest enabled index) under the API-known customUnitRateID', () => {
+        it('rebinds the default rate to the API-known customUnitRateID and keeps non-default rates with their source IDs', () => {
             const distanceUnitWithMultipleRates = {
                 customUnitID: 'srcDist',
                 name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
@@ -377,13 +377,14 @@ describe('PolicyUtils', () => {
                     ...distanceUnitWithMultipleRates,
                     customUnitID: 'newDist',
                     rates: {
+                        rateB: {customUnitRateID: 'rateB', name: 'New Rate 1', rate: 100, currency: 'USD', enabled: true, index: 1, attributes: {taxRateExternalID: 'tax_other'}},
                         newRate: {customUnitRateID: 'newRate', name: 'Default Rate', rate: 70, currency: 'USD', enabled: true, index: 0, attributes: {taxRateExternalID: 'tax_default'}},
                     },
                 },
             });
         });
 
-        it('drops all rates when no enabled rate exists', () => {
+        it('keeps source rates with their source IDs when no enabled rate exists', () => {
             const distanceUnitAllDisabled = {
                 customUnitID: 'srcDist',
                 name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
@@ -402,7 +403,9 @@ describe('PolicyUtils', () => {
                 perDiemCustomUnitID: 'newPerDiem',
                 customUnitRateID: 'newRate',
             });
-            expect(result?.newDist.rates).toEqual({});
+            expect(result?.newDist.rates).toEqual({
+                rateA: {customUnitRateID: 'rateA', name: 'Disabled', rate: 50, currency: 'USD', enabled: false, index: 0},
+            });
         });
 
         it('treats missing index as 0 when picking the default rate', () => {
@@ -426,9 +429,55 @@ describe('PolicyUtils', () => {
                 customUnitRateID: 'newRate',
             });
             expect(result?.newDist.rates).toEqual({
+                rateB: {customUnitRateID: 'rateB', name: 'Indexed Rate', rate: 100, currency: 'USD', enabled: true, index: 1},
                 newRate: {customUnitRateID: 'newRate', name: 'No-Index Rate', rate: 70, currency: 'USD', enabled: true},
             });
         });
+
+        it('preserves source iteration order so getDefaultMileageRate still picks the original default when a non-default rate has a missing index', () => {
+            // Source has 3 rates: Default (index 0), Indexed (index 1), and No-Index (no index field).
+            // No-Index ties with Default at the index-0 sort key after `?? CONST.DEFAULT_NUMBER_ID`.
+            // The optimistic clone must keep the source's iteration order so JavaScript's stable
+            // sort still places Default before No-Index — otherwise getDefaultMileageRate would
+            // pick whichever tied rate appeared first in iteration order, swapping the default.
+            const sourceUnit = {
+                customUnitID: 'srcDist',
+                name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
+                enabled: true,
+                attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES},
+                rates: {
+                    rateA: {customUnitRateID: 'rateA', name: 'Default Rate', rate: 72.5, currency: 'USD', enabled: true, index: 0},
+                    rateB: {customUnitRateID: 'rateB', name: 'Indexed Rate', rate: 100, currency: 'USD', enabled: true, index: 1},
+                    rateC: {customUnitRateID: 'rateC', name: 'No-Index Rate', rate: 200, currency: 'USD', enabled: true},
+                },
+            };
+            const policyWithTiedIndex: Policy = {
+                ...createRandomPolicy(0),
+                customUnits: {[sourceUnit.customUnitID]: sourceUnit},
+            };
+            const result = getCustomUnitsForDuplication(policyWithTiedIndex, true, false, {
+                distanceCustomUnitID: 'newDist',
+                perDiemCustomUnitID: 'newPerDiem',
+                customUnitRateID: 'newRate',
+            });
+
+            const cloned = result?.newDist;
+            if (!cloned) {
+                throw new Error('Expected cloned distance unit');
+            }
+            // Iteration order: the rebound default should still be first, before the No-Index rate.
+            const orderedKeys = Object.keys(cloned.rates);
+            expect(orderedKeys).toEqual(['newRate', 'rateB', 'rateC']);
+
+            // Sanity-check that getDefaultMileageRate's selection (enabled, sorted by index ?? 0,
+            // first) lands on the rebound default and not on No-Index Rate.
+            const pickedDefault = Object.values(cloned.rates)
+                .filter((rate) => rate.enabled !== false)
+                .sort((a, b) => (a.index ?? CONST.DEFAULT_NUMBER_ID) - (b.index ?? CONST.DEFAULT_NUMBER_ID))
+                .at(0);
+            expect(pickedDefault?.customUnitRateID).toBe('newRate');
+            expect(pickedDefault?.name).toBe('Default Rate');
+        });
     });
     describe('getRateDisplayValue', () => {
         it('should return an empty string for NaN', () => {
