cht-core
cht-core copied to clipboard
Ability to replace a CHW user without any connectivity
What feature do you want to improve? When a CHW is going to leave a deployment, they return their phone to their supervisor. In some deployments where there is no connectivity, the supervisor and the replacement CHW then need to travel to where there is connectivity. They can then ask an admin to provision a new user and contact, get the credentials and log in. This takes time away from when the new CHW could be visiting patients (we're ignoring the fact that the new CHW needs training)
Describe the improvement you'd like We'd like a way for the supervisor to authorize and provision the new CHW to use the existing device, but in a way that doesn't require any connectivity. Further, we want a way that all records created by the new and old CHW have the correct parents for reporting purposes by the supervisor and dashboards.
Describe alternatives you've considered Either of these happen in the field:
- the new CHW can not work and needs to wait to get connectivity and new credentials
- the new CHW simply starts using the existing device while logged in as the old CHW. it is unclear to the supervisor and dashboards which CHW did what work when
Additional context We have a deployment actively asking for this.
Outstanding items on this ticket:
- [ ] Allow for configuring which forms should trigger the
user_replace
transition (and, at the same time, rename thereplace_user
form ->user_replace
) - [ ] Update default replace form to support replacing any offline user (supervisor, OCA or CHW), no matter their level. Should also enable locking it down so it is only accessible to CHW & OCA users.
- [ ] @jkuester - Remove need for re-syncing for old user to get logged out. Need to trigger logout from the client when replication is complete.
- Account for what happens if more docs are added while actively replicating
- [ ] @jkuester - Handle what to do if multiple
replace_user
reports are added (while the user is offline) before any get synced. - [ ] @jkuester - Make sure user is finished replicating before re-parenting the original user's reports during the transition
- [ ] @m5r - Resolve conflicts with
master
Fixed items on this ticket (were outstanding before):
- [x] ~~Do we need to handle cases where the name for the new contact does not contain any chars that are
[a-z]|[A-Z]
? (See the current behavior in this test where a contact with all unicode chars in their name will not have any name to append the random numbers to...~~ Won't fix - works as documented - will fix in a later ticket - [x] Usernames with
;)
(and likely others) as derived from "contact full name", cause replace to fail: no 2nd user created, no SMS sent, current user not logged out - [x] (@jkuester) Contacts with out a phone number, cause replace to fail: no 2nd user created, no SMS sent, current user IS logged out
- [x] open a PR! attach to this ticket
- [x] Fix bug where dupes of new contact and new user are created
- [x] Change new user name from
OLD_USERNAME-replacement
to concatenate new contact'sFIRST
andLAST
, replacing any illegal characters (space etc) with a-
. - [x] Looks like new user doesn't have a role as well? This should match the same role as the user they're replacing
- [x] Make logout work after 2nd CHW syncs, acknowledging it will be second sync, not first sync, where they get logged out
- [x] ~~Rename form to be something like "Replace CHW"~~ not doing - see below!
- [x] Ensure "Add person" can be used as it was before when not replacing a CHW . currently no way to add a 2nd person to hierarchy. Likely means something like adding an optional field of "replace CHW?" which then shows the previously hidden "Secret Code" field
- [x] make "Phone Number" field required on Replace CHW form (maybe tricky w/ above item?)
- [x] get build passing tests
- [x] release a proper FR of above, beta.1 FTW!!
- [x] New users can not log in b/c their role is
usertype.user
instead ofCHW
- [x] SMS token login Links are sent with
https://localhost
instead of FQDN likehttps://192-168-68-26.my.local-ip.co
- [x] If you're on
beta.1
viahorti
and you go to the the admin UI to upgrade,beta.2
is not visible in GUI, you must usehorti
again. see screenshot.
Setup Steps
- Install the latest feature release by using
3.16.0-FR-offline-user-replace-beta.4
in yourhorti
call. Note you may see errors in the command line - the upgrade should still proceed. Here is a sample call to a dev instance:COUCH_URL=https://medic:[email protected]:8443/medic horti --local --install=3.16.0-FR-offline-user-replace-beta.4
- Check out this
7703-offline-user-replace
branch in yourcht-core
repo (needed for next step) - Use
cht-conf
to upload the app formreplace_user
. Here is a sample call to a dev instance:cht --url=https://medic:[email protected]:8443 convert-app-forms upload-app-forms -- replace_user
- Enable login by SMS) on your CHT instance:
- Enable token login by adding these 4 lines to your
app_settings.json
file and upload it via cht-conf - Ensure you have set the
app_url
settings value
- Enable token login by adding these 4 lines to your
- Enable the new
user_replace
Sentinel transition enabled in yourapp_settings.json
file and upload it via cht-conf
To trigger the Offline Replace behavior
- Log into the CHT with an offline CHW account. Ensure both the user and contact have valid phone numbers.
- Go offline either via browser or airplane mode
- From the current user's contact page, submit the
replace_user
form- For the Admin Code field, input any value
- Submit any number of reports (for example new pregnancy or death report)
- Go online and sync
- This will trigger the Sentinel transition to create a new user and re-parent any documents submitted by the original user after the
replace_user
form was submitted
- This will trigger the Sentinel transition to create a new user and re-parent any documents submitted by the original user after the
- A token login message for the new user should be visible in the Admin console on the Outgoing messages page
- Original user should be logged out
cc @jkuester
Plan of attack is to use a "create contact" for that has a special authorization code in it that only the supervisor knows. For more details, see this story map (private) and this video (public)
~~Instructions to test this today~~ Please see Setup Steps in ticket main body above.
@njogz & @m5r - SOOO great to see how quickly we're able to develop the first draft of this FR. It's looking great!
Based on my testing and call with @njogz, next up is:
- See body of ticket - keeping rolling list of bugs there for check box status!
Nice to have requests are:
- Don't require log out/log in as data can be VERY slow. Some how do an offline re-parenting of documents?
- Mute/Grey out/disable the prior primary contact
- What happens if the replacement CHW has incredibly slow bandwidth. What will happen if it takes days (or weeks!) to sync?
@njogz & @m5r - really great to see the forward progress on the ticket! I've updated to outstanding to do list and moved it to the body of the ticket so it's easier to find and update. We can check in tomorrow on the Allies call!
After wet get beta.1
of this FR out and we're looking for possible improvements from @loukmantidjani:
- Have a task get created to remind the new CHW to synchronize. Looking at tasks, I see we can have a
appliesTo
work withcontacts
. However, I could not find a way to have theresolvedIf
function that could know if the CHW had synchronized. Maybe it would be possible for sentinel to create a report that would satisfy theresolvedIf
so after the CHW syncs, the task will go away? - Remove the "Phone Number" field from the new contact form. This will always be the same phone number as the device you're literally holding. Less entry and less possibility for errors to just have the current users phone number populate the new user and new contact!
@m5r - testing as of b5c50ee806ae1029126d5c63b323dac2087a9dc7 shows that we create both a bunch of duplicate contacts, but also a bunch of duplicate users. I've unchecked this for now above:
Fix bug where dupe of new contact is created
Sorry if I'm catching this at an unhelpful time and you're like "I knooooooww this!" :sweat_smile:
@m5r - looking really great! The replacement user can now log in as their user type is correct (CHW). As well, we have a real live FR! We're down to two outstanding items to fix on the list and I think the dupe user is likely the more important one.
3.16.0-FR-offline-user-replace-beta.2
has shipped! I've tested it and found a few minor bugs. ticket body is updated!
I've also pushed the beta to our test instance, now with an awkward name: https://demo-bulk-upload-fr.dev.medicmobile.org
Contacts with out a phone number, cause replace to fail: no 2nd user created, no SMS sent, current user IS logged out
@mrjones-plip I am planning to look into this, but could use some clarification as to the details... Am I correct in thinking that this pertains to the issue where the user can have a phone
configured on both their contact doc and? IIRC the current code is not super consistent about which one gets used where and all that....
@jkuester correct! This is the case where the old contact does not have a phone number. Here's a video:
https://user-images.githubusercontent.com/8253488/187285038-78ce50be-ced6-4585-9d3b-721f24f72918.mp4
@mrjones-plip @m5r When it comes to re-parenting reports, we are currently updating both contact/_id
(with the id of the new CHW contact) as well as the contact/parent
object (essentially resetting the hierarchy). Currently this would technically allow for replacing a user with a new contact that is in a different place then the original contact (and then the reports would all be re-parented to the new place). However our current implementation of the replace_user
report, of course, does not support changing the place of the new contact.
My question is if it sounds reasonable to lock down the functionality on the server side so that we throw an error if the new contact from the replace_user
form actually belongs to a different place than the original contact (this could happen if the replace_user
form is edited). I believe that locking this down would considerably simplify our testing for this feature (and we could always add in support later for moving to a contact in a new place...).
My question is if it sounds reasonable to lock down the functionality on the server side
Thanks for spelling this out! My vote is as you suggested: lock it down, throw an error, make testing easier - ship it!
@jkuester - thanks so much for helping me validate the error I found in testing! Here's the steps I used to reproduce:
- stop couch container
-
git fetch&&git checkout 7703-offline-user-replace&&git pull origin
-
rm -rf COUCHDATADIR
- start couch container
- in 3 separate containers:
grunt
,grunt dev-api
,grunt dev-sentinel
- update ./config/default/app_settings.json with everything from here, changing URL to match dev env
- push app settings and form:
cht --url=https://medic:[email protected]/ upload-app-settings upload-app-forms -- replace_user
then when I go to admin area and try and create a user, I get this error. Noteworth that between steps 6 and 7, I'm able to create a user with out errors.
➜ cht-core git:(7703-offline-user-replace) ✗ grunt dev-api [2100/24978]Running "copy:api-bowser" (copy) task
Copied 1 file
Running "exec:api-dev" (exec) task
[nodemon] 2.0.13
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): api/**/* shared-libs/**/src/**
[nodemon] watching extensions: js,mjs,json
[nodemon] starting `node --inspect=0.0.0.0:9229 api/server.js --allow-cors` >> Debugger listening on ws://0.0.0.0:9229/bbd00001-2b0c-4aef-9fa1-b822329944c4
>> For help, see: https://nodejs.org/en/docs/inspector
2022-09-29 21:52:14 INFO: Running server checks…
Node Environment Options: 'undefined'
Node Version: 16.17.1 in development mode
CouchDB Version: 2.3.1
CouchDb Cluster ready
[SNIP]
RES 6cb174bf-10a2-45d4-9796-706b93317c23 ::ffff:127.0.0.1 - GET /api/v1/users-info?contact_id=761c425c-2efc-4d15-9c0f-2eff96039440&facility_id=7bebfa9f-4e8d-4acf-974b-41fac73dd903&role=chw HTTP/1.1 304 - 49.991 ms REQ 788e1bf3-0960-425e-9cfc-6f27f0b53002 ::ffff:127.0.0.1 - POST /api/v1/users/mary HTTP/1.1
2022-09-29 22:05:25 ERROR: Server error: TypeError: Cannot read properties of undefined (reading 'token_login')
at Object.get (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/transitions/src/config.js:11:28) at isTokenLoginEnabled (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/token-login.js:232:35)
at shouldEnableTokenLogin (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/token-login.js:228:40) at validateTokenLoginEdit (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/token-login.js:258:7) at Object.validateTokenLogin (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/token-login.js:272:10)
at Object.updateUser (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/users.js:972:40) at runMicrotasks (<anonymous>) at processTicksAndRejections (node:internal/process/task_queues:96:5) { [stack]: "TypeError: Cannot read properties of undefined (reading 'token_login')\n" +
' at Object.get (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/transitions/src/config.js:11:28)\n' + ' at isTokenLoginEnabled (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/token-login.js:232:35)\n' +
' at shouldEnableTokenLogin (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/token-login.js:228:40)\n' +
' at validateTokenLoginEdit (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/token-login.js:258:7)\n' +
' at Object.validateTokenLogin (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/token-login.js:272:10)\n' +
' at Object.updateUser (/home/mrjones/Documents/MedicMobile/cht-core/shared-libs/user-management/src/users.js:972:40)\n' +
' at runMicrotasks (<anonymous>)\n' +
' at processTicksAndRejections (node:internal/process/task_queues:96:5)',
[message]: "Cannot read properties of undefined (reading 'token_login')"
}
RES 788e1bf3-0960-425e-9cfc-6f27f0b53002 ::ffff:127.0.0.1 - POST /api/v1/users/mary HTTP/1.1 500 35 22.260 ms
Okay, I believe I have gotten to the bottom of the error reported by @mrjones-plip. I was able to recreate the issue and determined it only occurred when using cht-conf to run the upload-app-settings
action. To make things even better, the issue only presents when trying to run the upload-app-settings
action when there are actually no differences between the local settings and the remote....
It turns out the root cause seems to be that the api settings service initializes @medic/transitions
without passing any parameters (especially without passing the config settings). That causes invalid config to be propagated down to the @medic/user-management
lib which ultimately triggers the error.
I believe I have fixed the issue by updating the settings service to use the transitions library from the config instead of initializing it without parameters...
@mrjones-plip @m5r on the PR, @dianabarsan raised a very important point that we missed in our implementation. We had assumed that there was a 1-to-1 relationship between users and contacts such that every user was associated with a separate unique contact. However, there is nothing in the CHT that actually requires this to be the case. You can associate as many users as you want with the same contact. With that in mind, we need to decide how the system should behave if a user that is being replaced, is associated with a contact that other users are also associated with. I see 3 options:
- Throw an error stating that you cannot replace a user for a contact associated with multiple users
- The main downside of this is that there is no easy way that I know of to hide the replace form from a user associated with the same contact as other users...
- Do not update the other users. Leave them associated with the original contact.
- Update all the other users to also be associated with the new contact. (Do not delete these other users, but change the contact they are associated with).
Initially, I assumed #2
would be the proper way to go. But the more I think about this, the more I am not exactly sure what the goal of a supervisor would be when they submit a replace user report for a contact associated with multiple users... (in the past we had discussed the possibility of automatically muting the replaced contact.) Just want to make sure we know how we want this to behave before we lock in the code!
If we do go with #2
the big remaining question would be what to do if the original contact is the primary contact for their place. Should the new contact become the primary contact, or should the original contact (still associated with the other users) remain the primary contact?
Interesting! Love seeing you two do the deep dive on this to fine to tune - nice work!!
In an ideal world I think I'd do a 4th option:
- Show a "Caution: more than one user associated to this contact - proceed? (Yes) (No)"
And if they do "Yes", do option #3
above (maintains status quo). If they choose "No", do nothing.
You're right - #2
is not ideal. The main goal is unblock the supervisor who actively is missing a CHW. Getting an online CHT Admin involved is the last thing they'll want to do.
If #4
is not an option (this is still kinda an MVP!), go with option #3
. We can even put a note in the form (certainly a note in the docs!).
- Show a "Caution: more than one user associated to this contact - proceed? (Yes) (No)"
AFIK there is sadly no way, from within the form, to know that the user's contact is associated with multiple users. So, this is not an option.
Regarding #3
, would we need to retroactively re-parent reports submitted by the other users after the replace form was created (so they are associated with the new contact)? Or, is it sufficient that the next time these users sync they would pick up on the contact changes and start writing reports as the new contact?
It is "sufficient that the next time these users sync they would pick up on the contact" - thanks @jkuester !
I've removed this ticket from v3.17.0 and added it to v4.1.0
@jkuester @mrjones-plip I went ahead and implemented solution #2
because I would need a few more days to implement #3
properly with its associated tests. If that's okay, the PR is ready for review
If we really want #3
, I can dedicate some time next week to implement it
:+1: Good call, @m5r! I was thinking the exact same thing. IMHO if #2
is acceptable to everyone else for the MVP it makes sense to go with the path of least resistance.
@tatilepizs I am marking this officially ready for AT. (I believe I have got the CI build issues sorted now and it should pass as soon as Docker Hub stops failing everything with 503s...) https://github.com/medic/cht-core/tree/7703-offline-user-replace
I know you have already been looking at this, so I am not sure exactly what happens now. But I guess you will just make this as "Ready to Merge" when things are good from your perspective?
Yes, I can do a quick smoke test and move it to "Ready to Merge". Thanks @jkuester !
This is a really cool feature. I was testing it and everything works as expected. 🚢 🚢