From 2d0c02f980d56f220aa467277f10b2ef744533a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Qui=C3=B1ones?= Date: Thu, 13 Apr 2023 10:30:33 -0300 Subject: [PATCH 1/6] Tasks: Remove long_running from applyexternaltags There is a new status_fn argument in the register_task decorator. And it's mandatory when long_running is True. Anyways long_running should be used for tasks that take more than 10 minutes. The status_fn argument was introduced in commit: a878c8b5 https://phabricator.endlessm.com/T34559 --- kolibri_explore_plugin/tasks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/kolibri_explore_plugin/tasks.py b/kolibri_explore_plugin/tasks.py index 43dae1018..ae6576737 100644 --- a/kolibri_explore_plugin/tasks.py +++ b/kolibri_explore_plugin/tasks.py @@ -32,7 +32,6 @@ def validate(self, data): cancellable=True, permission_classes=[CanManageContent], queue=QUEUE, - long_running=True, ) def applyexternaltags(node_id, tags=None): call_command("applyexternaltags", node_id, tags=tags) From db12ad4c35e8993a0695bbc72f93b9562335514c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Qui=C3=B1ones?= Date: Thu, 13 Apr 2023 10:45:32 -0300 Subject: [PATCH 2/6] Download backend: Update call to validate_job_data The RegisteredTask.validate_job_data() method now returns a tuple. This was changed at Kolibri commit: d4f545bc3fa https://phabricator.endlessm.com/T34559 --- kolibri_explore_plugin/collectionviews.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri_explore_plugin/collectionviews.py b/kolibri_explore_plugin/collectionviews.py index 1016c0068..2efbb97c2 100644 --- a/kolibri_explore_plugin/collectionviews.py +++ b/kolibri_explore_plugin/collectionviews.py @@ -507,7 +507,7 @@ def _enqueue_current_task(self, user): def _call_task(task, user, **params): """Create, validate and enqueue a job.""" - job = task.validate_job_data(user, params) + job, _enqueue_args = task.validate_job_data(user, params) job_id = job_storage.enqueue_job( job, queue=DEFAULT_QUEUE, priority=Priority.HIGH ) From a6ae63ccc7106ed03ba79a59fa6fdfd076413ee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Qui=C3=B1ones?= Date: Fri, 14 Apr 2023 17:19:15 -0300 Subject: [PATCH 3/6] Remove call to ContentNodeResource.fetchNextContent The method was removed. https://phabricator.endlessm.com/T34559 --- .../assets/src/modules/topicsTree/handlers.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/kolibri_explore_plugin/assets/src/modules/topicsTree/handlers.js b/kolibri_explore_plugin/assets/src/modules/topicsTree/handlers.js index faa8f768d..790dd1e3b 100644 --- a/kolibri_explore_plugin/assets/src/modules/topicsTree/handlers.js +++ b/kolibri_explore_plugin/assets/src/modules/topicsTree/handlers.js @@ -128,14 +128,10 @@ export function showTopicsChannel(store, id) { } export function showTopicsContentInLightbox(store, id) { - const promises = [ - ContentNodeResource.fetchModel({ id }), - ContentNodeResource.fetchNextContent(id), - store.dispatch('setChannelInfo'), - ]; + const promises = [ContentNodeResource.fetchModel({ id }), store.dispatch('setChannelInfo')]; const shouldResolve = samePageCheckGenerator(store); Promise.all(promises).then( - ([content, nextContent]) => { + ([content]) => { if (shouldResolve()) { const currentChannel = store.getters.getChannelObject(content.channel_id); if (!currentChannel) { @@ -143,7 +139,7 @@ export function showTopicsContentInLightbox(store, id) { return; } store.commit('topicsTree/SET_STATE', { - content: contentState(content, nextContent), + content: contentState(content), channel: currentChannel, }); From e7d107f8c7bb057ce64254c16c2498290ed77544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Qui=C3=B1ones?= Date: Tue, 18 Apr 2023 16:06:29 -0300 Subject: [PATCH 4/6] Explore plugin: Add kolibri-constants dependency This is needed for the following rebase of ContentItem. https://phabricator.endlessm.com/T34559 --- kolibri_explore_plugin/assets/package.json | 3 ++- yarn.lock | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/kolibri_explore_plugin/assets/package.json b/kolibri_explore_plugin/assets/package.json index edd0a28a4..e3b016dff 100644 --- a/kolibri_explore_plugin/assets/package.json +++ b/kolibri_explore_plugin/assets/package.json @@ -10,7 +10,8 @@ "deep-object-diff": "^1.1.0", "eos-components": "^0.1.0", "lodash": "^4.17.21", - "vue-material-design-icons": "^4.12.1" + "vue-material-design-icons": "^4.12.1", + "kolibri-constants": "0.1.42" }, "devDependencies": { "vuex-router-sync": "^5.0.0", diff --git a/yarn.lock b/yarn.lock index 7b0d0e9e0..0ade7ce94 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9640,6 +9640,11 @@ known-css-properties@^0.24.0: resolved "https://registry.yarnpkg.com/known-css-properties/-/known-css-properties-0.24.0.tgz#19aefd85003ae5698a5560d2b55135bf5432155c" integrity sha512-RTSoaUAfLvpR357vWzAz/50Q/BmHfmE6ETSWfutT0AJiw10e6CmcdYRQJlLRd95B53D0Y2aD1jSxD3V3ySF+PA== +kolibri-constants@0.1.42: + version "0.1.42" + resolved "https://registry.yarnpkg.com/kolibri-constants/-/kolibri-constants-0.1.42.tgz#2f62a8d8b8894e5cfbd47ee6564b31018818c93f" + integrity sha512-2hUxYnzUEfhLFJO9egVSwYW8/PKob9wLeDYfB74mtIzgQ4zy6huRj3574WetK9gREi+W1Jcm7HGPsfZIFzFvrA== + "kolibri-design-system@https://github.com/learningequality/kolibri-design-system#269b294bf562dd7d0e3a616aaace97bf0d3433c3": version "1.3.0" resolved "https://github.com/learningequality/kolibri-design-system#269b294bf562dd7d0e3a616aaace97bf0d3433c3" From 3c84d23a9e02d7005f6e1d5df5e1576a8542ae65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Qui=C3=B1ones?= Date: Tue, 18 Apr 2023 16:28:50 -0300 Subject: [PATCH 5/6] ContentItem: Adapt to upstream changes Rebease ContentItem with current one in Learn plugin. Also copy back the useContentNodeProgress and useProgressTracking composables https://phabricator.endlessm.com/T34559 --- .../src/composables/useContentNodeProgress.js | 12 +- .../src/composables/useProgressTracking.js | 104 ++++++++++++++---- .../assets/src/views/ContentItem.vue | 20 ++-- .../assets/src/views/ContentModal.vue | 2 +- 4 files changed, 102 insertions(+), 36 deletions(-) diff --git a/kolibri_explore_plugin/assets/src/composables/useContentNodeProgress.js b/kolibri_explore_plugin/assets/src/composables/useContentNodeProgress.js index 7cfc290f7..a208699f5 100644 --- a/kolibri_explore_plugin/assets/src/composables/useContentNodeProgress.js +++ b/kolibri_explore_plugin/assets/src/composables/useContentNodeProgress.js @@ -13,7 +13,13 @@ import { ContentNodeProgressResource } from 'kolibri.resources'; const contentNodeProgressMap = reactive({}); export function setContentNodeProgress(progress) { - set(contentNodeProgressMap, progress.content_id, progress.progress); + // Avoid setting stale progress data - assume that progress increases monotonically + if ( + !contentNodeProgressMap[progress.content_id] || + progress.progress > contentNodeProgressMap[progress.content_id] + ) { + set(contentNodeProgressMap, progress.content_id, progress.progress); + } } export default function useContentNodeProgress() { @@ -32,7 +38,7 @@ export default function useContentNodeProgress() { force: true, }).then(progressData => { const progresses = progressData ? progressData : []; - for (let progress of progresses) { + for (const progress of progresses) { setContentNodeProgress(progress); } }); @@ -53,7 +59,7 @@ export default function useContentNodeProgress() { id, }).then(progressData => { const progresses = progressData ? progressData : []; - for (let progress of progresses) { + for (const progress of progresses) { setContentNodeProgress(progress); } }); diff --git a/kolibri_explore_plugin/assets/src/composables/useProgressTracking.js b/kolibri_explore_plugin/assets/src/composables/useProgressTracking.js index 32a6f43e6..b90521ebf 100644 --- a/kolibri_explore_plugin/assets/src/composables/useProgressTracking.js +++ b/kolibri_explore_plugin/assets/src/composables/useProgressTracking.js @@ -11,6 +11,7 @@ import isNumber from 'lodash/isNumber'; import isPlainObject from 'lodash/isPlainObject'; import isUndefined from 'lodash/isUndefined'; import { diff } from 'deep-object-diff'; +import Modalities from 'kolibri-constants/Modalities'; import client from 'kolibri.client'; import logger from 'kolibri.lib.logging'; import urls from 'kolibri.urls'; @@ -61,6 +62,12 @@ const replaceBlocklist = { replace: true, }; +function clearObject(obj) { + for (const key in obj) { + delete obj[key]; + } +} + export default function useProgressTracking(store) { store = store || getCurrentInstance().proxy.$store; const complete = ref(null); @@ -145,20 +152,23 @@ export default function useProgressTracking(store) { const data = response.data; set(context, valOrNull(data.context)); set(complete, valOrNull(data.complete)); - set(progress_state, threeDecimalPlaceRoundup(valOrNull(data.progress))); + set(progress_state, valOrNull(data.progress)); set(progress_delta, 0); set(time_spent, valOrNull(data.time_spent)); set(time_spent_delta, 0); set(session_id, valOrNull(data.session_id)); + clearObject(extra_fields); Object.assign(extra_fields, data.extra_fields || {}); set(mastery_criterion, valOrNull(data.mastery_criterion)); + pastattempts.splice(0); pastattempts.push(...(data.pastattempts || [])); + clearObject(pastattemptMap); Object.assign( pastattemptMap, data.pastattempts ? fromPairs(data.pastattempts.map(a => [a.id, a])) : {} ); set(totalattempts, valOrNull(data.totalattempts)); - set(unsaved_interactions, []); + unsaved_interactions.splice(0); }); } @@ -166,12 +176,12 @@ export default function useProgressTracking(store) { * Initialize a content session for progress tracking * To be called on page load for content renderers */ - function initContentSession({ nodeId, lessonId, quizId } = {}) { + function initContentSession({ node, lessonId, quizId, repeat = false } = {}) { const data = {}; - if (!nodeId && !quizId) { - throw TypeError('Must define either nodeId or quizId'); + if (!node && !quizId) { + throw TypeError('Must define either node or quizId'); } - if ((nodeId || lessonId) && quizId) { + if ((node || lessonId) && quizId) { throw TypeError('quizId must be the only defined parameter if defined'); } let sessionStarted = false; @@ -181,16 +191,61 @@ export default function useProgressTracking(store) { data.quiz_id = quizId; } - if (nodeId) { - sessionStarted = get(context) && get(context).node_id === nodeId; - data.node_id = nodeId; + if (node) { + if (!node.id) { + throw TypeError('node must have id property'); + } + if (!node.content_id) { + throw TypeError('node must have content_id property'); + } + if (!node.channel_id) { + throw TypeError('node must have channel_id property'); + } + if (!node.kind) { + throw TypeError('node must have kind property'); + } + sessionStarted = get(context) && get(context).node_id === node.id; + data.node_id = node.id; + data.content_id = node.content_id; + data.channel_id = node.channel_id; + data.kind = node.kind; if (lessonId) { sessionStarted = sessionStarted && get(context) && get(context).lesson_id === lessonId; data.lesson_id = lessonId; } + if (node.kind === 'exercise') { + if (!node.assessmentmetadata) { + throw new TypeError('node must have assessmentmetadata property'); + } + if (!node.assessmentmetadata.mastery_model) { + throw new TypeError( + 'node must have assessmentmetadata property with mastery_model property' + ); + } + if (!isPlainObject(node.assessmentmetadata.mastery_model)) { + throw new TypeError( + 'node must have assessmentmetadata property with plain object mastery_model property' + ); + } + if (!node.assessmentmetadata.mastery_model.type) { + throw new TypeError( + 'node must have assessmentmetadata property with mastery_model property with type property' + ); + } + data.mastery_model = node.assessmentmetadata.mastery_model; + if (node.options && node.options.modality === Modalities.QUIZ) { + // The mastery model and the modalities have different + // casing, so we don't reuse it here. + data.mastery_model = { type: 'quiz' }; + } + } } - if (sessionStarted) { + if (repeat) { + data.repeat = repeat; + } + + if (sessionStarted && !repeat) { return; } @@ -207,9 +262,8 @@ export default function useProgressTracking(store) { ); Object.assign(nowSavedInteraction, interaction); pastattemptMap[nowSavedInteraction.id] = nowSavedInteraction; - set(totalattempts, get(totalattempts) + 1); } else { - for (let key in interaction) { + for (const key in interaction) { if (!blocklist[key]) { pastattemptMap[interaction.id][key] = interaction[key]; } @@ -226,12 +280,13 @@ export default function useProgressTracking(store) { data, }).then(response => { if (response.data.attempts) { - for (let attempt of response.data.attempts) { + for (const attempt of response.data.attempts) { updateAttempt(attempt); } } if (response.data.complete) { set(complete, true); + set(progress_state, 1); if (store.getters.isUserLoggedIn && !wasComplete) { store.commit('INCREMENT_TOTAL_PROGRESS', 1); } @@ -307,7 +362,7 @@ export default function useProgressTracking(store) { // If it is successful call all of the resolve functions that we have stored // from all the Promises that have been returned while this specific debounce // has been active. - for (let [resolve] of updateContentSessionResolveRejectStack) { + for (const [resolve] of updateContentSessionResolveRejectStack) { resolve(result); } // Reset the stack for resolve/reject functions, so that future invocations @@ -316,7 +371,7 @@ export default function useProgressTracking(store) { }) .catch(err => { // If there is an error call reject for all previously returned promises. - for (let [, reject] of updateContentSessionResolveRejectStack) { + for (const [, reject] of updateContentSessionResolveRejectStack) { reject(err); } // Likewise reset the stack. @@ -337,6 +392,7 @@ export default function useProgressTracking(store) { // Used to ensure state is always saved when a session closes. force = false, } = {}) { + const wasComplete = get(progress_state) >= 1; if (get(session_id) === null) { throw ReferenceError(noSessionErrorText); } @@ -350,8 +406,9 @@ export default function useProgressTracking(store) { progress = _zeroToOne(progress); progress = threeDecimalPlaceRoundup(progress); if (get(progress_state) < progress) { - const newProgressDelta = - get(progress_delta) + threeDecimalPlaceRoundup(progress - get(progress_state)); + const newProgressDelta = _zeroToOne( + threeDecimalPlaceRoundup(get(progress_delta) + progress - get(progress_state)) + ); set(progress_delta, newProgressDelta); set(progress_state, progress); } @@ -362,7 +419,10 @@ export default function useProgressTracking(store) { } progressDelta = _zeroToOne(progressDelta); progressDelta = threeDecimalPlaceRoundup(progressDelta); - set(progress_delta, threeDecimalPlaceRoundup(get(progress_delta) + progressDelta)); + set( + progress_delta, + _zeroToOne(threeDecimalPlaceRoundup(get(progress_delta) + progressDelta)) + ); set( progress_state, Math.min(threeDecimalPlaceRoundup(get(progress_state) + progressDelta), 1) @@ -389,7 +449,7 @@ export default function useProgressTracking(store) { a => !a.id && a.item === interaction.item ); if (unsavedInteraction) { - for (let key in interaction) { + for (const key in interaction) { set(unsavedInteraction, key, interaction[key]); } } else { @@ -408,7 +468,8 @@ export default function useProgressTracking(store) { set(time_spent_delta, threeDecimalPlaceRoundup(get(time_spent_delta) + elapsedTime)); } - immediate = (!isUndefined(interaction) && !interaction.id) || immediate; + const completed = !wasComplete && get(progress_state) >= 1; + immediate = (!isUndefined(interaction) && !interaction.id) || completed || immediate; forceSessionUpdate = forceSessionUpdate || force; // Logic for promise returning debounce vendored and modified from: // https://github.com/sindresorhus/p-debounce/blob/main/index.js @@ -450,7 +511,7 @@ export default function useProgressTracking(store) { function stopTrackingProgress() { clearTrackingInterval(); try { - updateContentSession({ immediate: true, force: true }).catch(err => { + return updateContentSession({ immediate: true, force: true }).catch(err => { logging.debug(err); }); } catch (e) { @@ -462,6 +523,7 @@ export default function useProgressTracking(store) { throw e; } } + return Promise.resolve(); } onBeforeUnmount(stopTrackingProgress); diff --git a/kolibri_explore_plugin/assets/src/views/ContentItem.vue b/kolibri_explore_plugin/assets/src/views/ContentItem.vue index 0269a9518..d2f37f61e 100644 --- a/kolibri_explore_plugin/assets/src/views/ContentItem.vue +++ b/kolibri_explore_plugin/assets/src/views/ContentItem.vue @@ -12,7 +12,6 @@ state.core.session.full_name, }), withoutFullscreenBar() { - return GameAppIDs.includes(this.content.channel_id); - }, - contentNode() { - return this.content; + return GameAppIDs.includes(this.contentNode.channel_id); }, contentIsExercise() { return this.contentNode.kind === ContentNodeKinds.EXERCISE; @@ -149,9 +145,6 @@ totalattempts: this.totalattempts, }; }, - contentNodeId() { - return this.contentNode.id; - }, assessment() { if (this.contentNode.kind !== ContentNodeKinds.EXERCISE) { return null; @@ -162,7 +155,7 @@ }, created() { return this.initContentSession({ - nodeId: this.contentNodeId, + node: this.contentNode, }).then(() => { this.sessionReady = true; this.setWasIncomplete(); @@ -180,7 +173,7 @@ * source of truth for referencing progress of content nodes. */ cacheProgress() { - setContentNodeProgress({ content_id: this.content.content_id, progress: this.progress }); + setContentNodeProgress({ content_id: this.contentNode.id, progress: this.progress }); }, updateInteraction({ progress, interaction }) { this.updateContentSession({ @@ -207,6 +200,11 @@ @import '../styles'; + .content-renderer { + // Needs to be one less than the ScrollingHeader's z-index of 4 + z-index: 3; + } + // Fix icon offset in the Kolibri plugins: .content-renderer::v-deep .button img, .content-renderer::v-deep .button svg { diff --git a/kolibri_explore_plugin/assets/src/views/ContentModal.vue b/kolibri_explore_plugin/assets/src/views/ContentModal.vue index 514654a9b..245ee099e 100644 --- a/kolibri_explore_plugin/assets/src/views/ContentModal.vue +++ b/kolibri_explore_plugin/assets/src/views/ContentModal.vue @@ -10,7 +10,7 @@ :title="content.title" headerCloseVariant="light" > - + From d76212ddc69942419939a38a1e951ff9da60c951 Mon Sep 17 00:00:00 2001 From: Dylan McCall Date: Tue, 25 Apr 2023 18:51:26 -0700 Subject: [PATCH 6/6] Fix error if welcomeScreen/js/*map does not exist --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c5712f272..85f2fa513 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "lint-frontend": "kolibri-tools lint -p 'kolibri_explore_plugin/assets/**/*.{js,vue,scss,less,css}'", "lint-frontend:format": "yarn run lint-frontend --write", "bump-version": "./scripts/bump_version.py", - "deploy:welcome": "cp -rf packages/welcome-screen/dist kolibri_explore_plugin/welcomeScreen && rm kolibri_explore_plugin/welcomeScreen/js/*map", + "deploy:welcome": "cp -rf packages/welcome-screen/dist kolibri_explore_plugin/welcomeScreen && rm -f kolibri_explore_plugin/welcomeScreen/js/*map", "release": "./scripts/check_can_relese.sh && twine upload -s dist/*" }, "workspaces": [