From a0e0a681c9b3c4016609f977b39cf4ed50cfa2ff Mon Sep 17 00:00:00 2001 From: Ethan Soergel Date: Tue, 29 Oct 2024 12:48:10 -0400 Subject: [PATCH 1/5] Move location fields out of user data This is an alternative approach to https://github.com/dimagi/commcare-hq/pull/35292 I move location data out of user data entirely, injecting it at the last minute. We already did this for web users as of https://github.com/dimagi/commcare-hq/pull/34401, but there, the fields were _always_ present, whereas for mobile workers, they're only present if not null. This commit brings web users in line with the behavior for mobile workers, for better parity --- corehq/apps/api/resources/v0_1.py | 8 ++++ corehq/apps/callcenter/sync_usercase.py | 6 +-- .../apps/user_importer/tests/test_importer.py | 11 ------ corehq/apps/users/models.py | 14 +++---- corehq/apps/users/tests/test_user_data.py | 4 +- corehq/apps/users/user_data.py | 39 +++---------------- 6 files changed, 25 insertions(+), 57 deletions(-) diff --git a/corehq/apps/api/resources/v0_1.py b/corehq/apps/api/resources/v0_1.py index 3797f8e049b4..1181187c763a 100644 --- a/corehq/apps/api/resources/v0_1.py +++ b/corehq/apps/api/resources/v0_1.py @@ -17,6 +17,7 @@ from corehq.apps.groups.models import Group from corehq.apps.user_importer.helpers import UserChangeLogger from corehq.apps.users.models import CommCareUser, HqPermissions, WebUser +from corehq.apps.users.util import user_location_data from corehq.const import USER_CHANGE_VIA_API @@ -105,6 +106,13 @@ def dehydrate(self, bundle): def dehydrate_user_data(self, bundle): user_data = bundle.obj.get_user_data(bundle.obj.domain).to_dict() + if location_id := bundle.obj.get_location_id(bundle.obj.domain): + # This is all available in the top level, but add in here for backwards-compatibility + user_data['commcare_location_id'] = location_id + user_data['commcare_location_ids'] = user_location_data( + bundle.obj.get_location_ids(bundle.obj.domain)) + user_data['commcare_primary_case_sharing_id'] = location_id + if self.determine_format(bundle.request) == 'application/xml': # attribute names can't start with digits in xml user_data = {k: v for k, v in user_data.items() if not k[0].isdigit()} diff --git a/corehq/apps/callcenter/sync_usercase.py b/corehq/apps/callcenter/sync_usercase.py index 80a6a269f79e..44a6c4af9bb6 100644 --- a/corehq/apps/callcenter/sync_usercase.py +++ b/corehq/apps/callcenter/sync_usercase.py @@ -136,10 +136,10 @@ def valid_element_name(name): fields = {k: v for k, v in user.get_user_data(domain).items() if valid_element_name(k)} - if user.is_web_user(): - fields['commcare_location_id'] = user.get_location_id(domain) + if location_id := user.get_location_id(domain): + fields['commcare_location_id'] = location_id fields['commcare_location_ids'] = user_location_data(user.get_location_ids(domain)) - fields['commcare_primary_case_sharing_id'] = user.get_location_id(domain) + fields['commcare_primary_case_sharing_id'] = location_id # language or phone_number can be null and will break # case submission diff --git a/corehq/apps/user_importer/tests/test_importer.py b/corehq/apps/user_importer/tests/test_importer.py index 4c6c2f5c35df..57d5fc6aefe1 100644 --- a/corehq/apps/user_importer/tests/test_importer.py +++ b/corehq/apps/user_importer/tests/test_importer.py @@ -452,7 +452,6 @@ def test_location_not_list(self): False ) self.assertEqual(self.user.location_id, self.loc1._id) - self.assert_user_data_item('commcare_location_id', self.user.location_id) # multiple locations self.assertListEqual([self.loc1._id], self.user.assigned_location_ids) @@ -495,12 +494,8 @@ def test_location_add(self): ) # first location should be primary location self.assertEqual(self.user.location_id, self.loc1._id) - self.assert_user_data_item('commcare_location_id', self.user.location_id) - self.assert_user_data_item('commcare_primary_case_sharing_id', self.user.location_id) # multiple locations self.assertListEqual([loc._id for loc in [self.loc1, self.loc2]], self.user.assigned_location_ids) - # non-primary location - self.assert_user_data_item('commcare_location_ids', " ".join([self.loc1._id, self.loc2._id])) user_history = UserHistory.objects.get(action=UserModelAction.CREATE.value, user_id=self.user.get_id, @@ -551,7 +546,6 @@ def test_location_remove(self): # user should have no locations self.assertEqual(self.user.location_id, None) - self.assert_user_data_item('commcare_location_id', None) self.assertListEqual(self.user.assigned_location_ids, []) user_history = UserHistory.objects.get(action=UserModelAction.UPDATE.value, @@ -578,8 +572,6 @@ def test_primary_location_replace(self): # user's primary location should be loc1 self.assertEqual(self.user.location_id, self.loc1._id) - self.assert_user_data_item('commcare_location_id', self.loc1._id) - self.assert_user_data_item('commcare_location_ids', " ".join([self.loc1._id, self.loc2._id])) self.assertListEqual(self.user.assigned_location_ids, [self.loc1._id, self.loc2._id]) user_history = UserHistory.objects.get(action=UserModelAction.CREATE.value, @@ -604,8 +596,6 @@ def test_primary_location_replace(self): # user's location should now be loc2 self.assertEqual(self.user.location_id, self.loc2._id) - self.assert_user_data_item('commcare_location_ids', self.loc2._id) - self.assert_user_data_item('commcare_location_id', self.loc2._id) self.assertListEqual(self.user.assigned_location_ids, [self.loc2._id]) user_history = UserHistory.objects.get(action=UserModelAction.UPDATE.value, @@ -653,7 +643,6 @@ def test_location_replace(self): # user's location should now be loc2 self.assertEqual(self.user.location_id, self.loc2._id) - self.assert_user_data_item('commcare_location_id', self.loc2._id) self.assertListEqual(self.user.assigned_location_ids, [self.loc2._id]) user_history = UserHistory.objects.get(action=UserModelAction.UPDATE.value, diff --git a/corehq/apps/users/models.py b/corehq/apps/users/models.py index f7eae87d46b1..5b76cf988255 100644 --- a/corehq/apps/users/models.py +++ b/corehq/apps/users/models.py @@ -1166,6 +1166,12 @@ def get_user_session_data(self, domain): f'{SYSTEM_PREFIX}_phone_number': self.phone_number, f'{SYSTEM_PREFIX}_user_type': self._get_user_type(), }) + + if location_id := self.get_location_id(domain): + session_data['commcare_location_id'] = location_id + session_data['commcare_location_ids'] = user_location_data(self.get_location_ids(domain)) + session_data['commcare_primary_case_sharing_id'] = location_id + return session_data def get_owner_ids(self, domain): @@ -2646,14 +2652,6 @@ def get_location(self, domain): def get_usercase_by_domain(self, domain): return CommCareCase.objects.get_case_by_external_id(domain, self._id, USERCASE_TYPE) - def get_user_session_data(self, domain): - # TODO can we do this for both types of users and remove the fields from user data? - session_data = super(WebUser, self).get_user_session_data(domain) - session_data['commcare_location_id'] = self.get_location_id(domain) - session_data['commcare_location_ids'] = user_location_data(self.get_location_ids(domain)) - session_data['commcare_primary_case_sharing_id'] = self.get_location_id(domain) - return session_data - class FakeUser(WebUser): """ diff --git a/corehq/apps/users/tests/test_user_data.py b/corehq/apps/users/tests/test_user_data.py index 643f79a92bf2..cc7a80f69355 100644 --- a/corehq/apps/users/tests/test_user_data.py +++ b/corehq/apps/users/tests/test_user_data.py @@ -324,5 +324,5 @@ def test_no_location_info_in_user_data_when_no_location_assigned(self): def test_change_data_provided_by_system_will_raise_user_data_error(self): user_data = self.user.get_user_data(self.domain) - with self.assertRaisesMessage(UserDataError, "'commcare_location_id' cannot be set directly"): - user_data['commcare_location_id'] = self.loc1.location_id + with self.assertRaisesMessage(UserDataError, "'commcare_project' cannot be set directly"): + user_data['commcare_project'] = 'blahblah' diff --git a/corehq/apps/users/user_data.py b/corehq/apps/users/user_data.py index 77c4481c7c6c..d9cdfed00d0f 100644 --- a/corehq/apps/users/user_data.py +++ b/corehq/apps/users/user_data.py @@ -1,16 +1,11 @@ -from django.conf import settings from django.contrib.auth.models import User from django.db import models, transaction from django.utils.functional import cached_property from django.utils.translation import gettext as _ -from corehq.apps.users.util import user_location_data from dimagi.utils.chunked import chunked from corehq.apps.custom_data_fields.models import ( - COMMCARE_LOCATION_ID, - COMMCARE_LOCATION_IDS, - COMMCARE_PRIMARY_CASE_SHARING_ID, COMMCARE_PROJECT, PROFILE_SLUG, CustomDataFieldsProfile, @@ -18,8 +13,6 @@ is_system_key, ) -LOCATION_KEYS = {COMMCARE_LOCATION_ID, COMMCARE_LOCATION_IDS, COMMCARE_PRIMARY_CASE_SHARING_ID} - class UserDataError(Exception): ... @@ -67,36 +60,16 @@ def save(self): @property def _provided_by_system(self): - provided_data = { + return { **(self.profile.fields if self.profile else {}), PROFILE_SLUG: self.profile_id or '', COMMCARE_PROJECT: self.domain, } - def _add_location_data(): - if self._couch_user.get_location_id(self.domain): - provided_data[COMMCARE_LOCATION_ID] = self._couch_user.get_location_id(self.domain) - provided_data[COMMCARE_PRIMARY_CASE_SHARING_ID] = self._couch_user.get_location_id(self.domain) - - if self._couch_user.get_location_ids(self.domain): - provided_data[COMMCARE_LOCATION_IDS] = user_location_data( - self._couch_user.get_location_ids(self.domain)) - - # Some test don't have an actual user existed - # Web User don't store location fields in user data - if (self._couch_user or not settings.UNIT_TESTING) and self._couch_user.is_commcare_user(): - _add_location_data() - - return provided_data - - @property - def _system_keys(self): - return set(self._provided_by_system.keys()) | LOCATION_KEYS - def to_dict(self): return { **self._schema_defaults, - **{k: v for k, v in self._local_to_user.items() if k not in self._system_keys}, + **{k: v for k, v in self._local_to_user.items() if k not in self._provided_by_system}, **self._provided_by_system, } @@ -189,7 +162,7 @@ def get(self, key, default=None): return self.to_dict().get(key, default) def __setitem__(self, key, value): - if key in self._system_keys: + if key in self._provided_by_system: if value == self._provided_by_system.get(key, object()): return raise UserDataError(_("'{}' cannot be set directly").format(key)) @@ -209,17 +182,17 @@ def update(self, data, profile_id=...): self.profile_id = profile_id for k, v in data.items(): if k != PROFILE_SLUG: - if v or k not in self._system_keys: + if v or k not in self._provided_by_system: self[k] = v return original != self.to_dict() or original_profile != self.profile_id def __delitem__(self, key): - if key in self._system_keys: + if key in self._provided_by_system: raise UserDataError(_("{} cannot be deleted").format(key)) del self._local_to_user[key] def pop(self, key, default=...): - if key in self._system_keys: + if key in self._provided_by_system: raise UserDataError(_("{} cannot be deleted").format(key)) try: ret = self._local_to_user[key] From 1fa5719ab2ba330c88415e9d50791d62f18f3efe Mon Sep 17 00:00:00 2001 From: Ethan Soergel Date: Tue, 29 Oct 2024 17:00:49 -0400 Subject: [PATCH 2/5] Move commcare_project out too --- corehq/apps/api/resources/v0_1.py | 1 + corehq/apps/callcenter/sync_usercase.py | 1 + corehq/apps/reports/tests/test_esaccessors.py | 1 - corehq/apps/user_importer/tests/test_importer.py | 13 +++---------- corehq/apps/users/models.py | 1 + corehq/apps/users/tests/test_user_data.py | 13 ------------- corehq/apps/users/user_data.py | 2 -- 7 files changed, 6 insertions(+), 26 deletions(-) diff --git a/corehq/apps/api/resources/v0_1.py b/corehq/apps/api/resources/v0_1.py index 1181187c763a..ee1c4c1afd28 100644 --- a/corehq/apps/api/resources/v0_1.py +++ b/corehq/apps/api/resources/v0_1.py @@ -113,6 +113,7 @@ def dehydrate_user_data(self, bundle): bundle.obj.get_location_ids(bundle.obj.domain)) user_data['commcare_primary_case_sharing_id'] = location_id + user_data['commcare_project'] = bundle.obj.domain if self.determine_format(bundle.request) == 'application/xml': # attribute names can't start with digits in xml user_data = {k: v for k, v in user_data.items() if not k[0].isdigit()} diff --git a/corehq/apps/callcenter/sync_usercase.py b/corehq/apps/callcenter/sync_usercase.py index 44a6c4af9bb6..f9c7c46872c6 100644 --- a/corehq/apps/callcenter/sync_usercase.py +++ b/corehq/apps/callcenter/sync_usercase.py @@ -155,6 +155,7 @@ def valid_element_name(name): 'hq_user_id': user.get_id, 'first_name': user.first_name or '', 'last_name': user.last_name or '', + 'commcare_project': domain, }) return fields diff --git a/corehq/apps/reports/tests/test_esaccessors.py b/corehq/apps/reports/tests/test_esaccessors.py index b7356090ae83..7f0a58ccec4d 100644 --- a/corehq/apps/reports/tests/test_esaccessors.py +++ b/corehq/apps/reports/tests/test_esaccessors.py @@ -1025,7 +1025,6 @@ def test_active_user_query(self): self.assertEqual(len(results), 1) metadata = results[0].pop('user_data_es') self.assertEqual({ - 'commcare_project': 'user-esaccessors-test', PROFILE_SLUG: self.profile.id, 'job': 'reporter', 'office': 'phone_booth', diff --git a/corehq/apps/user_importer/tests/test_importer.py b/corehq/apps/user_importer/tests/test_importer.py index 57d5fc6aefe1..95d16d3427a0 100644 --- a/corehq/apps/user_importer/tests/test_importer.py +++ b/corehq/apps/user_importer/tests/test_importer.py @@ -131,8 +131,7 @@ def _test_user_data(self, is_web_upload=False): self.upload_record.pk, is_web_upload ) - self.assert_user_data_equals({ - 'commcare_project': self.domain.name, 'key': 'F#', 'commcare_profile': '', 'mode': ''}) + self.assert_user_data_equals({'key': 'F#', 'commcare_profile': '', 'mode': ''}) # Update user_data import_users_and_groups( @@ -143,8 +142,7 @@ def _test_user_data(self, is_web_upload=False): self.upload_record.pk, is_web_upload ) - self.assert_user_data_equals({ - 'commcare_project': self.domain.name, 'key': 'Bb', 'commcare_profile': '', 'mode': ''}) + self.assert_user_data_equals({'key': 'Bb', 'commcare_profile': '', 'mode': ''}) # set user data to blank import_users_and_groups( @@ -178,12 +176,7 @@ def _test_user_data_profile(self, is_web_upload=False): is_web_upload ) - self.assert_user_data_equals({ - 'commcare_project': self.domain.name, - 'key': 'F#', - 'mode': 'minor', - PROFILE_SLUG: self.profile.id, - }) + self.assert_user_data_equals({'key': 'F#', 'mode': 'minor', PROFILE_SLUG: self.profile.id}) user_history = UserHistory.objects.get( user_id=self.user.get_id, changed_by=self.uploading_user.get_id, diff --git a/corehq/apps/users/models.py b/corehq/apps/users/models.py index 5b76cf988255..8c6b5b81447b 100644 --- a/corehq/apps/users/models.py +++ b/corehq/apps/users/models.py @@ -1161,6 +1161,7 @@ def get_user_session_data(self, domain): session_data[COMMCARE_USER_TYPE_KEY] = COMMCARE_USER_TYPE_DEMO session_data.update({ + f'{SYSTEM_PREFIX}_project': domain, f'{SYSTEM_PREFIX}_first_name': self.first_name, f'{SYSTEM_PREFIX}_last_name': self.last_name, f'{SYSTEM_PREFIX}_phone_number': self.phone_number, diff --git a/corehq/apps/users/tests/test_user_data.py b/corehq/apps/users/tests/test_user_data.py index cc7a80f69355..3a1929972dc8 100644 --- a/corehq/apps/users/tests/test_user_data.py +++ b/corehq/apps/users/tests/test_user_data.py @@ -38,7 +38,6 @@ def make_web_user(self): def test_user_data_accessor(self): user = self.make_commcare_user() user_data = user.get_user_data(self.domain) - self.assertEqual(user_data['commcare_project'], self.domain) user_data.update({ 'cruise': 'control', 'this': 'road', @@ -61,12 +60,10 @@ def test_web_users(self): # Each domain has a separate user_data object self.assertEqual(web_user.get_user_data(self.domain).to_dict(), { - 'commcare_project': self.domain, 'commcare_profile': '', 'what_domain_is_it': 'domain 1', }) self.assertEqual(web_user.get_user_data('another_domain').to_dict(), { - 'commcare_project': 'another_domain', 'commcare_profile': '', 'what_domain_is_it': 'domain 2', }) @@ -198,14 +195,12 @@ def test_add_and_remove_profile(self): # Custom user data profiles get their data added to metadata automatically for mobile users user_data = self.init_user_data({'yearbook_quote': 'Not all who wander are lost.'}) self.assertEqual(user_data.to_dict(), { - 'commcare_project': self.domain, 'commcare_profile': '', 'yearbook_quote': 'Not all who wander are lost.', }) user_data.profile_id = 'blues' self.assertEqual(user_data.to_dict(), { - 'commcare_project': self.domain, 'commcare_profile': 'blues', 'favorite_color': 'blue', # provided by the profile 'yearbook_quote': 'Not all who wander are lost.', @@ -214,7 +209,6 @@ def test_add_and_remove_profile(self): # Remove profile should remove it and related fields user_data.profile_id = None self.assertEqual(user_data.to_dict(), { - 'commcare_project': self.domain, 'commcare_profile': '', 'yearbook_quote': 'Not all who wander are lost.', }) @@ -317,12 +311,5 @@ def test_no_location_info_in_user_data_when_no_location_assigned(self): user_data = self.user.get_user_data(self.domain) self.assertEqual(user_data.to_dict(), { - 'commcare_project': self.domain, 'commcare_profile': '', }) - - def test_change_data_provided_by_system_will_raise_user_data_error(self): - user_data = self.user.get_user_data(self.domain) - - with self.assertRaisesMessage(UserDataError, "'commcare_project' cannot be set directly"): - user_data['commcare_project'] = 'blahblah' diff --git a/corehq/apps/users/user_data.py b/corehq/apps/users/user_data.py index d9cdfed00d0f..03e54a0467a5 100644 --- a/corehq/apps/users/user_data.py +++ b/corehq/apps/users/user_data.py @@ -6,7 +6,6 @@ from dimagi.utils.chunked import chunked from corehq.apps.custom_data_fields.models import ( - COMMCARE_PROJECT, PROFILE_SLUG, CustomDataFieldsProfile, CustomDataFieldsDefinition, @@ -63,7 +62,6 @@ def _provided_by_system(self): return { **(self.profile.fields if self.profile else {}), PROFILE_SLUG: self.profile_id or '', - COMMCARE_PROJECT: self.domain, } def to_dict(self): From a115a9aab5847b968f451feeafff0cc48ce1d264 Mon Sep 17 00:00:00 2001 From: Ethan Soergel Date: Thu, 31 Oct 2024 11:17:02 -0400 Subject: [PATCH 3/5] Add location ID back to UCR data sources A few projects do use this field, see for details: https://docs.google.com/document/d/1MM1lOnFArfr8F6CbgQ9OvvzpWBlwZJEB780YFA7NGls/edit?tab=t.0 --- corehq/apps/userreports/expressions/specs.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/corehq/apps/userreports/expressions/specs.py b/corehq/apps/userreports/expressions/specs.py index ac424af2d0e9..994e86aaeb75 100644 --- a/corehq/apps/userreports/expressions/specs.py +++ b/corehq/apps/userreports/expressions/specs.py @@ -708,6 +708,8 @@ def _get_user(domain, doc_id): else: doc[key] = getattr(user, key) doc['user_data'] = user.get_user_data(domain).to_dict() + # add location ID in here to not break existing reports - they _should_ just use user.location_id + doc['user_data']['commcare_location_id'] = user.get_location_id(domain) return doc From b4434e4cf0a0b9829aa79a162c036672cf6d0d5e Mon Sep 17 00:00:00 2001 From: Ethan Soergel Date: Tue, 26 Nov 2024 10:38:51 -0500 Subject: [PATCH 4/5] update tests --- corehq/apps/users/tests/test_location_assignment.py | 5 ----- .../ex-submodules/casexml/apps/phone/tests/dummy.py | 12 ++---------- .../casexml/apps/phone/tests/test_restore_user.py | 2 ++ 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/corehq/apps/users/tests/test_location_assignment.py b/corehq/apps/users/tests/test_location_assignment.py index 3d7301d5d1ef..461b0151de17 100644 --- a/corehq/apps/users/tests/test_location_assignment.py +++ b/corehq/apps/users/tests/test_location_assignment.py @@ -129,20 +129,15 @@ def test_create_with_location(self): def assertPrimaryLocation(self, expected): self.assertEqual(self.user.location_id, expected) - self.assertEqual(self.user.get_user_data(self.domain).get('commcare_location_id'), expected) self.assertTrue(expected in self.user.assigned_location_ids) def assertAssignedLocations(self, expected_location_ids): user = CommCareUser.get(self.user._id) self.assertListEqual(user.assigned_location_ids, expected_location_ids) - actual_ids = user.get_user_data(self.domain).get('commcare_location_ids', '') - actual_ids = actual_ids.split(' ') if actual_ids else [] - self.assertListEqual(actual_ids, expected_location_ids) def assertNonPrimaryLocation(self, expected): self.assertNotEqual(self.user.location_id, expected) self.assertTrue(expected in self.user.assigned_location_ids) - self.assertTrue(expected in self.user.get_user_data(self.domain).get('commcare_location_ids')) class WebUserLocationAssignmentTest(TestCase): diff --git a/corehq/ex-submodules/casexml/apps/phone/tests/dummy.py b/corehq/ex-submodules/casexml/apps/phone/tests/dummy.py index 3387fac1edb1..3a6143d4cbf7 100644 --- a/corehq/ex-submodules/casexml/apps/phone/tests/dummy.py +++ b/corehq/ex-submodules/casexml/apps/phone/tests/dummy.py @@ -7,12 +7,6 @@ DUMMY_PASSWORD = "changeme" DUMMY_PROJECT = "domain" -LOCATION_IDS_DATA_FIELDS = """ - - """ -COMMCARE_PRIMARY_CASE_SHARING_ID = """ - """ - def dummy_user_xml(user=None): username = user.username if user else DUMMY_USERNAME @@ -30,8 +24,8 @@ def dummy_user_xml(user=None): {} - {} - {} + + {} {} @@ -42,8 +36,6 @@ def dummy_user_xml(user=None): password, user_id, date_to_xml_string(date_joined), - LOCATION_IDS_DATA_FIELDS if user_type == 'web' else '', - COMMCARE_PRIMARY_CASE_SHARING_ID if user_type == 'web' else '', project, user_type, ) diff --git a/corehq/ex-submodules/casexml/apps/phone/tests/test_restore_user.py b/corehq/ex-submodules/casexml/apps/phone/tests/test_restore_user.py index 26ec2bd0f586..1ea66fd1b059 100644 --- a/corehq/ex-submodules/casexml/apps/phone/tests/test_restore_user.py +++ b/corehq/ex-submodules/casexml/apps/phone/tests/test_restore_user.py @@ -30,6 +30,8 @@ def test_get_commtrack_location_id(): (WebUser(), 'web'), (CommCareUser(domain=DOMAIN), 'commcare'), ]) +@patch('corehq.apps.users.models._AuthorizableMixin.get_domain_membership', + Mock(return_value=DomainMembership(domain=DOMAIN))) @use("db") def test_user_types(user, expected_type): with patch_user_data_db_layer(): From 1f70d86464abb79c4d29a268fa54583328687d80 Mon Sep 17 00:00:00 2001 From: Ethan Soergel Date: Tue, 26 Nov 2024 10:40:20 -0500 Subject: [PATCH 5/5] Use modern string formatting The positional arguments of the first fn in particular are brittle --- .../casexml/apps/phone/tests/dummy.py | 53 +++++++------------ 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/corehq/ex-submodules/casexml/apps/phone/tests/dummy.py b/corehq/ex-submodules/casexml/apps/phone/tests/dummy.py index 3a6143d4cbf7..ee54bce43e18 100644 --- a/corehq/ex-submodules/casexml/apps/phone/tests/dummy.py +++ b/corehq/ex-submodules/casexml/apps/phone/tests/dummy.py @@ -16,48 +16,33 @@ def dummy_user_xml(user=None): project = user.domain if user else DUMMY_PROJECT user_type = 'web' if isinstance(user, OTARestoreWebUser) else 'commcare' - return """ + return f""" - {} - {} - {} - {} + {username} + {password} + {user_id} + {date_to_xml_string(date_joined)} - {} - {} + {project} + {user_type} arbitrary - """.format( - username, - password, - user_id, - date_to_xml_string(date_joined), - project, - user_type, - ) - - -DUMMY_RESTORE_XML_TEMPLATE = (""" - - %(message)s - - %(restore_id)s - - %(user_xml)s - %(case_xml)s - -""") + """ def dummy_restore_xml(restore_id, case_xml="", items=None, user=None): - return DUMMY_RESTORE_XML_TEMPLATE % { - "restore_id": restore_id, - "items_xml": '' if items is None else (' items="%s"' % items), - "user_xml": dummy_user_xml(user), - "case_xml": case_xml, - "message": "Successfully restored account mclovin!" - } + items_xml = '' if items is None else f' items="{items}"' + return f""" + + Successfully restored account mclovin! + + {restore_id} + + {dummy_user_xml(user)} + {case_xml} + + """