From 23637165fbd7f7b1345bd5dc040ab025f99b1ba6 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Fri, 20 May 2022 11:29:09 +0200 Subject: [PATCH 1/7] remove user filter if no users are selected --- corehq/apps/reports/standard/inspect.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/corehq/apps/reports/standard/inspect.py b/corehq/apps/reports/standard/inspect.py index 1cf937787027..e66ba6eb2954 100644 --- a/corehq/apps/reports/standard/inspect.py +++ b/corehq/apps/reports/standard/inspect.py @@ -74,6 +74,12 @@ def default_datespan(self): return datespan_from_beginning(self.domain_object, self.timezone) def _get_users_filter(self, mobile_user_and_group_slugs): + if ( + EMWF.no_filters_selected(mobile_user_and_group_slugs) + and self.request.couch_user.has_permission(self.domain, 'access_all_locations') + ): + return None + user_ids = (EMWF.user_es_query(self.domain, mobile_user_and_group_slugs, self.request.couch_user) @@ -102,8 +108,11 @@ def es_query(self): query = (form_es.FormES() .domain(self.domain) .filter(time_filter(gte=self.datespan.startdate, - lt=self.datespan.enddate_adjusted)) - .filter(self._get_users_filter(mobile_user_and_group_slugs))) + lt=self.datespan.enddate_adjusted))) + + users_filter = self._get_users_filter(mobile_user_and_group_slugs) + if users_filter: + query = query.filter(users_filter) # filter results by app and xmlns if applicable if FormsByApplicationFilter.has_selections(self.request): From c19f787ba5aaa0c836da6588bdd4588cad217ef3 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Fri, 20 May 2022 11:30:26 +0200 Subject: [PATCH 2/7] remove system form filtering This was only filtering on a selection of system forms and is also unnecessary since there are already other filters that won't match system forms. --- corehq/apps/reports/standard/inspect.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/corehq/apps/reports/standard/inspect.py b/corehq/apps/reports/standard/inspect.py index e66ba6eb2954..a176f17b05cb 100644 --- a/corehq/apps/reports/standard/inspect.py +++ b/corehq/apps/reports/standard/inspect.py @@ -120,11 +120,6 @@ def es_query(self): if form_values: query = query.OR(*[self._form_filter(f) for f in form_values]) - # Exclude system forms unless they selected "Unknown User" - if HQUserType.UNKNOWN not in EMWF.selected_user_types(mobile_user_and_group_slugs): - query = query.NOT(form_es.xmlns( - list(SYSTEM_FORM_XMLNS_MAP.keys()) - )) return query @property From 54c51aa078e8113aaefd47f0b6f0db8fb3c413e2 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Fri, 20 May 2022 13:22:01 +0200 Subject: [PATCH 3/7] include forms with no app ID in get_all_xmlns_app_id_pairs_submitted_to_in_domain --- .../commands/calculate_physical_size.py | 10 ++++- corehq/apps/es/aggregations.py | 8 +++- corehq/ex-submodules/couchforms/analytics.py | 3 +- .../couchforms/tests/test_analytics.py | 39 ++++++++++++------- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/corehq/apps/domain/management/commands/calculate_physical_size.py b/corehq/apps/domain/management/commands/calculate_physical_size.py index 7fdbb88d9b02..7dca1dd82c12 100644 --- a/corehq/apps/domain/management/commands/calculate_physical_size.py +++ b/corehq/apps/domain/management/commands/calculate_physical_size.py @@ -3,10 +3,11 @@ from django.core.management.base import BaseCommand -from corehq.apps.es import FormES, CaseSearchES, CaseES +from corehq.apps.es import FormES, CaseSearchES, CaseES, filters from corehq.apps.reports.analytics.esaccessors import ( get_case_types_for_domain_es, ) +from corehq.const import MISSING_APP_ID from couchforms.analytics import ( get_all_xmlns_app_id_pairs_submitted_to_in_domain, ) @@ -118,9 +119,14 @@ def _get_form_size_stats(domain, sample_size): query = (FormES() .domain(domain) .sort('received_on', desc=True) - .app(app_id) .xmlns(xmlns) .size(sample_size)) + + if app_id == MISSING_APP_ID: + query = query.filter(filters.missing("app_id")) + else: + query = query.app(app_id) + num_forms, avg_size = _get_totals_for_query(query) total_bytes += num_forms * avg_size total_forms += num_forms diff --git a/corehq/apps/es/aggregations.py b/corehq/apps/es/aggregations.py index 7e07bb8ca483..0d9fb7f49db0 100644 --- a/corehq/apps/es/aggregations.py +++ b/corehq/apps/es/aggregations.py @@ -204,11 +204,15 @@ class TermsAggregation(Aggregation): :param name: aggregation name :param field: name of the field to bucket on :param size: + :param missing: define how documents that are missing a value should be treated. + By default, they will be ignored. If a value is supplied here it will be used where + the value is missing. + """ type = "terms" result_class = BucketResult - def __init__(self, name, field, size=None): + def __init__(self, name, field, size=None, missing=None): assert re.match(r'\w+$', name), \ "Names must be valid python variable names, was {}".format(name) self.name = name @@ -216,6 +220,8 @@ def __init__(self, name, field, size=None): "field": field, "size": size if size is not None else SIZE_LIMIT, } + if missing: + self.body["missing"] = missing def order(self, field, order="asc", reset=True): query = deepcopy(self) diff --git a/corehq/ex-submodules/couchforms/analytics.py b/corehq/ex-submodules/couchforms/analytics.py index f49545cc5faa..702cb81c64ca 100644 --- a/corehq/ex-submodules/couchforms/analytics.py +++ b/corehq/ex-submodules/couchforms/analytics.py @@ -2,6 +2,7 @@ from corehq.apps.es import FormES from corehq.apps.es.aggregations import TermsAggregation +from corehq.const import MISSING_APP_ID from corehq.util.quickcache import quickcache from corehq.util.couch import stale_ok @@ -74,7 +75,7 @@ def get_all_xmlns_app_id_pairs_submitted_to_in_domain(domain): query = (FormES() .domain(domain) .aggregation( - TermsAggregation("app_id", "app_id").aggregation( + TermsAggregation("app_id", "app_id", missing=MISSING_APP_ID).aggregation( TermsAggregation("xmlns", "xmlns.exact"))) .remove_default_filter("has_xmlns") .remove_default_filter("has_user") diff --git a/corehq/ex-submodules/couchforms/tests/test_analytics.py b/corehq/ex-submodules/couchforms/tests/test_analytics.py index 42cc2043e518..1d87be08bd9c 100644 --- a/corehq/ex-submodules/couchforms/tests/test_analytics.py +++ b/corehq/ex-submodules/couchforms/tests/test_analytics.py @@ -4,6 +4,7 @@ from django.test import TestCase from requests import ConnectionError +from corehq.const import MISSING_APP_ID from couchforms.analytics import ( app_has_been_submitted_to_in_last_30_days, domain_has_submission_in_last_30_days, @@ -130,27 +131,35 @@ class CouchformsESAnalyticsTest(TestCase): def setUpClass(cls): super(CouchformsESAnalyticsTest, cls).setUpClass() - def create_form_and_sync_to_es(received_on): - with process_pillow_changes('xform-pillow', {'skip_ucr': True}): - with process_pillow_changes('DefaultChangeFeedPillow'): - metadata = TestFormMetadata(domain=cls.domain, app_id=cls.app_id, - xmlns=cls.xmlns, received_on=received_on) - form = get_form_ready_to_save(metadata, is_db_test=True) - form_processor = FormProcessorInterface(domain=cls.domain) - form_processor.save_processed_models([form]) - return form - - from casexml.apps.case.tests.util import delete_all_xforms - delete_all_xforms() cls.now = datetime.datetime.utcnow() cls._60_days = datetime.timedelta(days=60) cls.domain = 'my_crazy_analytics_domain' cls.app_id = uuid.uuid4().hex cls.xmlns = 'my://crazy.xmlns/' + + def create_form(received_on, app_id=cls.app_id, xmlns=cls.xmlns): + metadata = TestFormMetadata(domain=cls.domain, app_id=app_id, + xmlns=xmlns, received_on=received_on) + form = get_form_ready_to_save(metadata, is_db_test=True) + form_processor = FormProcessorInterface(domain=cls.domain) + form_processor.save_processed_models([form]) + return form + + def create_forms_and_sync_to_es(): + forms = [] + with process_pillow_changes('xform-pillow', {'skip_ucr': True}): + with process_pillow_changes('DefaultChangeFeedPillow'): + for received_on in [cls.now, cls.now - cls._60_days]: + forms.append(create_form(received_on)) + forms.append(create_form(cls.now, app_id=None, xmlns="system")) + return forms + + from casexml.apps.case.tests.util import delete_all_xforms + delete_all_xforms() with trap_extra_setup(ConnectionError): cls.elasticsearch = get_es_new() initialize_index_and_mapping(cls.elasticsearch, XFORM_INDEX_INFO) - cls.forms = [create_form_and_sync_to_es(cls.now), create_form_and_sync_to_es(cls.now - cls._60_days)] + cls.forms = create_forms_and_sync_to_es() cls.elasticsearch.indices.refresh(XFORM_INDEX_INFO.alias) @@ -160,7 +169,7 @@ def tearDownClass(cls): FormProcessorTestUtils.delete_all_cases_forms_ledgers(cls.domain) super(CouchformsESAnalyticsTest, cls).tearDownClass() - def test_get_number_of_cases_in_domain(self): + def test_get_number_of_forms_in_domain(self): self.assertEqual( get_number_of_forms_in_domain(self.domain), len(self.forms) @@ -187,4 +196,4 @@ def test_app_has_been_submitted_to_in_last_30_days(self): def test_get_all_xmlns_app_id_pairs_submitted_to_in_domain(self): self.assertEqual( get_all_xmlns_app_id_pairs_submitted_to_in_domain(self.domain), - {(self.xmlns, self.app_id)}) + {(self.xmlns, self.app_id), ("system", MISSING_APP_ID)}) From 6c145daa0fe7e604b1dccd62dc8611164dd761f7 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Fri, 20 May 2022 13:30:41 +0200 Subject: [PATCH 4/7] exclude forms with no app_id by default --- corehq/apps/reports/standard/inspect.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/corehq/apps/reports/standard/inspect.py b/corehq/apps/reports/standard/inspect.py index a176f17b05cb..44cb5cd4b6d2 100644 --- a/corehq/apps/reports/standard/inspect.py +++ b/corehq/apps/reports/standard/inspect.py @@ -119,6 +119,8 @@ def es_query(self): form_values = list(self.all_relevant_forms.values()) if form_values: query = query.OR(*[self._form_filter(f) for f in form_values]) + else: + query = query.NOT(es_filters.missing("app_id")) return query From 8fda9ee8c8e9afa46d99f8ed366f010f52706deb Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Fri, 20 May 2022 13:32:37 +0200 Subject: [PATCH 5/7] get name for system form name map --- corehq/apps/reports/filters/forms.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/corehq/apps/reports/filters/forms.py b/corehq/apps/reports/filters/forms.py index ef9bfddc0f52..3890cd8f0899 100644 --- a/corehq/apps/reports/filters/forms.py +++ b/corehq/apps/reports/filters/forms.py @@ -7,6 +7,7 @@ from couchdbkit.exceptions import ResourceNotFound from memoized import memoized +from corehq.apps.hqcase.utils import SYSTEM_FORM_XMLNS_MAP from couchforms.analytics import ( get_all_xmlns_app_id_pairs_submitted_to_in_domain, ) @@ -404,7 +405,13 @@ def get_unknown_form_name(self, xmlns, app_id=None, none_if_not_found=False): return list(form['name'].values())[0] guessed_name = guess_form_name_from_submissions_using_xmlns(self.domain, xmlns) - return guessed_name or (None if none_if_not_found else _("Name Unknown")) + if guessed_name: + return guessed_name + + if xmlns in SYSTEM_FORM_XMLNS_MAP: + return SYSTEM_FORM_XMLNS_MAP[xmlns] + + return None if none_if_not_found else _("Name Unknown") @staticmethod def get_translated_value(display_lang, app_langs, obj): From a5be2b2d61f0f1accb3bf6bc87fafd4fa8541b87 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Fri, 20 May 2022 13:33:50 +0200 Subject: [PATCH 6/7] translate system form names --- corehq/apps/hqcase/utils.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/corehq/apps/hqcase/utils.py b/corehq/apps/hqcase/utils.py index b4063343f1e2..6b462524b7c3 100644 --- a/corehq/apps/hqcase/utils.py +++ b/corehq/apps/hqcase/utils.py @@ -3,19 +3,17 @@ from xml.etree import cElementTree as ElementTree from django.template.loader import render_to_string +from django.utils.translation import gettext_lazy from casexml.apps.case.mock import CaseBlock from casexml.apps.case.util import property_changed_in_action -from corehq.apps.es.cases import CaseES -from corehq.apps.es import filters from dimagi.utils.parsing import json_format_datetime +from corehq.apps.es import filters +from corehq.apps.es.cases import CaseES from corehq.apps.receiverwrapper.util import submit_form_locally from corehq.apps.users.util import SYSTEM_USER_ID -from corehq.form_processor.exceptions import ( - CaseNotFound, - MissingFormXml, -) +from corehq.form_processor.exceptions import CaseNotFound, MissingFormXml from corehq.form_processor.models import CommCareCase CASEBLOCK_CHUNKSIZE = 100 @@ -23,8 +21,8 @@ EDIT_FORM_XMLNS = 'http://commcarehq.org/case/edit' SYSTEM_FORM_XMLNS_MAP = { - SYSTEM_FORM_XMLNS: 'System Form', - EDIT_FORM_XMLNS: 'Data Cleaning Form', + SYSTEM_FORM_XMLNS: gettext_lazy('System Form'), + EDIT_FORM_XMLNS: gettext_lazy('Data Cleaning Form'), } ALLOWED_CASE_IDENTIFIER_TYPES = [ From d8d4346e5d61e41b8ac2e2ea6dcdcfdfd547c2aa Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Fri, 20 May 2022 14:23:50 +0200 Subject: [PATCH 7/7] improve label and help text wording --- .../filters/form_app_module_drilldown.html | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/corehq/apps/reports/templates/reports/filters/form_app_module_drilldown.html b/corehq/apps/reports/templates/reports/filters/form_app_module_drilldown.html index b7e2cee0c1e9..a84d58ac88da 100644 --- a/corehq/apps/reports/templates/reports/filters/form_app_module_drilldown.html +++ b/corehq/apps/reports/templates/reports/filters/form_app_module_drilldown.html @@ -15,8 +15,11 @@ {% trans 'Known Forms' %} + data-content="{% blocktrans %} + These are forms that can be matched to an existing or deleted CommCare Application + in your project. + {% endblocktrans %}"> +
@@ -27,11 +30,15 @@ name="{{ slug }}_{{ unknown.slug }}" id="{{ css_id }}_{{ unknown.slug }}_show" value="yes"> - {% trans 'Unknown Forms (Possibly Deleted)' %} + {% trans 'Unknown Forms' %} + data-content="{% blocktrans %} + These forms do not belong to a CommCare Application (existing or deleted) in your project. + It might mean that these forms once belonged to an application but the application form + has since been removed. They may also be 'system forms' or forms from an integration or API. + {% endblocktrans %}"> +