-
-
Notifications
You must be signed in to change notification settings - Fork 218
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(#8877): Look up users from their facility_id
or contact_id
#8928
Conversation
b207c2b
to
3fba6d8
Compare
…act, we need to handle them gracefully
facility_id
or contact_id
@kennsippell could you give it a look and see if this would help with the user management needs raised in #8877 ? You also mentioned wanting a route to lookup a single user by username. It is not included it in this PR but there is a WIP incremental PR #8959 we could ship next if that's valuable for user management |
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 is complete from my perspective! Before merging, it would be good to get some feedback on the following:
@dianabarsan can you have a quick look, when you get the chance, at the new migration and the new views for the _users
db? I just want to be sure that is what you expected! (And we do not add migrations/views all the time so just want to double-check my work here... 🤞 ).
@m5r can you have a look at the revisions I made? The biggest change was switching from the _find
Mango query to normal Couch views. I ended up doing this for these reasons:
- Performance for the un-indexed Mango query was 14x worse than with the views with 10000 users. The performance of the Mango query was not abysmal, but it seemed like we did not have much of a reason to take the performance hit. (FTR, I did look into indexing the Mango query. That provided equivalent performance as the view, but we do not currently have a pattern of creating/maintaining Mango indexes like we do views, so more work is needed before we can deploy them).
- As far as I can tell, we do not actually use the Mango
find
call anywhere in our code right now except for the DB migrations (where custom views do not make sense). This made me reluctant to introduce the pattern here (unless this is a direction we intentionally want to move...) - I guess the
pouchdb-find
dependency is required to use that functionality and not all the consumers ofuser-management
are currently importing that dependency (e.g. sentinel). This is not a huge deal since sentinel could always pull in that package if we ever start to hit this code flow, but it added to my unease. - I found that the Mango
find
has a default25
doc limit on what it returns. We can set a higher limit, but there does not seem to be a way to go unlimited. Whenever we do usefind
we should be sure to implement paging logic so we get all the results... (Actually logged an issue because neither of the other places where we are usingPouch.find
are actually accounting for this limit either... 🤦 )
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.
Great work here! Looking really good.
I left a few small requests inline.
Co-authored-by: Diana Barsan <[email protected]>
Co-authored-by: Diana Barsan <[email protected]>
Co-authored-by: Diana Barsan <[email protected]>
Co-authored-by: Diana Barsan <[email protected]>
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.
Amazing work! Not only is this a nice feature, we're also pushing for consistency and security of data by having the contact_id stored in the user doc. Love it!
Description
Issue: #8877
Docs PR: medic/cht-docs#1318
This extends the
GET /api/v2/users
route to support passing afacility_id
and/or acontact_id
in query parameters and get back a list of users who matched. It's gated behind the samecan_view_users
permission.Code review checklist
Compose URLs
If Build CI hasn't passed, these may 404:
License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.