[HOLD for payment 2024-08-01] [$250] Close account - Unable to close unverified account

[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.86-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/4652683
Email or phone of affected tester (no customers): natnael.expensify+0a89fdndafadf@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Sign up with a new account
2. Go to Settings > Security > Close account
3. Enter default email > click 'Close account'

Expected Result:

Account should be closed

Actual Result:

Account is not closed

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

Bug6519711_1718909872400.Screen_Recording_2024-06-20_at_9.48.20_at_night.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e5a38773a3b72e10
• Upwork Job ID: 1804323493948615973
• Last Price Increase: 2024-07-13
• Automatic offers:
• mkhutornyi | Reviewer | 103155489

Issue Owner

Current Issue Owner: @kevinksullivan / @isabelastisser

[melvin-bot]

Triggered auto assignment to @isabelastisser (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.

[lanitochka17]

@isabelastisser 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

[cretadn22]

Proposal

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

VBA Link does not sign user in when redirected

What is the root cause of that problem?

the App only call CLOSE_ACCOUNT api. this is the response from BE

the BE does not remove the session on Onyx

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

App/src/libs/actions/User.ts

Line 100 in ed50007

API.write(WRITE_COMMANDS.CLOSE_ACCOUNT, parameters, {

We could use signOutAndRedirectToSignIn function to reset the token and navigate to the sign-in page.

What alternative solutions did you explore? (Optional)

we could also remove the session from optimistic data.

[bernhardoj]

Proposal

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

Unable to close unverified account. The request completes but the user isn't logged out.

What is the root cause of that problem?

When the request is completed, there is already a clear onyx command to clear the onyx so the user is logged out. However, when we receive the response, doesClientNeedToBeUpdated returns true which will call OnyxUpdates.saveUpdateInformation which triggers GetMissingOnyxMessages but fails because the account is already closed, so the response onyx data is never applied.

App/src/libs/Middleware/SaveResponseInOnyx.ts

Lines 28 to 33 in 633e708

	if (requestsToIgnoreLastUpdateID.includes(request.command) || !OnyxUpdates.doesClientNeedToBeUpdated(Number(response?.previousUpdateID ?? 0))) {
	return OnyxUpdates.apply(responseToApply);
	}
	// Save the update IDs to Onyx so they can be used to fetch incremental updates if the client gets out of sync from the server
	OnyxUpdates.saveUpdateInformation(responseToApply);

The reason doesClientNeedToBeUpdated returns true is because the lastUpdateIDAppliedToClient is smaller than the previousUpdateID response from CloseAccount.

App/src/libs/actions/OnyxUpdates.ts

Lines 162 to 169 in 633e708

	const lastUpdateIDFromClient = clientLastUpdateID || lastUpdateIDAppliedToClient;
	// If we don't have any value in lastUpdateIDFromClient, this is the first time we're receiving anything, so we need to do a last reconnectApp
	if (!lastUpdateIDFromClient) {
	return true;
	}
	return lastUpdateIDFromClient < previousUpdateID;

You can see that previousUpdateID is the lastUpdateID of the previous request.

But for the CloseAccount response, the previousUpdateID is bigger than the previous request lastUpdateID, so the app detected there is missing onyx data.

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

Fix it on the BE so the correct previousUpdateID is returned or if we want to fix this on the FE, we can add CloseAccount request to the requestsToIgnoreLastUpdateID list because we don't care if there is missing data when closing an account.

App/src/libs/Middleware/SaveResponseInOnyx.ts

Lines 6 to 8 in 633e708

	// If we're executing any of these requests, we don't need to trigger our OnyxUpdates flow to update the current data even if our current value is out of
	// date because all these requests are updating the app to the most current state.
	const requestsToIgnoreLastUpdateID: string[] = [WRITE_COMMANDS.OPEN_APP, SIDE_EFFECT_REQUEST_COMMANDS.RECONNECT_APP, SIDE_EFFECT_REQUEST_COMMANDS.GET_MISSING_ONYX_MESSAGES];

[melvin-bot]

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

[melvin-bot]

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

[isabelastisser]

@mkhutornyi please review the proposals above when you have a chance. Thanks!

[jasham]

Is this bug is still open or assigned to someone else ?

[melvin-bot]

📣 @jasham! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[jasham]

Contributor details
Your Expensify account email: jasham009@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/jasham4ryou

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[jasham]

Is this bug is still open or assigned to someone else ?

[isabelastisser]

@mkhutornyi, please review the proposals above!

@jasham, this is still open.

[isabelastisser]

@mkhutornyi I DM'd you for visibility. Thanks!

[mkhutornyi]

reviewing

[mkhutornyi]

Pulling engineer for review since it's possible to fix in BE
🎀👀🎀

[melvin-bot]

Triggered auto assignment to @AndrewGable, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

Triggered auto assignment to @kevinksullivan (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.

[isabelastisser]

I will be OOO tomorrow and next week, so I am reassigning this until I return on July 29. Thanks, @kevinksullivan!

Status: Waiting for PR to merge.

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

• Fix can't close account of a new created account #45673

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

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

• @bernhardoj requires payment through NewDot Manual Requests
• @mkhutornyi requires payment automatic offer (Reviewer)

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

• [@mkhutornyi] The PR that introduced the bug has been identified. Link to the PR:
• [@mkhutornyi] 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:
• [@mkhutornyi] 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:
• [@mkhutornyi] Determine if we should create a regression test for this bug.
• [@mkhutornyi] 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.
• [@kevinksullivan / @isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[isabelastisser]

I'm back and taking this back! Thanks, @kevinksullivan!

[isabelastisser]

Hey @mkhutornyi, please complete the BZ list above. Thanks!

[bernhardoj]

Requested in ND.

[isabelastisser]

Payment summary:

@bernhardoj requires payment through NewDot Manual Requests $250 / Contributor PENDING
@mkhutornyi requires payment automatic offer $250 (Reviewer) C+ PAID

[isabelastisser]

@mkhutornyi, please complete the BZ list. I will DM you for visibility. Thanks!

[JmillsExpensify]

$250 approved for @bernhardoj

[mkhutornyi]

The root cause was in backend. No offending PR in frontend.

Regression test:
https://expensify.testrail.io/index.php?/tests/view/4652683

[isabelastisser]

All set! Closing.