diff --git a/kolibri/core/exams/models.py b/kolibri/core/exams/models.py index b7e254d5b13..567049165ef 100644 --- a/kolibri/core/exams/models.py +++ b/kolibri/core/exams/models.py @@ -178,6 +178,10 @@ def get_questions(self): questions.append(question) return questions + def save(self, *args, **kwargs): + self.question_count = len(self.get_questions()) + super().save(*args, **kwargs) + class DraftExam(AbstractExam): @@ -283,6 +287,13 @@ def save(self, *args, **kwargs): if getattr(self, "active", False) is True: if getattr(self, "date_activated") is None: self.date_activated = timezone.now() + # Remove any empty sections from the question sources + # No need to update the question count here, as sections with no questions + # will not have been counted in the question count. + if self.data_model_version == 3: + self.question_sources = [ + section for section in self.question_sources if section.get("questions") + ] super(Exam, self).save(*args, **kwargs) def infer_dataset(self, *args, **kwargs): diff --git a/kolibri/core/exams/serializers.py b/kolibri/core/exams/serializers.py index 95ec1b0a3af..873b3a08e8a 100644 --- a/kolibri/core/exams/serializers.py +++ b/kolibri/core/exams/serializers.py @@ -155,10 +155,19 @@ def validate(self, attrs): "Cannot update question_sources on an Exam object", code=error_constants.INVALID, ) - question_sources = attrs.get("question_sources") or [] - attrs["question_count"] = sum( - len(source.get("questions", [])) for source in question_sources - ) + if not self.instance and not attrs["draft"]: + if not attrs["question_sources"]: + raise ValidationError( + "Cannot create an Exam without any question_sources", + code=error_constants.INVALID, + ) + if all( + not section["questions"] for section in attrs["question_sources"] + ): + raise ValidationError( + "Cannot create an Exam without any questions", + code=error_constants.INVALID, + ) if "learners_see_fixed_order" in attrs and is_non_draft_exam: raise ValidationError( @@ -228,6 +237,11 @@ def update(self, instance, validated_data): # noqa elif instance_is_draft and not new_draft_value: # If this is a draft, but we are updating it to be an exam, then we need to create the new exam # and delete the draft + if not instance.question_count: + raise ValidationError( + "Cannot publish a draft exam with no questions", + code=error_constants.INVALID, + ) if "assignments" not in validated_data: # First check if the assignments are being updated, if not, then we need to set them # to a queryset of collections - this will silently ignore any assignments for collections @@ -269,9 +283,6 @@ def update(self, instance, validated_data): # noqa instance.question_sources = validated_data.pop( "question_sources", instance.question_sources ) - instance.question_count = validated_data.pop( - "question_count", instance.question_count - ) instance.learners_see_fixed_order = validated_data.pop( "learners_see_fixed_order", instance.learners_see_fixed_order ) diff --git a/kolibri/core/exams/test/test_exam_api.py b/kolibri/core/exams/test/test_exam_api.py index e6f245251d1..0be21878e76 100644 --- a/kolibri/core/exams/test/test_exam_api.py +++ b/kolibri/core/exams/test/test_exam_api.py @@ -68,9 +68,25 @@ def setUpTestData(cls): cls.classroom = Classroom.objects.create(name="Classroom", parent=cls.facility) kwargs = dict( title="title", - question_count=1, collection=cls.classroom, creator=cls.admin, + question_sources=[ + { + "section_id": uuid.uuid4().hex, + "section_title": "Test Section Title", + "description": "Test descripton for Section", + "questions": [ + { + "exercise_id": uuid.uuid4().hex, + "question_id": uuid.uuid4().hex, + "title": "Test question Title", + "counter_in_exercise": 0, + } + ], + "question_count": 1, + "learners_see_fixed_order": False, + } + ], ) if cls.class_object == models.Exam: kwargs["active"] = True @@ -508,14 +524,14 @@ def test_exam_model_get_questions_v3(self): def test_exam_model_get_questions_v2_v1(self): self.login_as_admin() self.exam.data_model_version = 2 - self.exam.question_sources.append( + self.exam.question_sources = [ { "exercise_id": uuid.uuid4().hex, "question_id": uuid.uuid4().hex, "title": "Title", "counter_in_exercise": 0, } - ) + ] self.exam.save() self.assertEqual(len(self.exam.get_questions()), 1) @@ -628,6 +644,59 @@ def test_logged_in_admin_exam_update_cannot_update_learners_see_fixed_order(self self.exam.learners_see_fixed_order, previous_learners_see_fixed_order ) + def test_logged_in_admin_exam_can_create_and_publish_remove_empty_sections(self): + self.login_as_admin() + exam = self.make_basic_exam() + exam["draft"] = False + exam["question_sources"].append( + { + "section_id": uuid.uuid4().hex, + "section_title": "Test Section Title", + "description": "Test descripton for Section", + "question_count": 0, + "questions": [], + "learners_see_fixed_order": False, + } + ) + response = self.post_new_exam(exam) + self.assertEqual(response.status_code, 201) + exam_id = response.data["id"] + exam = models.Exam.objects.get(id=exam_id) + self.assertEqual(len(exam.question_sources), 1) + + def test_logged_in_admin_exam_cant_create_and_publish_empty_quiz(self): + self.login_as_admin() + exam = self.make_basic_exam() + exam["draft"] = False + exam["question_sources"] = [] + response = self.post_new_exam(exam) + self.assertEqual(response.status_code, 400) + + def test_logged_in_admin_exam_cant_create_and_publish_empty_sections(self): + self.login_as_admin() + exam = self.make_basic_exam() + exam["draft"] = False + exam["question_sources"] = [ + { + "section_id": uuid.uuid4().hex, + "section_title": "Test Section Title", + "description": "Test descripton for Section", + "question_count": 0, + "questions": [], + "learners_see_fixed_order": False, + }, + { + "section_id": uuid.uuid4().hex, + "section_title": "Test Section Title", + "description": "Test descripton for Section", + "question_count": 0, + "questions": [], + "learners_see_fixed_order": False, + }, + ] + response = self.post_new_exam(exam) + self.assertEqual(response.status_code, 400) + class ExamDraftAPITestCase(BaseExamTest, APITestCase): class_object = models.DraftExam @@ -782,3 +851,38 @@ def test_logged_in_admin_exam_can_create_archive_false(self): exam["archive"] = False response = self.post_new_exam(exam) self.assertEqual(response.status_code, 201) + + def test_logged_in_admin_exam_can_update_and_publish_remove_empty_sections(self): + self.login_as_admin() + self.exam.question_sources = self.make_basic_sections(1) + [ + { + "section_id": uuid.uuid4().hex, + "section_title": "Test Section Title", + "description": "Test descripton for Section", + "question_count": 0, + "questions": [], + "learners_see_fixed_order": False, + } + ] + self.exam.save() + response = self.patch_updated_exam(self.exam.id, {"draft": False}) + self.assertEqual(response.status_code, 200) + exam_id = response.data["id"] + exam = models.Exam.objects.get(id=exam_id) + self.assertEqual(len(exam.question_sources), 1) + + def test_logged_in_admin_exam_cant_update_and_publish_empty_quiz(self): + self.login_as_admin() + self.exam.question_sources = [ + { + "section_id": uuid.uuid4().hex, + "section_title": "Test Section Title", + "description": "Test descripton for Section", + "question_count": 0, + "questions": [], + "learners_see_fixed_order": False, + } + ] + self.exam.save() + response = self.patch_updated_exam(self.exam.id, {"draft": False}) + self.assertEqual(response.status_code, 400) diff --git a/kolibri/plugins/coach/assets/src/views/common/commonCoachStrings.js b/kolibri/plugins/coach/assets/src/views/common/commonCoachStrings.js index 8c0e5532cbc..3678efc7b77 100644 --- a/kolibri/plugins/coach/assets/src/views/common/commonCoachStrings.js +++ b/kolibri/plugins/coach/assets/src/views/common/commonCoachStrings.js @@ -518,11 +518,15 @@ const coachStrings = createTranslator('CommonCoachStrings', { }, openQuizModalDetail: { message: - 'Starting the quiz will make it visible to learners and they will be able to answer questions', - + 'Starting the quiz will make it visible to learners and they will be able to answer questions.', context: "Text shown on a modal pop-up window when the user clicks the 'Start Quiz' button. This explains what will happen when the user confirms the action of starting the quiz.", }, + openQuizModalEmptySections: { + message: 'Any sections without questions will be removed from the quiz.', + context: + "Text shown on a modal pop-up window when the user clicks the 'Start Quiz' button. This explains that empty sections will be removed from the quiz.", + }, closeQuizLabel: { message: 'End quiz', context: diff --git a/kolibri/plugins/coach/assets/src/views/plan/CoachExamsPage/index.vue b/kolibri/plugins/coach/assets/src/views/plan/CoachExamsPage/index.vue index 9eabe4b7098..72ec57c7c93 100644 --- a/kolibri/plugins/coach/assets/src/views/plan/CoachExamsPage/index.vue +++ b/kolibri/plugins/coach/assets/src/views/plan/CoachExamsPage/index.vue @@ -140,6 +140,13 @@ @submit="handleOpenQuiz(activeQuiz.id)" >

{{ openQuizModalDetail$() }}

+

+ {{ openQuizModalEmptySections$() }} +

{{ lodQuizDetail$() }}

{{ fileSizeToDownload$({ size: activeQuiz.size_string }) }}

@@ -222,6 +229,7 @@ openQuizLabel$, closeQuizLabel$, openQuizModalDetail$, + openQuizModalEmptySections$, closeQuizModalDetail$, lodQuizDetail$, fileSizeToDownload$, @@ -260,6 +268,7 @@ openQuizLabel$, closeQuizLabel$, openQuizModalDetail$, + openQuizModalEmptySections$, closeQuizModalDetail$, lodQuizDetail$, fileSizeToDownload$, diff --git a/kolibri/plugins/coach/assets/src/views/reports/ReportsQuizListPage.vue b/kolibri/plugins/coach/assets/src/views/reports/ReportsQuizListPage.vue index e67e412e799..d01e9c9b5fa 100644 --- a/kolibri/plugins/coach/assets/src/views/reports/ReportsQuizListPage.vue +++ b/kolibri/plugins/coach/assets/src/views/reports/ReportsQuizListPage.vue @@ -116,6 +116,13 @@ @submit="handleOpenQuiz(modalQuiz.id)" >

{{ coachString('openQuizModalDetail') }}

+

+ {{ coachString('openQuizModalEmptySections') }} +

{{ coachString('lodQuizDetail') }}

{{ coachString('fileSizeToDownload', { size: modalQuiz.size_string }) }}

diff --git a/kolibri/plugins/coach/class_summary_api.py b/kolibri/plugins/coach/class_summary_api.py index 2797cbfaf54..be4717f62ca 100644 --- a/kolibri/plugins/coach/class_summary_api.py +++ b/kolibri/plugins/coach/class_summary_api.py @@ -271,7 +271,7 @@ def serialize_lessons(request, pk): def _map_exam(item): - data_model_version = item.pop("data_model_version") + data_model_version = item.get("data_model_version") if data_model_version == 3: item["node_ids"] = [ question["exercise_id"]