From d7ae55aabed04281888bc9298477aabff65fc408 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Fri, 26 Apr 2024 09:54:36 -0700 Subject: [PATCH 01/15] clean-up exam utils & selectQuestions & tests Also gets reports working --- kolibri/core/assets/src/exams/utils.js | 258 ++++++++---------- kolibri/core/assets/test/exams/utils.spec.js | 148 +++++----- .../assets/src/composables/useQuizCreation.js | 2 +- .../coach/assets/src/utils/selectQuestions.js | 2 +- .../test/selectRandomExamQuestions.spec.js | 6 +- 5 files changed, 194 insertions(+), 222 deletions(-) diff --git a/kolibri/core/assets/src/exams/utils.js b/kolibri/core/assets/src/exams/utils.js index de50f32f3b7..1d8cde8c1f8 100644 --- a/kolibri/core/assets/src/exams/utils.js +++ b/kolibri/core/assets/src/exams/utils.js @@ -1,4 +1,3 @@ -import every from 'lodash/every'; import uniq from 'lodash/uniq'; import { v4 as uuidv4 } from 'uuid'; import { ExamResource, ContentNodeResource } from 'kolibri.resources'; @@ -15,7 +14,7 @@ import { ExamResource, ContentNodeResource } from 'kolibri.resources'; * @returns {array} - pseudo-randomized list of question objects compatible with v1 like: * { exercise_id, question_id } */ -function convertExamQuestionSourcesV0V2(questionSources, seed, questionIds) { +function convertExamQuestionSourcesV0V1(questionSources, seed, questionIds) { // This is the original PRNG that was used and MUST BE KEPT as-is. Logic from: // https://github.com/LouisT/SeededShuffle/blob/8d71a917d2f64e18fa554dbe660c7f5e6578e13e/index.js // (For more reliable seeded shuffling in other parts of the code base, use @@ -63,16 +62,14 @@ function convertExamQuestionSourcesV0V2(questionSources, seed, questionIds) { function convertExamQuestionSourcesV1V2(questionSources) { // In case a V1 quiz already has this with the old name, rename it - if (every(questionSources, 'counterInExercise')) { - return questionSources.map(source => { - const copy = source; - copy.counter_in_exercise = copy.counterInExercise; + return annotateQuestionSourcesWithCounter( + questionSources.map(question => { + const copy = question; + copy.counter_in_exercise = copy.counter_in_exercise || copy.counterInExercise; delete copy.counterInExercise; return copy; - }); - } - - return annotateQuestionSourcesWithCounter(questionSources); + }) + ); } function annotateQuestionsWithItem(questions) { @@ -84,106 +81,105 @@ function annotateQuestionsWithItem(questions) { /* Given a V2 question_sources, return V3 structure with those questions within one new section */ /** - * @param {Array} questionSources - a V2 question_sources object - * @param {boolean} learners_see_fixed_order - whether the questions should be randomized or not + * @param {Exam} learners_see_fixed_order - whether the questions should be randomized or not * - a V2 quiz will have this value on itself, but a V3 quiz will have it * on each section, so it should be passed in here * @returns V3 formatted question_sources */ -export function convertV2toV3(questionSources, exam) { - questionSources = questionSources || []; // Default value while requiring all params - const questions = annotateQuestionsWithItem(questionSources); - return { - section_id: uuidv4(), - section_title: '', - description: '', - resource_pool: [], - questions, - learners_see_fixed_order: exam.learners_see_fixed_order, - question_count: exam.question_count, - }; -} - -export function revertV3toV2(questionSources) { - if (!questionSources.length) { - return []; - } - return questionSources[0].questions; +export function convertExamQuestionSourcesV2toV3({ question_sources, learners_see_fixed_order }) { + // In V2, question_sources are questions so we add them + // to the newly created section's `questions` property + const questions = question_sources; + return [ + { + section_id: uuidv4(), + section_title: '', + description: '', + questions, + learners_see_fixed_order, + question_count: questions.length, + }, + ]; } /** - * @param {object} exam - an exam object of any question_sources version - * @returns V3 formatted question_sources + * Fetches the content nodes for an exam and converts the exam to the latest data_model_version + * + * data_model_version 0 (V0): + * - question_sources here refer to exercise nodes that the exam drew questions from at that time + * + * data_model_version 1 (V1): + * - question_sources is changed to now refer to the questions themselves by including the + * exercise_id and question_id along with a title + * + * data_model_version 2 (V2): + * - The objects in question_sources are now annotated with a counter_in_exercise field + * + * data_model_version 3 (V3): + * - question_sources now refers to a list of sections, each with their own list of questions */ -export function convertExamQuestionSourcesToV3(exam, extraArgs = {}) { - if (exam.data_model_version !== 3) { - const V2_sources = convertExamQuestionSources(exam, extraArgs); - return [convertV2toV3(V2_sources, exam)]; - } - - return exam.question_sources; -} -/** - * @returns V2 formatted question_sources - */ -export function convertExamQuestionSources(exam, extraArgs = {}) { +export async function convertExamQuestionSources(exam) { const { data_model_version } = exam; - if (data_model_version === 0) { - // TODO contentNodes are only needed for V0 -> V2 conversion, but a request to the - // ContentNode API is made regardless of the version being converted - if (extraArgs.contentNodes === undefined) { - throw new Error( - "Missing 'contentNodes' array, which is required when converting a V0 Exam model" - ); + + return new Promise(resolve => { + if (data_model_version === 0) { + const ids = uniq(exam.question_sources.map(item => item.exercise_id)); + return ContentNodeResource.fetchCollection({ + getParams: { + ids, + no_available_filtering: true, + }, + }).then(exercises => { + const questionIds = exercises.reduce((nodeIds, node) => { + nodeIds[node.id] = node.assessmentmetadata + ? node.assessmentmetadata.assessment_item_ids + : []; + return nodeIds; + }, []); + exam.question_sources = convertExamQuestionSourcesV0V1( + exam.question_sources, + exam.seed, + questionIds + ); + resolve(exam); + }); + } else { + resolve(exam); } - if (exam.seed === undefined) { - throw new Error("Missing 'seed' integer, which is required when converting a V0 Exam model"); + }).then(exam => { + if (data_model_version <= 1) { + exam.question_sources = convertExamQuestionSourcesV1V2(exam.question_sources); } - const { contentNodes } = extraArgs; - const questionIds = {}; - contentNodes.forEach(node => { - questionIds[node.id] = node.assessmentmetadata - ? node.assessmentmetadata.assessment_item_ids - : []; - }); - return annotateQuestionsWithItem( - convertExamQuestionSourcesV0V2(exam.question_sources, exam.seed, questionIds) - ); - } - if (data_model_version === 1) { - return annotateQuestionsWithItem(convertExamQuestionSourcesV1V2(exam.question_sources)); - } - - // For backwards compatibility. If you are using V3, use the convertExamQuestionSourcesToV3 func - if (data_model_version === 3) { - return revertV3toV2(exam.question_sources); - } - - return annotateQuestionsWithItem(exam.question_sources); + if (data_model_version <= 2) { + exam.question_sources = convertExamQuestionSourcesV2toV3(exam); + } + if (data_model_version <= 3) { + // Annotate all of the questions with the `item` field + exam.question_sources = exam.question_sources.map(section => { + section.questions = annotateQuestionsWithItem(section.questions); + return section; + }); + return exam; + } + }); } -export function fetchNodeDataAndConvertExam(exam) { - const { data_model_version } = exam; - if (data_model_version >= 3) { - /* For backwards compatibility, we need to convert V3 to V2 */ - exam.question_sources = revertV3toV2(exam.question_sources); - return Promise.resolve(exam); - } - if (data_model_version == 2) { - exam.question_sources = annotateQuestionsWithItem(exam.question_sources); - return Promise.resolve(exam); - } +/** + * @returns {Promise} - resolves to an object with the exam and the exercises + */ +export async function fetchExamWithContent(exam) { + // All data_model_versions have the `exercise_id` field + const ids = uniq(exam.question_sources.map(item => item.exercise_id)); return ContentNodeResource.fetchCollection({ getParams: { - ids: uniq(exam.question_sources.map(item => item.exercise_id)), + ids, no_available_filtering: true, }, - }).then(contentNodes => { - return { - ...exam, - question_sources: convertExamQuestionSources(exam, { contentNodes }), - }; + }).then(async exercises => { + return convertExamQuestionSources(exam, exercises).then(exam => { + return { exam, exercises }; + }); }); } @@ -193,6 +189,9 @@ export function fetchNodeDataAndConvertExam(exam) { export function annotateQuestionSourcesWithCounter(questionSources) { const counterInExerciseMap = {}; return questionSources.map(source => { + if (source.counter_in_exercise) { + return source; + } const { exercise_id } = source; if (!counterInExerciseMap[exercise_id]) { counterInExerciseMap[exercise_id] = 0; @@ -206,54 +205,31 @@ export function annotateQuestionSourcesWithCounter(questionSources) { // idk the best place to place this function export function getExamReport(examId, tryIndex = 0, questionNumber = 0, interactionIndex = 0) { - return new Promise((resolve, reject) => { - const examPromise = ExamResource.fetchModel({ id: examId }); - - examPromise.then( - exam => { - const questionSources = exam.question_sources; - - let contentPromise; - - if (questionSources.length) { - contentPromise = ContentNodeResource.fetchCollection({ - getParams: { - ids: uniq(questionSources.map(item => item.exercise_id)), - no_available_filtering: true, - }, - }); - } else { - contentPromise = Promise.resolve([]); - } - - contentPromise.then( - contentNodes => { - const questions = convertExamQuestionSources(exam, { contentNodes }); - - // When all the Exercises are not available on the server - if (questions.length === 0) { - return resolve({ exam }); - } - - const exercise = contentNodes.find( - node => node.id === questions[questionNumber].exercise_id - ); - - const payload = { - exerciseContentNodes: [...contentNodes], - exam, - questions, - tryIndex: Number(tryIndex), - questionNumber: Number(questionNumber), - exercise, - interactionIndex: Number(interactionIndex), - }; - resolve(payload); - }, - error => reject(error) - ); - }, - error => reject(error) - ); + return ExamResource.fetchModel({ id: examId }).then(examData => { + return fetchExamWithContent(examData).then(({ exam, exercises }) => { + // When all the Exercises are not available on the server + if (exam.question_count === 0) { + return exam; + } + + // TODO: Reports will eventually want to have the proper section-specific data to render + // the report page - but we are not updating the report UI yet. + const questions = exam.question_sources.reduce((qs, sect) => { + qs = [...qs, ...sect.questions]; + return qs; + }, []); + + const exercise = exercises.find(node => node.id === questions[questionNumber].exercise_id); + + return { + exerciseContentNodes: [...exercises], + exam, + questions, + tryIndex: Number(tryIndex), + questionNumber: Number(questionNumber), + exercise, + interactionIndex: Number(interactionIndex), + }; + }); }); } diff --git a/kolibri/core/assets/test/exams/utils.spec.js b/kolibri/core/assets/test/exams/utils.spec.js index 58203cd58b9..57c4c1ad266 100644 --- a/kolibri/core/assets/test/exams/utils.spec.js +++ b/kolibri/core/assets/test/exams/utils.spec.js @@ -1,5 +1,6 @@ import map from 'lodash/map'; -import { convertExamQuestionSources, convertExamQuestionSourcesToV3 } from '../../src/exams/utils'; +import { ContentNodeResource } from 'kolibri.resources'; +import { convertExamQuestionSources } from '../../src/exams/utils'; // map of content IDs to lists of question IDs const QUESTION_IDS = { @@ -80,94 +81,100 @@ const contentNodes = map(QUESTION_IDS, (assessmentIds, nodeId) => { }; }); +jest.mock('kolibri.resources'); +ContentNodeResource.fetchCollection = jest.fn(() => Promise.resolve(contentNodes)); + describe('exam utils', () => { - describe('convertExamQuestionSourcesToV3 converting from any previous version to V3', () => { - // Stolen from test below to ensure we're getting expected V2 values... as expected - const exam = { - data_model_version: 1, - learners_see_fixed_order: true, - question_count: 8, - question_sources: [ + describe('convertExamQuestionSources converting from any previous version to V3', () => { + let exam; + let expectedSources; + beforeEach(() => { + // Stolen from test below to ensure we're getting expected V2 values... as expected + exam = { + data_model_version: 1, + learners_see_fixed_order: true, + question_count: 8, + question_sources: [ + { + exercise_id: 'E1', + question_id: 'Q1', + title: 'Question 1', + }, + { + exercise_id: 'E1', + question_id: 'Q2', + title: 'Question 2', + }, + { + exercise_id: 'E2', + question_id: 'Q1', + title: 'Question 1', + }, + ], + }; + expectedSources = [ { exercise_id: 'E1', question_id: 'Q1', title: 'Question 1', + counter_in_exercise: 1, + item: 'E1:Q1', }, { exercise_id: 'E1', question_id: 'Q2', title: 'Question 2', + counter_in_exercise: 2, + item: 'E1:Q2', }, { exercise_id: 'E2', question_id: 'Q1', title: 'Question 1', + counter_in_exercise: 1, + item: 'E2:Q1', }, - ], - }; - const expectedSources = [ - { - exercise_id: 'E1', - question_id: 'Q1', - title: 'Question 1', - counter_in_exercise: 1, - item: 'E1:Q1', - }, - { - exercise_id: 'E1', - question_id: 'Q2', - title: 'Question 2', - counter_in_exercise: 2, - item: 'E1:Q2', - }, - { - exercise_id: 'E2', - question_id: 'Q1', - title: 'Question 1', - counter_in_exercise: 1, - item: 'E2:Q1', - }, - ]; - it('returns an array of newly structured objects with old question sources in questions', () => { - const converted = convertExamQuestionSourcesToV3(exam); + ]; + }); + + it('returns an array of newly structured objects with old question sources in questions', async () => { + const converted = await convertExamQuestionSources(exam); // The section id is randomly generated so just test that it is there and is set on the object - expect(converted[0].section_id).toBeTruthy(); - expect(converted).toEqual([ + expect(converted.question_sources[0].section_id).toBeTruthy(); + expect(converted.question_sources).toEqual([ { - section_id: converted[0].section_id, + section_id: converted.question_sources[0].section_id, section_title: '', description: '', - resource_pool: [], questions: expectedSources.sort(), learners_see_fixed_order: true, - question_count: 8, + question_count: 3, }, ]); }); - it('sets the fixed order property to what the exam property value is', () => { + it('sets the fixed order property to what the exam property value is', async () => { exam.learners_see_fixed_order = false; - const converted = convertExamQuestionSourcesToV3(exam); - expect(converted[0].learners_see_fixed_order).toEqual(false); + const converted = await convertExamQuestionSources(exam); + expect(converted.question_sources[0].learners_see_fixed_order).toEqual(false); }); - it('always sets the question_count and learners_see_fixed_order properties to the original exam values', () => { + it('always sets the question_count and learners_see_fixed_order properties to the original exam values', async () => { exam.learners_see_fixed_order = false; - exam.question_count = 49; - const converted = convertExamQuestionSourcesToV3(exam); - expect(converted).toEqual([ + exam.question_count = 12; + const converted = await convertExamQuestionSources(exam); + expect(converted.question_sources).toEqual([ { - section_id: converted[0].section_id, + section_id: converted.question_sources[0].section_id, section_title: '', description: '', - resource_pool: [], questions: expectedSources.sort(), learners_see_fixed_order: false, - question_count: 49, + question_count: 3, // There are only 3 questions in the question_sources }, ]); }); }); describe('convertExamQuestionSources converting from V1 to V2', () => { - it('returns a question_sources array with a counter_in_exercise field', () => { + it('returns a question_sources array with a counter_in_exercise field', async () => { const exam = { data_model_version: 1, question_sources: [ @@ -188,7 +195,7 @@ describe('exam utils', () => { }, ], }; - const converted = convertExamQuestionSources(exam); + const converted = await convertExamQuestionSources(exam); const expectedOutput = [ { exercise_id: 'E1', @@ -212,9 +219,9 @@ describe('exam utils', () => { item: 'E2:Q1', }, ]; - expect(converted).toEqual(expectedOutput); + expect(converted.question_sources[0].questions).toEqual(expectedOutput); }); - it('renames counterInExercise field if it has it', () => { + it('renames counterInExercise field if it has it', async () => { const exam = { data_model_version: 1, question_sources: [ @@ -232,8 +239,8 @@ describe('exam utils', () => { }, ], }; - const converted = convertExamQuestionSources(exam); - expect(converted).toEqual([ + const converted = await convertExamQuestionSources(exam); + expect(converted.question_sources[0].questions).toEqual([ { question_id: 'Q1', exercise_id: 'E1', @@ -253,18 +260,7 @@ describe('exam utils', () => { }); describe('convertExamQuestionSources converting from V0 to V2', () => { - it('throws an error if the required "contentNodes" data is not provided', () => { - const exam = { - data_model_version: 0, - seed: 1, - question_sources: [], - }; - expect(() => { - convertExamQuestionSources(exam); - }).toThrow(); - }); - - it('should return 10 specific ordered questions from 3 exercises', () => { + it('should return 10 specific ordered questions from 3 exercises', async () => { const exam = { data_model_version: 0, seed: 423, @@ -286,7 +282,7 @@ describe('exam utils', () => { }, ], }; - const converted = convertExamQuestionSources(exam, { contentNodes }); + const converted = await convertExamQuestionSources(exam); /* The selected questions should be: @@ -365,14 +361,14 @@ describe('exam utils', () => { }, ]; - expect(converted).toEqual( + expect(converted.question_sources[0].questions).toEqual( expectedOutput.map(q => { q.item = `${q.exercise_id}:${q.question_id}`; return q; }) ); }); - it('should return 10 specific ordered questions from 1 exercise', () => { + it('should return 10 specific ordered questions from 1 exercise', async () => { const exam = { data_model_version: 0, question_sources: [ @@ -384,7 +380,7 @@ describe('exam utils', () => { ], seed: 837, }; - const converted = convertExamQuestionSources(exam, { contentNodes }); + const converted = await convertExamQuestionSources(exam); const expectedOutput = [ { counter_in_exercise: 20, @@ -447,14 +443,14 @@ describe('exam utils', () => { title: 'Count with small numbers', }, ]; - expect(converted).toEqual( + expect(converted.question_sources[0].questions).toEqual( expectedOutput.map(q => { q.item = `${q.exercise_id}:${q.question_id}`; return q; }) ); }); - it('should return 3 specific ordered questions from 3 exercises', () => { + it('should return 3 specific ordered questions from 3 exercises', async () => { const exam = { data_model_version: 0, question_sources: [ @@ -476,7 +472,7 @@ describe('exam utils', () => { ], seed: 168, }; - const converted = convertExamQuestionSources(exam, { contentNodes }); + const converted = await convertExamQuestionSources(exam); const expectedOutput = [ { counter_in_exercise: 9, @@ -497,7 +493,7 @@ describe('exam utils', () => { title: 'Find 1 more or 1 less than a number', }, ]; - expect(converted).toEqual( + expect(converted.question_sources[0].questions).toEqual( expectedOutput.map(q => { q.item = `${q.exercise_id}:${q.question_id}`; return q; diff --git a/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js b/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js index 1fae7eb924d..079c87020f8 100644 --- a/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js +++ b/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js @@ -411,7 +411,7 @@ export default function useQuizCreation() { (count, r) => count + r.assessmentmetadata.assessment_item_ids.length, 0 ); - const exerciseIds = pool.map(r => r.content_id); + const exerciseIds = pool.map(r => r.exercise_id); const exerciseTitles = pool.map(r => r.title); const questionIdArrays = pool.map(r => r.unique_question_ids); return selectQuestions( diff --git a/kolibri/plugins/coach/assets/src/utils/selectQuestions.js b/kolibri/plugins/coach/assets/src/utils/selectQuestions.js index 9781a00b60d..239377888f1 100644 --- a/kolibri/plugins/coach/assets/src/utils/selectQuestions.js +++ b/kolibri/plugins/coach/assets/src/utils/selectQuestions.js @@ -74,7 +74,7 @@ export default function selectQuestions( if (!find(output, { id: uId })) { output.push({ counter_in_exercise: questionIdArrays[ri].indexOf(uId) + 1, - exercise_id: exerciseIds[ri], + exercise_id: uId.includes(':') ? uId.split(':')[0] : uId, question_id: uId.split(':')[1], id: uId, title: exerciseTitles[ri], diff --git a/kolibri/plugins/coach/assets/test/selectRandomExamQuestions.spec.js b/kolibri/plugins/coach/assets/test/selectRandomExamQuestions.spec.js index cbcc96f3ff2..5d8f13a8302 100644 --- a/kolibri/plugins/coach/assets/test/selectRandomExamQuestions.spec.js +++ b/kolibri/plugins/coach/assets/test/selectRandomExamQuestions.spec.js @@ -5,9 +5,9 @@ jest.mock('kolibri.lib.logging'); const EXERCISES_IDS = ['A', 'B', 'C']; const EXERCISES_TITLES = ['title_x', 'title_y', 'title_z']; const QUESTION_IDS = [ - ['A1', 'A2', 'A3'], - ['B1', 'B2', 'B3', 'B4', 'B5', 'B6', 'B7', 'B8'], - ['C1', 'C2', 'C3', 'C4', 'C5', 'C6', 'C7', 'C8', 'C9'], + ['A:A1', 'A:A2', 'A:A3'], + ['B:B1', 'B:B2', 'B:B3', 'B:B4', 'B:B5', 'B:B6', 'B:B7', 'B:B8'], + ['C:C1', 'C:C2', 'C:C3', 'C:C4', 'C:C5', 'C:C6', 'C:C7', 'C:C8', 'C:C9'], ]; function countQuestions(exerciseId, questionList) { From e046244efa7bc6aa103d1ab43f88f8b7b004ccb9 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Fri, 26 Apr 2024 10:39:07 -0700 Subject: [PATCH 02/15] reports & diff questions reporting fix --- kolibri/core/assets/src/exams/utils.js | 29 ++++++++++++------- .../src/modules/questionDetail/handlers.js | 4 +-- .../src/modules/questionList/actions.js | 21 +++++++++----- .../src/views/plan/QuizSummaryPage/api.js | 28 +++++------------- .../src/views/plan/QuizSummaryPage/index.vue | 5 +++- 5 files changed, 46 insertions(+), 41 deletions(-) diff --git a/kolibri/core/assets/src/exams/utils.js b/kolibri/core/assets/src/exams/utils.js index 1d8cde8c1f8..8903472b38e 100644 --- a/kolibri/core/assets/src/exams/utils.js +++ b/kolibri/core/assets/src/exams/utils.js @@ -169,16 +169,25 @@ export async function convertExamQuestionSources(exam) { * @returns {Promise} - resolves to an object with the exam and the exercises */ export async function fetchExamWithContent(exam) { - // All data_model_versions have the `exercise_id` field - const ids = uniq(exam.question_sources.map(item => item.exercise_id)); - return ContentNodeResource.fetchCollection({ - getParams: { - ids, - no_available_filtering: true, - }, - }).then(async exercises => { - return convertExamQuestionSources(exam, exercises).then(exam => { - return { exam, exercises }; + return convertExamQuestionSources(exam).then(converted => { + exam.question_sources = converted.question_sources; + const ids = uniq( + exam.question_sources.reduce((acc, section) => { + acc = [...acc, ...section.questions.map(item => item.exercise_id)]; + return acc; + }, []) + ); + + return ContentNodeResource.fetchCollection({ + getParams: { + ids, + no_available_filtering: true, + }, + }).then(exercises => { + return { + exam, + exercises, + }; }); }); } diff --git a/kolibri/plugins/coach/assets/src/modules/questionDetail/handlers.js b/kolibri/plugins/coach/assets/src/modules/questionDetail/handlers.js index 6264204bc3a..4599e89fb15 100644 --- a/kolibri/plugins/coach/assets/src/modules/questionDetail/handlers.js +++ b/kolibri/plugins/coach/assets/src/modules/questionDetail/handlers.js @@ -1,6 +1,6 @@ import { ContentNodeResource } from 'kolibri.resources'; import store from 'kolibri.coreVue.vuex.store'; -import { fetchNodeDataAndConvertExam } from 'kolibri.utils.exams'; +import { fetchExamWithContent } from 'kolibri.utils.exams'; import { coachStrings } from '../../views/common/commonCoachStrings'; export function questionRootRedirectHandler(params, name, next) { @@ -41,7 +41,7 @@ function showQuestionDetailView(params) { // Additionally the questionId is actually the unique item value, made // of a combination of 'question_id:exercise_id' const baseExam = store.state.classSummary.examMap[quizId]; - promise = fetchNodeDataAndConvertExam(baseExam).then(exam => { + promise = fetchExamWithContent(baseExam).then(({ exam }) => { exerciseNodeId = exam.question_sources.find(source => source.item === questionId).exercise_id; return exam; }); diff --git a/kolibri/plugins/coach/assets/src/modules/questionList/actions.js b/kolibri/plugins/coach/assets/src/modules/questionList/actions.js index 461a45d5de5..6aa3738e269 100644 --- a/kolibri/plugins/coach/assets/src/modules/questionList/actions.js +++ b/kolibri/plugins/coach/assets/src/modules/questionList/actions.js @@ -2,7 +2,7 @@ import get from 'lodash/get'; import pickBy from 'lodash/pickBy'; import Modalities from 'kolibri-constants/Modalities'; import { ContentNodeResource, ExamResource } from 'kolibri.resources'; -import { fetchNodeDataAndConvertExam } from 'kolibri.utils.exams'; +import { fetchExamWithContent } from 'kolibri.utils.exams'; import { coachStrings } from '../../views/common/commonCoachStrings'; import ExerciseDifficulties from './../../apiResources/exerciseDifficulties'; import QuizDifficulties from './../../apiResources/quizDifficulties'; @@ -19,7 +19,7 @@ export function setItemStats(store, { classId, exerciseId, quizId, lessonId, gro resource = QuizDifficulties; itemPromise = ExamResource.fetchModel({ id: quizId, - }).then(fetchNodeDataAndConvertExam); + }).then(fetchExamWithContent); } else { pk = exerciseId; practiceQuiz = @@ -44,21 +44,26 @@ export function setItemStats(store, { classId, exerciseId, quizId, lessonId, gro return Promise.all([itemPromise, difficultiesPromise]).then(([item, stats]) => { if (quizId) { - store.commit('SET_STATE', { exam: item }); + const exam = item.exam; + store.commit('SET_STATE', { exam }); // If no one attempted one of the questions, it could get missed out of the list // of difficult questions, so use the exam data to fill in the blanks here. - stats = item.question_sources.map(source => { - const stat = stats.find(stat => stat.item === source.item) || { + const allQuestions = exam.question_sources.reduce((qs, section) => { + qs = [...qs, ...section.questions]; + return qs; + }, []); + stats = allQuestions.map(question => { + const stat = stats.find(stat => stat.item === question.item) || { correct: 0, total: (stats[0] || {}).total || 0, }; const title = coachStrings.$tr('nthExerciseName', { - name: source.title, - number: source.counter_in_exercise, + name: question.title, + number: question.counter_in_exercise, }); return { ...stat, - ...source, + ...question, title, }; }); diff --git a/kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/api.js b/kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/api.js index 65b5dffa253..69e97909ea6 100644 --- a/kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/api.js +++ b/kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/api.js @@ -1,28 +1,16 @@ -import map from 'lodash/map'; -import { fetchNodeDataAndConvertExam } from 'kolibri.utils.exams'; -import { ExamResource, ContentNodeResource } from 'kolibri.resources'; +import { fetchExamWithContent } from 'kolibri.utils.exams'; +import { ExamResource } from 'kolibri.resources'; export function fetchQuizSummaryPageData(examId) { - const payload = { - // To display the title, status, etc. of the Quiz - exam: {}, - // To render the exercises in QuestionListPreview > ContentRenderer - exerciseContentNodes: {}, - }; return ExamResource.fetchModel({ id: examId }) .then(exam => { - return fetchNodeDataAndConvertExam(exam); + return fetchExamWithContent(exam); }) - .then(exam => { - payload.exam = exam; - return ContentNodeResource.fetchCollection({ - getParams: { - ids: map(exam.question_sources, 'exercise_id'), - }, - }).then(contentNodes => { - payload.exerciseContentNodes = contentNodes; - return payload; - }); + .then(({ exam, exercises }) => { + return { + exerciseContentNodes: exercises, + exam, + }; }); } diff --git a/kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/index.vue b/kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/index.vue index 319f85f8d39..a9d3ed76184 100644 --- a/kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/index.vue +++ b/kolibri/plugins/coach/assets/src/views/plan/QuizSummaryPage/index.vue @@ -118,7 +118,10 @@ /* eslint-disable-next-line kolibri/vue-no-unused-vuex-properties */ ...mapState('classSummary', ['groupMap', 'learnerMap']), selectedQuestions() { - return this.quiz.question_sources; + return this.quiz.question_sources.reduce((acc, section) => { + acc = [...acc, ...section.questions]; + return acc; + }, []); }, quizIsRandomized() { return !this.quiz.learners_see_fixed_order; From 75b3fcab9a773e849266aecf509710fdfbacb333 Mon Sep 17 00:00:00 2001 From: ozer550 Date: Thu, 4 Apr 2024 17:09:18 +0530 Subject: [PATCH 03/15] fixed redirection and class assignment for quiz --- .../coach/assets/src/composables/useQuizCreation.js | 3 ++- .../coach/assets/src/views/plan/CreateExamPage/index.vue | 9 ++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js b/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js index 079c87020f8..5a507c0d5a3 100644 --- a/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js +++ b/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js @@ -264,7 +264,8 @@ export default function useQuizCreation() { * use */ function initializeQuiz(collection) { - set(_quiz, objectWithDefaults({ collection }, Quiz)); + const assignments = [collection]; + set(_quiz, objectWithDefaults({ collection, assignments }, Quiz)); const newSection = addSection(); setActiveSection(newSection.section_id); _fetchChannels(); diff --git a/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/index.vue b/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/index.vue index eb4975a4078..a1898b90f9c 100644 --- a/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/index.vue +++ b/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/index.vue @@ -27,7 +27,7 @@ @@ -116,6 +116,13 @@ this.initializeQuiz(this.$route.params.classId); this.quizInitialized = true; }, + methods: { + saveQuizAndRedirect() { + this.saveQuiz().then(() => { + this.$router.replace({ name: PageNames.EXAMS }); + }); + }, + }, $trs: { createNewExamLabel: { message: 'Create new quiz', From c320ece433d56068a00563e3ed7052b9900c76cb Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Mon, 29 Apr 2024 13:07:19 -0700 Subject: [PATCH 04/15] fix examViewer handlers to use updated exam utils --- .../assets/src/modules/examViewer/handlers.js | 100 +++++++++--------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/kolibri/plugins/learn/assets/src/modules/examViewer/handlers.js b/kolibri/plugins/learn/assets/src/modules/examViewer/handlers.js index c35dfe4ea0d..35cd4d25c01 100644 --- a/kolibri/plugins/learn/assets/src/modules/examViewer/handlers.js +++ b/kolibri/plugins/learn/assets/src/modules/examViewer/handlers.js @@ -1,7 +1,7 @@ import { ContentNodeResource, ExamResource } from 'kolibri.resources'; import uniq from 'lodash/uniq'; import samePageCheckGenerator from 'kolibri.utils.samePageCheckGenerator'; -import { convertExamQuestionSourcesToV3 } from 'kolibri.utils.exams'; +import { convertExamQuestionSources } from 'kolibri.utils.exams'; import shuffled from 'kolibri.utils.shuffled'; import { ClassesPageNames } from '../../constants'; import { LearnerClassroomResource } from '../../apiResources'; @@ -55,61 +55,63 @@ export function showExam(store, params, alreadyOnQuiz) { contentNodes => { if (shouldResolve()) { // If necessary, convert the question source info - const question_sources = convertExamQuestionSourcesToV3(exam, { contentNodes }); + convertExamQuestionSources(exam).then(converted => { + const { question_sources } = converted; - // When necessary, randomize the questions for the learner. - // Seed based on the user ID so they see a consistent order each time. - question_sources.forEach(section => { - if (!section.learners_see_fixed_order) { - section.questions = shuffled( - section.questions, - store.state.core.session.user_id - ); - } - }); + // When necessary, randomize the questions for the learner. + // Seed based on the user ID so they see a consistent order each time. + question_sources.forEach(section => { + if (!section.learners_see_fixed_order) { + section.questions = shuffled( + section.questions, + store.state.core.session.user_id + ); + } + }); - const allQuestions = question_sources.reduce((acc, section) => { - acc = [...acc, ...section.questions]; - return acc; - }, []); + const allQuestions = question_sources.reduce((acc, section) => { + acc = [...acc, ...section.questions]; + return acc; + }, []); - // Exam is drawing solely on malformed exercise data, best to quit now - if (allQuestions.some(question => !question.question_id)) { - store.dispatch( - 'handleError', - `This quiz cannot be displayed:\nQuestion sources: ${JSON.stringify( - allQuestions - )}\nExam: ${JSON.stringify(exam)}` - ); - return; - } - // Illegal question number! - else if (questionNumber >= allQuestions.length) { - store.dispatch( - 'handleError', - `Question number ${questionNumber} is not valid for this quiz` - ); - return; - } + // Exam is drawing solely on malformed exercise data, best to quit now + if (allQuestions.some(question => !question.question_id)) { + store.dispatch( + 'handleError', + `This quiz cannot be displayed:\nQuestion sources: ${JSON.stringify( + allQuestions + )}\nExam: ${JSON.stringify(exam)}` + ); + return; + } + // Illegal question number! + else if (questionNumber >= allQuestions.length) { + store.dispatch( + 'handleError', + `Question number ${questionNumber} is not valid for this quiz` + ); + return; + } - const contentNodeMap = {}; + const contentNodeMap = {}; - for (const node of contentNodes) { - contentNodeMap[node.id] = node; - } + for (const node of contentNodes) { + contentNodeMap[node.id] = node; + } - for (const question of allQuestions) { - question.missing = !contentNodeMap[question.exercise_id]; - } - exam.question_sources = question_sources; - store.commit('examViewer/SET_STATE', { - contentNodeMap, - exam, - questionNumber, - questions: allQuestions, + for (const question of allQuestions) { + question.missing = !contentNodeMap[question.exercise_id]; + } + exam.question_sources = question_sources; + store.commit('examViewer/SET_STATE', { + contentNodeMap, + exam, + questionNumber, + questions: allQuestions, + }); + store.commit('CORE_SET_PAGE_LOADING', false); + store.commit('CORE_SET_ERROR', null); }); - store.commit('CORE_SET_PAGE_LOADING', false); - store.commit('CORE_SET_ERROR', null); } }, error => { From 3d33300ae14d57addb3ba2de5fbac1345388cbae Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Wed, 1 May 2024 13:46:35 -0700 Subject: [PATCH 05/15] streamline and clarify version stepping; bump data_model_version in front-end data as it is converted --- kolibri/core/assets/src/exams/utils.js | 36 +++++++++++++++++--------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/kolibri/core/assets/src/exams/utils.js b/kolibri/core/assets/src/exams/utils.js index 8903472b38e..3e60b66f679 100644 --- a/kolibri/core/assets/src/exams/utils.js +++ b/kolibri/core/assets/src/exams/utils.js @@ -14,7 +14,7 @@ import { ExamResource, ContentNodeResource } from 'kolibri.resources'; * @returns {array} - pseudo-randomized list of question objects compatible with v1 like: * { exercise_id, question_id } */ -function convertExamQuestionSourcesV0V1(questionSources, seed, questionIds) { +function convertExamQuestionSourcesV0V2(questionSources, seed, questionIds) { // This is the original PRNG that was used and MUST BE KEPT as-is. Logic from: // https://github.com/LouisT/SeededShuffle/blob/8d71a917d2f64e18fa554dbe660c7f5e6578e13e/index.js // (For more reliable seeded shuffling in other parts of the code base, use @@ -72,6 +72,10 @@ function convertExamQuestionSourcesV1V2(questionSources) { ); } +/** + * This function applies an `item` field to each question in the array which is similar to + * where we elsewhere use `exercise_id:question_id` to uniquely identify a question. + */ function annotateQuestionsWithItem(questions) { return questions.map(question => { question.item = `${question.exercise_id}:${question.question_id}`; @@ -137,31 +141,39 @@ export async function convertExamQuestionSources(exam) { : []; return nodeIds; }, []); - exam.question_sources = convertExamQuestionSourcesV0V1( + exam.question_sources = convertExamQuestionSourcesV0V2( exam.question_sources, exam.seed, questionIds ); + // v1 -> v2 only updates the `counter_in_exercise` field if it's in camelCase + // so we can set the data_model_version to 2 here to skip that code + exam.data_model_version = 2; resolve(exam); }); } else { resolve(exam); } }).then(exam => { - if (data_model_version <= 1) { + if (data_model_version === 1) { exam.question_sources = convertExamQuestionSourcesV1V2(exam.question_sources); + exam.data_model_version = 2; } - if (data_model_version <= 2) { + + if (data_model_version === 2) { exam.question_sources = convertExamQuestionSourcesV2toV3(exam); + exam.data_model_version = 3; } - if (data_model_version <= 3) { - // Annotate all of the questions with the `item` field - exam.question_sources = exam.question_sources.map(section => { - section.questions = annotateQuestionsWithItem(section.questions); - return section; - }); - return exam; - } + + // TODO This avoids updating older code that used `item` to refer to the unique question id + // we've started calling `id` (ie, in useQuizCreation.js) but we should update this to use `id` + // everywhere and remove this line + exam.question_sources = exam.question_sources.map(section => { + section.questions = annotateQuestionsWithItem(section.questions); + return section; + }); + + return exam; }); } From 093740f4f278a947496c97eb6bdf2bf38678052a Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Wed, 1 May 2024 15:23:11 -0700 Subject: [PATCH 06/15] convertExamQuestionSources - replace promise chains with simpler async --- kolibri/core/assets/src/exams/utils.js | 85 ++++++++++++-------------- 1 file changed, 38 insertions(+), 47 deletions(-) diff --git a/kolibri/core/assets/src/exams/utils.js b/kolibri/core/assets/src/exams/utils.js index 3e60b66f679..9ad9db638d8 100644 --- a/kolibri/core/assets/src/exams/utils.js +++ b/kolibri/core/assets/src/exams/utils.js @@ -124,57 +124,48 @@ export function convertExamQuestionSourcesV2toV3({ question_sources, learners_se */ export async function convertExamQuestionSources(exam) { - const { data_model_version } = exam; - - return new Promise(resolve => { - if (data_model_version === 0) { - const ids = uniq(exam.question_sources.map(item => item.exercise_id)); - return ContentNodeResource.fetchCollection({ - getParams: { - ids, - no_available_filtering: true, - }, - }).then(exercises => { - const questionIds = exercises.reduce((nodeIds, node) => { - nodeIds[node.id] = node.assessmentmetadata - ? node.assessmentmetadata.assessment_item_ids - : []; - return nodeIds; - }, []); - exam.question_sources = convertExamQuestionSourcesV0V2( - exam.question_sources, - exam.seed, - questionIds - ); - // v1 -> v2 only updates the `counter_in_exercise` field if it's in camelCase - // so we can set the data_model_version to 2 here to skip that code - exam.data_model_version = 2; - resolve(exam); - }); - } else { - resolve(exam); - } - }).then(exam => { - if (data_model_version === 1) { - exam.question_sources = convertExamQuestionSourcesV1V2(exam.question_sources); - exam.data_model_version = 2; - } + if (exam.data_model_version === 0) { + const ids = uniq(exam.question_sources.map(item => item.exercise_id)); + const exercises = await ContentNodeResource.fetchCollection({ + getParams: { + ids, + no_available_filtering: true, + }, + }); + const questionIds = exercises.reduce((nodeIds, node) => { + nodeIds[node.id] = node.assessmentmetadata ? node.assessmentmetadata.assessment_item_ids : []; + return nodeIds; + }, []); + exam.question_sources = convertExamQuestionSourcesV0V2( + exam.question_sources, + exam.seed, + questionIds + ); + // v1 -> v2 only updates the `counter_in_exercise` field if it's in camelCase + // so we can set the data_model_version to 2 here to skip that code + exam.data_model_version = 2; + } - if (data_model_version === 2) { - exam.question_sources = convertExamQuestionSourcesV2toV3(exam); - exam.data_model_version = 3; - } + if (exam.data_model_version === 1) { + exam.question_sources = convertExamQuestionSourcesV1V2(exam.question_sources); + exam.data_model_version = 2; + } - // TODO This avoids updating older code that used `item` to refer to the unique question id - // we've started calling `id` (ie, in useQuizCreation.js) but we should update this to use `id` - // everywhere and remove this line - exam.question_sources = exam.question_sources.map(section => { - section.questions = annotateQuestionsWithItem(section.questions); - return section; - }); + if (exam.data_model_version === 2) { + exam.question_sources = convertExamQuestionSourcesV2toV3(exam); + exam.data_model_version = 3; + } - return exam; + // TODO This avoids updating older code that used `item` to refer to the unique question id + // we've started calling `id` (ie, in useQuizCreation.js) but we should update this to use `id` + // everywhere and remove this line + // Now we know we have the latest V3 structure + exam.question_sources = exam.question_sources.map(section => { + section.questions = annotateQuestionsWithItem(section.questions); + return section; }); + + return exam; } /** From 3b048034c5fcc7f660faf0731e04f9397377334a Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Wed, 1 May 2024 15:36:59 -0700 Subject: [PATCH 07/15] indicate that selectQuestion's "question ids" must be the unique composite :-separated ID --- .../coach/assets/src/utils/selectQuestions.js | 4 ++-- .../test/selectRandomExamQuestions.spec.js | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/kolibri/plugins/coach/assets/src/utils/selectQuestions.js b/kolibri/plugins/coach/assets/src/utils/selectQuestions.js index 239377888f1..caa174f3ce5 100644 --- a/kolibri/plugins/coach/assets/src/utils/selectQuestions.js +++ b/kolibri/plugins/coach/assets/src/utils/selectQuestions.js @@ -17,8 +17,8 @@ const getTotalOfQuestions = sumBy(qArray => qArray.length); * @param {Number} numQuestions - target number of questions * @param {String[]} exerciseIds - QuizExercise IDs * @param {String[]} exerciseTitle - QuizExercise titles - * @param {Array[String[]]} questionIdArrays - QuizQuestion (assessment) ID arrays corresponding - * to each exercise by index (ie, questionIdArrays[i] corresponds to exerciseIds[i]) + * @param {Array[String[]]} questionIdArrays - QuizQuestion (assessmentitem) unique IDs in the + * composite format `exercise_id:question_id` * @param {number} seed - value to seed the random shuffle with * * @return {QuizQuestion[]} diff --git a/kolibri/plugins/coach/assets/test/selectRandomExamQuestions.spec.js b/kolibri/plugins/coach/assets/test/selectRandomExamQuestions.spec.js index 5d8f13a8302..c34940b579c 100644 --- a/kolibri/plugins/coach/assets/test/selectRandomExamQuestions.spec.js +++ b/kolibri/plugins/coach/assets/test/selectRandomExamQuestions.spec.js @@ -4,7 +4,7 @@ jest.mock('kolibri.lib.logging'); const EXERCISES_IDS = ['A', 'B', 'C']; const EXERCISES_TITLES = ['title_x', 'title_y', 'title_z']; -const QUESTION_IDS = [ +const UNIQUE_QUESTION_IDS = [ ['A:A1', 'A:A2', 'A:A3'], ['B:B1', 'B:B2', 'B:B3', 'B:B4', 'B:B5', 'B:B6', 'B:B7', 'B:B8'], ['C:C1', 'C:C2', 'C:C3', 'C:C4', 'C:C5', 'C:C6', 'C:C7', 'C:C8', 'C:C9'], @@ -30,7 +30,7 @@ expect.extend({ describe('selectQuestions function', () => { it('will choose even distributions across multiple exercises', () => { const numQs = 8; - const output = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, QUESTION_IDS, 1); + const output = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, UNIQUE_QUESTION_IDS, 1); expect(output.length).toEqual(numQs); expect(countQuestions('A', output)).toBeOneOf(2, 3); expect(countQuestions('B', output)).toBeOneOf(2, 3); @@ -43,7 +43,7 @@ describe('selectQuestions function', () => { numQs, EXERCISES_IDS.slice(2), EXERCISES_TITLES.slice(2), - QUESTION_IDS.slice(2), + UNIQUE_QUESTION_IDS.slice(2), 1 ); expect(output.length).toEqual(numQs); @@ -54,7 +54,7 @@ describe('selectQuestions function', () => { it('handles a small number of questions from a few exercises', () => { const numQs = 2; - const output = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, QUESTION_IDS, 1); + const output = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, UNIQUE_QUESTION_IDS, 1); expect(output.length).toEqual(numQs); expect(countQuestions('A', output)).toBeOneOf(0, 1); expect(countQuestions('B', output)).toBeOneOf(0, 1); @@ -63,7 +63,7 @@ describe('selectQuestions function', () => { it('will handle exercises with smaller numbers of questions', () => { const numQs = 18; - const output = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, QUESTION_IDS, 1); + const output = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, UNIQUE_QUESTION_IDS, 1); expect(output.length).toEqual(numQs); expect(countQuestions('A', output)).toBe(3); expect(countQuestions('B', output)).toBeOneOf(7, 8); @@ -72,15 +72,15 @@ describe('selectQuestions function', () => { it('will choose the same questions for the same seed', () => { const numQs = 5; - const output1 = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, QUESTION_IDS, 1); - const output2 = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, QUESTION_IDS, 1); + const output1 = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, UNIQUE_QUESTION_IDS, 1); + const output2 = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, UNIQUE_QUESTION_IDS, 1); expect(output1).toEqual(output2); }); it('will choose different questions for different seeds', () => { const numQs = 5; - const output1 = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, QUESTION_IDS, 1); - const output2 = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, QUESTION_IDS, 2); + const output1 = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, UNIQUE_QUESTION_IDS, 1); + const output2 = selectQuestions(numQs, EXERCISES_IDS, EXERCISES_TITLES, UNIQUE_QUESTION_IDS, 2); expect(output1).not.toEqual(output2); }); }); From 541b91fd253cc85ff8818418e2723f75dd1305e4 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Wed, 1 May 2024 15:55:06 -0700 Subject: [PATCH 08/15] use some instead of every in v1->v2 --- kolibri/core/assets/src/exams/utils.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/kolibri/core/assets/src/exams/utils.js b/kolibri/core/assets/src/exams/utils.js index 9ad9db638d8..7d3842bf632 100644 --- a/kolibri/core/assets/src/exams/utils.js +++ b/kolibri/core/assets/src/exams/utils.js @@ -1,4 +1,5 @@ import uniq from 'lodash/uniq'; +import some from 'lodash/some'; import { v4 as uuidv4 } from 'uuid'; import { ExamResource, ContentNodeResource } from 'kolibri.resources'; @@ -61,15 +62,17 @@ function convertExamQuestionSourcesV0V2(questionSources, seed, questionIds) { } function convertExamQuestionSourcesV1V2(questionSources) { + if (some(questionSources, 'counterInExercise')) { + for (const question of questionSources) { + if (!question.counterInExercise) { + continue; + } + question.counter_in_exercise = question.counterInExercise; + delete question.counterInExercise; + } + } // In case a V1 quiz already has this with the old name, rename it - return annotateQuestionSourcesWithCounter( - questionSources.map(question => { - const copy = question; - copy.counter_in_exercise = copy.counter_in_exercise || copy.counterInExercise; - delete copy.counterInExercise; - return copy; - }) - ); + return annotateQuestionSourcesWithCounter(questionSources); } /** From 462b252ef29e3207957342907c0cee06c929e57a Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Mon, 6 May 2024 13:05:43 -0700 Subject: [PATCH 09/15] make footer background opaque, fix spacing in SectionEditor --- .../assets/src/views/plan/CreateExamPage/SectionEditor.vue | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/SectionEditor.vue b/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/SectionEditor.vue index fbff1bb24f8..bc955b4ed04 100644 --- a/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/SectionEditor.vue +++ b/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/SectionEditor.vue @@ -501,6 +501,10 @@ margin-right: 0.5em; } + .section-settings-content { + margin-bottom: 7em; + } + .section-settings-heading { margin-bottom: 0.5em; font-size: 1em; @@ -557,6 +561,7 @@ left: 0; padding: 1em; margin-top: 1em; + background-color: #ffffff; border-top: 1px solid black; > div { From c67bf2c5cfcdb7e825f20ed29ccf1ccbe4dc079b Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Mon, 6 May 2024 13:12:04 -0700 Subject: [PATCH 10/15] apply brand colors to question mark this may be better suited for an svg though --- .../views/plan/CreateExamPage/CreateQuizSection.vue | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/CreateQuizSection.vue b/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/CreateQuizSection.vue index 9f012363ede..ba463b94dff 100644 --- a/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/CreateQuizSection.vue +++ b/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/CreateQuizSection.vue @@ -137,8 +137,14 @@ style="text-align: center; padding: 0 0 1em 0; max-width: 350px; margin: 0 auto;" > -
- ? +
+ ?

@@ -669,7 +675,6 @@ margin: auto; line-height: 1.7; text-align: center; - background-color: #dbc3d4; } .help-icon-style { From 892a5f8aed3e241af83d4da020aff7f7525a4d17 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Mon, 6 May 2024 13:32:54 -0700 Subject: [PATCH 11/15] include classId param when replacing route after quiz save --- .../coach/assets/src/views/plan/CreateExamPage/index.vue | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/index.vue b/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/index.vue index a1898b90f9c..978de5bf4aa 100644 --- a/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/index.vue +++ b/kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/index.vue @@ -119,7 +119,10 @@ methods: { saveQuizAndRedirect() { this.saveQuiz().then(() => { - this.$router.replace({ name: PageNames.EXAMS }); + this.$router.replace({ + name: PageNames.EXAMS, + classId: this.$route.params.classId, + }); }); }, }, From 93bc3e19ff48b11b700765e4f1e6077b4c77761f Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Mon, 6 May 2024 13:39:42 -0700 Subject: [PATCH 12/15] update comments to reflect info about `id` vs `item` in quiz creation --- kolibri/core/assets/src/exams/utils.js | 3 --- kolibri/plugins/coach/assets/src/utils/selectQuestions.js | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/kolibri/core/assets/src/exams/utils.js b/kolibri/core/assets/src/exams/utils.js index 7d3842bf632..5b48f57dac1 100644 --- a/kolibri/core/assets/src/exams/utils.js +++ b/kolibri/core/assets/src/exams/utils.js @@ -159,9 +159,6 @@ export async function convertExamQuestionSources(exam) { exam.data_model_version = 3; } - // TODO This avoids updating older code that used `item` to refer to the unique question id - // we've started calling `id` (ie, in useQuizCreation.js) but we should update this to use `id` - // everywhere and remove this line // Now we know we have the latest V3 structure exam.question_sources = exam.question_sources.map(section => { section.questions = annotateQuestionsWithItem(section.questions); diff --git a/kolibri/plugins/coach/assets/src/utils/selectQuestions.js b/kolibri/plugins/coach/assets/src/utils/selectQuestions.js index caa174f3ce5..55741b9d159 100644 --- a/kolibri/plugins/coach/assets/src/utils/selectQuestions.js +++ b/kolibri/plugins/coach/assets/src/utils/selectQuestions.js @@ -76,6 +76,7 @@ export default function selectQuestions( counter_in_exercise: questionIdArrays[ri].indexOf(uId) + 1, exercise_id: uId.includes(':') ? uId.split(':')[0] : uId, question_id: uId.split(':')[1], + // TODO See #12127 re: replacing all `id` with `item` id: uId, title: exerciseTitles[ri], }); From 9acc56d582a5a94052d5aefa6b9d78bc10d450c8 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Wed, 8 May 2024 14:19:57 -0700 Subject: [PATCH 13/15] use contentnode id where previously used content_id in useQuizCreation --- .../plugins/coach/assets/src/composables/useQuizCreation.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js b/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js index 5a507c0d5a3..71e98f305ed 100644 --- a/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js +++ b/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js @@ -188,7 +188,7 @@ export default function useQuizCreation() { */ function selectRandomQuestionsFromResources(numQuestions, pool = [], excludedIds = []) { pool = pool.length ? pool : get(activeResourcePool); - const exerciseIds = pool.map(r => r.content_id); + const exerciseIds = pool.map(r => r.id); const exerciseTitles = pool.map(r => r.title); const questionIdArrays = pool.map(r => r.unique_question_ids); return selectQuestions( @@ -400,7 +400,7 @@ export default function useQuizCreation() { /** @type {ComputedRef} The active section's `resource_pool` */ const activeResourceMap = computed(() => get(activeResourcePool).reduce((acc, resource) => { - acc[resource.content_id] = resource; + acc[resource.id] = resource; return acc; }, {}) ); From b4dfc1d527ad5dfdd01b2ef5afaabc4291cd4ee7 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Wed, 8 May 2024 14:47:15 -0700 Subject: [PATCH 14/15] parseInt `section.question_count` when counting all the questions --- kolibri/plugins/coach/assets/src/composables/useQuizCreation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js b/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js index 71e98f305ed..c8003026eb0 100644 --- a/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js +++ b/kolibri/plugins/coach/assets/src/composables/useQuizCreation.js @@ -277,7 +277,7 @@ export default function useQuizCreation() { */ function saveQuiz() { const totalQuestions = get(allSections).reduce((acc, section) => { - acc += section.question_count; + acc += parseInt(section.question_count); return acc; }, 0); From 749f5649762cfaaf6333ec0a0347582d2ebea892 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Wed, 8 May 2024 15:22:03 -0700 Subject: [PATCH 15/15] simplify showExam handler using new exam.util function --- .../assets/src/modules/examViewer/handlers.js | 135 +++++++----------- 1 file changed, 52 insertions(+), 83 deletions(-) diff --git a/kolibri/plugins/learn/assets/src/modules/examViewer/handlers.js b/kolibri/plugins/learn/assets/src/modules/examViewer/handlers.js index 35cd4d25c01..548219a7b17 100644 --- a/kolibri/plugins/learn/assets/src/modules/examViewer/handlers.js +++ b/kolibri/plugins/learn/assets/src/modules/examViewer/handlers.js @@ -1,7 +1,6 @@ -import { ContentNodeResource, ExamResource } from 'kolibri.resources'; -import uniq from 'lodash/uniq'; +import { ExamResource } from 'kolibri.resources'; import samePageCheckGenerator from 'kolibri.utils.samePageCheckGenerator'; -import { convertExamQuestionSources } from 'kolibri.utils.exams'; +import { fetchExamWithContent } from 'kolibri.utils.exams'; import shuffled from 'kolibri.utils.shuffled'; import { ClassesPageNames } from '../../constants'; import { LearnerClassroomResource } from '../../apiResources'; @@ -29,97 +28,67 @@ export function showExam(store, params, alreadyOnQuiz) { ([classroom, exam]) => { if (shouldResolve()) { store.commit('classAssignments/SET_CURRENT_CLASSROOM', classroom); + fetchExamWithContent(exam).then(({ exam: converted, exercises: contentNodes }) => { + if (shouldResolve()) { + const { question_sources } = converted; - let contentPromise; - let allExerciseIds = []; - if (exam.data_model_version == 3) { - allExerciseIds = uniq( - exam.question_sources.reduce((acc, section) => { - acc = [...acc, ...section.questions.map(q => q.exercise_id)]; + // When necessary, randomize the questions for the learner. + // Seed based on the user ID so they see a consistent order each time. + question_sources.forEach(section => { + if (!section.learners_see_fixed_order) { + section.questions = shuffled(section.questions, store.state.core.session.user_id); + } + }); + // If necessary, convert the question source info + const allQuestions = question_sources.reduce((acc, section) => { + acc = [...acc, ...section.questions]; return acc; - }, []) - ); - } else { - allExerciseIds = exam.question_sources.map(q => q.exercise_id); - } - if (allExerciseIds.length) { - contentPromise = ContentNodeResource.fetchCollection({ - getParams: { - ids: allExerciseIds, - }, - }); - } else { - contentPromise = Promise.resolve([]); - } - contentPromise.then( - contentNodes => { - if (shouldResolve()) { - // If necessary, convert the question source info - convertExamQuestionSources(exam).then(converted => { - const { question_sources } = converted; + }, []); - // When necessary, randomize the questions for the learner. - // Seed based on the user ID so they see a consistent order each time. - question_sources.forEach(section => { - if (!section.learners_see_fixed_order) { - section.questions = shuffled( - section.questions, - store.state.core.session.user_id - ); - } - }); - - const allQuestions = question_sources.reduce((acc, section) => { - acc = [...acc, ...section.questions]; - return acc; - }, []); - - // Exam is drawing solely on malformed exercise data, best to quit now - if (allQuestions.some(question => !question.question_id)) { - store.dispatch( - 'handleError', - `This quiz cannot be displayed:\nQuestion sources: ${JSON.stringify( - allQuestions - )}\nExam: ${JSON.stringify(exam)}` - ); - return; - } - // Illegal question number! - else if (questionNumber >= allQuestions.length) { - store.dispatch( - 'handleError', - `Question number ${questionNumber} is not valid for this quiz` - ); - return; - } + // Exam is drawing solely on malformed exercise data, best to quit now + if (allQuestions.some(question => !question.question_id)) { + store.dispatch( + 'handleError', + `This quiz cannot be displayed:\nQuestion sources: ${JSON.stringify( + allQuestions + )}\nExam: ${JSON.stringify(exam)}` + ); + return; + } + // Illegal question number! + else if (questionNumber >= allQuestions.length) { + store.dispatch( + 'handleError', + `Question number ${questionNumber} is not valid for this quiz` + ); + return; + } - const contentNodeMap = {}; + const contentNodeMap = {}; - for (const node of contentNodes) { - contentNodeMap[node.id] = node; - } + for (const node of contentNodes) { + contentNodeMap[node.id] = node; + } - for (const question of allQuestions) { - question.missing = !contentNodeMap[question.exercise_id]; - } - exam.question_sources = question_sources; - store.commit('examViewer/SET_STATE', { - contentNodeMap, - exam, - questionNumber, - questions: allQuestions, - }); - store.commit('CORE_SET_PAGE_LOADING', false); - store.commit('CORE_SET_ERROR', null); - }); + for (const question of allQuestions) { + question.missing = !contentNodeMap[question.exercise_id]; } - }, + exam.question_sources = question_sources; + store.commit('examViewer/SET_STATE', { + contentNodeMap, + exam, + questionNumber, + questions: allQuestions, + }); + store.commit('CORE_SET_PAGE_LOADING', false); + store.commit('CORE_SET_ERROR', null); + } + }), error => { shouldResolve() ? store.dispatch('handleApiError', { error, reloadOnReconnect: true }) : null; - } - ); + }; } }, error => {