diff --git a/accounts/queries.py b/accounts/queries.py index 4af1f8ab0..229061929 100644 --- a/accounts/queries.py +++ b/accounts/queries.py @@ -3,7 +3,7 @@ import ast import operator from datetime import date -from functools import reduce +from functools import lru_cache, reduce from itertools import chain from django.db import models @@ -97,9 +97,14 @@ def age_range_eligibility_for_study(child_age_range, study) -> bool: def get_child_eligibility_for_study(child_obj, study_obj): + # Order matters for performance: this runs once per child-study pair across the + # full announcement-email scan. The age-range check is pure Python (no DB), so it + # goes first to short-circuit the majority of ineligible pairs before we incur the + # DB queries in get_child_participation_eligibility or the expression evaluation in + # get_child_eligibility. return ( - get_child_participation_eligibility(child_obj, study_obj) - and _child_in_age_range_for_study(child_obj, study_obj) + _child_in_age_range_for_study(child_obj, study_obj) + and get_child_participation_eligibility(child_obj, study_obj) and get_child_eligibility(child_obj, study_obj.criteria_expression) ) @@ -215,9 +220,18 @@ def get_child_eligibility(child_obj, criteria_expr): return True +@lru_cache(maxsize=1024) def compile_expression(boolean_algebra_expression: str): """Compiles a boolean algebra expression into a python function. + The result is cached (keyed on the expression string) because a criteria + expression only depends on the study, not the child. Without this, the + announcement-email scan re-parses and re-compiles the same expression once + per child-study pair. The number of distinct expressions is bounded by the + number of distinct study criteria (i.e. number of studies), and in reality + there is substantial overlap in expressions across studies, so the cache + stays small. + Args: boolean_algebra_expression: a string boolean algebra expression. diff --git a/env_dist b/env_dist index 17580ecac..8aebf5477 100644 --- a/env_dist +++ b/env_dist @@ -52,6 +52,32 @@ JSPSYCH_S3_ACCESS_KEY_ID= JSPSYCH_S3_SECRET_ACCESS_KEY= JSPSYCH_S3_BUCKET= +# Serve the CHS jsPsych (@lookit/*) packages from locally running dev builds instead of +# the published unpkg URLs stored in the database. When enabled, the jsPsych study runner +# swaps in local dev-server URLs (built in project/settings.py) and drops SRI integrity +# for them. Requires serving each package locally (npm run dev -w @lookit/* in each +# lookit-jspsych package, or honcho start if used to serve multiple packages; +# see https://github.com/lookit/lookit-jspsych#serve-multiple-packages). +# Uncomment to enable, then recreate the web container so the new .env value is +# picked up (`docker compose up -d web` -- a plain `restart` does NOT re-read this file). +# To use the published/unpkg URLs from the DB, either comment this out or set it to a +# falsy value (False/false/0/no/off/empty all disable it). +# JSPSYCH_LOCAL_PLUGINS=True +# +# The dev-server ports below match the defaults set in settings.py and rollup.config.dev.mjs +# files for lookit-jspsych packages, so normally you should only need to set +# JSPSYCH_LOCAL_PLUGINS=True above. The variables below can be uncommented and changed to +# override the host and/or individual ports - only needed if the local lookit-jspsych packages are +# hosted elsewhere or are served on different ports (e.g. because of a conflict). +# These should match whatever you're using to serve each package. +# JSPSYCH_LOCAL_PLUGIN_HOST=http://localhost +# JSPSYCH_LOCAL_PORT_INITJSPSYCH=10001 +# JSPSYCH_LOCAL_PORT_DATA=10002 +# JSPSYCH_LOCAL_PORT_SURVEYS=10003 +# JSPSYCH_LOCAL_PORT_RECORD=10004 +# JSPSYCH_LOCAL_PORT_STYLE=10005 +# JSPSYCH_LOCAL_PORT_TEMPLATES=10006 + # Default repo and branch to use for experiment runner EMBER_EXP_PLAYER_BRANCH=master EMBER_EXP_PLAYER_REPO=https://github.com/lookit/ember-lookit-frameplayer diff --git a/exp/tests/test_response_views.py b/exp/tests/test_response_views.py index 6dbc7637e..87360da59 100644 --- a/exp/tests/test_response_views.py +++ b/exp/tests/test_response_views.py @@ -2,10 +2,12 @@ import datetime import io import json +import os import re +import tempfile import uuid import zipfile -from unittest.mock import patch +from unittest.mock import MagicMock, patch from django.test import Client, TestCase, override_settings from django.urls import reverse @@ -18,6 +20,7 @@ from exp.views.responses import ( StudyResponseSetResearcherFields, get_frame_data, + write_overview_to_temp_file, ) from exp.views.responses_data import RESPONSE_COLUMNS from studies.models import ConsentRuling, Lab, Response, Study, StudyType, Video @@ -1578,6 +1581,151 @@ def test_psychds_download_excludes_unconsented_responses(self): f"Data from unconsented response found in {name}", ) + def test_write_overview_to_temp_file_handles_non_ascii(self): + """Regression test for sentry error LOOKIT-BACKEND-GN. + + The overview temp file must be written as UTF-8 so non-ASCII data + (e.g. the acute accent '\xb4') does not raise UnicodeEncodeError under + an ASCII default locale. The production uWSGI container defaults to an + ASCII locale, so we force that here to reproduce the crash regardless + of the host's locale (which is often UTF-8 in development). + """ + header_list = ["response__id", "child__name"] + session_list = [{"response__id": "1", "child__name": "Rene\xb4 O’Brien"}] + + # CPython resolves a text-mode file's default encoding from the C + # locale, which can't be patched in-process. Instead, simulate the + # production container (whose default text encoding is ASCII) by + # supplying an ASCII default only when the code under test opens a + # text-mode temp file without an explicit encoding. The fix passes + # encoding="utf-8", so it is unaffected; the unfixed code is not. + real_ntf = tempfile.NamedTemporaryFile + + def ascii_default_ntf(*args, **kwargs): + if "b" not in kwargs.get("mode", "r"): + kwargs.setdefault("encoding", "ascii") + return real_ntf(*args, **kwargs) + + with patch( + "exp.views.responses.tempfile.NamedTemporaryFile", + side_effect=ascii_default_ntf, + ): + path = write_overview_to_temp_file(session_list, header_list) + try: + with open(path, encoding="utf-8") as f: + contents = f.read() + finally: + os.unlink(path) + self.assertIn("Rene\xb4 O’Brien", contents) + + def test_psychds_download_with_non_ascii_response_data(self): + """The psychds download must not crash on non-ASCII response data.""" + non_ascii_name = "Rene\xb4" + child = G( + Child, + user=self.non_preview_participant, + given_name=non_ascii_name, + birthday=datetime.date.today() - datetime.timedelta(366), + ) + response = G( + Response, + child=child, + study=self.study, + study_type=self.study.study_type, + completed=True, + completed_consent_frame=True, + sequence=["0-video-config", "1-video-setup", "2-my-consent-frame"], + exp_data={ + "0-video-config": {"frameType": "DEFAULT"}, + "2-my-consent-frame": {"frameType": "CONSENT"}, + "3-my-exit-frame": {"frameType": "EXIT", "feedback": non_ascii_name}, + }, + demographic_snapshot=self.non_preview_demo, + ) + G( + ConsentRuling, + response=response, + action="accepted", + arbiter=self.study_reader, + ) + + self.client.force_login(self.study_reader) + http_response, zip_bytes = self._get_psychds_zip() + self.assertEqual(http_response.status_code, 200) + # zip is valid and the non-ASCII value round-trips as UTF-8 + with zipfile.ZipFile(io.BytesIO(zip_bytes)) as zf: + contents = b"".join(zf.read(name) for name in zf.namelist()) + self.assertIn(non_ascii_name.encode("utf-8"), contents) + + def test_framedata_dict_csv_task_handles_non_ascii(self): + """Regression test for the frame data dictionary download. + + studies.tasks.build_framedata_dict writes a CSV whose frame IDs and + keys come from researcher-authored protocol data, which can be + non-ASCII. The temp file must be written as UTF-8 so it does not raise + UnicodeEncodeError under the production container's ASCII locale. + """ + from studies.tasks import build_framedata_dict + + # exp_data yields a non-ASCII frame id ("café-frame") and key ("réponse") + response = G( + Response, + child=self.non_preview_child, + study=self.study, + study_type=self.study.study_type, + completed=True, + completed_consent_frame=True, + sequence=["0-video-config", "1-café-frame"], + exp_data={ + "0-video-config": {"frameType": "DEFAULT"}, + "1-café-frame": {"frameType": "DEFAULT", "réponse": "oui"}, + }, + demographic_snapshot=self.non_preview_demo, + ) + G( + ConsentRuling, + response=response, + action="accepted", + arbiter=self.study_reader, + ) + + # Simulate the container's ASCII default encoding: apply it only when + # a text-mode file is opened without an explicit encoding. The fix + # passes encoding="utf-8", so it is unaffected; the unfixed code is not. + real_open = open + + def ascii_default_open(*args, **kwargs): + mode = kwargs.get("mode") or (args[1] if len(args) > 1 else "r") + if "b" not in mode: + kwargs.setdefault("encoding", "ascii") + return real_open(*args, **kwargs) + + uploaded = {} + + def capture_upload(path): + with real_open(path, encoding="utf-8") as f: + uploaded["contents"] = f.read() + + mock_blob = MagicMock() + mock_blob.exists.return_value = False + mock_blob.upload_from_filename.side_effect = capture_upload + mock_blob.generate_signed_url.return_value = "https://example.com/signed" + + with ( + override_settings( + GS_PROJECT_ID="test-project", GS_PRIVATE_BUCKET_NAME="test-bucket" + ), + patch("studies.tasks.gc_storage") as mock_gc, + patch("studies.tasks.send_mail"), + patch("builtins.open", side_effect=ascii_default_open), + ): + mock_gc.blob.Blob.return_value = mock_blob + build_framedata_dict("frames_dict", self.study.uuid, self.study_reader.uuid) + + mock_blob.upload_from_filename.assert_called_once() + self.assertIn("café-frame", uploaded["contents"]) + self.assertIn("réponse", uploaded["contents"]) + class ResponseViewResearcherUpdateFieldsTestCase(TestCase): def setUp(self): diff --git a/exp/tests/test_runner_views.py b/exp/tests/test_runner_views.py index 8c795ed50..7c765dab8 100644 --- a/exp/tests/test_runner_views.py +++ b/exp/tests/test_runner_views.py @@ -3,7 +3,7 @@ from unittest.mock import Mock, patch import requests -from django.test import Client, TestCase +from django.test import Client, TestCase, override_settings from django.urls import reverse from django_dynamic_fixture import G from guardian.shortcuts import assign_perm @@ -498,3 +498,62 @@ def test_jspsych_preview_context_contains_autoload_plugins(self, mock_aws): # Should include the autoload plugin self.assertIn(self.autoload_plugin, autoload_plugins) + + @override_settings(JSPSYCH_LOCAL_PLUGINS=False) + @patch("exp.views.study.get_jspsych_aws_values") + def test_preview_chs_plugins_keep_db_urls_without_local_overlay(self, mock_aws): + """With JSPSYCH_LOCAL_PLUGINS off, preview CHS plugins use their DB URLs.""" + mock_aws.return_value = { + "accessKeyId": "test-key", + "secretAccessKey": "test-secret", + "sessionToken": "test-token", + "expiration": "2099-12-31T23:59:59Z", + } + self.client.force_login(self.user) + response = self.client.get( + reverse( + "exp:preview-jspsych", + kwargs={"uuid": self.study.uuid, "child_id": self.child.uuid}, + ) + ) + + chs_plugin = next( + p for p in response.context["chs_plugins"] if p.name == "CHS Templates" + ) + self.assertEqual(chs_plugin.url, "https://unpkg.com/@lookit/templates@3.2.0") + self.assertEqual(chs_plugin.integrity, "sha384-test3") + + @override_settings( + JSPSYCH_LOCAL_PLUGINS=True, + JSPSYCH_LOCAL_PLUGIN_URLS={ + "CHS Templates": "http://localhost:10006/index.browser.js" + }, + ) + @patch("exp.views.study.get_jspsych_aws_values") + def test_preview_chs_plugins_use_local_urls_with_overlay(self, mock_aws): + """With JSPSYCH_LOCAL_PLUGINS on, mapped preview CHS plugins point at the local + dev server and have their SRI integrity cleared.""" + mock_aws.return_value = { + "accessKeyId": "test-key", + "secretAccessKey": "test-secret", + "sessionToken": "test-token", + "expiration": "2099-12-31T23:59:59Z", + } + self.client.force_login(self.user) + response = self.client.get( + reverse( + "exp:preview-jspsych", + kwargs={"uuid": self.study.uuid, "child_id": self.child.uuid}, + ) + ) + + chs_plugin = next( + p for p in response.context["chs_plugins"] if p.name == "CHS Templates" + ) + self.assertEqual(chs_plugin.url, "http://localhost:10006/index.browser.js") + self.assertEqual(chs_plugin.integrity, "") + # The overlay only mutates in-memory objects, not the database. + self.chs_plugin.refresh_from_db() + self.assertEqual( + self.chs_plugin.url, "https://unpkg.com/@lookit/templates@3.2.0" + ) diff --git a/exp/views/responses.py b/exp/views/responses.py index f0fd2d71a..59b67bfb5 100644 --- a/exp/views/responses.py +++ b/exp/views/responses.py @@ -399,7 +399,9 @@ def make_chunk(paginator, page_num, header_options): def write_overview_to_temp_file(session_list, header_list): - tmp = tempfile.NamedTemporaryFile(delete=False, suffix=".csv", mode="w") + tmp = tempfile.NamedTemporaryFile( + delete=False, suffix=".csv", mode="w", encoding="utf-8" + ) tmp.write(",".join(header_list)) for session_row in session_list: tmp.write( @@ -1762,7 +1764,7 @@ def render_to_response(self, context, **response_kwargs): tmp_all_response = tempfile.NamedTemporaryFile(delete=False, suffix=".json") try: - with open(tmp_all_response.name, "w") as f: + with open(tmp_all_response.name, "w", encoding="utf-8") as f: for page_num in paginator.page_range: f.write(make_chunk(paginator, page_num, header_options)) diff --git a/exp/views/study.py b/exp/views/study.py index 0ee79d8a1..aee2c8f7d 100644 --- a/exp/views/study.py +++ b/exp/views/study.py @@ -4,6 +4,7 @@ from functools import reduce from typing import Any, Dict, NamedTuple, Text +from django.conf import settings from django.contrib import messages from django.contrib.auth.mixins import UserPassesTestMixin from django.db.models import Q @@ -27,7 +28,6 @@ ResearcherLoginRequiredMixin, SingleObjectFetchProtocol, ) -from project import settings from studies.forms import ( DEFAULT_GENERATOR, EFPForm, @@ -57,6 +57,7 @@ TRANSITION_LABELS, ) from web.views import ( + _apply_local_plugin_overlay, create_external_response, get_external_url, get_jspsych_aws_values, @@ -977,9 +978,14 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]: context["jspsych_library"] = JSPsychPlugin.objects.filter( category=JSPsychPlugin.Category.JSPSYCH_LIBRARY, autoload=True ).order_by("order") - context["chs_plugins"] = JSPsychPlugin.objects.filter( + chs_plugins = JSPsychPlugin.objects.filter( category=JSPsychPlugin.Category.CHS_JSPSYCH, autoload=True ).order_by("order") + # In local development, serve the CHS (@lookit/*) packages from local dev builds + # instead of the published URLs stored in the database. No-op in staging/prod. + if settings.JSPSYCH_LOCAL_PLUGINS: + chs_plugins = _apply_local_plugin_overlay(chs_plugins) + context["chs_plugins"] = chs_plugins context["autoload_plugins"] = ( JSPsychPlugin.objects.filter(autoload=True) .exclude( diff --git a/project/settings.py b/project/settings.py index 5fb7965fb..352daa835 100644 --- a/project/settings.py +++ b/project/settings.py @@ -66,6 +66,44 @@ ) JSPSYCH_S3_BUCKET = os.environ.get("JSPSYCH_S3_BUCKET", "fakeBucketName") +# Local development overlay for CHS jsPsych (@lookit/*) packages. When enabled, the +# jsPsych study runner views swap the DB URLs for the CHS plugins below to locally +# served dev builds and drop their SRI integrity/crossorigin (local bundles change on +# every rebuild, so a fixed hash would fail). Leave JSPSYCH_LOCAL_PLUGINS unset (or set +# to a falsy value) in staging/production so plugins load from the database URLs. +# NB: parse the string explicitly because bool("False") is True, so a plain +# bool(os.environ.get(...)) would treat JSPSYCH_LOCAL_PLUGINS=False as enabled. +JSPSYCH_LOCAL_PLUGINS = os.environ.get("JSPSYCH_LOCAL_PLUGINS", "").strip().lower() in { + "1", + "true", + "yes", + "on", +} +# Host serving the local lookit-jspsych dev builds. Override in .env only if you serve +# them somewhere other than plain-http localhost. +JSPSYCH_LOCAL_PLUGIN_HOST = os.environ.get( + "JSPSYCH_LOCAL_PLUGIN_HOST", "http://localhost" +) +# Per-CHS-plugin dev-server definition: (JSPsychPlugin.name, port env var, default +# port, served file). Default ports match the lookit-jspsych `npm run serve` ports; +# override an individual port in .env (e.g. JSPSYCH_LOCAL_PORT_DATA=20002) if you have a +# local conflict. The plugin names must match JSPsychPlugin.name records and the file +# names are fixed by each package's build, so both stay in code rather than .env. +_JSPSYCH_LOCAL_PLUGIN_DEFS = [ + ("CHS Init jsPsych", "JSPSYCH_LOCAL_PORT_INITJSPSYCH", "10001", "index.browser.js"), + ("CHS Data", "JSPSYCH_LOCAL_PORT_DATA", "10002", "index.browser.js"), + ("CHS Surveys", "JSPSYCH_LOCAL_PORT_SURVEYS", "10003", "index.browser.js"), + ("CHS Record", "JSPSYCH_LOCAL_PORT_RECORD", "10004", "index.browser.js"), + ("CHS Style", "JSPSYCH_LOCAL_PORT_STYLE", "10005", "index.css"), + ("CHS Templates", "JSPSYCH_LOCAL_PORT_TEMPLATES", "10006", "index.browser.js"), +] +# Built map of JSPsychPlugin.name -> locally served URL, consumed by the view when +# JSPSYCH_LOCAL_PLUGINS is on. +JSPSYCH_LOCAL_PLUGIN_URLS = { + name: f"{JSPSYCH_LOCAL_PLUGIN_HOST}:{os.environ.get(port_var, default_port)}/{filename}" + for name, port_var, default_port, filename in _JSPSYCH_LOCAL_PLUGIN_DEFS +} + # Application definition INSTALLED_APPS = [ diff --git a/studies/tasks.py b/studies/tasks.py index af5eb2728..1df414d0e 100644 --- a/studies/tasks.py +++ b/studies/tasks.py @@ -59,7 +59,9 @@ FROM accounts_child ac INNER JOIN accounts_user au on au.id = ac.user_id CROSS JOIN ( - SELECT id AS study_id + SELECT id AS study_id, + min_age_years, min_age_months, min_age_days, + max_age_years, max_age_months, max_age_days FROM studies_study WHERE state = 'active' AND public = true @@ -67,6 +69,16 @@ WHERE au.is_active = true AND ac.deleted = false AND au.email_new_studies = true + AND ac.birthday IS NOT NULL + -- Age-range eligibility, pushed down from Python so we never materialize the + -- (children x studies) pairs where the child is out of the study's age range -- + -- which is the vast majority of them. Mirrors accounts.queries.study_age_range / + -- child_in_age_range_for_study_days_difference exactly: year = 365 days, + -- month = 30 days, and both bounds inclusive. + AND (CURRENT_DATE - ac.birthday) + >= (ss.min_age_years * 365 + ss.min_age_months * 30 + ss.min_age_days) + AND (CURRENT_DATE - ac.birthday) + <= (ss.max_age_years * 365 + ss.max_age_months * 30 + ss.max_age_days) EXCEPT ( SELECT DISTINCT ac.user_id, sr.child_id, @@ -74,30 +86,33 @@ FROM studies_response sr INNER JOIN accounts_child ac on sr.child_id = ac.id INNER JOIN studies_studytype sst on sr.study_type_id = sst.id - WHERE (sr.completed_consent_frame = true AND sst.id = 1) + -- Internal studies (EFP id 1, jsPsych id 3) count as participation once the + -- child has completed the consent frame; external studies (id 2) always count. + WHERE (sr.completed_consent_frame = true AND sst.id IN (1, 3)) 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 @@ -469,7 +484,7 @@ def build_framedata_dict(filename, study_uuid, requesting_user_uuid): # if it doesn't exist build the file with tempfile.TemporaryDirectory() as temp_directory: file_path = os.path.join(temp_directory, csv_filename) - with open(file_path, "w") as csv_file: + with open(file_path, "w", encoding="utf-8") as csv_file: writer = csv.DictWriter( csv_file, quoting=csv.QUOTE_NONNUMERIC, diff --git a/studies/tests.py b/studies/tests.py index 232c0e536..c1a7af038 100644 --- a/studies/tests.py +++ b/studies/tests.py @@ -293,11 +293,28 @@ def setUp(self): def test_potential_message_targets(self): targets = list(potential_message_targets()) - # Two targets for participant 1: three children for both studies. These - # will be weeded out downstream, as they all fail to meet criteria in one way - # or another. + # Participant 1: only the disabled child (age-eligible for both studies) shows + # up here, giving 2 targets. They're weeded out downstream on criteria. The + # older and younger children are now excluded up front by the age-range filter + # in the SQL query, rather than downstream in _validated. self.assertEqual( - quantify(mt.user_id == self.participant_one.id for mt in targets), 6 + quantify(mt.user_id == self.participant_one.id for mt in targets), 2 + ) + self.assertEqual( + { + mt.study_id + for mt in targets + if mt.user_id == self.participant_one.id + and mt.child_id == self.disabled_child.id + }, + {self.study_one.id, self.study_two.id}, + ) + # Out-of-age-range children are filtered out by the SQL query. + self.assertFalse( + any( + mt.child_id in (self.older_child.id, self.younger_child.id) + for mt in targets + ) ) # Participant #2 @@ -563,6 +580,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) @@ -664,6 +777,76 @@ def test_potential_message_targets_external(self): # Check that the message target no longer has this child for this study self.assertNotIn(message_target, potential_message_targets()) + def _active_jspsych_study_target(self): + """Set up an active, public jsPsych study with an age-eligible user/child.""" + user = G(User, is_active=True) + child = G( + Child, + user=user, + # 1 year old, inside the study's age range so the SQL age filter keeps it. + birthday=date.today() - timedelta(days=365), + ) + study = G( + Study, + name="jsPsych Study", + study_type=StudyType.get_jspsych(), + image=SimpleUploadedFile("fake_image.png", b"", content_type="image/png"), + public=True, + max_age_years=2, + criteria_expression="", + ) + study.state = "active" + study.save() + + # Double check this is a jsPsych study + self.assertTrue(study.study_type.is_jspsych) + + return ( + child, + study, + MessageTarget( + user_id=user.pk, + child_id=child.pk, + study_id=study.pk, + ), + ) + + def test_potential_message_targets_jspsych(self): + # jsPsych (internal study type 3) participation must exclude the child from + # that study's announcement targets, exactly like Ember Frame Player (type 1). + child, study, message_target = self._active_jspsych_study_target() + + # Check that user/child are potential message targets in new jsPsych study + self.assertIn(message_target, potential_message_targets()) + + # Add response from this child for this study, past the consent trial + G( + Response, + study=study, + study_type=study.study_type, + child=child, + completed_consent_frame=True, + ) + + # Check that the message target no longer has this child for this study + self.assertNotIn(message_target, potential_message_targets()) + + def test_potential_message_targets_jspsych_without_consent(self): + # Control for the opposite direction: a jsPsych response that never reached + # the consent trial does not count as participation, so the family must stay + # in the target set (guards against over-excluding). + child, study, message_target = self._active_jspsych_study_target() + + G( + Response, + study=study, + study_type=study.study_type, + child=child, + completed_consent_frame=False, + ) + + self.assertIn(message_target, potential_message_targets()) + def test_validated_skips_none_user(self): result = list(_validated([(None, [])])) self.assertEqual(result, []) diff --git a/web/templates/web/jspsych-study-detail.html b/web/templates/web/jspsych-study-detail.html index 6587c686a..bbd1440d7 100644 --- a/web/templates/web/jspsych-study-detail.html +++ b/web/templates/web/jspsych-study-detail.html @@ -33,19 +33,19 @@ {% for plugin in jspsych_library %} {% if plugin.file_type == "css" %} - + {% endif %} {% endfor %} {% for plugin in autoload_plugins %} {% if plugin.file_type == "css" %} - + {% endif %} {% endfor %} {% for plugin in chs_plugins %} {% if plugin.file_type == "css" %} - + {% endif %} {% endfor %} @@ -57,23 +57,23 @@ {% for plugin in jspsych_library %} {% if plugin.file_type == "js" %} - + {% endif %} {% endfor %} {% for plugin in autoload_plugins %} {% if plugin.file_type == "js" %} - + {% endif %} {% endfor %} {% for plugin in study_plugins %} - + {% endfor %} {% for plugin in chs_plugins %} {% if plugin.file_type == "js" %} - + {% endif %} {% endfor %} diff --git a/web/tests/test_views.py b/web/tests/test_views.py index 890c66e6c..35bf8b4e1 100644 --- a/web/tests/test_views.py +++ b/web/tests/test_views.py @@ -5,7 +5,7 @@ from django.contrib.sites.models import Site from django.core.files.uploadedfile import SimpleUploadedFile -from django.test import Client, TestCase +from django.test import Client, TestCase, override_settings from django.urls import reverse from django.views.generic.list import MultipleObjectMixin from django_dynamic_fixture import G @@ -1053,6 +1053,67 @@ def test_jspsych_experiment_context_contains_chs_plugins(self, mock_aws): # Should include CHS plugin (no show_in_ui filter for participant view) self.assertIn(self.chs_plugin, chs_plugins) + @override_settings(JSPSYCH_LOCAL_PLUGINS=False) + @patch("web.views.get_jspsych_aws_values") + def test_chs_plugins_keep_db_urls_without_local_overlay(self, mock_aws): + """With JSPSYCH_LOCAL_PLUGINS off, CHS plugins use their DB URLs.""" + mock_aws.return_value = { + "accessKeyId": "test-key", + "secretAccessKey": "test-secret", + "sessionToken": "test-token", + "expiration": "2099-12-31T23:59:59Z", + } + client = Client() + client.force_login(self.user) + response = client.get( + reverse( + "web:jspsych-experiment", + kwargs={"uuid": self.study.uuid, "child_id": self.child.uuid}, + ) + ) + + chs_plugin = next( + p for p in response.context["chs_plugins"] if p.name == "CHS Templates" + ) + self.assertEqual(chs_plugin.url, "https://unpkg.com/@lookit/templates@3.2.0") + self.assertEqual(chs_plugin.integrity, "sha384-test3") + + @override_settings( + JSPSYCH_LOCAL_PLUGINS=True, + JSPSYCH_LOCAL_PLUGIN_URLS={ + "CHS Templates": "http://localhost:10006/index.browser.js" + }, + ) + @patch("web.views.get_jspsych_aws_values") + def test_chs_plugins_use_local_urls_with_overlay(self, mock_aws): + """With JSPSYCH_LOCAL_PLUGINS on, mapped CHS plugins point at the local dev + server and have their SRI integrity cleared.""" + mock_aws.return_value = { + "accessKeyId": "test-key", + "secretAccessKey": "test-secret", + "sessionToken": "test-token", + "expiration": "2099-12-31T23:59:59Z", + } + client = Client() + client.force_login(self.user) + response = client.get( + reverse( + "web:jspsych-experiment", + kwargs={"uuid": self.study.uuid, "child_id": self.child.uuid}, + ) + ) + + chs_plugin = next( + p for p in response.context["chs_plugins"] if p.name == "CHS Templates" + ) + self.assertEqual(chs_plugin.url, "http://localhost:10006/index.browser.js") + self.assertEqual(chs_plugin.integrity, "") + # The overlay only mutates in-memory objects, not the database. + self.chs_plugin.refresh_from_db() + self.assertEqual( + self.chs_plugin.url, "https://unpkg.com/@lookit/templates@3.2.0" + ) + @patch("web.views.get_jspsych_aws_values") def test_jspsych_experiment_context_contains_autoload_plugins(self, mock_aws): """Test that experiment context includes autoload_plugins.""" diff --git a/web/views.py b/web/views.py index 83c3221a8..02875e760 100644 --- a/web/views.py +++ b/web/views.py @@ -7,6 +7,7 @@ import boto3 from botocore.exceptions import ClientError +from django.conf import settings from django.contrib import messages from django.contrib.auth import authenticate, login, signals from django.contrib.auth.mixins import UserPassesTestMixin @@ -39,7 +40,6 @@ ) from accounts.utils import hash_id from exp.mixins.paginator_mixin import PaginatorMixin -from project import settings from studies.helpers import get_experiment_absolute_url from studies.models import ( JSPsychPlugin, @@ -109,6 +109,26 @@ def get_external_url(study: Study, response: Response) -> Text: return url.geturl() +def _apply_local_plugin_overlay(plugins): + """Point CHS plugins at locally served dev builds for local development. + + Any plugin whose name appears in settings.JSPSYCH_LOCAL_PLUGIN_URLS has its ``url`` + pointed at the local dev server and its ``integrity`` cleared (local bundles change + on every rebuild, so a fixed SRI hash would fail; a blank integrity also drops the + crossorigin attr in the template). Objects are mutated in memory only -- nothing is + written to the DB. + + Callers should gate this on settings.JSPSYCH_LOCAL_PLUGINS. + """ + local_urls = settings.JSPSYCH_LOCAL_PLUGIN_URLS + overlaid = list(plugins) + for plugin in overlaid: + if plugin.name in local_urls: + plugin.url = local_urls[plugin.name] + plugin.integrity = "" + return overlaid + + def get_jspsych_response(context, is_preview=False): study = context["study"] child_uuid = context["view"].kwargs["child_id"] @@ -959,9 +979,14 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]: context["jspsych_library"] = JSPsychPlugin.objects.filter( category=JSPsychPlugin.Category.JSPSYCH_LIBRARY, autoload=True ).order_by("order") - context["chs_plugins"] = JSPsychPlugin.objects.filter( + chs_plugins = JSPsychPlugin.objects.filter( category=JSPsychPlugin.Category.CHS_JSPSYCH, autoload=True ).order_by("order") + # In local development, serve the CHS (@lookit/*) packages from local dev builds + # instead of the published URLs stored in the database. No-op in staging/prod. + if settings.JSPSYCH_LOCAL_PLUGINS: + chs_plugins = _apply_local_plugin_overlay(chs_plugins) + context["chs_plugins"] = chs_plugins context["autoload_plugins"] = ( JSPsychPlugin.objects.filter(autoload=True) .exclude(