Update preview plan quiz #12685

AlexVelezLl · 2024-09-27T15:34:54Z

Summary

Update quiz summary page to:

Show learners and difficult questions report
Show date created, date activated and class name in quiz status section
Update the options button, and add a preview button in the header

Note that I havent updated page back buttons actions, as we will probably reestructure page routes in the future, and dont want to mess with it yet.

References

Closes #12671.

Screenshots

Reviewer guidance

Check that current quiz summary page match the acceptance criteria described in #12671.

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

github-actions · 2024-09-27T16:05:40Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.18.0.dev0_git.20241014165401.pex
Windows Installer (EXE)	kolibri-0.18.0.dev0+git.20241014165401-windows-setup-unsigned.exe
Debian Package	kolibri_0.18.0.dev0+git.20241014165401-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.18.0.dev0+git.20241014165401.dmg
Android Package (APK)	kolibri-0.18.0.dev0+git.20241014165401-0.1.4-debug.apk
TAR file	kolibri-0.18.0.dev0+git.20241014165401.tar.gz
WHL file	kolibri-0.18.0.dev0+git.20241014165401-py2.py3-none-any.whl

marcellamaki

Hi @AlexVelezLl - this looks great and is very nearly there! I did notice one problem with viewing "not started" quizzes due to some underlying assumptions in the "item stats" functions in the handlers. I think conditionalizing it might be the simplest path forward, and I added a few more details inline.

It's okay for this to wait until after your trip to the mentor summit! It's not blocking other work that we will be working on next week.

marcellamaki · 2024-10-03T13:50:59Z

kolibri/plugins/coach/assets/src/views/common/QuizLessonDetailsHeader.vue

@@ -20,6 +20,7 @@
            />
          </h1>
          <StatusElapsedTime
+            v-if="examOrLesson !== 'exam'"


This is curiosity, not a leading question -- I can't quite figure out why this change is needed just from reading the diff. I can see that this component (the file QuizLessonDetailsHeader is used in other changed files, and that the StatusElapsedTime has also been updated. No need to add a code comment, but can you just explain to me? It's probably very straightforward but I just can't quite get there

Oh the thing is that we use this QuizLessonDetailsHeader for quizzes and lessons. And we were updating the quiz page to have the creation time as part of the QuizStatus component, so in the figma spec we have that we dont need to show the creation time here in the header anymore.

But as this header component was also being used for lessons, and we dont have yet the creation time in the LessonStatus component (which I assumed will be part of another issue to update the lessons page), I preferred to put this conditional of v-if="examOrLesson !== 'exam'" so that it is not shown in the quizzes as it is in the figma spec, but not "accidentally" remove it from the lessons yet, and rather do it on purpose once the lessons page has been refactored.

marcellamaki · 2024-10-03T13:53:01Z

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

+
+      // quizIsRandomized() {
+      //   return !this.quiz.learners_see_fixed_order;
+      // },


For this and for the "orderDescriptionString" - both have been commented out. Will we need them going forward?

Whoooppsss I forgot to remove them

marcellamaki · 2024-10-03T15:22:42Z

kolibri/plugins/coach/assets/src/routes/planExamRoutes.js

    component: QuizSummaryPage,
+    handler: generateQuestionListHandler(['quizId']),


This handler, in a very not-straightforward way, is working in some cases, but causing a 500 error when you open a quiz that has not yet been started. Another function within the handler is dispatching questionList/setItemStats, which is assuming that QuizDifficulties (aka "Difficult Questions") exists.

One way to go might be to updates the setItemsStats action, to me that seems as though it would be simpler to conditionalize that, rather that to recreate a similar action to build state for draft quizzes. It also seems to only be called in that one function (showQuestionListView), so I think there would be minimal chance of regression.

@rtibbles any thoughts here re: coach handlers/state management that I'm not thinking of?

I guess the main thing is that for a draft quiz, we will get 500s because things are missing. We should update the API endpoint that is returning a 500, to instead return a 404 when the quiz is only a draft, and then make the handler handle the 404 gracefully (or just not even try it if it already knows the quiz is a draft).

More generally, most of the reports stuff is inapplicable when a quiz is still a draft, so we may need to utilize empty states here well too, and disabling tabs when not applicable?

From the code read through, I think there are already updates in the UI for empty state when not applicable, but the data is still being fetched (or, trying to be and erroring)

The problem is also with the handler, I havent looked in deph all that it was doing, but it was fetching de exam and the resources again, but we dont need this, as we already do this in the fetchQuizSummaryPageData. So I will better remove this handler for this route and handle this within the fetchQuizSummaryPage function, and conditionally fetch the difficult questions if the exam is not a draft.

so we may need to utilize empty states here well too, and disabling tabs when not applicable?

Yes, we currently have a condition to not show the tab if the quiz is a draft: https://github.com/AlexVelezLl/kolibri/blob/d915ea27c67992f6a9c290f618bffaa004f33e52/kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/index.vue#L204

AlexVelezLl · 2024-10-03T19:22:38Z

Thank you @marcellamaki!! I have pushed the changes. At the end I fixed the 500 error in the api (just in case), and also removed the generateQuestionListHandler and handled the fetching of the difficult questions inside the quizSummaryPage api file: a006f88

marcellamaki · 2024-10-04T14:57:06Z

The code updates here make sense to me, and a quick manual testing checks out. I'm going to ask @pcenov to do some extra manual QA next week, but I think this is looking good!

nucleogenesis · 2024-10-05T00:10:22Z

kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/api.js

+    qs = [...qs, ...section.questions];
+
+    return qs;


Non-blocking: You could return [...qs, ...section.questions] here rather than assigning to the qs and returning it.

Oh yes, that would be much more simpler. Thanks!

nucleogenesis

I may be mistaken or missing something re: my comment around the ?.() but I'll consider it a blocking change until we can clarify.

nucleogenesis · 2024-10-05T00:18:17Z

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

+        });
+      },
+      exportCSV() {
+        this.$refs.table.exportCSV?.();


I'm not sure that the ?. will avoid the error here. I tried this out in the node repl. I think that this is a blocker unless it's fully expected for exportCSV to not be defined, in which case this can be rewritten to avoid the resulting errors in the console.

> x = {a: 1} { a: 1 } > x.a?.() Uncaught TypeError: x.a is not a function

Oh, no, no, the ?.() is not supposed to be a validator to see if the exportCSV is a function, but rather that it is defined, and avoid breaking if its not. exportCSV is a very specific identifier and it is a verb, so having a non-function exportCSV attribute defined within the table ref component sounds like a very edge case. We could also write a couple of lines to make the validation validation more robust if needed, and just do

if (typeof this.$refs.table.exportCSV === 'function') { this.$refs.table.exportCSV(); }

At the end, I just updated this, just in case, to make the typeof validation. 👐

Right on, thanks Alex. That makes sense and this way avoids the possible error altogether.

pcenov · 2024-10-07T11:12:34Z

LGTM as well @marcellamaki - no issues observed while manually testing other than the 'Preview' button not working and clicking the Back button which currently brings us to the Reports tab but both of these are out of scope for this PR.

AlexVelezLl · 2024-10-14T17:02:31Z

Thank you @nucleogenesis! I just pushed the changes :)

nucleogenesis

Thanks Alex, the changes LGTM!

AlexVelezLl · 2024-10-15T13:11:34Z

Thank you all!

AlexVelezLl added 3 commits September 27, 2024 08:12

Move reports into quizsummarypage

41e0ac3

Add ReportsDifficultQuestionsTable

950f81a

add export csv

a535618

github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) DEV: frontend labels Sep 27, 2024

Add info to quizStatus

35f1795

AlexVelezLl force-pushed the update-preview-plan-quiz branch from 4ce565d to 35f1795 Compare September 30, 2024 16:13

AlexVelezLl marked this pull request as ready for review September 30, 2024 16:14

AlexVelezLl added 3 commits September 30, 2024 14:53

Add made visible label

6ebe48a

Update preview button and quiz summary options

b117f6a

Update learners label

a51769c

AlexVelezLl added this to the Kolibri 0.18: Coach Resource Discovery milestone Sep 30, 2024

AlexVelezLl added the TODO: needs review Waiting for review label Sep 30, 2024

AlexVelezLl requested review from rtibbles, nucleogenesis and marcellamaki September 30, 2024 20:57

Fix print state

d915ea2

marcellamaki requested changes Oct 3, 2024

View reviewed changes

AlexVelezLl added 2 commits October 3, 2024 14:19

Remove dead code

13aa97b

Fetch difficult questions in quizsummarypage api

a006f88

github-actions bot added the DEV: backend Python, databases, networking, filesystem... label Oct 3, 2024

nucleogenesis reviewed Oct 5, 2024

View reviewed changes

nucleogenesis requested changes Oct 5, 2024

View reviewed changes

marcellamaki self-assigned this Oct 8, 2024

Requested changes

4998b30

nucleogenesis approved these changes Oct 14, 2024

View reviewed changes

AlexVelezLl merged commit b81d95d into learningequality:develop Oct 15, 2024
34 checks passed

AlexVelezLl deleted the update-preview-plan-quiz branch October 15, 2024 13:11

AlexVelezLl mentioned this pull request Nov 8, 2024

Coach main navigation refactor #12775

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update preview plan quiz #12685

Update preview plan quiz #12685

AlexVelezLl commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading

marcellamaki left a comment

marcellamaki Oct 3, 2024

AlexVelezLl Oct 3, 2024

marcellamaki Oct 3, 2024

AlexVelezLl Oct 3, 2024

marcellamaki Oct 3, 2024

rtibbles Oct 3, 2024

marcellamaki Oct 3, 2024

AlexVelezLl Oct 3, 2024 •

edited

Loading

AlexVelezLl commented Oct 3, 2024

marcellamaki commented Oct 4, 2024

nucleogenesis Oct 5, 2024

AlexVelezLl Oct 14, 2024

nucleogenesis left a comment

nucleogenesis Oct 5, 2024

AlexVelezLl Oct 14, 2024

AlexVelezLl Oct 14, 2024

nucleogenesis Oct 14, 2024

pcenov commented Oct 7, 2024

AlexVelezLl commented Oct 14, 2024

nucleogenesis left a comment

AlexVelezLl commented Oct 15, 2024

		component: QuizSummaryPage,
		handler: generateQuestionListHandler(['quizId']),

Update preview plan quiz #12685

Update preview plan quiz #12685

Conversation

AlexVelezLl commented Sep 27, 2024 • edited Loading

Summary

References

Screenshots

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

github-actions bot commented Sep 27, 2024 • edited Loading

marcellamaki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexVelezLl Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

AlexVelezLl commented Oct 3, 2024

marcellamaki commented Oct 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nucleogenesis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcenov commented Oct 7, 2024

AlexVelezLl commented Oct 14, 2024

nucleogenesis left a comment

Choose a reason for hiding this comment

AlexVelezLl commented Oct 15, 2024

AlexVelezLl commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024 •

edited

Loading

AlexVelezLl Oct 3, 2024 •

edited

Loading