From 2e28575a79b00e7a63b2992fa7bf01e467655601 Mon Sep 17 00:00:00 2001 From: AddisonDunn Date: Thu, 25 Jul 2024 11:21:36 -0400 Subject: [PATCH 1/9] create basic test for LocationSearchView --- corehq/apps/locations/tests/test_views.py | 51 ++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/corehq/apps/locations/tests/test_views.py b/corehq/apps/locations/tests/test_views.py index bf3f43f4ae90..3b5b0f382456 100644 --- a/corehq/apps/locations/tests/test_views.py +++ b/corehq/apps/locations/tests/test_views.py @@ -11,9 +11,12 @@ from corehq.apps.es.users import user_adapter from corehq.apps.locations.exceptions import LocationConsistencyError from corehq.apps.locations.models import LocationType +from corehq.apps.locations.tests.util import make_loc from corehq.apps.locations.views import LocationTypesView -from corehq.apps.users.models import WebUser from corehq.util.test_utils import flag_enabled +from corehq.apps.users.dbaccessors import delete_all_users +from corehq.apps.users.models import WebUser + OTHER_DETAILS = { 'expand_from': None, @@ -152,3 +155,49 @@ def test_invalid_remove_has_users(self, _): data = {'loc_types': [loc_type1, loc_type2]} with self.assertRaises(LocationConsistencyError): self.send_request(data) + + +class LocationSearchViewTest(TestCase): + @classmethod + def setUpClass(cls): + super(LocationSearchViewTest, cls).setUpClass() + delete_all_users() + cls.domain = "test-domain" + cls.project = create_domain(cls.domain) + cls.username = "request-er" + cls.password = "foobar" + cls.web_user = WebUser.create(cls.domain, cls.username, cls.password, None, None) + cls.web_user.add_domain_membership(cls.domain, is_admin=True) + cls.web_user.set_role(cls.domain, "admin") + cls.web_user.save() + cls.loc_type1 = LocationType(domain=cls.domain, name='type1', code='code1') + cls.loc_type1.save() + cls.loc1 = make_loc( + 'loc_1', type=cls.loc_type1, domain=cls.domain + ) + cls.loc2 = make_loc( + 'loc_2', type=cls.loc_type1, domain=cls.domain + ) + + def setUp(self): + self.client.login(username=self.username, password=self.password) + + @classmethod + def tearDownClass(cls): + cls.project.delete() + delete_all_users() + return super().tearDownClass() + + @mock.patch('django_prbac.decorators.has_privilege', return_value=True) + def send_request(self, url, data, _): + return self.client.get(url, {'json': json.dumps(data)}) + + def test_search_view_basic(self): + url = reverse('location_search', args=[self.domain]) + data = {'q': 'loc'} + response = self.send_request(url, data) + self.assertEqual(response.status_code, 200) + results = json.loads(response.content)['results'] + self.assertEqual(results[0]['id'], self.loc1.location_id) + self.assertEqual(results[1]['id'], self.loc2.location_id) + From 21c66e542893acee5adfa4b6f7f50914ea969693 Mon Sep 17 00:00:00 2001 From: AddisonDunn Date: Thu, 25 Jul 2024 11:24:27 -0400 Subject: [PATCH 2/9] add url for getting locations with has_users=True only --- corehq/apps/locations/tests/test_views.py | 19 +++++++++++++++++++ corehq/apps/locations/urls.py | 2 ++ corehq/apps/locations/views.py | 9 ++++++++- corehq/apps/reports/filters/controllers.py | 4 ++++ corehq/messaging/scheduling/async_handlers.py | 2 +- 5 files changed, 34 insertions(+), 2 deletions(-) diff --git a/corehq/apps/locations/tests/test_views.py b/corehq/apps/locations/tests/test_views.py index 3b5b0f382456..f09f1653dce1 100644 --- a/corehq/apps/locations/tests/test_views.py +++ b/corehq/apps/locations/tests/test_views.py @@ -201,3 +201,22 @@ 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') + def test_search_view_has_users_only(self): + loc_type2 = LocationType(domain=self.domain, name='type2', code='code2') + loc_type2.has_users = False + loc_type2.save() + self.loc3 = make_loc( + 'loc_3', type=loc_type2, domain=self.domain + ) + self.loc3 = make_loc( + 'loc_4', type=loc_type2, domain=self.domain + ) + url = reverse('location_search_has_users_only', args=[self.domain]) + data = {'q': 'loc'} + response = self.send_request(url, data) + self.assertEqual(response.status_code, 200) + results = json.loads(response.content)['results'] + self.assertEqual(len(results), 2) + self.assertEqual(results[0]['id'], self.loc1.location_id) + self.assertEqual(results[1]['id'], self.loc2.location_id) diff --git a/corehq/apps/locations/urls.py b/corehq/apps/locations/urls.py index 1e9e95c9bd55..1aa747c4875b 100644 --- a/corehq/apps/locations/urls.py +++ b/corehq/apps/locations/urls.py @@ -31,6 +31,8 @@ url(r'^$', default, name='default_locations_view'), 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'), 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_status/(?P(?:dl-)?[0-9a-fA-Z]{25,32})/$', LocationImportStatusView.as_view(), diff --git a/corehq/apps/locations/views.py b/corehq/apps/locations/views.py index 3ea7f673a109..48d151ee3600 100644 --- a/corehq/apps/locations/views.py +++ b/corehq/apps/locations/views.py @@ -260,6 +260,11 @@ class LocationOptionsController(EmwfOptionsController): namespace_locations = False case_sharing_only = False + def __init__(self, *args): + args = list(args) + self.include_locations_with_no_users_allowed = args.pop(3) + super().__init__(*args) + @property def data_sources(self): return [ @@ -270,11 +275,13 @@ def data_sources(self): @method_decorator(locations_access_required, name='dispatch') @location_safe class LocationsSearchView(EmwfOptionsView): + include_locations_with_no_users_allowed = True @property @memoized def options_controller(self): - return LocationOptionsController(self.request, self.domain, self.search) + return LocationOptionsController(self.request, self.domain, self.search, + self.include_locations_with_no_users_allowed) @method_decorator(use_bootstrap5, name='dispatch') diff --git a/corehq/apps/reports/filters/controllers.py b/corehq/apps/reports/filters/controllers.py index 44a61831e36e..3596d4d713f9 100644 --- a/corehq/apps/reports/filters/controllers.py +++ b/corehq/apps/reports/filters/controllers.py @@ -1,5 +1,6 @@ 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 @@ -96,6 +97,9 @@ 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): + locations = locations.filter(location_type__has_users=True) return locations.accessible_to_user(self.domain, self.request.couch_user) def get_locations_size(self, query): diff --git a/corehq/messaging/scheduling/async_handlers.py b/corehq/messaging/scheduling/async_handlers.py index 2cbabdb7e5d7..9e080b8e1a25 100644 --- a/corehq/messaging/scheduling/async_handlers.py +++ b/corehq/messaging/scheduling/async_handlers.py @@ -72,7 +72,7 @@ def schedule_user_organization_recipients_response(self): def _get_user_organization_response(self, case_sharing_only=False): from corehq.apps.locations.views import LocationOptionsController controller = LocationOptionsController(self.request, self.request.domain, - self.data.get('searchString'), + self.data.get('searchString'), False, case_sharing_only=case_sharing_only) (count, results) = controller.get_options() return results[:10] From 2135843b344eb32d0eb748644cd27690826d4537 Mon Sep 17 00:00:00 2001 From: AddisonDunn Date: Thu, 25 Jul 2024 11:25:31 -0400 Subject: [PATCH 3/9] lint --- corehq/messaging/scheduling/async_handlers.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/corehq/messaging/scheduling/async_handlers.py b/corehq/messaging/scheduling/async_handlers.py index 9e080b8e1a25..fef5ae53178c 100644 --- a/corehq/messaging/scheduling/async_handlers.py +++ b/corehq/messaging/scheduling/async_handlers.py @@ -159,18 +159,18 @@ def sms_case_registration_owner_id_response(self): 'id': u['id'], 'text': _("User: {}").format(u['text']), } for u in users - ] + - [ + ] + + [ { 'id': g['id'], 'text': _("User Group: {}").format(g['text']), } for g in groups - ] + - [ + ] + + [ { - 'id': l['id'], - 'text': _("Organization: {}").format(l['text']), - } for l in locations + 'id': loc['id'], + 'text': _("Organization: {}").format(loc['text']), + } for loc in locations ] ) From 258745c84a2d843965b9ff6c788114c60d34233e Mon Sep 17 00:00:00 2001 From: AddisonDunn Date: Thu, 25 Jul 2024 11:29:01 -0400 Subject: [PATCH 4/9] allow filtering by has_users in location select widget --- corehq/apps/locations/forms.py | 9 +++++++-- corehq/apps/users/forms.py | 4 +++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/corehq/apps/locations/forms.py b/corehq/apps/locations/forms.py index bca2b0432d00..2cc4466a6c5a 100644 --- a/corehq/apps/locations/forms.py +++ b/corehq/apps/locations/forms.py @@ -13,6 +13,7 @@ from dimagi.utils.couch.database import iter_docs +from corehq import toggles from corehq.apps.custom_data_fields.edit_entity import ( CUSTOM_DATA_FIELD_PREFIX, CustomDataEditor, @@ -43,13 +44,17 @@ class LocationSelectWidget(forms.Widget): - def __init__(self, domain, attrs=None, id='supply-point', multiselect=False, placeholder=None): + def __init__(self, domain, attrs=None, id='supply-point', multiselect=False, placeholder=None, + include_locations_with_no_users_allowed=True): super(LocationSelectWidget, self).__init__(attrs) self.domain = domain self.id = id self.multiselect = multiselect self.placeholder = placeholder - self.query_url = reverse('location_search', args=[self.domain]) + url_name = 'location_search' + if not include_locations_with_no_users_allowed and toggles.LOCATION_HAS_USERS.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' def render(self, name, value, attrs=None, renderer=None): diff --git a/corehq/apps/users/forms.py b/corehq/apps/users/forms.py index 30726560bd65..23792e1250e0 100644 --- a/corehq/apps/users/forms.py +++ b/corehq/apps/users/forms.py @@ -1159,7 +1159,8 @@ def __init__(self, domain: str, *args, **kwargs): super(BaseLocationForm, self).__init__(*args, **kwargs) self.domain = domain self.fields['assigned_locations'].widget = LocationSelectWidget( - self.domain, multiselect=True, id='id_assigned_locations' + self.domain, multiselect=True, id='id_assigned_locations', + include_locations_with_no_users_allowed=False ) self.fields['assigned_locations'].help_text = ExpandedMobileWorkerFilter.location_search_help self.fields['primary_location'].widget = PrimaryLocationWidget( @@ -1631,6 +1632,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 ) self.fields['location_id'].widget.query_url = "{url}?show_all=true".format( url=self.fields['location_id'].widget.query_url From e7bec3b6cb205759d056a1db1afe14c3636c1733 Mon Sep 17 00:00:00 2001 From: AddisonDunn Date: Thu, 25 Jul 2024 12:19:23 -0400 Subject: [PATCH 5/9] change form name to clarify it is for selecting locations for a user --- corehq/apps/registration/forms.py | 4 ++-- corehq/apps/users/forms.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/corehq/apps/registration/forms.py b/corehq/apps/registration/forms.py index e7a141d4d030..34512537bc4e 100644 --- a/corehq/apps/registration/forms.py +++ b/corehq/apps/registration/forms.py @@ -22,7 +22,7 @@ from corehq.apps.domain.models import Domain from corehq.apps.hqwebapp import crispy as hqcrispy from corehq.apps.programs.models import Program -from corehq.apps.users.forms import BaseLocationForm, BaseTableauUserForm +from corehq.apps.users.forms import SelectUserLocationForm, BaseTableauUserForm from corehq.apps.users.models import CouchUser @@ -483,7 +483,7 @@ def clean_email(self): return "" -class AdminInvitesUserForm(BaseLocationForm): +class AdminInvitesUserForm(SelectUserLocationForm): email = forms.EmailField(label="Email Address", max_length=User._meta.get_field('email').max_length) role = forms.ChoiceField(choices=(), label="Project Role") diff --git a/corehq/apps/users/forms.py b/corehq/apps/users/forms.py index 23792e1250e0..a92a97c84a9b 100644 --- a/corehq/apps/users/forms.py +++ b/corehq/apps/users/forms.py @@ -1141,7 +1141,7 @@ def render(self, name, value, attrs=None, renderer=None): }) -class BaseLocationForm(forms.Form): +class SelectUserLocationForm(forms.Form): assigned_locations = forms.CharField( label=gettext_noop("Locations"), required=False, @@ -1156,7 +1156,7 @@ class BaseLocationForm(forms.Form): def __init__(self, domain: str, *args, **kwargs): from corehq.apps.locations.forms import LocationSelectWidget self.request = kwargs.pop('request') - super(BaseLocationForm, self).__init__(*args, **kwargs) + super(SelectUserLocationForm, self).__init__(*args, **kwargs) self.domain = domain self.fields['assigned_locations'].widget = LocationSelectWidget( self.domain, multiselect=True, id='id_assigned_locations', @@ -1186,7 +1186,7 @@ def _user_has_permission_to_access_locations(self, new_location_ids): self.domain, self.request.couch_user)) def clean(self): - self.cleaned_data = super(BaseLocationForm, self).clean() + self.cleaned_data = super(SelectUserLocationForm, self).clean() primary_location_id = self.cleaned_data['primary_location'] assigned_location_ids = self.cleaned_data.get('assigned_locations', []) @@ -1210,7 +1210,7 @@ def clean(self): return self.cleaned_data -class CommtrackUserForm(BaseLocationForm): +class CommtrackUserForm(SelectUserLocationForm): program_id = forms.ChoiceField( label=gettext_noop("Program"), choices=(), From 238bc7a14cc5a1af65b2a4186dd69141de1136d2 Mon Sep 17 00:00:00 2001 From: AddisonDunn Date: Thu, 25 Jul 2024 12:27:27 -0400 Subject: [PATCH 6/9] backend validation that locations can have users --- corehq/apps/users/forms.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/corehq/apps/users/forms.py b/corehq/apps/users/forms.py index a92a97c84a9b..01e2d1d7abdf 100644 --- a/corehq/apps/users/forms.py +++ b/corehq/apps/users/forms.py @@ -58,6 +58,7 @@ 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, ) @@ -1177,7 +1178,9 @@ 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(): + raise forms.ValidationError( + _('One or more of the locations you specified cannot have users assigned.')) return [location.location_id for location in locations] def _user_has_permission_to_access_locations(self, new_location_ids): From c63458eaefce8e1297222a0d717b7cae2c367890 Mon Sep 17 00:00:00 2001 From: AddisonDunn Date: Mon, 29 Jul 2024 15:27:17 -0400 Subject: [PATCH 7/9] make arg a kwarg, its simpler --- corehq/apps/locations/views.py | 7 +++---- corehq/messaging/scheduling/async_handlers.py | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/corehq/apps/locations/views.py b/corehq/apps/locations/views.py index 48d151ee3600..da134fa2514d 100644 --- a/corehq/apps/locations/views.py +++ b/corehq/apps/locations/views.py @@ -260,9 +260,8 @@ class LocationOptionsController(EmwfOptionsController): namespace_locations = False case_sharing_only = False - def __init__(self, *args): - args = list(args) - self.include_locations_with_no_users_allowed = args.pop(3) + def __init__(self, *args, include_locations_with_no_users_allowed=True): + self.include_locations_with_no_users_allowed = include_locations_with_no_users_allowed super().__init__(*args) @property @@ -281,7 +280,7 @@ class LocationsSearchView(EmwfOptionsView): @memoized def options_controller(self): return LocationOptionsController(self.request, self.domain, self.search, - self.include_locations_with_no_users_allowed) + include_locations_with_no_users_allowed=self.include_locations_with_no_users_allowed) @method_decorator(use_bootstrap5, name='dispatch') diff --git a/corehq/messaging/scheduling/async_handlers.py b/corehq/messaging/scheduling/async_handlers.py index fef5ae53178c..cd27eb2db3bf 100644 --- a/corehq/messaging/scheduling/async_handlers.py +++ b/corehq/messaging/scheduling/async_handlers.py @@ -72,7 +72,7 @@ def schedule_user_organization_recipients_response(self): def _get_user_organization_response(self, case_sharing_only=False): from corehq.apps.locations.views import LocationOptionsController controller = LocationOptionsController(self.request, self.request.domain, - self.data.get('searchString'), False, + self.data.get('searchString'), case_sharing_only=case_sharing_only) (count, results) = controller.get_options() return results[:10] From 86f906a63893fdfcb068621d0c7ff0e3f8c561bd Mon Sep 17 00:00:00 2001 From: AddisonDunn Date: Tue, 30 Jul 2024 10:30:15 -0400 Subject: [PATCH 8/9] make test name the exact same as view name --- corehq/apps/locations/tests/test_views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/corehq/apps/locations/tests/test_views.py b/corehq/apps/locations/tests/test_views.py index f09f1653dce1..e978b5c7b1f1 100644 --- a/corehq/apps/locations/tests/test_views.py +++ b/corehq/apps/locations/tests/test_views.py @@ -157,10 +157,10 @@ def test_invalid_remove_has_users(self, _): self.send_request(data) -class LocationSearchViewTest(TestCase): +class LocationsSearchViewTest(TestCase): @classmethod def setUpClass(cls): - super(LocationSearchViewTest, cls).setUpClass() + super(LocationsSearchViewTest, cls).setUpClass() delete_all_users() cls.domain = "test-domain" cls.project = create_domain(cls.domain) From 9bf39ed50f465289825263dbcca02b7272d30aee Mon Sep 17 00:00:00 2001 From: AddisonDunn Date: Tue, 30 Jul 2024 14:51:45 -0400 Subject: [PATCH 9/9] lint --- corehq/apps/locations/tests/test_views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/corehq/apps/locations/tests/test_views.py b/corehq/apps/locations/tests/test_views.py index 43166b9e344b..516bf9128d75 100644 --- a/corehq/apps/locations/tests/test_views.py +++ b/corehq/apps/locations/tests/test_views.py @@ -21,7 +21,6 @@ from corehq.util.workbook_json.excel import WorkbookJSONError - OTHER_DETAILS = { 'expand_from': None, 'expand_to': None,