Cht-core: Add ability to hide the action/option to create a contact

[derickl]

What feature do you want to improve?
We automatically add a contact-creation action for each contact that is parented at a place. This is great.

However there are situations where we'd like to we'd like to only show specific contact creation actions while still allowing contact creations through other means -e.g dynamically generated child docs or via scripting.

Describe alternatives you've considered
There's currently no perfect workaround for this. We've however used a full-stop as a placeholder and hid the action's icon. This is however not ideal.

Additional context

[abbyad]

Thanks for reporting the request @derickl!

It would be helpful to get more clarity on what is going on here, and what is being requested for those that may not already have the background context. Can you provide a description or diagram of the hierarchy, and what those two forms are?

[garethbowen]

Currently we have permissions for can_add_place and can_add_person but it sounds like we should have one permission per contact type so it can be shown and hidden per role. To do this we could add an additional property to the type configuration for the configurer to optionally provide a permission key string which can then be configured through the normal role configuration process either in the app settings app or medic-conf.

Alternatively, and this may be more sensible, we could respect the permissions already configurable in the xform doc. I imagine right now if you select a type for which you don't have the xform permissions it'll error out which is reasonably terrible.

@derickl Would this work for you?

[abbyad]

we could respect the permissions already configurable in the xform doc

Are the XForm doc permissions you are referring to the context.expression of App Forms? From my experience that decides if the form is shown as an option. The form can still be loaded directly via URL, so it wouldn't error when accessing it. That said, we treat Contact forms quite differently from App forms now, and we don't have properties files for Contact forms. Extending our current permissions may seem like a more natural fit unless we plan to make Contact forms concepts more similar to those of App forms.

[garethbowen]

Are the XForm doc permissions you are referring to the context.expression of App Forms?

I was talking about the context.permission property, so you could have...

  "context": {
    "permission": "can_create_family"
  }

And then give that permission only to certain roles. You're right - we don't usually apply this to contact forms but we could make that change.

The form can still be loaded directly via URL, so it wouldn't error when accessing it.

We should probably fix that so it errors if you don't have the right permission.

[derickl]

For our case, users will never create that place type from the UI. It'll be created via integrations hence the need to hide its create action.

[garethbowen]

Right, so if we added a configurable permission and you didn't give that permission to any role (except admin who has all permissions) that'd work for your use case?

[derickl]

@garethbowen

Alternatively, and this may be more sensible, we could respect the permissions already configurable in the xform doc. I imagine right now if you select a type for which you don't have the xform permissions it'll error out which is reasonably terrible.

IF we used the xform property file, then I think that could work

Right, so if we added a configurable permission and you didn't give that permission to any role (except admin who has all permissions) that'd work for your use case?

When you say admin, are you referring to the user admin or full access users? It it's the former, it could work. If the latter, not.

[garethbowen]

The couchdb admin, which on 2.x was called admin and now is called medic by convention.

[derickl]

That would work for the exact problem we have unless new requirements invalidate it. Would we have leeway to add more than one type of permission? Or is this solution mean to have one extra permission as a stop gap?

[garethbowen]

Would we have leeway to add more than one type of permission?

I'm not sure what this means.

The feature I'm proposing would mean if a user does not have the permission to complete a form then that form should not be shown in the menu. I believe this already works for Report forms, we just need to implement it the same way for Contact forms. I like this solution because it's useful for other use cases too.

[abbyad]

This sounds as though with this issue we'll be able to add the permissions directly to the form's doc in the short term, and that soon after (if not immediately) we'll also make adjustments to medic-conf to use properties files for all Contact forms too. Just highlighting that for consistency we'll want to consider consolidating how form title translations are done between the two types, which is related to https://github.com/medic/cht-core/issues/6392.

[MaxDiz]

This issue was raised as a usability issue during MoH Ke deployment testing. It's not a blocker for the deployment, but the team feels that the workaround is not ideal (UX-wise). Adding to the next roadmap planning meeting for discussion.

[dianabarsan]

The form can still be loaded directly via URL, so it wouldn't error when accessing it.

We should probably fix that so it errors if you don't have the right permission.

@garethbowen Should I do this now or create a new issue? (for contact & report forms I presume).

we'll also make adjustments to medic-conf to use properties files for all Contact forms too.

@abbyad Fortunately, no changes needed. App developers just need to create the properties files and re-upload.

we'll want to consider consolidating how form title translations are done between the two types, which is related to #6392.

Maybe this should be a different issue. Code-wise, they are entirely different.

[garethbowen]

Should I do this now or create a new issue? (for contact & report forms I presume).

Feel free to raise a separate issue - it has a completely different motive than this one (security vs ux).

Maybe this should be a different issue. Code-wise, they are entirely different.

Definitely.

[dianabarsan]

Added https://github.com/medic/cht-core/issues/6505 for the security/authorization issue.

[garethbowen]

Ready for AT in 6401-contact-forms-permissions.

Ensure that if you set a form context with an expression or permission that disallows the current user from completing that form then it's not shown as an option in the action bar.

Relevant documentation: https://docs.communityhealthtoolkit.org/apps/reference/forms/app/#formsappform_namepropertiesjson

[ngaruko]

LGTM.
Tested by adding "permission":"can_register_pregnancies" and"permission":"can_report_death" to pregnancy.properties.json and death_report.properties.json respectively (and removing expression, they seem to work in an OR fashion - not AND - which I guess is intentional @garethbowen ?)... and adding can-register_pregancies and can_report_death to permission with chw enabled.

Before

After

[dianabarsan]

@ngaruko we're restricting displaying new contact actions, not new report actions (though thanks for checking we didn't regress there!).

[abbyad]

@ngaruko do you have screenshots and the code snippets for showing/hiding the option to add contacts?

@derickl, as the person who opened the issue could you do a pass at AT to also verify that the behaviour matches what is needed?

[newtewt]

@ngaruko did you move this back so that @derickl could get a chance to look at this? Just wondering what happened to make it go backwards. Thanks!

[dianabarsan]

Well, @newtewt , thanks for bringing this forward, cause it made me realize I forgot to handle LHS actionbar :) Pushed a change on the branch to fix.

[derickl]

Passes my AT. I was able to show/hide a contact form by playing around with random permission strings.

An added benefit of this code is that https://github.com/medic/cht-core/pull/6501/files#diff-02c4e250ad1c6e44de8d5949f51961eaR236-R237 prevents showing the create option for non-existent forms.

Apologies for taking this long to get back to folks.

[ngaruko]

Thanks @derickl

[garethbowen]

Merged!