-
Notifications
You must be signed in to change notification settings - Fork 59
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 login for multiple users #2779
base: main
Are you sure you want to change the base?
Conversation
@@ -29,7 +29,7 @@ data class ApplicationConfiguration( | |||
val languages: List<String> = listOf("en"), | |||
val useDarkTheme: Boolean = false, | |||
val syncInterval: Long = 15, | |||
val syncStrategies: List<String> = listOf(), | |||
val syncStrategy: List<String> = listOf(), |
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.
since this is a list I think as is as a plural makes sense
if (value is PractitionerDetails) { | ||
Timber.e(it) | ||
} |
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.
guessing this is temporary for debugging?
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.
Yeah it's just for debugging
9be6b93
to
e121c22
Compare
caffff1
to
ac7c05f
Compare
8dafdc7
to
4413416
Compare
@LZRS is this ready for review? |
It can be reviewed but there are still some code changes pending, as we look to consult further (with tpms) on how/what user roles to check for and also update tests |
Currently, only supporting multiple user logins when online. Will be adding support for offline as well in subsequent sessions, as we continue to discuss on how to effectively handle the pin login usecase |
74fea1b
to
81e8fb2
Compare
81e8fb2
to
39ac63e
Compare
39ac63e
to
6a6904a
Compare
8e67b8c
to
a029ab6
Compare
@LZRS this ready for review? |
Yeah, it's ready for an initial review although we're still deliberating on how to go about pin login. For the suggested workflows of logging out and logging in to change user, and allowing selection of user from dropdown from pin login page, we're still not certain how to handle/support a user resetting their pin |
Is it a requirement to support switching of users using pin login instead of the normal login? Also on the same, will we need to support users resetting the pins by going back to the login page and re-entering password? @ndegwamartin @dubdabasoduba |
Had a discussion on this and agreed on allowing user to continue using navigation back to login to reset a their pin and also support dropdown selection in pin login screen for switching between users when offline (Users in the dropdown are to be listed through their names instead of usernames, for some security). |
232f6e5
to
d6948aa
Compare
@HenryRae cc @rowo Concerning login for multiple users there is need for user selection when performing login using a PIN. We are seeking your guidance on the UI for the user selection widget in the Pin Login screen. As shown above it has been implemented as a drop down menu. Is this sufficient? Could you provide mock ups for how best to implement this. |
@Rkareko my concern with this approach is: When user logs out, ideally they should only be able to access the app when they log back in the app using username & password. Not with just PIN Proposal: We could change login with password to login with PIN. That way, when one logs out, and reselects their username from the list, they just enter PIN. Cc: @rowo @LZRS @DebbieArita |
IMPORTANT: Where possible all PRs must be linked to a Github issue
Fixes #2330
Engineer Checklist
strings.xml
file./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the project's style guideCode Reviewer Checklist
strings.xml
file