Skip to content

Commit 5276766

Browse files
committed
Drop successData null cleanup of source non-default rate IDs
In the staging server's actual behavior, the duplicated workspace preserves the source's non-default rate IDs — so the optimistic clone's keys and the eventual server keys line up and Onyx merges cleanly into the same set of rates with no UI duplicates. The defensive null cleanup was guarding against a hypothetical case where the server generates fresh non-default rate IDs (which we did not observe). Drop the cleanup to keep this PR minimal: the cloneCustomUnitWithNewIDs fix in PolicyUtils.ts is what restores offline visibility (#89865); no extra successData write is needed in the observed server behavior. If the server ever switches to generated non-default IDs, that's a follow-up patch (and would surface as a fresh duplicate-rates issue). Also drops the now-unused getDefaultDistanceRate import from Policy.ts and the test that asserted the absence of the cleanup write.
1 parent 0ef827f commit 5276766

2 files changed

Lines changed: 5 additions & 122 deletions

File tree

src/libs/actions/Policy/Policy.ts

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -90,14 +90,7 @@ import Permissions from '@libs/Permissions';
9090
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
9191
import * as PhoneNumber from '@libs/PhoneNumber';
9292
import * as PolicyUtils from '@libs/PolicyUtils';
93-
import {
94-
getCustomUnitsForDuplication,
95-
getDefaultDistanceRate,
96-
getMemberAccountIDsForWorkspace,
97-
goBackWhenEnableFeature,
98-
isControlPolicy,
99-
navigateToExpensifyCardPage,
100-
} from '@libs/PolicyUtils';
93+
import {getCustomUnitsForDuplication, getMemberAccountIDsForWorkspace, goBackWhenEnableFeature, isControlPolicy, navigateToExpensifyCardPage} from '@libs/PolicyUtils';
10194
import * as ReportUtils from '@libs/ReportUtils';
10295
import {hasValidModifiedAmount} from '@libs/TransactionUtils';
10396
import type {Feature} from '@pages/OnboardingInterestedFeatures/types';
@@ -3243,19 +3236,6 @@ function buildDuplicatePolicyData(policy: Policy, options: DuplicatePolicyDataOp
32433236
const {customUnitID: distanceCustomUnitID, customUnitRateID} = buildOptimisticDistanceRateCustomUnits(outputCurrency);
32443237
const perDiemCustomUnitID = generateCustomUnitID();
32453238

3246-
// Source non-default distance rate IDs — kept in optimistic data (by cloneCustomUnitWithNewIDs)
3247-
// so they're visible offline, then nulled in successData so the server's Pusher response can
3248-
// repopulate with fresh server IDs without leaving optimistic-vs-server duplicates. Only
3249-
// computed when the user actually selected distance rates for duplication; otherwise no
3250-
// optimistic distance unit was created and there's nothing to clean up.
3251-
const sourceDistanceUnit = parts?.distance ? Object.values(policy?.customUnits ?? {}).find((unit) => unit.name === CONST.CUSTOM_UNITS.NAME_DISTANCE) : undefined;
3252-
const sourceDistanceDefaultRate = getDefaultDistanceRate(sourceDistanceUnit?.rates);
3253-
const sourceNonDefaultDistanceRateIDs = sourceDistanceUnit
3254-
? Object.values(sourceDistanceUnit.rates)
3255-
.filter((rate) => rate.customUnitRateID !== sourceDistanceDefaultRate?.customUnitRateID)
3256-
.map((rate) => rate.customUnitRateID)
3257-
: [];
3258-
32593239
const optimisticAnnounceChat = ReportUtils.buildOptimisticAnnounceChat(targetPolicyID, [...policyMemberAccountIDs], currentUserAccountID);
32603240
const announceRoomChat = optimisticAnnounceChat.announceChatData;
32613241

@@ -3371,13 +3351,6 @@ function buildDuplicatePolicyData(policy: Policy, options: DuplicatePolicyDataOp
33713351
type: null,
33723352
areReportFieldsEnabled: null,
33733353
},
3374-
...(sourceNonDefaultDistanceRateIDs.length > 0 && {
3375-
customUnits: {
3376-
[distanceCustomUnitID]: {
3377-
rates: Object.fromEntries(sourceNonDefaultDistanceRateIDs.map((id) => [id, null])),
3378-
},
3379-
},
3380-
}),
33813354
},
33823355
},
33833356
{

tests/actions/PolicyTest.ts

Lines changed: 4 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -866,102 +866,12 @@ describe('actions/Policy', () => {
866866
}
867867
expect(distanceUnit.rates[defaultRateID]).toBe(defaultRate);
868868

869-
// Resume the mocked fetch so successData fires and clears the optimistic source rate IDs.
870-
// After success, only the rebound default remains until a Pusher response (not mocked here)
871-
// would repopulate the rates with fresh server IDs.
869+
// Resume the mocked fetch so the API call resolves; the duplicated workspace's optimistic
870+
// rates remain visible until a Pusher response (not mocked here) repopulates them with
871+
// the server's rate set. We rely on the server preserving the source's non-default rate
872+
// IDs in the duplicate, so the optimistic state and the eventual server state line up.
872873
await mockFetch.resume?.();
873874
await waitForBatchedUpdates();
874-
875-
const duplicateAfterSuccess: OnyxEntry<PolicyType> = await new Promise((resolve) => {
876-
const connection = Onyx.connect({
877-
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
878-
callback: (workspace) => {
879-
Onyx.disconnect(connection);
880-
resolve(workspace);
881-
},
882-
});
883-
});
884-
885-
const distanceUnitAfter = Object.values(duplicateAfterSuccess?.customUnits ?? {}).find((unit) => unit.name === CONST.CUSTOM_UNITS.NAME_DISTANCE);
886-
if (!distanceUnitAfter) {
887-
throw new Error('Expected duplicated distance unit to still exist after success');
888-
}
889-
const ratesAfter = Object.entries(distanceUnitAfter.rates).filter(([, rate]) => rate !== null);
890-
expect(ratesAfter).toHaveLength(1);
891-
expect(ratesAfter.at(0)?.[1]?.name).toBe('Default Rate');
892-
});
893-
894-
it('duplicate workspace without distance rates does not write any customUnits cleanup for the source distance rates', async () => {
895-
await Onyx.set(ONYXKEYS.SESSION, {email: ESH_EMAIL, accountID: ESH_ACCOUNT_ID});
896-
897-
const sourceDistanceUnitID = 'srcDistUnitB';
898-
const sourceExtraRateID = 'srcExtraRateB';
899-
const fakePolicy: PolicyType = {
900-
...createRandomPolicy(20, CONST.POLICY.TYPE.TEAM),
901-
customUnits: {
902-
[sourceDistanceUnitID]: {
903-
customUnitID: sourceDistanceUnitID,
904-
name: CONST.CUSTOM_UNITS.NAME_DISTANCE,
905-
enabled: true,
906-
attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES},
907-
rates: {
908-
srcDefaultRateB: {customUnitRateID: 'srcDefaultRateB', name: 'Default Rate', rate: 72.5, currency: 'USD', enabled: true, index: 0},
909-
[sourceExtraRateID]: {customUnitRateID: sourceExtraRateID, name: 'Extra Rate', rate: 100, currency: 'USD', enabled: true, index: 1},
910-
},
911-
},
912-
},
913-
};
914-
await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy);
915-
await waitForBatchedUpdates();
916-
917-
const policyID = Policy.generatePolicyID();
918-
const options = {
919-
currentUserAccountID: ESH_ACCOUNT_ID,
920-
currentUserEmail: ESH_EMAIL,
921-
policyName: 'No Distance Duplicate',
922-
policyID: fakePolicy.id,
923-
targetPolicyID: policyID,
924-
welcomeNote: 'Join my policy',
925-
parts: {
926-
people: false,
927-
reports: false,
928-
connections: false,
929-
categories: false,
930-
tags: false,
931-
taxes: false,
932-
perDiem: false,
933-
reimbursements: false,
934-
expenses: false,
935-
distance: false,
936-
invoices: false,
937-
exportLayouts: false,
938-
},
939-
localCurrency: 'USD',
940-
};
941-
942-
Policy.duplicateWorkspace(fakePolicy, options);
943-
await waitForBatchedUpdates();
944-
945-
const duplicatePolicy: OnyxEntry<PolicyType> = await new Promise((resolve) => {
946-
const connection = Onyx.connect({
947-
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
948-
callback: (workspace) => {
949-
Onyx.disconnect(connection);
950-
resolve(workspace);
951-
},
952-
});
953-
});
954-
955-
// The duplicate should not have any distance custom unit because parts.distance is false.
956-
// In particular, it should not contain a partial/orphan distance unit consisting only of
957-
// nulled-out source rate IDs (which would be the regression the gating prevents).
958-
const distanceUnits = Object.values(duplicatePolicy?.customUnits ?? {}).filter((unit) => unit.name === CONST.CUSTOM_UNITS.NAME_DISTANCE);
959-
expect(distanceUnits).toHaveLength(0);
960-
// The source's extra rate ID must not appear anywhere in the duplicate's customUnits as
961-
// a stale "null" entry from successData cleanup.
962-
for (const unit of Object.values(duplicatePolicy?.customUnits ?? {})) {
963-
expect(Object.keys(unit.rates ?? {})).not.toContain(sourceExtraRateID);
964-
}
965875
});
966876

967877
it('creates a new workspace with BASIC approval mode if the introSelected is MANAGE_TEAM', async () => {

0 commit comments

Comments
 (0)