diff --git a/studies/models.py b/studies/models.py index 26e0bd3c0..5f9456ea6 100644 --- a/studies/models.py +++ b/studies/models.py @@ -8,6 +8,7 @@ import fleep from botocore.exceptions import ClientError from django.conf import settings +from django.contrib import messages from django.contrib.auth.models import Group, Permission from django.contrib.postgres.fields import ArrayField from django.core.validators import MaxValueValidator, MinValueValidator @@ -590,13 +591,47 @@ def has_reached_max_responses(self) -> bool: return False return self.valid_response_count >= self.max_responses - def check_and_pause_if_at_max_responses(self): + def check_and_pause_if_at_max_responses( + self, send_researcher_email=False, request=None + ): """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. + + Args: + send_researcher_email: If True, send notification email to researchers + with CHANGE_STUDY_STATUS permission. + request: If provided, display a Django messages banner to the user. """ - # 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 + + # 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). + # 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 + + if send_researcher_email: + self._notify_researchers_of_max_responses_pause() + + if request: + messages.warning( + request, + f'Study "{self.name}" has been automatically paused because it ' + f"reached the response limit ({self.valid_response_count}/{self.max_responses}).", + ) @property def consent_videos(self): @@ -825,12 +860,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( @@ -845,6 +884,28 @@ def notify_administrators_of_pause(self, ev): **context, ) + def _notify_researchers_of_max_responses_pause(self): + """Send email to researchers notifying them the study was auto-paused + because it reached the maximum number of responses.""" + context = { + "study_name": self.name, + "study_id": self.pk, + "study_uuid": str(self.uuid), + "max_responses": self.max_responses, + "valid_response_count": self.valid_response_count, + } + send_mail.delay( + "notify_researchers_of_max_responses_pause", + f"{self.name}: Study paused - response limit reached", + settings.EMAIL_FROM_ADDRESS, + bcc=list( + self.users_with_study_perms( + StudyPermission.CHANGE_STUDY_STATUS + ).values_list("username", flat=True) + ), + **context, + ) + def notify_administrators_of_deactivation(self, ev): context = { "lab_name": self.lab.name, @@ -998,6 +1059,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 @@ -1344,9 +1411,9 @@ def take_action_on_exp_data(sender, instance, created, **kwargs): else: dispatch_frame_action(response) - # If this response is complete, then check if this study has reached max responses and pause if needed + # If this response is complete, then check if this study has reached max responses and, if so, pause the study and email researchers. if response.completed: - response.study.check_and_pause_if_at_max_responses() + response.study.check_and_pause_if_at_max_responses(send_researcher_email=True) class FeedbackApiManager(models.Manager): diff --git a/studies/templates/emails/notify_researchers_of_max_responses_pause.html b/studies/templates/emails/notify_researchers_of_max_responses_pause.html new file mode 100644 index 000000000..3df89229f --- /dev/null +++ b/studies/templates/emails/notify_researchers_of_max_responses_pause.html @@ -0,0 +1,17 @@ +{% load web_extras %} +

Dear Study Researchers,

+

+ Your study {{ study_name }} has been automatically paused + because it reached the maximum number of valid responses + ({{ valid_response_count }}/{{ max_responses }}). +

+

+ If you would like to collect more data, you can increase the study's response limit and/or edit the valid/invalid status of individual responses, and then re-start it (your study will NOT be re-started automatically). +
+ Your study can be found here. +

+

+ Best, +
+ Lookit Bot +

diff --git a/studies/templates/emails/notify_researchers_of_max_responses_pause.txt b/studies/templates/emails/notify_researchers_of_max_responses_pause.txt new file mode 100644 index 000000000..3ff720281 --- /dev/null +++ b/studies/templates/emails/notify_researchers_of_max_responses_pause.txt @@ -0,0 +1,11 @@ +{% load web_extras %} +Dear Study Researchers, + +Your study {{ study_name }} has been automatically paused because it reached the maximum number of valid responses ({{ valid_response_count }}/{{ max_responses }}). + +If you would like to collect more data, you can increase the study's response limit and/or edit the valid/invalid status of individual responses, and then re-start it (your study will NOT be re-started automatically). + +Your study can be found here: {% absolute_url 'exp:study' study_id %} + +Best, +Lookit Bot diff --git a/studies/tests.py b/studies/tests.py index ab15a1385..5c4089a87 100644 --- a/studies/tests.py +++ b/studies/tests.py @@ -1,7 +1,7 @@ import json import re from datetime import date, datetime, timedelta, timezone -from unittest.mock import patch +from unittest.mock import MagicMock, patch from botocore.exceptions import ClientError, ParamValidationError from django.conf import settings @@ -841,7 +841,55 @@ 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 + ): + """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 +915,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, - ) - Response.objects.filter(pk=invalid4.pk).update( - eligibility=[ResponseEligibility.ELIGIBLE] + 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 +951,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 +994,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 +1003,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,24 +1012,141 @@ 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()) + self._create_eligible_responses(study, count=4) + self.assertTrue(study.has_reached_max_responses) - # 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] - ) + def test_check_and_pause_if_at_max_responses_no_limit_set(self): + """Study without max_responses set should not pause.""" + study = self._create_study_with_lab( + "No Limit Study", max_responses=None, state="active" + ) - self.assertTrue(study.has_reached_max_responses) + 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 = self._create_study_with_lab("Not Active Study", 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 = self._create_study_with_lab( + "Under Limit Study", max_responses=5, state="active" + ) + self._create_eligible_responses(study, count=2) + + 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 = self._create_study_with_lab( + "At Limit Study", max_responses=2, state="active" + ) + self._create_eligible_responses(study, count=2) + + 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 = self._create_study_with_lab( + "Over Limit Study", max_responses=2, state="active" + ) + self._create_eligible_responses(study, count=4) + + study.check_and_pause_if_at_max_responses() + study.refresh_from_db() + + self.assertEqual(study.state, "paused") + + @patch("studies.models.send_mail") + def test_check_and_pause_sends_researcher_email_when_requested( + self, mock_send_mail + ): + """Researcher notification email is sent when send_researcher_email=True.""" + study = self._create_study_with_lab( + "Email Test", max_responses=2, state="active" + ) + self._create_eligible_responses(study, count=2) + + study.check_and_pause_if_at_max_responses(send_researcher_email=True) + + researcher_calls = [ + c + for c in mock_send_mail.delay.call_args_list + if c[0][0] == "notify_researchers_of_max_responses_pause" + ] + self.assertEqual(len(researcher_calls), 1) + + @patch("studies.models.send_mail") + def test_check_and_pause_no_researcher_email_by_default(self, mock_send_mail): + """Researcher notification email is not sent by default.""" + study = self._create_study_with_lab( + "No Email Test", max_responses=2, state="active" + ) + self._create_eligible_responses(study, count=2) + + study.check_and_pause_if_at_max_responses() + + researcher_calls = [ + c + for c in mock_send_mail.delay.call_args_list + if c[0][0] == "notify_researchers_of_max_responses_pause" + ] + self.assertEqual(len(researcher_calls), 0) + + @patch("studies.models.send_mail") + @patch("studies.models.messages") + def test_check_and_pause_shows_banner_when_request_provided( + self, mock_messages, mock_send_mail + ): + """A Django messages warning is added when request is provided.""" + study = self._create_study_with_lab( + "Banner Test", max_responses=2, state="active" + ) + self._create_eligible_responses(study, count=2) + mock_request = MagicMock() + + study.check_and_pause_if_at_max_responses(request=mock_request) + + mock_messages.warning.assert_called_once() + call_args = mock_messages.warning.call_args + self.assertEqual(call_args[0][0], mock_request) + self.assertIn("automatically paused", call_args[0][1]) + + researcher_calls = [ + c + for c in mock_send_mail.delay.call_args_list + if c[0][0] == "notify_researchers_of_max_responses_pause" + ] + self.assertEqual(len(researcher_calls), 0) + + @patch("studies.models.send_mail") + @patch("studies.models.messages") + def test_check_and_pause_no_banner_by_default(self, mock_messages, mock_send_mail): + """No Django message is added when request is not provided.""" + study = self._create_study_with_lab( + "No Banner Test", max_responses=2, state="active" + ) + self._create_eligible_responses(study, count=2) + + study.check_and_pause_if_at_max_responses() + + mock_messages.warning.assert_not_called() class VideoModelTestCase(TestCase):