From 12f4a54e9c291db352a3c45d3ea9fcc1cb6513dd Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 14:18:59 +0200 Subject: [PATCH 01/12] update docstring with latest best practice --- corehq/apps/hqcase/utils.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/corehq/apps/hqcase/utils.py b/corehq/apps/hqcase/utils.py index 95c033922be6..535250f508f5 100644 --- a/corehq/apps/hqcase/utils.py +++ b/corehq/apps/hqcase/utils.py @@ -39,17 +39,16 @@ def submit_case_blocks(case_blocks, domain, username="system", user_id=None, """ Submits casexml in a manner similar to how they would be submitted from a phone. - :param xmlns: Form XMLNS. Format: IRI or URN. Historically this was - used in some places to uniquely identify the subsystem that posted - the cases; `device_id` is now recommended for that purpose. Going - forward, it is recommended to use the default value along with - `device_id`, which indicates that the cases were submitted by an - internal system process. + :param xmlns: Form XMLNS. Format: IRI or URN. This should be used to + identify the subsystem that posted the cases. This should be a constant + value without any dynamic components. Ideally the XMLNS should be + added to ``SYSTEM_FORM_XMLNS_MAP`` for more user-friendly display. + See ``SYSTEM_FORM_XMLNS_MAP`` form examples. :param device_id: Identifier for the source of posted cases. Ideally - this should uniquely identify the subsystem that is posting cases to - make it easier to trace the source. All new code should use this - argument. A human recognizable value is recommended outside of test - code. Example: "auto-close-rule-" + this should uniquely identify the exact subsystem configuration that + is posting cases to make it easier to trace the source. Used in combination with + XMLNS this allows pinpointing the exact source. Example: If the cases are being + generated by an Automatic Case Rule, then the device_id should be the rule's ID. :param form_extras: Dict of additional kwargs to pass through to ``SubmissionPost`` :param max_wait: Maximum time (in seconds) to allow the process to be delayed if the project is over its submission rate limit. From 3204cc1cdd75d79d6edfa264d6cb7f94c8202681 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 14:22:04 +0200 Subject: [PATCH 02/12] rename form_extras to submission_extras --- corehq/apps/hqcase/utils.py | 8 +++---- .../casexml/apps/case/cleanup.py | 6 ++--- .../casexml/apps/case/mock/mock.py | 22 +++++++++---------- .../casexml/apps/case/tests/test_factory.py | 8 +++---- .../casexml/apps/case/tests/test_rebuild.py | 20 ++++++++--------- .../casexml/apps/phone/tests/test_new_sync.py | 6 ++--- .../apps/phone/tests/test_sync_mode.py | 4 ++-- .../ex-submodules/casexml/apps/phone/utils.py | 2 +- 8 files changed, 38 insertions(+), 38 deletions(-) diff --git a/corehq/apps/hqcase/utils.py b/corehq/apps/hqcase/utils.py index 535250f508f5..29480154d349 100644 --- a/corehq/apps/hqcase/utils.py +++ b/corehq/apps/hqcase/utils.py @@ -35,7 +35,7 @@ def submit_case_blocks(case_blocks, domain, username="system", user_id=None, xmlns=None, attachments=None, form_id=None, - form_extras=None, case_db=None, device_id=None, max_wait=...): + submission_extras=None, case_db=None, device_id=None, max_wait=...): """ Submits casexml in a manner similar to how they would be submitted from a phone. @@ -49,14 +49,14 @@ def submit_case_blocks(case_blocks, domain, username="system", user_id=None, is posting cases to make it easier to trace the source. Used in combination with XMLNS this allows pinpointing the exact source. Example: If the cases are being generated by an Automatic Case Rule, then the device_id should be the rule's ID. - :param form_extras: Dict of additional kwargs to pass through to ``SubmissionPost`` + :param submission_extras: Dict of additional kwargs to pass through to ``SubmissionPost`` :param max_wait: Maximum time (in seconds) to allow the process to be delayed if the project is over its submission rate limit. See the docstring for submit_form_locally for meaning of values. returns the UID of the resulting form. """ - form_extras = form_extras or {} + submission_extras = submission_extras or {} attachments = attachments or {} now = json_format_datetime(datetime.datetime.utcnow()) if not isinstance(case_blocks, str): @@ -78,7 +78,7 @@ def submit_case_blocks(case_blocks, domain, username="system", user_id=None, attachments=attachments, case_db=case_db, max_wait=max_wait, - **form_extras + **submission_extras ) return result.xform, result.cases diff --git a/corehq/ex-submodules/casexml/apps/case/cleanup.py b/corehq/ex-submodules/casexml/apps/case/cleanup.py index ed3c99c3653d..1f225941c1fa 100644 --- a/corehq/ex-submodules/casexml/apps/case/cleanup.py +++ b/corehq/ex-submodules/casexml/apps/case/cleanup.py @@ -119,9 +119,9 @@ def claim_case(domain, restore_user, host_id, host_type=None, host_name=None, de ) } ).as_xml() - form_extras = {} + submission_extras = {} if restore_user.request_user: - form_extras["auth_context"] = AuthContext( + submission_extras["auth_context"] = AuthContext( domain=domain, user_id=restore_user.request_user_id, authenticated=True @@ -129,7 +129,7 @@ def claim_case(domain, restore_user, host_id, host_type=None, host_name=None, de submit_case_blocks( [ElementTree.tostring(claim_case_block, encoding='utf-8').decode('utf-8')], domain=domain, - form_extras=form_extras, + submission_extras=submission_extras, username=restore_user.full_username, user_id=restore_user.user_id, device_id=device_id, diff --git a/corehq/ex-submodules/casexml/apps/case/mock/mock.py b/corehq/ex-submodules/casexml/apps/case/mock/mock.py index 91489178d58c..dd4a07e8d621 100644 --- a/corehq/ex-submodules/casexml/apps/case/mock/mock.py +++ b/corehq/ex-submodules/casexml/apps/case/mock/mock.py @@ -70,10 +70,10 @@ class CaseFactory(object): creates and saves the case directly, which is faster. """ - def __init__(self, domain=None, case_defaults=None, form_extras=None): + def __init__(self, domain=None, case_defaults=None, submission_extras=None): self.domain = domain or 'test-domain' self.case_defaults = case_defaults if case_defaults is not None else {} - self.form_extras = form_extras if form_extras is not None else {} + self.submission_extras = submission_extras if submission_extras is not None else {} def get_case_block(self, case_id, **kwargs): for k, v in self.case_defaults.items(): @@ -97,14 +97,14 @@ def get_blocks(structure): return [block for structure in case_structures for block in get_blocks(structure)] - def post_case_blocks(self, caseblocks, form_extras=None, user_id=None, device_id=None, xmlns=None): + def post_case_blocks(self, caseblocks, submission_extras=None, user_id=None, device_id=None, xmlns=None): from corehq.apps.hqcase.utils import submit_case_blocks - submit_form_extras = copy.copy(self.form_extras) - if form_extras is not None: - submit_form_extras.update(form_extras) + submit_form_extras = copy.copy(self.submission_extras) + if submission_extras is not None: + submit_form_extras.update(submission_extras) return submit_case_blocks( caseblocks, - form_extras=submit_form_extras, + submission_extras=submit_form_extras, domain=self.domain, user_id=user_id, device_id=device_id, @@ -144,16 +144,16 @@ def close_case(self, case_id): """ return self.create_or_update_case(CaseStructure(case_id=case_id, attrs={'close': True}))[0] - def create_or_update_case(self, case_structure, form_extras=None, user_id=None, device_id=None, xmlns=None): + def create_or_update_case(self, case_structure, submission_extras=None, user_id=None, device_id=None, xmlns=None): return self.create_or_update_cases( - [case_structure], form_extras=form_extras, user_id=user_id, device_id=device_id, xmlns=xmlns + [case_structure], submission_extras=submission_extras, user_id=user_id, device_id=device_id, xmlns=xmlns ) - def create_or_update_cases(self, case_structures, form_extras=None, user_id=None, device_id=None, xmlns=None): + def create_or_update_cases(self, case_structures, submission_extras=None, user_id=None, device_id=None, xmlns=None): from corehq.form_processor.models import CommCareCase self.post_case_blocks( [b.as_text() for b in self.get_case_blocks(case_structures)], - form_extras, + submission_extras, user_id=user_id, device_id=device_id, xmlns=xmlns, diff --git a/corehq/ex-submodules/casexml/apps/case/tests/test_factory.py b/corehq/ex-submodules/casexml/apps/case/tests/test_factory.py index d2891b13fd9c..efdba20a4947 100644 --- a/corehq/ex-submodules/casexml/apps/case/tests/test_factory.py +++ b/corehq/ex-submodules/casexml/apps/case/tests/test_factory.py @@ -175,7 +175,7 @@ def test_form_extras(self): token_id = uuid.uuid4().hex factory = CaseFactory(domain=domain) [case] = factory.create_or_update_case(CaseStructure( - attrs={'create': True}), form_extras={'last_sync_token': token_id}) + attrs={'create': True}), submission_extras={'last_sync_token': token_id}) form = XFormInstance.objects.get_form(case.xform_ids[0], domain) self.assertEqual(token_id, form.last_sync_token) @@ -184,7 +184,7 @@ def test_form_extras_default(self): # have to enable loose sync token validation for the domain or create actual SyncLog documents. # this is the easier path. token_id = uuid.uuid4().hex - factory = CaseFactory(domain=domain, form_extras={'last_sync_token': token_id}) + factory = CaseFactory(domain=domain, submission_extras={'last_sync_token': token_id}) case = factory.create_case() form = XFormInstance.objects.get_form(case.xform_ids[0], domain) self.assertEqual(token_id, form.last_sync_token) @@ -192,8 +192,8 @@ def test_form_extras_default(self): def test_form_extras_override_defaults(self): domain = uuid.uuid4().hex token_id = uuid.uuid4().hex - factory = CaseFactory(domain=domain, form_extras={'last_sync_token': token_id}) + factory = CaseFactory(domain=domain, submission_extras={'last_sync_token': token_id}) [case] = factory.create_or_update_case(CaseStructure( - attrs={'create': True}), form_extras={'last_sync_token': 'differenttoken'}) + attrs={'create': True}), submission_extras={'last_sync_token': 'differenttoken'}) form = XFormInstance.objects.get_form(case.xform_ids[0], domain) self.assertEqual('differenttoken', form.last_sync_token) diff --git a/corehq/ex-submodules/casexml/apps/case/tests/test_rebuild.py b/corehq/ex-submodules/casexml/apps/case/tests/test_rebuild.py index 7ffc1bb49327..59c014af5841 100644 --- a/corehq/ex-submodules/casexml/apps/case/tests/test_rebuild.py +++ b/corehq/ex-submodules/casexml/apps/case/tests/test_rebuild.py @@ -17,7 +17,7 @@ def _post_util(create=False, case_id=None, user_id=None, owner_id=None, - case_type=None, form_extras=None, close=False, date_modified=None, + case_type=None, submission_extras=None, close=False, date_modified=None, **kwargs): def uid(): @@ -31,7 +31,7 @@ def uid(): date_modified=date_modified, update=kwargs, close=close) - submit_case_blocks([block.as_text()], REBUILD_TEST_DOMAIN, form_extras=form_extras) + submit_case_blocks([block.as_text()], REBUILD_TEST_DOMAIN, submission_extras=submission_extras) return case_id @@ -97,11 +97,11 @@ def test_form_archiving(self): now = datetime.utcnow() # make sure we timestamp everything so they have the right order case_id = _post_util(create=True, p1='p1-1', p2='p2-1', - form_extras={'received_on': now}) + submission_extras={'received_on': now}) _post_util(case_id=case_id, p2='p2-2', p3='p3-2', p4='p4-2', - form_extras={'received_on': now + timedelta(seconds=1)}) + submission_extras={'received_on': now + timedelta(seconds=1)}) _post_util(case_id=case_id, p4='p4-3', p5='p5-3', close=True, - form_extras={'received_on': now + timedelta(seconds=2)}) + submission_extras={'received_on': now + timedelta(seconds=2)}) case = CommCareCase.objects.get_case(case_id, REBUILD_TEST_DOMAIN) closed_by = case.closed_by @@ -208,11 +208,11 @@ def test_archie_modified_on(self): # make sure we timestamp everything so they have the right order create_block = CaseBlock(case_id, create=True, date_modified=way_earlier) submit_case_blocks( - [create_block.as_text()], 'test-domain', form_extras={'received_on': way_earlier} + [create_block.as_text()], 'test-domain', submission_extras={'received_on': way_earlier} ) update_block = CaseBlock(case_id, update={'foo': 'bar'}, date_modified=earlier) submit_case_blocks( - [update_block.as_text()], 'test-domain', form_extras={'received_on': earlier} + [update_block.as_text()], 'test-domain', submission_extras={'received_on': earlier} ) case = CommCareCase.objects.get_case(case_id, 'test-domain') @@ -226,11 +226,11 @@ def test_archie_modified_on(self): def test_archive_against_deleted_case(self): now = datetime.utcnow() # make sure we timestamp everything so they have the right order - case_id = _post_util(create=True, p1='p1', form_extras={'received_on': now}) + case_id = _post_util(create=True, p1='p1', submission_extras={'received_on': now}) _post_util(case_id=case_id, p2='p2', - form_extras={'received_on': now + timedelta(seconds=1)}) + submission_extras={'received_on': now + timedelta(seconds=1)}) _post_util(case_id=case_id, p3='p3', - form_extras={'received_on': now + timedelta(seconds=2)}) + submission_extras={'received_on': now + timedelta(seconds=2)}) case = CommCareCase.objects.get_case(case_id, REBUILD_TEST_DOMAIN) CommCareCase.objects.soft_delete_cases(REBUILD_TEST_DOMAIN, [case_id]) diff --git a/corehq/ex-submodules/casexml/apps/phone/tests/test_new_sync.py b/corehq/ex-submodules/casexml/apps/phone/tests/test_new_sync.py index b0214e0f4ee4..8345ae6e702b 100644 --- a/corehq/ex-submodules/casexml/apps/phone/tests/test_new_sync.py +++ b/corehq/ex-submodules/casexml/apps/phone/tests/test_new_sync.py @@ -84,7 +84,7 @@ def test_legacy_support_toggle(self): factory.create_or_update_cases([ CaseStructure(case_id=parent_id, attrs={'owner_id': 'different'}), CaseStructure(case_id=child_id, attrs={'owner_id': 'different'}), - ], form_extras={'last_sync_token': sync_log._id}) + ], submission_extras={'last_sync_token': sync_log._id}) # doing it again should fail since they are no longer relevant @@ -93,7 +93,7 @@ def test_legacy_support_toggle(self): # factory.create_or_update_cases([ # CaseStructure(case_id=child_id, attrs={'owner_id': 'different'}), # CaseStructure(case_id=parent_id, attrs={'owner_id': 'different'}), - # ], form_extras={'last_sync_token': sync_log._id}) + # ], submission_extras={'last_sync_token': sync_log._id}) # enabling the toggle should prevent the failure the second time # though we also need to hackily set the request object in the threadlocals @@ -103,4 +103,4 @@ def test_legacy_support_toggle(self): factory.create_or_update_cases([ CaseStructure(case_id=child_id, attrs={'owner_id': 'different'}), CaseStructure(case_id=parent_id, attrs={'owner_id': 'different'}), - ], form_extras={'last_sync_token': sync_log._id}) + ], submission_extras={'last_sync_token': sync_log._id}) diff --git a/corehq/ex-submodules/casexml/apps/phone/tests/test_sync_mode.py b/corehq/ex-submodules/casexml/apps/phone/tests/test_sync_mode.py index 5f783adb06ee..5bd77df08d38 100644 --- a/corehq/ex-submodules/casexml/apps/phone/tests/test_sync_mode.py +++ b/corehq/ex-submodules/casexml/apps/phone/tests/test_sync_mode.py @@ -118,7 +118,7 @@ def setUp(self): super(DeprecatedBaseSyncTest, self).setUp() self.sync_log = self.device.last_sync.log self.factory = self.device.case_factory - self.factory.form_extras = { + self.factory.submission_extras = { 'last_sync_token': self.sync_log._id, } @@ -1931,7 +1931,7 @@ def test_submission_with_bad_log_toggle_enabled(self): submit_case_blocks( [CaseBlock(create=True, case_id='bad-log-toggle-enabled').as_text()], 'submission-domain-with-toggle', - form_extras={"last_sync_token": 'not-a-valid-synclog-id'}, + submission_extras={"last_sync_token": 'not-a-valid-synclog-id'}, ) def test_restore_with_bad_log_toggle_enabled(self): diff --git a/corehq/ex-submodules/casexml/apps/phone/utils.py b/corehq/ex-submodules/casexml/apps/phone/utils.py index eadf73b6ffec..4fa68e938816 100644 --- a/corehq/ex-submodules/casexml/apps/phone/utils.py +++ b/corehq/ex-submodules/casexml/apps/phone/utils.py @@ -242,7 +242,7 @@ def post_changes(self, *args, **kw): form = self.case_factory.post_case_blocks( [b.as_text() for b in self.case_blocks], device_id=self.id, - form_extras={"last_sync_token": token}, + submission_extras={"last_sync_token": token}, user_id=self.user_id, )[0] self.case_blocks = [] From 5e19c374d9c9519ad10087158931301b17023527 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 14:27:09 +0200 Subject: [PATCH 03/12] lint --- .../casexml/apps/case/mock/mock.py | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/corehq/ex-submodules/casexml/apps/case/mock/mock.py b/corehq/ex-submodules/casexml/apps/case/mock/mock.py index dd4a07e8d621..70def0f8ecd3 100644 --- a/corehq/ex-submodules/casexml/apps/case/mock/mock.py +++ b/corehq/ex-submodules/casexml/apps/case/mock/mock.py @@ -94,8 +94,10 @@ def get_blocks(structure): for block in get_blocks(index.related_structure): yield block - return [block for structure in case_structures - for block in get_blocks(structure)] + return [ + block for structure in case_structures + for block in get_blocks(structure) + ] def post_case_blocks(self, caseblocks, submission_extras=None, user_id=None, device_id=None, xmlns=None): from corehq.apps.hqcase.utils import submit_case_blocks @@ -144,12 +146,20 @@ def close_case(self, case_id): """ return self.create_or_update_case(CaseStructure(case_id=case_id, attrs={'close': True}))[0] - def create_or_update_case(self, case_structure, submission_extras=None, user_id=None, device_id=None, xmlns=None): + def create_or_update_case( + self, case_structure, submission_extras=None, user_id=None, device_id=None, xmlns=None + ): return self.create_or_update_cases( - [case_structure], submission_extras=submission_extras, user_id=user_id, device_id=device_id, xmlns=xmlns + [case_structure], + submission_extras=submission_extras, + user_id=user_id, + device_id=device_id, + xmlns=xmlns ) - def create_or_update_cases(self, case_structures, submission_extras=None, user_id=None, device_id=None, xmlns=None): + def create_or_update_cases( + self, case_structures, submission_extras=None, user_id=None, device_id=None, xmlns=None + ): from corehq.form_processor.models import CommCareCase self.post_case_blocks( [b.as_text() for b in self.get_case_blocks(case_structures)], From 28dbf680d928d024e0c477f8b61d03503925625e Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 14:44:14 +0200 Subject: [PATCH 04/12] add form_name to submit_case_blocks --- corehq/apps/hqcase/templates/hqcase/xml/case_block.xml | 2 +- corehq/apps/hqcase/utils.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/corehq/apps/hqcase/templates/hqcase/xml/case_block.xml b/corehq/apps/hqcase/templates/hqcase/xml/case_block.xml index 600b1630a7fd..d2da4c26ccc8 100644 --- a/corehq/apps/hqcase/templates/hqcase/xml/case_block.xml +++ b/corehq/apps/hqcase/templates/hqcase/xml/case_block.xml @@ -1,5 +1,5 @@ - + {% if form_data %} {% for name, value in form_data.items %} <{{ name }}>{{ value }} diff --git a/corehq/apps/hqcase/utils.py b/corehq/apps/hqcase/utils.py index 29480154d349..83429f4da703 100644 --- a/corehq/apps/hqcase/utils.py +++ b/corehq/apps/hqcase/utils.py @@ -35,7 +35,7 @@ def submit_case_blocks(case_blocks, domain, username="system", user_id=None, xmlns=None, attachments=None, form_id=None, - submission_extras=None, case_db=None, device_id=None, max_wait=...): + submission_extras=None, case_db=None, device_id=None, form_name=None, max_wait=...): """ Submits casexml in a manner similar to how they would be submitted from a phone. @@ -49,6 +49,8 @@ def submit_case_blocks(case_blocks, domain, username="system", user_id=None, is posting cases to make it easier to trace the source. Used in combination with XMLNS this allows pinpointing the exact source. Example: If the cases are being generated by an Automatic Case Rule, then the device_id should be the rule's ID. + :param form_name: Human readable version of the device_id. For example the + Automatic Case Rule name. :param submission_extras: Dict of additional kwargs to pass through to ``SubmissionPost`` :param max_wait: Maximum time (in seconds) to allow the process to be delayed if the project is over its submission rate limit. @@ -64,6 +66,7 @@ def submit_case_blocks(case_blocks, domain, username="system", user_id=None, form_id = form_id or uuid.uuid4().hex form_xml = render_to_string('hqcase/xml/case_block.xml', { 'xmlns': xmlns or SYSTEM_FORM_XMLNS, + 'name': form_name, 'case_block': case_blocks, 'time': now, 'uid': form_id, From 7141f86cd08f2490772339b67cb29c0be8d319f4 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 14:51:17 +0200 Subject: [PATCH 05/12] pass form_name from case update rules --- corehq/apps/data_interfaces/models.py | 4 ++-- corehq/apps/hqcase/utils.py | 6 ++++-- custom/covid/rules/custom_actions.py | 3 +++ custom/gcc_sangath/rules/custom_actions.py | 1 + 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/corehq/apps/data_interfaces/models.py b/corehq/apps/data_interfaces/models.py index 5bf3b9262df9..f49976c2b9b2 100644 --- a/corehq/apps/data_interfaces/models.py +++ b/corehq/apps/data_interfaces/models.py @@ -910,7 +910,7 @@ def when_case_matches(self, case, rule): if case_id == case.case_id: continue result = update_case(case.domain, case_id, case_properties=properties, close=False, - xmlns=AUTO_UPDATE_XMLNS, max_wait=15, device_id=rule.id) + xmlns=AUTO_UPDATE_XMLNS, max_wait=15, device_id=rule.id, form_name=rule.name) rule.log_submission(result[0].form_id) num_related_updates += 1 @@ -923,7 +923,7 @@ def when_case_matches(self, case, rule): if close_case or properties: result = update_case(case.domain, case.case_id, case_properties=properties, close=close_case, - xmlns=AUTO_UPDATE_XMLNS, max_wait=15, device_id=rule.id) + xmlns=AUTO_UPDATE_XMLNS, max_wait=15, device_id=rule.id, form_name=rule.name) rule.log_submission(result[0].form_id) diff --git a/corehq/apps/hqcase/utils.py b/corehq/apps/hqcase/utils.py index 83429f4da703..48b52d08c421 100644 --- a/corehq/apps/hqcase/utils.py +++ b/corehq/apps/hqcase/utils.py @@ -135,7 +135,7 @@ def _get_update_or_close_case_block(case_id, case_properties=None, close=False, def update_case(domain, case_id, case_properties=None, close=False, - xmlns=None, device_id=None, owner_id=None, max_wait=...): + xmlns=None, device_id=None, form_name=None, owner_id=None, max_wait=...): """ Updates or closes a case (or both) by submitting a form. domain - the case's domain @@ -143,8 +143,9 @@ def update_case(domain, case_id, case_properties=None, close=False, case_properties - to update the case, pass in a dictionary of {name1: value1, ...} to ignore case updates, leave this argument out close - True to close the case, False otherwise - xmlns - pass in an xmlns to use it instead of the default + xmlns - see submit_case_blocks xmlns docs device_id - see submit_case_blocks device_id docs + form_name - see submit_case_blocks form_name docs max_wait - Maximum time (in seconds) to allow the process to be delayed if the project is over its submission rate limit. See the docstring for submit_form_locally for meaning of values @@ -156,6 +157,7 @@ def update_case(domain, case_id, case_properties=None, close=False, user_id=SYSTEM_USER_ID, xmlns=xmlns, device_id=device_id, + form_name=form_name, max_wait=max_wait ) diff --git a/custom/covid/rules/custom_actions.py b/custom/covid/rules/custom_actions.py index 8099028de2cb..61aa7525ef06 100644 --- a/custom/covid/rules/custom_actions.py +++ b/custom/covid/rules/custom_actions.py @@ -33,6 +33,7 @@ def set_all_activity_complete_date_to_today(case, rule): }, xmlns=AUTO_UPDATE_XMLNS, device_id=__name__ + ".set_all_activity_complete_date_to_today", + form_name=rule.name, ) rule.log_submission(submission.form_id) @@ -78,6 +79,7 @@ def close_cases_assigned_to_checkin(checkin_case, rule): case_properties=blank_properties, xmlns=AUTO_UPDATE_XMLNS, device_id=__name__ + ".close_cases_assigned_to_checkin", + form_name=rule.name, ) rule.log_submission(submission.form_id) @@ -87,6 +89,7 @@ def close_cases_assigned_to_checkin(checkin_case, rule): close=True, xmlns=AUTO_UPDATE_XMLNS, device_id=__name__ + ".close_cases_assigned_to_checkin", + form_name=rule.name, ) rule.log_submission(close_checkin_submission.form_id) diff --git a/custom/gcc_sangath/rules/custom_actions.py b/custom/gcc_sangath/rules/custom_actions.py index cc24f71905aa..a8aaf9383be7 100644 --- a/custom/gcc_sangath/rules/custom_actions.py +++ b/custom/gcc_sangath/rules/custom_actions.py @@ -32,6 +32,7 @@ def sanitize_session_peer_rating(session_case, rule): case_properties=case_updates, xmlns=AUTO_UPDATE_XMLNS, device_id=__name__ + ".sanitize_session_peer_rating", + form_name=rule.name, ) num_updates = 1 rule.log_submission(submission.form_id) From 666e0e00e6b68e89bde393910dd2cfa755eba2fc Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 15:16:48 +0200 Subject: [PATCH 06/12] refactor _FormType --- corehq/apps/reports/display.py | 70 +++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/corehq/apps/reports/display.py b/corehq/apps/reports/display.py index cc7f54153888..d7de491781ae 100644 --- a/corehq/apps/reports/display.py +++ b/corehq/apps/reports/display.py @@ -88,39 +88,49 @@ def __init__(self, domain, xmlns, app_id): def get_label(self, lang=None, separator=None): if separator is None: separator = " > " + + return ( + self.get_label_from_app(lang, separator) + or self.get_name_from_xml() + or self.xmlns + ) + + def get_label_from_app(self, lang, separator): form = get_form_analytics_metadata(self.domain, self.app_id, self.xmlns) - if form and form.get('app'): - langs = form['app']['langs'] - app_name = form['app']['name'] + app = form and form.get('app') + if not app: + return - if form.get('is_user_registration'): - form_name = "User Registration" - title = separator.join([app_name, form_name]) - else: - def _menu_name(menu, lang): - if lang and menu.get(lang): - return menu.get(lang) - else: - for lang in langs + list(menu): - menu_name = menu.get(lang) - if menu_name is not None: - return menu_name - return "?" - - module_name = _menu_name(form["module"]["name"], lang) - form_name = _menu_name(form["form"]["name"], lang) - title = separator.join([app_name, module_name, form_name]) - - if form.get('app_deleted'): - title += ' [Deleted]' - if form.get('duplicate'): - title += " [Multiple Forms]" - name = title - elif self.xmlns in SYSTEM_FORM_XMLNS_MAP: - name = SYSTEM_FORM_XMLNS_MAP[self.xmlns] + langs = form['app']['langs'] + app_name = form['app']['name'] + + if form.get('is_user_registration'): + form_name = "User Registration" + title = separator.join([app_name, form_name]) else: - name = self.xmlns - return name + def _menu_name(menu, lang): + if lang and menu.get(lang): + return menu.get(lang) + else: + for lang in langs + list(menu): + menu_name = menu.get(lang) + if menu_name is not None: + return menu_name + return "?" + + module_name = _menu_name(form["module"]["name"], lang) + form_name = _menu_name(form["form"]["name"], lang) + title = separator.join([app_name, module_name, form_name]) + + if form.get('app_deleted'): + title += ' [Deleted]' + if form.get('duplicate'): + title += " [Multiple Forms]" + return title + + def get_name_from_xml(self): + if self.xmlns in SYSTEM_FORM_XMLNS_MAP: + return SYSTEM_FORM_XMLNS_MAP[self.xmlns] def xmlns_to_name(domain, xmlns, app_id, lang=None, separator=None): From 8121700cb8d5c9b54f5d177c639e7a441c56894a Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 15:20:38 +0200 Subject: [PATCH 07/12] include form name in non-app forms if supplied --- corehq/apps/reports/display.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/corehq/apps/reports/display.py b/corehq/apps/reports/display.py index d7de491781ae..d0f322f359f5 100644 --- a/corehq/apps/reports/display.py +++ b/corehq/apps/reports/display.py @@ -80,10 +80,11 @@ def readable_form_name(self): class _FormType(object): - def __init__(self, domain, xmlns, app_id): + def __init__(self, domain, xmlns, app_id, form_name): self.domain = domain self.xmlns = xmlns self.app_id = app_id + self.form_name = form_name def get_label(self, lang=None, separator=None): if separator is None: @@ -91,8 +92,8 @@ def get_label(self, lang=None, separator=None): return ( self.get_label_from_app(lang, separator) - or self.get_name_from_xml() - or self.xmlns + or self.get_name_from_xml(separator) + or self.append_form_name(self.xmlns, separator) ) def get_label_from_app(self, lang, separator): @@ -128,10 +129,16 @@ def _menu_name(menu, lang): title += " [Multiple Forms]" return title - def get_name_from_xml(self): + def get_name_from_xml(self, separator): if self.xmlns in SYSTEM_FORM_XMLNS_MAP: - return SYSTEM_FORM_XMLNS_MAP[self.xmlns] + readable_xmlns = SYSTEM_FORM_XMLNS_MAP[self.xmlns] + return self.append_form_name(readable_xmlns, separator) + def append_form_name(self, name, separator): + if self.form_name: + name = separator.join([name, self.form_name]) + return name -def xmlns_to_name(domain, xmlns, app_id, lang=None, separator=None): - return _FormType(domain, xmlns, app_id).get_label(lang, separator) + +def xmlns_to_name(domain, xmlns, app_id, lang=None, separator=None, form_name=None): + return _FormType(domain, xmlns, app_id, form_name).get_label(lang, separator) From 2273a91745459f2e2a293afb6c512fb6cdccca4f Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 15:46:17 +0200 Subject: [PATCH 08/12] tests for xmlns_to_name --- corehq/apps/reports/tests/test_display.py | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 corehq/apps/reports/tests/test_display.py diff --git a/corehq/apps/reports/tests/test_display.py b/corehq/apps/reports/tests/test_display.py new file mode 100644 index 000000000000..e67925a64580 --- /dev/null +++ b/corehq/apps/reports/tests/test_display.py @@ -0,0 +1,24 @@ +from django.test import SimpleTestCase + +from corehq.apps.hqcase.utils import SYSTEM_FORM_XMLNS, SYSTEM_FORM_XMLNS_MAP + +from corehq.apps.reports.display import xmlns_to_name +from corehq.util.test_utils import generate_cases + +UNKNOWN_XMLNS = "http://demo.co/form" +KNOWN_XMLNS = SYSTEM_FORM_XMLNS +KNOWN_XMLNS_DISPLAY = SYSTEM_FORM_XMLNS_MAP[KNOWN_XMLNS] + + +class TestXmlnsToName(SimpleTestCase): + @generate_cases([ + (UNKNOWN_XMLNS, UNKNOWN_XMLNS), + (f"{UNKNOWN_XMLNS} > form name", UNKNOWN_XMLNS, "form name"), + (f"{UNKNOWN_XMLNS} ] form name", UNKNOWN_XMLNS, "form name", " ] "), + (KNOWN_XMLNS_DISPLAY, KNOWN_XMLNS), + (f"{KNOWN_XMLNS_DISPLAY} > form name", KNOWN_XMLNS, "form name"), + (f"{KNOWN_XMLNS_DISPLAY} ] form name", KNOWN_XMLNS, "form name", " ] "), + ]) + def test_xmlns_to_name(self, expected, xmlns, form_name=None, separator=None): + name = xmlns_to_name("domain", xmlns, "123", separator=separator, form_name=form_name) + self.assertEqual(name, expected) From 7601265b4db32c8aabbc760b537eedb432355956 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 15:46:32 +0200 Subject: [PATCH 09/12] patch DB lookup --- corehq/apps/reports/tests/test_display.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/corehq/apps/reports/tests/test_display.py b/corehq/apps/reports/tests/test_display.py index e67925a64580..c63ac6ddb5a0 100644 --- a/corehq/apps/reports/tests/test_display.py +++ b/corehq/apps/reports/tests/test_display.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + from django.test import SimpleTestCase from corehq.apps.hqcase.utils import SYSTEM_FORM_XMLNS, SYSTEM_FORM_XMLNS_MAP @@ -10,6 +12,7 @@ KNOWN_XMLNS_DISPLAY = SYSTEM_FORM_XMLNS_MAP[KNOWN_XMLNS] +@patch("corehq.apps.reports.display.get_form_analytics_metadata", new=lambda *a, **k: None) class TestXmlnsToName(SimpleTestCase): @generate_cases([ (UNKNOWN_XMLNS, UNKNOWN_XMLNS), From c16f7b722ac06326cf3a990d6624667b82aa911a Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 15:48:15 +0200 Subject: [PATCH 10/12] coerce to string --- corehq/apps/reports/display.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corehq/apps/reports/display.py b/corehq/apps/reports/display.py index d0f322f359f5..0f3d9ea430b1 100644 --- a/corehq/apps/reports/display.py +++ b/corehq/apps/reports/display.py @@ -131,7 +131,7 @@ def _menu_name(menu, lang): def get_name_from_xml(self, separator): if self.xmlns in SYSTEM_FORM_XMLNS_MAP: - readable_xmlns = SYSTEM_FORM_XMLNS_MAP[self.xmlns] + readable_xmlns = str(SYSTEM_FORM_XMLNS_MAP[self.xmlns]) return self.append_form_name(readable_xmlns, separator) def append_form_name(self, name, separator): From 39b38f222aa0ced02d235835acc9c84ac446e0a7 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 15:49:53 +0200 Subject: [PATCH 11/12] update calls to xmlns_to_name to include name from xml --- corehq/apps/reports/display.py | 5 +++-- corehq/apps/reports/standard/cases/case_data.py | 1 + corehq/apps/reports/standard/forms/reports.py | 1 + corehq/ex-submodules/casexml/apps/case/util.py | 3 ++- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/corehq/apps/reports/display.py b/corehq/apps/reports/display.py index 0f3d9ea430b1..d9422696836e 100644 --- a/corehq/apps/reports/display.py +++ b/corehq/apps/reports/display.py @@ -26,7 +26,7 @@ def replace(self, *args): return StringWithAttributes(string) -class FormDisplay(): +class FormDisplay: def __init__(self, form_doc, report, lang=None): self.form = form_doc @@ -74,7 +74,8 @@ def readable_form_name(self): self.report.domain, self.form.get("xmlns"), app_id=self.form.get("app_id"), - lang=self.lang + lang=self.lang, + form_name=self.form.get("@name"), ) diff --git a/corehq/apps/reports/standard/cases/case_data.py b/corehq/apps/reports/standard/cases/case_data.py index b78425fb94d8..cb26bbdd8274 100644 --- a/corehq/apps/reports/standard/cases/case_data.py +++ b/corehq/apps/reports/standard/cases/case_data.py @@ -268,6 +268,7 @@ def form_to_json(domain, form, timezone): form.xmlns, app_id=form.app_id, lang=get_language(), + form_name=form.form_data.get('@name') ) received_on = ServerTime(form.received_on).user_time(timezone).done().strftime(DATE_FORMAT) diff --git a/corehq/apps/reports/standard/forms/reports.py b/corehq/apps/reports/standard/forms/reports.py index b4744227a809..81604f021593 100644 --- a/corehq/apps/reports/standard/forms/reports.py +++ b/corehq/apps/reports/standard/forms/reports.py @@ -142,6 +142,7 @@ def _fmt_date(somedate): self.domain, xform_dict.get('xmlns'), app_id=xform_dict.get('app_id'), + form_name=xform_dict['form'].get('@name'), ) form_username = xform_dict['form']['meta'].get('username', EMPTY_USER) else: diff --git a/corehq/ex-submodules/casexml/apps/case/util.py b/corehq/ex-submodules/casexml/apps/case/util.py index c801053ea847..0c7c1960d93d 100644 --- a/corehq/ex-submodules/casexml/apps/case/util.py +++ b/corehq/ex-submodules/casexml/apps/case/util.py @@ -187,12 +187,13 @@ def get_case_history(case): changes = defaultdict(dict) for form in XFormInstance.objects.get_forms(case.xform_ids, case.domain): + name = xmlns_to_name(case.domain, form.xmlns, form.app_id, form_name=form.form_data.get('@name')) case_blocks = extract_case_blocks(form) for block in case_blocks: if block.get('@case_id') == case.case_id: property_changes = { 'Form ID': form.form_id, - 'Form Name': xmlns_to_name(case.domain, form.xmlns, form.app_id), + 'Form Name': name, 'Form Received On': form.received_on, 'Form Submitted By': form.metadata.username, } From db0a758d5d345b5a7922efec86cd3bcb8e37b576 Mon Sep 17 00:00:00 2001 From: Simon Kelly Date: Thu, 21 Jul 2022 15:50:14 +0200 Subject: [PATCH 12/12] extract defaults out of loop --- corehq/ex-submodules/casexml/apps/case/util.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/corehq/ex-submodules/casexml/apps/case/util.py b/corehq/ex-submodules/casexml/apps/case/util.py index 0c7c1960d93d..9fbfd116bdc3 100644 --- a/corehq/ex-submodules/casexml/apps/case/util.py +++ b/corehq/ex-submodules/casexml/apps/case/util.py @@ -188,15 +188,16 @@ def get_case_history(case): changes = defaultdict(dict) for form in XFormInstance.objects.get_forms(case.xform_ids, case.domain): name = xmlns_to_name(case.domain, form.xmlns, form.app_id, form_name=form.form_data.get('@name')) + defaults = { + 'Form ID': form.form_id, + 'Form Name': name, + 'Form Received On': form.received_on, + 'Form Submitted By': form.metadata.username, + } case_blocks = extract_case_blocks(form) for block in case_blocks: if block.get('@case_id') == case.case_id: - property_changes = { - 'Form ID': form.form_id, - 'Form Name': name, - 'Form Received On': form.received_on, - 'Form Submitted By': form.metadata.username, - } + property_changes = defaults.copy() property_changes.update(block.get('create', {})) property_changes.update(block.get('update', {})) changes[form.form_id].update(property_changes)