Location Type 'has users' util function check #34820
Conversation
corehq/apps/locations/util.py
Outdated
| users = user_ids_at_locations(location_ids) | ||
| return bool(users) |
There was a problem hiding this comment.
This will fetch all IDs of matching docs, which is unnecessary - this should be a smidge more efficient:
| users = user_ids_at_locations(location_ids) | |
| return bool(users) | |
| return bool(UserES().location(location_ids).count()) |
Looks like we could do even better, but I don't think we have the wrappers for that yet.
corehq/apps/locations/views.py
Outdated
| if not loc_type.get('has_users'): | ||
| try: | ||
| existing_loc_type_sql_obj = LocationType.objects.get(pk=pk) | ||
| except ObjectDoesNotExist: # Location type not created yet |
There was a problem hiding this comment.
Looks like there's also a _is_fake_pk bit of logic that you can use to avoid looking up uncreated location types.
corehq/apps/locations/views.py
Outdated
| if not loc_type.get('has_users'): | ||
| if _is_fake_pk(pk): | ||
| return | ||
| if does_location_type_have_users(LocationType.objects.get(pk=pk)): |
There was a problem hiding this comment.
| if does_location_type_have_users(LocationType.objects.get(pk=pk)): | |
| if does_location_type_have_users(LocationType.objects.get(pk=pk, domain=domain)): | |
| `` |
There was a problem hiding this comment.
Yeah, just good practice, since the ID is untrusted input. A malicious actor could make a request with IDs from another domain (and since they're incrementing integer IDs, they're guessable). Wouldn't give them anything useful in this case, but still good to guard against. I've toyed with the idea making a domain filter required by default for all SQL models unless specifically exempted, but haven't really gone anywhere with that yet.
There was a problem hiding this comment.
Oh, I see, interesting.
Product Description
Jira ticket that's part of an epic to create a "has users" configuration option for organization levels to mark that a level has no users and should not be included in the location dropdown when assigning locations to a user.
This PR has no user-facing impact.
Technical Summary
Adds a util method that checks whether a location type has any users in it (w test), and backend validation that a location type passed in does not have any users if the "has users" setting is being turned off.
Safety Assurance
Adds feature flag, LOCATION_HAS_USERS
Safety Assurance
Safety story
All logic is behind a FF check. Checked locally to verify the org levels page could save without error when the FF is off.
Automated test coverage
Added for the util function to make sure it works.
QA Plan
QA will be done at the end of this feature's build.
Rollback instructions
Labels & Review