diff --git a/exp/tests/test_response_views.py b/exp/tests/test_response_views.py index 24eff0002..dacb841a0 100644 --- a/exp/tests/test_response_views.py +++ b/exp/tests/test_response_views.py @@ -204,14 +204,15 @@ def setUp(self): ), reverse("exp:study-attachments", kwargs={"pk": self.study.pk}), ] - # For testing researcher-editable response fields: researcher_session_status, researcher_payment_status, researcher_star + # For testing researcher-editable response fields: researcher_session_status, researcher_payment_status, researcher_star, is_valid self.editable_fields = StudyResponseSetResearcherFields.EDITABLE_FIELDS default_values = [ "", "", False, - ] # These correspond to session status, payment status, and star - new_values = ["follow_up", "to_pay", True] + True, + ] # These correspond to session status, payment status, star, and valid response + new_values = ["follow_up", "to_pay", True, False] self.fields_default_values = { self.editable_fields[i]: default_values[i] for i in range(len(self.editable_fields)) @@ -678,7 +679,8 @@ def setUp(self): "", "", False, - ] # These correspond to session status, payment status, and star + True, + ] # These correspond to session status, payment status, star, and valid response self.fields_default_values = { self.editable_fields[i]: default_values[i] for i in range(len(self.editable_fields)) @@ -1457,15 +1459,16 @@ def setUp(self): action="accepted", arbiter=self.other_researcher, ) - # For testing researcher-editable response fields: researcher_session_status, researcher_payment_status, researcher_star + # For testing researcher-editable response fields: researcher_session_status, researcher_payment_status, researcher_star, is_valid self.editable_fields = StudyResponseSetResearcherFields.EDITABLE_FIELDS default_values = [ "", "", False, - ] # These correspond to session status, payment status, and star - new_values = ["follow_up", "to_pay", True] - invalid_values = ["some_other_string", 42, "true"] + True, + ] # These correspond to session status, payment status, star, and valid response + new_values = ["follow_up", "to_pay", True, False] + invalid_values = ["some_other_string", 42, "true", "not_a_bool"] self.fields_default_values = { self.editable_fields[i]: default_values[i] for i in range(len(self.editable_fields)) @@ -1517,11 +1520,12 @@ def test_update_fails_with_invalid_values(self): url = reverse( "exp:study-responses-researcher-update", kwargs={"pk": self.study.pk} ) - # These correspond to the fields: session status, payment status, star + # These correspond to the fields: session status, payment status, star, valid response err_strings = [ "Invalid request: Session Status must be one of ", "Invalid request: Payment Status must be one of ", "Invalid request: Star field must be a boolean value.", + "Invalid request: Valid Response must be a boolean value.", ] fields_err_strings = { self.editable_fields[i]: err_strings[i] diff --git a/exp/views/responses.py b/exp/views/responses.py index ba43e4476..72c3e4a81 100644 --- a/exp/views/responses.py +++ b/exp/views/responses.py @@ -132,7 +132,7 @@ def get_response_headers( } selected_standard_header_ids = [ col.id - for col in RESPONSE_COLUMNS[0:-2] + for col in RESPONSE_COLUMNS[0:-3] if col.id not in unselected_optional_ids ] return selected_standard_header_ids + sorted( @@ -879,6 +879,7 @@ def get_context_data(self, **kwargs): "response__researcher_payment_status", "response__researcher_session_status", "response__researcher_star", + "response__is_valid", ] context["session_status_options"] = list(Response.SESSION_STATUS_CHOICES) @@ -1120,6 +1121,7 @@ class StudyResponseSetResearcherFields( "researcher_session_status", "researcher_payment_status", "researcher_star", + "is_valid", ] def user_can_edit_response(self): @@ -1195,6 +1197,14 @@ def post(self, request, *args, **kwargs): {"error": "Invalid request: Star field must be a boolean value."}, status=400, ) + elif field_id == self.EDITABLE_FIELDS[3]: + if not isinstance(value, bool): + return JsonResponse( + { + "error": "Invalid request: Valid Response must be a boolean value." + }, + status=400, + ) # Try updating the Response object try: diff --git a/exp/views/responses_data.py b/exp/views/responses_data.py index 689524da9..7c7239409 100644 --- a/exp/views/responses_data.py +++ b/exp/views/responses_data.py @@ -382,6 +382,12 @@ class ResponseDataColumn(NamedTuple): extractor=lambda resp: resp.researcher_star, name="Star", ), + ResponseDataColumn( + id="response__is_valid", + description="Whether this response is counted as valid", + extractor=lambda resp: resp.is_valid, + name="Valid Response", + ), ] # Columns for demographic data downloads. Extractor functions expect Response values dict, diff --git a/scss/study-responses.scss b/scss/study-responses.scss index 18ec44171..a5099e071 100644 --- a/scss/study-responses.scss +++ b/scss/study-responses.scss @@ -31,11 +31,32 @@ select.researcher-editable:disabled { cursor: not-allowed; } -input[type="checkbox"].researcher-editable:disabled+label .icon-star { +input[type="checkbox"].researcher-editable:disabled+label .icon-star, +input[type="checkbox"].researcher-editable:disabled+label .icon-valid-check, +input[type="checkbox"].researcher-editable:disabled+label .icon-valid-xmark { color: lightgray; + fill: lightgray; cursor: not-allowed; } +// Valid/invalid response icons: show check or xmark, only one at a time +.icon-valid-check, +.icon-valid-xmark { + display: none; +} + +.icon-valid-check.icon-valid-filled { + display: inline; + color: var(--bs-success); + fill: var(--bs-success); +} + +.icon-valid-xmark.icon-invalid-filled { + display: inline; + color: $red; + fill: $red; +} + // Response info box .truncate-parent-feedback { overflow: hidden; diff --git a/studies/migrations/0106_add_researcher_valid_response.py b/studies/migrations/0106_add_researcher_valid_response.py new file mode 100644 index 000000000..35ec0dabb --- /dev/null +++ b/studies/migrations/0106_add_researcher_valid_response.py @@ -0,0 +1,66 @@ +from django.db import migrations, models + +REJECTED = "rejected" +EXTERNAL_STUDY_TYPE_ID = 2 + + +def compute_is_valid(apps, schema_editor): + """Compute is_valid for all existing responses using the valid_response_count criteria. + + A response is valid if: + - is_preview is False + - eligibility is "Eligible" or blank/empty + + For internal studies, responses must also: + - completed is True + - completed_consent_frame is True + - the most recent consent ruling is not "rejected" + """ + Response = apps.get_model("studies", "Response") + ConsentRuling = apps.get_model("studies", "ConsentRuling") + + # Step 1: Mark all preview responses as invalid + Response.objects.filter(is_preview=True).update(is_valid=False) + + # Step 2: Mark responses with ineligible eligibility as invalid + # Valid eligibility: empty list OR contains "Eligible" + # Invalid: non-empty list that doesn't contain "Eligible" + Response.objects.exclude( + models.Q(eligibility=[]) | models.Q(eligibility__contains=["Eligible"]) + ).update(is_valid=False) + + # Step 3: For internal studies only, mark incomplete responses and responses without consent frames as invalid + Response.objects.exclude(study__study_type_id=EXTERNAL_STUDY_TYPE_ID).filter( + models.Q(completed=False) | models.Q(completed_consent_frame=False) + ).update(is_valid=False) + + # Step 4: For internal studies, mark responses with rejected consent as invalid + # Get the most recent consent ruling for each response using a subquery + newest_ruling_subquery = models.Subquery( + ConsentRuling.objects.filter(response=models.OuterRef("pk")) + .order_by("-created_at") + .values("action")[:1] + ) + rejected_response_ids = list( + Response.objects.exclude(study__study_type_id=EXTERNAL_STUDY_TYPE_ID) + .annotate(current_ruling=newest_ruling_subquery) + .filter(current_ruling=REJECTED) + .values_list("id", flat=True) + ) + if rejected_response_ids: + Response.objects.filter(id__in=rejected_response_ids).update(is_valid=False) + + +class Migration(migrations.Migration): + dependencies = [ + ("studies", "0105_add_max_responses_to_study"), + ] + + operations = [ + migrations.AddField( + model_name="response", + name="is_valid", + field=models.BooleanField(default=True), + ), + migrations.RunPython(compute_is_valid, migrations.RunPython.noop), + ] diff --git a/studies/models.py b/studies/models.py index f1afad753..5e9ffa669 100644 --- a/studies/models.py +++ b/studies/models.py @@ -1172,7 +1172,6 @@ class Response(models.Model): ("communication_complete", _("Communication complete")), ("withdrawn_closed", _("Withdrawn or closed")), ) - uuid = models.UUIDField(default=uuid.uuid4, unique=True, db_index=True) study = models.ForeignKey( Study, on_delete=models.PROTECT, related_name="responses" @@ -1214,6 +1213,7 @@ class Response(models.Model): choices=SESSION_STATUS_CHOICES, max_length=22, blank=True ) researcher_star = models.BooleanField(default=False) + is_valid = models.BooleanField(default=True) def __str__(self): return self.display_name diff --git a/studies/templates/studies/study_responses.html b/studies/templates/studies/study_responses.html index 5603a8fcf..09ccbdc0a 100644 --- a/studies/templates/studies/study_responses.html +++ b/studies/templates/studies/study_responses.html @@ -118,8 +118,24 @@ id="response-{{ forloop.counter }}" data-response-id="{{ response.response__id }}" data-response-uuid="{{ response.response__uuid }}"> - - {% if response.response__is_preview %}P{% endif %} + + {% if response.response__is_preview %} + P + {% else %} + + + {% endif %} {{ response.child_id_slug }} {{ response.response__id }} @@ -169,7 +185,8 @@ {% endfor %} - + - + + + @@ -242,7 +270,15 @@ - {% comment %} Empty footer element for the star column {% endcomment %} + diff --git a/web/static/custom_bootstrap5.css b/web/static/custom_bootstrap5.css index c423229fd..aad067408 100644 --- a/web/static/custom_bootstrap5.css +++ b/web/static/custom_bootstrap5.css @@ -45,10 +45,27 @@ select.researcher-editable:disabled { background-color: lightgray; cursor: not-allowed; } -input[type="checkbox"].researcher-editable:disabled + label .icon-star { +input[type="checkbox"].researcher-editable:disabled + label .icon-star, +input[type="checkbox"].researcher-editable:disabled + label .icon-valid-check, +input[type="checkbox"].researcher-editable:disabled + label .icon-valid-xmark { color: lightgray; + fill: lightgray; cursor: not-allowed; } +.icon-valid-check, +.icon-valid-xmark { + display: none; } + +.icon-valid-check.icon-valid-filled { + display: inline; + color: var(--bs-success); + fill: var(--bs-success); } + +.icon-valid-xmark.icon-invalid-filled { + display: inline; + color: #ff5f5c; + fill: #ff5f5c; } + .truncate-parent-feedback { overflow: hidden; text-overflow: ellipsis; diff --git a/web/static/js/study-responses.js b/web/static/js/study-responses.js index b3958a26b..f929da353 100644 --- a/web/static/js/study-responses.js +++ b/web/static/js/study-responses.js @@ -75,9 +75,9 @@ $('.researcher-editable').change( const target = event.target; target.disabled = true; const currentResponseId = target.closest('tr').dataset.responseId; - const fieldName = 'researcher_' + target.name.replace("-", "_"); - // The Star element's value is "on"/"off" but the database needs a boolean - const fieldValue = (target.name == "star") ? target.checked : target.value; + const fieldName = target.dataset.field || ('researcher_' + target.name.replace("-", "_")); + // Checkbox elements' values are "on"/"off" but the database needs a boolean + const fieldValue = (target.type === "checkbox") ? target.checked : target.value; const data = { responseId: currentResponseId, field: fieldName, @@ -210,7 +210,7 @@ const resp_table = $("#individualResponsesTable").DataTable({ order: [[2, 'desc']], // Sort on "Response ID" column (most recent first). Sorting on Date doesn't work as expected. columnDefs: [ { className: "column-text-search", targets: [1, 2, 4] }, // add class to text search columns - { className: "column-dropdown-search", targets: [5, 6, 7] }, // add class to dropdown search columns + { className: "column-dropdown-search", targets: [0, 5, 6, 7, 8] }, // add class to dropdown search columns // This tells datatables to sort "Time Elapsed" and "Date" by "Response ID" column's data. Date sorting isn't working (it doesn't take the full timestamp into account). The right way to do this is to pass the full timestamp into the template and tell datatables how to parse and display it, but I couldn't get that to work (docs https://datatables.net/examples/datetime/.) { orderData: 2, targets: [2, 3, 4] }, { targets: 3, type: 'date', render: dateColRender} @@ -238,6 +238,14 @@ function updateAJAXCellData(target) { el.classList.toggle('icon-star-filled') }) td.dataset.sort = "False" == td.dataset.sort ? "True" : "False" + // JS uses unicode rather than HTML &# chars. ★ = \u2605, ☆ = \u2606 + td.dataset.filter = target.checked ? "Starred \u2605" : "Unstarred \u2606" + } else if (classes.contains("valid-checkbox")) { + td.querySelector('.icon-valid-check').classList.toggle('icon-valid-filled') + td.querySelector('.icon-valid-xmark').classList.toggle('icon-invalid-filled') + td.dataset.sort = "False" == td.dataset.sort ? "True" : "False" + // JS uses unicode rather than HTML &# chars. ✔ = \u2714, ✘ = \u2718 + td.dataset.filter = target.checked ? "Valid \u2714" : "Invalid \u2718" } resp_table.rows().invalidate("dom").draw(false);