Skip to content

Conversation

@ouchadam
Copy link
Contributor

@ouchadam ouchadam commented Sep 29, 2021

Fixes #3898

When an key info response doesn't contain a passphrase the login verification flow would still attempt to visit the passphrase screen when going back

Fixed by simply checking if a passphrase is available when going back and dismissing if no passphrase is available

  • Adds a suite of tests around the SharedSecureStorageViewModelTest, these tests also found an issue where we're posting ViewEvents as part of the ViewModel init flow, this is problematic because we're unable to register an observer before init is called (will raise a separate issue)
BEFORE AFTER
before-no-passphrase after-no-passphrase

** logging into an account which doesn't have a passphrase set

…assphrase screen

fixed by handling the back case to skip the passphrase screen if we don't have one
…te and back flow

- introduces a temporary workaorund the unit tests Mavericks by including a no op LifecycleRegistry
- manually sets instant rx schedulers via the static helpers, the layers that set the schedulers are not currently injectable
copy(
step = SharedSecureStorageViewState.Step.EnterPassphrase
)
if (state.hasPassphrase) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this block is the fix, everything else in this PR is for the unit tests

Copy link
Member

Choose a reason for hiding this comment

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

perfect, thanks!

@github-actions
Copy link

github-actions bot commented Sep 29, 2021

Unit Test Results

  42 files  ±0    42 suites  ±0   51s ⏱️ ±0s
  83 tests ±0    83 ✔️ ±0  0 💤 ±0  0 ±0 
220 runs  ±0  220 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 357d7ee. ± Comparison against base commit 357d7ee.

♻️ This comment has been updated with latest results.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for improving / setting up a better test environment.

@bmarty bmarty merged commit 357d7ee into develop Sep 29, 2021
@bmarty bmarty deleted the feature/adm/login-key-verification-flow branch September 29, 2021 13:05
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.

Wrong navigation after login

3 participants