[HOLD for payment 2023-08-03] [HOLD for payment 2023-08-02] Contact methods - System throws an error when refreshing the secondary login verification page

[kbecciv]

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

---

Action Performed:

1. Click on 'Profile' avatar
2. Click on 'Profile' link
3. Click on 'Contact methods' link
4. Click on 'New contact method' button
5. Enter email & click on 'Add' button
6. Open the newly added contact method
7. Refresh the page

Expected Result:

System shouldn't throw error, & should display magic code input field

Actual Result:

System throws an error, & hides the magic code input field

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.45.1
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

2023-07-25.15.21.22.1.mp4

Recording.3896.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Natnael-Guchima
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690284278814549

View all open jobs on GitHub

[OSBotify]

👋 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.

[melvin-bot]

Triggered auto assignment to @Gonals (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[s-alves10]

Proposal

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

System throws an error when refreshing the secondary login verification page

What is the root cause of that problem?

This is a regression from #22727

When we add a new contact, AddNewContactMethod API is called. The problem is here

App/src/libs/actions/PersistedRequests.js

Lines 27 to 38 in 0e2f742

	function remove(requestToRemove) {
	/**
	* We only remove the first matching request because the order of requests matters.
	* If we were to remove all matching requests, we can end up with a final state that is different than what the user intended.
	*/
	const index = _.findIndex(persistedRequests, (persistedRequest) => _.isEqual(persistedRequest, requestToRemove));
	if (index !== -1) {
	persistedRequests.splice(index, 1);
	}
	Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests);
	}

After we call the API, we remove it from the queue. But as you can see from the above code, splice is a mutating method and persistedRequests value isn't changed. So oynx.set determines that it's not changed and not store to the storage.

We can verify this from the fact that onyx callback here isn't called after we remove the request.

That's why AddNewContactMethod remains in the onyx store and is called again after the refresh. This is the root cause.

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

We need to clone the persistedRequests and splice and set again.

This works as expected

What alternative solutions did you explore? (Optional)

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[Pujan92]

splice is a non-mutating function

@s-alves10 splice is a mutating method.

[s-alves10]

@s-alves10 splice is a mutating method.

oh, yes. sorry. mutating is causing the problem.

[puneetlath]

Reverting here: #23551

[puneetlath]

@Natnael-Guchima contract here: https://www.upwork.com/nx/wm/pre-hire/c/8577561/offer/25747540

[Natnael-Guchima]

Accepted the offer. Thanks @puneetlath

[puneetlath]

Paid, thanks!

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

• [CP Staging] Revert "fix remove only first request" #23551

If no regressions arise, payment will be issued on 2023-08-02. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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

• [CP Staging] Revert "fix remove only first request" #23551

If no regressions arise, payment will be issued on 2023-08-03. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty