feat: Adding human readable 403 error access restricted (#1569)

Updated to have human-readable forbidden error (403)
openedx · Jan 10, 2025 · e6bce56 · e6bce56
1 parent 811be22
commit e6bce56
Show file tree

Hide file tree

Showing 8 changed files with 173 additions and 64 deletions.
diff --git a/src/course-outline/constants.js b/src/course-outline/constants.js
@@ -87,4 +87,5 @@ export const API_ERROR_TYPES = /** @type {const} */ ({
   networkError: 'networkError',
   serverError: 'serverError',
   unknown: 'unknown',
+  forbidden: 'forbidden',
 });
diff --git a/src/course-outline/data/slice.js b/src/course-outline/data/slice.js
@@ -104,7 +104,7 @@ const slice = createSlice({
         ...payload,
       };
     },
-    fetchStatusBarSelPacedSuccess: (state, { payload }) => {
+    fetchStatusBarSelfPacedSuccess: (state, { payload }) => {
       state.statusBarData.isSelfPaced = payload.isSelfPaced;
     },
     updateSavingStatus: (state, { payload }) => {
@@ -206,7 +206,7 @@ export const {
   updateStatusBar,
   updateCourseActions,
   fetchStatusBarChecklistSuccess,
-  fetchStatusBarSelPacedSuccess,
+  fetchStatusBarSelfPacedSuccess,
   updateFetchSectionLoadingStatus,
   updateCourseLaunchQueryStatus,
   updateSavingStatus,

diff --git a/src/course-outline/data/thunk.js b/src/course-outline/data/thunk.js
@@ -1,7 +1,7 @@
 import { RequestStatus } from '../../data/constants';
 import { updateClipboardData } from '../../generic/data/slice';
 import { NOTIFICATION_MESSAGES } from '../../constants';
-import { API_ERROR_TYPES, COURSE_BLOCK_NAMES } from '../constants';
+import { COURSE_BLOCK_NAMES } from '../constants';
 import {
   hideProcessingNotification,
   showProcessingNotification,
@@ -10,6 +10,7 @@ import {
   getCourseBestPracticesChecklist,
   getCourseLaunchChecklist,
 } from '../utils/getChecklistForStatusBar';
+import { getErrorDetails } from '../utils/getErrorDetails';
 import {
   addNewCourseItem,
   deleteCourseItem,
@@ -41,7 +42,7 @@ import {
   updateStatusBar,
   updateCourseActions,
   fetchStatusBarChecklistSuccess,
-  fetchStatusBarSelPacedSuccess,
+  fetchStatusBarSelfPacedSuccess,
   updateSavingStatus,
   updateSectionList,
   updateFetchSectionLoadingStatus,
@@ -54,24 +55,6 @@ import {
   updateCourseLaunchQueryStatus,
 } from './slice';
 
-const getErrorDetails = (error, dismissible = true) => {
-  const errorInfo = { dismissible };
-  if (error.response?.data) {
-    const { data } = error.response;
-    if ((typeof data === 'string' && !data.includes('</html>')) || typeof data === 'object') {
-      errorInfo.data = JSON.stringify(data);
-    }
-    errorInfo.status = error.response.status;
-    errorInfo.type = API_ERROR_TYPES.serverError;
-  } else if (error.request) {
-    errorInfo.type = API_ERROR_TYPES.networkError;
-  } else {
-    errorInfo.type = API_ERROR_TYPES.unknown;
-    errorInfo.data = error.message;
-  }
-  return errorInfo;
-};
-
 export function fetchCourseOutlineIndexQuery(courseId) {
   return async (dispatch) => {
     dispatch(updateOutlineIndexLoadingStatus({ status: RequestStatus.IN_PROGRESS }));
@@ -125,7 +108,7 @@ export function fetchCourseLaunchQuery({
       const data = await getCourseLaunch({
         courseId, gradedOnly, validateOras, all,
       });
-      dispatch(fetchStatusBarSelPacedSuccess({ isSelfPaced: data.isSelfPaced }));
+      dispatch(fetchStatusBarSelfPacedSuccess({ isSelfPaced: data.isSelfPaced }));
       dispatch(fetchStatusBarChecklistSuccess(getCourseLaunchChecklist(data)));
 
       dispatch(updateCourseLaunchQueryStatus({ status: RequestStatus.SUCCESSFUL }));

diff --git a/src/course-outline/page-alerts/PageAlerts.jsx b/src/course-outline/page-alerts/PageAlerts.jsx
@@ -343,13 +343,38 @@ const PageAlerts = ({
   const renderApiErrors = () => {
     let errorList = Object.entries(errors).filter(obj => obj[1] !== null).map(([k, v]) => {
       switch (v.type) {
-        case API_ERROR_TYPES.serverError:
+        case API_ERROR_TYPES.forbidden: {
+          const description = intl.formatMessage(messages.forbiddenAlertBody, {
+            LMS: (
+              <Hyperlink
+                destination={`${getConfig().LMS_BASE_URL}`}
+                target="_blank"
+                showLaunchIcon={false}
+              >
+                {intl.formatMessage(messages.forbiddenAlertLmsUrl)}
+              </Hyperlink>
+            ),
+          });
           return {
             key: k,
-            desc: v.data || intl.formatMessage(messages.serverErrorAlertBody),
+            desc: description,
+            title: intl.formatMessage(messages.forbiddenAlert),
+            dismissible: v.dismissible,
+          };
+        }
+        case API_ERROR_TYPES.serverError: {
+          const description = (
+            <Truncate lines={2}>
+              {v.data || intl.formatMessage(messages.serverErrorAlertBody)}
+            </Truncate>
+          );
+          return {
+            key: k,
+            desc: description,
             title: intl.formatMessage(messages.serverErrorAlert),
             dismissible: v.dismissible,
           };
+        }
         case API_ERROR_TYPES.networkError:
           return {
             key: k,
@@ -378,7 +403,7 @@ const PageAlerts = ({
             dismissError={() => dispatch(dismissError(msgObj.key))}
           >
             <Alert.Heading>{msgObj.title}</Alert.Heading>
-            {msgObj.desc && <Truncate lines={2}>{msgObj.desc}</Truncate>}
+            {msgObj.desc}
           </ErrorAlert>
         ) : (
           <Alert
@@ -387,7 +412,7 @@ const PageAlerts = ({
             key={msgObj.key}
           >
             <Alert.Heading>{msgObj.title}</Alert.Heading>
-            {msgObj.desc && <Truncate lines={2}>{msgObj.desc}</Truncate>}
+            {msgObj.desc}
           </Alert>
         )
       ))

diff --git a/src/course-outline/page-alerts/PageAlerts.test.jsx b/src/course-outline/page-alerts/PageAlerts.test.jsx
@@ -71,19 +71,19 @@ describe('<PageAlerts />', () => {
   });
 
   it('renders null when no alerts are present', () => {
-    const { queryByTestId } = renderComponent();
-    expect(queryByTestId('browser-router')).toBeEmptyDOMElement();
+    renderComponent();
+    expect(screen.queryByTestId('browser-router')).toBeEmptyDOMElement();
   });
 
   it('renders configuration alerts', async () => {
-    const { queryByText } = renderComponent({
+    renderComponent({
       ...pageAlertsData,
       notificationDismissUrl: 'some-url',
       handleDismissNotification,
     });
 
-    expect(queryByText(messages.configurationErrorTitle.defaultMessage)).toBeInTheDocument();
-    const dismissBtn = queryByText('Dismiss');
+    expect(screen.queryByText(messages.configurationErrorTitle.defaultMessage)).toBeInTheDocument();
+    const dismissBtn = screen.queryByText('Dismiss');
     await act(async () => fireEvent.click(dismissBtn));
 
     expect(handleDismissNotification).toBeCalled();
@@ -117,7 +117,7 @@ describe('<PageAlerts />', () => {
   });
 
   it('renders deprecation warning alerts', async () => {
-    const { queryByText } = renderComponent({
+    renderComponent({
       ...pageAlertsData,
       deprecatedBlocksInfo: {
         blocks: [['url1', 'block1'], ['url2']],
@@ -126,20 +126,20 @@ describe('<PageAlerts />', () => {
       },
     });
 
-    expect(queryByText(messages.deprecationWarningTitle.defaultMessage)).toBeInTheDocument();
-    expect(queryByText(messages.deprecationWarningBlocksText.defaultMessage)).toBeInTheDocument();
-    expect(queryByText('block1')).toHaveAttribute('href', 'url1');
-    expect(queryByText(messages.deprecatedComponentName.defaultMessage)).toHaveAttribute('href', 'url2');
+    expect(screen.queryByText(messages.deprecationWarningTitle.defaultMessage)).toBeInTheDocument();
+    expect(screen.queryByText(messages.deprecationWarningBlocksText.defaultMessage)).toBeInTheDocument();
+    expect(screen.queryByText('block1')).toHaveAttribute('href', 'url1');
+    expect(screen.queryByText(messages.deprecatedComponentName.defaultMessage)).toHaveAttribute('href', 'url2');
 
-    const feedbackLink = queryByText(messages.advancedSettingLinkText.defaultMessage);
+    const feedbackLink = screen.queryByText(messages.advancedSettingLinkText.defaultMessage);
     expect(feedbackLink).toBeInTheDocument();
     expect(feedbackLink).toHaveAttribute('href', `${getConfig().STUDIO_BASE_URL}/some-url`);
-    expect(queryByText('lti')).toBeInTheDocument();
-    expect(queryByText('video')).toBeInTheDocument();
+    expect(screen.queryByText('lti')).toBeInTheDocument();
+    expect(screen.queryByText('video')).toBeInTheDocument();
   });
 
   it('renders proctoring alerts with mfe settings link', async () => {
-    const { queryByText } = renderComponent({
+    renderComponent({
       ...pageAlertsData,
       mfeProctoredExamSettingsUrl: 'mfe-url',
       proctoringErrors: [
@@ -148,15 +148,15 @@ describe('<PageAlerts />', () => {
       ],
     });
 
-    expect(queryByText('error 1')).toBeInTheDocument();
-    expect(queryByText('error 2')).toBeInTheDocument();
-    expect(queryByText('message 1')).toBeInTheDocument();
-    expect(queryByText('message 2')).toBeInTheDocument();
-    expect(queryByText(messages.proctoredSettingsLinkText.defaultMessage)).toHaveAttribute('href', 'mfe-url');
+    expect(screen.queryByText('error 1')).toBeInTheDocument();
+    expect(screen.queryByText('error 2')).toBeInTheDocument();
+    expect(screen.queryByText('message 1')).toBeInTheDocument();
+    expect(screen.queryByText('message 2')).toBeInTheDocument();
+    expect(screen.queryByText(messages.proctoredSettingsLinkText.defaultMessage)).toHaveAttribute('href', 'mfe-url');
   });
 
   it('renders proctoring alerts without mfe settings link', async () => {
-    const { queryByText } = renderComponent({
+    renderComponent({
       ...pageAlertsData,
       advanceSettingsUrl: '/some-url',
       proctoringErrors: [
@@ -165,11 +165,11 @@ describe('<PageAlerts />', () => {
       ],
     });
 
-    expect(queryByText('error 1')).toBeInTheDocument();
-    expect(queryByText('error 2')).toBeInTheDocument();
-    expect(queryByText('message 1')).toBeInTheDocument();
-    expect(queryByText('message 2')).toBeInTheDocument();
-    expect(queryByText(messages.advancedSettingLinkText.defaultMessage)).toHaveAttribute(
+    expect(screen.queryByText('error 1')).toBeInTheDocument();
+    expect(screen.queryByText('error 2')).toBeInTheDocument();
+    expect(screen.queryByText('message 1')).toBeInTheDocument();
+    expect(screen.queryByText('message 2')).toBeInTheDocument();
+    expect(screen.queryByText(messages.advancedSettingLinkText.defaultMessage)).toHaveAttribute(
       'href',
       `${getConfig().STUDIO_BASE_URL}/some-url`,
     );
@@ -181,10 +181,10 @@ describe('<PageAlerts />', () => {
       conflictingFiles: [],
       errorFiles: ['error.css'],
     });
-    const { queryByText } = renderComponent();
-    expect(queryByText(messages.newFileAlertTitle.defaultMessage)).toBeInTheDocument();
-    expect(queryByText(messages.errorFileAlertTitle.defaultMessage)).toBeInTheDocument();
-    expect(queryByText(messages.newFileAlertAction.defaultMessage)).toHaveAttribute(
+    renderComponent();
+    expect(screen.queryByText(messages.newFileAlertTitle.defaultMessage)).toBeInTheDocument();
+    expect(screen.queryByText(messages.errorFileAlertTitle.defaultMessage)).toBeInTheDocument();
+    expect(screen.queryByText(messages.newFileAlertAction.defaultMessage)).toHaveAttribute(
       'href',
       `${getConfig().STUDIO_BASE_URL}/assets/course-id`,
     );
@@ -196,26 +196,51 @@ describe('<PageAlerts />', () => {
       conflictingFiles: ['some.css', 'some.js'],
       errorFiles: [],
     });
-    const { queryByText } = renderComponent();
-    expect(queryByText(messages.conflictingFileAlertTitle.defaultMessage)).toBeInTheDocument();
-    expect(queryByText(messages.newFileAlertAction.defaultMessage)).toHaveAttribute(
+    renderComponent();
+    expect(screen.queryByText(messages.conflictingFileAlertTitle.defaultMessage)).toBeInTheDocument();
+    expect(screen.queryByText(messages.newFileAlertAction.defaultMessage)).toHaveAttribute(
       'href',
       `${getConfig().STUDIO_BASE_URL}/assets/course-id`,
     );
   });
 
   it('renders api error alerts', async () => {
-    const { queryByText } = renderComponent({
+    renderComponent({
       ...pageAlertsData,
       errors: {
         outlineIndexApi: { data: 'some error', status: 400, type: API_ERROR_TYPES.serverError },
         courseLaunchApi: { type: API_ERROR_TYPES.networkError },
         reindexApi: { type: API_ERROR_TYPES.unknown, data: 'some unknown error' },
       },
     });
-    expect(queryByText(messages.networkErrorAlert.defaultMessage)).toBeInTheDocument();
-    expect(queryByText(messages.serverErrorAlert.defaultMessage)).toBeInTheDocument();
-    expect(queryByText('some error')).toBeInTheDocument();
-    expect(queryByText('some unknown error')).toBeInTheDocument();
+    expect(screen.queryByText(messages.networkErrorAlert.defaultMessage)).toBeInTheDocument();
+    expect(screen.queryByText(messages.serverErrorAlert.defaultMessage)).toBeInTheDocument();
+    expect(screen.queryByText('some error')).toBeInTheDocument();
+    expect(screen.queryByText('some unknown error')).toBeInTheDocument();
+  });
+
+  it('renders forbidden api error alerts', async () => {
+    renderComponent({
+      ...pageAlertsData,
+      errors: {
+        outlineIndexApi: {
+          data: 'some error', status: 403, type: API_ERROR_TYPES.forbidden, dismissable: false,
+        },
+      },
+    });
+    expect(screen.queryByText(messages.forbiddenAlert.defaultMessage)).toBeInTheDocument();
+    expect(screen.queryByText(messages.forbiddenAlertBody.defaultMessage)).toBeInTheDocument();
+  });
+
+  it('renders api error alerts when status is not 403', async () => {
+    renderComponent({
+      ...pageAlertsData,
+      errors: {
+        outlineIndexApi: {
+          data: 'some error', status: 500, type: API_ERROR_TYPES.serverError, dismissable: true,
+        },
+      },
+    });
+    expect(screen.queryByText('some error')).toBeInTheDocument();
   });
 });
diff --git a/src/course-outline/page-alerts/messages.js b/src/course-outline/page-alerts/messages.js
@@ -121,6 +121,21 @@ const messages = defineMessages({
     defaultMessage: 'Network error',
     description: 'Generic network error alert.',
   },
+  forbiddenAlert: {
+    id: 'course-authoring.course-outline.page-alert.forbidden.title',
+    defaultMessage: 'Access Restricted',
+    description: 'Forbidden(403) alert title',
+  },
+  forbiddenAlertBody: {
+    id: 'course-authoring.course-outline.page-alert.forbidden.body',
+    defaultMessage: 'It looks like you’re trying to access a page you don’t have permission to view. Contact your admin if you think this is a mistake, or head back to the {LMS}.',
+    description: 'Forbidden(403) alert body',
+  },
+  forbiddenAlertLmsUrl: {
+    id: 'course-authoring.course-outline.page-alert.lms',
+    defaultMessage: 'LMS',
+    description: 'LMS base redirection url',
+  },
 });
 
 export default messages;
diff --git a/src/course-outline/utils/getErrorDetails.js b/src/course-outline/utils/getErrorDetails.js
@@ -0,0 +1,24 @@
+import { API_ERROR_TYPES } from '../constants';
+
+export const getErrorDetails = (error, dismissible = true) => {
+  const errorInfo = { dismissible };
+  if (error.response?.status === 403) {
+    // For 403 status the error shouldn't be dismissible
+    errorInfo.dismissible = false;
+    errorInfo.type = API_ERROR_TYPES.forbidden;
+    errorInfo.status = error.response.status;
+  } else if (error.response?.data) {
+    const { data } = error.response;
+    if ((typeof data === 'string' && !data.includes('</html>')) || typeof data === 'object') {
+      errorInfo.data = JSON.stringify(data);
+    }
+    errorInfo.status = error.response.status;
+    errorInfo.type = API_ERROR_TYPES.serverError;
+  } else if (error.request) {
+    errorInfo.type = API_ERROR_TYPES.networkError;
+  } else {
+    errorInfo.type = API_ERROR_TYPES.unknown;
+    errorInfo.data = error.message;
+  }
+  return errorInfo;
+};