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.

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

Count descendants not ancestors #12394

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 2 deletions kolibri/core/assets/src/api-resources/contentNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ export default new Resource({
fetchRandomCollection({ getParams: params }) {
return this.getListEndpoint('random', params);
},
fetchDescendants(ids, getParams = {}) {
return this.getListEndpoint('descendants', { ids, ...getParams });
fetchDescendantCounts(getParams) {
return this.getListEndpoint('descendant_counts', { ...getParams });
},
fetchDescendantsAssessments(ids) {
return this.getListEndpoint('descendants_assessments', { ids });
Expand Down
32 changes: 8 additions & 24 deletions kolibri/core/content/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -861,33 +861,17 @@ def random(self, request, **kwargs):
return Response(self.serialize(queryset))

@action(detail=False)
def descendants(self, request):
def descendant_counts(self, request):
"""
Returns a slim view all the descendants of a set of content nodes (as designated by the passed in ids).
In addition to id, title, kind, and content_id, each node is also annotated with the ancestor_id of one
of the ids that are passed into the request.
In the case where a node has more than one ancestor in the set of content nodes requested, duplicates of
that content node are returned, each annotated with one of the ancestor_ids for a node.
Return the number of descendants for each node in the queryset.
"""
ids = self.request.query_params.get("ids", None)
if not ids:
# Don't allow unfiltered queries
if not self.request.query_params:
return Response([])
kind = self.request.query_params.get("descendant_kind", None)
nodes = self.filter_queryset(self.get_queryset())
data = []
for node in nodes:

def copy_node(new_node):
new_node["ancestor_id"] = node.id
new_node["is_leaf"] = new_node.get("kind") != content_kinds.TOPIC
return new_node

node_data = node.get_descendants().filter(available=True)
if kind:
node_data = node_data.filter(kind=kind)
data += map(
copy_node, node_data.values("id", "title", "kind", "content_id")
)
queryset = self.filter_queryset(self.get_queryset())

data = queryset.values("id", "on_device_resources")

return Response(data)

@action(detail=False)
Expand Down
56 changes: 17 additions & 39 deletions kolibri/core/content/test/test_content_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1074,70 +1074,48 @@ def test_contentnode_descendants_assessments_exercise_parent_sum_siblings_one_un
sibling_assessment_metadata.number_of_assessments,
)

def test_contentnode_descendants_topic_siblings_ancestor_ids(self):
def test_contentnode_descendant_counts_topic_siblings_ancestor_ids(self):
root = content.ContentNode.objects.get(parent__isnull=True)
topics = content.ContentNode.objects.filter(
parent=root, kind=content_kinds.TOPIC
)
topic_ids = topics.values_list("id", flat=True)
response = self.client.get(
reverse("kolibri:core:contentnode-descendants"),
reverse("kolibri:core:contentnode-descendant-counts"),
data={"ids": ",".join(topic_ids)},
)
for datum in response.data:
topic = topics.get(id=datum["ancestor_id"])
self.assertTrue(topic.get_descendants().filter(id=datum["id"]).exists())

def test_contentnode_descendants_topic_siblings_kind_filter(self):
root = content.ContentNode.objects.get(parent__isnull=True)
topics = content.ContentNode.objects.filter(
parent=root, kind=content_kinds.TOPIC
)
topic_ids = topics.values_list("id", flat=True)
response = self.client.get(
reverse("kolibri:core:contentnode-descendants"),
data={
"ids": ",".join(topic_ids),
"descendant_kind": content_kinds.EXERCISE,
},
)
for datum in response.data:
topic = topics.get(id=datum["ancestor_id"])
self.assertTrue(
topic.get_descendants()
.filter(id=datum["id"], kind=content_kinds.EXERCISE)
.exists()
lookup = {datum["id"]: datum for datum in response.data}
for topic in topics:
self.assertEqual(
lookup[topic.id]["on_device_resources"], topic.on_device_resources
)

def test_contentnode_descendants_topic_parent_child_ancestor_ids(self):
def test_contentnode_descendant_counts_topic_parent_child_ancestor_ids(self):
root = content.ContentNode.objects.get(parent__isnull=True)
topic = content.ContentNode.objects.filter(
parent=root, kind=content_kinds.TOPIC, children__isnull=False
).first()
response = self.client.get(
reverse("kolibri:core:contentnode-descendants"),
reverse("kolibri:core:contentnode-descendant-counts"),
data={"ids": ",".join((root.id, topic.id))},
)
topic_items = [
datum for datum in response.data if datum["ancestor_id"] == topic.id
]
for node in topic.get_descendants(include_self=False).filter(available=True):
self.assertTrue(next(item for item in topic_items if item["id"] == node.id))
root_items = [
datum for datum in response.data if datum["ancestor_id"] == root.id
]
for node in root.get_descendants(include_self=False).filter(available=True):
self.assertTrue(next(item for item in root_items if item["id"] == node.id))
lookup = {datum["id"]: datum for datum in response.data}
self.assertEqual(
lookup[root.id]["on_device_resources"], root.on_device_resources
)
self.assertEqual(
lookup[topic.id]["on_device_resources"], topic.on_device_resources
)

def test_contentnode_descendants_availability(self):
def test_contentnode_descendant_counts_availability(self):
content.ContentNode.objects.all().update(available=False)
root = content.ContentNode.objects.get(parent__isnull=True)
topics = content.ContentNode.objects.filter(
parent=root, kind=content_kinds.TOPIC
)
topic_ids = topics.values_list("id", flat=True)
response = self.client.get(
reverse("kolibri:core:contentnode-descendants"),
reverse("kolibri:core:contentnode-descendant-counts"),
data={"ids": ",".join(topic_ids)},
)
self.assertEqual(len(response.data), 0)
Expand Down
102 changes: 48 additions & 54 deletions kolibri/plugins/coach/assets/src/modules/lessonResources/handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@ import chunk from 'lodash/chunk';
import { LessonsPageNames } from '../../constants/lessonsConstants';

async function showResourceSelectionPage(store, params) {
const { lessonId, contentList, pageName, bookmarksList, ancestors = [] } = params;
const {
lessonId,
contentList,
pageName,
bookmarksList,
ancestors = [],
descendantCounts = [],
} = params;
const pendingSelections = store.state.lessonSummary.workingResources || [];
const cache = store.state.lessonSummary.resourceCache || {};
const initClassInfoPromise = store.dispatch('initClassInfo', params.classId);
Expand All @@ -32,14 +39,17 @@ async function showResourceSelectionPage(store, params) {
store.commit('lessonSummary/resources/SET_BOOKMARKS_LIST', bookmarksList);
store.commit('lessonSummary/resources/SET_STATE', {
contentList: [],
ancestors: [],
ancestors,
});
store.dispatch('notLoading');

if (lessonId) {
const loadRequirements = [store.dispatch('lessonSummary/updateCurrentLesson', lessonId)];
return Promise.all(loadRequirements).then(([currentLesson]) => {
// TODO make a state mapper
const resourceIds = currentLesson.resources.map(resourceObj => resourceObj.contentnode_id);
const setResourceCachePromise = store.dispatch(
'lessonSummary/getResourceCache',
resourceIds,
);
// contains selections that were commited to server prior to opening this page
if (!pendingSelections.length) {
store.commit('lessonSummary/SET_WORKING_RESOURCES', currentLesson.resources);
Expand All @@ -49,51 +59,29 @@ async function showResourceSelectionPage(store, params) {
store.commit('lessonSummary/resources/SET_ANCESTORS', ancestors);
}

const ancestorCounts = {};
const descendantCountsObject = {};
for (const descendantCount of descendantCounts.data || descendantCounts) {
descendantCountsObject[descendantCount.id] = descendantCount.on_device_resources;
}

const resourceAncestors = store.state.lessonSummary.workingResources.map(
resource => (cache[resource.contentnode_id] || {}).ancestors || [],
);
// store ancestor ids to get their descendants later
const ancestorIds = new Set();
store.commit('lessonSummary/resources/SET_DESCENDANT_COUNTS', descendantCountsObject);

resourceAncestors.forEach(ancestorArray =>
ancestorArray.forEach(ancestor => {
ancestorIds.add(ancestor.id);
if (ancestorCounts[ancestor.id]) {
ancestorCounts[ancestor.id].count++;
} else {
ancestorCounts[ancestor.id] = {};
// total number of working/added resources
ancestorCounts[ancestor.id].count = 1;
// total number of descendants
ancestorCounts[ancestor.id].total = 0;
}
}),
);
ContentNodeResource.fetchDescendants(Array.from(ancestorIds)).then(nodes => {
nodes.data.forEach(node => {
// exclude topics from total resource calculation
if (node.kind !== ContentNodeKinds.TOPIC) {
ancestorCounts[node.ancestor_id].total++;
}
// carry pendingSelections over from other interactions in this modal
store.commit('lessonSummary/resources/SET_CONTENT_LIST', contentList);
if (params.searchResults) {
store.commit('lessonSummary/resources/SET_SEARCH_RESULTS', params.searchResults);
}
store.commit('SET_PAGE_NAME', pageName);
if (pageName === LessonsPageNames.SELECTION_SEARCH) {
store.commit('SET_TOOLBAR_ROUTE', {
name: LessonsPageNames.SELECTION_ROOT,
});
store.commit('lessonSummary/resources/SET_ANCESTOR_COUNTS', ancestorCounts);
// carry pendingSelections over from other interactions in this modal
store.commit('lessonSummary/resources/SET_CONTENT_LIST', contentList);
if (params.searchResults) {
store.commit('lessonSummary/resources/SET_SEARCH_RESULTS', params.searchResults);
}
store.commit('SET_PAGE_NAME', pageName);
if (pageName === LessonsPageNames.SELECTION_SEARCH) {
store.commit('SET_TOOLBAR_ROUTE', {
name: LessonsPageNames.SELECTION_ROOT,
});
} else {
store.commit('SET_TOOLBAR_ROUTE', {
name: LessonsPageNames.SUMMARY,
});
}
} else {
store.commit('SET_TOOLBAR_ROUTE', {
name: LessonsPageNames.SUMMARY,
});
}
return setResourceCachePromise.then(() => {
store.dispatch('notLoading');
});
});
Expand All @@ -111,13 +99,17 @@ export function showLessonResourceSelectionRootPage(store, params) {
is_leaf: false,
};
});

return showResourceSelectionPage(store, {
classId: params.classId,
lessonId: params.lessonId,
contentList: channelContentList,
pageName: LessonsPageNames.SELECTION_ROOT,
});
return ContentNodeResource.fetchDescendantCounts({ parent__isnull: true }).then(
descendantCounts => {
return showResourceSelectionPage(store, {
classId: params.classId,
lessonId: params.lessonId,
contentList: channelContentList,
pageName: LessonsPageNames.SELECTION_ROOT,
descendantCounts,
});
},
);
});
}

Expand All @@ -128,9 +120,10 @@ export function showLessonResourceSelectionTopicPage(store, params) {
const loadRequirements = [
ContentNodeResource.fetchModel({ id: topicId }),
ContentNodeResource.fetchCollection({ getParams: { parent: topicId } }),
ContentNodeResource.fetchDescendantCounts({ parent: topicId }),
];

return Promise.all(loadRequirements).then(([topicNode, childNodes]) => {
return Promise.all(loadRequirements).then(([topicNode, childNodes, descendantCounts]) => {
const topicContentList = childNodes.map(node => {
return { ...node, thumbnail: getContentNodeThumbnail(node) };
});
Expand All @@ -140,6 +133,7 @@ export function showLessonResourceSelectionTopicPage(store, params) {
lessonId: params.lessonId,
contentList: topicContentList,
pageName: LessonsPageNames.SELECTION,
descendantCounts,
ancestors: [...topicNode.ancestors, topicNode],
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as actions from './actions';
function defaultState() {
return {
bookmarksList: [],
ancestorCounts: {},
descendantCounts: {},
ancestors: [],
contentList: [],
searchResults: {
Expand Down Expand Up @@ -44,8 +44,8 @@ export default {
SET_BOOKMARKS_LIST(state, bookmarks) {
state.bookmarksList = bookmarks;
},
SET_ANCESTOR_COUNTS(state, ancestorCountsObject) {
state.ancestorCounts = ancestorCountsObject;
SET_DESCENDANT_COUNTS(state, descendantCountsObject) {
state.descendantCounts = descendantCountsObject;
},
SET_CONTENT_LIST(state, contentList) {
state.contentList = contentList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
authorizedRole="adminOrCoach"
icon="close"
:pageTitle="coachString('createLessonAction')"
:route="{ name: 'PLAN_LESSONS_ROOT' }"
:route="{ name: 'PLAN_LESSONS_ROOT', params: { classId } }"
>
<KPageContainer>
<AssignmentDetailsModal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@
computed: {
...mapState(['pageName']),
...mapState('classSummary', { classId: 'id' }),
...mapState('lessonSummary', ['currentLesson', 'workingResources']),
...mapState('lessonSummary', ['currentLesson', 'resourceCache', 'workingResources']),
...mapState('lessonSummary/resources', [
'ancestorCounts',
'descendantCounts',
'contentList',
'bookmarksList',
'searchResults',
Expand Down Expand Up @@ -485,15 +485,16 @@
},
selectionMetadata(content) {
let count = 0;
let total = 0;
if (this.ancestorCounts[content.id]) {
count = this.ancestorCounts[content.id].count;
total = this.ancestorCounts[content.id].total;
for (const wr of this.workingResources) {
const resource = this.resourceCache[wr.contentnode_id];
if (resource && resource.ancestors.find(ancestor => ancestor.id === content.id)) {
count += 1;
}
}
if (count) {
return this.$tr('selectionInformation', {
count,
total,
total: this.descendantCounts[content.id],
});
}
return '';
Expand Down