Skip to content

Commit

Permalink
Merge pull request #35003 from dimagi/ad/move-location-has-users-FF
Browse files Browse the repository at this point in the history
Move location 'has users' FF to be under new flag
  • Loading branch information
AddisonDunn authored Aug 27, 2024
2 parents e759fc7 + a728ee8 commit c2ad9f5
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 33 deletions.
5 changes: 3 additions & 2 deletions corehq/apps/locations/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,15 @@

class LocationSelectWidget(forms.Widget):
def __init__(self, domain, attrs=None, id='supply-point', multiselect=False, placeholder=None,
include_locations_with_no_users_allowed=True):
for_user_location_selection=False):
super(LocationSelectWidget, self).__init__(attrs)
self.domain = domain
self.id = id
self.multiselect = multiselect
self.placeholder = placeholder
url_name = 'location_search'
if not include_locations_with_no_users_allowed and toggles.LOCATION_HAS_USERS.enabled(self.domain):
if (for_user_location_selection
and toggles.USH_RESTORE_FILE_LOCATION_CASE_SYNC_RESTRICTION.enabled(self.domain)):
url_name = 'location_search_has_users_only'
self.query_url = reverse(url_name, args=[self.domain])
self.template = 'locations/manage/partials/autocomplete_select_widget.html'
Expand Down
4 changes: 0 additions & 4 deletions corehq/apps/locations/templates/locations/location_types.html
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,6 @@ <h2>{% trans "Organization Levels" %}</h2>
{% endblocktrans %}">
</span>
</th>
{% endif %}
{% if request|toggle_enabled:"LOCATION_HAS_USERS" %}
<th data-bind="visible: advanced_mode">
{% trans "Has Users" %}
<span class="hq-help-template"
Expand Down Expand Up @@ -268,8 +266,6 @@ <h2>{% trans "Organization Levels" %}</h2>
enable: view_descendants">
</select>
</td>
{% endif %}
{% if request|toggle_enabled:"LOCATION_HAS_USERS" %}
<td data-bind="visible: $parent.advanced_mode">
<input data-bind="checked: has_users_setting, disable: has_users_setting() && actually_has_users()" type="checkbox" />
</td>
Expand Down
4 changes: 2 additions & 2 deletions corehq/apps/locations/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def test_invalid_child_data(self):
with self.assertRaises(LocationConsistencyError):
self.send_request(data)

@flag_enabled('LOCATION_HAS_USERS')
@flag_enabled('USH_RESTORE_FILE_LOCATION_CASE_SYNC_RESTRICTION')
@mock.patch('corehq.apps.locations.views.does_location_type_have_users', return_value=True)
def test_invalid_remove_has_users(self, _):
loc_type1 = OTHER_DETAILS.copy()
Expand Down Expand Up @@ -203,7 +203,7 @@ def test_search_view_basic(self):
self.assertEqual(results[0]['id'], self.loc1.location_id)
self.assertEqual(results[1]['id'], self.loc2.location_id)

@flag_enabled('LOCATION_HAS_USERS')
@flag_enabled('USH_RESTORE_FILE_LOCATION_CASE_SYNC_RESTRICTION')
def test_search_view_has_users_only(self):
loc_type2 = LocationType(domain=self.domain, name='type2', code='code2')
loc_type2.has_users = False
Expand Down
2 changes: 1 addition & 1 deletion corehq/apps/locations/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
url(r'^list/$', LocationsListView.as_view(), name=LocationsListView.urlname),
url(r'^location_search/$', LocationsSearchView.as_view(), name='location_search'),
url(r'^location_search_has_users_only/$', LocationsSearchView.as_view(
include_locations_with_no_users_allowed=False), name='location_search_has_users_only'),
include_locations_with_no_users=False), name='location_search_has_users_only'),
url(r'^location_types/$', LocationTypesView.as_view(), name=LocationTypesView.urlname),
url(r'^import/$', waf_allow('XSS_BODY')(LocationImportView.as_view()), name=LocationImportView.urlname),
url(r'^import/bulk_location_upload_api/$', bulk_location_upload_api, name='bulk_location_upload_api'),
Expand Down
12 changes: 6 additions & 6 deletions corehq/apps/locations/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,9 @@ class LocationOptionsController(EmwfOptionsController):
namespace_locations = False
case_sharing_only = False

def __init__(self, *args, include_locations_with_no_users_allowed=True):
self.include_locations_with_no_users_allowed = include_locations_with_no_users_allowed
def __init__(self, *args, include_locations_with_no_users=True):
super().__init__(*args)
self.include_locations_with_no_users = include_locations_with_no_users

@property
def data_sources(self):
Expand All @@ -280,13 +280,13 @@ def data_sources(self):
@method_decorator(locations_access_required, name='dispatch')
@location_safe
class LocationsSearchView(EmwfOptionsView):
include_locations_with_no_users_allowed = True
include_locations_with_no_users = True

@property
@memoized
def options_controller(self):
return LocationOptionsController(self.request, self.domain, self.search,
include_locations_with_no_users_allowed=self.include_locations_with_no_users_allowed)
include_locations_with_no_users=self.include_locations_with_no_users)


@method_decorator(use_bootstrap5, name='dispatch')
Expand Down Expand Up @@ -434,7 +434,7 @@ def _validate_has_users_config(loc_type_payload, pk):
payload_loc_type_name_by_pk[loc_type['pk']] = loc_type['name']
if loc_type.get('code'):
payload_loc_type_code_by_pk[loc_type['pk']] = loc_type['code']
if toggles.LOCATION_HAS_USERS.enabled(self.domain):
if toggles.USH_RESTORE_FILE_LOCATION_CASE_SYNC_RESTRICTION.enabled(self.domain):
_validate_has_users_config(loc_type, pk)
names = list(payload_loc_type_name_by_pk.values())
names_are_unique = len(names) == len(set(names))
Expand Down Expand Up @@ -819,7 +819,7 @@ def products_form(self):
def users_form(self):
if not (self.can_edit_commcare_users or self.can_access_all_locations):
return None
if toggles.LOCATION_HAS_USERS.enabled(self.domain) and not self.location.location_type.has_users:
if not self.location.location_type.has_users:
return None
form = UsersAtLocationForm(
request=self.request,
Expand Down
5 changes: 2 additions & 3 deletions corehq/apps/reports/filters/controllers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from memoized import memoized

from corehq import toggles
from corehq.apps.enterprise.models import EnterprisePermissions
from corehq.apps.es import GroupES, UserES, groups
from corehq.apps.locations.models import SQLLocation
Expand Down Expand Up @@ -46,6 +45,7 @@ def __init__(self, request, domain, search, case_sharing_only=False):
self.domain = domain
self.search = search
self.case_sharing_only = case_sharing_only
self.include_locations_with_no_users = True

@property
@memoized
Expand Down Expand Up @@ -97,8 +97,7 @@ def get_locations_query(self, query):

if self.case_sharing_only:
locations = locations.filter(location_type__shares_cases=True)
if (toggles.LOCATION_HAS_USERS.enabled(self.domain)
and not self.include_locations_with_no_users_allowed):
if not self.include_locations_with_no_users:
locations = locations.filter(location_type__has_users=True)
return locations.accessible_to_user(self.domain, self.request.couch_user)

Expand Down
2 changes: 1 addition & 1 deletion corehq/apps/user_importer/tests/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ def test_cant_remove_location(self):
assert validation_result == self.validator.error_message_location_access.format(
self.locations['Suffolk'].site_code)

@flag_enabled('LOCATION_HAS_USERS')
@flag_enabled('USH_RESTORE_FILE_LOCATION_CASE_SYNC_RESTRICTION')
def test_location_not_has_users(self):
self.editable_user.reset_locations(self.domain, [self.locations['Middlesex'].location_id])
self.locations['Cambridge'].location_type.has_users = False
Expand Down
2 changes: 1 addition & 1 deletion corehq/apps/user_importer/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ def _validate_location_has_users(self, spec):
def validate_spec(self, spec):
user_access_error = self._validate_uploading_user_access(spec)
location_cannot_have_users_error = None
if toggles.LOCATION_HAS_USERS.enabled(self.domain):
if toggles.USH_RESTORE_FILE_LOCATION_CASE_SYNC_RESTRICTION.enabled(self.domain):
location_cannot_have_users_error = self._validate_location_has_users(spec)
return user_access_error or location_cannot_have_users_error

Expand Down
7 changes: 3 additions & 4 deletions corehq/apps/users/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
from corehq.const import LOADTEST_HARD_LIMIT, USER_CHANGE_VIA_WEB
from corehq.pillows.utils import MOBILE_USER_TYPE, WEB_USER_TYPE
from corehq.toggles import (
LOCATION_HAS_USERS,
TWO_STAGE_USER_PROVISIONING,
TWO_STAGE_USER_PROVISIONING_BY_SMS,
)
Expand Down Expand Up @@ -1161,7 +1160,7 @@ def __init__(self, domain: str, *args, **kwargs):
self.domain = domain
self.fields['assigned_locations'].widget = LocationSelectWidget(
self.domain, multiselect=True, id='id_assigned_locations',
include_locations_with_no_users_allowed=False
for_user_location_selection=True
)
self.fields['assigned_locations'].help_text = ExpandedMobileWorkerFilter.location_search_help
self.fields['primary_location'].widget = PrimaryLocationWidget(
Expand All @@ -1178,7 +1177,7 @@ def clean_assigned_locations(self):
locations = get_locations_from_ids(location_ids, self.domain)
except SQLLocation.DoesNotExist:
raise forms.ValidationError(_('One or more of the locations was not found.'))
if LOCATION_HAS_USERS.enabled(self.domain) and locations.filter(location_type__has_users=False).exists():
if locations.filter(location_type__has_users=False).exists():
raise forms.ValidationError(
_('One or more of the locations you specified cannot have users assigned.'))
return [location.location_id for location in locations]
Expand Down Expand Up @@ -1635,7 +1634,7 @@ def __init__(self, *args, **kwargs):
id='id_location_id',
placeholder=_("All Locations"),
attrs={'data-bind': 'value: location_id'},
include_locations_with_no_users_allowed=False
for_user_location_selection=True
)
self.fields['location_id'].widget.query_url = "{url}?show_all=true".format(
url=self.fields['location_id'].widget.query_url
Expand Down
13 changes: 4 additions & 9 deletions corehq/toggles/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2904,14 +2904,16 @@ def domain_has_privilege_from_toggle(privilege_slug, domain):

USH_RESTORE_FILE_LOCATION_CASE_SYNC_RESTRICTION = StaticToggle(
'ush_restore_file_location_case_sync_restriction',
'USH: Limit the location-owned cases that show up in a user\'s restore file',
'USH: Limit the location-owned cases in a user\'s restore file, and allow marking whether a '
'location can have users assigned or not.',
TAG_CUSTOM,
namespaces=[NAMESPACE_DOMAIN],
help_link='https://dimagi.atlassian.net/wiki/spaces/USH/pages/2252210196/Prevent+Syncing+of+Lower+Level+Locations', # noqa: E501
description="""
In the 'Organizational Level' section of location management, web admins can specify which org level to
expand to when syncing the location-owned cases included in a user's restore file. Limits cases in a user's
restore file and thus can improve performance.
restore file and thus can improve performance. Also allows marking whether a location can have users assigned
to it.
"""
)

Expand All @@ -2937,10 +2939,3 @@ def domain_has_privilege_from_toggle(privilege_slug, domain):
tag=TAG_CUSTOM,
namespaces=[NAMESPACE_DOMAIN],
)

LOCATION_HAS_USERS = StaticToggle(
slug='location_has_users',
label='USH Dev: Allows marking whether a location should have users assigned or not.',
tag=TAG_PRODUCT,
namespaces=[NAMESPACE_DOMAIN],
)

0 comments on commit c2ad9f5

Please sign in to comment.