[HOLD for payment 2022-07-05] [$500] Android/IOS - Each of the Workspace option setting are unlocked after delete VBA from OldDot

[kbecciv]

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

---

Action Performed:

Precondition: create a new expensifail account

1. Launce the app
2. Go to Settings - Enable the Staging Setting
3. Go to any Workspace and click Connect bank account
4. Select Connect by Plaid
5. Use Wells Fargo bank
   Username: user_good | Password: pass_good
   CompanyName: Alberta Bobbeth Charleson
   CompanyTaxID: 123456789
   First Name: Alberta
   Last Name: Charleson
6. Go to Setting - Payments
7. Verify the Bank account is displayed
8. Go to OldDot with the same account
9. Go to Setting - Account - Payments
10. Delete the VBA
11. Go to New Expensify app
12. Tap on Setting - Workspace
13. Check each of the Workspace option setting and make sure it locked

Expected Result:

Each of the Workspace option setting are locked after delete VBA from OldDot

Actual Result:

Each of the Workspace option setting are unlocked after delete VBA from OldDot

Workaround:

Unknown

Platform:

Where is this issue occurring?

• iOS
• Android

Version Number: 1.1.33.2

Reproducible in staging?: Yes

Reproducible in production?: No

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5424994_Screen_Recording_20220126-213514_New_Expensify__1_.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause

Slack conversation:

View all open jobs on GitHub

[MelvinBot]

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

[deetergp]

I am 95% sure this is be something an external contributor could fix.

[MelvinBot]

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

[kadiealexander]

Posted to Upwork https://www.upwork.com/jobs/~010dee2324de4cf53e

[MelvinBot]

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

[MelvinBot]

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

[mananjadhav]

I am posting this proposal. I've got the root cause and posting a solution. If my understanding for VBA is incorrect and solution is not what is expected, can someone help with some context and I'll correct it accordingly.

Root Cause:

App/src/pages/workspace/WorkspacePageWithSections.js

Lines 67 to 71 in ddae433

	const achState = lodashGet(this.props.reimbursementAccount, 'achData.state', '');
	const hasVBA = achState === BankAccount.STATE.OPEN;
	const isUsingECard = lodashGet(this.props.user, 'isUsingExpensifyCard', false);
	const policyID = lodashGet(this.props.route, 'params.policyID');

The state achState from Onyx.reimbursementAccount is OPEN even after we delete the payment method from the list. Hence all the pages in Workspaces are in an unlocked state.

Now irrespective of whether I delete from the NewDot or OldDot, deleting the BankAccount from Payments -> Select Bank Account -> Delete, only removes the bank account from Onyx.bankAccountList and has no impact on ONYX.reimbursementAccount.

Solution

We already have a function to resetFreePlanBankAccount.

App/src/libs/actions/ReimbursementAccount/resetFreePlanBankAccount.js

Lines 12 to 15 in ddae433

	function resetFreePlanBankAccount() {
	const bankAccountID = lodashGet(store.getReimbursementAccountInSetup(), 'bankAccountID');
	if (!bankAccountID) {
	throw new Error('Missing bankAccountID when attempting to reset free plan bank account');

We can update the following BankAccounts.deleteBankAccount action:

App/src/libs/actions/BankAccounts.js

Lines 105 to 108 in ddae433

	function deleteBankAccount(bankAccountID) {
	API.DeleteBankAccount({
	bankAccountID,
	}).then((response) => {

function deleteBankAccount(bankAccountID) {
   const reimbursementBankAccountId = lodashGet(store.getReimbursementAccountInSetup(), 'bankAccountID');
   if(reimbursementBankAccountId === bankAccountID) {
        BankAccounts.resetFreePlanBankAccount();
        return;
   }  

  // Old code as is
  API.DeleteBankAccount({
        bankAccountID,
    }).then((response) => {

...
...

}

Another part about syncing from OldDot, we add another action in BankAccounts, cleanLocalReimbursementData.

This gets called in PaymentMethods.getPaymentMethods, after Onyx.multiSet

App/src/libs/actions/PaymentMethods.js

Lines 76 to 80 in ddae433

	.then((response) => {
	// Convert bank accounts/cards from an array of objects, to a map with the bankAccountID as the key
	const bankAccounts = _.object(_.map(lodashGet(response, 'bankAccountList', []), bankAccount => [bankAccount.bankAccountID, bankAccount]));
	const debitCards = _.object(_.map(lodashGet(response, 'fundList', []), fund => [fund.fundID, fund]));
	Onyx.multiSet({

function cleanLocalReimbursementData(bankAccounts) {
    const bankAccountID = lodashGet(store.getReimbursementAccountInSetup(), 'bankAccountID');

   // We check if the bank account list doesn't have the reimbursementAccount
    if(!_.find(bankAccounts, bankAccount => bankAccount.bankAccountID === bankAccountID)) { 
        Onyx.merge(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {achData: null, shouldShowResetModal: false});
    }
}

[rushatgabhane]

Hmm, I'm still trying to understand Manan's proposal, and any regressions that might arise if we go ahead with it.

@pecanoro feel free to take this from me.

[rushatgabhane]

@pecanoro unassigning myself, don't have the bandwidth for this one

[pecanoro]

Sorry for the delay!! I think the proposal would work well, I have been trying to find what regressions it could cause but if there is no reimbursement account I don't see why we need to keep it on newDot.

@mananjadhav I have assigned you to the issue, feel free to apply for the job.

[MelvinBot]

📣 @mananjadhav You have been assigned to this job by @pecanoro!
Please apply to this job in Upwork 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 📖

[kadiealexander]

@pecanoro any thoughts on testing this internally?

[pecanoro]

Oh yeah, we don't have test credentials for contributors. Can you open the PR and I will test it internally?

[kadiealexander]

@mananjadhav were you able to open the PR as per @pecanoro's request above?

[mananjadhav]

Hey sorry I missed this.

@pecanoro I have raised the draft PR. Will you need an open PR? I am not sure what I'll fill in the checklist and the screenshots hence I didn't open. If you can check from branch then that'll be great.

[kadiealexander]

@pecanoro did you get a chance to take a look at this one?

[pecanoro]

Is this the PR? #8023 Sorry, since I wasn't tagged there I wasn't sure which PR was the one for me to test the changes.

[kadiealexander]

@mananjadhav could you please confirm which PR to test?

[mananjadhav]

Something is wrong, I didn't get the notification until @kadiealexander mentioned me again.

Is this the PR? #8023

Yes, @pecanoro that's the PR. I've opened the PR ready for review but added a tag in title [DO NOT MERGE].

[kadiealexander]

@pecanoro just checking in! How's testing going?

[pecanoro]

I finally found some time to test it! Sorry for the delay

[mananjadhav]

I am on this one. Adding some code to update certain flags on Onyx. This will need sometime to manually update flags.

[mananjadhav]

I am just setting up the workspace on the new machine. Old one crashed. I'll have these by tomorrow. Sorry for the delay.

[mananjadhav]

Hi @pecanoro I had some issues with my new workstation and the last PR ended up having multiple unsigned commits. I've closed the old PR and raised a new one #9490. Please review this (only difference between the old one and new one is that this PR is properly synced with latest main).

Thank you and @kadiealexander for your patience on this one. Appreciate your help in testing and pulling this through.

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot]

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

• fix: Lock workspace settings after Delete #9490

If no regressions arise, payment will be issued on 2022-07-05. 🎊

[kadiealexander]

All paid, thanks @mananjadhav!