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.

Sign up for GitHub

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-1905 - Instructors and Students pages are now filtering by institution #138

Merged
merged 1 commit into from
Dec 16, 2024
Merged

PADV-1905 - Instructors and Students pages are now filtering by institution #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/features/Classes/ClassesPage/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { useLocation } from 'react-router-dom';
import ClassesTable from 'features/Classes/ClassesTable';
import ClassesFilters from 'features/Classes/ClassesFilters';

import { updateCurrentPage, resetClassesTable } from 'features/Classes/data/slice';
import { updateCurrentPage, updateFilters, resetClassesTable } from 'features/Classes/data/slice';
import { fetchClassesData } from 'features/Classes/data/thunks';
import { initialPage } from 'features/constants';

Expand Down Expand Up @@ -35,6 +35,7 @@ const ClassesPage = () => {

return () => {
dispatch(resetClassesTable());
dispatch(updateFilters({}));
};
}, [selectedInstitution, dispatch, currentPage]); // eslint-disable-line react-hooks/exhaustive-deps

Expand Down
7 changes: 6 additions & 1 deletion src/features/Common/data/_test_/api.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ describe('Common api services', () => {

expect(httpClientMock.get).toHaveBeenCalledTimes(1);
expect(httpClientMock.get).toHaveBeenCalledWith(
`${COURSE_OPERATIONS_API_V2}/license-pool/?limit=true&institution_id=1&page=1&`,
`${COURSE_OPERATIONS_API_V2}/license-pool/?limit=true&institution_id=1`,
{
params: {
page: 1,
},
},
);
});

Expand Down
7 changes: 6 additions & 1 deletion src/features/Common/data/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@ function getCoursesByInstitution(institutionId, limit, page, filters) {
}

function getLicensesByInstitution(institutionId, limit, page = 1, urlParamsFilters = '') {
const params = {
page,
...urlParamsFilters,
};
return getAuthenticatedHttpClient().get(
`${getConfig().COURSE_OPERATIONS_API_V2_BASE_URL}/license-pool`
+ `/?limit=${limit}&institution_id=${institutionId}&page=${page}&${urlParamsFilters}`,
+ `/?limit=${limit}&institution_id=${institutionId}`,
{ params },
);
}

Expand Down
13 changes: 4 additions & 9 deletions src/features/Courses/CoursesPage/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Pagination } from '@edx/paragon';
import CoursesTable from 'features/Courses/CoursesTable';
import CoursesFilters from 'features/Courses/CoursesFilters';

import { updateCurrentPage, resetCoursesTable } from 'features/Courses/data/slice';
import { updateCurrentPage, updateFilters, resetCoursesTable } from 'features/Courses/data/slice';
import { fetchCoursesData } from 'features/Courses/data/thunks';
import { initialPage } from 'features/constants';

Expand All @@ -18,19 +18,14 @@ const CoursesPage = () => {

useEffect(() => {
if (Object.keys(selectedInstitution).length > 0) {
dispatch(fetchCoursesData(selectedInstitution.id, initialPage));
dispatch(fetchCoursesData(selectedInstitution.id, currentPage));
}

return () => {
dispatch(resetCoursesTable());
dispatch(updateFilters({}));
};
}, [selectedInstitution, dispatch]); // eslint-disable-line react-hooks/exhaustive-deps

useEffect(() => {
if (Object.keys(selectedInstitution).length > 0) {
dispatch(fetchCoursesData(selectedInstitution.id, currentPage, stateCourses.filters));
}
}, [currentPage]); // eslint-disable-line react-hooks/exhaustive-deps
}, [selectedInstitution, dispatch, currentPage]);

const handlePagination = (targetPage) => {
setCurrentPage(targetPage);
Expand Down
2 changes: 1 addition & 1 deletion src/features/Dashboard/data/_test_/redux.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Dashboard redux tests', () => {

test('successful fetch licenses data', async () => {
const licensesApiUrl = `${process.env.COURSE_OPERATIONS_API_V2_BASE_URL}/license-pool`
+ '/?limit=false&institution_id=1&page=1&';
+ '/?limit=false&institution_id=1';
const mockResponse = [
{
licenseName: 'License Name 1',
Expand Down
11 changes: 4 additions & 7 deletions src/features/Instructors/InstructorsPage/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import InstructorsFilters from 'features/Instructors/InstructorsFilters';
import AddInstructors from 'features/Instructors/AddInstructors';
import { Button } from 'react-paragon-topaz';

import { updateCurrentPage, resetInstructorsRequest } from 'features/Instructors/data/slice';
import { updateCurrentPage, updateFilters, resetInstructorsRequest } from 'features/Instructors/data/slice';
import { fetchInstructorsData } from 'features/Instructors/data/thunks';
import { initialPage } from 'features/constants';

Expand All @@ -21,17 +21,14 @@ const InstructorsPage = () => {

useEffect(() => {
if (Object.keys(selectedInstitution).length > 0) {
dispatch(fetchInstructorsData(selectedInstitution.id, initialPage));
dispatch(fetchInstructorsData(selectedInstitution.id, currentPage));
}

return () => {
dispatch(resetInstructorsRequest());
dispatch(updateFilters({}));
};
}, [selectedInstitution, dispatch]); // eslint-disable-line react-hooks/exhaustive-deps

useEffect(() => {
dispatch(fetchInstructorsData(selectedInstitution.id, currentPage, stateInstructors.filters));
}, [currentPage]); // eslint-disable-line react-hooks/exhaustive-deps
}, [selectedInstitution, dispatch, currentPage]);

const handlePagination = (targetPage) => {
setCurrentPage(targetPage);
Expand Down
5 changes: 1 addition & 4 deletions src/features/Licenses/LicensesFilters/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,11 @@ const LicensesFilters = ({ resetPagination }) => {
const form = e.target;
const formData = new FormData(form);
const formJson = Object.fromEntries(formData.entries());
const urlParamsFilters = Object.entries(formJson)
.map(([key, value]) => `${key}=${encodeURIComponent(value)}`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something that worries me is the information is not being encoded, should be worried?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@01001110J should it be encoded in that part of the code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far I remember ( please confirm with @AuraAlba ) we should send the info encoded to avoid to get unexpected responses. The request params should be encoded, but please confirm with Aura first to be sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@01001110J I tested with a License name with special characters and it worked.

image

.join('&');

try {
dispatch(updateFilters(formJson));
dispatch(updateCurrentPage(initialPage));
dispatch(fetchLicensesData(institution.id, initialPage, urlParamsFilters));
dispatch(fetchLicensesData(institution.id, initialPage, formJson));
} catch (error) {
logError(error);
}
Expand Down
11 changes: 4 additions & 7 deletions src/features/Licenses/LicensesPage/index.jsx
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIlters are not working in licenses page

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useDispatch, useSelector } from 'react-redux';
import { Pagination, Container } from '@edx/paragon';

import { initialPage } from 'features/constants';
import { updateCurrentPage, resetLicensesTable } from 'features/Licenses/data/slice';
import { updateCurrentPage, updateFilters, resetLicensesTable } from 'features/Licenses/data/slice';
import { fetchLicensesData } from 'features/Licenses/data';
import LicensesTable from 'features/Licenses/LicensesTable';
import LicensesFilters from 'features/Licenses/LicensesFilters';
Expand All @@ -25,17 +25,14 @@ const LicensesPage = () => {

useEffect(() => {
if (Object.keys(selectedInstitution).length > 0) {
dispatch(fetchLicensesData(selectedInstitution?.id, initialPage));
dispatch(fetchLicensesData(selectedInstitution.id, currentPage));
}

return () => {
dispatch(resetLicensesTable());
dispatch(updateFilters({}));
};
}, [selectedInstitution, dispatch]);

useEffect(() => {
dispatch(fetchLicensesData(selectedInstitution?.id, currentPage, stateLicenses.filters));
}, [currentPage]); // eslint-disable-line react-hooks/exhaustive-deps
}, [selectedInstitution, dispatch, currentPage]);
01001110J marked this conversation as resolved.
Show resolved Hide resolved

return (
<Container size="xl" className="px-4">
Expand Down
4 changes: 2 additions & 2 deletions src/features/Licenses/data/_test_/redux.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('Licenses redux tests', () => {

test('successful fetch licenses data', async () => {
const licensesApiUrl = `${process.env.COURSE_OPERATIONS_API_V2_BASE_URL}/license-pool`
+ '/?limit=true&institution_id=1&page=1&';
+ '/?limit=true&institution_id=1';
const mockResponse = {
results: [
{
Expand Down Expand Up @@ -67,7 +67,7 @@ describe('Licenses redux tests', () => {

test('failed fetch licenses data', async () => {
const licensesApiUrl = `${process.env.COURSE_OPERATIONS_API_V2_BASE_URL}/license-pool`
+ '/?limit=false&institution_id=1&page=1&';
+ '/?limit=false&institution_id=1';
axiosMock.onGet(licensesApiUrl)
.reply(500);

Expand Down
11 changes: 4 additions & 7 deletions src/features/Students/StudentsPage/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import StudentsFilters from 'features/Students/StudentsFilters';
import StudentsMetrics from 'features/Students/StudentsMetrics';
import Container from '@edx/paragon/dist/Container';
import { Pagination } from '@edx/paragon';
import { updateCurrentPage, resetStudentsTable } from 'features/Students/data/slice';
import { updateCurrentPage, updateFilters, resetStudentsTable } from 'features/Students/data/slice';
import { fetchStudentsData } from 'features/Students/data/thunks';
import { initialPage } from 'features/constants';

Expand All @@ -17,17 +17,14 @@ const StudentsPage = () => {

useEffect(() => {
if (Object.keys(selectedInstitution).length > 0) {
dispatch(fetchStudentsData(selectedInstitution.id, initialPage));
dispatch(fetchStudentsData(selectedInstitution.id, currentPage));
}

return () => {
dispatch(resetStudentsTable());
dispatch(updateFilters({}));
};
}, [selectedInstitution, dispatch]); // eslint-disable-line react-hooks/exhaustive-deps

useEffect(() => {
dispatch(fetchStudentsData(selectedInstitution.id, currentPage, stateStudents.filters));
}, [currentPage]); // eslint-disable-line react-hooks/exhaustive-deps
}, [selectedInstitution, dispatch, currentPage]);

const resetPagination = () => {
setCurrentPage(initialPage);
Expand Down
Loading