Skip to content

Commit 3ce4e8b

Browse files
committed
Pick source's default distance rate by index when cloning for duplication
The server-side DUPLICATE_POLICY assigns the API-known customUnitRateID to the source's default rate (lowest index, matching getDefaultMileageRate). Previously the optimistic clone picked the first enabled rate by Object.values iteration order, which can differ from index order when rates are reordered or returned in non-index order. The mismatch caused offline-created distance expenses to silently change rate (and tax) once the server response merged back into Onyx. Sort by index ascending, pick the first enabled, and bind the API-known customUnitRateID to that rate so optimistic and server data align on the same key.
1 parent 5f10010 commit 3ce4e8b

2 files changed

Lines changed: 53 additions & 4 deletions

File tree

src/libs/PolicyUtils.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,17 +309,19 @@ function hasEligibleActiveAdminFromWorkspaces(policies: OnyxCollection<Policy> |
309309

310310
function cloneCustomUnitWithNewIDs(unit: CustomUnit, newCustomUnitID: string, newDefaultRateID?: string): CustomUnit {
311311
if (newDefaultRateID) {
312-
// For distance units: use the provided rate ID for the default rate,
313-
// matching the customUnitRateID sent to the DUPLICATE_POLICY API
314-
const defaultRate = Object.values(unit.rates).find((rate) => rate.enabled) ?? Object.values(unit.rates).at(0);
312+
// The server-side DUPLICATE_POLICY assigns newDefaultRateID to the source's default rate
313+
// (lowest index, matching getDefaultMileageRate). Mirror that here so the optimistic data
314+
// doesn't get overwritten when the server response merges. Other source rates get fresh
315+
// server IDs, so we drop them from the optimistic state to avoid stale duplicates.
316+
const sortedRates = Object.values(unit.rates).sort((a, b) => (a.index ?? Number.MAX_SAFE_INTEGER) - (b.index ?? Number.MAX_SAFE_INTEGER));
317+
const defaultRate = sortedRates.find((rate) => rate.enabled !== false) ?? sortedRates.at(0);
315318
return {
316319
...unit,
317320
customUnitID: newCustomUnitID,
318321
rates: defaultRate ? {[newDefaultRateID]: {...defaultRate, customUnitRateID: newDefaultRateID}} : {},
319322
};
320323
}
321324

322-
// For other units (per diem): only update the customUnitID
323325
return {
324326
...unit,
325327
customUnitID: newCustomUnitID,

tests/unit/PolicyUtilsTest.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,53 @@ describe('PolicyUtils', () => {
354354
getCustomUnitsForDuplication(policyWithoutCustomUnits, true, true, {distanceCustomUnitID: otherUnit.customUnitID, perDiemCustomUnitID: perDiemUnit.customUnitID}),
355355
).toBeUndefined();
356356
});
357+
358+
it('clones the source default rate (lowest enabled index) under the API-known customUnitRateID', () => {
359+
const distanceUnitWithMultipleRates = {
360+
customUnitID: 'srcDist',
361+
name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
362+
enabled: true,
363+
attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, taxEnabled: true},
364+
rates: {
365+
rateB: {customUnitRateID: 'rateB', name: 'New Rate 1', rate: 100, currency: 'USD', enabled: true, index: 1, attributes: {taxRateExternalID: 'tax_other'}},
366+
rateA: {customUnitRateID: 'rateA', name: 'Default Rate', rate: 70, currency: 'USD', enabled: true, index: 0, attributes: {taxRateExternalID: 'tax_default'}},
367+
},
368+
};
369+
const policyWithMultipleRates: Policy = {
370+
...createRandomPolicy(0),
371+
customUnits: {[distanceUnitWithMultipleRates.customUnitID]: distanceUnitWithMultipleRates},
372+
};
373+
const result = getCustomUnitsForDuplication(policyWithMultipleRates, true, false, {distanceCustomUnitID: 'newDist', perDiemCustomUnitID: 'newPerDiem', customUnitRateID: 'newRate'});
374+
expect(result).toEqual({
375+
newDist: {
376+
...distanceUnitWithMultipleRates,
377+
customUnitID: 'newDist',
378+
rates: {
379+
newRate: {customUnitRateID: 'newRate', name: 'Default Rate', rate: 70, currency: 'USD', enabled: true, index: 0, attributes: {taxRateExternalID: 'tax_default'}},
380+
},
381+
},
382+
});
383+
});
384+
385+
it('falls back to the first rate when no enabled rate exists', () => {
386+
const distanceUnitAllDisabled = {
387+
customUnitID: 'srcDist',
388+
name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
389+
enabled: true,
390+
attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES},
391+
rates: {
392+
rateA: {customUnitRateID: 'rateA', name: 'Disabled', rate: 50, currency: 'USD', enabled: false, index: 0},
393+
},
394+
};
395+
const policyAllDisabled: Policy = {
396+
...createRandomPolicy(0),
397+
customUnits: {[distanceUnitAllDisabled.customUnitID]: distanceUnitAllDisabled},
398+
};
399+
const result = getCustomUnitsForDuplication(policyAllDisabled, true, false, {distanceCustomUnitID: 'newDist', perDiemCustomUnitID: 'newPerDiem', customUnitRateID: 'newRate'});
400+
expect(result?.newDist.rates).toEqual({
401+
newRate: {customUnitRateID: 'newRate', name: 'Disabled', rate: 50, currency: 'USD', enabled: false, index: 0},
402+
});
403+
});
357404
});
358405
describe('getRateDisplayValue', () => {
359406
it('should return an empty string for NaN', () => {

0 commit comments

Comments
 (0)