[HOLD for payment 2024-04-09] [Simplified Collect][Distance rates] - Distance rates disappear when changing default workspace currency offline

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.56-2
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

• User is admin of Collect workspace.

1. Go to staging.new.expensify.com.
2. Go to Profile > Workspaces > Collect workspace.
3. Go to Distance rates.
4. Add a few distance rates.
5. Go offline.
6. Go to workspace profile > Currency > Change to a different currency.
7. Go to Distance rates.

Expected Result:

All the available distance rates will still be present after changing workspace default currency offline.

Actual Result:

Only one distance rate is present after changing workspace default currency offline. The rest reappears after returning online.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6425812_1711365129588.20240325_190626.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010ef3b0c77b5b3ce7
• Upwork Job ID: 1772808192831074304
• Last Price Increase: 2024-03-27

Issue Owner

Current Issue Owner: @alitoshmatov

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[izarutskaya]

We think that this bug might be related to #wave-control.

[abzokhattab]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Distance rates disappear when changing default workspace currency offline

What is the root cause of that problem?

when changing the currency, we set the optimistic value of the default rate value only, we dont set the rest

App/src/libs/actions/Policy.ts

Lines 1338 to 1367 in 250baa9

	const distanceRate = Object.values(distanceUnit?.rates ?? {}).find((rate) => rate.name === CONST.CUSTOM_UNITS.DEFAULT_RATE);
	const optimisticData: OnyxUpdate[] = [
	{
	// We use SET because it's faster than merge and avoids a race condition when setting the currency and navigating the user to the Bank account page in confirmCurrencyChangeAndHideModal
	onyxMethod: Onyx.METHOD.SET,
	key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
	value: {
	...policy,
	pendingFields: {
	generalSettings: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
	},
	// Clear errorFields in case the user didn't dismiss the general settings error
	errorFields: {
	generalSettings: null,
	},
	name,
	outputCurrency: currency,
	...(distanceUnit?.customUnitID && distanceRate?.customUnitRateID
	? {
	customUnits: {
	[distanceUnit?.customUnitID]: {
	...distanceUnit,
	rates: {
	[distanceRate?.customUnitRateID]: {
	...distanceRate,
	currency,
	},

What changes do you think we should make in order to solve the problem?

we need to update all available rates .. not only the default one:

   const distanceRates = distanceUnit?.rates ?? {};
    Object.keys(distanceRates).forEach((id) => {
        distanceRates[id] = {
            ...distanceRates[id],
            currency,
        };
    });

and change the optimistic value here to:

      ...(distanceUnit?.customUnitID
                    ? {
                          customUnits: {
                              [distanceUnit?.customUnitID]: {
                                  ...distanceUnit,
                                  rates: distanceRates,
                              },
                          },
                      }
                    : {}),

also we need to change the failure value here to the original rates object:

                          customUnits: distanceUnit?.rates,

POC

Screen.Recording.2024-03-25.at.4.31.46.PM.mov

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~010ef3b0c77b5b3ce7

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @alitoshmatov (Internal)

[MrMuzyk]

I am Michał from Callstack - expert contributor group. I’d like to work on this job.

[Christinadobrzyn]

hi! Could you confirm if this job is related ? I think it might be?

#36680

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: update distance rates when changing currency offline #39046

If no regressions arise, payment will be issued on 2024-04-09. 🎊

For reference, here are some details about the assignees on this issue:

• @MrMuzyk does not require payment (Contractor)
• @alitoshmatov requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@alitoshmatov] The PR that introduced the bug has been identified. Link to the PR:
• [@alitoshmatov] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@alitoshmatov] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@alitoshmatov] Determine if we should create a regression test for this bug.
• [@alitoshmatov] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[luacmartins]

@kadiealexander I think #36680 is its own thing.

[kadiealexander]

@alitoshmatov please complete the checklist and apply here so we can get you paid and close this one.

[melvin-bot]

Payment Summary

Upwork Job

• Contributor: @MrMuzyk is from an agency-contributor and not due payment
• Reviewer: @alitoshmatov paid $500 via Upwork

BugZero Checklist (@kadiealexander)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1772808192831074304/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[kadiealexander]

Bumped @alitoshmatov here.

[alitoshmatov]

• The PR that introduced the bug has been identified. Link to the PR: feat: policy add distance rate #38035
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/38035/files#r1561280352
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: No need
• Determine if we should create a regression test for this bug. No need

[kadiealexander]

@alitoshmatov I've sent an Upwork contract, please accept!