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

[NT-1299] Apple Sign In Processing View dismissal bugfix #1211

Merged
merged 1 commit into from
May 29, 2020

Conversation

justinswart
Copy link
Contributor

📲 What

Fixes an bug which caused the processing view to never dismiss after logging in via Apple Sign In from the profile tab.

🤔 Why

A number of things are happening all at once here:

  • Apple Pay Sign In event completes.
  • User object is refreshed.
  • User is logged into the environment.
  • Notifications are posted for the app to update its state.
  • Processing view is dismissed.

Once the notifications are posted about the user login state having changed our RootViewModel recreates all of the VCs and sets them again on the tab bar. This was causing the LoginToutViewController to be deallocated before the observing closure could execute and hide the processing view.

One way of solving this would be to have another input on the view model to inform of the processing view having been dismissed. Another fairly simple solution is to place the posting of the notifications on the next run loop with a zero nanosecond delay. This PR implements the latter and adds a test.

🛠 How

Imposed a zero nanosecond delay on LoginToutViewModel's postNotification output to ensure that it will emit last.

✅ Acceptance criteria

  • In a logged out state, tap on the profile tab, sign in with Apple - you should not find yourself stuck with the processing view never dismissing.

@@ -31,6 +31,8 @@ final class LoginToutViewModelTests: TestCase {
override func setUp() {
super.setUp()

self.vm = LoginToutViewModel()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order for the test to pass we needed to create the VM after the scheduler is set in the environment. We've had to do this in a number of places.

Copy link
Contributor

@ifbarrera ifbarrera left a comment

Choose a reason for hiding this comment

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

💯 works like a dream

@justinswart justinswart merged commit ac370e5 into release-4.6.0 May 29, 2020
@justinswart justinswart deleted the NT-1299-apple-pay-processing-bugfix branch May 29, 2020 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants