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

Add quiz report visibility control for coaches #13064

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Open

Add quiz report visibility control for coaches #13064

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
09301e8
Update exam model, views, and api with instant_report_visibility fiel…
LianaHarris360 Feb 11, 2025
3b64d66
Add exam field instant_report_visibility for use in the frontend
LianaHarris360 Feb 11, 2025
e39ccdb
Update commonCoachStrings with new strings for report visibility sele…
LianaHarris360 Feb 11, 2025
39bb753
Add report visibility selection button to frontend quiz creation
LianaHarris360 Feb 11, 2025
fa607e3
Add report visibility label & description to quiz status page
LianaHarris360 Feb 11, 2025
972728c
Update learner view to show quiz reports only if the exam is closed o…
LianaHarris360 Feb 11, 2025
272a948
Update exam api tests with instant_report_visibility field
LianaHarris360 Feb 11, 2025
80645ac
Set data prop instantReportVisibility using assignment.instant_report…
LianaHarris360 Feb 12, 2025
41f8f75
Set instantReportVisibility to default assignment instant_report_visi…
LianaHarris360 Feb 12, 2025
39a71e1
Update exam instant_report_visibility field with null=True and defaul…
LianaHarris360 Feb 20, 2025
d026a02
Display the report visibility timestamp as exam archive date if insta…
LianaHarris360 Feb 20, 2025
c4fca87
Add updated migration field operations for exams
LianaHarris360 Feb 21, 2025
995180c
Return true for null instant_report_visibility values
LianaHarris360 Feb 24, 2025
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
1 change: 1 addition & 0 deletions kolibri/core/exams/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class ExamViewset(ValuesViewset):
"creator",
"data_model_version",
"learners_see_fixed_order",
"instant_report_visibility",
Copy link
Member

Choose a reason for hiding this comment

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

The only thing I can think of here, is that we should probably return True for this if the backend value is null - that will save the frontend from having to worry about the nullability of the field, and leave it entirely as a backend concern (as we only really introduced it for the purposes of migration).

"date_created",
)

Expand Down
23 changes: 23 additions & 0 deletions kolibri/core/exams/migrations/0010_add_exam_report_visibility_field.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django 3.2.25 on 2025-02-07 15:17
from django.db import migrations
from django.db import models


class Migration(migrations.Migration):

dependencies = [
("exams", "0009_alter_exam_date_created"),
]

operations = [
migrations.AddField(
model_name="draftexam",
name="instant_report_visibility",
field=models.BooleanField(default=True),
),
migrations.AddField(
model_name="exam",
name="instant_report_visibility",
field=models.BooleanField(default=True),
),
]
5 changes: 5 additions & 0 deletions kolibri/core/exams/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ class Meta:
"""
data_model_version = models.SmallIntegerField(default=3)

# If True, learners have instant access to exam reports after submission.
# Otherwise, reports are visible only after the coach ends the exam.
instant_report_visibility = models.BooleanField(default=True)
Copy link
Member

Choose a reason for hiding this comment

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

I think we may want to be careful about exactly how we add this field - my specific thought is in relation to the migration this will produce on KDP.

When we add a Django model field with a default value, this can result in a very long running migration, if the table is large.

The way to avoid this is to allow the field to be nullable null=True - and handle the null values, and return them as True via the API.

We create the migration with null=True and no default. Then we add the default=True and create another migration. At this point, we can merge the two migration files into one, just as long as the AddField operations are first, and then the AlterField to add the default comes after.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is a good point. It would be simpler and safer to allow the field to be null and return a True value for it. I will update the field and do the migration adjustments.


def __str__(self):
return self.title

Expand Down Expand Up @@ -209,6 +213,7 @@ def to_exam(self):
collection=self.collection,
creator=self.creator,
data_model_version=self.data_model_version,
instant_report_visibility=self.instant_report_visibility,
date_created=self.date_created,
)
return exam
Expand Down
5 changes: 5 additions & 0 deletions kolibri/core/exams/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class Meta:
"archive",
"assignments",
"learners_see_fixed_order",
"instant_report_visibility",
"learner_ids",
"draft",
)
Expand Down Expand Up @@ -270,6 +271,10 @@ def update(self, instance, validated_data): # noqa
instance.learners_see_fixed_order = validated_data.pop(
"learners_see_fixed_order", instance.learners_see_fixed_order
)
instance.instant_report_visibility = validated_data.pop(
"instant_report_visibility",
instance.instant_report_visibility,
)
if not instance_is_draft:
# Update the non-draft specific fields
instance.active = validated_data.pop("active", instance.active)
Expand Down
12 changes: 12 additions & 0 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 @@ -84,6 +84,7 @@ def setUpTestData(cls):
}
],
"learners_see_fixed_order": False,
"instant_report_visibility": True,
}
],
)
Expand All @@ -98,6 +99,7 @@ def make_basic_exam(self):
"draft": self.draft,
"collection": self.classroom.id,
"learners_see_fixed_order": False,
"instant_report_visibility": True,
"question_sources": sections,
"assignments": [],
}
Expand Down Expand Up @@ -421,6 +423,7 @@ def test_retrieve_exam(self):
"creator",
"data_model_version",
"learners_see_fixed_order",
"instant_report_visibility",
"date_created",
]:
self.assertIn(field, response.data)
Expand All @@ -433,6 +436,7 @@ def test_post_exam_v2_model_fails(self):
"active": True,
"collection": self.classroom.id,
"learners_see_fixed_order": False,
"instant_report_visibility": True,
"question_sources": [],
"assignments": [],
"date_activated": None,
Expand Down Expand Up @@ -503,6 +507,14 @@ def test_admin_can_update_learner_sees_fixed_order(self):
self.assertEqual(response.status_code, 200)
self.assertExamExists(id=self.exam.id, learners_see_fixed_order=True)

def test_admin_can_update_instant_report_visibility(self):
self.login_as_admin()
response = self.patch_updated_exam(
self.exam.id, {"instant_report_visibility": False}
)
self.assertEqual(response.status_code, 200)
self.assertExamExists(id=self.exam.id, instant_report_visibility=False)


class ExamAPITestCase(BaseExamTest, APITestCase):
class_object = models.Exam
Expand Down
5 changes: 5 additions & 0 deletions kolibri/plugins/coach/assets/src/composables/quizCreationSpecs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,9 @@ export const Quiz = {
type: Boolean,
default: true,
},
// Default to quiz reports being visible immediately after learner submits quiz
instant_report_visibility: {
type: Boolean,
default: true,
},
};
1 change: 1 addition & 0 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 @@ -26,6 +26,7 @@ const fieldsToSave = [
'learner_ids',
'collection',
'learners_see_fixed_order',
'instant_report_visibility',
'draft',
'active',
'archive',
Expand Down
1 change: 1 addition & 0 deletions kolibri/plugins/coach/assets/src/modules/examShared/exams.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export function examState(exam) {
questionSources: exam.question_sources,
assignments: exam.assignments,
learnersSeeFixedOrder: exam.learners_see_fixed_order,
instantReportVisibility: exam.instant_report_visibility,
dataModelVersion: exam.data_model_version,
seed: exam.seed,
};
Expand Down
27 changes: 27 additions & 0 deletions kolibri/plugins/coach/assets/src/views/common/QuizStatus.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,28 @@
</KGridItem>
</div>

<!-- Report Visibility -->
<div
v-if="!exam.archive"
class="status-item"
>
<KGridItem
class="status-label"
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Label"
>
<span>{{ coachString('reportVisibilityLabel') }}</span>
</KGridItem>
<KGridItem
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Value"
>
<span>{{ reportVisibilityStatus }}</span>
</KGridItem>
</div>

<!-- Class name -->
<div class="status-item">
<KGridItem
Expand Down Expand Up @@ -394,6 +416,11 @@
return null;
}
},
reportVisibilityStatus() {
return this.exam.instant_report_visibility
? this.coachString('afterLearnerSubmitsQuizLabel')
: this.coachString('afterCoachEndsQuizLabel');
},
layout12Label() {
return { span: this.$isPrint ? 3 : 12 };
},
Expand Down
96 changes: 89 additions & 7 deletions kolibri/plugins/coach/assets/src/views/common/assignments/AssignmentDetailsModal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@

<KGridItem
:layout4="{ span: 3 }"
:layout8="{ span: 7 }"
:layout12="{ span: 11 }"
:layout8="{ span: assignmentIsQuiz ? 3 : 7 }"
:layout12="{ span: assignmentIsQuiz ? 5 : 11 }"
>
<KTextbox
ref="titleField"
Expand All @@ -47,11 +47,37 @@
@keydown.enter="submitData"
/>
</KGridItem>
<KGridItem
:layout4="{ span: 1 }"
:layout8="{ span: 1 }"
:layout12="{ span: 1 }"
/>
<template v-if="assignmentIsQuiz">
<KGridItem
:layout4="{ span: 1 }"
:layout8="{ span: 1, alignment: 'left' }"
:layout12="{ span: 1, alignment: 'left' }"
>
<KIcon
icon="circleCheckmark"
:class="windowIsSmall ? 'style-icon' : 'checkmark-style-icon'"
/>
</KGridItem>
<KGridItem
:layout4="{ span: 3 }"
:layout8="{ span: 3 }"
:layout12="{ span: 5 }"
>
<KSelect
:label="reportVisibilityLabel$()"
:options="reportVisibilityOptions"
:value="reportVisibilityValue"
:help="
instantReportVisibility
? afterLearnerSubmitsQuizDescription$()
: afterCoachEndsQuizDescription$()
"
:style="windowIsSmall ? 'margin-left: -1em' : 'margin-left: -3em'"
class="visibility-score-select"
@change="option => (instantReportVisibility = option.value)"
/>
</KGridItem>
</template>
<KGridItem
:layout4="{ span: 3 }"
:layout8="{ span: 7 }"
Expand Down Expand Up @@ -132,6 +158,7 @@
import UiAlert from 'kolibri-design-system/lib/keen/UiAlert';
import BottomAppBar from 'kolibri/components/BottomAppBar';
import commonCoreStrings from 'kolibri/uiText/commonCoreStrings';
import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow';
import { coachStrings } from '../../common/commonCoachStrings';
import RecipientSelector from './RecipientSelector';
import SidePanelRecipientsSelector from './SidePanelRecipientsSelector';
Expand All @@ -146,6 +173,7 @@
},
mixins: [commonCoreStrings],
setup() {
const { windowIsSmall } = useKResponsiveWindow();
const {
recipientsLabel$,
descriptionLabel$,
Expand All @@ -154,15 +182,26 @@
saveQuizError$,
quizDuplicateTitleError$,
lessonDuplicateTitleError$,
reportVisibilityLabel$,
afterLearnerSubmitsQuizLabel$,
afterCoachEndsQuizLabel$,
afterLearnerSubmitsQuizDescription$,
afterCoachEndsQuizDescription$,
} = coachStrings;
return {
windowIsSmall,
recipientsLabel$,
descriptionLabel$,
titleLabel$,
saveLessonError$,
saveQuizError$,
quizDuplicateTitleError$,
lessonDuplicateTitleError$,
reportVisibilityLabel$,
afterLearnerSubmitsQuizLabel$,
afterCoachEndsQuizLabel$,
afterLearnerSubmitsQuizDescription$,
afterCoachEndsQuizDescription$,
};
},
props: {
Expand Down Expand Up @@ -217,6 +256,7 @@
formIsSubmitted: false,
showServerError: false,
showTitleError: false,
instantReportVisibility: true,
Copy link
Member

Choose a reason for hiding this comment

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

Should this be dynamic rather than hard-set? For example, when I open an exam for editing, should this be set to the value previously saved to the exam?

Copy link
Member

Choose a reason for hiding this comment

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

Oi, just realized that you've set it in mounted - curious if you tried assigning it to instantReportVisibility: this.assignment.instant_report_visibility || true; here rather than in mounted below?

Copy link
Member Author

@LianaHarris360 LianaHarris360 Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, the only thing is if this.assignement.instant_report_visibility is false, true would still be returned. But I could do:
instantReportVisibility: this.assignment.instant_report_visibility ?? true
Since doing so would remove the need to set it in the mounted hook, I will update the PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've just read that the Nullish coalescing operator (??) is mainly supported on newer browsers so I've decided to set it as instantReportVisibility: this.assignment.instant_report_visibility !== false, because false !== false evaluates to false, and for all other values, it is set to true.

Copy link
Member Author

Choose a reason for hiding this comment

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

Update: I went with setting instantReportVisibility: this.assignment.instant_report_visibility, since that is what previously happened in the mounted hook anyway.

Setting it as: instantReportVisibility: this.assignment.instant_report_visibility !== false, caused lessons to also contain a instant_report_visibility which broke the tests, and only quizzes should have this field.

This field is set to true by default in kolibri/plugins/coach/assets/src/composables/quizCreationSpecs.js, so when a new quiz is created, it has the correct setting. So it was unnecessary to set it in mounted(), thanks for pointing this out!

};
},
computed: {
Expand Down Expand Up @@ -284,8 +324,22 @@
assignments: this.selectedCollectionIds,
active: this.activeIsSelected,
learner_ids: this.adHocLearners,
instant_report_visibility: this.instantReportVisibility,
};
},
reportVisibilityOptions() {
return [
{ label: this.afterLearnerSubmitsQuizLabel$(), value: true },
{ label: this.afterCoachEndsQuizLabel$(), value: false },
];
},
reportVisibilityValue() {
return (
this.reportVisibilityOptions.find(
option => option.value === this.instantReportVisibility,
) || {}
);
},
},
watch: {
title() {
Expand All @@ -300,6 +354,9 @@
adHocLearners() {
this.$emit('update', { learner_ids: this.adHocLearners });
},
instantReportVisibility() {
this.$emit('update', { instant_report_visibility: this.instantReportVisibility });
},
submitObject() {
if (this.showServerError) {
this.$nextTick(() => {
Expand All @@ -308,6 +365,9 @@
}
},
},
mounted() {
this.instantReportVisibility = this.assignment.instant_report_visibility;
},
methods: {
submitData() {
this.showServerError = false;
Expand Down Expand Up @@ -406,13 +466,35 @@
margin-left: -1em;
}

/deep/ .ui-select-feedback {
background: #ffffff !important;
}

/deep/ .ui-select-label {
background: #f5f5f5;
border-bottom-color: #666666;
border-bottom-style: solid;
border-bottom-width: 1px;
}

.visibility-score-select {
border-bottom: 0 !important;
}

.style-icon {
width: 2em;
height: 2em;
margin-top: 0.5em;
margin-left: 1em;
}

.checkmark-style-icon {
width: 2em;
height: 2em;
margin-top: 0.5em;
margin-left: -1em;
}

fieldset {
padding: 0;
margin: 24px 0;
Expand Down
23 changes: 23 additions & 0 deletions kolibri/plugins/coach/assets/src/views/common/commonCoachStrings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,29 @@ const coachStrings = createTranslator('CommonCoachStrings', {
message: 'Ungrouped learners',
context: 'Refers to learners who are not part of a specific group.',
},
reportVisibilityLabel: {
message: 'Report visibility',
context: 'Label for the switch that controls the visibility of the quiz report to learners.',
},
afterLearnerSubmitsQuizLabel: {
message: 'After learner submits quiz',
context: 'Refers to option for learners to see their quiz report after they submit their quiz.',
},
afterCoachEndsQuizLabel: {
message: 'After coach ends the quiz',
context:
'Refers to option for learners to see their quiz report only after the coach ends the quiz.',
},
afterLearnerSubmitsQuizDescription: {
message: 'Learners see their quiz report immediately after submitting',
context:
'Description of the "After coach ends the quiz" option for quiz report visibility to learners.',
},
afterCoachEndsQuizDescription: {
message: 'Learners see their quiz report only when the coach ends the quiz',
context:
'Description of the "After learner submits quiz" option for quiz report visibility to learners.',
},

// notifications
updatedNotification: {
Expand Down
Loading
Loading