Merge pull request #12252 from rtibbles/empty_sectionals

Remove empty sections from quizzes on publish

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):

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
                 )

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)

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:

kolibri/plugins/coach/assets/src/views/plan/CoachExamsPage/index.vue

@@ -140,6 +140,13 @@
           @submit="handleOpenQuiz(activeQuiz.id)"
         >
           <p>{{ openQuizModalDetail$() }}</p>
+          <p
+            v-if="
+              activeQuiz.data_model_version === 3 &&
+                activeQuiz.question_sources.some(s => (!s.questions || s.questions.length === 0))"
+          >
+            {{ openQuizModalEmptySections$() }}
+          </p>
           <p>{{ lodQuizDetail$() }}</p>
           <p>{{ fileSizeToDownload$({ size: activeQuiz.size_string }) }}</p>
         </KModal>
@@ -222,6 +229,7 @@
         openQuizLabel$,
         closeQuizLabel$,
         openQuizModalDetail$,
+        openQuizModalEmptySections$,
         closeQuizModalDetail$,
         lodQuizDetail$,
         fileSizeToDownload$,
@@ -260,6 +268,7 @@
         openQuizLabel$,
         closeQuizLabel$,
         openQuizModalDetail$,
+        openQuizModalEmptySections$,
         closeQuizModalDetail$,
         lodQuizDetail$,
         fileSizeToDownload$,

kolibri/plugins/coach/assets/src/views/reports/ReportsQuizListPage.vue

@@ -116,6 +116,13 @@
           @submit="handleOpenQuiz(modalQuiz.id)"
         >
           <p>{{ coachString('openQuizModalDetail') }}</p>
+          <p
+            v-if="
+              modalQuiz.data_model_version === 3 &&
+                modalQuiz.question_sources.some(s => (!s.questions || s.questions.length === 0))"
+          >
+            {{ coachString('openQuizModalEmptySections') }}
+          </p>
           <p>{{ coachString('lodQuizDetail') }}</p>
           <p>{{ coachString('fileSizeToDownload', { size: modalQuiz.size_string }) }}</p>
         </KModal>

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"]