Skip to content

Commit

Permalink
fix(materials): display custom error page when file or folder is miss…
Browse files Browse the repository at this point in the history
…ing in course materials
  • Loading branch information
phungmanhcuong committed Oct 30, 2024
1 parent fe47f4d commit 23414e8
Show file tree
Hide file tree
Showing 14 changed files with 179 additions and 39 deletions.
11 changes: 10 additions & 1 deletion app/controllers/course/material/materials_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
# frozen_string_literal: true
class Course::Material::MaterialsController < Course::Material::Controller
load_and_authorize_resource :material, through: :folder, class: 'Course::Material'
load_and_authorize_resource :material, through: :folder, class: 'Course::Material', except: :load_default

def load_default
@folder = Course::Material::Folder.where(course_id: current_course.id, parent_id: nil).order(:created_at).first
if @folder
render json: @folder, status: :ok
else
render json: { error: 'No folders available' }, status: :not_found

Check warning on line 10 in app/controllers/course/material/materials_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/course/material/materials_controller.rb#L10

Added line #L10 was not covered by tests
end
end

def show
authorize!(:read_owner, @material.folder)
Expand Down
12 changes: 11 additions & 1 deletion client/app/api/course/Materials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { FileListData } from 'types/course/material/files';

import { APIResponse } from 'api/types';

import { FolderMiniEntity } from '../../types/course/material/folders';

import BaseCourseAPI from './Base';

const getShouldDownloadFromContentDisposition = (
Expand All @@ -15,8 +17,12 @@ const getShouldDownloadFromContentDisposition = (
};

export default class MaterialsAPI extends BaseCourseAPI {
get #materialPrefix(): string {
return `/courses/${this.courseId}/materials`;
}

get #urlPrefix(): string {
return `/courses/${this.courseId}/materials/folders`;
return `${this.#materialPrefix}/folders`;
}

fetch(folderId: number, materialId: number): APIResponse<FileListData> {
Expand All @@ -25,6 +31,10 @@ export default class MaterialsAPI extends BaseCourseAPI {
);
}

fetchDefault(): APIResponse<FolderMiniEntity> {
return this.client.get(`${this.#materialPrefix}`);
}

/**
* Attempts to download the file at the given `url` as a `Blob` and returns
* its URL and disposition. Remember to `revoke` the URL when no longer needed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import { Typography } from '@mui/material';

import Page from 'lib/components/core/layouts/Page';

interface BaseDownloadFilePageProps {
interface BaseRetrieveMaterialPageProps {
illustration: ReactNode;
title: string;
description: string;
children?: ReactNode;
}

const BaseDownloadFilePage = (
props: BaseDownloadFilePageProps,
const BaseRetrieveMaterialPage = (
props: BaseRetrieveMaterialPageProps,
): JSX.Element => (
<Page className="h-full m-auto flex flex-col items-center justify-center text-center">
{props.illustration}
Expand All @@ -32,4 +32,4 @@ const BaseDownloadFilePage = (
</Page>
);

export default BaseDownloadFilePage;
export default BaseRetrieveMaterialPage;
10 changes: 5 additions & 5 deletions client/app/bundles/course/material/files/DownloadingFilePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Link from 'lib/components/core/Link';
import useEffectOnce from 'lib/hooks/useEffectOnce';
import useTranslation from 'lib/hooks/useTranslation';

import BaseDownloadFilePage from './components/BaseDownloadFilePage';
import BaseRetrieveMaterialPage from '../component/BaseRetrieveMaterialPage';

const DEFAULT_FILE_NAME = 'file';

Expand Down Expand Up @@ -51,7 +51,7 @@ const SuccessDownloadingFilePage = (
const { t } = useTranslation();

return (
<BaseDownloadFilePage
<BaseRetrieveMaterialPage
description={t(translations.downloadingDescription)}
illustration={
<DownloadingOutlined className="text-[6rem]" color="success" />
Expand All @@ -61,7 +61,7 @@ const SuccessDownloadingFilePage = (
<Link className="mt-10" href={props.url}>
{t(translations.tryDownloadingAgain)}
</Link>
</BaseDownloadFilePage>
</BaseRetrieveMaterialPage>
);
};

Expand All @@ -71,7 +71,7 @@ const ErrorStartingDownloadFilePage = (
const { t } = useTranslation();

return (
<BaseDownloadFilePage
<BaseRetrieveMaterialPage
description={t(translations.clickToDownloadFileDescription)}
illustration={
<div className="relative">
Expand All @@ -93,7 +93,7 @@ const ErrorStartingDownloadFilePage = (
>
{props.name}
</Button>
</BaseDownloadFilePage>
</BaseRetrieveMaterialPage>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Cancel, InsertDriveFileOutlined } from '@mui/icons-material';
import Link from 'lib/components/core/Link';
import useTranslation from 'lib/hooks/useTranslation';

import BaseDownloadFilePage from './components/BaseDownloadFilePage';
import BaseRetrieveMaterialPage from '../component/BaseRetrieveMaterialPage';

const translations = defineMessages({
problemRetrievingFile: {
Expand All @@ -30,7 +30,7 @@ const ErrorRetrievingFilePage = (): JSX.Element => {
const workbinURL = `/courses/${params.courseId}/materials/folders/${params.folderId}`;

return (
<BaseDownloadFilePage
<BaseRetrieveMaterialPage
description={t(translations.problemRetrievingFileDescription)}
illustration={
<div className="relative">
Expand All @@ -47,7 +47,7 @@ const ErrorRetrievingFilePage = (): JSX.Element => {
<Link className="mt-10" to={workbinURL}>
{t(translations.goToTheWorkbin)}
</Link>
</BaseDownloadFilePage>
</BaseRetrieveMaterialPage>
);
};

Expand Down
15 changes: 15 additions & 0 deletions client/app/bundles/course/material/folderLoader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { LoaderFunction, redirect } from 'react-router-dom';
import { getIdFromUnknown } from 'utilities';

import CourseAPI from 'api/course';

const folderLoader: LoaderFunction = async ({ params }) => {
const folderId = getIdFromUnknown(params?.folderId);
if (!folderId) return redirect('/');

const { data } = await CourseAPI.folders.fetch(folderId);

return data;
};

export default folderLoader;
24 changes: 19 additions & 5 deletions client/app/bundles/course/material/folders/handles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,25 @@ import { getIdFromUnknown } from 'utilities';
import CourseAPI from 'api/course';
import { CrumbPath, DataHandle } from 'lib/hooks/router/dynamicNest';

export const loadDefaultMaterialId = async (): Promise<number> => {
const {
data: { id },
} = await CourseAPI.materials.fetchDefault();
return id;
};

const getFolderTitle = async (
courseUrl: string,
courseId: string,
folderId: number,
): Promise<CrumbPath> => {
const { data } = await CourseAPI.folders.fetch(folderId);
const courseUrl = `/courses/${courseId}`;
let data;
try {
({ data } = await CourseAPI.folders.fetch(folderId));
} catch (error) {
const defaultMaterialId = await loadDefaultMaterialId();
({ data } = await CourseAPI.folders.fetch(defaultMaterialId));
}

const workbinUrl = `${courseUrl}/materials/folders/${data.breadcrumbs[0].id}`;

Expand All @@ -32,13 +46,13 @@ const getFolderTitle = async (
* e.g., `useDynamicNest` cannot know if we move out from Folder 2 to Folder 1 from the URL.
*/
export const folderHandle: DataHandle = (match) => {
const courseId = match.params?.courseId;
const folderId = getIdFromUnknown(match.params?.folderId);
if (!courseId) throw new Error(`Invalid course id: ${courseId}`);
if (!folderId) throw new Error(`Invalid folder id: ${folderId}`);

const courseUrl = `/courses/${match.params.courseId}`;

return {
shouldRevalidate: true,
getData: () => getFolderTitle(courseUrl, folderId),
getData: () => getFolderTitle(courseId, folderId),
};
};
29 changes: 15 additions & 14 deletions client/app/bundles/course/material/folders/operations.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Operation } from 'store';
import {
FolderData,
FolderFormData,
MaterialFormData,
MaterialUploadFormData,
Expand Down Expand Up @@ -62,20 +63,20 @@ const formatMaterialAttributes = (data: MaterialFormData): FormData => {
return payload;
};

export function loadFolder(folderId: number): Operation<SaveFolderAction> {
return async (dispatch) =>
CourseAPI.folders.fetch(folderId).then((response) => {
const data = response.data;
return dispatch(
actions.saveFolder(
data.currFolderInfo,
data.subfolders,
data.materials,
data.advanceStartAt,
data.permissions,
),
);
});
export function dispatchFolderData(
data: FolderData,
): Operation<SaveFolderAction> {
return async (dispatch) => {
return dispatch(
actions.saveFolder(
data.currFolderInfo,
data.subfolders,
data.materials,
data.advanceStartAt,
data.permissions,
),
);
};
}

export function createFolder(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import React, { useEffect, useState } from 'react';
import { defineMessages } from 'react-intl';
import { useParams } from 'react-router-dom';
import { Cancel, FolderOutlined } from '@mui/icons-material';

import { loadDefaultMaterialId } from 'course/material/folders/handles';
import Link from 'lib/components/core/Link';
import useTranslation from 'lib/hooks/useTranslation';

import BaseRetrieveMaterialPage from '../../component/BaseRetrieveMaterialPage';

const translations = defineMessages({
problemRetrievingFolder: {
id: 'course.material.folders.ErrorRetrievingFolderPage.problemRetrievingFolder',
defaultMessage: 'Problem retrieving folder',
},
problemRetrievingFolderDescription: {
id: 'course.material.folders.ErrorRetrievingFolderPage.problemRetrievingFolderDescription',
defaultMessage:
"Either it no longer exists, you don't have the permission to access it, or something unexpected happened when we were trying to retrieve it.",
},
goToTheWorkbin: {
id: 'course.material.folders.ErrorRetrievingFolderPage.goToTheWorkbin',
defaultMessage: 'Go to the Workbin',
},
});

const Illustration = (): JSX.Element => (
<div className="relative">
<FolderOutlined className="text-[6rem]" color="disabled" />
<Cancel
className="absolute bottom-0 -right-2 text-[4rem] bg-white rounded-full"
color="error"
/>
</div>
);

const useWorkbinURL = (courseId: string | undefined): string => {
const [workbinURL, setWorkbinURL] = useState(`/courses/${courseId}`);

useEffect(() => {
if (courseId) {
loadDefaultMaterialId().then((defaultMaterialId) => {
setWorkbinURL(
`/courses/${courseId}/materials/folders/${defaultMaterialId}`,
);
});
}
}, [courseId]);

return workbinURL;
};

const ErrorRetrievingFolderPage = (): JSX.Element => {
const { t } = useTranslation();
const params = useParams();
const workbinURL = useWorkbinURL(params.courseId);

return (
<BaseRetrieveMaterialPage
description={t(translations.problemRetrievingFolderDescription)}
illustration={<Illustration />}
title={t(translations.problemRetrievingFolder)}
>
<Link className="mt-10" to={workbinURL}>
{t(translations.goToTheWorkbin)}
</Link>
</BaseRetrieveMaterialPage>
);
};

export default ErrorRetrievingFolderPage;
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FC, ReactElement, useEffect, useState } from 'react';
import { defineMessages } from 'react-intl';
import { useParams } from 'react-router-dom';
import { useLoaderData, useParams } from 'react-router-dom';

import EditButton from 'lib/components/core/buttons/EditButton';
import Page from 'lib/components/core/layouts/Page';
Expand All @@ -10,12 +10,13 @@ import { getCourseId } from 'lib/helpers/url-helpers';
import { useAppDispatch, useAppSelector } from 'lib/hooks/store';
import useTranslation from 'lib/hooks/useTranslation';

import { FolderData } from '../../../../../../types/course/material/folders';
import DownloadFolderButton from '../../components/buttons/DownloadFolderButton';
import NewSubfolderButton from '../../components/buttons/NewSubfolderButton';
import UploadFilesButton from '../../components/buttons/UploadFilesButton';
import MaterialUpload from '../../components/misc/MaterialUpload';
import WorkbinTable from '../../components/tables/WorkbinTable';
import { loadFolder } from '../../operations';
import { dispatchFolderData } from '../../operations';
import {
getCurrFolderInfo,
getFolderMaterials,
Expand Down Expand Up @@ -48,13 +49,16 @@ const FolderShow: FC = () => {
const materials = useAppSelector(getFolderMaterials);
const currFolderInfo = useAppSelector(getCurrFolderInfo);
const permissions = useAppSelector(getFolderPermissions);
const loaderData = useLoaderData() as FolderData;

const [isLoading, setIsLoading] = useState(true);
useEffect(() => {
if (folderId) {
dispatch(loadFolder(+folderId)).finally(() => setIsLoading(false));
if (loaderData) {
dispatch(dispatchFolderData(loaderData)).finally(() =>
setIsLoading(false),
);
}
}, [dispatch, folderId]);
}, [dispatch, loaderData]);

if (isLoading) {
return <LoadingIndicator />;
Expand Down
4 changes: 4 additions & 0 deletions client/app/routers/AuthenticatedApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import LessonPlanShow from 'bundles/course/lesson-plan/pages/LessonPlanShow';
import LevelsIndex from 'bundles/course/level/pages/LevelsIndex';
import DownloadingFilePage from 'bundles/course/material/files/DownloadingFilePage';
import ErrorRetrievingFilePage from 'bundles/course/material/files/ErrorRetrievingFilePage';
import ErrorRetrievingFolderPage from 'bundles/course/material/folders/pages/ErrorRetrievingFolderPage';
import FolderShow from 'bundles/course/material/folders/pages/FolderShow';
import TimelineDesigner from 'bundles/course/reference-timelines/TimelineDesigner';
import ResponseEdit from 'bundles/course/survey/pages/ResponseEdit';
Expand Down Expand Up @@ -127,6 +128,7 @@ import {
forumTopicHandle,
} from 'course/forum/handles';
import { leaderboardHandle } from 'course/leaderboard/handles';
import folderLoader from 'course/material/folderLoader';
import { folderHandle } from 'course/material/folders/handles';
import materialLoader from 'course/material/materialLoader';
import { videoWatchHistoryHandle } from 'course/statistics/handles';
Expand Down Expand Up @@ -221,7 +223,9 @@ const authenticatedRouter: Translated<RouteObject[]> = (t) =>
children: [
{
index: true,
loader: folderLoader,
element: <FolderShow />,
errorElement: <ErrorRetrievingFolderPage />,
},
{
path: 'files/:materialId',
Expand Down
Loading

0 comments on commit 23414e8

Please sign in to comment.