From 9ceca35ad6081a059c27677f9702f9311cf10453 Mon Sep 17 00:00:00 2001 From: Michaela Date: Tue, 19 Oct 2021 16:56:52 +0200 Subject: [PATCH] Fix criteria for displaying sections with non-classes content 'Continue learning on your own' and 'Explore channels' should be displayed only after learner finished all their classes resources and quizzes rather than when not having any classes resources or quizzes in progress --- .../__mocks__/useLearnerResources.js | 1 + .../__tests__/useLearnerResources.spec.js | 108 ++++++++- .../src/composables/useLearnerResources.js | 16 ++ .../views/HomePage/__tests__/HomePage.spec.js | 219 ++++++++---------- .../learn/assets/src/views/HomePage/index.vue | 18 +- 5 files changed, 234 insertions(+), 128 deletions(-) diff --git a/kolibri/plugins/learn/assets/src/composables/__mocks__/useLearnerResources.js b/kolibri/plugins/learn/assets/src/composables/__mocks__/useLearnerResources.js index d7f78d50e20..125251880f7 100644 --- a/kolibri/plugins/learn/assets/src/composables/__mocks__/useLearnerResources.js +++ b/kolibri/plugins/learn/assets/src/composables/__mocks__/useLearnerResources.js @@ -38,6 +38,7 @@ const MOCK_DEFAULTS = { resumableClassesQuizzes: [], resumableClassesResources: [], resumableNonClassesContentNodes: [], + learnerFinishedAllClasses: false, getClass: jest.fn(), getClassActiveLessons: jest.fn(), getClassActiveQuizzes: jest.fn(), diff --git a/kolibri/plugins/learn/assets/src/composables/__tests__/useLearnerResources.spec.js b/kolibri/plugins/learn/assets/src/composables/__tests__/useLearnerResources.spec.js index 280f5466a37..a2c72215cc5 100644 --- a/kolibri/plugins/learn/assets/src/composables/__tests__/useLearnerResources.spec.js +++ b/kolibri/plugins/learn/assets/src/composables/__tests__/useLearnerResources.spec.js @@ -1,5 +1,6 @@ -import { ContentNodeResource, ContentNodeProgressResource } from 'kolibri.resources'; +import cloneDeep from 'lodash/cloneDeep'; +import { ContentNodeResource, ContentNodeProgressResource } from 'kolibri.resources'; import { PageNames, ClassesPageNames } from '../../constants'; import { LearnerClassroomResource } from '../../apiResources'; import useLearnerResources from '../useLearnerResources'; @@ -11,6 +12,7 @@ const { resumableClassesQuizzes, resumableClassesResources, resumableNonClassesContentNodes, + learnerFinishedAllClasses, getClass, getClassActiveLessons, getClassActiveQuizzes, @@ -118,6 +120,10 @@ const TEST_CLASSES = [ { contentnode_id: 'resource-2' }, { contentnode_id: 'resource-3-in-progress' }, ], + progress: { + resource_progress: 0, + total_resources: 3, + }, }, { id: 'class-1-active-lesson-2', @@ -129,6 +135,10 @@ const TEST_CLASSES = [ { contentnode_id: 'resource-4' }, { contentnode_id: 'resource-5-in-progress' }, ], + progress: { + resource_progress: 1, + total_resources: 3, + }, }, { id: 'class-1-inactive-lesson', @@ -136,6 +146,10 @@ const TEST_CLASSES = [ is_active: false, collection: 'class-1', resources: [{ contentnode_id: 'resource-5-in-progress' }], + progress: { + resource_progress: 0, + total_resources: 1, + }, }, ], }, @@ -177,6 +191,10 @@ const TEST_CLASSES = [ { contentnode_id: 'resource-2' }, { contentnode_id: 'resource-1-in-progress' }, ], + progress: { + resource_progress: 1, + total_resources: 3, + }, }, { id: 'class-2-inactive-lesson', @@ -187,14 +205,50 @@ const TEST_CLASSES = [ { contentnode_id: 'resource-7' }, { contentnode_id: 'resource-8-in-progress' }, ], + progress: { + resource_progress: 0, + total_resources: 2, + }, }, ], }, }, ]; +// A helper that takes an object with test classes +// and returns its copy with all lessons and quizzes +// progress changed to complete +function finishClasses(classes) { + const finishedClasses = cloneDeep(classes); + return finishedClasses.map(c => { + c.assignments.exams.forEach(quiz => { + quiz.progress.closed = true; + }); + c.assignments.lessons.forEach(lesson => { + lesson.progress.resource_progress = lesson.progress.total_resources; + }); + return c; + }); +} + +// A helper that takes an object with test classes +// and returns its copy with all lessons and quizzes +// changed to inactive +function inactivateClasses(classes) { + const inactiveClasses = cloneDeep(classes); + return inactiveClasses.map(c => { + c.assignments.exams.forEach(quiz => { + quiz.active = false; + }); + c.assignments.lessons.forEach(lesson => { + lesson.is_active = false; + }); + return c; + }); +} + describe(`useLearnerResources`, () => { - beforeAll(() => { + beforeEach(() => { ContentNodeResource.fetchResume.mockResolvedValue(TEST_RESUMABLE_CONTENT_NODES); ContentNodeProgressResource.fetchCollection.mockResolvedValue(TEST_CONTENT_NODES_PROGRESSES); fetchResumableContentNodes(); @@ -222,6 +276,10 @@ describe(`useLearnerResources`, () => { { contentnode_id: 'resource-2' }, { contentnode_id: 'resource-3-in-progress' }, ], + progress: { + resource_progress: 0, + total_resources: 3, + }, }, { id: 'class-1-active-lesson-2', @@ -233,6 +291,10 @@ describe(`useLearnerResources`, () => { { contentnode_id: 'resource-4' }, { contentnode_id: 'resource-5-in-progress' }, ], + progress: { + resource_progress: 1, + total_resources: 3, + }, }, { id: 'class-2-active-lesson-1', @@ -244,6 +306,10 @@ describe(`useLearnerResources`, () => { { contentnode_id: 'resource-2' }, { contentnode_id: 'resource-1-in-progress' }, ], + progress: { + resource_progress: 1, + total_resources: 3, + }, }, ]); }); @@ -371,6 +437,36 @@ describe(`useLearnerResources`, () => { }); }); + describe(`learnerFinishedAllClasses`, () => { + it(`returns 'true' if a learner has no classes`, () => { + LearnerClassroomResource.fetchCollection.mockResolvedValue([]); + fetchClasses().then(() => { + expect(learnerFinishedAllClasses.value).toBe(true); + }); + }); + + it(`returns 'true' if a learner has no active lessons and quizzes`, () => { + LearnerClassroomResource.fetchCollection.mockResolvedValue(inactivateClasses(TEST_CLASSES)); + fetchClasses().then(() => { + expect(learnerFinishedAllClasses.value).toBe(true); + }); + }); + + it(`returns 'false' if a learner hasn't finished all lessons and quizzes yet`, () => { + LearnerClassroomResource.fetchCollection.mockResolvedValue(TEST_CLASSES); + fetchClasses().then(() => { + expect(learnerFinishedAllClasses.value).toBe(false); + }); + }); + + it(`returns 'true' if a learner finished all lessons and quizzes`, () => { + LearnerClassroomResource.fetchCollection.mockResolvedValue(finishClasses(TEST_CLASSES)); + fetchClasses().then(() => { + expect(learnerFinishedAllClasses.value).toBe(true); + }); + }); + }); + describe(`getClass`, () => { it(`returns a class`, () => { expect(getClass('class-2')).toEqual(TEST_CLASSES[1]); @@ -390,6 +486,10 @@ describe(`useLearnerResources`, () => { { contentnode_id: 'resource-2' }, { contentnode_id: 'resource-3-in-progress' }, ], + progress: { + resource_progress: 0, + total_resources: 3, + }, }, { id: 'class-1-active-lesson-2', @@ -401,6 +501,10 @@ describe(`useLearnerResources`, () => { { contentnode_id: 'resource-4' }, { contentnode_id: 'resource-5-in-progress' }, ], + progress: { + resource_progress: 1, + total_resources: 3, + }, }, ]); }); diff --git a/kolibri/plugins/learn/assets/src/composables/useLearnerResources.js b/kolibri/plugins/learn/assets/src/composables/useLearnerResources.js index f4a7dd46518..447273b1358 100644 --- a/kolibri/plugins/learn/assets/src/composables/useLearnerResources.js +++ b/kolibri/plugins/learn/assets/src/composables/useLearnerResources.js @@ -157,6 +157,21 @@ export default function useLearnerResources() { ); }); + /** + * @returns {Boolean} - `true` if a learner finished all active + * classes lessons and quizzes (or when there are none) + * @public + */ + const learnerFinishedAllClasses = computed(() => { + const hasUnfinishedLesson = get(activeClassesLessons).some(lesson => { + return lesson.progress.resource_progress < lesson.progress.total_resources; + }); + const hasUnfinishedQuiz = get(activeClassesQuizzes).some(quiz => { + return !quiz.progress.closed; + }); + return !(hasUnfinishedLesson || hasUnfinishedQuiz); + }); + /** * @param {String} classId * @returns {Object} A class @@ -362,6 +377,7 @@ export default function useLearnerResources() { resumableClassesQuizzes, resumableClassesResources, resumableNonClassesContentNodes, + learnerFinishedAllClasses, getClass, getResumableContentNode, getResumableContentNodeProgress, diff --git a/kolibri/plugins/learn/assets/src/views/HomePage/__tests__/HomePage.spec.js b/kolibri/plugins/learn/assets/src/views/HomePage/__tests__/HomePage.spec.js index 20c57a8f28d..a9ad05a5211 100644 --- a/kolibri/plugins/learn/assets/src/views/HomePage/__tests__/HomePage.spec.js +++ b/kolibri/plugins/learn/assets/src/views/HomePage/__tests__/HomePage.spec.js @@ -45,8 +45,8 @@ function getClassesSection(wrapper) { return wrapper.find('[data-test="classes"]'); } -function getContinueLearningSection(wrapper) { - return wrapper.find('[data-test="continueLearning"]'); +function getContinueLearningFromClassesSection(wrapper) { + return wrapper.find('[data-test="continueLearningFromClasses"]'); } function getRecentLessonsSection(wrapper) { @@ -57,6 +57,10 @@ function getRecentQuizzesSection(wrapper) { return wrapper.find('[data-test="recentQuizzes"]'); } +function getContinueLearningOnYourOwnSection(wrapper) { + return wrapper.find('[data-test="continueLearningOnYourOwn"]'); +} + function getExploreChannelsSection(wrapper) { return wrapper.find('[data-test="exploreChannels"]'); } @@ -105,20 +109,20 @@ describe(`HomePage`, () => { }); }); - describe(`"Continue learning from classes/on your own" section`, () => { + describe(`"Continue learning from classes" section`, () => { it(`the section is not displayed for a guest user`, () => { const wrapper = makeWrapper(); - expect(getContinueLearningSection(wrapper).exists()).toBe(false); + expect(getContinueLearningFromClassesSection(wrapper).exists()).toBe(false); }); it(`the section is not displayed for a signed in user who has - no resources or quizzes in progress`, () => { + no classes resources or quizzes in progress`, () => { useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); const wrapper = makeWrapper(); - expect(getContinueLearningSection(wrapper).exists()).toBe(false); + expect(getContinueLearningFromClassesSection(wrapper).exists()).toBe(false); }); - describe(`for a signed in user who has some classes resources or quizzes in progress`, () => { + describe(`for a signed in user who has some resources or quizzes in progress`, () => { beforeEach(() => { useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); useLearnerResources.mockImplementation(() => @@ -152,25 +156,9 @@ describe(`HomePage`, () => { ); }); - it(`"Continue learning on your own" section is not displayed`, () => { - const wrapper = makeWrapper(); - expect(wrapper.text()).not.toContain('Continue learning on your own'); - }); - - it(`"Continue learning from your classes" section is displayed`, () => { + it(`the section is displayed and contains classes resources and quizzes in progress`, () => { const wrapper = makeWrapper(); - expect(wrapper.text()).toContain('Continue learning from your classes'); - }); - - it(`resources in progress outside of classes are not displayed`, () => { - const wrapper = makeWrapper(); - expect(getContinueLearningSection(wrapper).text()).not.toContain('Non-class resource 1'); - expect(getContinueLearningSection(wrapper).text()).not.toContain('Non-class resource 2'); - }); - - it(`classes resources and quizzes in progress are displayed`, () => { - const wrapper = makeWrapper(); - const links = getContinueLearningSection(wrapper).findAll('a'); + const links = getContinueLearningFromClassesSection(wrapper).findAll('a'); expect(links.length).toBe(4); expect(links.at(0).text()).toBe('Class resource 1'); expect(links.at(1).text()).toBe('Class resource 2'); @@ -178,10 +166,20 @@ describe(`HomePage`, () => { expect(links.at(3).text()).toBe('Class quiz 2'); }); + it(`resources in progress outside of classes are not displayed`, () => { + const wrapper = makeWrapper(); + expect(getContinueLearningFromClassesSection(wrapper).text()).not.toContain( + 'Non-class resource 1' + ); + expect(getContinueLearningFromClassesSection(wrapper).text()).not.toContain( + 'Non-class resource 2' + ); + }); + it(`clicking a resource navigates to the class resource page`, () => { const wrapper = makeWrapper(); expect(wrapper.vm.$route.path).toBe('/'); - const links = getContinueLearningSection(wrapper).findAll('a'); + const links = getContinueLearningFromClassesSection(wrapper).findAll('a'); links.at(0).trigger('click'); expect(wrapper.vm.$route.path).toBe('/class-resource'); }); @@ -189,71 +187,11 @@ describe(`HomePage`, () => { it(`clicking a quiz navigates to the class quiz page`, () => { const wrapper = makeWrapper(); expect(wrapper.vm.$route.path).toBe('/'); - const links = getContinueLearningSection(wrapper).findAll('a'); + const links = getContinueLearningFromClassesSection(wrapper).findAll('a'); links.at(2).trigger('click'); expect(wrapper.vm.$route.path).toBe('/class-quiz'); }); }); - - describe(`for a signed in user who doesn't have any classes resources or quizzes in progress - and has some resources in progress outside of classes`, () => { - beforeEach(() => { - useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); - useLearnerResources.mockImplementation(() => - useLearnerResourcesMock({ - resumableNonClassesContentNodes: [ - { id: 'non-class-resource-1', title: 'Non-class resource 1' }, - { id: 'non-class-resource-2', title: 'Non-class resource 2' }, - ], - getTopicContentNodeLink() { - return { path: '/topic-resource' }; - }, - }) - ); - }); - - it(`"Continue learning from your classes" section is not displayed`, () => { - const wrapper = makeWrapper(); - expect(wrapper.text()).not.toContain('Continue learning from your classes'); - }); - - it(`"Continue learning on your own" section is not displayed - when access to unassigned content is not allowed`, () => { - const wrapper = makeWrapper(); - expect(wrapper.text()).not.toContain('Continue learning on your own'); - }); - - describe(`when access to unassigned content is allowed`, () => { - beforeEach(() => { - useDeviceSettings.mockImplementation(() => - useDeviceSettingsMock({ - canAccessUnassignedContent: true, - }) - ); - }); - - it(`"Continue learning on your own" section is displayed`, () => { - const wrapper = makeWrapper(); - expect(wrapper.text()).toContain('Continue learning on your own'); - }); - - it(`resources in progress outside of classes are displayed`, () => { - const wrapper = makeWrapper(); - const links = getContinueLearningSection(wrapper).findAll('a'); - expect(links.length).toBe(2); - expect(links.at(0).text()).toBe('Non-class resource 1'); - expect(links.at(1).text()).toBe('Non-class resource 2'); - }); - - it(`clicking a resource navigates to the topic resource page`, () => { - const wrapper = makeWrapper(); - expect(wrapper.vm.$route.path).toBe('/'); - const links = getContinueLearningSection(wrapper).findAll('a'); - links.at(0).trigger('click'); - expect(wrapper.vm.$route.path).toBe('/topic-resource'); - }); - }); - }); }); describe(`"Recent lessons" section`, () => { @@ -326,72 +264,117 @@ describe(`HomePage`, () => { }); }); - describe(`"Explore channels" section`, () => { - it(`the section is not displayed when there are no channels available`, () => { + describe(`"Continue learning on your own" section`, () => { + it(`the section is not displayed for a guest user`, () => { const wrapper = makeWrapper(); - expect(getExploreChannelsSection(wrapper).exists()).toBe(false); + expect(getContinueLearningOnYourOwnSection(wrapper).exists()).toBe(false); }); - describe(`when there are some channels available`, () => { - beforeEach(() => { - useChannels.mockImplementation(() => useChannelsMock({ channels: [{ id: 'channel-1' }] })); - }); + it(`the section is not displayed for a signed in user + who hasn't finished all their classes resources and quizzes yet`, () => { + useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); + const wrapper = makeWrapper(); + expect(getContinueLearningOnYourOwnSection(wrapper).exists()).toBe(false); + }); - it(`the section is not displayed for a signed in user - who has some resumable classes resources`, () => { + describe(`for a signed in user + who has finished all their classes resources and quizzes + and has some non-classes resources in progress`, () => { + beforeEach(() => { useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); useLearnerResources.mockImplementation(() => useLearnerResourcesMock({ - resumableClassesResources: [ - { contentNodeId: 'class-resource-1', lessonId: 'class-1-lesson', classId: 'class-1' }, - { contentNodeId: 'class-resource-2', lessonId: 'class-2-lesson', classId: 'class-2' }, + learnerFinishedAllClasses: true, + resumableNonClassesContentNodes: [ + { id: 'non-class-resource-1', title: 'Non-class resource 1' }, + { id: 'non-class-resource-2', title: 'Non-class resource 2' }, ], - getClassResourceLink() { - return { path: '/class-resource' }; + getTopicContentNodeLink() { + return { path: '/topic-resource' }; }, }) ); + }); + it(`the section is not displayed when access to unassigned content is not allowed`, () => { const wrapper = makeWrapper(); - expect(getExploreChannelsSection(wrapper).exists()).toBe(false); + expect(getContinueLearningOnYourOwnSection(wrapper).exists()).toBe(false); + }); + + describe(`when access to unassigned content is allowed`, () => { + beforeEach(() => { + useDeviceSettings.mockImplementation(() => + useDeviceSettingsMock({ + canAccessUnassignedContent: true, + }) + ); + }); + + it(`the section is displayed and contains non-classes resources in progress`, () => { + const wrapper = makeWrapper(); + expect(getContinueLearningOnYourOwnSection(wrapper).exists()).toBe(true); + const links = getContinueLearningOnYourOwnSection(wrapper).findAll('a'); + expect(links.length).toBe(2); + expect(links.at(0).text()).toBe('Non-class resource 1'); + expect(links.at(1).text()).toBe('Non-class resource 2'); + }); + + it(`clicking a resource navigates to the topic resource page`, () => { + const wrapper = makeWrapper(); + expect(wrapper.vm.$route.path).toBe('/'); + const links = getContinueLearningOnYourOwnSection(wrapper).findAll('a'); + links.at(0).trigger('click'); + expect(wrapper.vm.$route.path).toBe('/topic-resource'); + }); + }); + }); + }); + + describe(`"Explore channels" section`, () => { + it(`the section is not displayed when there are no channels available`, () => { + const wrapper = makeWrapper(); + expect(getExploreChannelsSection(wrapper).exists()).toBe(false); + }); + + describe(`when there are some channels available`, () => { + beforeEach(() => { + useChannels.mockImplementation(() => useChannelsMock({ channels: [{ id: 'channel-1' }] })); }); it(`the section is not displayed for a signed in user - who has some resumable quizzes`, () => { + who hasn't finished all their classes resources and quizzes yet`, () => { useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); - useLearnerResources.mockImplementation(() => - useLearnerResourcesMock({ - resumableClassesQuizzes: [ - { id: 'class-quiz-1', title: 'Class quiz 1' }, - { id: 'class-quiz-2', title: 'Class quiz 2' }, - ], - getClassQuizLink() { - return { path: '/class-quiz' }; - }, - }) - ); - const wrapper = makeWrapper(); expect(getExploreChannelsSection(wrapper).exists()).toBe(false); }); it(`the section is not displayed for a signed in user - who has no resumable classes resources or quizzes + who has finished all their classes resources and quizzes when access to unassigned content is not allowed`, () => { useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); + useLearnerResources.mockImplementation(() => + useLearnerResourcesMock({ + learnerFinishedAllClasses: true, + }) + ); const wrapper = makeWrapper(); expect(getExploreChannelsSection(wrapper).exists()).toBe(false); }); it(`the section is displayed for a signed in user - who has no resumable classes resources or quizzes + who has finished all their classes resources and quizzes when access to unassigned content is allowed`, () => { + useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); + useLearnerResources.mockImplementation(() => + useLearnerResourcesMock({ + learnerFinishedAllClasses: true, + }) + ); useDeviceSettings.mockImplementation(() => useDeviceSettingsMock({ canAccessUnassignedContent: true, }) ); - useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); const wrapper = makeWrapper(); expect(getExploreChannelsSection(wrapper).exists()).toBe(true); }); diff --git a/kolibri/plugins/learn/assets/src/views/HomePage/index.vue b/kolibri/plugins/learn/assets/src/views/HomePage/index.vue index 5b105c3ca0e..e57adecc763 100644 --- a/kolibri/plugins/learn/assets/src/views/HomePage/index.vue +++ b/kolibri/plugins/learn/assets/src/views/HomePage/index.vue @@ -12,7 +12,9 @@ v-if="continueLearningFromClasses || continueLearningOnYourOwn" class="section" :fromClasses="continueLearningFromClasses" - data-test="continueLearning" + :data-test="continueLearningFromClasses ? + 'continueLearningFromClasses' : + 'continueLearningOnYourOwn'" /> { - return get(resumableClassesQuizzes).length > 0 || get(resumableClassesResources).length > 0; - }); const continueLearningFromClasses = computed( - () => get(isUserLoggedIn) && get(canResumeClasses) + () => + (get(isUserLoggedIn) && get(resumableClassesQuizzes).length > 0) || + get(resumableClassesResources).length > 0 ); const continueLearningOnYourOwn = computed( () => get(isUserLoggedIn) && + get(learnerFinishedAllClasses) && get(canAccessUnassignedContent) && - !get(canResumeClasses) && get(resumableNonClassesContentNodes).length > 0 ); const hasActiveClassesLessons = computed( @@ -111,7 +113,8 @@ const displayExploreChannels = computed(() => { return ( get(hasChannels) && - (!get(isUserLoggedIn) || (!get(canResumeClasses) && get(canAccessUnassignedContent))) + (!get(isUserLoggedIn) || + (get(learnerFinishedAllClasses) && get(canAccessUnassignedContent))) ); }); @@ -123,7 +126,6 @@ activeClassesQuizzes, hasActiveClassesLessons, hasActiveClassesQuizzes, - canResumeClasses, continueLearningFromClasses, continueLearningOnYourOwn, displayExploreChannels,