Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue where continue button is disabled when canceling account selection #7464

Merged
merged 8 commits into from
Oct 30, 2023

Conversation

jameswoo-stripe
Copy link
Contributor

@jameswoo-stripe jameswoo-stripe commented Oct 20, 2023

Summary

  • Fix issue where continue button is disabled when canceling account selection. Use collect bank account result to refresh the screen
  • Added a browserstack test for US Bank account (only the cancel case)

Motivation

Allow users to launch bank account selection again.

Testing

  • Added tests
  • Modified tests
  • Manually verified

Changelog

  • [Fixed] Fixed a bug where a user that cancels the US Bank Account selection flow prevents the user from launching it again.

@jameswoo-stripe jameswoo-stripe requested review from a team as code owners October 20, 2023 00:03
@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch from bf2c2e5 to 329b462 Compare October 20, 2023 00:05
@jameswoo-stripe jameswoo-stripe changed the title Fix issue where pay button is disabled when canceling account selection Fix issue where continue button is disabled when canceling account selection Oct 20, 2023
samer-stripe
samer-stripe previously approved these changes Oct 20, 2023
samer-stripe
samer-stripe previously approved these changes Oct 20, 2023
@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch from 3abed5c to 0e5927c Compare October 25, 2023 18:05
@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch 3 times, most recently from 22150da to 42a2554 Compare October 25, 2023 19:30
merchantName = viewModel.formattedMerchantName(),
onPrimaryButtonClick = viewModel::handlePrimaryButtonClick,
onPrimaryButtonStateChanged = viewModel::handlePrimaryButtonStateChanged
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just call handlePrimaryButtonStateChanged(StartProcessing) from inside handlePrimaryButtonClick?

We also don’t seem to reset the processing state. Do we need to do that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping about whether or not we need to reset the state

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to, it is handled when the bank account is returned from FC SDK, by setting the isProcessing flag to false.

@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch from 42a2554 to 81f43a4 Compare October 27, 2023 18:53
@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch from 81f43a4 to 0c6357d Compare October 27, 2023 19:07
@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch from 2598cd0 to 6e9642e Compare October 27, 2023 19:13
Comment on lines 149 to 152
val currentScreenState = viewModel.currentScreenState.stateIn(viewModel.viewModelScope).value
viewModel.handlePrimaryButtonClick(currentScreenState as USBankAccountFormScreenState.VerifyWithMicrodeposits)

assertThat(awaitItem().screenState).isEqualTo(currentScreenState.copy(isProcessing = true))
assertThat(awaitItem().screenState).isEqualTo(currentScreenState)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this is testing now. We confirm that the screen state doesn‘t change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The screenstate should no longer change since we moved the isProcessing update to BillingDetails collection flow

@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch from 6e9642e to c2e0754 Compare October 27, 2023 21:41
@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch from c2e0754 to f3529d2 Compare October 27, 2023 21:43
@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch from f3529d2 to d9458d5 Compare October 27, 2023 21:43
@jameswoo-stripe jameswoo-stripe force-pushed the jameswoo/us-bank-account-cancel-fix branch from d9458d5 to 4f7ec64 Compare October 30, 2023 16:21
@jameswoo-stripe jameswoo-stripe merged commit a47ee61 into master Oct 30, 2023
6 checks passed
@jameswoo-stripe jameswoo-stripe deleted the jameswoo/us-bank-account-cancel-fix branch October 30, 2023 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants