cht-core icon indicating copy to clipboard operation
cht-core copied to clipboard

Ability to replace a CHW user without any connectivity

Open mrjones-plip opened this issue 2 years ago • 13 comments

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 the replace_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's FIRST and LAST, 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 of CHW
  • [x] SMS token login Links are sent with https://localhost instead of FQDN like https://192-168-68-26.my.local-ip.co
  • [x] If you're on beta.1 via horti and you go to the the admin UI to upgrade, beta.2 is not visible in GUI, you must use horti again. see screenshot.

Setup Steps

  • Install the latest feature release by using 3.16.0-FR-offline-user-replace-beta.4 in your horti 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 your cht-core repo (needed for next step)
  • Use cht-conf to upload the app form replace_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 the new user_replace Sentinel transition enabled in your app_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
  • 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

mrjones-plip avatar Jul 27 '22 23:07 mrjones-plip

cc @jkuester

mrjones-plip avatar Jul 27 '22 23:07 mrjones-plip

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)

mrjones-plip avatar Jul 27 '22 23:07 mrjones-plip

~~Instructions to test this today~~ Please see Setup Steps in ticket main body above.

mrjones-plip avatar Jul 29 '22 16:07 mrjones-plip

@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?

mrjones-plip avatar Jul 29 '22 16:07 mrjones-plip

@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!

mrjones-plip avatar Aug 01 '22 23:08 mrjones-plip

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 with contacts. However, I could not find a way to have the resolvedIf function that could know if the CHW had synchronized. Maybe it would be possible for sentinel to create a report that would satisfy the resolvedIf 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!

mrjones-plip avatar Aug 05 '22 18:08 mrjones-plip

@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:

mrjones-plip avatar Aug 10 '22 23:08 mrjones-plip

@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.

mrjones-plip avatar Aug 24 '22 04:08 mrjones-plip

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

mrjones-plip avatar Aug 27 '22 00:08 mrjones-plip

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 avatar Aug 29 '22 19:08 jkuester

@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 avatar Aug 29 '22 19:08 mrjones-plip

@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...).

jkuester avatar Sep 09 '22 20:09 jkuester

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!

mrjones-plip avatar Sep 13 '22 17:09 mrjones-plip

@jkuester - thanks so much for helping me validate the error I found in testing! Here's the steps I used to reproduce:

  1. stop couch container
  2. git fetch&&git checkout 7703-offline-user-replace&&git pull origin
  3. rm -rf COUCHDATADIR
  4. start couch container
  5. in 3 separate containers: grunt, grunt dev-api, grunt dev-sentinel
  6. update ./config/default/app_settings.json with everything from here, changing URL to match dev env
  7. 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  

mrjones-plip avatar Sep 29 '22 22:09 mrjones-plip

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...

jkuester avatar Sep 30 '22 02:09 jkuester

@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:

  1. Throw an error stating that you cannot replace a user for a contact associated with multiple users
    1. 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...
  2. Do not update the other users. Leave them associated with the original contact.
  3. 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?

jkuester avatar Oct 07 '22 21:10 jkuester

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:

  1. 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!).

mrjones-plip avatar Oct 07 '22 21:10 mrjones-plip

  1. 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?

jkuester avatar Oct 07 '22 21:10 jkuester

It is "sufficient that the next time these users sync they would pick up on the contact" - thanks @jkuester !

mrjones-plip avatar Oct 07 '22 21:10 mrjones-plip

I've removed this ticket from v3.17.0 and added it to v4.1.0

latin-panda avatar Oct 11 '22 02:10 latin-panda

@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

m5r avatar Oct 19 '22 14:10 m5r

:+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.

jkuester avatar Oct 19 '22 15:10 jkuester

@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?

jkuester avatar Nov 28 '22 22:11 jkuester

Yes, I can do a quick smoke test and move it to "Ready to Merge". Thanks @jkuester !

tatilepizs avatar Nov 28 '22 22:11 tatilepizs

This is a really cool feature. I was testing it and everything works as expected. 🚢 🚢

tatilepizs avatar Dec 01 '22 20:12 tatilepizs