diff --git a/studies/tasks.py b/studies/tasks.py index bbf796283..1d63df071 100644 --- a/studies/tasks.py +++ b/studies/tasks.py @@ -78,26 +78,27 @@ OR (sst.id = 2) ) ), - latest_study_notifications_for_children AS ( - SELECT amr.user_id, - amcoi.child_id, - am.related_study_id, - MAX(am.email_sent_timestamp) as latest_sent_time + prior_study_notifications_for_children AS ( + -- Any announcement message row for this triplet counts as "already targeted", + -- even if email_sent_timestamp is NULL (e.g., a crash between sending the + -- email and saving the timestamp), so we never re-send for the same triplet. + SELECT DISTINCT amr.user_id, + amcoi.child_id, + am.related_study_id FROM accounts_message am INNER JOIN accounts_message_children_of_interest amcoi on am.id = amcoi.message_id INNER JOIN accounts_message_recipients amr on am.id = amr.message_id WHERE (amr.user_id, amcoi.child_id, am.related_study_id) IN (SELECT * FROM message_targets) - GROUP BY amr.user_id, amcoi.child_id, am.related_study_id ) SELECT mt.user_id, mt.child_id, mt.study_id FROM message_targets mt - LEFT OUTER JOIN latest_study_notifications_for_children lsnfc - ON lsnfc.user_id = mt.user_id - AND lsnfc.child_id = mt.child_id - AND lsnfc.related_study_id = mt.study_id -WHERE lsnfc.latest_sent_time IS NULL + LEFT OUTER JOIN prior_study_notifications_for_children psnfc + ON psnfc.user_id = mt.user_id + AND psnfc.child_id = mt.child_id + AND psnfc.related_study_id = mt.study_id +WHERE psnfc.user_id IS NULL ORDER BY mt.user_id, mt.child_id, mt.study_id; """ MAX_EMAILS_PER_STUDY = 50 diff --git a/studies/tests.py b/studies/tests.py index 387c6f51c..a544bd875 100644 --- a/studies/tests.py +++ b/studies/tests.py @@ -562,6 +562,102 @@ def test_study_excluded_from_targets_after_message(self): study_child_mapping, {self.study_two: [self.child_two, self.child_three]} ) + def test_study_excluded_from_targets_when_message_has_no_sent_timestamp(self): + # The triplet is a valid message target before any message exists for it. + self.assertIn( + (self.participant_two.id, self.child_three.id, self.study_one.id), + [ + (mt.user_id, mt.child_id, mt.study_id) + for mt in potential_message_targets() + ], + ) + + # Simulate a crash between sending the announcement email and saving the + # sent timestamp: the message row exists but email_sent_timestamp is NULL. + # The triplet must still be excluded so that a family/child is never emailed + # twice about the same study. + message = Message.objects.create( + related_study=self.study_one, email_sent_timestamp=None + ) + message.recipients.add(self.participant_two) + message.children_of_interest.add(self.child_three) + + self.assertNotIn( + (self.participant_two.id, self.child_three.id, self.study_one.id), + [ + (mt.user_id, mt.child_id, mt.study_id) + for mt in potential_message_targets() + ], + ) + + def test_message_excludes_only_its_own_triplet(self): + # A sent announcement must suppress ONLY its exact user/child/study + # triplet. If it suppressed a different child, study, or user, a family + # would silently never be told about a study they're eligible for. + def current_triplets(): + return [ + (mt.user_id, mt.child_id, mt.study_id) + for mt in potential_message_targets() + ] + + target_same = ( + self.participant_two.id, + self.child_three.id, + self.study_one.id, + ) + target_other_study = ( + self.participant_two.id, + self.child_three.id, + self.study_two.id, + ) + target_other_child = ( + self.participant_two.id, + self.child_two.id, + self.study_two.id, + ) + + # All three are valid targets before any message is sent... + before = current_triplets() + self.assertIn(target_same, before) + self.assertIn(target_other_study, before) + self.assertIn(target_other_child, before) + # ...as are participant_one's targets, used to check cross-user isolation. + participant_one_before = [t for t in before if t[0] == self.participant_one.id] + self.assertTrue(participant_one_before) + + # Send an announcement for exactly one triplet. + message = Message.objects.create( + related_study=self.study_one, + email_sent_timestamp=datetime.now(timezone.utc), + ) + message.recipients.add(self.participant_two) + message.children_of_interest.add(self.child_three) + + after = current_triplets() + # The exact triplet is now excluded... + self.assertNotIn(target_same, after) + # ...but a different study for the same child is untouched, + self.assertIn(target_other_study, after) + # a different child of the same user/study is untouched, + self.assertIn(target_other_child, after) + # and no other user's targets changed. + self.assertEqual( + [t for t in after if t[0] == self.participant_one.id], + participant_one_before, + ) + + def test_potential_message_targets_inactive_user(self): + # An inactive user (e.g. one marked as spam) must never be a target. + user = G(User, is_active=True) + G(Child, user=user, birthday=date.today() - timedelta(days=365)) + + self.assertTrue(any(m.user_id == user.id for m in potential_message_targets())) + + user.is_active = False + user.save() + + self.assertFalse(any(m.user_id == user.id for m in potential_message_targets())) + def test_announcement_email_to_child_with_long_name(self): # Family with a child with a long name long_name_family = G(User, nickname="Mama", is_active=True)