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 preview plan quiz #12685

Merged
merged 11 commits into from
Oct 15, 2024
Merged

Update preview plan quiz #12685

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
41e0ac3
Move reports into quizsummarypage
AlexVelezLl Sep 27, 2024
950f81a
Add ReportsDifficultQuestionsTable
AlexVelezLl Sep 27, 2024
a535618
add export csv
AlexVelezLl Sep 27, 2024
35f1795
Add info to quizStatus
AlexVelezLl Sep 30, 2024
6ebe48a
Add made visible label
AlexVelezLl Sep 30, 2024
b117f6a
Update preview button and quiz summary options
AlexVelezLl Sep 30, 2024
a51769c
Update learners label
AlexVelezLl Sep 30, 2024
d915ea2
Fix print state
AlexVelezLl Sep 30, 2024
13aa97b
Remove dead code
AlexVelezLl Oct 3, 2024
a006f88
Fetch difficult questions in quizsummarypage api
AlexVelezLl Oct 3, 2024
4998b30
Requested changes
AlexVelezLl Oct 14, 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
4 changes: 3 additions & 1 deletion kolibri/plugins/coach/assets/src/routes/planExamRoutes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import ReplaceQuestions from '../views/plan/CreateExamPage/ReplaceQuestions.vue'
import CoachExamsPage from '../views/plan/CoachExamsPage';
import QuizSummaryPage from '../views/plan/QuizSummaryPage';
import SectionOrder from '../views/plan/CreateExamPage/SectionOrder';
import { generateQuestionListHandler } from '../modules/questionList/handlers';

export default [
{
Expand Down Expand Up @@ -53,8 +54,9 @@ export default [
},
{
name: QuizSummaryPage.name,
path: '/:classId/plan/quizzes/:quizId',
path: '/:classId/plan/quizzes/:quizId/:tabId?',
component: QuizSummaryPage,
handler: generateQuestionListHandler(['quizId']),
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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)

Copy link
Member Author

@AlexVelezLl AlexVelezLl Oct 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

meta: {
titleParts: ['QUIZ_NAME', 'quizzesLabel', 'CLASS_NAME'],
},
Expand Down
1 change: 1 addition & 0 deletions kolibri/plugins/coach/assets/src/views/common/QuizLessonDetailsHeader.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
/>
</h1>
<StatusElapsedTime
v-if="examOrLesson !== 'exam'"
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

v-show="!$isPrint"
:date="createdDate"
actionType="created"
Expand Down
89 changes: 61 additions & 28 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 @@ -82,16 +82,23 @@
>
<KGridItem
class="status-label"
:layout4="{ span: 4 }"
style="padding-bottom: 16px"
:layout4="{ span: 3 }"
:layout8="{ span: 4 }"
:layout12="{ span: 12 }"
:layout12="{ span: 10 }"
>
{{ $tr('reportVisibleToLearnersLabel') }}
<span> {{ $tr('reportVisibleToLearnersLabel') }} </span>
<StatusElapsedTime
v-if="exam.active"
:date="examDateOpened"
actionType="madeVisible"
style="font-weight: normal"
/>
</KGridItem>
<KGridItem
:layout4="{ span: 4 }"
:layout4="{ span: 1 }"
:layout8="{ span: 4 }"
:layout12="{ span: 12 }"
:layout12="{ span: 2 }"
>
<KSwitch
name="toggle-quiz-visibility"
Expand All @@ -104,74 +111,71 @@
</KGridItem>
</div>

<!-- Class name -->
<div
v-show="$isPrint"
class="status-item"
>
<!-- Recipients -->
<div class="status-item">
<KGridItem
class="status-label"
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Label"
>
{{ coachString('classLabel') }}
{{ coachString('recipientsLabel') }}
</KGridItem>
<KGridItem
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Value"
>
<div>
{{ className }}
<Recipients
:groupNames="groupAndAdHocLearnerNames"
:hasAssignments="exam.assignments.length > 0"
/>
</div>
</KGridItem>
</div>

<!-- Recipients -->
<!-- Average Score -->
<div class="status-item">
<KGridItem
class="status-label"
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Label"
>
{{ coachString('recipientsLabel') }}
<span>{{ coachString('avgScoreLabel') }}</span>
<AverageScoreTooltip
v-show="!$isPrint"
class="avg-score-info"
/>
</KGridItem>
<KGridItem
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Value"
>
<div>
<Recipients
:groupNames="groupAndAdHocLearnerNames"
:hasAssignments="exam.assignments.length > 0"
/>
</div>
<Score :value="avgScore" />
</KGridItem>
</div>

<!-- Average Score -->
<!-- Class name -->
<div class="status-item">
<KGridItem
class="status-label"
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Label"
>
<span>{{ coachString('avgScoreLabel') }}</span>
<AverageScoreTooltip
v-show="!$isPrint"
class="avg-score-info"
/>
{{ coachString('classLabel') }}
</KGridItem>
<KGridItem
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Value"
>
<Score :value="avgScore" />
<div>
{{ className }}
</div>
</KGridItem>
</div>

Expand Down Expand Up @@ -215,7 +219,29 @@
:layout8="{ span: 4 }"
:layout12="{ span: 12 }"
>
<p>{{ exam.size_string ? exam.size_string : '--' }}</p>
<span>{{ exam.size_string ? exam.size_string : '--' }}</span>
</KGridItem>
</div>

<!-- Date created -->
<div class="status-item">
<KGridItem
class="status-label"
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Label"
>
{{ coreString('dateCreated') }}
</KGridItem>
<KGridItem
:layout4="{ span: 4 }"
:layout8="{ span: 4 }"
:layout12="layout12Value"
>
<ElapsedTime
:date="examDateCreated"
style="margin-top: 8px"
/>
</KGridItem>
</div>
</KGrid>
Expand Down Expand Up @@ -347,6 +373,13 @@
':hover': { 'background-color': this.$darken1(this.$themePalette.red.v_1100) },
};
},
examDateCreated() {
if (this.exam.date_created) {
return new Date(this.exam.date_created);
} else {
return null;
}
},
examDateArchived() {
if (this.exam.date_archived) {
return new Date(this.exam.date_archived);
Expand Down
30 changes: 29 additions & 1 deletion kolibri/plugins/coach/assets/src/views/common/StatusElapsedTime.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
const HOUR = MINUTE * 60;
const DAY = HOUR * 24;

const ACTION_TYPES = ['created', 'closed', 'opened'];
const ACTION_TYPES = ['created', 'closed', 'opened', 'madeVisible'];

export default {
name: 'StatusElapsedTime',
Expand Down Expand Up @@ -64,6 +64,8 @@
return this.$tr('closedSecondsAgo', strParams);
case 'opened':
return this.$tr('openedSecondsAgo', strParams);
case 'madeVisible':
return this.$tr('madeVisibleSecondsAgo', strParams);
default:
return '';
}
Expand All @@ -78,6 +80,8 @@
return this.$tr('closedMinutesAgo', strParams);
case 'opened':
return this.$tr('openedMinutesAgo', strParams);
case 'madeVisible':
return this.$tr('madeVisibleMinutesAgo', strParams);
default:
return '';
}
Expand All @@ -92,6 +96,8 @@
return this.$tr('closedHoursAgo', strParams);
case 'opened':
return this.$tr('openedHoursAgo', strParams);
case 'madeVisible':
return this.$tr('madeVisibleHoursAgo', strParams);
default:
return '';
}
Expand All @@ -105,6 +111,8 @@
return this.$tr('closedDaysAgo', strParams);
case 'opened':
return this.$tr('openedDaysAgo', strParams);
case 'madeVisible':
return this.$tr('madeVisibleDaysAgo', strParams);
default:
return '';
}
Expand Down Expand Up @@ -182,6 +190,26 @@
message: 'Started {days} {days, plural, one {day} other {days}} ago',
context: 'Indicates that a quiz was started a number of days prior to the current date.',
},
madeVisibleSecondsAgo: {
message: 'Made visible {seconds} {seconds, plural, one {second} other {seconds}} ago',
context:
'Indicates that a quiz was made visible a number of seconds prior to the current time, but is always less than 1 minute ago.',
},
madeVisibleMinutesAgo: {
message: 'Made visible {minutes} {minutes, plural, one {minute} other {minutes}} ago',
context:
'Indicates that a quiz was made visible a number of minutes prior to the current time, but the time is always less than 1 hour ago.',
},
madeVisibleHoursAgo: {
message: 'Made visible {hours} {hours, plural, one {hour} other {hours}} ago',
context:
'Indicates that a quiz was made visible a number of hours prior to the current time, but the time is always less than one day ago',
},
madeVisibleDaysAgo: {
message: 'Made visible {days} {days, plural, one {day} other {days}} ago',
context:
'Indicates that a quiz was made visible a number of days prior to the current date.',
},
},
};

Expand Down
6 changes: 3 additions & 3 deletions kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/QuizOptionsDropdownMenu.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<template>

<KButton
<KIconButton
hasDropdown
icon="optionsHorizontal"
appearance="flat-button"
:text="coreString('optionsLabel')"
>
<template #menu>
<KDropdownMenu
:options="options"
@select="$emit('select', $event.value)"
/>
</template>
</KButton>
</KIconButton>

</template>

Expand Down
Loading