-
Notifications
You must be signed in to change notification settings - Fork 499
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
Feat: add a build setting flag to always show the server selection screen in login/registration flow. #7541
Conversation
a23a168
to
fd251ac
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #7541 +/- ##
===========================================
- Coverage 12.36% 12.22% -0.15%
===========================================
Files 1646 1646
Lines 163465 163517 +52
Branches 67128 67128
===========================================
- Hits 20213 19983 -230
- Misses 142589 142888 +299
+ Partials 663 646 -17
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
RiotSwiftUI/Modules/Authentication/Common/Service/MatrixSDK/AuthenticationService.swift
Outdated
Show resolved
Hide resolved
7a369ae
to
7d27935
Compare
7d27935
to
6e251b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, glad a simple solution worked for this!
// Use the homeserver defined by a provisioningLink or by the user (if none is set, the default one will be used) | ||
let homeserverAddress = authenticationService.provisioningLink?.homeserverUrl ?? authenticationService.state.homeserver.addressFromUser | ||
|
||
// Check if the user must select a server | ||
if BuildSettings.forceHomeserverSelection, homeserverAddress == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First off, good job remembering to check the provisioning link - I would have forgotten about that 😃
However we shouldn't include the addressFromUser
here as it alters the intended behaviour where navigating back to the carousel and starting again defaults to matrix.org once more. Could probably remove the let homeserverAddress
here, check the provisioning link directly in the if and then call startFlow(flow)
like before without an address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I updated it that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thanks 👍
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👏
This PR adds a flag in the BuildSettings to force the user to set a homeserver url instead of using the default one.