Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PADV-1443 fix encode and decode url names #101

Merged
merged 1 commit into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/features/Classes/Class/ClassPage/Actions.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import EnrollStudent from 'features/Classes/EnrollStudent';
const Actions = ({ previousPage }) => {
const location = useLocation();
const history = useHistory();
const { courseId, classId } = useParams();
const { courseName, className } = useParams();
const classes = useSelector((state) => state.classes.allClasses.data);

const queryParams = new URLSearchParams(location.search);
Expand All @@ -35,7 +35,7 @@ const Actions = ({ previousPage }) => {
const handleEnrollStudentModal = () => setIsEnrollModalOpen(!isEnrollModalOpen);

const handleManageButton = () => {
history.push(`/manage-instructors/${courseId}/${classId}?classId=${queryClassId}&previous=${previousPage}`);
history.push(`/manage-instructors/${courseName}/${className}?classId=${queryClassId}&previous=${previousPage}`);
};

return (
Expand All @@ -49,7 +49,7 @@ const Actions = ({ previousPage }) => {
</Button>
<Button
as="a"
onClick={() => setAssignStaffRole(classLink, classId)}
onClick={() => setAssignStaffRole(classLink, queryClassId)}
className="text-decoration-none text-white button-view-class mr-3"
>
<i className="fa-solid fa-arrow-up-right-from-square mr-2 mb-1" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('columns', () => {

const { getByText } = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201/test%20ccx1?classId=ccx-v1:demo+demo1+2020+ccx@3']}>
<Route path="/courses/:courseId/:classId">
<Route path="/courses/:courseName/:className">
<StudentColumn />
</Route>
</MemoryRouter>,
Expand Down Expand Up @@ -138,7 +138,7 @@ describe('columns', () => {

const { getByText } = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201/test%20ccx1?classId=ccx-v1:demo+demo1+2020+ccx@3']}>
<Route path="/courses/:courseId/:classId">
<Route path="/courses/:courseName/:className">
<StatusColumn />
</Route>
</MemoryRouter>,
Expand Down Expand Up @@ -193,7 +193,7 @@ describe('columns', () => {

const { getByText } = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201/test%20ccx1?classId=ccx-v1:demo+demo1+2020+ccx@3']}>
<Route path="/courses/:courseId/:classId">
<Route path="/courses/:courseName/:className">
<ExamColumn />
</Route>
</MemoryRouter>,
Expand Down
4 changes: 2 additions & 2 deletions src/features/Classes/Class/ClassPage/__test__/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('ClassesPage', () => {
test('renders classes data and pagination', async () => {
const component = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201/test%20ccx1?classId=ccx-v1:demo+demo1+2020+ccx@3']}>
<Route path="/courses/:courseId/:classId">
<Route path="/courses/:courseName/:className">
<ClassPage />
</Route>
</MemoryRouter>,
Expand All @@ -72,7 +72,7 @@ describe('ClassesPage', () => {
test('renders actions', async () => {
const { getByText, getByTestId } = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201/test%20ccx1?classId=ccx-v1:demo+demo1+2020+ccx@3']}>
<Route path="/courses/:courseId/:classId">
<Route path="/courses/:courseName/:className">
<ClassPage />
</Route>
</MemoryRouter>,
Expand Down
20 changes: 11 additions & 9 deletions src/features/Classes/Class/ClassPage/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ const ClassPage = () => {
const location = useLocation();
const history = useHistory();
const dispatch = useDispatch();
const { courseId, classId } = useParams();
const { courseName, className } = useParams();
const queryParams = new URLSearchParams(location.search);
const previousPage = queryParams.get('previous') || 'classes';
const courseNameDecoded = decodeURIComponent(courseName);
const classNameDecoded = decodeURIComponent(className);

const institutionRef = useRef(undefined);
const [currentPage, setCurrentPage] = useState(initialPage);
Expand All @@ -42,20 +44,20 @@ const ClassPage = () => {
useEffect(() => {
const initialTitle = document.title;

document.title = classId;
document.title = classNameDecoded;
// Leaves a gap time space to prevent being override by ActiveTabUpdater component
setTimeout(() => dispatch(updateActiveTab(previousPage)), 100);

return () => {
document.title = initialTitle;
};
}, [dispatch, classId, previousPage]);
}, [dispatch, classNameDecoded, previousPage]);

useEffect(() => {
if (institution.id) {
const params = {
course_name: courseId,
class_name: classId,
course_name: courseNameDecoded,
class_name: classNameDecoded,
limit: true,
};

Expand All @@ -66,18 +68,18 @@ const ClassPage = () => {
dispatch(resetStudentsTable());
dispatch(updateCurrentPage(initialPage));
};
}, [dispatch, institution.id, courseId, classId, currentPage]);
}, [dispatch, institution.id, courseNameDecoded, classNameDecoded, currentPage]);

useEffect(() => {
if (institution.id) {
dispatch(fetchAllClassesData(institution.id, courseId));
dispatch(fetchAllClassesData(institution.id, courseNameDecoded));
}

return () => {
dispatch(resetClassesTable());
dispatch(resetClasses());
};
}, [dispatch, institution.id, courseId]);
}, [dispatch, institution.id, courseNameDecoded]);

useEffect(() => {
if (institution.id !== undefined && institutionRef.current === undefined) {
Expand All @@ -96,7 +98,7 @@ const ClassPage = () => {
<Button onClick={() => history.goBack()} className="mr-3 link back-arrow" variant="tertiary">
<i className="fa-solid fa-arrow-left" />
</Button>
<h3 className="h2 mb-0 course-title">Class details: {classId}</h3>
<h3 className="h2 mb-0 course-title">Class details: {classNameDecoded}</h3>
</div>
</div>

Expand Down
4 changes: 2 additions & 2 deletions src/features/Classes/ClassesTable/columns.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const columns = [
accessor: 'className',
Cell: ({ row }) => (
<Link
to={`/courses/${row.values.masterCourseName}/${row.values.className}?classId=${row.original.classId}&previous=classes`}
to={`/courses/${encodeURIComponent(row.original.masterCourseName)}/${encodeURIComponent(row.values.className)}?classId=${row.original.classId}&previous=classes`}
className="text-truncate link"
>
{row.values.className}
Expand Down Expand Up @@ -125,7 +125,7 @@ const columns = [
</Dropdown.Item>
<Dropdown.Item>
<Link
to={`/manage-instructors/${masterCourseName}/${row.values.className}?classId=${classId}&previous=classes`}
to={`/manage-instructors/${encodeURIComponent(masterCourseName)}/${encodeURIComponent(row.values.className)}?classId=${classId}&previous=classes`}
className="text-truncate text-decoration-none custom-text-black"
>
<i className="fa-regular fa-chalkboard-user mr-2 mb-1" />
Expand Down
2 changes: 1 addition & 1 deletion src/features/Classes/EnrollStudent/__test__/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import '@testing-library/jest-dom/extend-expect';
import EnrollStudent from 'features/Classes/EnrollStudent';

jest.mock('react-router-dom', () => ({
useParams: jest.fn(() => ({ courseId: 'Demo course', classId: 'demo class' })),
useParams: jest.fn(() => ({ courseName: 'Demo course', className: 'demo class' })),
useLocation: jest.fn().mockReturnValue({ search: '?classId=demo class' }),
}));

Expand Down
12 changes: 7 additions & 5 deletions src/features/Classes/EnrollStudent/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ const successToastMessage = 'Email invite has been sent successfully';
const EnrollStudent = ({ isOpen, onClose, queryClassId }) => {
const dispatch = useDispatch();

const { courseId, classId } = useParams();
const { courseName, className } = useParams();
const [showToast, setShowToast] = useState(false);
const [isLoading, setLoading] = useState(false);
const institution = useSelector((state) => state.main.selectedInstitution);
const courseNameDecoded = decodeURIComponent(courseName);
const classNameDecoded = decodeURIComponent(className);

const handleEnrollStudent = async (e) => {
e.preventDefault();
Expand All @@ -48,15 +50,15 @@ const EnrollStudent = ({ isOpen, onClose, queryClassId }) => {
await handleEnrollments(formData, queryClassId);

const params = {
course_name: courseId,
class_name: classId,
course_name: courseNameDecoded,
class_name: classNameDecoded,
limit: true,
};

dispatch(fetchStudentsData(institution.id, initialPage, params));

// Get the classes info updated with the new number of students enrolled.
dispatch(fetchAllClassesData(institution.id, courseId));
dispatch(fetchAllClassesData(institution.id, courseNameDecoded));
setShowToast(true);
onClose();
} catch (error) {
Expand All @@ -82,7 +84,7 @@ const EnrollStudent = ({ isOpen, onClose, queryClassId }) => {
</ModalDialog.Header>
<ModalDialog.Body className="body-container h-100">
<p className="text-uppercase font-weight-bold sub-title">
Class: {classId}
Class: {classNameDecoded}
</p>
{isLoading && (
<div className="w-100 h-100 d-flex justify-content-center align-items-center">
Expand Down
4 changes: 2 additions & 2 deletions src/features/Classes/InstructorCard/__test__/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import InstructorCard from 'features/Classes/InstructorCard';

jest.mock('react-router-dom', () => ({
useParams: jest.fn(() => ({
courseId: 'Demo course',
classId: 'demo class',
courseName: 'Demo course',
className: 'demo class',
})),
useLocation: jest.fn().mockReturnValue({ search: '?classId=demo+class' }),
}));
Expand Down
12 changes: 7 additions & 5 deletions src/features/Classes/InstructorCard/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ const INSTRUCTORS_NUMBER = 3;
const InstructorCard = () => {
const location = useLocation();
const dispatch = useDispatch();
const { courseId, classId } = useParams();
const { courseName, className } = useParams();
const institution = useSelector((state) => state.main.selectedInstitution);
const instructors = useSelector((state) => state.instructors.selectOptions.data);
const classes = useSelector((state) => state.classes.allClasses);
const courseNameDecoded = decodeURIComponent(courseName);
const classNameDecoded = decodeURIComponent(className);

const queryParams = new URLSearchParams(location.search);
const classIdQuery = queryParams.get('classId')?.replaceAll(' ', '+');
Expand All @@ -44,8 +46,8 @@ const InstructorCard = () => {
return (
<article className="instructor-wrapper mb-4 d-flex flex-column flex-sm-row justify-content-between align-items-start">
<div className="d-flex flex-column w-75 justify-content-between h-100">
<h3 className="text-color text-uppercase font-weight-bold text-truncate w-75" title={classId}>
{classId}
<h3 className="text-color text-uppercase font-weight-bold text-truncate w-75" title={classNameDecoded}>
{classNameDecoded}
</h3>
{isLoadingClasses && (
<div className="w-100 h-100 d-flex justify-content-center align-items-center">
Expand All @@ -58,8 +60,8 @@ const InstructorCard = () => {
)}
{!isLoadingClasses && (
<>
<h4 className="text-color text-uppercase font-weight-bold text-truncate w-75" title={courseId}>
{courseId}
<h4 className="text-color text-uppercase font-weight-bold text-truncate w-75" title={courseNameDecoded}>
{courseNameDecoded}
</h4>
<div className="text-uppercase">
<i className="fa-regular fa-calendar mr-2" />
Expand Down
4 changes: 2 additions & 2 deletions src/features/Courses/AddClass/_test_/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Add class modal', () => {
test('render add class modal', () => {
const { getByText, getByPlaceholderText } = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201']}>
<Route path="/courses/:courseId">
<Route path="/courses/:courseName">
<AddClass isOpen onClose={() => { }} courseInfo={courseInfoMocked} />
</Route>
</MemoryRouter>,
Expand Down Expand Up @@ -68,7 +68,7 @@ describe('Add class modal', () => {
test('cancel button in add classmodal', () => {
const { getByText, getByPlaceholderText } = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201']}>
<Route path="/courses/:courseId">
<Route path="/courses/:courseName">
<AddClass isOpen onClose={() => { }} courseInfo={courseInfoMocked} />
</Route>
</MemoryRouter>,
Expand Down
10 changes: 6 additions & 4 deletions src/features/Courses/AddClass/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import 'features/Courses/AddClass/index.scss';
const AddClass = ({
isOpen, onClose, courseInfo, isCoursePage, isEditing, isDetailClassPage,
}) => {
const { courseId } = useParams();
const { courseName } = useParams();
const dispatch = useDispatch();
const selectedInstitution = useSelector((state) => state.main.selectedInstitution);
const instructorsList = useSelector((state) => state.instructors.selectOptions.data);
Expand All @@ -37,6 +37,8 @@ const AddClass = ({
const [startDate, setStartDate] = useState('');
const [endDate, setEndDate] = useState('');

const courseNameDecoded = courseName ? decodeURIComponent(courseName) : '';

const handleAddClass = async (e) => {
e.preventDefault();
const form = e.target;
Expand All @@ -59,9 +61,9 @@ const AddClass = ({
logError(error);
} finally {
if (isDetailClassPage) {
dispatch(fetchAllClassesData(selectedInstitution.id, courseId));
dispatch(fetchAllClassesData(selectedInstitution.id, courseNameDecoded));
} else {
dispatch(fetchClassesData(selectedInstitution.id, initialPage, courseId));
dispatch(fetchClassesData(selectedInstitution.id, initialPage, courseNameDecoded));
dispatch(updateClassesCurrentPage(initialPage));
}
}
Expand All @@ -85,7 +87,7 @@ const AddClass = ({
dispatch(fetchCoursesData(selectedInstitution.id, initialPage));
dispatch(updateCoursesCurrentPage(initialPage));
} else {
dispatch(fetchClassesData(selectedInstitution.id, initialPage, courseId));
dispatch(fetchClassesData(selectedInstitution.id, initialPage, courseNameDecoded));
dispatch(updateClassesCurrentPage(initialPage));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('columns', () => {

renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201']}>
<Route path="/courses/:courseId">
<Route path="/courses/:courseName">
<Component />
</Route>
</MemoryRouter>,
Expand Down Expand Up @@ -157,7 +157,7 @@ describe('columns', () => {

const component = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201']}>
<Route path="/courses/:courseId">
<Route path="/courses/:courseName">
<Component />
</Route>
</MemoryRouter>,
Expand Down Expand Up @@ -211,7 +211,7 @@ describe('columns', () => {

const component = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201']}>
<Route path="/courses/:courseId">
<Route path="/courses/:courseName">
<ComponentNoInstructor />
</Route>
</MemoryRouter>,
Expand Down Expand Up @@ -256,7 +256,7 @@ describe('columns', () => {

const component = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201']}>
<Route path="/courses/:courseId">
<Route path="/courses/:courseName">
<Component />
</Route>
</MemoryRouter>,
Expand Down
6 changes: 3 additions & 3 deletions src/features/Courses/CourseDetailTable/columns.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ const columns = [
Header: 'Class',
accessor: 'className',
Cell: ({ row }) => {
const { courseId } = useParams();
const { courseName } = useParams();
return (
<Link
to={`/courses/${courseId}/${row.values.className}?classId=${row.original.classId}&previous=courses`}
to={`/courses/${courseName}/${encodeURIComponent(row.values.className)}?classId=${row.original.classId}&previous=courses`}
className="text-truncate link"
>
{row.values.className}
Expand Down Expand Up @@ -131,7 +131,7 @@ const columns = [
</Dropdown.Item>
<Dropdown.Item>
<Link
to={`/manage-instructors/${masterCourseName}/${row.values.className}?classId=${classId}`}
to={`/manage-instructors/${encodeURIComponent(masterCourseName)}/${encodeURIComponent(row.values.className)}?classId=${classId}`}
className="text-truncate text-decoration-none custom-text-black"
>
<i className="fa-regular fa-chalkboard-user mr-2 mb-1" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('CoursesDetailPage', () => {
test('Should render the table and the course info', async () => {
const component = renderWithProviders(
<MemoryRouter initialEntries={['/courses/Demo%20Course%201']}>
<Route path="/courses/:classId">
<Route path="/courses/:courseName">
<CoursesDetailPage />
</Route>
</MemoryRouter>,
Expand Down
Loading
Loading