Skip to content

Commit

Permalink
Update lesson editing to handle missing content.
Browse files Browse the repository at this point in the history
  • Loading branch information
rtibbles committed Aug 4, 2020
1 parent c16dfff commit 0ea795a
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 153 deletions.
23 changes: 0 additions & 23 deletions kolibri/core/lessons/serializers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from collections import OrderedDict

from django.db.models import Q
from rest_framework.serializers import ListField
from rest_framework.serializers import ModelSerializer
from rest_framework.serializers import PrimaryKeyRelatedField
Expand All @@ -16,7 +15,6 @@
from kolibri.core.auth.models import FacilityUser
from kolibri.core.auth.models import Membership
from kolibri.core.auth.utils import create_adhoc_group_for_learners
from kolibri.core.content.models import ContentNode


class ResourceSerializer(Serializer):
Expand Down Expand Up @@ -89,27 +87,6 @@ def validate(self, attrs):
code=error_constants.UNIQUE,
)

def validate_resources(self, resources):
# Validates that every ContentNode passed into resources is actually installed
# on the server. NOTE that this could cause problems if content is deleted from
# device.
resource_query = Q()
for resource in resources:
resource_query |= Q(
content_id=resource["content_id"],
channel_id=resource["channel_id"],
id=resource["contentnode_id"],
)

available_resources = ContentNode.objects.filter(
resource_query, available=True
).count()
if available_resources < len(resources):
raise ValidationError(
"One or more of the selected resources is not available"
)
return resources

def to_internal_value(self, data):
data = OrderedDict(data)
data["created_by"] = self.context["request"].user.id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ function showResourceSelectionPage(store, params) {
// TODO make a state mapper
// contains selections that were commited to server prior to opening this page
if (!pendingSelections.length) {
const preselectedResources = currentLesson.resources.map(
resourceObj => resourceObj.contentnode_id
);
store.commit('lessonSummary/SET_WORKING_RESOURCES', preselectedResources);
store.commit('lessonSummary/SET_WORKING_RESOURCES', currentLesson.resources);
}

if (ancestors.length) {
Expand Down
27 changes: 8 additions & 19 deletions kolibri/plugins/coach/assets/src/modules/lessonSummary/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,25 +58,14 @@ export function getResourceCache(store, resourceIds) {
}
}

export function saveLessonResources(store, { lessonId, resourceIds }) {
return store.dispatch('getResourceCache', resourceIds).then(resourceCache => {
const resources = resourceIds.map(resourceId => {
const node = resourceCache[resourceId];
return {
contentnode_id: resourceId,
channel_id: node.channel_id,
content_id: node.content_id,
};
});

return LessonResource.saveModel({
id: lessonId,
data: { resources },
}).then(lesson => {
// Update the class summary now that there is a change to a lesson
return store.dispatch('classSummary/refreshClassSummary', null, { root: true }).then(() => {
return lesson;
});
export function saveLessonResources(store, { lessonId, resources }) {
return LessonResource.saveModel({
id: lessonId,
data: { resources },
}).then(lesson => {
// Update the class summary now that there is a change to a lesson
return store.dispatch('classSummary/refreshClassSummary', null, { root: true }).then(() => {
return lesson;
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export function setLessonSummaryState(store, params) {
const resourceIds = currentLesson.resources.map(resourceObj => resourceObj.contentnode_id);

return store.dispatch('lessonSummary/getResourceCache', resourceIds).then(() => {
store.commit('lessonSummary/SET_WORKING_RESOURCES', resourceIds);
store.commit('lessonSummary/SET_WORKING_RESOURCES', currentLesson.resources);
store.commit('lessonSummary/SET_LEARNER_GROUPS', learnerGroups);
store.commit('SET_PAGE_NAME', LessonsPageNames.SUMMARY);
});
Expand Down
28 changes: 13 additions & 15 deletions kolibri/plugins/coach/assets/src/modules/lessonSummary/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import isArray from 'lodash/isArray';
import lessonResources from '../lessonResources';
import * as actions from './actions';

Expand Down Expand Up @@ -37,21 +36,20 @@ export default {
SET_WORKING_RESOURCES(state, workingResources) {
state.workingResources = [...workingResources];
},
ADD_TO_WORKING_RESOURCES(state, ids) {
if (typeof ids === 'string') {
state.workingResources.push(ids);
} else if (isArray(ids)) {
state.workingResources = [...state.workingResources, ...ids];
}
ADD_TO_WORKING_RESOURCES(state, resources) {
state.workingResources = [
...state.workingResources,
...resources.map(r => ({
contentnode_id: r.id,
content_id: r.content_id,
channel_id: r.channel_id,
})),
];
},
REMOVE_FROM_WORKING_RESOURCES(state, ids) {
if (typeof ids === 'string') {
state.workingResources = state.workingResources.filter(resourceId => resourceId !== ids);
} else if (isArray(ids)) {
state.workingResources = state.workingResources.filter(
resourceId => !ids.includes(resourceId)
);
}
REMOVE_FROM_WORKING_RESOURCES(state, resources) {

This comment has been minimized.

Copy link
@jonboiser

jonboiser Aug 11, 2020

Contributor

@rtibbles I think the problem in #7451 might be because this change leaves out the case where resources is a string. The call in

this.removeFromWorkingResources(resourceId);

seems to be a String resourceId

This comment has been minimized.

Copy link
@rtibbles

rtibbles Aug 11, 2020

Author Member

Gotcha

state.workingResources = state.workingResources.filter(
workingResource => !resources.find(r => r.id === workingResource.contentnode_id)
);
},
ADD_TO_RESOURCE_CACHE(state, { node, channelTitle }) {
if (node && node.id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export default [
if (fromRoute.name === LessonsPageNames.SELECTION_CONTENT_PREVIEW) {
preHandlerPromise = store.dispatch('lessonSummary/saveLessonResources', {
lessonId: toRoute.params.lessonId,
resourceIds: store.state.lessonSummary.workingResources,
resources: store.state.lessonSummary.workingResources,
});
} else {
preHandlerPromise = Promise.resolve();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
:showLabel="false"
:checked="contentIsChecked(content)"
:indeterminate="contentIsIndeterminate(content)"
@change="handleCheckboxChange(content.id, $event)"
@change="handleCheckboxChange(content, $event)"
/>
<LessonContentCard
:class="{ 'with-checkbox': needCheckboxes }"
Expand Down Expand Up @@ -131,8 +131,8 @@
},
},
methods: {
handleCheckboxChange(contentId, checked) {
this.$emit('change_content_card', { contentId, checked });
handleCheckboxChange(content, checked) {
this.$emit('change_content_card', { content, checked });
},
},
$trs: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
import debounce from 'lodash/debounce';
import every from 'lodash/every';
import pickBy from 'lodash/pickBy';
import xor from 'lodash/xor';
import { ContentNodeKinds } from 'kolibri.coreVue.vuex.constants';
import BottomAppBar from 'kolibri.coreVue.components.BottomAppBar';
import commonCoreStrings from 'kolibri.coreVue.mixins.commonCoreStrings';
Expand Down Expand Up @@ -120,7 +119,6 @@
role: this.$route.query.role || null,
},
isExiting: false,
workingResourcesCopy: [...this.$store.state.lessonSummary.workingResources],
moreResultsState: null,
};
},
Expand Down Expand Up @@ -194,7 +192,8 @@
return 'visible';
},
contentIsInLesson() {
return ({ id }) => this.workingResources.includes(id);
return ({ id }) =>
Boolean(this.workingResources.find(resource => resource.contentnode_id === id));
},
addableContent() {
// Content in the topic that can be added if 'Select All' is clicked
Expand Down Expand Up @@ -240,37 +239,31 @@
},
},
beforeRouteLeave(to, from, next) {
// Only autosave if changes have been made
if (xor(this.workingResources, this.workingResourcesCopy).length > 0) {
// Block the UI and show a notification in case last save takes too long
this.isExiting = true;
const isSamePage = samePageCheckGenerator(this.$store);
setTimeout(() => {
if (isSamePage()) {
this.createSnackbar(this.$tr('saveBeforeExitSnackbarText'));
}
}, 500);
// Block the UI and show a notification in case last save takes too long
this.isExiting = true;
const isSamePage = samePageCheckGenerator(this.$store);
setTimeout(() => {
if (isSamePage()) {
this.createSnackbar(this.$tr('saveBeforeExitSnackbarText'));
}
}, 500);
// Cancel any debounced calls
this.debouncedSaveResources.cancel();
this.saveLessonResources({
lessonId: this.lessonId,
resourceIds: [...this.workingResources],
// Cancel any debounced calls
this.debouncedSaveResources.cancel();
this.saveLessonResources({
lessonId: this.lessonId,
resources: [...this.workingResources],
})
.then(() => {
this.clearSnackbar();
this.isExiting = false;
next();
})
.then(() => {
this.clearSnackbar();
this.isExiting = false;
next();
})
.catch(() => {
this.showResourcesChangedError();
this.isExiting = false;
next(false);
});
} else {
this.isExiting = false;
next();
}
.catch(() => {
this.showResourcesChangedError();
this.isExiting = false;
next(false);
});
},
methods: {
...mapActions(['createSnackbar', 'clearSnackbar']),
Expand Down Expand Up @@ -300,16 +293,16 @@
node: { ...resource },
});
});
this.addToWorkingResources(this.addableContent.map(({ id }) => id));
this.addToWorkingResources(this.addableContent);
} else {
this.removeFromSelectedResources(this.contentList.map(({ id }) => id));
this.removeFromSelectedResources(this.contentList);
}
},
addToSelectedResources(contentId) {
addToSelectedResources(content) {
this.addToResourceCache({
node: this.contentList.find(n => n.id === contentId),
node: this.contentList.find(n => n.id === content.id),
});
this.addToWorkingResources([contentId]);
this.addToWorkingResources([content]);
},
contentIsDirectoryKind({ kind }) {
return kind === ContentNodeKinds.TOPIC || kind === ContentNodeKinds.CHANNEL;
Expand Down Expand Up @@ -360,11 +353,11 @@
}
return '';
},
toggleSelected({ checked, contentId }) {
toggleSelected({ checked, content }) {
if (checked) {
this.addToSelectedResources(contentId);
this.addToSelectedResources(content);
} else {
this.removeFromSelectedResources(contentId);
this.removeFromSelectedResources(content);
}
},
handleSearchTerm(searchTerm) {
Expand Down
Loading

0 comments on commit 0ea795a

Please sign in to comment.