[$500] [Workflows] Workspace - Unable to auto update currency to USD for the first time

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.59.0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4468674&group_by=cases:section_id&group_order=asc&group_id=283225
Issue reported by: Applause - Internal Team

Action Performed:

1. Log in with a new expensifail account
2. Create a workspace with HUF as default currency
3. Navigate to Workspace settings - More features
4. Enable workflows
5. Open workflows
6. Go offline
7. Go online
8. Click on "Connect bank account"
9. Click on "Update to USD"

Expected Result:

I should be able to do it for the first time

Actual Result:

Unable to auto update currency to USD for the first time. "Oops! It appears that your workspace currency is set to a different currency than USD. To proceed, please set it to USD and try again" message appears. I'm able to do it for the second time

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

Add any screenshot/video evidence

Bug6435801_1712077002501.bandicam_2024-04-02_18-44-37-180.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ef1de5890d617574
• Upwork Job ID: 1775668746526109696
• Last Price Increase: 2024-05-01

[melvin-bot]

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

[lanitochka17]

@sakluger FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #wave-collect - Release 1

[sakluger]

I agree this is a wave-collect bug.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01ef1de5890d617574

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt (External)

[allgandalf]

The actual bug here is different, we allow the user to connect bank account even when the policy is still in draft stage i.e.CreateWorkspace has got a response from BE yet

So i will write my proposal accordingly

Proposal

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

Add bank account is not disabled even when the policy is in draft state

What is the root cause of that problem?

Whenever we create a policy, the policy is first in a draft state, the policy details aren't returned from the BE. so basically policy won't have all the field present.

Now when we immediately click Connect Bank account, we are sent to the modal to update the currency to USD.

When we click on update currency, we essential open the RHP ReimbursementAccountPage, in which we check for hasUnsupportedCurrency and if that is true then we show error of Opps!...:

App/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx

Lines 411 to 413 in 3ba42e6

	} else if (hasUnsupportedCurrency) {
	errorText = translate('bankAccount.hasCurrencyError');
	}

hasUnsupportedCurrency is checked using the following:

App/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx

Line 405 in 3ba42e6

const hasUnsupportedCurrency = (policy?.outputCurrency ?? '') !== CONST.CURRENCY.USD;

Initially when we create a workspace we still don't have the value of policy?.outputCurrency and hence it will be set to empty string now this is matched with the check !== CONST.CURRENCY.USD which would set the value of hasUnsupportedCurrency and we see the error message

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

We should disable the option to disable the connect bank account creation if we have pendingAction in the policy

Note this can be polished to only disable the connect bank option if the policy pendingAction is create, but that can be discussed.

So we need to update the disabled prop to:

disabled={isOffline || !isPolicyAdmin || (!!policy?.pendingAction)}

What alternative solutions did you explore? (Optional)

N/A

[s77rt]

I'm not able to reproduce

[s77rt]

@GandalfGwaihir

we allow the user to connect bank account even when the policy is still in draft stage i.e.CreateWorkspace has got a response from BE yet

Why do you think that the workspace is still in draft in this case? Also even if the CreateWorkspace is not done yet the UpdateWorkspaceGeneralSettings and OpenReimbursementAccountPage requests won't be made until the workspace is created

[allgandalf]

Why do you think that the workspace is still in draft in this case? Also even if the CreateWorkspace is not done yet the UpdateWorkspaceGeneralSettings and OpenReimbursementAccountPage requests won't be made until the workspace is created

When we create a workspace we set it's data optimistically:

App/src/libs/actions/Policy.ts

Lines 1990 to 2012 in 14ff944

	const optimisticData: OnyxUpdate[] = [
	{
	onyxMethod: Onyx.METHOD.SET,
	key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
	value: {
	id: policyID,
	type: CONST.POLICY.TYPE.TEAM,
	name: workspaceName,
	role: CONST.POLICY.ROLE.ADMIN,
	owner: sessionEmail,
	ownerAccountID: sessionAccountID,
	isPolicyExpenseChatEnabled: true,
	outputCurrency,
	pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
	customUnits,
	areCategoriesEnabled: true,
	areTagsEnabled: false,
	areDistanceRatesEnabled: false,
	areWorkflowsEnabled: false,
	areReportFieldsEnabled: false,
	areConnectionsEnabled: false,
	},
	},

If you see the video below, the create workspace api has still not got an response from the BE but we take the values of the created workspace from optimistically created data:

simplescreenrecorder-2024-04-04_19.59.41.mp4

It is after we get response for create workspace, that we call OpenReimbursementAccountPage you can see these calls with delay if you toggle 3G network/or low internet on network tab
c.c. @s77rt

[s77rt]

@nkdengineer Thanks for the proposal. Your RCA is not correct

If the API UpdateWorkspaceGeneralSettings is done before CreateWorkspace

Write requests are sequential and follow the FIFO principal. The second request will be sent only after the first one is completed.

[s77rt]

@GandalfGwaihir From your video I see that OpenReimbursementAccountPage is executed before UpdateWorkspaceGeneralSettings which is not the correct order. Let's investigate that

[nkdengineer]

@s77rt Thanks for your feedback.

[nkdengineer]

@s77rt I updated the RCA in my proposal

[s77rt]

@nkdengineer Thanks for the update. I still don't think the RCA is correct/complete

the outputCurrency will be EUR for a while, until the API in step 2 is successfully

Even if the currency is overwritten back to EUR. The bank page shouldn't load (OpenReimbursementAccountPage should not be be executed) until the currency is updated correctly to USD (after UpdateWorkspaceGeneralSettings is completed)

[melvin-bot]

@sakluger, @s77rt Whoops! This issue is 2 days overdue. Let's get this updated quick!

[s77rt]

Still looking for proposals

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[s77rt]

@GandalfGwaihir Thanks for the update. Your RCA is still not complete. Before we open the ReimbursementAccountPage we are making a call to UpdateWorkspaceGeneralSettings which updates the policy.outputCurrency both optimistically and on server's response. Why are we still seeing the error?

[nkdengineer]

Proposal

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

• The api OpenReimbursementAccountPage is called before UpdateWorkspaceGeneralSettings.

What is the root cause of that problem?

• Assume that the API are called in expected order: EnablePolicyWorkflows => UpdateWorkspaceGeneralSettings => OpenReimbursementAccountPage. But in this case, the actual order is EnablePolicyWorkflows => OpenReimbursementAccountPage => UpdateWorkspaceGeneralSettings, that leads to the bug. But why? Below is detail.

• After the API EnablePolicyWorkflows is called successfully, the below logic is called:
  

  App/src/libs/Middleware/SaveResponseInOnyx.ts

  Lines 36 to 39 in 1e510b5

  	return Promise.resolve({
  	...response,
  	shouldPauseQueue: true,
  	});

  
  that leads to the below logic is called:
  

  App/src/libs/Network/SequentialQueue.ts

  Lines 74 to 76 in 1e510b5

  	if (response?.shouldPauseQueue) {
  	pause();
  	}

• Calling pause will set isQueuePaused to true.

• Then, because the isQueuePaused is true, the below logic is called:
  

  App/src/libs/Network/SequentialQueue.ts

  Lines 58 to 60 in 1e510b5

  	if (isQueuePaused) {
  	return Promise.resolve();
  	}

• Because Promise.resolve() is returned, leads to the below logic is called:
  

  App/src/libs/Network/SequentialQueue.ts

  Lines 130 to 135 in 1e510b5

  	process().finally(() => {
  	isSequentialQueueRunning = false;
  	resolveIsReadyPromise?.();
  	currentRequest = null;
  	flushOnyxUpdatesQueue();
  	});

• Because resolveIsReadyPromise is called, leads to the below promise is resolved:
  

  App/src/libs/API/index.ts

  Line 162 in db9e01c

  SequentialQueue.waitForIdle().then(() => makeRequestWithSideEffects(command, apiCommandParameters, onyxData, CONST.API_REQUEST_TYPE.READ));

• As we can see, the API OpenReimbursementAccountPage is called.

What alternative solutions did you explore? (Optional)

• We can update:
  

  App/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx

  Line 188 in 1e510b5

  disabled={isOffline || !isPolicyAdmin}

  
  to:

                            disabled={isOffline || !isPolicyAdmin || !!policy?.pendingFields?.areWorkflowsEnabled}

[nkdengineer]

@s77rt I removed my original proposal and posted a new one with the new RCA and solution

[s77rt]

@nkdengineer Thanks for the update.

After the API EnablePolicyWorkflows is called successfully, the below logic is called

That logic is only executed when the client is not in sync (missing onyx updates). We have an early return that occur in the rest of the cases

App/src/libs/Middleware/SaveResponseInOnyx.ts

Lines 28 to 30 in 1e510b5

	if (requestsToIgnoreLastUpdateID.includes(request.command) || !OnyxUpdates.doesClientNeedToBeUpdated(Number(response?.previousUpdateID ?? 0))) {
	return OnyxUpdates.apply(responseToApply);
	}

Were you able to reproduce the missing onyx updates case?

[nkdengineer]

@s77rt

Were you able to reproduce the missing onyx updates case

• Yeah, I can reproduce it. First, I add the below log function to this one:

   console.log("**********************", {command: request.command})

• Here is the result:

Screen.Recording.2024-04-21.at.08.08.07.mov

[nkdengineer]

Also, there are 2 things please help note:

• I tried to remove this line and the bug is gone.
• In the above video, the key point is that you need to click on "New workspace" and then toggle "Workflows" feature right after that.

[s77rt]

@nkdengineer Thanks for checking that but the root cause is still unclear. Can you elaborate more? I see you got a not found view, perhaps its related.

[nkdengineer]

@s77rt

I see you got a not found view, perhaps its related.

This will be fixed here

[nkdengineer]

root cause is still unclear

Could you identify any unclear sections in my RCA so that I can make updates accordingly? Thank you.

[s77rt]

@nkdengineer The chain effect makes sense but what trigged that chain is not clear i.e. why is the client out-of-sync? (specifically why this happens only in this scenario)

[nkdengineer]

why is the client out-of-sync

Here is what I see when creating a new workspace and then toggle workflow feature:

1. API CreateWorkspace returns lastUpdateID: 277284366 and previousUpdateID: 277283967 so we set lastUpdateIDAppliedToClient to 277284366.

2. Then API GetMissingOnyxMessages returns lastUpdateID: 277284259 and previousUpdateID: 277283967 so we set lastUpdateIDAppliedToClient to 277284259

3. Then API EnablePolicyWorkflows returns lastUpdateID: 277284459 and previousUpdateID: 277284395. In here, we compare the lastUpdateIDFromClient to previousUpdateID and because 277284259 < 277284395, so doesClientNeedToBeUpdated return true

[s77rt]

@nkdengineer Why in your case you are making a call to GetMissingOnyxMessages? This can't be related to CreateWorkspace alone. I can't reproduce the bug that way

Screen.Recording.2024-04-23.at.7.03.31.PM.mov

Screen.Recording.2024-04-23.at.7.04.04.PM.mov

[nkdengineer]

Why in your case you are making a call to GetMissingOnyxMessages

Do you always not encounter the call to GetMissingOnyxMessages? From my perspective, there are times when GetMissingOnyxMessages is not called, and in those cases, the bug does not manifest

[s77rt]

@nkdengineer We need to identify what trigger that in the first place and preferably fix it there.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[sakluger]

Looks like the solution is still being discussed.

[s77rt]

Not overdue. Still looking for clear RCA

[melvin-bot]

@sakluger @s77rt this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[sakluger]

Sounds like it's not reproducible, let's close the issue.