Create User Invitation API #35575
Create User Invitation API #35575
Conversation
There is a slight difference in how bulk upload and the API works. For bulk upload, the existing
user information is typically included. So if the field is required, then the existing value is expected
to be included in the bulk upload. If the field is either empty ("''") or does not exist (None), validation
will failed as "the field is required" for bulk upload. However for the API, if it is a PUT and the field is not
included (None), then that should not fail the required validation.
…ocation is changed. Separate this out from User Access
…rs between post and put
…e to be explicit that primary location is one of the assigned locations
… function call so can not be cached by only the domain
dimagimon
added
Risk: High
Jtang-1
force-pushed
the
jt/create_user_invitation_api
branch
from
f6346e7 to
dcf7e2d
Compare
Jtang-1
requested review from
a team,
MartinRiese,
AddisonDunn and
Robert-Costello
and removed request for
a team
…ed within the profile
…r create the object
… validation can be reused for API
…he rationale is because the sitecode value can change
…o extract that validation out to be independent
This is to allow the api parameter keys to be decoupled from the validation key names
|
Been making a bunch of changes... but I should be done now. Bumping for review |
| def obj_create(self, bundle, **kwargs): | ||
| domain = kwargs['domain'] | ||
| validator = WebUserResourceValidator(domain, bundle.request.couch_user) | ||
| spec = WebUserSpec( |
There was a problem hiding this comment.
To avoid enumerating the fields again here, I think you can just do this:
spec = WebUserSpec(**bundle.data)That will throw an exception if any unrecognized fields are included (I'm not sure whether tastypie cleans or prevents unrecognized fields). You could also do something like this if you do need explicit handling:
allowed_fields = [field.name for field in dataclasses.fields(WebUserSpec)]
spec = WebUserSpec(**{k: v for k, v in bundle.data.items() if k in allowed_fields})
has_unrecognized = any(k not in allowed_fields for k in bundle.data)There was a problem hiding this comment.
It makes sense to use the unpacking operator here since the WebUserSpec key matches the TastyPie field names. But I purposely introduced the WebUserSpec to decouple the key names used for validation and the TastyPie field names. This came in handy in the followup PR here
Although I still need to directly reference the Tastypie field names when doing validate_parameters...
| assigned_locs = SQLLocation.active_objects.filter( | ||
| location_id__in=spec.assigned_location_ids, domain=domain) | ||
| real_ids = [loc.location_id for loc in assigned_locs] | ||
|
|
||
| if missing_ids := set(spec.assigned_location_ids) - set(real_ids): | ||
| raise ImmediateHttpResponse(JsonResponse( |
There was a problem hiding this comment.
Could the location validation stuff go in the validator class?
There was a problem hiding this comment.
Yes it can, I decided to put it here to avoid having to fetch the location objects again. But maybe that extra fetch isn't a big deal, would definitely be cleaner to do the validation inside the validator class
There was a problem hiding this comment.
Ah, I definitely do support minimizing lookups, especially for an API, which can easily get spammed heavily. The validator code already looks up locations, though I think it's pretty far deep - not sure how straightforward it'd be to get access to it - it's inside validate_assigned_locations_allow_users
1. avoid pulling whole objects from DB and fetch just location ids 2. primary_location location id can be accessed directly via primary_location_id 3. change "errors" to "error" since "errors" is used elsewhere as a list
dimagimon
added
the
reindex/migration
Jtang-1
removed
the
reindex/migration
Product Description
Adds the
POSTendpointhttps://www.commcarehq.org/a/[domain]/api/invitation/v1/to create web user invitation.Technical Summary
USH-5183
Brief tech spec doc
The supported parameters are: email, role, primary_location, assigned_locations, profile, custom_user_data, tableau_role, tableau_groups.
In this PR, I also made some update to the validation in preparation for it needing to support both creation and updates.
Noneand an empty string""can be treated the same on creation. But when doing an update, caution needs to be done to handleNone(where no update is done to that parameter) and an empty string""(where the parameter is being set to None) differently. On update, most of the time, validation for the parameter can be skipped if it is not updated but validation needs to be done if it is being set to"".Feature Flag
no feature flag
Safety Assurance
Safety story
Tested locally. This is a completely new endpoint so does not affecting any existing features.
Automated test coverage
There exists tests for API input parameters. This PR adds a test that the endpoint successfully creates and Invitation.
QA Plan
no QA
Rollback instructions
Labels & Review