From a93266e8088e909a211760ae720ab8b687e6ffe0 Mon Sep 17 00:00:00 2001 From: Martin Riese Date: Wed, 4 Dec 2024 10:42:47 -0600 Subject: [PATCH 1/2] Empty strings in recipient filter match unset prop If a user field/property is created after the user and the user has not been updated yet, it is not in the user data or user case properties. In that case the conditional alert recipient filter will fail if it uses said field. With this change a filter value "" will match if a field does not exist yet in the user data or user case properties. --- .../scheduling_partitioned/models.py | 5 ++-- .../scheduling/tests/test_recipients.py | 23 +++++++++++++++---- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/corehq/messaging/scheduling/scheduling_partitioned/models.py b/corehq/messaging/scheduling/scheduling_partitioned/models.py index 2f1375aba5ae..f311a2ce6647 100644 --- a/corehq/messaging/scheduling/scheduling_partitioned/models.py +++ b/corehq/messaging/scheduling/scheduling_partitioned/models.py @@ -262,10 +262,11 @@ def _passes_user_data_filter(self, contact): user_data = contact.get_user_data(self.domain) for key, value_or_property_name in self.memoized_schedule.user_data_filter.items(): if key not in user_data: - return False + actual_values_set = {""} + else: + actual_values_set = self.convert_to_set(user_data[key]) allowed_values_set = {self._get_filter_value(v) for v in self.convert_to_set(value_or_property_name)} - actual_values_set = self.convert_to_set(user_data[key]) if actual_values_set.isdisjoint(allowed_values_set): return False diff --git a/corehq/messaging/scheduling/tests/test_recipients.py b/corehq/messaging/scheduling/tests/test_recipients.py index c20297d4fe8a..e4976875780d 100644 --- a/corehq/messaging/scheduling/tests/test_recipients.py +++ b/corehq/messaging/scheduling/tests/test_recipients.py @@ -80,14 +80,17 @@ def testIgnoreSpacesBracesReturnProperty(self): class PassesUserDataFilterTest(TestCase): domain = 'passes-user-data-filter-test' + mobile_user = None @classmethod def setUpClass(cls): super(PassesUserDataFilterTest, cls).setUpClass() cls.domain_obj = create_domain(cls.domain) - user_data = {"wants_email": "yes", "color": "green"} + user_data = {"wants_email": "yes", "color": "green", "empty": ""} cls.mobile_user = CommCareUser.create(cls.domain, 'mobile', 'abc', None, None, user_data=user_data) + create_case_2(cls.domain, case_type=USERCASE_TYPE, external_id=cls.mobile_user.user_id, + case_json=user_data, save=True) @classmethod def tearDownClass(cls): @@ -122,9 +125,7 @@ def test_fails_with_user_data_filter_because_one_value_does_not_match(self): ._passes_user_data_filter(self.mobile_user)) def test_passes_with_user_case_filter(self): - case = create_case_2(self.domain, case_type="thing", case_json={"case_color": "red"}) - create_case_2(self.domain, case_type=USERCASE_TYPE, external_id=self.mobile_user.user_id, - case_json={"wants_email": "yes", "color": "red"}, save=True) + case = create_case_2(self.domain, case_type="thing", case_json={"case_color": "green"}) schedule = AlertSchedule() schedule.use_user_case_for_filter = True @@ -132,6 +133,20 @@ def test_passes_with_user_case_filter(self): self.assertTrue(ScheduleInstance(case=case, domain=self.domain, schedule=schedule) ._passes_user_data_filter(self.mobile_user)) + def test_empty_string_matches_unset_property(self): + schedule = AlertSchedule() + schedule.use_user_case_for_filter = False + schedule.user_data_filter = {"empty": [""], "unset": ["yes", ""]} + self.assertTrue(ScheduleInstance(schedule=schedule) + ._passes_user_data_filter(self.mobile_user)) + + def test_empty_string_matches_unset_property_user_case(self): + schedule = AlertSchedule() + schedule.use_user_case_for_filter = True + schedule.user_data_filter = {"empty": [""], "unset": ["yes", ""]} + self.assertTrue(ScheduleInstance(domain=self.domain, schedule=schedule) + ._passes_user_data_filter(self.mobile_user)) + class SchedulingRecipientTest(TestCase): domain = 'scheduling-recipient-test' From fb1f4ba24ad51a65ce71a90ec16a7a9d61ec3589 Mon Sep 17 00:00:00 2001 From: Martin Riese Date: Wed, 4 Dec 2024 11:08:07 -0600 Subject: [PATCH 2/2] Use dict.get to specify fallback value --- .../messaging/scheduling/scheduling_partitioned/models.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/corehq/messaging/scheduling/scheduling_partitioned/models.py b/corehq/messaging/scheduling/scheduling_partitioned/models.py index f311a2ce6647..c89dca31414a 100644 --- a/corehq/messaging/scheduling/scheduling_partitioned/models.py +++ b/corehq/messaging/scheduling/scheduling_partitioned/models.py @@ -261,12 +261,8 @@ def _passes_user_data_filter(self, contact): else: user_data = contact.get_user_data(self.domain) for key, value_or_property_name in self.memoized_schedule.user_data_filter.items(): - if key not in user_data: - actual_values_set = {""} - else: - actual_values_set = self.convert_to_set(user_data[key]) - allowed_values_set = {self._get_filter_value(v) for v in self.convert_to_set(value_or_property_name)} + actual_values_set = self.convert_to_set(user_data.get(key, "")) if actual_values_set.isdisjoint(allowed_values_set): return False