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

Supervisor CHW Create

Open mrjones-plip opened this issue 2 years ago โ€ข 1 comments

What feature do you want to improve? Right now there is no way to for an offline user, like a supervisor, to create another user, like a CHW. Instead they need to reach out to an CHT admin, an online user, to create the account for them. This can often take a while and leaves the supervisor with a device and a CHW that are prevented from giving care to the area's patients.

Describe the improvement you'd like It would be great if supervisors, using an authentication code, were allowed to create a CHW user in an area. This would cut out a lot of the delay in the current flow.

Describe alternatives you've considered NA

Additional context This is similar to the offline replace feature #7703, but differs in that the supervisor would use their own device and the new CHW would get a new device provisioned for them.

mrjones-plip avatar Aug 30 '22 21:08 mrjones-plip

@m5r and @jkuester (and @tatilepizs ) - here's the story map we reviewed today inline (and original - private to Medic):

image

mrjones-plip avatar Aug 30 '22 21:08 mrjones-plip

See recent slack discussion, but some updates are:

  • This will live under the "New person" button, not the "add report" button
  • There will be no sub-menu under "new person" (eg "create CHW"), instead existing contact form(s) will need to be modified to have an optional yes_no field for "Create user?" as well any fields needed by the sentinel transition.
  • For default config, the logic will be something like if (role == 'chw') { create_user(); }
  • For more advanced configs with >10 contact forms, they can update the forms have user created implicitly if they want
  • If anyone needs extra permissions on top of their existing hierarchy, they can use the "Enter admin PIN" trick we did in offline replace

image

mrjones-plip avatar Dec 16 '22 17:12 mrjones-plip

After considerable discussion with @m5r and rolling this over a bunch in my mind today here are some scattered design thoughts that I have:

For creating a user for a contact, I think we should (as discussed) use a trigger field that the form can set on the contact that will trigger the creation of the user. I recommend we use: create_user_for_contact = true. When the webapp sees this field on a contact, it can use the create-user-for-contacts service to add a new object to the array on the contact: user_for_contact.create = [{ status: 'READY' }]. (This should work when adding or editing a person. AND it should support adding multiple users for the same contact over time.) Then the webapp should remove the create_user_for_contact field from the contact doc so no additional users are triggered the next time the contact is edited.

I see two options for where to put the code to handle this trigger:

  1. Put it in the contact-save service. Pro is that it should be pretty simple to add here.

  2. Put it in the create-user-for-contacts client-side transition. Pros: It is consistent with where we put the replace logic. It will only get run when the create-user-for-contacts transition is enabled. It should support adding users for contacts created/edited via contact forms and also contacts created by app forms (the contact-save service approach would not work with app forms)! The con is that we currently do not support running client-side transitions for online users, but that is something we want to add support for anyway....

One final question I want to raise is if we need any kind of constraints around this functionality. For replace you can only trigger it for the currently logged-in user and it only works with a new contact that is in the same place as the current user's contact. However, for this new create functionality, we don't have any constraints built in yet. So, if the user has permissions to add/edit a person contact and the form let them set the create_user_for_contact = true it can automatically trigger a new user to be created with whatever roles the contact has (and a login link will be sent to whatever phone number is configured for the contact.....). Are we making it too easy to create users? :)

jkuester avatar Dec 20 '22 23:12 jkuester

e2e Test cases to add

create-user-for-contacts.wdio-spec

  • ~~user replace:~~
    • ~~Update does not create a new user when the replace_user form is submitted for online user to assert the user is replaced for an online user~~
    • ~~for an offline user:~~
      • ~~Move the following tests out of the for an offline user block since they do not depend on the user actually being offline:~~
        • ~~does not assign new person as primary contact of parent place if original person was not primary~~
        • ~~creates new user when replace_user form is submitted for contact associated with multiple users~~
        • ~~does not create a new user or re-parent reports when the transition is disabled~~
  • user add:
    • add person:
      • [x] creates a new user when adding a CHW as an offline user -> Tatiana
      • [x] creates a new user when adding a CHW Supervisor as an online user -> Tatiana
      • [x] creates a new user when adding a person while adding a place -> Tatiana
      • [x] creates a new user when adding a person in an app form
      • [x] does not create a new user when the transition is disabled -> Tatiana
      • [x] does not create a new user when the transition fails
      • [x] does not create a new user when editing a person
    • ~~edit person:~~
      • ~~creates additional user for person already associated with an existing user~~
      • ~~only creates one user for the first version of a contact to sync and additional conflicting requests to add are ignored~~

jkuester avatar Jan 05 '23 22:01 jkuester

@jkuester - thanks so much for the write up!

I see two options for where to put the code to handle this trigger

My vote is "Put it in the create-user-for-contacts client-side transition". Let's get it out in a FR as quick as can be so partners can assess it.

Are we making it too easy to create users?

Nope! Deployments can:

  1. Not enable the feature (default)
  2. Put a field that requires a PIN to submit it
  3. Train end users on how to use it

We can be sure to add these three points to the docs!

e2e Test cases to add

These should not block the FR - I don't think they are, but just being explicit ;)

mrjones-plip avatar Jan 05 '23 23:01 mrjones-plip

FR is out! 4.1.0-FR-supervisor-chw-create-beta.2 has been published (the only catch is that I am not sure exactly how to install a FR on 4.x) Regardless, you can still install the same code on a 4.x instance by just upgrading to the 4.1.0-FR-supervisor-chw-create branch build (which has also been published).

@tatilepizs this should be ready for some testing now! You can follow the Usage steps that I put in the issue description to trigger the desired behavior. Also, I added a comment earlier with some initial thoughts on the e2e tests that we need. Feel free to add/modify that list as you see fit. I think we are good to branch off of 7753-supervisor-chw-create as the current "stable" branch which we can target with test changes. Feel free to add your name next to some of the e2e test changes and start working on those when you get the chance! I will do the same as I have time.

jkuester avatar Jan 06 '23 00:01 jkuester

@Tatiana et al. - As a stop gap solution, you can totally just use Docker Helper and change your compose file by editing the 5 instances of string: public.ecr.aws/s5s3h4s7 in ~/.medic/cht-docker/PROJECT_NAME/compose/cht-core.yml . You need to replace the last section after the : to have the new tag of 4.1.0-4.1.0-FR-supervisor-chw-create-beta.4 .

so the healthcheck: service, for example, would look like this:

  healthcheck:
    image: public.ecr.aws/s5s3h4s7/cht-haproxy-healthcheck:4.1.0-4.1.0-FR-supervisor-chw-create-beta.4
    restart: always

After that - Do a stop and then start it and away you go!

$ ./cht-docker-compose.sh another_one.env stop
Stopping project "another_one"... done 

$ ./cht-docker-compose.sh another_one.env     

[+] Running 1/1
 โ ฟ Container another_one-dir-cht-upgrade-service-1  Started                                                                                                                                   0.4s
Starting project "another_one". First run takes a while. Will try for up to five minutes.......

  Success! "another_one" is set up:

Anything is easier than running the full dev stack :upside_down_face:

mrjones-plip avatar Jan 06 '23 01:01 mrjones-plip

@n-orlowski @tatilepizs I would appreciate your thoughts on whether we should support adding a user to an existing contact (or if we should limit the functionality to just support adding a user when adding a new contact)? Both are possible and not too hard to implement from a technical perspective.

The main consideration I have if we want to support adding a user while editing a contact is how the UX should look for that. Should it be a question in the edit-person contact form ("Do you want to add a user?")? That seems a little confusing. Should we support triggering the add-user functionality from an app form so that projects could create an "Add User" form where they pick an existing contact to add a user for? Other ideas?

If we choose to just support adding a user for a new contact, I do not think we are going to end up coding into a corner where it would be really hard to expand the functionality in the future. We could release an MVP of this functionality that only supports adding a user for new contacts, but then later we could come back and expand the functionality to support adding users to an existing contact if that is requested.....

jkuester avatar Jan 06 '23 16:01 jkuester

whether we should support adding a user to an existing contact (or if we should limit the functionality to just support adding a user when adding a new contact)? Both are possible and not too hard to implement from a technical perspective.

Great questions - Let's think on this next week!

As it stands, current functionality is that it both creates a user when creating a contact and create a user when editing a contact. In testing, I ended up with 2 contacts and 7 users (see below ;) I think i'm leaning toward an explicit "create new user when saving" or something and just let the user decide. Amazing nice to have would be showing how many users are associated to that contact, but that's out of scope!

I'd love to hear what UX and QA folks have to say (great idea to ask) and am happy to share my instance (or revive our demo instance :thinking: ). Let's figure this early next week.

image

mrjones-plip avatar Jan 07 '23 00:01 mrjones-plip

I think that it will be better to just add users for new contacts or add something specific and that the user can decide, for now. It is a little confusing to add a user when an existing contact is edited, especially if the edited information is not relevant.

I have always had questions about the roles of the contacts and users and their functionality, and maybe now is the right time to ask ๐Ÿ™‚ . I am not sure if it is important that the role of the contact and the user are the same, because what would happen if a contact is created with the CHW role and a user is created with the same role at that moment, and then the role of the contact is changed to facility manager for example, will the user still be valid? should we also update the role for the user? should we create a new user with that new role? I don't know if this is something that could really happen, or if once a contact is created it is really difficult to change their role.

Let me know if I am completely wrong, but I think that all the permissions about creating reports, for example, are based on the user role and not on the contact role, I don't know what is the usage of the contact role in the code, and if it is really important at some point.

tatilepizs avatar Jan 07 '23 02:01 tatilepizs

As it stands, current functionality is that it both creates a user when creating a contact and create a user when editing a contact. In testing

@mrjones-plip Just to be clear, this behavior is due to a bug in the current branch implementation of the person:edit form. It is not the intended behavior. But, yes, your point still stands that creating users while editing the contact can be confusing.

jkuester avatar Jan 09 '23 13:01 jkuester

@tatilepizs good questions. Thanks for helping round out the conversation here! Here are my thoughts:

I think that all the permissions about creating reports, for example, are based on the user role and not on the contact role, I don't know what is the usage of the contact role in the code

Yeah, so IMHO the current CHT behavior here is pretty nasty. AFIK all the Couch/CHT build-in permissions checking is done via the roles stored on the user doc from the _users DB. However, for projects that want to add custom role-based logic into their forms (or want to base form accessibility in the form properties context expression) on the role of the current user they do not have access to the doc from the _users DB in these places. Instead, the user's contact doc (from the medic DB) is available. So, the typical practice (illustrated by our default config) is to duplicate the user's role data into the contact doc as well.

what would happen if a contact is created with the CHW role and a user is created with the same role at that moment, and then the role of the contact is changed to facility manager for example, will the user still be valid?

I think we would consider the user configuration to be "inconsistent" (kind of "invalid" but not completely broken). The data in the _users doc will be considered the source of truth for all the Couch/CHT logic, but custom logic in the forms might not work as intended if the contact doc has a different role.

should we also update the role for the user?

I think this would be good functionality to consider adding in the future, but is out of the scope of this current project. (It is also worth noting that this issue of the _user and contact docs getting out of sync is an existing problem and not something being introduced by these changes.)

should we create a new user with that new role?

No, I doubt this is what we would want to do. If we want to support "automatically" adding a new user for an existing contact, it seems like it needs to happen only in a very specific set of circumstances which would not be surprising to the user.....

jkuester avatar Jan 09 '23 14:01 jkuester

Perfect @jkuester, thank you for all the details, it is so much clearer now ๐Ÿ™‚

tatilepizs avatar Jan 09 '23 14:01 tatilepizs

@tatilepizs - yes - excellent questions to ask - I second @jkuester thanks for adding them.

@jkuester - speaking of seconding, I second all of your positions you suggested above. Excellent responses to excellent questions!

mrjones-plip avatar Jan 09 '23 16:01 mrjones-plip

For the record, in a discussion during the Allies Sync, today, we decided to move forwards with only supporting adding users to new contacts.

It is worth noting that this will simplify UX logic in the webapp (and dramatically reduce the number of e2e test cases). However, counter-intuitively, the Sentinel logic will still have to allow for adding users to existing contacts can be created offline and edited several times before being synced to the server.

jkuester avatar Jan 10 '23 16:01 jkuester

My takeaways from the design discussion today with @mrjones-plip @dianabarsan @m5r:

  • We don't want to make client-side transitions available to online users:
    • Online users have a live connection to the server database and so will pick up change from Sentinels transitions in real time as they happen.
    • Most potential applications of client-side transitions (e.g. creating users for contacts) for online users can be completely replaced by Sentinel transitions (without requiring any pre-processing in client-side transitions).
    • Reducing additional logic in webapp is good from a performance perspective. Better to push our logic flows (especially for online users) towards async non-blocking processing on the server side instead of online users trying to run a bunch of logic locally in the client.
    • Avoiding magical changes to documents that happen when trying to save them in the webapp is a good thing. There is more visibility/traceability to the work that Sentinel does.
  • We will not be adding support for replacing online users at this time
    • It is still technically feasible to do this (by adding more code to the Sentinel transition that will listen and react to replace_forms that get submitted).
    • It should not be excessively complex to implement, but will require dedicated development effort (we will not get it for free... :disappointed:). So, this will have to wait for its own project.
  • This project's goal is to update the create_user_for_contact Sentinel transition to add new users for newly added contacts:
    • The transition should listen for new person contacts with user_for_contact.add = true
    • A contact is considered to be "new" if there is no existing infodoc for it in medic-sentinel
    • No changes to webapp should be required. The create contact forms will be responsible for populating user_for_contact.add.

I will update the design in the issue description with these changes....

jkuester avatar Jan 11 '23 17:01 jkuester

@n-orlowski @tatilepizs et. al. What do you think about updating the default config forms to limit the Roles available to select based on the type of place that will be the parent for the new person contact? Currently when creating a person, you can select any role, regardless of where in the hierarchy the person is being created.

๏ฟฝHowever, we are going to be using this selected role when creating a new user for the contact. So, whenever a contact is created with the chw role, we are going to create a new chw user. Does it make sense to limit the role choices available in the form so that the user can only select the chw role when adding a person to a health_center, etc?

I think it comes down to how opinionated the default config should be. Actual production config would never let just any user create a new chw_supervisor person at any level of the hierarchy, but that is currently how the default config works. We can either leave it like it is and let new users get added anywhere in the hierarchy or we can update the config to be more opinionated...


If we want to make the config more opinionated (by limiting the possible roles available based on the level in the hierarchy) then the next question is what the role visibility should be.

However, before determining visibility, we need some mapping between the roles available when creating a new person contact and the actual roles for users in the default config.

Current contact level roles (set on the contact doc):

  • chw
  • chw_supervisor
  • nurse
  • manager
  • patient
  • other (freetext)

Current user level roles (set on the _users doc):

  • data_entry
  • analytics
  • gateway
  • program_officer
  • crfo
  • chw_supervisor
  • chw

The chw and chw_supervisor roles are pretty easy, but do we want to create users for any of the other contact roles?

Thoughts?

jkuester avatar Jan 11 '23 19:01 jkuester

I think it comes down to how opinionated the default config should be.

Great question! I think it's more important to get the feature A) well documented and B) demo-able in the simplest manor. We're still in MVP land, so it means we don't focus on the perfect implementation, especially when it comes to default config.

To this end, I think we should make no changes to the default config and leave it as is. It's blunt right now and will create all kinds of extra users, and that's OK! It's clear that this is meant to be changed.

I could see that while we're doing up the docs we might want to add a supervisor form with limited roles that slots into the existing default hierarchy to demonstrate how a more realistic deployment would use it. If this is the case, we may opt to remove the feature from the generic edit contact form, but that's a decision to be made at the time we're authoring the docs.

mrjones-plip avatar Jan 11 '23 19:01 mrjones-plip

@mrjones-plip @n-orlowski @tatilepizs how committed are we to only allowing users to be added for new contacts? It turns out it is basically impossible to know in Sentinel if a doc is new or not. So, we would either need to build that functionality into Sentinel or just commit to supporting adding users for existing contacts.

I don't think the new functionality in Sentinel would be very hard, but it is a whole new batch of work/tests/etc. Supporting adding users for existing contacts would involve minimal changes to the implementation code, but would require a number of new tests to be added to cover the new edit scenarios.... (And we might want to consider what a form for adding a user to an existing contact would look like... This question was one of the main reasons we decided not to go this way in the first place.)

jkuester avatar Feb 16 '23 22:02 jkuester

thanks for the heads up @jkuester !

Per our call just now, we decided since the amount work to add new tests to cover the scenario of user creation upon both new and existing contacts is within an order of magnitude of the same amount of work to bend sentinel to our will, we'll update sentinel to be aware if it has seen a contact before.

This means functionality will be as we intended and users are only created when contacts are created, not when they're edited.

mrjones-plip avatar Feb 17 '23 00:02 mrjones-plip

The CHT has historically had issues with misconfigured users, that had many thousands of documents, and impacted server performance significantly.

We got better at communicating limits:

  • when admins create users from the admin app, a warning is displayed when computed number of docs exceeds 10k
  • when cht conf creates users, before any of the accounts are actually created - the cht-conf user must confirm creation for accounts that exceed 10k docs
  • last line of defence, when users first log in

We wish to get better at handling users with large numbers of docs, but I'm not sure we're quite there yet.

My question is: are there any plans of making this feature aware of doc count limits and possibly decline user creation if limits are exceeded?

dianabarsan avatar Mar 03 '23 13:03 dianabarsan

@dianabarsan this is a good question! I guess there is nothing from a technical perspective that would prevent us from checking the doc count in the transition before creating the user. My main concern is that with both admin and cht-conf, the server administrator is actively involved and is prompted to figure out what to do. In the case of this feature, the feedback loop to the administrator is a lot longer. As things are, we would write the error to the errors object on the contact doc, but it would require some manual debugging to figure out why the expected user was not automatically created....

(Also, just wanted to note that I would think the user-replace workflow would be excluded from this check since we can assume if they already have a user with a ton of docs, and are trying to replace them, we should not prevent the replacement....)

@dianabarsan @mrjones-plip should I maybe at least long an issue for this and we can address it in the future? (Or is this something we need as part of this MVP?)

jkuester avatar Mar 03 '23 13:03 jkuester

@tatilepizs this feature is ready for AT (just a formality for this issue, I think)! (Assuming the preceding comment does not generate any new changes here....)

Only one thing has recently changed in the workflow here and that is around the edge-case behavior of what happens when a contact is marked both to create and to replace users at the same time. In that case, we should ignore the create, and just perform the replace. To recreate:

  • Stop Sentinel
  • Create a new chw contact (contact A) with a valid phone number (using the person-create form).
    • This will set the create flag on the new contact, but a new users will not be created since Sentinel is down.
  • Using the Admin app, manually create a new user for the contact
  • Login as the new user and submit the replace_user form to create contact B.
  • Restart Sentinel
    • Once Sentinel finishes processing the changes, it should have:
      • Deactivated the existing user associated with A
      • Created a new replacement user associated with B
      • NOT created any additional users associated with A (since the create action is ignored)

jkuester avatar Mar 03 '23 14:03 jkuester

Great news @jkuester! ๐ŸŽ‰

I will test this scenario that you mentioned in the previous comment, I am really curious. But as you said all the AT was already done during the process, this is in fact just a formality. ๐Ÿ˜„ thank you!

tatilepizs avatar Mar 03 '23 17:03 tatilepizs

Test details

Config: Default Environment: Local, branch 7753-supervisor-chw-create Platform: WebApp Browser: Chrome


This feature is working perfectly ๐ŸŽ‰

For the edge-case that is mentioned in a previous comment, here is the test:

Contact created while sentinel was stopped.

image image

User created manually.

image image

Logged in as the new user and replace_user form submitted

image image

Sentinel was restarted. The old user was deactivated and a new user associated with the replacement user was created. No additional users.

image image

The outgoing message was generated and the login link worked as expected.

image image


๐Ÿšข ๐Ÿšข

tatilepizs avatar Mar 04 '23 01:03 tatilepizs