Merge pull request #12755 from nucleogenesis/0.17--quiz-routing-auth-…

…error Redirect user when loading class summary results in 403
learningequality · Dec 19, 2024 · f27d1ac · f27d1ac
2 parents af1eedb + 8d398a9
commit f27d1ac
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 15 deletions.
diff --git 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/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/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue b/kolibri/plugins/coach/assets/src/views/quizzes/QuizSummaryPage/index.vue
@@ -267,7 +267,12 @@
        * @public
        */
       setError(error) {
-        this.$store.dispatch('handleApiError', { error });
+        try {
+          this.$store.dispatch('handleApiError', { error });
+        } 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');
       },

diff --git a/packages/kolibri/components/AuthMessage.vue b/packages/kolibri/components/AuthMessage.vue
@@ -14,13 +14,15 @@
         :text="linkText"
         :href="signInLink"
         appearance="basic-link"
+        data-test="signinlink"
       />
     </p>
     <p v-else>
-      <KRouterLink
+      <KExternalLink
         :text="$tr('goBackToHomeAction')"
-        :to="{ path: '/' }"
+        :href="rootUrl"
         appearance="basic-link"
+        data-test="gohomelink"
       />
     </p>
   </div>
@@ -68,6 +70,9 @@
       },
     },
     computed: {
+      rootUrl() {
+        return urls['kolibri:core:redirect_user']();
+      },
       detailsText() {
         return this.details || this.$tr(this.authorizedRole);
       },

diff --git 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();
@@ -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);
   });
 });