From f51943cce106d952f377d84cf7c94cb63b2ea40e Mon Sep 17 00:00:00 2001 From: Becky Gilbert Date: Wed, 11 Feb 2026 14:46:53 -0800 Subject: [PATCH 1/6] add check_and_pause_if_at_max_responses logic: pause if study has a max responses value, is active, and has reached limit; update pause trigger to make caller/user arg optional --- studies/models.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/studies/models.py b/studies/models.py index 26e0bd3c0..cc43e555f 100644 --- a/studies/models.py +++ b/studies/models.py @@ -593,10 +593,22 @@ def has_reached_max_responses(self) -> bool: def check_and_pause_if_at_max_responses(self): """Check if max responses reached and pause the study if so. - Only pauses if the study is currently active. + Only pauses if the study is currently active. Uses the state machine's + pause trigger to properly transition and run callbacks. """ - # TODO: Implement logic to pause study when max responses reached - pass + if self.max_responses is None: + return + + if self.state != "active": + return + + if not self.has_reached_max_responses: + return + + # Use the state machine's pause trigger to properly transition + # and run callbacks (like notify_administrators_of_pause) + self.pause() # No user since this is system-triggered + self.save() @property def consent_videos(self): @@ -825,12 +837,16 @@ def notify_administrators_of_activation(self, ev): ) def notify_administrators_of_pause(self, ev): + user = ev.kwargs.get("user") + caller_name = ( + user.get_short_name() if user else "System (max responses reached)" + ) context = { "lab_name": self.lab.name, "study_name": self.name, "study_id": self.pk, "study_uuid": str(self.uuid), - "researcher_name": ev.kwargs.get("user").get_short_name(), + "researcher_name": caller_name, "action": ev.transition.dest, } send_mail.delay( From fc078640b9d98259cba6ab827cbc90e8420ffbee Mon Sep 17 00:00:00 2001 From: Becky Gilbert Date: Wed, 11 Feb 2026 14:49:17 -0800 Subject: [PATCH 2/6] add tests for Study model check_and_pause_if_at_max_responses method --- studies/tests.py | 148 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/studies/tests.py b/studies/tests.py index ab15a1385..2f62cb7f7 100644 --- a/studies/tests.py +++ b/studies/tests.py @@ -1089,6 +1089,154 @@ def test_has_reached_max_responses_exceeded(self): self.assertTrue(study.has_reached_max_responses) + def test_check_and_pause_if_at_max_responses_no_limit_set(self): + """Study without max_responses set should not pause.""" + study = Study.objects.create( + name="No Limit Study", + lab=Lab.objects.create( + name="Test Lab No Limit", + institution="Test", + contact_email="test@test.com", + ), + study_type=StudyType.get_ember_frame_player(), + max_responses=None, + ) + study.state = "active" + study.save() + + study.check_and_pause_if_at_max_responses() + study.refresh_from_db() + + self.assertEqual(study.state, "active") + + def test_check_and_pause_if_at_max_responses_not_active(self): + """Study not in active state should not pause.""" + study = Study.objects.create( + name="Not Active Study", + lab=Lab.objects.create( + name="Test Lab Not Active", + institution="Test", + contact_email="test@test.com", + ), + study_type=StudyType.get_ember_frame_player(), + max_responses=1, + ) + # Study is in "created" state by default + self.assertEqual(study.state, "created") + + study.check_and_pause_if_at_max_responses() + study.refresh_from_db() + + self.assertEqual(study.state, "created") + + def test_check_and_pause_if_at_max_responses_not_reached(self): + """Active study that hasn't reached max_responses should not pause.""" + study = Study.objects.create( + name="Under Limit Study", + lab=Lab.objects.create( + name="Test Lab Under Limit", + institution="Test", + contact_email="test@test.com", + ), + study_type=StudyType.get_ember_frame_player(), + max_responses=5, + ) + study.state = "active" + study.save() + user = User.objects.create(is_active=True) + child = Child.objects.create(user=user, birthday=date.today()) + + # Add only 2 responses (under limit of 5) + for _ in range(2): + r = Response.objects.create( + study=study, + child=child, + study_type=study.study_type, + demographic_snapshot=user.latest_demographics, + completed=True, + is_preview=False, + ) + Response.objects.filter(pk=r.pk).update( + eligibility=[ResponseEligibility.ELIGIBLE] + ) + + study.check_and_pause_if_at_max_responses() + study.refresh_from_db() + + self.assertEqual(study.state, "active") + + def test_check_and_pause_if_at_max_responses_limit_reached(self): + """Active study that has reached max_responses should pause.""" + study = Study.objects.create( + name="At Limit Study", + lab=Lab.objects.create( + name="Test Lab At Limit", + institution="Test", + contact_email="test@test.com", + ), + study_type=StudyType.get_ember_frame_player(), + max_responses=2, + ) + study.state = "active" + study.save() + user = User.objects.create(is_active=True) + child = Child.objects.create(user=user, birthday=date.today()) + + # Add exactly 2 responses (at limit of 2) + for _ in range(2): + r = Response.objects.create( + study=study, + child=child, + study_type=study.study_type, + demographic_snapshot=user.latest_demographics, + completed=True, + is_preview=False, + ) + Response.objects.filter(pk=r.pk).update( + eligibility=[ResponseEligibility.ELIGIBLE] + ) + + study.check_and_pause_if_at_max_responses() + study.refresh_from_db() + + self.assertEqual(study.state, "paused") + + def test_check_and_pause_if_at_max_responses_limit_exceeded(self): + """Active study that has exceeded max_responses should pause.""" + study = Study.objects.create( + name="Over Limit Study", + lab=Lab.objects.create( + name="Test Lab Over Limit", + institution="Test", + contact_email="test@test.com", + ), + study_type=StudyType.get_ember_frame_player(), + max_responses=2, + ) + study.state = "active" + study.save() + user = User.objects.create(is_active=True) + child = Child.objects.create(user=user, birthday=date.today()) + + # Add 4 responses (exceeds limit of 2) + for _ in range(4): + r = Response.objects.create( + study=study, + child=child, + study_type=study.study_type, + demographic_snapshot=user.latest_demographics, + completed=True, + is_preview=False, + ) + Response.objects.filter(pk=r.pk).update( + eligibility=[ResponseEligibility.ELIGIBLE] + ) + + study.check_and_pause_if_at_max_responses() + study.refresh_from_db() + + self.assertEqual(study.state, "paused") + class VideoModelTestCase(TestCase): def setUp(self): From d82a4a6e9e14dafd108c10f6b215939fba099b3a Mon Sep 17 00:00:00 2001 From: Becky Gilbert Date: Wed, 11 Feb 2026 16:45:36 -0800 Subject: [PATCH 3/6] refactor tests to pass sonar code duplication checks --- studies/tests.py | 351 +++++++++++++---------------------------------- 1 file changed, 93 insertions(+), 258 deletions(-) diff --git a/studies/tests.py b/studies/tests.py index 2f62cb7f7..0d8410065 100644 --- a/studies/tests.py +++ b/studies/tests.py @@ -842,6 +842,53 @@ def test_get_jspsych(self): class StudyModelTestCase(TestCase): + def _create_response( + self, study, child, user, completed=True, is_preview=False, eligibility=None + ): + """Create a single response, optionally overriding eligibility after creation. + + Note: Response.save() auto-sets eligibility, so we use .update() to override it. + Pass eligibility=None to skip the override (keep auto-set value). + """ + r = Response.objects.create( + study=study, + child=child, + study_type=study.study_type, + demographic_snapshot=user.latest_demographics, + completed=completed, + is_preview=is_preview, + ) + if eligibility is not None: + Response.objects.filter(pk=r.pk).update(eligibility=eligibility) + return r + + def _create_eligible_responses(self, study, count): + """Create a user, child, and the given number of eligible responses for a study.""" + user = User.objects.create(is_active=True) + child = Child.objects.create(user=user, birthday=date.today()) + for _ in range(count): + self._create_response( + study, child, user, eligibility=[ResponseEligibility.ELIGIBLE] + ) + return user, child + + def _create_study_with_lab(self, name, max_responses, state=None): + """Create a study with a lab, optionally setting its state.""" + study = Study.objects.create( + name=name, + lab=Lab.objects.create( + name=f"Test Lab {name}", + institution="Test", + contact_email="test@test.com", + ), + study_type=StudyType.get_ember_frame_player(), + max_responses=max_responses, + ) + if state: + study.state = state + study.save() + return study + def test_responses_for_researcher_external_studies(self): study = Study.objects.create( study_type=StudyType.get_external(), @@ -867,69 +914,31 @@ def test_valid_response_count_internal_study(self): user = User.objects.create(is_active=True) child = Child.objects.create(user=user, birthday=date.today()) - # Note: Response.save() auto-sets eligibility, so we update this value after creation - - # Valid response: completed, not preview, eligible - valid1 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=valid1.pk).update(eligibility=[]) - # Valid response: completed, not preview, empty eligibility - valid2 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=valid2.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] + self._create_response(study, child, user, eligibility=[]) + # Valid response: completed, not preview, eligible + self._create_response( + study, child, user, eligibility=[ResponseEligibility.ELIGIBLE] ) - # Invalid: preview response - invalid3 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, + self._create_response( + study, + child, + user, is_preview=True, + eligibility=[ResponseEligibility.ELIGIBLE], ) - Response.objects.filter(pk=invalid3.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] - ) - # Invalid: not completed - invalid4 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, + self._create_response( + study, + child, + user, completed=False, - is_preview=False, + eligibility=[ResponseEligibility.ELIGIBLE], ) - Response.objects.filter(pk=invalid4.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] - ) - # Invalid: ineligible - invalid5 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=invalid5.pk).update( - eligibility=[ResponseEligibility.INELIGIBLE_OLD] + self._create_response( + study, child, user, eligibility=[ResponseEligibility.INELIGIBLE_OLD] ) self.assertEqual(study.valid_response_count, 2) @@ -941,66 +950,30 @@ def test_valid_response_count_external_study(self): child = Child.objects.create(user=user, birthday=date.today()) # Valid: not preview, eligible, completed - r1 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=r1.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] + self._create_response( + study, child, user, eligibility=[ResponseEligibility.ELIGIBLE] ) - # Valid: not preview, eligible, NOT completed (should still count for external) - r2 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, + self._create_response( + study, + child, + user, completed=False, - is_preview=False, - ) - Response.objects.filter(pk=r2.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] + eligibility=[ResponseEligibility.ELIGIBLE], ) - # Valid: not preview, empty eligibility, NOT completed - r3 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=False, - is_preview=False, - ) - Response.objects.filter(pk=r3.pk).update(eligibility=[]) - + self._create_response(study, child, user, completed=False, eligibility=[]) # Invalid: preview response (should not count) - r4 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, + self._create_response( + study, + child, + user, is_preview=True, + eligibility=[ResponseEligibility.ELIGIBLE], ) - Response.objects.filter(pk=r4.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] - ) - # Invalid: ineligible (should not count) - r5 = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=r5.pk).update( - eligibility=[ResponseEligibility.INELIGIBLE_CRITERIA] + self._create_response( + study, child, user, eligibility=[ResponseEligibility.INELIGIBLE_CRITERIA] ) # 3 valid responses: r1, r2, r3 (completed field ignored for external) @@ -1020,23 +993,7 @@ def test_has_reached_max_responses_not_reached(self): study_type=StudyType.get_ember_frame_player(), max_responses=5, ) - user = User.objects.create(is_active=True) - child = Child.objects.create(user=user, birthday=date.today()) - - # Add 2 valid responses - for _ in range(2): - r = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=r.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] - ) - + self._create_eligible_responses(study, count=2) self.assertFalse(study.has_reached_max_responses) def test_has_reached_max_responses_reached(self): @@ -1045,23 +1002,7 @@ def test_has_reached_max_responses_reached(self): study_type=StudyType.get_ember_frame_player(), max_responses=3, ) - user = User.objects.create(is_active=True) - child = Child.objects.create(user=user, birthday=date.today()) - - # Add 3 valid responses - for _ in range(3): - r = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=r.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] - ) - + self._create_eligible_responses(study, count=3) self.assertTrue(study.has_reached_max_responses) def test_has_reached_max_responses_exceeded(self): @@ -1070,39 +1011,14 @@ def test_has_reached_max_responses_exceeded(self): study_type=StudyType.get_ember_frame_player(), max_responses=2, ) - user = User.objects.create(is_active=True) - child = Child.objects.create(user=user, birthday=date.today()) - - # Add 4 valid responses (exceeds limit of 2) - for _ in range(4): - r = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=r.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] - ) - + self._create_eligible_responses(study, count=4) self.assertTrue(study.has_reached_max_responses) def test_check_and_pause_if_at_max_responses_no_limit_set(self): """Study without max_responses set should not pause.""" - study = Study.objects.create( - name="No Limit Study", - lab=Lab.objects.create( - name="Test Lab No Limit", - institution="Test", - contact_email="test@test.com", - ), - study_type=StudyType.get_ember_frame_player(), - max_responses=None, + study = self._create_study_with_lab( + "No Limit Study", max_responses=None, state="active" ) - study.state = "active" - study.save() study.check_and_pause_if_at_max_responses() study.refresh_from_db() @@ -1111,16 +1027,7 @@ def test_check_and_pause_if_at_max_responses_no_limit_set(self): def test_check_and_pause_if_at_max_responses_not_active(self): """Study not in active state should not pause.""" - study = Study.objects.create( - name="Not Active Study", - lab=Lab.objects.create( - name="Test Lab Not Active", - institution="Test", - contact_email="test@test.com", - ), - study_type=StudyType.get_ember_frame_player(), - max_responses=1, - ) + study = self._create_study_with_lab("Not Active Study", max_responses=1) # Study is in "created" state by default self.assertEqual(study.state, "created") @@ -1131,34 +1038,10 @@ def test_check_and_pause_if_at_max_responses_not_active(self): def test_check_and_pause_if_at_max_responses_not_reached(self): """Active study that hasn't reached max_responses should not pause.""" - study = Study.objects.create( - name="Under Limit Study", - lab=Lab.objects.create( - name="Test Lab Under Limit", - institution="Test", - contact_email="test@test.com", - ), - study_type=StudyType.get_ember_frame_player(), - max_responses=5, + study = self._create_study_with_lab( + "Under Limit Study", max_responses=5, state="active" ) - study.state = "active" - study.save() - user = User.objects.create(is_active=True) - child = Child.objects.create(user=user, birthday=date.today()) - - # Add only 2 responses (under limit of 5) - for _ in range(2): - r = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=r.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] - ) + self._create_eligible_responses(study, count=2) study.check_and_pause_if_at_max_responses() study.refresh_from_db() @@ -1167,34 +1050,10 @@ def test_check_and_pause_if_at_max_responses_not_reached(self): def test_check_and_pause_if_at_max_responses_limit_reached(self): """Active study that has reached max_responses should pause.""" - study = Study.objects.create( - name="At Limit Study", - lab=Lab.objects.create( - name="Test Lab At Limit", - institution="Test", - contact_email="test@test.com", - ), - study_type=StudyType.get_ember_frame_player(), - max_responses=2, + study = self._create_study_with_lab( + "At Limit Study", max_responses=2, state="active" ) - study.state = "active" - study.save() - user = User.objects.create(is_active=True) - child = Child.objects.create(user=user, birthday=date.today()) - - # Add exactly 2 responses (at limit of 2) - for _ in range(2): - r = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=r.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] - ) + self._create_eligible_responses(study, count=2) study.check_and_pause_if_at_max_responses() study.refresh_from_db() @@ -1203,34 +1062,10 @@ def test_check_and_pause_if_at_max_responses_limit_reached(self): def test_check_and_pause_if_at_max_responses_limit_exceeded(self): """Active study that has exceeded max_responses should pause.""" - study = Study.objects.create( - name="Over Limit Study", - lab=Lab.objects.create( - name="Test Lab Over Limit", - institution="Test", - contact_email="test@test.com", - ), - study_type=StudyType.get_ember_frame_player(), - max_responses=2, + study = self._create_study_with_lab( + "Over Limit Study", max_responses=2, state="active" ) - study.state = "active" - study.save() - user = User.objects.create(is_active=True) - child = Child.objects.create(user=user, birthday=date.today()) - - # Add 4 responses (exceeds limit of 2) - for _ in range(4): - r = Response.objects.create( - study=study, - child=child, - study_type=study.study_type, - demographic_snapshot=user.latest_demographics, - completed=True, - is_preview=False, - ) - Response.objects.filter(pk=r.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] - ) + self._create_eligible_responses(study, count=4) study.check_and_pause_if_at_max_responses() study.refresh_from_db() From 47d8df9c3dc0644df16fed2ce2485e54af291730 Mon Sep 17 00:00:00 2001 From: Becky Gilbert Date: Wed, 11 Feb 2026 16:46:41 -0800 Subject: [PATCH 4/6] override celery since these tests involve state changes that send emails --- studies/tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/studies/tests.py b/studies/tests.py index 0d8410065..5d3d2d6df 100644 --- a/studies/tests.py +++ b/studies/tests.py @@ -841,6 +841,7 @@ def test_get_jspsych(self): self.assertFalse(StudyType.get_jspsych().is_external) +@override_settings(CELERY_TASK_ALWAYS_EAGER=True) class StudyModelTestCase(TestCase): def _create_response( self, study, child, user, completed=True, is_preview=False, eligibility=None From 8c5ab618b2964662bfcfccf7cda23389df65f21a Mon Sep 17 00:00:00 2001 From: Becky Gilbert Date: Wed, 11 Feb 2026 16:48:58 -0800 Subject: [PATCH 5/6] fix issue with tests failing because study image field was not up-to-date or treated as changed because of None vs empty string comparison --- studies/models.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/studies/models.py b/studies/models.py index cc43e555f..d0663015d 100644 --- a/studies/models.py +++ b/studies/models.py @@ -605,6 +605,10 @@ def check_and_pause_if_at_max_responses(self): if not self.has_reached_max_responses: return + # Refresh from DB to ensure the in-memory study is current before + # the pause transition triggers a save (via _finalize_state_change). + self.refresh_from_db() + # Use the state machine's pause trigger to properly transition # and run callbacks (like notify_administrators_of_pause) self.pause() # No user since this is system-triggered @@ -1014,6 +1018,12 @@ def check_modification_of_approved_study( ): continue # Skip, since the actual JSON content is the same - only exact_text changing if new != current: + # For file fields (e.g. image), None and "" are equivalent empty + # values that can differ between in-memory defaults and DB-loaded + # values. Treat them as unchanged. + if hasattr(current, "name") and hasattr(new, "name"): + if (current.name or "") == (new.name or ""): + continue important_fields_changed = True break From 08243dadcd21aa1d65816fea644003ded204b5cd Mon Sep 17 00:00:00 2001 From: Becky Gilbert Date: Wed, 11 Feb 2026 16:50:22 -0800 Subject: [PATCH 6/6] remove unnecessary study.save after study.pause --- studies/models.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/studies/models.py b/studies/models.py index d0663015d..18e23cb54 100644 --- a/studies/models.py +++ b/studies/models.py @@ -611,8 +611,9 @@ def check_and_pause_if_at_max_responses(self): # Use the state machine's pause trigger to properly transition # and run callbacks (like notify_administrators_of_pause) + # Note: no explicit save() needed here because the state machine's + # _finalize_state_change callback already saves the model. self.pause() # No user since this is system-triggered - self.save() @property def consent_videos(self):