LearnerSummaryPage: Fix quiz/lesson report links #13037
Conversation
github-actions
bot
added
APP: Coach
Build Artifacts
|
|
Hi @nucleogenesis! while testing I found this different behavior when clicking on the quiz links. link.mp4 |
| @@ -174,7 +170,10 @@ | |||
| return this.getLearnersForExam(quiz).includes(this.learner.id); | |||
| }, | |||
| quizLink(quizId) { | |||
| return this.classRoute(PageNames.REPORTS_LEARNER_REPORT_QUIZ_PAGE_ROOT, { quizId }); | |||
| return this.classRoute(PageNames.QUIZ_LEARNER_REPORT, { quizId }); | |||
| }, | |||
There was a problem hiding this comment.
I think the route definitions for this page is missing the meta.titleParts. (https://github.com/learningequality/kolibri/blob/13027-broken-links-learner-summary-page/kolibri/plugins/coach/assets/src/composables/useCoreCoach.js#L46) which might explain the failed page title. So, you might needs to check the route configuration for those specific pages (QUIZ_LEARNER_REPORT and LESSON_LEARNER_REPORT) and ensure their meta fields have titleParts defined.
There was a problem hiding this comment.
Looks like it was caused by the appBarTitle being set specifically rather than letting it use the title parts, should be good now.
Thanks @AllanOXDi !
…ePage use route.meta.titleParts
nucleogenesis
force-pushed
the
13027-broken-links-learner-summary-page
branch
from
b64b850 to
a85c025
Compare
|
I think that maybe we need to remove the GROUP_NAME from all (or at least some) of the titleParts in the report title. This is causing the error when generating the page title - see the change in this commit where I updated the params -- basically, I'd also need to pass a Putting the group name in the title appears to be vestigial from the previous UX where a coach could end up on reports through the Groups UI. Now the coach cannot get to a report in a group-specific context and users can be in multiple groups, so it seems irrelevant to the report page itself. Thoughts @AllanOXDi @AlexVelezLl @marcellamaki ? |
|
Hmmm, I would suggest just one alternative is that we can validate if the param |
| @@ -190,7 +186,10 @@ | |||
| return this.getLearnersForExam(quiz).includes(this.learner.id); | |||
| }, | |||
| quizLink(quizId) { | |||
| return this.classRoute(PageNames.REPORTS_LEARNER_REPORT_QUIZ_PAGE_ROOT, { quizId }); | |||
| return this.classRoute(PageNames.QUIZ_LEARNER_REPORT, { quizId }); | |||
There was a problem hiding this comment.
Here this route is failing because we are not passing the questionId, interactionIndex and tryIndex params. Here it would be better to use the QUIZ_LEARNER_PAGE_ROOT page that the only thing that it does is that it redirects it to the QUIZ_LEARNER_REPORT page but defines default values for these params, so we dont need to do it here.
|
Looking good, no more broken links! 👍🏽 Maybe connected to what is discussed above, I would point out that the presentation of the lesson not visible and quiz not started are visually different, that is, not consistent: clicking on the not started quiz appears as if there are no questions in it. We should at least add a string
|
Summary
LessonSummaryPage was using the wrong page names when generating the links to the proper reports. This PR corrects them.
References
Fixes #13027
Reviewer guidance
Click the links in
Coach -> Learners -> <Specific Learner Summary>for Lessons and Quizzes, they should work.