diff --git a/isic/core/templates/core/widgets/multiselect_picker.html b/isic/core/templates/core/widgets/multiselect_picker.html new file mode 100644 index 00000000..94da06e6 --- /dev/null +++ b/isic/core/templates/core/widgets/multiselect_picker.html @@ -0,0 +1,72 @@ + + +{{ widget.value|default_if_none:''|json_script:widget_value_id }} +{{ choice_list|json_script:choice_list_id }} + +
+
+ +
+ +
+ + +
+ +
+ +
+ +
+ selected +
+
diff --git a/isic/studies/forms.py b/isic/studies/forms.py index 448e3534..d5edb592 100644 --- a/isic/studies/forms.py +++ b/isic/studies/forms.py @@ -129,6 +129,13 @@ class OfficialQuestionForm(forms.Form): class CustomQuestionForm(forms.Form): prompt = forms.CharField() + question_type = forms.ChoiceField( + choices=[ + (Question.QuestionType.SELECT.value, "Single Select"), + (Question.QuestionType.MULTISELECT.value, "Multi Select"), + ], + initial=Question.QuestionType.SELECT.value, + ) choices = forms.CharField( help_text="A list of possible choices, one per line.", widget=forms.Textarea() ) diff --git a/isic/studies/migrations/0004_add_multiselect_question_type.py b/isic/studies/migrations/0004_add_multiselect_question_type.py new file mode 100644 index 00000000..ad0f3630 --- /dev/null +++ b/isic/studies/migrations/0004_add_multiselect_question_type.py @@ -0,0 +1,24 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("studies", "0003_alter_questionchoice_text"), + ] + + operations = [ + migrations.AlterField( + model_name="question", + name="type", + field=models.CharField( + choices=[ + ("select", "Select"), + ("multiselect", "Multiselect"), + ("number", "Number"), + ("diagnosis", "Diagnosis"), + ], + default="select", + max_length=11, + ), + ), + ] diff --git a/isic/studies/models/question.py b/isic/studies/models/question.py index 21d04eae..76aed8a8 100644 --- a/isic/studies/models/question.py +++ b/isic/studies/models/question.py @@ -3,21 +3,24 @@ from django.db.models.constraints import UniqueConstraint from django.db.models.query_utils import Q from django.forms.fields import CharField as FormCharField -from django.forms.fields import ChoiceField +from django.forms.fields import ChoiceField, MultipleChoiceField from django.forms.widgets import RadioSelect from django_extensions.db.models import TimeStampedModel -from isic.studies.widgets import DiagnosisPicker +from isic.studies.widgets import DiagnosisPicker, MultiselectPicker class Question(TimeStampedModel): class QuestionType(models.TextChoices): SELECT = "select", "Select" + MULTISELECT = "multiselect", "Multiselect" NUMBER = "number", "Number" DIAGNOSIS = "diagnosis", "Diagnosis" prompt = models.CharField(max_length=400) - type = models.CharField(max_length=9, choices=QuestionType.choices, default=QuestionType.SELECT) + type = models.CharField( + max_length=11, choices=QuestionType.choices, default=QuestionType.SELECT + ) official = models.BooleanField() # TODO: maybe add a default field @@ -34,7 +37,7 @@ class Meta(TimeStampedModel.Meta): def __str__(self) -> str: return self.prompt - def to_form_field(self, *, required: bool) -> ChoiceField | FormCharField: + def to_form_field(self, *, required: bool) -> ChoiceField | MultipleChoiceField | FormCharField: if self.type == self.QuestionType.SELECT: return ChoiceField( required=required, @@ -42,6 +45,13 @@ def to_form_field(self, *, required: bool) -> ChoiceField | FormCharField: label=self.prompt, widget=RadioSelect, ) + elif self.type == self.QuestionType.MULTISELECT: + return MultipleChoiceField( + required=required, + choices=[(choice.pk, choice.text) for choice in self.choices.all()], + label=self.prompt, + widget=MultiselectPicker, + ) elif self.type == self.QuestionType.NUMBER: # TODO: Use floatfield/intfield return FormCharField( @@ -63,6 +73,11 @@ def choices_for_display(self): return [f"All {self.choices.count()} diagnoses"] elif self.type == self.QuestionType.SELECT: return [choice.text for choice in self.choices.all()] + elif self.type == self.QuestionType.MULTISELECT: + count = self.choices.count() + if count > 10: + return [f"Select from {count} options"] + return [choice.text for choice in self.choices.all()] elif self.type == self.QuestionType.NUMBER: return ["Numeric"] else: diff --git a/isic/studies/models/response.py b/isic/studies/models/response.py index ebc3c792..e8292384 100644 --- a/isic/studies/models/response.py +++ b/isic/studies/models/response.py @@ -17,6 +17,8 @@ class ResponseQuerySet(models.QuerySet): def for_display(self) -> Generator[dict[str, Any]]: + choice_text_cache: dict[int, str] = {} + for response in ( self.annotate( value_answer=Cast(F("value"), CharField()), @@ -27,6 +29,7 @@ def for_display(self) -> Generator[dict[str, Any]]: annotator=F("annotation__annotator__profile__hash_id"), annotation_duration=F("annotation__created") - F("annotation__start_time"), question_prompt=F("question__prompt"), + question_type=F("question__type"), answer=Case( When( question__type__in=[ @@ -47,7 +50,9 @@ def for_display(self) -> Generator[dict[str, Any]]: "annotator", "annotation_duration", "question_prompt", + "question_type", "answer", + "value", ) .iterator() ): @@ -58,6 +63,19 @@ def for_display(self) -> Generator[dict[str, Any]]: # 2 days, H:M:S.ms annotation_duration = response["annotation_duration"].total_seconds() + answer = response["answer"] + if response["question_type"] == Question.QuestionType.MULTISELECT: + value = response["value"] + if value and "choices" in value: + choice_pks = value["choices"] + missing_pks = [pk for pk in choice_pks if pk not in choice_text_cache] + if missing_pks: + for choice in QuestionChoice.objects.filter(pk__in=missing_pks): + choice_text_cache[choice.pk] = choice.text + answer = "|".join(choice_text_cache[pk] for pk in choice_pks) + else: + answer = "" + yield { "study_id": response["study_id"], "study": response["study"], @@ -65,7 +83,8 @@ def for_display(self) -> Generator[dict[str, Any]]: "annotator": response["annotator"], "annotation_duration": annotation_duration, "question": response["question_prompt"], - "answer": response["answer"], + "question_type": response["question_type"], + "answer": answer, } diff --git a/isic/studies/tests/test_create_study.py b/isic/studies/tests/test_create_study.py index bd23e271..4611a3b3 100644 --- a/isic/studies/tests/test_create_study.py +++ b/isic/studies/tests/test_create_study.py @@ -36,6 +36,7 @@ def test_create_study( "custom-TOTAL_FORMS": 1, "custom-MAX_NUM_FORMS": "", "custom-0-prompt": "A Custom Question", + "custom-0-question_type": "select", "custom-0-choices": "choice1\nchoice2", "custom-0-required": True, }, diff --git a/isic/studies/tests/test_views.py b/isic/studies/tests/test_views.py index e2bc30f4..17edd799 100644 --- a/isic/studies/tests/test_views.py +++ b/isic/studies/tests/test_views.py @@ -3,7 +3,8 @@ import pytest from isic.factories import UserFactory -from isic.studies.models import Question +from isic.studies.models import Question, Response +from isic.studies.models.question_choice import QuestionChoice from isic.studies.tests.factories import ( AnnotationFactory, QuestionFactory, @@ -90,3 +91,53 @@ def test_study_add_annotators(client, django_capture_on_commit_callbacks, image_ assert r.status_code == 302 assert study.tasks.filter(annotator=new_user).exists() + + +@pytest.mark.django_db +def test_multiselect_question_response_and_export(staff_client) -> None: + question = QuestionFactory.create( + type=Question.QuestionType.MULTISELECT, + choices=[], + ) + choice1 = QuestionChoice.objects.create(question=question, text="Option A") + QuestionChoice.objects.create(question=question, text="Option B") + choice3 = QuestionChoice.objects.create(question=question, text="Option C") + + study = StudyFactory.create(public=False, questions=[question]) + annotation = AnnotationFactory.create(study=study) + + Response.objects.create( + annotation=annotation, + question=question, + value={"choices": [choice1.pk, choice3.pk]}, + ) + + r = staff_client.get(reverse("study-download-responses", args=[study.pk])) + assert r.status_code == 200 + lines = r.content.decode().splitlines() + assert len(lines) == 2 + assert "Option A|Option C" in lines[1] + + +@pytest.mark.django_db +def test_multiselect_question_empty_response(staff_client) -> None: + question = QuestionFactory.create( + type=Question.QuestionType.MULTISELECT, + choices=[], + ) + QuestionChoice.objects.create(question=question, text="Option A") + + study = StudyFactory.create(public=False, questions=[question]) + annotation = AnnotationFactory.create(study=study) + + Response.objects.create( + annotation=annotation, + question=question, + value={"choices": []}, + ) + + r = staff_client.get(reverse("study-download-responses", args=[study.pk])) + assert r.status_code == 200 + lines = r.content.decode().splitlines() + assert len(lines) == 2 + assert lines[1].endswith(",") diff --git a/isic/studies/views.py b/isic/studies/views.py index b9e580f5..9a93b6d8 100644 --- a/isic/studies/views.py +++ b/isic/studies/views.py @@ -131,7 +131,11 @@ def study_create_view(request): ) for custom_question in custom_question_formset.cleaned_data: - q = Question.objects.create(prompt=custom_question["prompt"], official=False) + q = Question.objects.create( + prompt=custom_question["prompt"], + type=custom_question["question_type"], + official=False, + ) for choice in custom_question["choices"]: QuestionChoice.objects.create(question=q, text=choice) study.questions.add(q, through_defaults={"required": custom_question["required"]}) @@ -329,7 +333,14 @@ def study_responses_csv(request, pk): f'attachment; filename="{slugify(study.name)}_responses_{current_time}.csv"' ) - fieldnames = ["image", "annotator", "annotation_duration", "question", "answer"] + fieldnames = [ + "image", + "annotator", + "annotation_duration", + "question", + "question_type", + "answer", + ] writer = EscapingDictWriter(http_response, fieldnames) writer.writeheader() for study_responses_data in study_responses.for_display(): @@ -394,10 +405,16 @@ def study_task_detail(request, pk): if float(response_value).is_integer() else float(response_value), ) + elif question.type == Question.QuestionType.MULTISELECT: + choice_pks = [int(pk) for pk in response_value] + annotation.responses.create( + question=question, value={"choices": choice_pks} + ) else: choices = {x.pk: x for x in question.choices.all()} - choice = choices[int(response_value)] - annotation.responses.create(question=question, choice=choice) + annotation.responses.create( + question=question, choice=choices[int(response_value)] + ) return maybe_redirect_to_next_study_task(request, study_task.study) else: diff --git a/isic/studies/widgets.py b/isic/studies/widgets.py index 70d43663..1c4695c7 100644 --- a/isic/studies/widgets.py +++ b/isic/studies/widgets.py @@ -15,3 +15,14 @@ def get_context(self, name: str, value: Any, attrs: dict[str, Any] | None) -> di # set the default so templates won't complain about undefined variables context.setdefault("value", None) return context + + +class MultiselectPicker(forms.CheckboxSelectMultiple): + template_name = "core/widgets/multiselect_picker.html" + + def get_context(self, name: str, value: Any, attrs: dict[str, Any] | None) -> dict[str, Any]: + context = super().get_context(name, value, attrs) + context["choice_list"] = [{"pk": choice[0], "text": choice[1]} for choice in self.choices] + context["widget_value_id"] = f"multiselect-value-{name}" + context["choice_list_id"] = f"multiselect-choices-{name}" + return context