Skip to content

Commit 2aca6a2

Browse files
ValforteValforte
authored
Merge pull request #89448 from wildan-m/wildan/86116-fix-duplicate-workspace-distance-custom-unit
Fix Expense - In duplicate workspace, creating distance manual expense shows unexpected error
2 parents 5a0600a + e68cfd0 commit 2aca6a2

3 files changed

Lines changed: 117 additions & 6 deletions

File tree

‎src/libs/PolicyUtils.ts‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -298,17 +298,42 @@ function hasEligibleActiveAdminFromWorkspaces(policies: OnyxCollection<Policy> |
298298
return false;
299299
}
300300

301+
function cloneCustomUnitWithNewIDs(unit: CustomUnit, newCustomUnitID: string, newDefaultRateID?: string): CustomUnit {
302+
if (newDefaultRateID) {
303+
// The server-side DUPLICATE_POLICY assigns newDefaultRateID to the source's default rate.
304+
// Mirror getDefaultMileageRate's selection (enabled rates, sorted by index with
305+
// CONST.DEFAULT_NUMBER_ID for missing indexes) so the optimistic clone aligns with the
306+
// rate the expense flow will later treat as default. Other source rates get fresh server
307+
// IDs, so we drop them from the optimistic state to avoid stale duplicates.
308+
const defaultRate = Object.values(unit.rates)
309+
.filter((rate) => rate.enabled !== false)
310+
.sort((a, b) => (a.index ?? CONST.DEFAULT_NUMBER_ID) - (b.index ?? CONST.DEFAULT_NUMBER_ID))
311+
.at(0);
312+
return {
313+
...unit,
314+
customUnitID: newCustomUnitID,
315+
rates: defaultRate ? {[newDefaultRateID]: {...defaultRate, customUnitRateID: newDefaultRateID}} : {},
316+
};
317+
}
318+
319+
return {
320+
...unit,
321+
customUnitID: newCustomUnitID,
322+
};
323+
}
324+
301325
function getCustomUnitsForDuplication(
302326
policy: Policy,
303327
isDistanceRatesOptionSelected: boolean,
304328
isPerDiemOptionSelected: boolean,
305329
customUnitIDs: {
306330
distanceCustomUnitID: string;
307331
perDiemCustomUnitID: string;
332+
customUnitRateID?: string;
308333
},
309334
): Record<string, CustomUnit> | undefined {
310335
const customUnits = policy?.customUnits;
311-
const {distanceCustomUnitID, perDiemCustomUnitID} = customUnitIDs ?? {};
336+
const {distanceCustomUnitID, perDiemCustomUnitID, customUnitRateID} = customUnitIDs ?? {};
312337

313338
if ((!isDistanceRatesOptionSelected && !isPerDiemOptionSelected) || !customUnits || Object.keys(customUnits).length === 0) {
314339
return undefined;
@@ -339,20 +364,23 @@ function getCustomUnitsForDuplication(
339364
return undefined;
340365
}
341366

342-
return {[distanceCustomUnitID]: distanceCustomUnit, [perDiemCustomUnitID]: perDiemUnit};
367+
return {
368+
[distanceCustomUnitID]: cloneCustomUnitWithNewIDs(distanceCustomUnit, distanceCustomUnitID, customUnitRateID),
369+
[perDiemCustomUnitID]: cloneCustomUnitWithNewIDs(perDiemUnit, perDiemCustomUnitID),
370+
};
343371
}
344372

345373
if (isDistanceRatesOptionSelected && distanceCustomUnitID) {
346374
if (!distanceCustomUnit) {
347375
return undefined;
348376
}
349-
return {[distanceCustomUnitID]: distanceCustomUnit};
377+
return {[distanceCustomUnitID]: cloneCustomUnitWithNewIDs(distanceCustomUnit, distanceCustomUnitID, customUnitRateID)};
350378
}
351379

352380
if (!perDiemUnit || !perDiemCustomUnitID) {
353381
return undefined;
354382
}
355-
return {[perDiemCustomUnitID]: perDiemUnit};
383+
return {[perDiemCustomUnitID]: cloneCustomUnitWithNewIDs(perDiemUnit, perDiemCustomUnitID)};
356384
}
357385

358386
/**

‎src/libs/actions/Policy/Policy.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3112,7 +3112,10 @@ function createDraftWorkspace({
31123112
return params;
31133113
}
31143114

3115-
function buildOptimisticDuplicatePolicy(sourcePolicy: Policy, policyOptions: DuplicatePolicyDataOptions & {distanceCustomUnitID: string; perDiemCustomUnitID: string}) {
3115+
function buildOptimisticDuplicatePolicy(
3116+
sourcePolicy: Policy,
3117+
policyOptions: DuplicatePolicyDataOptions & {distanceCustomUnitID: string; perDiemCustomUnitID: string; customUnitRateID: string},
3118+
) {
31163119
const {
31173120
policyName: duplicatedPolicyName = '',
31183121
targetPolicyID: duplicatedPolicyID,
@@ -3121,6 +3124,7 @@ function buildOptimisticDuplicatePolicy(sourcePolicy: Policy, policyOptions: Dup
31213124
localCurrency: duplicatedLocalCurrency,
31223125
distanceCustomUnitID: duplicatedDistanceCustomUnitID,
31233126
perDiemCustomUnitID: duplicatedPerDiemCustomUnitID,
3127+
customUnitRateID: duplicatedCustomUnitRateID,
31243128
} = policyOptions;
31253129

31263130
const isMemberFeatureSelected = duplicatedParts?.people;
@@ -3182,6 +3186,7 @@ function buildOptimisticDuplicatePolicy(sourcePolicy: Policy, policyOptions: Dup
31823186
customUnits: getCustomUnitsForDuplication(sourcePolicy, isDistanceRatesFeatureSelected, isPerDiemFeatureSelected, {
31833187
distanceCustomUnitID: duplicatedDistanceCustomUnitID,
31843188
perDiemCustomUnitID: duplicatedPerDiemCustomUnitID,
3189+
customUnitRateID: duplicatedCustomUnitRateID,
31853190
}),
31863191
taxRates: isTaxesFeatureSelected ? taxRatesWithoutPendingDelete : undefined,
31873192
rules: isCodingRulesFeatureSelected ? {codingRules: codingRulesWithoutPendingDelete} : undefined,
@@ -3264,7 +3269,7 @@ function buildDuplicatePolicyData(policy: Policy, options: DuplicatePolicyDataOp
32643269
{
32653270
onyxMethod: Onyx.METHOD.SET,
32663271
key: `${ONYXKEYS.COLLECTION.POLICY}${targetPolicyID}`,
3267-
value: buildOptimisticDuplicatePolicy(policy, {...options, targetPolicyID, distanceCustomUnitID, perDiemCustomUnitID}),
3272+
value: buildOptimisticDuplicatePolicy(policy, {...options, targetPolicyID, distanceCustomUnitID, perDiemCustomUnitID, customUnitRateID}),
32683273
},
32693274
{
32703275
onyxMethod: Onyx.METHOD.MERGE,

‎tests/unit/PolicyUtilsTest.ts‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,84 @@ describe('PolicyUtils', () => {
352352
getCustomUnitsForDuplication(policyWithoutCustomUnits, true, true, {distanceCustomUnitID: otherUnit.customUnitID, perDiemCustomUnitID: perDiemUnit.customUnitID}),
353353
).toBeUndefined();
354354
});
355+
356+
it('clones the source default rate (lowest enabled index) under the API-known customUnitRateID', () => {
357+
const distanceUnitWithMultipleRates = {
358+
customUnitID: 'srcDist',
359+
name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
360+
enabled: true,
361+
attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, taxEnabled: true},
362+
rates: {
363+
rateB: {customUnitRateID: 'rateB', name: 'New Rate 1', rate: 100, currency: 'USD', enabled: true, index: 1, attributes: {taxRateExternalID: 'tax_other'}},
364+
rateA: {customUnitRateID: 'rateA', name: 'Default Rate', rate: 70, currency: 'USD', enabled: true, index: 0, attributes: {taxRateExternalID: 'tax_default'}},
365+
},
366+
};
367+
const policyWithMultipleRates: Policy = {
368+
...createRandomPolicy(0),
369+
customUnits: {[distanceUnitWithMultipleRates.customUnitID]: distanceUnitWithMultipleRates},
370+
};
371+
const result = getCustomUnitsForDuplication(policyWithMultipleRates, true, false, {
372+
distanceCustomUnitID: 'newDist',
373+
perDiemCustomUnitID: 'newPerDiem',
374+
customUnitRateID: 'newRate',
375+
});
376+
expect(result).toEqual({
377+
newDist: {
378+
...distanceUnitWithMultipleRates,
379+
customUnitID: 'newDist',
380+
rates: {
381+
newRate: {customUnitRateID: 'newRate', name: 'Default Rate', rate: 70, currency: 'USD', enabled: true, index: 0, attributes: {taxRateExternalID: 'tax_default'}},
382+
},
383+
},
384+
});
385+
});
386+
387+
it('drops all rates when no enabled rate exists', () => {
388+
const distanceUnitAllDisabled = {
389+
customUnitID: 'srcDist',
390+
name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
391+
enabled: true,
392+
attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES},
393+
rates: {
394+
rateA: {customUnitRateID: 'rateA', name: 'Disabled', rate: 50, currency: 'USD', enabled: false, index: 0},
395+
},
396+
};
397+
const policyAllDisabled: Policy = {
398+
...createRandomPolicy(0),
399+
customUnits: {[distanceUnitAllDisabled.customUnitID]: distanceUnitAllDisabled},
400+
};
401+
const result = getCustomUnitsForDuplication(policyAllDisabled, true, false, {
402+
distanceCustomUnitID: 'newDist',
403+
perDiemCustomUnitID: 'newPerDiem',
404+
customUnitRateID: 'newRate',
405+
});
406+
expect(result?.newDist.rates).toEqual({});
407+
});
408+
409+
it('treats missing index as 0 when picking the default rate', () => {
410+
const distanceUnitWithMissingIndex = {
411+
customUnitID: 'srcDist',
412+
name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
413+
enabled: true,
414+
attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES},
415+
rates: {
416+
rateB: {customUnitRateID: 'rateB', name: 'Indexed Rate', rate: 100, currency: 'USD', enabled: true, index: 1},
417+
rateA: {customUnitRateID: 'rateA', name: 'No-Index Rate', rate: 70, currency: 'USD', enabled: true},
418+
},
419+
};
420+
const policyWithMissingIndex: Policy = {
421+
...createRandomPolicy(0),
422+
customUnits: {[distanceUnitWithMissingIndex.customUnitID]: distanceUnitWithMissingIndex},
423+
};
424+
const result = getCustomUnitsForDuplication(policyWithMissingIndex, true, false, {
425+
distanceCustomUnitID: 'newDist',
426+
perDiemCustomUnitID: 'newPerDiem',
427+
customUnitRateID: 'newRate',
428+
});
429+
expect(result?.newDist.rates).toEqual({
430+
newRate: {customUnitRateID: 'newRate', name: 'No-Index Rate', rate: 70, currency: 'USD', enabled: true},
431+
});
432+
});
355433
});
356434
describe('getRateDisplayValue', () => {
357435
it('should return an empty string for NaN', () => {

0 commit comments

Comments
 (0)