[HOLD for payment 2024-08-07] [$250] Workspace - Profile page is not grayed out when workspace is created offline and name is edited

[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: 9.0.11-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #45820

Action Performed:

1. Log in with any account.
2. Go offline.
3. Create a new workspace.
4. Navigate to profile > Change the name of the workspace while offline.
5. Observe the profile page after changing the name

Expected Result:

The profile page is grayed out

Actual Result:

The profile page is not grayed out

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

Bug6550619_1721747216180.2024-07-23_17_53_43.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018594720711f70e58
• Upwork Job ID: 1815796234833602525
• Last Price Increase: 2024-07-23
• Automatic offers:
• suneox | Contributor | 103236301
• huult | Contributor | 103249686

Issue Owner

Current Issue Owner: @kadiealexander

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Triggered auto assignment to @bondydaa (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[lanitochka17]

Issue is not reproducible in Production:

bandicam.2024-07-23.19-37-57-636.mp4

[lanitochka17]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~018594720711f70e58

[melvin-bot]

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

[bondydaa]

won't have anything to do with backend code if this is offline specific behavior

[Tony-MK]

@bondydaa, I believe this regression from https://github.com/Expensify/App/pull/45820/files.

[bondydaa]

@Tony-MK have you reverted locally and tested yet?

[bondydaa]

i just reverted it locally and confirmed.

@Tony-MK do you see any other fixes or should we just push a revert PR?

cc @cristipaval @suneox @huult

[suneox]

I don’t think it’s related to this PR. I’ll start checking now.

[nkdengineer]

Proposal

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

The profile page is not grayed out

What is the root cause of that problem?

We're only merging the name of the policy if we update the name while the create WS request is waiting. So the pendingFields and the other optimistic data aren't updated

App/src/libs/actions/Policy/Policy.ts

Lines 1121 to 1125 in b21a694

	Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {
	name,
	});
	PersistedRequests.update(createWorkspaceRequestChangedIndex, workspaceRequest);

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

We should merge all optimistic data here instead of only merging the name of the policy.

Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {
      name,
      pendingFields: {
          ...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,
      },
      outputCurrency: currency,
          ...(customUnitID && {
              customUnits: {
                  [customUnitID]: {
                      ...distanceUnit,
                      rates: optimisticRates,
                  },
              },
          }),
  });

App/src/libs/actions/Policy/Policy.ts

Lines 1121 to 1125 in b21a694

	Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {
	name,
	});
	PersistedRequests.update(createWorkspaceRequestChangedIndex, workspaceRequest);

What alternative solutions did you explore? (Optional)

Result

Screen.Recording.2024-07-24.at.00.38.15.mov

[huult]

Hi @suneox @bondydaa ,
I think we should update the behavior to gray out when creating a workspace in offline mode to make it consistent with other places. If not, we should only update the pending field when updating the workspace name.

        Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {
            name,
+           pendingFields: {
+               ...policy.pendingFields,
+               generalSettings: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
+           },
        });

[suneox]

@suneox, You preferred proposal does not merge all the optimisticData for the policy but rather creates new data which is not required.

Could you re-review my proposal?

@Tony-MK thank you for your proposal but based on the scope of the PR, we are only preventing the call to update the workspace name by this condition checking if the name has changed while there is still a create request pending. Therefore, we don’t need to merge all the optimistic data for updating the workspace name into the current policy.

Furthermore, it is unnecessarily complex.

I also agree that it is unnecessary. We just need to update based on the suggestion by the PR owner. So I have updated my comment to pick a simpler approach.

[huult]

Proposal

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

Profile page is not grayed out when workspace is created offline and name is edited

What is the root cause of that problem?

We updated the policy name, skipped calling the update workspace function while offline, and optimisticData isn't being updated.

Here is the code handle I mentioned:

App/src/libs/actions/Policy/Policy.ts

Lines 1107 to 1133 in 1084706

	const persistedRequests = PersistedRequests.getAll();
	const createWorkspaceRequestChangedIndex = persistedRequests.findIndex(
	(request) => request.data?.policyID === policyID && request.command === WRITE_COMMANDS.CREATE_WORKSPACE && request.data?.policyName !== name,
	);
	const createWorkspaceRequest = persistedRequests[createWorkspaceRequestChangedIndex];
	if (createWorkspaceRequest) {
	const workspaceRequest: Request = {
	...createWorkspaceRequest,
	data: {
	...createWorkspaceRequest.data,
	policyName: name,
	},
	};
	Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {
	name,
	});
	PersistedRequests.update(createWorkspaceRequestChangedIndex, workspaceRequest);
	return;
	}
	API.write(WRITE_COMMANDS.UPDATE_WORKSPACE_GENERAL_SETTINGS, params, {
	optimisticData,
	finallyData,
	failureData,
	});

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

To ensure consistent behavior, we will gray out the profile when creating a workspace in offline mode and addgeneralSettings to pendingFields for policy data when creating a new workspace in offline mode.

// App/blob/main/src/libs/actions/Policy/Policy.ts#L1564
         pendingFields: {
            autoReporting: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
            approvalMode: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
            reimbursementChoice: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
+           generalSettings: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD
        },

POC

Screen.Recording.2024-07-24.at.13.12.51.mov

What alternative solutions did you explore? (Optional)

[Tony-MK]

The PR author has the simplest solution. 👍

[melvin-bot]

📣 @huult 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[bondydaa]

ok assigning @huult to fix since it was a regression from their PR. thank you all for the help!

[huult]

Here’s my PR for review.

[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 9.0.14-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix profile page not greyed out #46130

If no regressions arise, payment will be issued on 2024-08-07. 🎊

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

• - @suneox requires payment automatic offer (Contributor)
• - @huult requires payment automatic offer (Contributor)

[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:

• [@suneox] The PR that introduced the bug has been identified. Link to the PR:
• [@suneox] 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:
• [@suneox] 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:
• [@suneox] Determine if we should create a regression test for this bug.
• [@suneox] 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:

[suneox]

BugZero Checklist

• [@suneox] The PR that introduced the bug has been identified. Link to the PR: The PR #45820 might have triggered this issue, but it wasn’t the root cause. The root cause is that when creating a workspace in offline mode, the pending field is not set and which is an existing issue. Therefore, I don’t think PR #45820 is responsible for this.
• [@suneox] 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: N/A
• [@suneox] 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: N/A
• [@suneox] Determine if we should create a regression test for this bug. No, we don’t need a regression test for it since it’s a missing implementation case that we’ve already handled.

[Tony-MK]

Hey @kadiealexander, due to identifying the pull request that introduced this issue, I am eligible for payment?

Thanks