-
Notifications
You must be signed in to change notification settings - Fork 731
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
Support entering mail in user invite screen #4051
Conversation
a567eb0
to
308ae7c
Compare
vector/src/main/java/im/vector/app/features/userdirectory/UserListViewModel.kt
Outdated
Show resolved
Hide resolved
vector/src/main/java/im/vector/app/features/userdirectory/UserListViewModel.kt
Show resolved
Hide resolved
vector/src/main/java/im/vector/app/features/userdirectory/UserListViewModel.kt
Outdated
Show resolved
Hide resolved
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.
Thanks, good work! A few remarks.
...oid/src/main/java/org/matrix/android/sdk/internal/session/identity/DefaultIdentityService.kt
Outdated
Show resolved
Hide resolved
vector/src/main/java/im/vector/app/features/userdirectory/FoundThreePidItem.kt
Outdated
Show resolved
Hide resolved
vector/src/main/java/im/vector/app/features/userdirectory/UserListController.kt
Outdated
Show resolved
Hide resolved
vector/src/main/java/im/vector/app/features/userdirectory/UserListController.kt
Outdated
Show resolved
Hide resolved
vector/src/main/java/im/vector/app/features/userdirectory/UserListController.kt
Outdated
Show resolved
Hide resolved
47e3945
to
0acf90d
Compare
vector/src/main/java/im/vector/app/features/userdirectory/UserListViewModel.kt
Outdated
Show resolved
Hide resolved
@@ -202,6 +202,8 @@ internal class DefaultIdentityService @Inject constructor( | |||
|
|||
identityStore.setUrl(urlCandidate) | |||
identityStore.setToken(token) | |||
// could we remember if it was previously given? | |||
identityStore.setUserConsent(false) |
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.
This line is not necessary, the line 203 already erase all the existing data, including the consent
@@ -202,6 +202,8 @@ internal class DefaultIdentityService @Inject constructor( | |||
|
|||
identityStore.setUrl(urlCandidate) | |||
identityStore.setToken(token) | |||
// could we remember if it was previously given? |
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.
For now we keep only one IdentityDataEntity, but yes, why not change that in the future
@@ -65,7 +65,7 @@ class DiscoverySettingsViewModel @AssistedInject constructor( | |||
setState { | |||
copy( | |||
identityServer = Success(identityServerUrl), | |||
userConsent = false | |||
userConsent = identityService.getUserConsent() |
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.
so I think this change is not necessary anymore
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.
Thanks for the update.
I think my last 3 remarks are not mandatory.
Let's merge this PR
This PR adds support to invite by mail from the user invite dialog.
If the search term is an email, then the app will try to lookup this email from the configured identity server. If a match is found the result will appear with some profile information, if no match it is still possible to tap the raw email and send the invite.
For now only email 3pid is supported (not phone numbers)
If there is no Identity Server the user will see a call to action to set it up
If the Identity server is setup but user didn't consent to use it, then a dedicated call to action is added:
Fixes #4042