From 68b2362b2668155dff6485d24710012b8a2b9eb1 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Tue, 18 Jun 2024 16:56:39 +0530 Subject: [PATCH 01/20] fixes current url for case type data dict json --- corehq/apps/data_dictionary/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corehq/apps/data_dictionary/urls.py b/corehq/apps/data_dictionary/urls.py index f2326f331e0a..0c75c6ab3b22 100644 --- a/corehq/apps/data_dictionary/urls.py +++ b/corehq/apps/data_dictionary/urls.py @@ -15,7 +15,7 @@ urlpatterns = [ url(r"^json/$", data_dictionary_json, name='data_dictionary_json'), - url(r"^json/?(?P\w+)/?$", data_dictionary_json, name='case_type_dictionary_json'), + url(r"^json/(?P[\w-]+)/$", data_dictionary_json, name='case_type_dictionary_json'), url(r"^create_case_type/$", create_case_type, name='create_case_type'), url(r"^deprecate_or_restore_case_type/(?P[\w-]+)/$", deprecate_or_restore_case_type, name='deprecate_or_restore_case_type'), From 0215ce71342f7998dfca91301466c9b759fb823c Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 18:41:55 +0530 Subject: [PATCH 02/20] creates a copy of existing view for better seeing changes --- corehq/apps/data_dictionary/views.py | 88 ++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index 0431ca8dfae1..8884d2be58a9 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -143,6 +143,94 @@ def data_dictionary_json(request, domain, case_type_name=None): }) +@login_and_domain_required +@requires_privilege_with_fallback(privileges.DATA_DICTIONARY) +def data_dictionary_json_v2(request, domain, case_type_name=None): + props = [] + fhir_resource_type_name_by_case_type = {} + fhir_resource_prop_by_case_prop = {} + queryset = CaseType.objects.filter(domain=domain) + if not request.GET.get('load_deprecated_case_types', False) == 'true': + queryset = queryset.filter(is_deprecated=False) + queryset = queryset.prefetch_related( + Prefetch('groups', queryset=CasePropertyGroup.objects.order_by('index')), + # order by pk for properties with same index, likely for automatically added properties + Prefetch('properties', queryset=CaseProperty.objects.order_by('group_id', 'index', 'pk')), + Prefetch('properties__allowed_values', queryset=CasePropertyAllowedValue.objects.order_by('allowed_value')) + ) + if toggles.FHIR_INTEGRATION.enabled(domain): + fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( + domain) + if case_type_name: + queryset = queryset.filter(name=case_type_name) + + case_type_app_module_count = get_case_type_app_module_count(domain) + data_validation_enabled = toggles.CASE_IMPORT_DATA_DICTIONARY_VALIDATION.enabled(domain) + used_props_by_case_type = get_used_props_by_case_type(domain) + geo_case_prop = get_geo_case_property(domain) + for case_type in queryset: + module_count = case_type_app_module_count.get(case_type.name, 0) + used_props = used_props_by_case_type[case_type.name] if case_type.name in used_props_by_case_type else [] + p = { + "name": case_type.name, + "fhir_resource_type": fhir_resource_type_name_by_case_type.get(case_type), + "groups": [], + "is_deprecated": case_type.is_deprecated, + "module_count": module_count, + "properties": [], + "is_safe_to_delete": len(used_props) == 0, + } + grouped_properties = { + group: [ + { + 'id': prop.id, + 'description': prop.description, + 'label': prop.label, + 'fhir_resource_prop_path': fhir_resource_prop_by_case_prop.get( + prop + ), + 'name': prop.name, + 'deprecated': prop.deprecated, + 'is_safe_to_delete': prop.name not in used_props and prop.name != geo_case_prop, + } + | ( + { + 'data_type': prop.data_type, + 'allowed_values': { + av.allowed_value: av.description + for av in prop.allowed_values.all() + }, + } + if data_validation_enabled + else {} + ) + for prop in props + ] + for group, props in itertools.groupby( + case_type.properties.all(), key=attrgetter('group_id') + ) + } + for group in case_type.groups.all(): + p["groups"].append({ + "id": group.id, + "name": group.name, + "description": group.description, + "deprecated": group.deprecated, + "properties": grouped_properties.get(group.id, []) + }) + + # Aggregate properties that don't have a group + p["groups"].append({ + "name": "", + "properties": grouped_properties.get(None, []) + }) + props.append(p) + return JsonResponse({ + 'case_types': props, + 'geo_case_property': geo_case_prop, + }) + + @login_and_domain_required @requires_privilege_with_fallback(privileges.DATA_DICTIONARY) @require_permission(HqPermissions.edit_data_dict) From 384263b3d074eaa824895851e1f7bdcb4e6b0e94 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 19:01:58 +0530 Subject: [PATCH 03/20] simple split for new v2 view --- corehq/apps/data_dictionary/urls.py | 8 ++- corehq/apps/data_dictionary/views.py | 89 +++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/corehq/apps/data_dictionary/urls.py b/corehq/apps/data_dictionary/urls.py index 0c75c6ab3b22..0e8e2e176056 100644 --- a/corehq/apps/data_dictionary/urls.py +++ b/corehq/apps/data_dictionary/urls.py @@ -1,10 +1,12 @@ -from django.urls import re_path as url +from django.urls import path, re_path as url from corehq.apps.data_dictionary.views import ( DataDictionaryView, ExportDataDictionaryView, UploadDataDictionaryView, data_dictionary_json, + data_dictionary_json_case_types, + data_dictionary_json_case_properties, update_case_property, update_case_property_description, create_case_type, @@ -16,6 +18,10 @@ urlpatterns = [ url(r"^json/$", data_dictionary_json, name='data_dictionary_json'), url(r"^json/(?P[\w-]+)/$", data_dictionary_json, name='case_type_dictionary_json'), + # TODO Remove _v2 in below two url and delete above two urls once javascript/template changes are done + path("json_v2/", data_dictionary_json_case_types, name='data_dictionary_json_case_types'), + path("json_v2//", data_dictionary_json_case_properties, + name='data_dictionary_json_case_properties'), url(r"^create_case_type/$", create_case_type, name='create_case_type'), url(r"^deprecate_or_restore_case_type/(?P[\w-]+)/$", deprecate_or_restore_case_type, name='deprecate_or_restore_case_type'), diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index 8884d2be58a9..8d03052bcc72 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -145,7 +145,7 @@ def data_dictionary_json(request, domain, case_type_name=None): @login_and_domain_required @requires_privilege_with_fallback(privileges.DATA_DICTIONARY) -def data_dictionary_json_v2(request, domain, case_type_name=None): +def data_dictionary_json_case_types(request, domain): props = [] fhir_resource_type_name_by_case_type = {} fhir_resource_prop_by_case_prop = {} @@ -161,8 +161,91 @@ def data_dictionary_json_v2(request, domain, case_type_name=None): if toggles.FHIR_INTEGRATION.enabled(domain): fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( domain) - if case_type_name: - queryset = queryset.filter(name=case_type_name) + + case_type_app_module_count = get_case_type_app_module_count(domain) + data_validation_enabled = toggles.CASE_IMPORT_DATA_DICTIONARY_VALIDATION.enabled(domain) + used_props_by_case_type = get_used_props_by_case_type(domain) + geo_case_prop = get_geo_case_property(domain) + for case_type in queryset: + module_count = case_type_app_module_count.get(case_type.name, 0) + used_props = used_props_by_case_type[case_type.name] if case_type.name in used_props_by_case_type else [] + p = { + "name": case_type.name, + "fhir_resource_type": fhir_resource_type_name_by_case_type.get(case_type), + "groups": [], + "is_deprecated": case_type.is_deprecated, + "module_count": module_count, + "properties": [], + "is_safe_to_delete": len(used_props) == 0, + } + grouped_properties = { + group: [ + { + 'id': prop.id, + 'description': prop.description, + 'label': prop.label, + 'fhir_resource_prop_path': fhir_resource_prop_by_case_prop.get( + prop + ), + 'name': prop.name, + 'deprecated': prop.deprecated, + 'is_safe_to_delete': prop.name not in used_props and prop.name != geo_case_prop, + } + | ( + { + 'data_type': prop.data_type, + 'allowed_values': { + av.allowed_value: av.description + for av in prop.allowed_values.all() + }, + } + if data_validation_enabled + else {} + ) + for prop in props + ] + for group, props in itertools.groupby( + case_type.properties.all(), key=attrgetter('group_id') + ) + } + for group in case_type.groups.all(): + p["groups"].append({ + "id": group.id, + "name": group.name, + "description": group.description, + "deprecated": group.deprecated, + "properties": grouped_properties.get(group.id, []) + }) + + # Aggregate properties that don't have a group + p["groups"].append({ + "name": "", + "properties": grouped_properties.get(None, []) + }) + props.append(p) + return JsonResponse({ + 'case_types': props, + 'geo_case_property': geo_case_prop, + }) + + +@login_and_domain_required +@requires_privilege_with_fallback(privileges.DATA_DICTIONARY) +def data_dictionary_json_case_properties(request, domain, case_type_name): + props = [] + fhir_resource_type_name_by_case_type = {} + fhir_resource_prop_by_case_prop = {} + queryset = CaseType.objects.filter(domain=domain) + queryset = queryset.prefetch_related( + Prefetch('groups', queryset=CasePropertyGroup.objects.order_by('index')), + # order by pk for properties with same index, likely for automatically added properties + Prefetch('properties', queryset=CaseProperty.objects.order_by('group_id', 'index', 'pk')), + Prefetch('properties__allowed_values', queryset=CasePropertyAllowedValue.objects.order_by('allowed_value')) + ) + if toggles.FHIR_INTEGRATION.enabled(domain): + fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( + domain) + queryset = queryset.filter(name=case_type_name) case_type_app_module_count = get_case_type_app_module_count(domain) data_validation_enabled = toggles.CASE_IMPORT_DATA_DICTIONARY_VALIDATION.enabled(domain) From fded18ab3534023383c6c9642d7c69ec5f7e3404 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 19:13:31 +0530 Subject: [PATCH 04/20] update response data for case types view --- corehq/apps/data_dictionary/views.py | 64 +++------------------------- 1 file changed, 6 insertions(+), 58 deletions(-) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index 8d03052bcc72..83c071e4a250 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -5,6 +5,7 @@ from django.contrib import messages from django.core.exceptions import ValidationError +from django.db.models import Count from django.db.models.query import Prefetch from django.db.transaction import atomic from django.http import HttpResponse, HttpResponseRedirect, JsonResponse @@ -146,85 +147,32 @@ def data_dictionary_json(request, domain, case_type_name=None): @login_and_domain_required @requires_privilege_with_fallback(privileges.DATA_DICTIONARY) def data_dictionary_json_case_types(request, domain): - props = [] fhir_resource_type_name_by_case_type = {} fhir_resource_prop_by_case_prop = {} - queryset = CaseType.objects.filter(domain=domain) + queryset = CaseType.objects.filter(domain=domain).annotate(properties_count=Count('property')) if not request.GET.get('load_deprecated_case_types', False) == 'true': queryset = queryset.filter(is_deprecated=False) - queryset = queryset.prefetch_related( - Prefetch('groups', queryset=CasePropertyGroup.objects.order_by('index')), - # order by pk for properties with same index, likely for automatically added properties - Prefetch('properties', queryset=CaseProperty.objects.order_by('group_id', 'index', 'pk')), - Prefetch('properties__allowed_values', queryset=CasePropertyAllowedValue.objects.order_by('allowed_value')) - ) if toggles.FHIR_INTEGRATION.enabled(domain): fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( domain) case_type_app_module_count = get_case_type_app_module_count(domain) - data_validation_enabled = toggles.CASE_IMPORT_DATA_DICTIONARY_VALIDATION.enabled(domain) used_props_by_case_type = get_used_props_by_case_type(domain) geo_case_prop = get_geo_case_property(domain) + case_types_data = [] for case_type in queryset: module_count = case_type_app_module_count.get(case_type.name, 0) used_props = used_props_by_case_type[case_type.name] if case_type.name in used_props_by_case_type else [] - p = { + case_types_data.append({ "name": case_type.name, "fhir_resource_type": fhir_resource_type_name_by_case_type.get(case_type), - "groups": [], "is_deprecated": case_type.is_deprecated, "module_count": module_count, - "properties": [], + "properties_count": case_type.properties_count, "is_safe_to_delete": len(used_props) == 0, - } - grouped_properties = { - group: [ - { - 'id': prop.id, - 'description': prop.description, - 'label': prop.label, - 'fhir_resource_prop_path': fhir_resource_prop_by_case_prop.get( - prop - ), - 'name': prop.name, - 'deprecated': prop.deprecated, - 'is_safe_to_delete': prop.name not in used_props and prop.name != geo_case_prop, - } - | ( - { - 'data_type': prop.data_type, - 'allowed_values': { - av.allowed_value: av.description - for av in prop.allowed_values.all() - }, - } - if data_validation_enabled - else {} - ) - for prop in props - ] - for group, props in itertools.groupby( - case_type.properties.all(), key=attrgetter('group_id') - ) - } - for group in case_type.groups.all(): - p["groups"].append({ - "id": group.id, - "name": group.name, - "description": group.description, - "deprecated": group.deprecated, - "properties": grouped_properties.get(group.id, []) - }) - - # Aggregate properties that don't have a group - p["groups"].append({ - "name": "", - "properties": grouped_properties.get(None, []) }) - props.append(p) return JsonResponse({ - 'case_types': props, + 'case_types': case_types_data, 'geo_case_property': geo_case_prop, }) From d0df53747618a1363ddbe160d6df27c1b8b14eae Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 19:29:41 +0530 Subject: [PATCH 05/20] test cases for new case types API --- .../apps/data_dictionary/tests/test_view.py | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/corehq/apps/data_dictionary/tests/test_view.py b/corehq/apps/data_dictionary/tests/test_view.py index fe213ff12f53..c2e1b1ff4d1f 100644 --- a/corehq/apps/data_dictionary/tests/test_view.py +++ b/corehq/apps/data_dictionary/tests/test_view.py @@ -1,4 +1,6 @@ import json +import random +import string import uuid from unittest.mock import patch @@ -213,6 +215,7 @@ def test_delete_case_property(self): self.assertIsNone(prop) +# TODO Remove this Test Class once we migrate to the new view @privilege_enabled(privileges.DATA_DICTIONARY) class DataDictionaryViewTest(TestCase): domain_name = uuid.uuid4().hex @@ -263,6 +266,129 @@ def test_no_view_access(self): self.assertEqual(response.status_code, 403) +@patch('corehq.apps.data_dictionary.views.get_case_type_app_module_count', return_value={}) +@patch('corehq.apps.data_dictionary.views.get_used_props_by_case_type', return_value={}) +@flag_enabled('CASE_IMPORT_DATA_DICTIONARY_VALIDATION') +@privilege_enabled(privileges.DATA_DICTIONARY) +class DataDictionaryJsonV2Test(TestCase): + domain_name = uuid.uuid4().hex + # TODO Replace this with the original view after migration + case_types_view_name = "data_dictionary_json_case_types" + + @classmethod + def setUpClass(cls): + super(DataDictionaryJsonV2Test, cls).setUpClass() + cls.domain = create_domain(cls.domain_name) + + cls.couch_user = WebUser.create(None, "test", "foobar", None, None) + cls.couch_user.add_domain_membership(cls.domain_name, is_admin=True) + cls.couch_user.save() + + cls.case_type_obj = CaseType.objects.create(name="case_type", domain=cls.domain_name) + cls.group_obj = CasePropertyGroup.objects.create(case_type=cls.case_type_obj, name="group") + cls.case_properties_with_group = cls._create_properties_for_case_type( + case_type=cls.case_type_obj, + properties_count=2, + group=cls.group_obj + ) + cls.case_properties_without_group = cls._create_properties_for_case_type( + case_type=cls.case_type_obj, + properties_count=2, + ) + + cls.deprecated_case_type_obj = CaseType.objects.create( + name="dep_case_type", + domain=cls.domain_name, + is_deprecated=True, + ) + + cls.fhir_resource_name = "fhir-sample" + cls.fhir_json_path = "sample.json.path" + cls.case_types_endpoint = reverse(cls.case_types_view_name, args=[cls.domain_name]) + + @classmethod + def tearDownClass(cls): + cls.case_type_obj.delete() + cls.deprecated_case_type_obj.delete() + cls.couch_user.delete(cls.domain_name, deleted_by=None) + cls.domain.delete() + super(DataDictionaryJsonV2Test, cls).tearDownClass() + + def setUp(self): + self.client.login(username='test', password='foobar') + + @classmethod + def _create_properties_for_case_type(cls, case_type, properties_count, group=None): + case_properties = [] + for index in range(properties_count): + prop_name = ''.join(random.choices(string.ascii_lowercase + string.digits, k=7)) + case_prop_obj = CaseProperty.objects.create( + case_type=case_type, + name=prop_name, + data_type='number', + group=group if group else None + ) + case_properties.append(case_prop_obj) + return case_properties + + @classmethod + def _get_case_types_json(cls, with_deprecated=False, fhir_enabled=False): + expected_output = { + "case_types": [ + { + "name": cls.case_type_obj.name, + "fhir_resource_type": cls.fhir_resource_name if fhir_enabled else None, + "is_safe_to_delete": True, + "is_deprecated": False, + "module_count": 0, + "properties_count": cls.case_type_obj.properties.count(), + }, + ], + "geo_case_property": GPS_POINT_CASE_PROPERTY, + } + if with_deprecated: + expected_output['case_types'].append( + { + "name": cls.deprecated_case_type_obj.name, + "fhir_resource_type": None, + "is_safe_to_delete": True, + "is_deprecated": True, + "module_count": 0, + "properties_count": cls.deprecated_case_type_obj.properties.count(), + } + ) + return expected_output + + def test_get_case_types_no_access(self, *args): + # uses a different client that is not logged in + response = Client().get(self.case_types_endpoint) + self.assertEqual(response.status_code, 302) + + def test_get_case_types(self, *args): + response = self.client.get(self.case_types_endpoint) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_types_json() + self.assertEqual(response.json(), expected_response) + + @flag_enabled('FHIR_INTEGRATION') + @patch('corehq.apps.data_dictionary.views.load_fhir_resource_mappings') + def test_get_case_types_fhir_enabled(self, mocked_load_fhir_resource_mappings, *args): + mocked_load_fhir_resource_mappings.return_value = ( + {self.case_type_obj: self.fhir_resource_name}, + {case_property: self.fhir_json_path for case_property in self.case_properties_with_group} + ) + response = self.client.get(self.case_types_endpoint) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_types_json(fhir_enabled=True) + self.assertEqual(response.json(), expected_response) + + def test_get_case_types_with_deprecated(self, *args): + response = self.client.get(self.case_types_endpoint, data={'load_deprecated_case_types': 'true'}) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_types_json(with_deprecated=True) + self.assertEqual(response.json(), expected_response) + + @privilege_enabled(privileges.DATA_DICTIONARY) class TestDeprecateOrRestoreCaseTypeView(TestCase): From 59eb921e301feeaaa7e3b26cf5d9458846ce751b Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 19:30:17 +0530 Subject: [PATCH 06/20] nit: rename test_view to test_views --- corehq/apps/data_dictionary/tests/{test_view.py => test_views.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename corehq/apps/data_dictionary/tests/{test_view.py => test_views.py} (100%) diff --git a/corehq/apps/data_dictionary/tests/test_view.py b/corehq/apps/data_dictionary/tests/test_views.py similarity index 100% rename from corehq/apps/data_dictionary/tests/test_view.py rename to corehq/apps/data_dictionary/tests/test_views.py From c9bb8c8d1e1e412e8ad0cb8db5c03b2a98e6a662 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 19:38:41 +0530 Subject: [PATCH 07/20] updates response for new case properties view --- corehq/apps/data_dictionary/views.py | 128 +++++++++++++-------------- 1 file changed, 61 insertions(+), 67 deletions(-) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index 83c071e4a250..ed95ab5d81c4 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -9,7 +9,7 @@ from django.db.models.query import Prefetch from django.db.transaction import atomic from django.http import HttpResponse, HttpResponseRedirect, JsonResponse -from django.shortcuts import redirect +from django.shortcuts import redirect, get_object_or_404 from django.urls import reverse from django.utils.decorators import method_decorator from django.utils.translation import gettext_lazy as _ @@ -180,86 +180,80 @@ def data_dictionary_json_case_types(request, domain): @login_and_domain_required @requires_privilege_with_fallback(privileges.DATA_DICTIONARY) def data_dictionary_json_case_properties(request, domain, case_type_name): - props = [] fhir_resource_type_name_by_case_type = {} fhir_resource_prop_by_case_prop = {} - queryset = CaseType.objects.filter(domain=domain) - queryset = queryset.prefetch_related( - Prefetch('groups', queryset=CasePropertyGroup.objects.order_by('index')), - # order by pk for properties with same index, likely for automatically added properties - Prefetch('properties', queryset=CaseProperty.objects.order_by('group_id', 'index', 'pk')), - Prefetch('properties__allowed_values', queryset=CasePropertyAllowedValue.objects.order_by('allowed_value')) - ) if toggles.FHIR_INTEGRATION.enabled(domain): fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( domain) - queryset = queryset.filter(name=case_type_name) - case_type_app_module_count = get_case_type_app_module_count(domain) + case_type = get_object_or_404( + CaseType.objects.annotate(properties_count=Count('property')), + domain=domain, + name=case_type_name + ) + case_type_data = { + "name": case_type.name, + "properties_count": case_type.properties_count, + "groups": [] + } + + properties_queryset = CaseProperty.objects.select_related('group').filter(case_type=case_type) + properties_queryset = properties_queryset.order_by('group_id', 'index', 'pk') + properties_queryset = properties_queryset.prefetch_related( + Prefetch('allowed_values', queryset=CasePropertyAllowedValue.objects.order_by('allowed_value')) + ) + data_validation_enabled = toggles.CASE_IMPORT_DATA_DICTIONARY_VALIDATION.enabled(domain) used_props_by_case_type = get_used_props_by_case_type(domain) geo_case_prop = get_geo_case_property(domain) - for case_type in queryset: - module_count = case_type_app_module_count.get(case_type.name, 0) - used_props = used_props_by_case_type[case_type.name] if case_type.name in used_props_by_case_type else [] - p = { - "name": case_type.name, - "fhir_resource_type": fhir_resource_type_name_by_case_type.get(case_type), - "groups": [], - "is_deprecated": case_type.is_deprecated, - "module_count": module_count, - "properties": [], - "is_safe_to_delete": len(used_props) == 0, - } - grouped_properties = { - group: [ + + used_props = used_props_by_case_type[case_type.name] if case_type.name in used_props_by_case_type else [] + + grouped_properties = { + group: [ + { + 'id': prop.id, + 'description': prop.description, + 'label': prop.label, + 'fhir_resource_prop_path': fhir_resource_prop_by_case_prop.get( + prop + ), + 'name': prop.name, + 'deprecated': prop.deprecated, + 'is_safe_to_delete': prop.name not in used_props and prop.name != geo_case_prop, + } + | ( { - 'id': prop.id, - 'description': prop.description, - 'label': prop.label, - 'fhir_resource_prop_path': fhir_resource_prop_by_case_prop.get( - prop - ), - 'name': prop.name, - 'deprecated': prop.deprecated, - 'is_safe_to_delete': prop.name not in used_props and prop.name != geo_case_prop, + 'data_type': prop.data_type, + 'allowed_values': { + av.allowed_value: av.description + for av in prop.allowed_values.all() + }, } - | ( - { - 'data_type': prop.data_type, - 'allowed_values': { - av.allowed_value: av.description - for av in prop.allowed_values.all() - }, - } - if data_validation_enabled - else {} - ) - for prop in props - ] - for group, props in itertools.groupby( - case_type.properties.all(), key=attrgetter('group_id') + if data_validation_enabled + else {} ) - } - for group in case_type.groups.all(): - p["groups"].append({ - "id": group.id, - "name": group.name, - "description": group.description, - "deprecated": group.deprecated, - "properties": grouped_properties.get(group.id, []) - }) - - # Aggregate properties that don't have a group - p["groups"].append({ - "name": "", - "properties": grouped_properties.get(None, []) + for prop in props + ] + for group, props in itertools.groupby( + properties_queryset, key=attrgetter('group_id') + ) + } + for group in case_type.groups.all(): + case_type_data["groups"].append({ + "id": group.id, + "name": group.name, + "description": group.description, + "deprecated": group.deprecated, + "properties": grouped_properties.get(group.id, []) }) - props.append(p) - return JsonResponse({ - 'case_types': props, - 'geo_case_property': geo_case_prop, + + # Aggregate properties that don't have a group + case_type_data["groups"].append({ + "name": "", + "properties": grouped_properties.get(None, []) }) + return JsonResponse(case_type_data) @login_and_domain_required From b67fec7bc81a4ec9b65590ca92206c3f59d622d9 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 19:42:08 +0530 Subject: [PATCH 08/20] refactor grouping logic for readibility and use groups from groupby --- corehq/apps/data_dictionary/views.py | 58 +++++++++++++--------------- 1 file changed, 26 insertions(+), 32 deletions(-) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index ed95ab5d81c4..395d1b1de0a7 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -207,52 +207,46 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): used_props_by_case_type = get_used_props_by_case_type(domain) geo_case_prop = get_geo_case_property(domain) - used_props = used_props_by_case_type[case_type.name] if case_type.name in used_props_by_case_type else [] + used_props = used_props_by_case_type.get(case_type.name, []) - grouped_properties = { - group: [ - { + for group_id, props in itertools.groupby(properties_queryset, key=attrgetter("group_id")): + props = list(props) + grouped_properties = [] + for prop in props: + prop_data = { 'id': prop.id, 'description': prop.description, 'label': prop.label, - 'fhir_resource_prop_path': fhir_resource_prop_by_case_prop.get( - prop - ), + 'fhir_resource_prop_path': fhir_resource_prop_by_case_prop.get(prop), 'name': prop.name, 'deprecated': prop.deprecated, 'is_safe_to_delete': prop.name not in used_props and prop.name != geo_case_prop, } - | ( - { + if data_validation_enabled: + prop_data.update({ 'data_type': prop.data_type, 'allowed_values': { av.allowed_value: av.description for av in prop.allowed_values.all() }, - } - if data_validation_enabled - else {} - ) - for prop in props - ] - for group, props in itertools.groupby( - properties_queryset, key=attrgetter('group_id') - ) - } - for group in case_type.groups.all(): - case_type_data["groups"].append({ - "id": group.id, - "name": group.name, - "description": group.description, - "deprecated": group.deprecated, - "properties": grouped_properties.get(group.id, []) - }) + }) + grouped_properties.append(prop_data) + + group_data = { + "name": "", + "properties": grouped_properties + } + # Note that properties can be without group + if group_id: + group = props[0].group + group_data.update({ + "id": group.id, + "name": group.name, + "description": group.description, + "deprecated": group.deprecated, + }) + case_type_data["groups"].append(group_data) - # Aggregate properties that don't have a group - case_type_data["groups"].append({ - "name": "", - "properties": grouped_properties.get(None, []) - }) return JsonResponse(case_type_data) From 6ebc16316a09f06937cbd464388ba4b50da5bfad Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 19:46:43 +0530 Subject: [PATCH 09/20] adds skip limit for case properties API --- corehq/apps/data_dictionary/views.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index 395d1b1de0a7..c2b34f521cec 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -186,6 +186,14 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( domain) + try: + skip = int(request.GET.get('skip', 0)) + limit = int(request.GET.get('limit', 500)) + if skip < 0 or limit < 0: + raise ValueError + except ValueError: + return JsonResponse({"error": _("skip and limit must be positive integers")}, status=400) + case_type = get_object_or_404( CaseType.objects.annotate(properties_count=Count('property')), domain=domain, @@ -198,7 +206,7 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): } properties_queryset = CaseProperty.objects.select_related('group').filter(case_type=case_type) - properties_queryset = properties_queryset.order_by('group_id', 'index', 'pk') + properties_queryset = properties_queryset.order_by('group_id', 'index', 'pk')[skip:skip + limit] properties_queryset = properties_queryset.prefetch_related( Prefetch('allowed_values', queryset=CasePropertyAllowedValue.objects.order_by('allowed_value')) ) From e7d7f3c448961523b444e6d5a805148db63d2b19 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 19:50:15 +0530 Subject: [PATCH 10/20] util for adding query params to url with test cases --- .../apps/data_dictionary/tests/test_util.py | 19 ++++++++++++++++++- corehq/apps/data_dictionary/util.py | 10 ++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/corehq/apps/data_dictionary/tests/test_util.py b/corehq/apps/data_dictionary/tests/test_util.py index 38783e4c6457..bc3d704c00fc 100644 --- a/corehq/apps/data_dictionary/tests/test_util.py +++ b/corehq/apps/data_dictionary/tests/test_util.py @@ -1,7 +1,7 @@ import uuid from unittest.mock import patch -from django.test import TestCase +from django.test import TestCase, SimpleTestCase from django.utils.translation import gettext from casexml.apps.case.mock import CaseBlock @@ -23,6 +23,7 @@ get_case_property_description_dict, get_case_property_label_dict, get_case_property_deprecated_dict, + update_url_query_params, ) from corehq.apps.es.case_search import case_search_adapter from corehq.apps.es.tests.utils import ( @@ -367,3 +368,19 @@ def test_get_used_props_by_case_type(self): self.assertEqual({'prop', 'other-prop'}, props) props = set(used_props_by_case_type['other-case-type']) - metadata_props self.assertEqual({'prop', 'foobar'}, props) + + +class TestUpdateUrlQueryParams(SimpleTestCase): + url = "http://example.com" + + def test_add_params(self): + result = update_url_query_params(self.url, {"fruit": "orange", "biscuits": "oreo"}) + self.assertEqual(result, f"{self.url}?fruit=orange&biscuits=oreo") + + def test_update_params(self): + result = update_url_query_params(f"{self.url}?fruit=apple", {"fruit": "orange", "biscuits": "oreo"}) + self.assertEqual(result, f"{self.url}?fruit=orange&biscuits=oreo") + + def test_no_params(self): + result = update_url_query_params(self.url, {}) + self.assertEqual(result, self.url) diff --git a/corehq/apps/data_dictionary/util.py b/corehq/apps/data_dictionary/util.py index 8d32da525de0..3938f98d3c0e 100644 --- a/corehq/apps/data_dictionary/util.py +++ b/corehq/apps/data_dictionary/util.py @@ -2,6 +2,7 @@ from itertools import groupby from operator import attrgetter import re +from urllib.parse import urlparse, urlencode, parse_qsl from django.core.exceptions import ValidationError from django.utils.translation import gettext @@ -416,3 +417,12 @@ def get_used_props_by_case_type(domain): props_by_case_type[case_type_bucket.key] = [] props_by_case_type[case_type_bucket.key].append(prop_bucket.key) return props_by_case_type + + +def update_url_query_params(url, params): + """Adds query params to the url. Overrides the value if param already exists.""" + parsed_url = urlparse(url) + current_params = dict(parse_qsl(parsed_url.query)) + merged_params = urlencode({**current_params, **params}) + # Note: _replace is a public method of namedtuple. Starts with _ to avoid conflicts with field names. + return parsed_url._replace(query=merged_params).geturl() From 1339a9d204d8f63436b20fb79527a86e2b18c221 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 20:02:22 +0530 Subject: [PATCH 11/20] adds pagination links --- corehq/apps/data_dictionary/views.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index c2b34f521cec..aadcc37781d4 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -31,6 +31,7 @@ delete_case_property, get_used_props_by_case_type, get_data_dict_props_by_case_type, + update_url_query_params, ) from corehq.apps.domain.decorators import login_and_domain_required from corehq.apps.hqwebapp.decorators import use_jquery_ui @@ -205,6 +206,9 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): "groups": [] } + current_url = request.build_absolute_uri() + case_type_data["_links"] = _get_pagination_links(current_url, case_type_data["properties_count"], skip, limit) + properties_queryset = CaseProperty.objects.select_related('group').filter(case_type=case_type) properties_queryset = properties_queryset.order_by('group_id', 'index', 'pk')[skip:skip + limit] properties_queryset = properties_queryset.prefetch_related( @@ -258,6 +262,18 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): return JsonResponse(case_type_data) +def _get_pagination_links(current_url, total_records, skip, limit): + links = {"self": update_url_query_params(current_url, {"skip": skip, "limit": limit})} + if skip: + links["previous"] = update_url_query_params( + current_url, + {"skip": max(skip - limit, 0), "limit": limit} + ) + if total_records > (skip + limit): + links["next"] = update_url_query_params(current_url, {"skip": skip + limit, "limit": limit}) + return links + + @login_and_domain_required @requires_privilege_with_fallback(privileges.DATA_DICTIONARY) @require_permission(HqPermissions.edit_data_dict) From 9f3423e6d20046acb47e813b5421d0730a2c6011 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 20:04:40 +0530 Subject: [PATCH 12/20] adds saved index to properties --- corehq/apps/data_dictionary/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index aadcc37781d4..1b6136fdeaa8 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -233,6 +233,7 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): 'name': prop.name, 'deprecated': prop.deprecated, 'is_safe_to_delete': prop.name not in used_props and prop.name != geo_case_prop, + 'index': prop.index, } if data_validation_enabled: prop_data.update({ From 7373e6cc502e148df13e1d730f01330eb92149bb Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 20:13:48 +0530 Subject: [PATCH 13/20] test cases for case properties view --- .../apps/data_dictionary/tests/test_views.py | 134 +++++++++++++++++- 1 file changed, 133 insertions(+), 1 deletion(-) diff --git a/corehq/apps/data_dictionary/tests/test_views.py b/corehq/apps/data_dictionary/tests/test_views.py index c2e1b1ff4d1f..00f34a999f1d 100644 --- a/corehq/apps/data_dictionary/tests/test_views.py +++ b/corehq/apps/data_dictionary/tests/test_views.py @@ -272,8 +272,8 @@ def test_no_view_access(self): @privilege_enabled(privileges.DATA_DICTIONARY) class DataDictionaryJsonV2Test(TestCase): domain_name = uuid.uuid4().hex - # TODO Replace this with the original view after migration case_types_view_name = "data_dictionary_json_case_types" + case_properties_view_name = "data_dictionary_json_case_properties" @classmethod def setUpClass(cls): @@ -317,6 +317,12 @@ def tearDownClass(cls): def setUp(self): self.client.login(username='test', password='foobar') + @classmethod + def case_properties_endpoint(cls, case_type=None): + if not case_type: + case_type = cls.case_type_obj.name + return reverse(cls.case_properties_view_name, args=[cls.domain_name, case_type]) + @classmethod def _create_properties_for_case_type(cls, case_type, properties_count, group=None): case_properties = [] @@ -359,6 +365,54 @@ def _get_case_types_json(cls, with_deprecated=False, fhir_enabled=False): ) return expected_output + @classmethod + def _get_case_properties_json(cls, case_type_obj, groups_properties_dict=None, skip=0, limit=500, + fhir_enabled=False): + expected_output = { + "name": case_type_obj.name, + "properties_count": case_type_obj.properties.count(), + "_links": { + "self": f"http://testserver/a/{cls.domain_name}/data_dictionary/json_v2/{case_type_obj.name}/" + f"?skip={skip}&limit={limit}" + }, + "groups": [] + } + if skip: + expected_output["_links"]["previous"] = (f"http://testserver/a/{cls.domain_name}/data_dictionary/" + f"json_v2/{case_type_obj.name}/?skip={skip - limit}" + f"&limit={limit}") + if case_type_obj.properties.count() > (skip + limit): + expected_output["_links"]["next"] = (f"http://testserver/a/{cls.domain_name}/data_dictionary/json_v2/" + f"{case_type_obj.name}/?skip={skip + limit}&limit={limit}") + if groups_properties_dict: + for group, properties in groups_properties_dict.items(): + group_data = { + "name": group.name if group else "", + "properties": [ + { + "id": prop.id, + "description": "", + "label": "", + "fhir_resource_prop_path": cls.fhir_json_path if fhir_enabled else None, + "name": prop.name, + "deprecated": False, + "is_safe_to_delete": True, + "data_type": "number", + "allowed_values": {}, + "index": prop.index, + } + for prop in properties + ], + } + if group: + group_data.update({ + "id": group.id, + "description": "", + "deprecated": False + }) + expected_output["groups"].append(group_data) + return expected_output + def test_get_case_types_no_access(self, *args): # uses a different client that is not logged in response = Client().get(self.case_types_endpoint) @@ -388,6 +442,84 @@ def test_get_case_types_with_deprecated(self, *args): expected_response = self._get_case_types_json(with_deprecated=True) self.assertEqual(response.json(), expected_response) + def test_get_case_properties_no_access(self, *args): + # uses a different client that is not logged in + response = Client().get(self.case_properties_endpoint()) + self.assertEqual(response.status_code, 302) + + def test_get_case_properties_404(self, *args): + response = self.client.get(self.case_properties_endpoint("does-not-exist")) + self.assertEqual(response.status_code, 404) + + def test_get_case_properties(self, *args): + response = self.client.get(self.case_properties_endpoint()) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_properties_json( + self.case_type_obj, + {self.group_obj: self.case_properties_with_group, None: self.case_properties_without_group}, + ) + self.assertEqual(response.json(), expected_response) + + @flag_enabled('FHIR_INTEGRATION') + @patch('corehq.apps.data_dictionary.views.load_fhir_resource_mappings') + def test_get_case_properties_fhir_enabled(self, mocked_load_fhir_resource_mappings, *args): + mocked_load_fhir_resource_mappings.return_value = ( + {self.case_type_obj: self.fhir_resource_name}, + {case_property: self.fhir_json_path for case_property in self.case_type_obj.properties.all()} + ) + response = self.client.get(self.case_properties_endpoint()) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_properties_json( + self.case_type_obj, + {self.group_obj: self.case_properties_with_group, None: self.case_properties_without_group}, + fhir_enabled=True + ) + self.assertEqual(response.json(), expected_response) + + def test_get_case_properties_with_skip_limit(self, *args): + response = self.client.get(self.case_properties_endpoint(), data={"skip": 2, "limit": 2}) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_properties_json( + self.case_type_obj, + {None: self.case_properties_without_group}, + skip=2, + limit=2, + ) + self.assertEqual(response.json(), expected_response) + + def test_get_case_properties_with_skip_limit_error(self, *args): + response = self.client.get(self.case_properties_endpoint(), data={"skip": -1, "limit": 2}) + self.assertEqual(response.status_code, 400) + + def test_get_case_properties_multi_page(self, *args): + response = self.client.get(self.case_properties_endpoint(), data={"skip": 0, "limit": 2}) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_properties_json( + self.case_type_obj, + {self.group_obj: self.case_properties_with_group}, + limit=2 + ) + self.assertEqual(response.json(), expected_response) + # Get Next Page + response = self.client.get(response.json()["_links"]["next"]) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_properties_json( + self.case_type_obj, + {None: self.case_properties_without_group}, + skip=2, + limit=2 + ) + self.assertEqual(response.json(), expected_response) + # Get Previous Page + response = self.client.get(response.json()["_links"]["previous"]) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_properties_json( + self.case_type_obj, + {self.group_obj: self.case_properties_with_group}, + limit=2 + ) + self.assertEqual(response.json(), expected_response) + @privilege_enabled(privileges.DATA_DICTIONARY) class TestDeprecateOrRestoreCaseTypeView(TestCase): From c0cb6bfdd755b14a7ae12f78891bdd4c9721d706 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 21:01:38 +0530 Subject: [PATCH 14/20] splits load_fhir_resource_mappings into two fuctions --- .../apps/data_dictionary/tests/test_views.py | 36 ++++++++++++------- corehq/apps/data_dictionary/views.py | 21 +++++------ corehq/motech/fhir/utils.py | 16 +++++---- 3 files changed, 43 insertions(+), 30 deletions(-) diff --git a/corehq/apps/data_dictionary/tests/test_views.py b/corehq/apps/data_dictionary/tests/test_views.py index 00f34a999f1d..5721e9ae65f0 100644 --- a/corehq/apps/data_dictionary/tests/test_views.py +++ b/corehq/apps/data_dictionary/tests/test_views.py @@ -425,12 +425,18 @@ def test_get_case_types(self, *args): self.assertEqual(response.json(), expected_response) @flag_enabled('FHIR_INTEGRATION') - @patch('corehq.apps.data_dictionary.views.load_fhir_resource_mappings') - def test_get_case_types_fhir_enabled(self, mocked_load_fhir_resource_mappings, *args): - mocked_load_fhir_resource_mappings.return_value = ( - {self.case_type_obj: self.fhir_resource_name}, - {case_property: self.fhir_json_path for case_property in self.case_properties_with_group} - ) + @patch('corehq.apps.data_dictionary.views.load_fhir_case_type_mapping') + @patch('corehq.apps.data_dictionary.views.load_fhir_case_properties_mapping') + def test_get_case_types_fhir_enabled( + self, + mocked_load_fhir_case_properties_mapping, + mocked_load_fhir_case_type_mapping, + *args + ): + mocked_load_fhir_case_type_mapping.return_value = {self.case_type_obj: self.fhir_resource_name} + mocked_load_fhir_case_properties_mapping.return_value = { + case_property: self.fhir_json_path for case_property in self.case_type_obj.properties.all() + } response = self.client.get(self.case_types_endpoint) self.assertEqual(response.status_code, 200) expected_response = self._get_case_types_json(fhir_enabled=True) @@ -461,12 +467,18 @@ def test_get_case_properties(self, *args): self.assertEqual(response.json(), expected_response) @flag_enabled('FHIR_INTEGRATION') - @patch('corehq.apps.data_dictionary.views.load_fhir_resource_mappings') - def test_get_case_properties_fhir_enabled(self, mocked_load_fhir_resource_mappings, *args): - mocked_load_fhir_resource_mappings.return_value = ( - {self.case_type_obj: self.fhir_resource_name}, - {case_property: self.fhir_json_path for case_property in self.case_type_obj.properties.all()} - ) + @patch('corehq.apps.data_dictionary.views.load_fhir_case_type_mapping') + @patch('corehq.apps.data_dictionary.views.load_fhir_case_properties_mapping') + def test_get_case_properties_fhir_enabled( + self, + mocked_load_fhir_case_properties_mapping, + mocked_load_fhir_case_type_mapping, + *args + ): + mocked_load_fhir_case_type_mapping.return_value = {self.case_type_obj: self.fhir_resource_name} + mocked_load_fhir_case_properties_mapping.return_value = { + case_property: self.fhir_json_path for case_property in self.case_type_obj.properties.all() + } response = self.client.get(self.case_properties_endpoint()) self.assertEqual(response.status_code, 200) expected_response = self._get_case_properties_json( diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index 1b6136fdeaa8..e22fe66ea097 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -40,7 +40,8 @@ from corehq.apps.users.decorators import require_permission from corehq.apps.users.models import HqPermissions from corehq.motech.fhir.utils import ( - load_fhir_resource_mappings, + load_fhir_case_type_mapping, + load_fhir_case_properties_mapping, load_fhir_resource_types, remove_fhir_resource_type, update_fhir_resource_type, @@ -73,8 +74,9 @@ def data_dictionary_json(request, domain, case_type_name=None): Prefetch('properties__allowed_values', queryset=CasePropertyAllowedValue.objects.order_by('allowed_value')) ) if toggles.FHIR_INTEGRATION.enabled(domain): - fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( - domain) + fhir_resource_type_name_by_case_type = load_fhir_case_type_mapping(domain) + fhir_resource_prop_by_case_prop = load_fhir_case_properties_mapping(domain) + if case_type_name: queryset = queryset.filter(name=case_type_name) @@ -149,13 +151,11 @@ def data_dictionary_json(request, domain, case_type_name=None): @requires_privilege_with_fallback(privileges.DATA_DICTIONARY) def data_dictionary_json_case_types(request, domain): fhir_resource_type_name_by_case_type = {} - fhir_resource_prop_by_case_prop = {} queryset = CaseType.objects.filter(domain=domain).annotate(properties_count=Count('property')) if not request.GET.get('load_deprecated_case_types', False) == 'true': queryset = queryset.filter(is_deprecated=False) if toggles.FHIR_INTEGRATION.enabled(domain): - fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( - domain) + fhir_resource_type_name_by_case_type = load_fhir_case_type_mapping(domain) case_type_app_module_count = get_case_type_app_module_count(domain) used_props_by_case_type = get_used_props_by_case_type(domain) @@ -181,11 +181,9 @@ def data_dictionary_json_case_types(request, domain): @login_and_domain_required @requires_privilege_with_fallback(privileges.DATA_DICTIONARY) def data_dictionary_json_case_properties(request, domain, case_type_name): - fhir_resource_type_name_by_case_type = {} fhir_resource_prop_by_case_prop = {} if toggles.FHIR_INTEGRATION.enabled(domain): - fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( - domain) + fhir_resource_prop_by_case_prop = load_fhir_case_properties_mapping(domain) try: skip = int(request.GET.get('skip', 0)) @@ -473,9 +471,8 @@ def generate_prop_dict(case_prop, fhir_resource_prop): fhir_resource_prop_by_case_prop = {} if export_fhir_data: - fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop = load_fhir_resource_mappings( - domain - ) + fhir_resource_type_name_by_case_type = load_fhir_case_type_mapping(domain) + fhir_resource_prop_by_case_prop = load_fhir_case_properties_mapping(domain) _add_fhir_resource_mapping_sheet(case_type_data, fhir_resource_type_name_by_case_type) for case_type in queryset: diff --git a/corehq/motech/fhir/utils.py b/corehq/motech/fhir/utils.py index 1105c4bf4a0f..3ed98eb1d63d 100644 --- a/corehq/motech/fhir/utils.py +++ b/corehq/motech/fhir/utils.py @@ -18,18 +18,22 @@ def resource_url(domain, fhir_version_name, resource_type, case_id): return absolute_reverse(get_view, args=(domain, fhir_version_name, resource_type, case_id)) -def load_fhir_resource_mappings(domain): +def load_fhir_case_type_mapping(domain): fhir_resource_types = FHIRResourceType.objects.select_related('case_type').filter(domain=domain) - fhir_resource_type_name_by_case_type = { + return { ft.case_type: ft.name for ft in fhir_resource_types } - fhir_resource_prop_by_case_prop = { + + +def load_fhir_case_properties_mapping(domain): + fhir_resource_properties = FHIRResourceProperty.objects.select_related('case_property').filter( + resource_type__domain=domain + ) + return { fr.case_property: fr.jsonpath - for fr in FHIRResourceProperty.objects.select_related('case_property').filter( - resource_type__in=fhir_resource_types) + for fr in fhir_resource_properties } - return fhir_resource_type_name_by_case_type, fhir_resource_prop_by_case_prop def update_fhir_resource_type(domain, case_type, fhir_resource_type): From eb66360927965f1e2680d858eff173a49207604b Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 21:03:42 +0530 Subject: [PATCH 15/20] nit: corrects TODO plus move test class --- .../apps/data_dictionary/tests/test_views.py | 362 +++++++++--------- 1 file changed, 181 insertions(+), 181 deletions(-) diff --git a/corehq/apps/data_dictionary/tests/test_views.py b/corehq/apps/data_dictionary/tests/test_views.py index 5721e9ae65f0..d0dc9dd8263d 100644 --- a/corehq/apps/data_dictionary/tests/test_views.py +++ b/corehq/apps/data_dictionary/tests/test_views.py @@ -215,7 +215,6 @@ def test_delete_case_property(self): self.assertIsNone(prop) -# TODO Remove this Test Class once we migrate to the new view @privilege_enabled(privileges.DATA_DICTIONARY) class DataDictionaryViewTest(TestCase): domain_name = uuid.uuid4().hex @@ -266,6 +265,187 @@ def test_no_view_access(self): self.assertEqual(response.status_code, 403) +@privilege_enabled(privileges.DATA_DICTIONARY) +class TestDeprecateOrRestoreCaseTypeView(TestCase): + + urlname = 'deprecate_or_restore_case_type' + domain = 'test-domain' + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.domain_obj = create_domain(cls.domain) + cls.admin_webuser = WebUser.create(cls.domain, 'test', 'foobar', None, None, is_admin=True) + cls.case_type_name = 'caseType' + cls.case_type_obj = CaseType(name=cls.case_type_name, domain=cls.domain) + cls.case_type_obj.save() + + CaseProperty(case_type=cls.case_type_obj, name='property').save() + CasePropertyGroup(case_type=cls.case_type_obj, name='group').save() + + def setUp(self): + self.endpoint = reverse(self.urlname, args=(self.domain, self.case_type_obj.name)) + self.client = Client() + self.client.login(username='test', password='foobar') + + @classmethod + def tearDownClass(cls): + cls.case_type_obj.delete() + cls.admin_webuser.delete(cls.domain, None) + cls.domain_obj.delete() + return super().tearDownClass() + + def _update_deprecate_state(self, is_deprecated): + case_type_obj = CaseType.objects.get(name=self.case_type_name) + case_type_obj.is_deprecated = is_deprecated + case_type_obj.save() + CaseProperty.objects.filter(case_type=case_type_obj).update(deprecated=is_deprecated) + CasePropertyGroup.objects.filter(case_type=case_type_obj).update(deprecated=is_deprecated) + + def test_deprecate_case_type(self): + response = self.client.post(self.endpoint, {'is_deprecated': 'true'}) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), {'status': 'success'}) + case_type_obj = CaseType.objects.get(name=self.case_type_name) + self.assertTrue(case_type_obj.is_deprecated) + + case_prop_count = CaseProperty.objects.filter(case_type=case_type_obj, deprecated=False).count() + self.assertEqual(case_prop_count, 0) + case_prop_group_count = CasePropertyGroup.objects.filter(case_type=case_type_obj, deprecated=False).count() + self.assertEqual(case_prop_group_count, 0) + + def test_restore_case_type(self): + self._update_deprecate_state(is_deprecated=True) + + response = self.client.post(self.endpoint, {'is_deprecated': 'false'}) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), {'status': 'success'}) + case_type_obj = CaseType.objects.get(name=self.case_type_name) + self.assertFalse(case_type_obj.is_deprecated) + + case_prop_count = CaseProperty.objects.filter(case_type=case_type_obj, deprecated=True).count() + self.assertEqual(case_prop_count, 0) + case_prop_group_count = CasePropertyGroup.objects.filter(case_type=case_type_obj, deprecated=True).count() + self.assertEqual(case_prop_group_count, 0) + + +# TODO Remove this Test Class once we migrate to the new view +@flag_enabled('CASE_IMPORT_DATA_DICTIONARY_VALIDATION') +@privilege_enabled(privileges.DATA_DICTIONARY) +class DataDictionaryJsonTest(TestCase): + domain_name = uuid.uuid4().hex + + @classmethod + def setUpClass(cls): + super(DataDictionaryJsonTest, cls).setUpClass() + cls.domain = create_domain(cls.domain_name) + cls.couch_user = WebUser.create(None, "test", "foobar", None, None) + cls.couch_user.add_domain_membership(cls.domain_name, is_admin=True) + cls.couch_user.save() + cls.case_type_obj = CaseType(name='caseType', domain=cls.domain_name) + cls.case_type_obj.save() + cls.case_prop_group = CasePropertyGroup(case_type=cls.case_type_obj, name='group') + cls.case_prop_group.save() + cls.case_prop_obj = CaseProperty( + case_type=cls.case_type_obj, + name='property', + data_type='number', + group=cls.case_prop_group + ) + cls.case_prop_obj.save() + cls.deprecated_case_type_obj = CaseType(name='depCaseType', domain=cls.domain_name, is_deprecated=True) + cls.deprecated_case_type_obj.save() + cls.client = Client() + + @classmethod + def tearDownClass(cls): + cls.case_type_obj.delete() + cls.deprecated_case_type_obj.delete() + cls.couch_user.delete(cls.domain_name, deleted_by=None) + cls.domain.delete() + super(DataDictionaryJsonTest, cls).tearDownClass() + + def setUp(self): + self.endpoint = reverse('data_dictionary_json', args=[self.domain_name]) + + @classmethod + def _get_case_type_json(self, with_deprecated=False): + expected_output = { + "case_types": [ + { + "name": "caseType", + "fhir_resource_type": None, + "is_safe_to_delete": True, + "groups": [ + { + "id": self.case_prop_group.id, + "name": "group", + "description": "", + "deprecated": False, + "properties": [ + { + "id": self.case_prop_obj.id, + "description": "", + "label": "", + "fhir_resource_prop_path": None, + "name": "property", + "deprecated": False, + "is_safe_to_delete": True, + "allowed_values": {}, + "data_type": "number", + }, + ], + }, + {"name": "", "properties": []}, + ], + "is_deprecated": False, + "module_count": 0, + "properties": [], + }, + ], + "geo_case_property": GPS_POINT_CASE_PROPERTY, + } + if with_deprecated: + expected_output['case_types'].append( + { + "name": "depCaseType", + "fhir_resource_type": None, + "is_safe_to_delete": True, + "groups": [ + { + "name": '', + "properties": [] + }, + ], + "is_deprecated": True, + "module_count": 0, + "properties": [], + } + ) + return expected_output + + def test_no_access(self): + response = self.client.get(self.endpoint) + self.assertEqual(response.status_code, 302) + + @patch('corehq.apps.data_dictionary.views.get_case_type_app_module_count', return_value={}) + @patch('corehq.apps.data_dictionary.views.get_used_props_by_case_type', return_value={}) + def test_get_json_success(self, *args): + self.client.login(username='test', password='foobar') + response = self.client.get(self.endpoint) + self.assertEqual(response.status_code, 200) + expected_response = self._get_case_type_json() + self.assertEqual(response.json(), expected_response) + + @patch('corehq.apps.data_dictionary.views.get_case_type_app_module_count', return_value={}) + @patch('corehq.apps.data_dictionary.views.get_used_props_by_case_type', return_value={}) + def test_get_json_success_with_deprecated_case_types(self, *args): + self.client.login(username='test', password='foobar') + response = self.client.get(self.endpoint, data={'load_deprecated_case_types': 'true'}) + expected_response = self._get_case_type_json(with_deprecated=True) + self.assertEqual(response.json(), expected_response) + + @patch('corehq.apps.data_dictionary.views.get_case_type_app_module_count', return_value={}) @patch('corehq.apps.data_dictionary.views.get_used_props_by_case_type', return_value={}) @flag_enabled('CASE_IMPORT_DATA_DICTIONARY_VALIDATION') @@ -533,186 +713,6 @@ def test_get_case_properties_multi_page(self, *args): self.assertEqual(response.json(), expected_response) -@privilege_enabled(privileges.DATA_DICTIONARY) -class TestDeprecateOrRestoreCaseTypeView(TestCase): - - urlname = 'deprecate_or_restore_case_type' - domain = 'test-domain' - - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.domain_obj = create_domain(cls.domain) - cls.admin_webuser = WebUser.create(cls.domain, 'test', 'foobar', None, None, is_admin=True) - cls.case_type_name = 'caseType' - cls.case_type_obj = CaseType(name=cls.case_type_name, domain=cls.domain) - cls.case_type_obj.save() - - CaseProperty(case_type=cls.case_type_obj, name='property').save() - CasePropertyGroup(case_type=cls.case_type_obj, name='group').save() - - def setUp(self): - self.endpoint = reverse(self.urlname, args=(self.domain, self.case_type_obj.name)) - self.client = Client() - self.client.login(username='test', password='foobar') - - @classmethod - def tearDownClass(cls): - cls.case_type_obj.delete() - cls.admin_webuser.delete(cls.domain, None) - cls.domain_obj.delete() - return super().tearDownClass() - - def _update_deprecate_state(self, is_deprecated): - case_type_obj = CaseType.objects.get(name=self.case_type_name) - case_type_obj.is_deprecated = is_deprecated - case_type_obj.save() - CaseProperty.objects.filter(case_type=case_type_obj).update(deprecated=is_deprecated) - CasePropertyGroup.objects.filter(case_type=case_type_obj).update(deprecated=is_deprecated) - - def test_deprecate_case_type(self): - response = self.client.post(self.endpoint, {'is_deprecated': 'true'}) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.json(), {'status': 'success'}) - case_type_obj = CaseType.objects.get(name=self.case_type_name) - self.assertTrue(case_type_obj.is_deprecated) - - case_prop_count = CaseProperty.objects.filter(case_type=case_type_obj, deprecated=False).count() - self.assertEqual(case_prop_count, 0) - case_prop_group_count = CasePropertyGroup.objects.filter(case_type=case_type_obj, deprecated=False).count() - self.assertEqual(case_prop_group_count, 0) - - def test_restore_case_type(self): - self._update_deprecate_state(is_deprecated=True) - - response = self.client.post(self.endpoint, {'is_deprecated': 'false'}) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.json(), {'status': 'success'}) - case_type_obj = CaseType.objects.get(name=self.case_type_name) - self.assertFalse(case_type_obj.is_deprecated) - - case_prop_count = CaseProperty.objects.filter(case_type=case_type_obj, deprecated=True).count() - self.assertEqual(case_prop_count, 0) - case_prop_group_count = CasePropertyGroup.objects.filter(case_type=case_type_obj, deprecated=True).count() - self.assertEqual(case_prop_group_count, 0) - - -@flag_enabled('CASE_IMPORT_DATA_DICTIONARY_VALIDATION') -@privilege_enabled(privileges.DATA_DICTIONARY) -class DataDictionaryJsonTest(TestCase): - domain_name = uuid.uuid4().hex - - @classmethod - def setUpClass(cls): - super(DataDictionaryJsonTest, cls).setUpClass() - cls.domain = create_domain(cls.domain_name) - cls.couch_user = WebUser.create(None, "test", "foobar", None, None) - cls.couch_user.add_domain_membership(cls.domain_name, is_admin=True) - cls.couch_user.save() - cls.case_type_obj = CaseType(name='caseType', domain=cls.domain_name) - cls.case_type_obj.save() - cls.case_prop_group = CasePropertyGroup(case_type=cls.case_type_obj, name='group') - cls.case_prop_group.save() - cls.case_prop_obj = CaseProperty( - case_type=cls.case_type_obj, - name='property', - data_type='number', - group=cls.case_prop_group - ) - cls.case_prop_obj.save() - cls.deprecated_case_type_obj = CaseType(name='depCaseType', domain=cls.domain_name, is_deprecated=True) - cls.deprecated_case_type_obj.save() - cls.client = Client() - - @classmethod - def tearDownClass(cls): - cls.case_type_obj.delete() - cls.deprecated_case_type_obj.delete() - cls.couch_user.delete(cls.domain_name, deleted_by=None) - cls.domain.delete() - super(DataDictionaryJsonTest, cls).tearDownClass() - - def setUp(self): - self.endpoint = reverse('data_dictionary_json', args=[self.domain_name]) - - @classmethod - def _get_case_type_json(self, with_deprecated=False): - expected_output = { - "case_types": [ - { - "name": "caseType", - "fhir_resource_type": None, - "is_safe_to_delete": True, - "groups": [ - { - "id": self.case_prop_group.id, - "name": "group", - "description": "", - "deprecated": False, - "properties": [ - { - "id": self.case_prop_obj.id, - "description": "", - "label": "", - "fhir_resource_prop_path": None, - "name": "property", - "deprecated": False, - "is_safe_to_delete": True, - "allowed_values": {}, - "data_type": "number", - }, - ], - }, - {"name": "", "properties": []}, - ], - "is_deprecated": False, - "module_count": 0, - "properties": [], - }, - ], - "geo_case_property": GPS_POINT_CASE_PROPERTY, - } - if with_deprecated: - expected_output['case_types'].append( - { - "name": "depCaseType", - "fhir_resource_type": None, - "is_safe_to_delete": True, - "groups": [ - { - "name": '', - "properties": [] - }, - ], - "is_deprecated": True, - "module_count": 0, - "properties": [], - } - ) - return expected_output - - def test_no_access(self): - response = self.client.get(self.endpoint) - self.assertEqual(response.status_code, 302) - - @patch('corehq.apps.data_dictionary.views.get_case_type_app_module_count', return_value={}) - @patch('corehq.apps.data_dictionary.views.get_used_props_by_case_type', return_value={}) - def test_get_json_success(self, *args): - self.client.login(username='test', password='foobar') - response = self.client.get(self.endpoint) - self.assertEqual(response.status_code, 200) - expected_response = self._get_case_type_json() - self.assertEqual(response.json(), expected_response) - - @patch('corehq.apps.data_dictionary.views.get_case_type_app_module_count', return_value={}) - @patch('corehq.apps.data_dictionary.views.get_used_props_by_case_type', return_value={}) - def test_get_json_success_with_deprecated_case_types(self, *args): - self.client.login(username='test', password='foobar') - response = self.client.get(self.endpoint, data={'load_deprecated_case_types': 'true'}) - expected_response = self._get_case_type_json(with_deprecated=True) - self.assertEqual(response.json(), expected_response) - - @privilege_enabled(privileges.DATA_DICTIONARY) class TestDeleteCaseType(TestCase): From 9037e5e15b4ae9e7fa84e5c7aa8e5568fb350dd8 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 21:10:37 +0530 Subject: [PATCH 16/20] nit: code placement plus use get for dict --- corehq/apps/data_dictionary/views.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index e22fe66ea097..1e291fee4829 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -151,11 +151,12 @@ def data_dictionary_json(request, domain, case_type_name=None): @requires_privilege_with_fallback(privileges.DATA_DICTIONARY) def data_dictionary_json_case_types(request, domain): fhir_resource_type_name_by_case_type = {} + if toggles.FHIR_INTEGRATION.enabled(domain): + fhir_resource_type_name_by_case_type = load_fhir_case_type_mapping(domain) + queryset = CaseType.objects.filter(domain=domain).annotate(properties_count=Count('property')) if not request.GET.get('load_deprecated_case_types', False) == 'true': queryset = queryset.filter(is_deprecated=False) - if toggles.FHIR_INTEGRATION.enabled(domain): - fhir_resource_type_name_by_case_type = load_fhir_case_type_mapping(domain) case_type_app_module_count = get_case_type_app_module_count(domain) used_props_by_case_type = get_used_props_by_case_type(domain) @@ -163,7 +164,7 @@ def data_dictionary_json_case_types(request, domain): case_types_data = [] for case_type in queryset: module_count = case_type_app_module_count.get(case_type.name, 0) - used_props = used_props_by_case_type[case_type.name] if case_type.name in used_props_by_case_type else [] + used_props = used_props_by_case_type.get(case_type.name, []) case_types_data.append({ "name": case_type.name, "fhir_resource_type": fhir_resource_type_name_by_case_type.get(case_type), @@ -181,10 +182,6 @@ def data_dictionary_json_case_types(request, domain): @login_and_domain_required @requires_privilege_with_fallback(privileges.DATA_DICTIONARY) def data_dictionary_json_case_properties(request, domain, case_type_name): - fhir_resource_prop_by_case_prop = {} - if toggles.FHIR_INTEGRATION.enabled(domain): - fhir_resource_prop_by_case_prop = load_fhir_case_properties_mapping(domain) - try: skip = int(request.GET.get('skip', 0)) limit = int(request.GET.get('limit', 500)) @@ -193,6 +190,10 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): except ValueError: return JsonResponse({"error": _("skip and limit must be positive integers")}, status=400) + fhir_resource_prop_by_case_prop = {} + if toggles.FHIR_INTEGRATION.enabled(domain): + fhir_resource_prop_by_case_prop = load_fhir_case_properties_mapping(domain) + case_type = get_object_or_404( CaseType.objects.annotate(properties_count=Count('property')), domain=domain, From a97f285d76fc745cf9222d286aa3c6113c454480 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Thu, 27 Jun 2024 21:12:28 +0530 Subject: [PATCH 17/20] nit: isort on related files --- .../apps/data_dictionary/tests/test_util.py | 26 ++++++++----------- .../apps/data_dictionary/tests/test_views.py | 15 ++++++----- corehq/apps/data_dictionary/urls.py | 11 ++++---- corehq/apps/data_dictionary/util.py | 10 ++++--- corehq/apps/data_dictionary/views.py | 26 ++++++++++--------- 5 files changed, 47 insertions(+), 41 deletions(-) diff --git a/corehq/apps/data_dictionary/tests/test_util.py b/corehq/apps/data_dictionary/tests/test_util.py index bc3d704c00fc..1dbc70d93f8b 100644 --- a/corehq/apps/data_dictionary/tests/test_util.py +++ b/corehq/apps/data_dictionary/tests/test_util.py @@ -1,35 +1,31 @@ import uuid from unittest.mock import patch -from django.test import TestCase, SimpleTestCase +from django.test import SimpleTestCase, TestCase from django.utils.translation import gettext from casexml.apps.case.mock import CaseBlock -from corehq.util.workbook_reading.datamodels import Cell - from corehq.apps.data_dictionary.models import CaseProperty, CaseType from corehq.apps.data_dictionary.tests.utils import setup_data_dictionary from corehq.apps.data_dictionary.util import ( + delete_case_property, generate_data_dictionary, - get_values_hints_dict, + get_case_property_deprecated_dict, + get_case_property_description_dict, + get_case_property_label_dict, get_column_headings, - map_row_values_to_column_names, - is_case_type_deprecated, get_data_dict_deprecated_case_types, - is_case_type_or_prop_name_valid, - delete_case_property, get_used_props_by_case_type, - get_case_property_description_dict, - get_case_property_label_dict, - get_case_property_deprecated_dict, + get_values_hints_dict, + is_case_type_deprecated, + is_case_type_or_prop_name_valid, + map_row_values_to_column_names, update_url_query_params, ) from corehq.apps.es.case_search import case_search_adapter -from corehq.apps.es.tests.utils import ( - case_search_es_setup, - es_test, -) +from corehq.apps.es.tests.utils import case_search_es_setup, es_test +from corehq.util.workbook_reading.datamodels import Cell @patch('corehq.apps.data_dictionary.util._get_properties_by_case_type') diff --git a/corehq/apps/data_dictionary/tests/test_views.py b/corehq/apps/data_dictionary/tests/test_views.py index d0dc9dd8263d..e38dced5cf46 100644 --- a/corehq/apps/data_dictionary/tests/test_views.py +++ b/corehq/apps/data_dictionary/tests/test_views.py @@ -7,15 +7,18 @@ from django.test import Client, TestCase from django.urls import reverse -from corehq.apps.data_dictionary.models import CaseProperty, CasePropertyGroup, CasePropertyAllowedValue, CaseType +from corehq import privileges +from corehq.apps.data_dictionary.models import ( + CaseProperty, + CasePropertyAllowedValue, + CasePropertyGroup, + CaseType, +) from corehq.apps.domain.shortcuts import create_domain from corehq.apps.geospatial.const import GPS_POINT_CASE_PROPERTY -from corehq.apps.users.models import WebUser, HqPermissions +from corehq.apps.users.models import HqPermissions, WebUser from corehq.apps.users.models_role import UserRole - -from corehq.util.test_utils import privilege_enabled -from corehq import privileges -from corehq.util.test_utils import flag_enabled +from corehq.util.test_utils import flag_enabled, privilege_enabled @privilege_enabled(privileges.DATA_DICTIONARY) diff --git a/corehq/apps/data_dictionary/urls.py b/corehq/apps/data_dictionary/urls.py index 0e8e2e176056..8f8f03af0c61 100644 --- a/corehq/apps/data_dictionary/urls.py +++ b/corehq/apps/data_dictionary/urls.py @@ -1,17 +1,18 @@ -from django.urls import path, re_path as url +from django.urls import path +from django.urls import re_path as url from corehq.apps.data_dictionary.views import ( DataDictionaryView, ExportDataDictionaryView, UploadDataDictionaryView, + create_case_type, data_dictionary_json, - data_dictionary_json_case_types, data_dictionary_json_case_properties, + data_dictionary_json_case_types, + delete_case_type, + deprecate_or_restore_case_type, update_case_property, update_case_property_description, - create_case_type, - deprecate_or_restore_case_type, - delete_case_type, ) from corehq.apps.hqwebapp.decorators import waf_allow diff --git a/corehq/apps/data_dictionary/util.py b/corehq/apps/data_dictionary/util.py index 3938f98d3c0e..ea34aca86e30 100644 --- a/corehq/apps/data_dictionary/util.py +++ b/corehq/apps/data_dictionary/util.py @@ -1,8 +1,8 @@ +import re from collections import defaultdict from itertools import groupby from operator import attrgetter -import re -from urllib.parse import urlparse, urlencode, parse_qsl +from urllib.parse import parse_qsl, urlencode, urlparse from django.core.exceptions import ValidationError from django.utils.translation import gettext @@ -19,7 +19,11 @@ CaseType, ) from corehq.apps.es.aggregations import NestedAggregation, TermsAggregation -from corehq.apps.es.case_search import CaseSearchES, CASE_PROPERTIES_PATH, PROPERTY_KEY +from corehq.apps.es.case_search import ( + CASE_PROPERTIES_PATH, + PROPERTY_KEY, + CaseSearchES, +) from corehq.motech.fhir.utils import update_fhir_resource_property from corehq.util.quickcache import quickcache diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index 1e291fee4829..5a4d36a9c084 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -9,7 +9,7 @@ from django.db.models.query import Prefetch from django.db.transaction import atomic from django.http import HttpResponse, HttpResponseRedirect, JsonResponse -from django.shortcuts import redirect, get_object_or_404 +from django.shortcuts import get_object_or_404, redirect from django.urls import reverse from django.utils.decorators import method_decorator from django.utils.translation import gettext_lazy as _ @@ -18,7 +18,12 @@ from couchexport.models import Format from couchexport.writers import Excel2007ExportWriter -from corehq import toggles +from corehq import privileges, toggles +from corehq.apps.accounting.decorators import ( + requires_privilege, + requires_privilege_with_fallback, +) +from corehq.apps.app_manager.dbaccessors import get_case_type_app_module_count from corehq.apps.data_dictionary.models import ( CaseProperty, CasePropertyAllowedValue, @@ -26,35 +31,32 @@ CaseType, ) from corehq.apps.data_dictionary.util import ( - save_case_property, - save_case_property_group, delete_case_property, - get_used_props_by_case_type, get_data_dict_props_by_case_type, + get_used_props_by_case_type, + save_case_property, + save_case_property_group, update_url_query_params, ) from corehq.apps.domain.decorators import login_and_domain_required +from corehq.apps.geospatial.utils import get_geo_case_property from corehq.apps.hqwebapp.decorators import use_jquery_ui from corehq.apps.hqwebapp.utils import get_bulk_upload_form from corehq.apps.settings.views import BaseProjectDataView from corehq.apps.users.decorators import require_permission from corehq.apps.users.models import HqPermissions from corehq.motech.fhir.utils import ( - load_fhir_case_type_mapping, load_fhir_case_properties_mapping, + load_fhir_case_type_mapping, load_fhir_resource_types, remove_fhir_resource_type, update_fhir_resource_type, ) -from corehq.apps.accounting.decorators import requires_privilege_with_fallback, requires_privilege -from corehq import privileges -from corehq.apps.app_manager.dbaccessors import get_case_type_app_module_count -from corehq.apps.geospatial.utils import get_geo_case_property from .bulk import ( - process_bulk_upload, - FHIR_RESOURCE_TYPE_MAPPING_SHEET, ALLOWED_VALUES_SHEET_SUFFIX, + FHIR_RESOURCE_TYPE_MAPPING_SHEET, + process_bulk_upload, ) From 69c5ea54869234e2e904bb8e7529897a1579f2b9 Mon Sep 17 00:00:00 2001 From: Ajeet Date: Wed, 3 Jul 2024 12:33:16 +0530 Subject: [PATCH 18/20] sends empty default group if no properties exist --- corehq/apps/data_dictionary/views.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index 5a4d36a9c084..c4980e41f127 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -260,6 +260,11 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): "deprecated": group.deprecated, }) case_type_data["groups"].append(group_data) + if not properties_queryset: + case_type_data["groups"].append({ + "name": "", + "properties": [] + }) return JsonResponse(case_type_data) From 606be7c3766acae41257a4b30e995e6cb0e98ada Mon Sep 17 00:00:00 2001 From: Ajeet Date: Wed, 3 Jul 2024 12:34:01 +0530 Subject: [PATCH 19/20] nit:readibility --- corehq/apps/data_dictionary/views.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index c4980e41f127..023268ac90ac 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -210,10 +210,15 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): current_url = request.build_absolute_uri() case_type_data["_links"] = _get_pagination_links(current_url, case_type_data["properties_count"], skip, limit) - properties_queryset = CaseProperty.objects.select_related('group').filter(case_type=case_type) - properties_queryset = properties_queryset.order_by('group_id', 'index', 'pk')[skip:skip + limit] - properties_queryset = properties_queryset.prefetch_related( - Prefetch('allowed_values', queryset=CasePropertyAllowedValue.objects.order_by('allowed_value')) + properties_queryset = ( + CaseProperty.objects + .select_related('group') + .filter(case_type=case_type) + .order_by('group_id', 'index', 'pk')[skip:skip + limit] + .prefetch_related(Prefetch( + 'allowed_values', + queryset=CasePropertyAllowedValue.objects.order_by('allowed_value') + )) ) data_validation_enabled = toggles.CASE_IMPORT_DATA_DICTIONARY_VALIDATION.enabled(domain) From 876d78eaf777dc7e416599b2a2922df2a6f0a37e Mon Sep 17 00:00:00 2001 From: Ajeet Date: Wed, 3 Jul 2024 12:55:37 +0530 Subject: [PATCH 20/20] refactor get_used_props_by_case_type to accept case type --- corehq/apps/data_dictionary/tests/test_util.py | 6 ++++++ corehq/apps/data_dictionary/util.py | 6 ++++-- corehq/apps/data_dictionary/views.py | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/corehq/apps/data_dictionary/tests/test_util.py b/corehq/apps/data_dictionary/tests/test_util.py index 1dbc70d93f8b..3a828b82f9db 100644 --- a/corehq/apps/data_dictionary/tests/test_util.py +++ b/corehq/apps/data_dictionary/tests/test_util.py @@ -365,6 +365,12 @@ def test_get_used_props_by_case_type(self): props = set(used_props_by_case_type['other-case-type']) - metadata_props self.assertEqual({'prop', 'foobar'}, props) + def test_get_used_props_by_case_type_with_case_type_param(self): + used_props_by_case_type = get_used_props_by_case_type(self.domain, 'case-type') + self.assertEqual(len(used_props_by_case_type), 1) + props = set(used_props_by_case_type['case-type']) + self.assertTrue(props.issuperset({'prop', 'other-prop'})) + class TestUpdateUrlQueryParams(SimpleTestCase): url = "http://example.com" diff --git a/corehq/apps/data_dictionary/util.py b/corehq/apps/data_dictionary/util.py index ea34aca86e30..aec6968dda2a 100644 --- a/corehq/apps/data_dictionary/util.py +++ b/corehq/apps/data_dictionary/util.py @@ -399,8 +399,8 @@ def is_case_type_or_prop_name_valid(case_prop_name): return match_obj is not None -@quickcache(vary_on=['domain'], timeout=60 * 10) -def get_used_props_by_case_type(domain): +@quickcache(vary_on=['domain', 'case_type'], timeout=60 * 10) +def get_used_props_by_case_type(domain, case_type=None): agg = TermsAggregation('case_types', 'type.exact').aggregation( NestedAggregation('case_props', CASE_PROPERTIES_PATH).aggregation( TermsAggregation('props', PROPERTY_KEY) @@ -412,6 +412,8 @@ def get_used_props_by_case_type(domain): .size(0) .aggregation(agg) ) + if case_type: + query = query.case_type(case_type) case_type_buckets = query.run().aggregations.case_types.buckets_list props_by_case_type = {} for case_type_bucket in case_type_buckets: diff --git a/corehq/apps/data_dictionary/views.py b/corehq/apps/data_dictionary/views.py index 023268ac90ac..c40398f2a0d9 100644 --- a/corehq/apps/data_dictionary/views.py +++ b/corehq/apps/data_dictionary/views.py @@ -222,7 +222,7 @@ def data_dictionary_json_case_properties(request, domain, case_type_name): ) data_validation_enabled = toggles.CASE_IMPORT_DATA_DICTIONARY_VALIDATION.enabled(domain) - used_props_by_case_type = get_used_props_by_case_type(domain) + used_props_by_case_type = get_used_props_by_case_type(domain, case_type_name) geo_case_prop = get_geo_case_property(domain) used_props = used_props_by_case_type.get(case_type.name, [])