From 0b70dada35635024bb5bf3a72daa6e26a67c97e2 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Fri, 25 Oct 2024 15:35:45 -0700 Subject: [PATCH 1/5] avoid type error in initClassInfo; use KExternalLink to redirect in authmessage w/ urls --- kolibri/plugins/coach/assets/src/modules/pluginModule.js | 8 +++++--- packages/kolibri/components/AuthMessage.vue | 8 ++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/kolibri/plugins/coach/assets/src/modules/pluginModule.js b/kolibri/plugins/coach/assets/src/modules/pluginModule.js index f94c127e1e7..a9788ec5790 100644 --- a/kolibri/plugins/coach/assets/src/modules/pluginModule.js +++ b/kolibri/plugins/coach/assets/src/modules/pluginModule.js @@ -131,9 +131,11 @@ export default { return Promise.all([ // Make sure we load any class list data, so that we know // whether this user has access to multiple classes or not. - store - .dispatch('classSummary/loadClassSummary', classId) - .then(summary => store.dispatch('setClassList', summary.facility_id)), + store.dispatch('classSummary/loadClassSummary', classId).then(summary => { + if (summary?.facility_id) { + store.dispatch('setClassList', summary?.facility_id); + } + }), store.dispatch('coachNotifications/fetchNotificationsForClass', classId), ]).catch(error => { store.dispatch('handleApiError', { error, reloadOnReconnect: true }); diff --git a/packages/kolibri/components/AuthMessage.vue b/packages/kolibri/components/AuthMessage.vue index 50cb83bf519..4df3ee5cd95 100644 --- a/packages/kolibri/components/AuthMessage.vue +++ b/packages/kolibri/components/AuthMessage.vue @@ -17,9 +17,9 @@ />

-

@@ -68,6 +68,10 @@ }, }, computed: { + ...mapGetters(['isUserLoggedIn']), + rootUrl() { + return urls['kolibri:core:redirect_user'](); + }, detailsText() { return this.details || this.$tr(this.authorizedRole); }, From d2acd3c50bda1cc7e5aeb36ec8922509c9cdcc29 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Fri, 25 Oct 2024 16:43:55 -0700 Subject: [PATCH 2/5] update authmessage test suite --- packages/kolibri/components/AuthMessage.vue | 2 ++ .../components/__tests__/auth-message.spec.js | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/kolibri/components/AuthMessage.vue b/packages/kolibri/components/AuthMessage.vue index 4df3ee5cd95..462e708fdfb 100644 --- a/packages/kolibri/components/AuthMessage.vue +++ b/packages/kolibri/components/AuthMessage.vue @@ -14,6 +14,7 @@ :text="linkText" :href="signInLink" appearance="basic-link" + data-test="signinlink" />

@@ -21,6 +22,7 @@ :text="$tr('goBackToHomeAction')" :href="rootUrl" appearance="basic-link" + data-test="gohomelink" />

diff --git a/packages/kolibri/components/__tests__/auth-message.spec.js b/packages/kolibri/components/__tests__/auth-message.spec.js index 2256abff5df..4c705dfd797 100644 --- a/packages/kolibri/components/__tests__/auth-message.spec.js +++ b/packages/kolibri/components/__tests__/auth-message.spec.js @@ -94,7 +94,7 @@ describe('auth message component', () => { it('shows correct link text if there is a user plugin', () => { const wrapper = makeWrapper(); - const link = wrapper.find('kexternallink-stub'); + const link = wrapper.find('[data-test=signinlink]'); expect(link.attributes()).toMatchObject({ href: 'http://localhost:8000/en/auth/#/signin?next=http%3A%2F%2Flocalhost%3A8000%2F%23%2Ftest_url', text: 'Sign in to Kolibri', @@ -104,7 +104,7 @@ describe('auth message component', () => { it('if the next param is in URL, it is what is used in the sign-in page link', () => { window.location.href = 'http://localhost:8000/#/some_other_url'; const wrapper = makeWrapper(); - const link = wrapper.find('kexternallink-stub'); + const link = wrapper.find('[data-test=signinlink]'); expect(link.attributes()).toMatchObject({ href: 'http://localhost:8000/en/auth/#/signin?next=http%3A%2F%2Flocalhost%3A8000%2F%23%2Fsome_other_url', text: 'Sign in to Kolibri', @@ -113,8 +113,17 @@ describe('auth message component', () => { }); it('shows correct link text if there is not a user plugin', () => { - const wrapper = makeWrapper(); - const link = wrapper.find('kexternallink-stub'); + // linkText checks to see if `userAuthPluginUrl` is truthy and it's either a + // function or undefined and if there is no user plugin, then it needs to be + // falsy for this test case + const wrapper = makeWrapper({ + computed: { + userAuthPluginUrl() { + return false; + }, + }, + }); + const link = wrapper.find('[data-test=signinlink]'); expect(link.attributes()).toMatchObject({ href: '/', text: 'Go to home page', @@ -124,6 +133,6 @@ describe('auth message component', () => { it('does not show a link if the user is logged in', () => { useUser.mockImplementation(() => useUserMock({ isUserLoggedIn: true })); const wrapper = makeWrapper(); - expect(wrapper.find('kexternallink-stub').exists()).toBe(false); + expect(wrapper.find('[data-test=signinlink]').exists()).toBe(false); }); }); From 37c2cc58b169fc3e1f29f1b21a2151a263c44c45 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Tue, 26 Nov 2024 16:08:38 -0800 Subject: [PATCH 3/5] AuthMessage updates fix post-rebase --- packages/kolibri/components/AuthMessage.vue | 1 - packages/kolibri/components/__tests__/auth-message.spec.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/kolibri/components/AuthMessage.vue b/packages/kolibri/components/AuthMessage.vue index 462e708fdfb..9e4c069b7f9 100644 --- a/packages/kolibri/components/AuthMessage.vue +++ b/packages/kolibri/components/AuthMessage.vue @@ -70,7 +70,6 @@ }, }, computed: { - ...mapGetters(['isUserLoggedIn']), rootUrl() { return urls['kolibri:core:redirect_user'](); }, diff --git a/packages/kolibri/components/__tests__/auth-message.spec.js b/packages/kolibri/components/__tests__/auth-message.spec.js index 4c705dfd797..285f09ec917 100644 --- a/packages/kolibri/components/__tests__/auth-message.spec.js +++ b/packages/kolibri/components/__tests__/auth-message.spec.js @@ -6,7 +6,7 @@ import { stubWindowLocation } from 'testUtils'; // eslint-disable-line import useUser, { useUserMock } from 'kolibri/composables/useUser'; // eslint-disable-line import AuthMessage from '../AuthMessage'; -jest.mock('kolibri/urls', () => ({})); +jest.mock('kolibri/urls'); jest.mock('kolibri/composables/useUser'); const localVue = createLocalVue(); From 3eaa320f8548f163b663338e930670d4e1bca4dd Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Wed, 27 Nov 2024 11:36:10 -0800 Subject: [PATCH 4/5] Revise CoachToolsModule's promise handling to keep things moving along even if an error occurs mid-navigation (for example, like in QuizSummary's beforeRouteEnter) --- kolibri/plugins/coach/assets/src/app.js | 13 ++++++++++--- .../src/views/quizzes/QuizSummaryPage/index.vue | 11 ++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/kolibri/plugins/coach/assets/src/app.js b/kolibri/plugins/coach/assets/src/app.js index fc0d1f84f4e..09617c3991d 100644 --- a/kolibri/plugins/coach/assets/src/app.js +++ b/kolibri/plugins/coach/assets/src/app.js @@ -139,9 +139,16 @@ class CoachToolsModule extends KolibriApp { } if (promises.length > 0) { - Promise.all(promises).then(next, error => { - this.store.dispatch('handleApiError', { error }); - }); + Promise.all(promises) + .catch(error => { + this.store.dispatch('handleApiError', { error }); + }) + .catch(() => { + // We catch here because `handleApiError` throws the error back again, in this case, + // we just want things to keep moving so that the AuthMessage shows as expected + next(); + }) + .then(next); } else { next(); } diff --git a/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue b/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue index bfe3a541018..338f73f2704 100644 --- a/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue +++ b/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue @@ -268,9 +268,14 @@ }, // @public setError(error) { - this.$store.dispatch('handleApiError', { error }); - this.loading = false; - this.$store.dispatch('notLoading'); + try { + this.$store.dispatch('handleApiError', { error }); + this.loading = false; + this.$store.dispatch('notLoading'); + } catch (e) { + // nothing to do here, just catching the error to avoid + // unhandled errors in the dispatch to handleApiError + } }, setCurrentAction(action) { if (action === 'EDIT_DETAILS') { From 8d398a9d7eb07d1256a659851489ddd40ae7a827 Mon Sep 17 00:00:00 2001 From: Jacob Pierce Date: Thu, 19 Dec 2024 10:55:31 -0800 Subject: [PATCH 5/5] Unset loading after trying to setError --- .../coach/assets/src/views/quizzes/QuizSummaryPage/index.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue b/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue index 338f73f2704..e71c1fc81b7 100644 --- a/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue +++ b/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue @@ -270,12 +270,12 @@ setError(error) { try { this.$store.dispatch('handleApiError', { error }); - this.loading = false; - this.$store.dispatch('notLoading'); } catch (e) { // nothing to do here, just catching the error to avoid // unhandled errors in the dispatch to handleApiError } + this.loading = false; + this.$store.dispatch('notLoading'); }, setCurrentAction(action) { if (action === 'EDIT_DETAILS') {