Add quiz report visibility control for coaches #13064
Conversation
…d to allow coach selection of report visibility
…ction and descriptions
…r instant_report_visibility is true
LianaHarris360
requested review from
rtibbles,
nucleogenesis and
marcellamaki
github-actions
bot
added
DEV: backend
Build Artifacts
|
| @@ -217,6 +256,7 @@ | |||
| formIsSubmitted: false, | |||
| showServerError: false, | |||
| showTitleError: false, | |||
| instantReportVisibility: true, | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
|
@radinamatic @pcenov - could this go on the QA team list for tomorrow? Thank you! |
|
Hi @LianaHarris360 - I confirm that everything is implemented and working as specified above. LOD.mp4 |
|
IMO the issue @pcenov reported re: the synced data not being reflected in the Learn UI might be an issue beyond the scope of this PR and should probably be a new issue addressed separately - should be confirmed w/ @marcellamaki |
|
Similar to recent PR I tested for non-editable quizzes from previous versions, it seemed to me that this feature could also have potential issues during upgrade scenarios, so I upgraded a 0.16.2 Kolibri install with the asset from this PR.
Full home folder used in this test case. |
kolibri/core/exams/models.py
Outdated
| @@ -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) | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Thank you for adding the home folder for that use case @radinamatic I think navigating away from the plugin and back to see the report is a problem I can fix within this PR. The learner should be able to see those updates as soon as they're available, especially if they’ve just recently synced. However, I can see how this might be outside the scope of the PR, so I can open an separate issue to address this if necessary. And the report visibility timestamp that the coach sees after the ending the quiz should be updated to reflect the correct timestamp that the report was available, if it was set to “After coach ends the quiz”. I will update this pull request to fix that. |
…t=True and merge migration changes
…nt_report_visibility is false
| @@ -69,6 +69,7 @@ class ExamViewset(ValuesViewset): | |||
| "creator", | |||
| "data_model_version", | |||
| "learners_see_fixed_order", | |||
| "instant_report_visibility", | |||
There was a problem hiding this comment.
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).
Summary
This pull request implements the functionality to enable coaches to select when a quiz report is visible to learners. Coaches can now choose to allow learners to see their quiz report immediately after submitting the quiz or only after the coach ends the quiz.
The Exam model, api, and serializer have been updated to include the boolean field
instant_report_visibility, which defaults to true, indicating that learners will instantly see their quiz report once they complete a quiz. When this field is false, learners will see their quiz reports after the quiz is closed. A new migration file has been created to include the new field to the exam model.When creating a new quiz, the icon
circleCheckmarkbeside the Report Visibility selection button will be available in KDS V5.0.0Creating a new quiz:
CreateNewQuiz.mov
Learner quiz report unavailable:
CannotViewQuizReport.mov
Quiz Status page:
References
Figma (see "Quiz report Visibility" including "Learner Experience" below)
Closes #12902
Reviewer guidance
instant_report_visibility