-
Notifications
You must be signed in to change notification settings - Fork 39
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2779 +/- ##
==========================================
+ Coverage 29.6% 64.8% +35.1%
- Complexity 658 998 +340
==========================================
Files 239 233 -6
Lines 11204 9431 -1773
Branches 1948 1713 -235
==========================================
+ Hits 3323 6116 +2793
+ Misses 7447 2232 -5215
- Partials 434 1083 +649
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -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
fd62d93
to
36b810f
Compare
36b810f
to
faebd25
Compare
@ndegwamartin This would mean that users wouldn't be able to change their pin once setup, right? |
@LZRS I get it now, if that's the use case then yeah lets implement it like it is in the new design cc @Rkareko |
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 |
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