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

Add tests for useQuizResources and update functionality for annotation and loading states #13080

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
117 changes: 44 additions & 73 deletions kolibri/plugins/coach/assets/src/composables/useQuizResources.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,12 @@ import useFetchTree from './useFetchTree';

const logger = logging.getLogger(__filename);
const _loadingMore = ref(false);
/**
* @typedef {Object} QuizResourcesConfig
* @property { computed <string|null|undefined> } topicId - The id of the root node to fetch the
* children for
*/

/**
* @module useQuizResources
* @param {QuizResourcesConfig} config
* @param {Object} config
* @param {computed<string|null|undefined>} config.topicId - The id of the root node
* @param {boolean} [config.practiceQuiz=false]
*/
export default function useQuizResources({ topicId, practiceQuiz = false } = {}) {
const params = {
Expand All @@ -27,7 +24,6 @@ export default function useQuizResources({ topicId, practiceQuiz = false } = {})
params.contains_quiz = true;
}

// Initialize useFetchTree methods with the given topicId computed property and params
const {
topic,
fetchTree,
Expand All @@ -39,108 +35,83 @@ export default function useQuizResources({ topicId, practiceQuiz = false } = {})
params,
});

/** @type {ref<ExerciseResource[]>} All resources which have been fetched that are the children of
* the given topicId annotated with assessment metadata */
const _resources = ref([]);

/** @type {ref<Boolean>} Whether we are currently fetching/processing the child nodes */
const _loading = ref(false);

/**
* Annotates the child TOPIC nodes with the number of assessments that are contained within them
* @param {ContentNode[]} results - The array of results from a content API call
* @returns {Promise<ContentNode[]>} - A promise that resolves when the annotations have been
* made and returns the annotated results
* Annotates the child TOPIC nodes with the number of assessments
* @param {ContentNode[]} results - The array of results from content API
* @returns {Promise<ContentNode[]>} - Promise resolving to annotated results
*/
async function annotateTopicsWithDescendantCounts(results) {
const topicIds = results
.filter(({ kind }) => kind === ContentNodeKinds.TOPIC || kind === ContentNodeKinds.CHANNEL)
.map(topic => topic.id);

return ContentNodeResource.fetchDescendantsAssessments(topicIds)
.then(({ data: topicsWithAssessmentCounts }) => {
const topicsWithAssessmentCountsMap = topicsWithAssessmentCounts.reduce((acc, topic) => {
acc[topic.id] = topic.num_assessments;
return acc;
}, {});
return (
results
.map(node => {
// We'll map so that the topics are updated in place with the num_assessments, others
// are left as-is
if ([ContentNodeKinds.TOPIC, ContentNodeKinds.CHANNEL].includes(node.kind)) {
if (!topicsWithAssessmentCountsMap[node.id]) {
// If there are no assessment descendants,
// return null so we can easily filter after
return null;
}
if (node.kind === ContentNodeKinds.TOPIC && !node.children) {
// If the topic has no children, we can assume it has no assessments
// Only do this check for topics, because CHANNEL kinds are normally
// ChannelMetadata objects masquerading as ContentNode objects
// and so don't have children
return null;
}
node.num_assessments = topicsWithAssessmentCountsMap[node.id];

return results
.map(node => {
if ([ContentNodeKinds.TOPIC, ContentNodeKinds.CHANNEL].includes(node.kind)) {
if (!topicsWithAssessmentCountsMap[node.id]) {
return null;
}
return node;
})
// Filter out any topics that have no assessments
// that we have already flagged as null above
.filter(Boolean)
);
if (node.kind === ContentNodeKinds.TOPIC && !node.children) {
return null;
}
node.num_assessments = topicsWithAssessmentCountsMap[node.id];
}
return node;
})
.filter(Boolean);
})
.catch(e => {
// TODO Work out best UX for this situation -- it may depend on if we're fetching more
// or the initial list of contents
logger.error(e);
});
}

/**
* @affects _resources - Sets the _resources to the results of the fetchTree call
* @affects _loading
* @returns {Promise<null>} - A promise that resolves when the annotations have been made and
*/
async function fetchQuizResources() {
function setResources(r) {
set(_resources, r);
}

const api = {
setResources,
resources: computed(() => get(_resources)),
loading: computed(() => get(_loading) || get(treeLoading)),
loadingMore: computed(() => get(_loadingMore)),
hasMore,
topic,
annotateTopicsWithDescendantCounts,
fetchQuizResources: undefined,
fetchMoreQuizResources: undefined,
};
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of defining the api in a separate object here?


api.fetchQuizResources = async function fetchQuizResources() {
set(_loading, true);
return fetchTree().then(async results => {
return annotateTopicsWithDescendantCounts(results).then(annotatedResults => {
return api.annotateTopicsWithDescendantCounts(results).then(annotatedResults => {
setResources(annotatedResults);
set(_loading, false);
});
});
}
};

/**
* @affects _resources - Appends the results of the fetchMore call to the _resources
* and annotates any new topics with descendant counts
* @affects _loading - fetchMore & annotateTopicsWithDescendantCounts update the loading states
*/
async function fetchMoreQuizResources() {
api.fetchMoreQuizResources = async function fetchMoreQuizResources() {
set(_loading, true);
set(_loadingMore, true);
return fetchMore().then(async results => {
return annotateTopicsWithDescendantCounts(results).then(annotatedResults => {
return api.annotateTopicsWithDescendantCounts(results).then(annotatedResults => {
set(_resources, [...get(_resources), ...annotatedResults]);
set(_loading, false);
set(_loadingMore, false);
});
});
}

function setResources(r) {
set(_resources, r);
}

return {
setResources,
resources: computed(() => get(_resources)),
loading: computed(() => get(_loading) || get(treeLoading)),
loadingMore: computed(() => get(_loadingMore)),
fetchQuizResources,
fetchMoreQuizResources,
hasMore,
topic,
annotateTopicsWithDescendantCounts,
};

return api;
}
Loading
Loading