From eb95fb829d5c760aecf76c8fc368e1667a747c21 Mon Sep 17 00:00:00 2001 From: MisRob Date: Wed, 14 Jun 2023 15:42:07 +0200 Subject: [PATCH 1/6] Align condition with the intention --- kolibri/core/assets/src/views/CoreTable.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/assets/src/views/CoreTable.vue b/kolibri/core/assets/src/views/CoreTable.vue index 15663d0a0a6..3c2814c97e4 100644 --- a/kolibri/core/assets/src/views/CoreTable.vue +++ b/kolibri/core/assets/src/views/CoreTable.vue @@ -84,7 +84,7 @@ }); // If we have loaded the data, but have no empty message and no rows, we log an error. - if (!this.dataLoading && this.emptyMessage && !tableHasRows) { + if (!this.dataLoading && !this.emptyMessage && !tableHasRows) { logging.error('CoreTable: No rows in table, but no empty message provided.'); } From 352c3ba33bcb13aa305da269d87af84e7d096ea9 Mon Sep 17 00:00:00 2001 From: MisRob Date: Wed, 14 Jun 2023 15:43:31 +0200 Subject: [PATCH 2/6] Remove unnecessary var --- kolibri/core/assets/src/views/CoreTable.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/assets/src/views/CoreTable.vue b/kolibri/core/assets/src/views/CoreTable.vue index 3c2814c97e4..26ecb05a184 100644 --- a/kolibri/core/assets/src/views/CoreTable.vue +++ b/kolibri/core/assets/src/views/CoreTable.vue @@ -93,7 +93,7 @@ * empty message if there are no rows in the table. If we have loaded data, have * an emptyMessage and have no rows. If we have rows, then we show the table alone */ - var dataStatusEl = this.dataLoading + const dataStatusEl = this.dataLoading ? createElement('p', [createElement(KCircularLoader)]) : !tableHasRows && createElement('p', this.emptyMessage); // Only show message if no rows From dcab66d6069d4eb0b9772f8905d0b913e5ed849b Mon Sep 17 00:00:00 2001 From: MisRob Date: Tue, 4 Jul 2023 09:42:50 +0200 Subject: [PATCH 3/6] Show loader in the table in Coach-Plan-Lessons --- .../assets/src/composables/useLessons.js | 15 ++++++ .../src/modules/lessonsRoot/handlers.js | 48 +++++++++++-------- .../src/views/plan/LessonsRootPage/index.vue | 16 +++++-- 3 files changed, 53 insertions(+), 26 deletions(-) create mode 100644 kolibri/plugins/coach/assets/src/composables/useLessons.js diff --git a/kolibri/plugins/coach/assets/src/composables/useLessons.js b/kolibri/plugins/coach/assets/src/composables/useLessons.js new file mode 100644 index 00000000000..5f9e872c0bd --- /dev/null +++ b/kolibri/plugins/coach/assets/src/composables/useLessons.js @@ -0,0 +1,15 @@ +import { ref } from 'kolibri.lib.vueCompositionApi'; + +// Place outside the function to keep the state +const lessonsAreLoading = ref(false); + +export function useLessons() { + function setLessonsLoading(loading) { + lessonsAreLoading.value = loading; + } + + return { + lessonsAreLoading, + setLessonsLoading, + }; +} diff --git a/kolibri/plugins/coach/assets/src/modules/lessonsRoot/handlers.js b/kolibri/plugins/coach/assets/src/modules/lessonsRoot/handlers.js index f12a57fb869..4f8ef5bdfbf 100644 --- a/kolibri/plugins/coach/assets/src/modules/lessonsRoot/handlers.js +++ b/kolibri/plugins/coach/assets/src/modules/lessonsRoot/handlers.js @@ -1,28 +1,34 @@ import { LearnerGroupResource } from 'kolibri.resources'; import { LessonsPageNames } from '../../constants/lessonsConstants'; +import { useLessons } from '../../composables/useLessons'; + +const { setLessonsLoading } = useLessons(); // Show the Lessons Root Page, where all the Lessons are listed for a given Classroom export function showLessonsRootPage(store, classId) { - return store.dispatch('loading').then(() => { - store.commit('lessonsRoot/SET_STATE', { - lessons: [], - learnerGroups: [], - }); - const loadRequirements = [ - // Fetch learner groups for the New Lesson Modal - LearnerGroupResource.fetchCollection({ getParams: { parent: classId } }), - store.dispatch('lessonsRoot/refreshClassLessons', classId), - ]; - return Promise.all(loadRequirements).then( - ([learnerGroups]) => { - store.commit('lessonsRoot/SET_LEARNER_GROUPS', learnerGroups); - store.commit('SET_PAGE_NAME', LessonsPageNames.PLAN_LESSONS_ROOT); - store.dispatch('notLoading'); - }, - error => { - store.dispatch('handleApiError', error); - store.dispatch('notLoading'); - } - ); + // on this page, don't handle loading state globally so we can do it locally + store.dispatch('notLoading'); + + setLessonsLoading(true); + store.commit('lessonsRoot/SET_STATE', { + lessons: [], + learnerGroups: [], }); + const loadRequirements = [ + // Fetch learner groups for the New Lesson Modal + LearnerGroupResource.fetchCollection({ getParams: { parent: classId } }), + store.dispatch('lessonsRoot/refreshClassLessons', classId), + ]; + + return Promise.all(loadRequirements).then( + ([learnerGroups]) => { + store.commit('lessonsRoot/SET_LEARNER_GROUPS', learnerGroups); + store.commit('SET_PAGE_NAME', LessonsPageNames.PLAN_LESSONS_ROOT); + setLessonsLoading(false); + }, + error => { + store.dispatch('handleApiError', error); + setLessonsLoading(false); + } + ); } diff --git a/kolibri/plugins/coach/assets/src/views/plan/LessonsRootPage/index.vue b/kolibri/plugins/coach/assets/src/views/plan/LessonsRootPage/index.vue index 5510e424a7e..8ecbe6901ff 100644 --- a/kolibri/plugins/coach/assets/src/views/plan/LessonsRootPage/index.vue +++ b/kolibri/plugins/coach/assets/src/views/plan/LessonsRootPage/index.vue @@ -26,7 +26,10 @@ /> - + -

- {{ $tr('noLessons') }} -

-

+

{{ coreString('noResultsLabel') }}

+ Date: Tue, 4 Jul 2023 09:55:33 +0200 Subject: [PATCH 4/6] Move handler to composable to keep the logic related to lessons loading in one place --- .../src/composables/__mocks__/useLessons.js | 46 +++++++++++++++++++ .../assets/src/composables/useLessons.js | 33 ++++++++++++- .../src/modules/lessonsRoot/handlers.js | 34 -------------- .../assets/src/routes/planLessonsRoutes.js | 5 +- 4 files changed, 82 insertions(+), 36 deletions(-) create mode 100644 kolibri/plugins/coach/assets/src/composables/__mocks__/useLessons.js delete mode 100644 kolibri/plugins/coach/assets/src/modules/lessonsRoot/handlers.js diff --git a/kolibri/plugins/coach/assets/src/composables/__mocks__/useLessons.js b/kolibri/plugins/coach/assets/src/composables/__mocks__/useLessons.js new file mode 100644 index 00000000000..3a95e46cc0f --- /dev/null +++ b/kolibri/plugins/coach/assets/src/composables/__mocks__/useLessons.js @@ -0,0 +1,46 @@ +/** + * `useLessons` composable function mock. + * + * If default values are sufficient for tests, + * you only need call `jest.mock('')` + * at the top of a test file. + * + * If you need to override some default values from some tests, + * you can import a helper function `useLessonsMock` that accepts + * an object with values to be overriden and use it together + * with `mockImplementation` as follows: + * + * ``` + * // eslint-disable-next-line import/named + * import useLessons, { useLessonsMock } from ''; + * + * jest.mock('') + * + * it('test', () => { + * useLessons.mockImplementation( + * () => useLessonsMock({ lessonsAreLoading: true }) + * ); + * }) + * ``` + * + * You can reset your mock implementation back to default values + * for other tests by calling the following in `beforeEach`: + * + * ``` + * useLessons.mockImplementation(() => useLessonsMock()) + * ``` + */ + +const MOCK_DEFAULTS = { + lessonsAreLoading: false, + showLessonsRootPage: jest.fn(), +}; + +export function useLessonsMock(overrides = {}) { + return { + ...MOCK_DEFAULTS, + ...overrides, + }; +} + +export default jest.fn(() => useLessonsMock()); diff --git a/kolibri/plugins/coach/assets/src/composables/useLessons.js b/kolibri/plugins/coach/assets/src/composables/useLessons.js index 5f9e872c0bd..270bc929ca0 100644 --- a/kolibri/plugins/coach/assets/src/composables/useLessons.js +++ b/kolibri/plugins/coach/assets/src/composables/useLessons.js @@ -1,4 +1,6 @@ import { ref } from 'kolibri.lib.vueCompositionApi'; +import { LearnerGroupResource } from 'kolibri.resources'; +import { LessonsPageNames } from '../constants/lessonsConstants'; // Place outside the function to keep the state const lessonsAreLoading = ref(false); @@ -8,8 +10,37 @@ export function useLessons() { lessonsAreLoading.value = loading; } + // Show the Lessons Root Page, where all the Lessons are listed for a given Classroom + function showLessonsRootPage(store, classId) { + // on this page, don't handle loading state globally so we can do it locally + store.dispatch('notLoading'); + + setLessonsLoading(true); + store.commit('lessonsRoot/SET_STATE', { + lessons: [], + learnerGroups: [], + }); + const loadRequirements = [ + // Fetch learner groups for the New Lesson Modal + LearnerGroupResource.fetchCollection({ getParams: { parent: classId } }), + store.dispatch('lessonsRoot/refreshClassLessons', classId), + ]; + + return Promise.all(loadRequirements).then( + ([learnerGroups]) => { + store.commit('lessonsRoot/SET_LEARNER_GROUPS', learnerGroups); + store.commit('SET_PAGE_NAME', LessonsPageNames.PLAN_LESSONS_ROOT); + setLessonsLoading(false); + }, + error => { + store.dispatch('handleApiError', error); + setLessonsLoading(false); + } + ); + } + return { lessonsAreLoading, - setLessonsLoading, + showLessonsRootPage, }; } diff --git a/kolibri/plugins/coach/assets/src/modules/lessonsRoot/handlers.js b/kolibri/plugins/coach/assets/src/modules/lessonsRoot/handlers.js deleted file mode 100644 index 4f8ef5bdfbf..00000000000 --- a/kolibri/plugins/coach/assets/src/modules/lessonsRoot/handlers.js +++ /dev/null @@ -1,34 +0,0 @@ -import { LearnerGroupResource } from 'kolibri.resources'; -import { LessonsPageNames } from '../../constants/lessonsConstants'; -import { useLessons } from '../../composables/useLessons'; - -const { setLessonsLoading } = useLessons(); - -// Show the Lessons Root Page, where all the Lessons are listed for a given Classroom -export function showLessonsRootPage(store, classId) { - // on this page, don't handle loading state globally so we can do it locally - store.dispatch('notLoading'); - - setLessonsLoading(true); - store.commit('lessonsRoot/SET_STATE', { - lessons: [], - learnerGroups: [], - }); - const loadRequirements = [ - // Fetch learner groups for the New Lesson Modal - LearnerGroupResource.fetchCollection({ getParams: { parent: classId } }), - store.dispatch('lessonsRoot/refreshClassLessons', classId), - ]; - - return Promise.all(loadRequirements).then( - ([learnerGroups]) => { - store.commit('lessonsRoot/SET_LEARNER_GROUPS', learnerGroups); - store.commit('SET_PAGE_NAME', LessonsPageNames.PLAN_LESSONS_ROOT); - setLessonsLoading(false); - }, - error => { - store.dispatch('handleApiError', error); - setLessonsLoading(false); - } - ); -} diff --git a/kolibri/plugins/coach/assets/src/routes/planLessonsRoutes.js b/kolibri/plugins/coach/assets/src/routes/planLessonsRoutes.js index ccb25166230..5bb0a742c38 100644 --- a/kolibri/plugins/coach/assets/src/routes/planLessonsRoutes.js +++ b/kolibri/plugins/coach/assets/src/routes/planLessonsRoutes.js @@ -8,10 +8,11 @@ import { showLessonResourceBookmarks, showLessonResourceBookmarksMain, } from '../modules/lessonResources/handlers'; -import { showLessonsRootPage } from '../modules/lessonsRoot/handlers'; import { showLessonSummaryPage } from '../modules/lessonSummary/handlers'; import { LessonsPageNames } from '../constants/lessonsConstants'; +import { useLessons } from '../composables/useLessons'; + import LessonsRootPage from '../views/plan/LessonsRootPage'; import LessonSummaryPage from '../views/plan/LessonSummaryPage'; import LessonResourceSelectionPage from '../views/plan/LessonResourceSelectionPage'; @@ -31,6 +32,8 @@ function path(...args) { return args.join(''); } +const { showLessonsRootPage } = useLessons(); + export default [ { name: LessonsPageNames.PLAN_LESSONS_ROOT, From 9261dababc3db6c45a9b2688231f3dfdd307ea25 Mon Sep 17 00:00:00 2001 From: MisRob Date: Tue, 4 Jul 2023 12:58:47 +0200 Subject: [PATCH 5/6] Show loader in the table in Coach-Plan-Groups --- .../coach/assets/src/composables/useGroups.js | 15 +++++++++++++++ .../coach/assets/src/modules/groups/handlers.js | 11 +++++++++-- .../assets/src/views/plan/GroupsPage/index.vue | 12 +++++++----- 3 files changed, 31 insertions(+), 7 deletions(-) create mode 100644 kolibri/plugins/coach/assets/src/composables/useGroups.js diff --git a/kolibri/plugins/coach/assets/src/composables/useGroups.js b/kolibri/plugins/coach/assets/src/composables/useGroups.js new file mode 100644 index 00000000000..9cac09dc6f9 --- /dev/null +++ b/kolibri/plugins/coach/assets/src/composables/useGroups.js @@ -0,0 +1,15 @@ +import { ref } from 'kolibri.lib.vueCompositionApi'; + +// Place outside the function to keep the state +const groupsAreLoading = ref(false); + +export function useGroups() { + function setGroupsLoading(loading) { + groupsAreLoading.value = loading; + } + + return { + groupsAreLoading, + setGroupsLoading, + }; +} diff --git a/kolibri/plugins/coach/assets/src/modules/groups/handlers.js b/kolibri/plugins/coach/assets/src/modules/groups/handlers.js index 918030e533d..ffe9e32b06b 100644 --- a/kolibri/plugins/coach/assets/src/modules/groups/handlers.js +++ b/kolibri/plugins/coach/assets/src/modules/groups/handlers.js @@ -1,8 +1,15 @@ import samePageCheckGenerator from 'kolibri.utils.samePageCheckGenerator'; import { LearnerGroupResource, FacilityUserResource } from 'kolibri.resources'; +import { useGroups } from '../../composables/useGroups'; + +const { setGroupsLoading } = useGroups(); export function showGroupsPage(store, classId) { - store.dispatch('loading'); + // on this page, don't handle loading state globally so we can do it locally + store.dispatch('notLoading'); + + setGroupsLoading(true); + const promises = [ FacilityUserResource.fetchCollection({ getParams: { member_of: classId }, @@ -36,7 +43,7 @@ export function showGroupsPage(store, classId) { groups, groupModalShown: false, }); - store.dispatch('notLoading'); + setGroupsLoading(false); store.dispatch('clearError'); } }, diff --git a/kolibri/plugins/coach/assets/src/views/plan/GroupsPage/index.vue b/kolibri/plugins/coach/assets/src/views/plan/GroupsPage/index.vue index daa81efcba6..bab3c09b1ef 100644 --- a/kolibri/plugins/coach/assets/src/views/plan/GroupsPage/index.vue +++ b/kolibri/plugins/coach/assets/src/views/plan/GroupsPage/index.vue @@ -19,7 +19,10 @@ /> - + -

- {{ $tr('noGroups') }} -

- Date: Tue, 4 Jul 2023 13:07:05 +0200 Subject: [PATCH 6/6] Move handler to composable to keep the logic related to groups loading in one place --- .../src/composables/__mocks__/useGroups.js | 46 +++++++++++++++ .../coach/assets/src/composables/useGroups.js | 58 ++++++++++++++++++- .../assets/src/modules/groups/handlers.js | 58 ------------------- .../coach/assets/src/routes/planRoutes.js | 4 +- 4 files changed, 106 insertions(+), 60 deletions(-) create mode 100644 kolibri/plugins/coach/assets/src/composables/__mocks__/useGroups.js delete mode 100644 kolibri/plugins/coach/assets/src/modules/groups/handlers.js diff --git a/kolibri/plugins/coach/assets/src/composables/__mocks__/useGroups.js b/kolibri/plugins/coach/assets/src/composables/__mocks__/useGroups.js new file mode 100644 index 00000000000..f1fcb9dde7e --- /dev/null +++ b/kolibri/plugins/coach/assets/src/composables/__mocks__/useGroups.js @@ -0,0 +1,46 @@ +/** + * `useGroups` composable function mock. + * + * If default values are sufficient for tests, + * you only need call `jest.mock('')` + * at the top of a test file. + * + * If you need to override some default values from some tests, + * you can import a helper function `useGroupsMock` that accepts + * an object with values to be overriden and use it together + * with `mockImplementation` as follows: + * + * ``` + * // eslint-disable-next-line import/named + * import useGroups, { useGroupsMock } from ''; + * + * jest.mock('') + * + * it('test', () => { + * useGroups.mockImplementation( + * () => useGroupsMock({ groupsAreLoading: true }) + * ); + * }) + * ``` + * + * You can reset your mock implementation back to default values + * for other tests by calling the following in `beforeEach`: + * + * ``` + * useGroups.mockImplementation(() => useGroupsMock()) + * ``` + */ + +const MOCK_DEFAULTS = { + groupsAreLoading: false, + showGroupsPage: jest.fn(), +}; + +export function useGroupsMock(overrides = {}) { + return { + ...MOCK_DEFAULTS, + ...overrides, + }; +} + +export default jest.fn(() => useGroupsMock()); diff --git a/kolibri/plugins/coach/assets/src/composables/useGroups.js b/kolibri/plugins/coach/assets/src/composables/useGroups.js index 9cac09dc6f9..595c9930712 100644 --- a/kolibri/plugins/coach/assets/src/composables/useGroups.js +++ b/kolibri/plugins/coach/assets/src/composables/useGroups.js @@ -1,4 +1,6 @@ import { ref } from 'kolibri.lib.vueCompositionApi'; +import samePageCheckGenerator from 'kolibri.utils.samePageCheckGenerator'; +import { LearnerGroupResource, FacilityUserResource } from 'kolibri.resources'; // Place outside the function to keep the state const groupsAreLoading = ref(false); @@ -8,8 +10,62 @@ export function useGroups() { groupsAreLoading.value = loading; } + function showGroupsPage(store, classId) { + // On this page, handle loading state locally + // TODO: Open follow-up so that we don't need to do this + store.dispatch('notLoading'); + + setGroupsLoading(true); + + const promises = [ + FacilityUserResource.fetchCollection({ + getParams: { member_of: classId }, + force: true, + }), + LearnerGroupResource.fetchCollection({ + getParams: { parent: classId }, + force: true, + }), + ]; + const shouldResolve = samePageCheckGenerator(store); + return Promise.all(promises).then( + ([classUsers, groupsCollection]) => { + if (shouldResolve()) { + const groups = groupsCollection.map(group => ({ ...group, users: [] })); + const groupUsersPromises = groups.map(group => + FacilityUserResource.fetchCollection({ + getParams: { member_of: group.id }, + force: true, + }) + ); + + Promise.all(groupUsersPromises).then( + groupsUsersCollection => { + if (shouldResolve()) { + groupsUsersCollection.forEach((groupUsers, index) => { + groups[index].users = [...groupUsers]; + }); + store.commit('groups/SET_STATE', { + classUsers: [...classUsers], + groups, + groupModalShown: false, + }); + setGroupsLoading(false); + store.dispatch('clearError'); + } + }, + error => (shouldResolve() ? store.dispatch('handleError', error) : null) + ); + } + }, + error => { + shouldResolve() ? store.dispatch('handleError', error) : null; + } + ); + } + return { groupsAreLoading, - setGroupsLoading, + showGroupsPage, }; } diff --git a/kolibri/plugins/coach/assets/src/modules/groups/handlers.js b/kolibri/plugins/coach/assets/src/modules/groups/handlers.js deleted file mode 100644 index ffe9e32b06b..00000000000 --- a/kolibri/plugins/coach/assets/src/modules/groups/handlers.js +++ /dev/null @@ -1,58 +0,0 @@ -import samePageCheckGenerator from 'kolibri.utils.samePageCheckGenerator'; -import { LearnerGroupResource, FacilityUserResource } from 'kolibri.resources'; -import { useGroups } from '../../composables/useGroups'; - -const { setGroupsLoading } = useGroups(); - -export function showGroupsPage(store, classId) { - // on this page, don't handle loading state globally so we can do it locally - store.dispatch('notLoading'); - - setGroupsLoading(true); - - const promises = [ - FacilityUserResource.fetchCollection({ - getParams: { member_of: classId }, - force: true, - }), - LearnerGroupResource.fetchCollection({ - getParams: { parent: classId }, - force: true, - }), - ]; - const shouldResolve = samePageCheckGenerator(store); - return Promise.all(promises).then( - ([classUsers, groupsCollection]) => { - if (shouldResolve()) { - const groups = groupsCollection.map(group => ({ ...group, users: [] })); - const groupUsersPromises = groups.map(group => - FacilityUserResource.fetchCollection({ - getParams: { member_of: group.id }, - force: true, - }) - ); - - Promise.all(groupUsersPromises).then( - groupsUsersCollection => { - if (shouldResolve()) { - groupsUsersCollection.forEach((groupUsers, index) => { - groups[index].users = [...groupUsers]; - }); - store.commit('groups/SET_STATE', { - classUsers: [...classUsers], - groups, - groupModalShown: false, - }); - setGroupsLoading(false); - store.dispatch('clearError'); - } - }, - error => (shouldResolve() ? store.dispatch('handleError', error) : null) - ); - } - }, - error => { - shouldResolve() ? store.dispatch('handleError', error) : null; - } - ); -} diff --git a/kolibri/plugins/coach/assets/src/routes/planRoutes.js b/kolibri/plugins/coach/assets/src/routes/planRoutes.js index 918bd3d0d23..05f1b5a2c3d 100644 --- a/kolibri/plugins/coach/assets/src/routes/planRoutes.js +++ b/kolibri/plugins/coach/assets/src/routes/planRoutes.js @@ -1,12 +1,14 @@ import store from 'kolibri.coreVue.vuex.store'; import { PageNames } from '../constants'; +import { useGroups } from '../composables/useGroups'; import GroupsPage from '../views/plan/GroupsPage'; import GroupMembersPage from '../views/plan/GroupMembersPage'; import GroupEnrollPage from '../views/plan/GroupEnrollPage'; -import { showGroupsPage } from '../modules/groups/handlers'; import planLessonsRoutes from './planLessonsRoutes'; import planExamRoutes from './planExamRoutes'; +const { showGroupsPage } = useGroups(); + export default [ ...planLessonsRoutes, ...planExamRoutes,