Skip to content

Commit b90bf90

Browse files
committed
Preserve source iteration order in cloneCustomUnitWithNewIDs
The previous version inserted non-default rates first and the rebound default last. When a non-default source rate had no `index` field (treated as CONST.DEFAULT_NUMBER_ID = 0 by getDefaultMileageRate's sort), it tied with the default's index 0, and JavaScript's stable sort then placed whichever came first in iteration order. With non-defaults inserted first, the default fell behind — so the rate the expense flow treated as default in the duplicated workspace was the no-index rate, not the original default. Iterate source rates in their original order and just swap the default's key to newDefaultRateID. The default keeps its first-tied position in the rates dict, getDefaultMileageRate's stable sort behaves as it does on the source, and the original default is picked.
1 parent b52c6b5 commit b90bf90

2 files changed

Lines changed: 51 additions & 5 deletions

File tree

src/libs/PolicyUtils.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -306,19 +306,20 @@ function cloneCustomUnitWithNewIDs(unit: CustomUnit, newCustomUnitID: string, ne
306306
// rate the expense flow will later treat as default. Non-default rates keep their source
307307
// IDs so they remain visible offline; successData clears them after the API succeeds so
308308
// the server response can repopulate with fresh server IDs without leaving duplicates.
309+
// Iteration order is preserved from the source (default's key swapped in place) so
310+
// getDefaultMileageRate's stable sort still picks the original default when other rates
311+
// have a missing index (which would otherwise tie at CONST.DEFAULT_NUMBER_ID).
309312
const defaultRate = Object.values(unit.rates)
310313
.filter((rate) => rate.enabled !== false)
311314
.sort((a, b) => (a.index ?? CONST.DEFAULT_NUMBER_ID) - (b.index ?? CONST.DEFAULT_NUMBER_ID))
312315
.at(0);
313316
const rates: Record<string, Rate> = {};
314317
for (const rate of Object.values(unit.rates)) {
315318
if (rate.customUnitRateID === defaultRate?.customUnitRateID) {
316-
continue;
319+
rates[newDefaultRateID] = {...rate, customUnitRateID: newDefaultRateID};
320+
} else {
321+
rates[rate.customUnitRateID] = rate;
317322
}
318-
rates[rate.customUnitRateID] = rate;
319-
}
320-
if (defaultRate) {
321-
rates[newDefaultRateID] = {...defaultRate, customUnitRateID: newDefaultRateID};
322323
}
323324
return {
324325
...unit,

tests/unit/PolicyUtilsTest.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,51 @@ describe('PolicyUtils', () => {
433433
newRate: {customUnitRateID: 'newRate', name: 'No-Index Rate', rate: 70, currency: 'USD', enabled: true},
434434
});
435435
});
436+
437+
it('preserves source iteration order so getDefaultMileageRate still picks the original default when a non-default rate has a missing index', () => {
438+
// Source has 3 rates: Default (index 0), Indexed (index 1), and No-Index (no index field).
439+
// No-Index ties with Default at the index-0 sort key after `?? CONST.DEFAULT_NUMBER_ID`.
440+
// The optimistic clone must keep the source's iteration order so JavaScript's stable
441+
// sort still places Default before No-Index — otherwise getDefaultMileageRate would
442+
// pick whichever tied rate appeared first in iteration order, swapping the default.
443+
const sourceUnit = {
444+
customUnitID: 'srcDist',
445+
name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
446+
enabled: true,
447+
attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES},
448+
rates: {
449+
rateA: {customUnitRateID: 'rateA', name: 'Default Rate', rate: 72.5, currency: 'USD', enabled: true, index: 0},
450+
rateB: {customUnitRateID: 'rateB', name: 'Indexed Rate', rate: 100, currency: 'USD', enabled: true, index: 1},
451+
rateC: {customUnitRateID: 'rateC', name: 'No-Index Rate', rate: 200, currency: 'USD', enabled: true},
452+
},
453+
};
454+
const policyWithTiedIndex: Policy = {
455+
...createRandomPolicy(0),
456+
customUnits: {[sourceUnit.customUnitID]: sourceUnit},
457+
};
458+
const result = getCustomUnitsForDuplication(policyWithTiedIndex, true, false, {
459+
distanceCustomUnitID: 'newDist',
460+
perDiemCustomUnitID: 'newPerDiem',
461+
customUnitRateID: 'newRate',
462+
});
463+
464+
const cloned = result?.newDist;
465+
if (!cloned) {
466+
throw new Error('Expected cloned distance unit');
467+
}
468+
// Iteration order: the rebound default should still be first, before the No-Index rate.
469+
const orderedKeys = Object.keys(cloned.rates);
470+
expect(orderedKeys).toEqual(['newRate', 'rateB', 'rateC']);
471+
472+
// Sanity-check that getDefaultMileageRate's selection (enabled, sorted by index ?? 0,
473+
// first) lands on the rebound default and not on No-Index Rate.
474+
const pickedDefault = Object.values(cloned.rates)
475+
.filter((rate) => rate.enabled !== false)
476+
.sort((a, b) => (a.index ?? CONST.DEFAULT_NUMBER_ID) - (b.index ?? CONST.DEFAULT_NUMBER_ID))
477+
.at(0);
478+
expect(pickedDefault?.customUnitRateID).toBe('newRate');
479+
expect(pickedDefault?.name).toBe('Default Rate');
480+
});
436481
});
437482
describe('getRateDisplayValue', () => {
438483
it('should return an empty string for NaN', () => {

0 commit comments

Comments
 (0)