However, there is no can_update_people permission. Things are somewhat complicated by the fact that the docs for can_create_people say:
Allows creation & editing of person contacts
There currently is no API endpoint for updating a person. We have existing endpoints for adding a person and for adding/updating a place. But, there is not an existing endpoint for updating a person.
Additionally, I do not see that we check any of these permissions when opening a contact form. I think all of the existing permissions are purely related to our REST endpoints. With that in mind, I am leaning towards saying that we should add a new can_update_people permission now that we are adding a new endpoint that allows for this action…
@diana@sugat (et al), would love to hear your thoughts on this!
Just by looking at the code in api/src/routing.js, I think the can_update_places permission exists because the update endpoint(/api/v1/places/:id) for places exists whereas since the update endpoint for people does not exist, the can_update_people permission does not, so the can_update_people could be created if the update endpoint is to be added.
@jkuester There seems to be a can_update_people in a few of the deployed instances’ config. If we do add the new permission in the default config, how do the instances where the can_update_people permission does not already exist get it? Given that such instances do exist.
This is a good question. My first thought is that it should be okay since the permission would apply to the new REST endpoint. Anyone who wants to use the new REST endpoint will have to configure the new permission.
However, I think it is more complicated than that. An online user logged into webapp and using cht-datasource will be making these REST calls with their user context.
One possibility is that we just grandfather in anyone with some combination of the existing permissions. Not a big fan of that, though… Another option since we are so close to 5.0.0 would be to just call it out as a breaking change and require everyone to add this permission to any user roles that should be able to update persons.
@diana do you have any context about how we have handled similar situations in the past?
add a migration to add the new permission to settings - and here we can copy the roles that are assigned to another “grandfather” permission - so add the permission for all the roles that have can_create_people for example.
@sugat@apoorvapendse, I do have some good news here in terms of further clarifying our discussion earlier today (or yesterday for both of you… ). My earlier reading of the can_update_places logic in api missed the fact that this logic here allows the user to edit the place if they have eithercan_editorcan_update_places. Additionally, the can_edit permission does control the visibility of the “Edit” button in the webapp UI.
So, I believe this answers pretty much all of our various concerns here. Namely, we can add a can_update_people permission, but when we enforce it in the api code, we also allow anyone that already has the can_edit permission to call the endpoint (even if they do not have the can_update_people permission). This gets users “grandfathered” in as Diana mentioned (without needing the extra migration step).
It is also important to note that the existing logic in api around the can_create_places and can_create_people permissions also has the same override logic which allows users with the can_edit permission. I think we should just follow this same pattern for our new create/update endpoints.
So, for the new endpoints we should verify:
The user must be an “online” user.
The user must have either the specific permission for the endpoint orcan_edit.
This is a good question. My first thought is that it should be okay since the permission would apply to the new REST endpoint. Anyone who wants to use the new REST endpoint will have to configure the new permission.
). My earlier reading of the