[HOLD for payment 2024-06-05] [$250] iOS - Wallet - App crashes when scrolling up and down bank account selection list

[izarutskaya]

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

---

Version Number: 1.4.66-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Launch New Expensify app.
2. Go to Account settings.
3. Go to Wallet.
4. Tap Add bank account > Personal bank account.
5. Log in to Chase account.
6. On ND, scroll up and down the bank account selection modal.

Expected Result:

App will not crash.

Actual Result:

App crashes.

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

Bug6462096_1714078021885.RPReplay_Final1714077927.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0113aa04e4d3e2832e
• Upwork Job ID: 1790371872885301248
• Last Price Increase: 2024-05-14
• Automatic offers:
• Ollyws | Reviewer | 0
• bernhardoj | Contributor | 0

Issue Owner

Current Issue Owner: @abekkala

[melvin-bot]

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

[izarutskaya]

We think this issue might be related to the #vip-vsb.

[melvin-bot]

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

[abekkala]

I wasn't able to reproduce this - applying re-test weekly label

[melvin-bot]

@abekkala Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[bernhardoj]

Proposal

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

App crash when scrolling in the add personal bank account plaid bank account picker.

What is the root cause of that problem?

The crash will happen if you scroll to the "Choose an account" item. The issue started to happen after the fabric support was added to @react-native-picker/picker.

The "Choose an account" is a placeholder item with an empty value.

App/src/components/AddPlaidBankAccount.tsx

Lines 311 to 314 in 8383d80

	placeholder={{
	value: '',
	label: translate('bankAccount.chooseAnAccount'),
	}}

If the value is empty, it will be converted to nil when creating the picker items map/dict.
https://github.com/react-native-picker/picker/blob/7b7a95f4fe842d0a0c696310f9f10a8267247227/ios/RNCPickerComponentView.mm#L51

When selection the picker item, if the value is nil, it will be converted to kCFNull which will crash inside RCTStringFromNSString.
https://github.com/react-native-picker/picker/blob/7b7a95f4fe842d0a0c696310f9f10a8267247227/ios/RNCPicker.mm#L160

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

Don't convert to nil if the value is empty. So, we can replace RCTNSStringFromStringNilIfEmpty with RCTNSStringFromString. I think we can do the same for label and testID.

Then, we can remove RCTNullIfNil from here since it's not possible anymore to have a null value because we always cast the value to string.

[melvin-bot]

@abekkala 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[abekkala]

we're retesting this

[melvin-bot]

@abekkala this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot]

@abekkala Eep! 4 days overdue now. Issues have feelings too...

[bernhardoj]

It's still reproducible. You just need to select the Choose an account item.

Screen.Recording.2024-05-14.at.10.02.15.mov

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0113aa04e4d3e2832e

[melvin-bot]

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

[bernhardoj]

Upstream PR is merged. Waiting for a new picker version to be released.

[amyevans]

Nice! 🙌

[bernhardoj]

The fix has been released. PR to update the version is ready.

cc: @Ollyws @amyevans

[abekkala]

Deployed to staging May 27

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

• Fix crash when selectin the bank account picker placeholder item #42404

If no regressions arise, payment will be issued on 2024-06-05. 🎊

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

• @Ollyws requires payment automatic offer (Reviewer)
• @bernhardoj 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:

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

[abekkala]

PAYMENT SUMMARY FOR JUNE 05

• Fix: @bernhardoj [$250, if no regressions] Offer link
• PR Review: @Ollyws [$250, if no regressions] Offer link, please complete checklist above

[abekkala]

@bernhardoj payment sent and contract ended - thank you! 🎉

@Ollyws can you please complete the checklist above? Thanks!

[abekkala]

@Ollyws can you please accept the payment offer in Upwork and complete the checklist above?

[abekkala]

@Ollyws can you please accept the payment offer in Upwork and complete the checklist above so that we can close this one out?

[Ollyws]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR:

This was an upstream bug so we can't really pin this on any PR in our repo.

• 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

• 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

• Determine if we should create a regression test for this bug.

I don't think it's necessary but if we did want one the steps would be:

1. Go to Settings > Wallet
2. Press Add bank account and choose Personal bank account
3. Complete the plaid bank account step
4. In the bank account picker page, verify the initial value is the placeholder, "Choose an account"
5. Press the picker and change it to any value
6. Press the picker again and change it back to the placeholder, "Choose an account"
7. Verify the app doesn't crash

[Ollyws]

Requested payment on ND.

[abekkala]

PAYMENT SUMMARY:

• @Ollyws PR Review [$250]

[JmillsExpensify]

$250 approved for @Ollyws