Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update exam.question_count calculation #12196

Merged
merged 6 commits into from
May 24, 2024
Merged

Update exam.question_count calculation #12196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e335845
Update exam.question_count calculation
AlexVelezLl May 22, 2024
4580d82
Fix send quiz button margins
AlexVelezLl May 22, 2024
7b89f69
Calculate question_count in backend
AlexVelezLl May 23, 2024
094e1fa
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] May 23, 2024
967becc
Update tests
AlexVelezLl May 23, 2024
4ecc912
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions kolibri/core/exams/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class ExamSerializer(ModelSerializer):
creator = PrimaryKeyRelatedField(
read_only=False, queryset=FacilityUser.objects.all()
)
question_count = IntegerField()
question_count = IntegerField(allow_null=True)
date_archived = DateTimeTzField(allow_null=True)
date_activated = DateTimeTzField(allow_null=True)

Expand All @@ -83,7 +83,7 @@ class Meta:
"learners_see_fixed_order",
"learner_ids",
)
read_only_fields = ("data_model_version",)
read_only_fields = ("data_model_version", "question_count")

def validate(self, attrs):
title = attrs.get("title")
Expand Down Expand Up @@ -137,6 +137,12 @@ def to_internal_value(self, data):
else:
# Otherwise we are just updating the exam, so allow a partial update
self.partial = True

question_sources = data.get("question_sources", [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to update this: https://github.com/learningequality/kolibri/blob/develop/kolibri/core/exams/test/test_exam_api.py#L45 to remove this from the data we send to the backend in tests, and then add a test to assert that the value is properly set on create?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do this in #12200 to keep the clean-up separate from the fix here.

data["question_count"] = sum(
len(source.get("questions", [])) for source in question_sources
)

return super(ExamSerializer, self).to_internal_value(data)

def create(self, validated_data):
Expand Down
14 changes: 12 additions & 2 deletions kolibri/core/exams/test/test_exam_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,8 @@ def setUpTestData(cls):

def make_basic_exam(self):
sections = self.make_basic_sections(1)
total_questions = sum(section["question_count"] for section in sections)
return {
"title": "Exam",
"question_count": total_questions,
"active": True,
"collection": self.classroom.id,
"learners_see_fixed_order": False,
Expand Down Expand Up @@ -551,3 +549,15 @@ def test_exam_model_get_questions_v2_v1(self):

self.exam.save()
self.assertEqual(len(self.exam.get_questions()), 1)

def test_exam_question_count_calculation(self):
self.login_as_admin()
exam = self.make_basic_exam()
question_count = sum(
len(source["questions"]) for source in exam["question_sources"]
)
response = self.post_new_exam(exam)
exam_id = response.data["id"]
self.assertEqual(response.status_code, 201)
exam_model_instance = models.Exam.objects.get(id=exam_id)
self.assertEqual(exam_model_instance.question_count, question_count)
9 changes: 0 additions & 9 deletions kolibri/plugins/coach/assets/src/composables/useQuizCreation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,6 @@ export default function useQuizCreation() {
* @throws {Error} if quiz is not valid
*/
function saveQuiz() {
const totalQuestions = get(allSections).reduce((acc, section) => {
acc += parseInt(section.question_count);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nucleogenesis - this value again feels like something we maybe shouldn't even store in the backend, as all it seems to represent is a user intention of how many questions there should be in the section?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's a good point - it can always be derived from the length of questions anyway.

@AlexVelezLl I'll include the removal of saving this in my PR #12200

return acc;
}, 0);

set(_quiz, {
...get(_quiz),
question_count: totalQuestions,
});
if (!validateQuiz(get(_quiz))) {
throw new Error(`Quiz is not valid: ${JSON.stringify(get(_quiz))}`);
}
Expand Down
1 change: 1 addition & 0 deletions kolibri/plugins/learn/assets/src/views/ExamPage/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,7 @@

.answered {
display: inline-block;
margin-right: 8px;
margin-left: 8px;
white-space: nowrap;
}
Expand Down