FTUE - Fix trusting certificates during edit server flow #6873
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ouchadam
added
the
Z-FTUE
ouchadam
requested review from
a team and
fedrunov
and removed request for
a team
ouchadam
commented
Aug 18, 2022
| ?.let { startAuthenticationFlow(finalLastAction, it, serverTypeOverride = null) } | ||
| } | ||
| when (action.retryAction) { | ||
| is OnboardingAction.HomeServerChange -> handleHomeserverChange(action.retryAction, fingerprint = action.fingerprint) |
Contributor
Author
There was a problem hiding this comment.
is OnboardingAction.HomeServerChange.SelectHomeServer to is OnboardingAction.HomeServerChange is the fix, the other changes are refactors to enable unit testing the change
ouchadam
commented
Aug 18, 2022
| .build() | ||
| action.retryAction, | ||
| // Will be replaced by the task | ||
| homeServerConnectionConfigFactory.create("https://dummy.org", action.fingerprint) |
Contributor
Author
There was a problem hiding this comment.
reuses the factory to avoid directly interacting with android's Uri and making the change untestable without further refactors
- allows a retryAction to be provided to the event to avoid mutatble state within the view model along with providing a clear path of execution
- which in turn allows the android Uri to be bypassed and a unit test around the direct local certificate case added
…rtificate accept action - adds unit tests around the edit/selection cases
ouchadam
force-pushed
the
feature/adm/allow-trusting-certificates
branch
from
47f1e2e to
e948fe0
Compare
|
Kudos, SonarCloud Quality Gate passed! |
6 tasks
ouchadam
added
the
Z-NextRelease
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change
Content
EditHomeserverflow when handling certificate trustingHomeserverConfigstate in favour of recreating when needed (when trusting the certificate) which massively simplifies the unit testing setupretryActionwhen creating theCertificateError, avoids needing non persisted mutable state in the view model and reduces unit test setupMotivation and context
Fixes #6864 - Certificate trusting during the edit homeserver flow doing nothing
Screenshots / GIFs
Tests
I already have an accountflowEDITnextwithmatrix.orgselectedTested devices