Lk/fix filter (#320)

* fix: cohort and track filtering

* chore: normalize using response.data instead of chaining
This commit is contained in:
leangseu-edx
2023-03-23 11:16:23 -04:00
committed by GitHub
parent abc68f4224
commit 541a661dcc
11 changed files with 39 additions and 66 deletions

View File

@@ -13,7 +13,7 @@ export const useStudentGroupsFilterData = ({ updateQueryParams }) => {
const fetchGrades = thunkActions.grades.useFetchGrades();
const handleUpdateTrack = (event) => {
const selectedTrackItem = tracks.find(track => track.name === event.target.value);
const selectedTrackItem = tracks.find(track => track.slug === event.target.value);
const track = selectedTrackItem ? selectedTrackItem.slug.toString() : null;
updateQueryParams({ track });
updateTrack(track);
@@ -21,21 +21,22 @@ export const useStudentGroupsFilterData = ({ updateQueryParams }) => {
};
const handleUpdateCohort = (event) => {
const selectedCohortItem = cohorts.find(cohort => cohort.name === event.target.value);
const selectedCohortItem = cohorts.find(cohort => cohort.id === parseInt(event.target.value, 10));
const cohort = selectedCohortItem ? selectedCohortItem.id.toString() : null;
// the param expected to be cohort_id
updateQueryParams({ cohort });
updateCohort(cohort);
fetchGrades();
};
return {
cohorts: {
value: selectedCohortEntry?.name || '',
value: selectedCohortEntry?.id || '',
isDisabled: cohorts.length === 0,
handleChange: handleUpdateCohort,
entries: cohorts.map(({ id: value, name }) => ({ value, name })),
},
tracks: {
value: selectedTrackEntry?.name || '',
value: selectedTrackEntry?.slug || '',
handleChange: handleUpdateTrack,
entries: tracks.map(({ slug: value, name }) => ({ value, name })),
},

View File

@@ -71,7 +71,7 @@ describe('useAssignmentFilterData hook', () => {
describe('output', () => {
describe('cohorts', () => {
test('value from hook', () => {
expect(out.cohorts.value).toEqual(testCohort.name);
expect(out.cohorts.value).toEqual(testCohort.id);
});
test('disabled iff no cohorts found', () => {
expect(out.cohorts.isDisabled).toEqual(false);
@@ -93,7 +93,7 @@ describe('useAssignmentFilterData hook', () => {
});
describe('handleEvent', () => {
it('updates filter and query params and fetches grades', () => {
out.cohorts.handleChange({ target: { value: testCohort.name } });
out.cohorts.handleChange({ target: { value: testCohort.id } });
expect(updateCohort).toHaveBeenCalledWith(testCohort.id.toString());
expect(updateQueryParams).toHaveBeenCalledWith({ cohort: testCohort.id.toString() });
expect(fetch).toHaveBeenCalled();
@@ -108,7 +108,7 @@ describe('useAssignmentFilterData hook', () => {
});
describe('tracks', () => {
test('value from hook', () => {
expect(out.tracks.value).toEqual(testTrack.name);
expect(out.tracks.value).toEqual(testTrack.slug);
});
test('entries map slug to value', () => {
const { entries } = out.tracks;
@@ -124,7 +124,7 @@ describe('useAssignmentFilterData hook', () => {
});
describe('handleEvent', () => {
it('updates filter and query params and fetches grades', () => {
out.tracks.handleChange({ target: { value: testTrack.name } });
out.tracks.handleChange({ target: { value: testTrack.slug } });
expect(updateTrack).toHaveBeenCalledWith(testTrack.slug.toString());
expect(updateQueryParams).toHaveBeenCalledWith({ track: testTrack.slug.toString() });
expect(fetch).toHaveBeenCalled();

View File

@@ -1,11 +1,4 @@
import React from 'react';
import { StrictDict } from 'utils';
import { actions, selectors, thunkActions } from 'data/redux/hooks';
import * as module from './hooks';
export const state = StrictDict({
includeCourseRoleMembers: (val) => React.useState(val), // eslint-disable-line
});
export const useGradebookFiltersData = ({ updateQueryParams }) => {
const includeCourseRoleMembers = selectors.filters.useIncludeCourseRoleMembers();
@@ -13,13 +6,7 @@ export const useGradebookFiltersData = ({ updateQueryParams }) => {
const closeMenu = thunkActions.app.useCloseFilterMenu();
const fetchGrades = thunkActions.grades.useFetchGrades();
const [
localIncludeCourseRoleMembers,
setLocalIncludeCourseRoleMembers,
] = module.state.includeCourseRoleMembers(includeCourseRoleMembers);
const handleIncludeTeamMembersChange = ({ target: { checked } }) => {
setLocalIncludeCourseRoleMembers(checked);
updateIncludeCourseRoleMembers(checked);
fetchGrades();
updateQueryParams({ includeCourseRoleMembers: checked });
@@ -28,7 +15,7 @@ export const useGradebookFiltersData = ({ updateQueryParams }) => {
closeMenu,
includeCourseTeamMembers: {
handleChange: handleIncludeTeamMembersChange,
value: localIncludeCourseRoleMembers,
value: includeCourseRoleMembers,
},
};
};

View File

@@ -1,4 +1,3 @@
import { MockUseState } from 'testUtils';
import { actions, selectors, thunkActions } from 'data/redux/hooks';
import * as hooks from './hooks';
@@ -15,8 +14,6 @@ jest.mock('data/redux/hooks', () => ({
},
}));
const state = new MockUseState(hooks);
selectors.filters.useIncludeCourseRoleMembers.mockReturnValue(true);
const updateIncludeCourseRoleMembers = jest.fn();
actions.filters.useUpdateIncludeCourseRoleMembers.mockReturnValue(updateIncludeCourseRoleMembers);
@@ -29,12 +26,8 @@ const updateQueryParams = jest.fn();
let out;
describe('GradebookFiltersData component hooks', () => {
describe('state', () => {
state.testGetter(state.keys.includeCourseRoleMembers);
});
describe('useGradebookFiltersData', () => {
beforeEach(() => {
state.mock();
out = hooks.useGradebookFiltersData({ updateQueryParams });
});
describe('behavior', () => {
@@ -55,7 +48,6 @@ describe('GradebookFiltersData component hooks', () => {
test('includeCourseTeamMembers handleChange', () => {
const event = { target: { checked: false } };
out.includeCourseTeamMembers.handleChange(event);
expect(state.setState.includeCourseRoleMembers).toHaveBeenCalledWith(false);
expect(updateIncludeCourseRoleMembers).toHaveBeenCalledWith(false);
expect(fetchGrades).toHaveBeenCalledWith();
expect(updateQueryParams).toHaveBeenCalledWith({ includeCourseRoleMembers: false });

View File

@@ -12,7 +12,7 @@ export const getGradesUrl = () => `${getUrlPrefix()}grades/v1/`;
export const getGradebookUrl = () => `${getGradesUrl()}gradebook/${courseId}/`;
export const getBulkUpdateUrl = () => `${getGradebookUrl()}bulk-update`;
export const getInterventionUrl = () => `${getBulkGradesUrl()}intervention/`;
export const getCohortsUrl = () => `${getUrlPrefix()}courses/${courseId}/cohorts/`;
export const getCohortsUrl = () => `${getUrlPrefix()}cohorts/v1/courses/${courseId}/cohorts/`;
export const getTracksUrl = () => `${getEnrollmentUrl()}course/${courseId}?include_expired=1`;
export const getBulkHistoryUrl = () => `${getBulkUpdateUrl()}history/`;
export const getAssignmentTypesUrl = () => stringifyUrl(`${getGradebookUrl()}grading-info`, { graded_only: true });

View File

@@ -15,8 +15,7 @@ export const fetchAssignmentTypes = () => (
(dispatch) => {
dispatch(fetching.started());
return lms.api.fetch.assignmentTypes()
.then(response => response.data)
.then((data) => {
.then(({ data }) => {
dispatch(fetching.received(Object.keys(data.assignment_types)));
dispatch(gotGradesFrozen(data.grades_frozen));
dispatch(gotBulkManagementConfig(data.can_see_bulk_management));

View File

@@ -7,9 +7,8 @@ export const fetchCohorts = () => (
(dispatch) => {
dispatch(actions.cohorts.fetching.started());
return lms.api.fetch.cohorts()
.then(response => response.data)
.then((data) => {
dispatch(actions.cohorts.fetching.received(data.cohorts));
.then(({ data }) => {
dispatch(actions.cohorts.fetching.received(data));
})
.catch(() => {
dispatch(actions.cohorts.fetching.error());

View File

@@ -11,7 +11,7 @@ jest.mock('data/services/lms', () => ({
}));
const responseData = {
cohorts: {
data: {
some: 'COHorts',
other: 'cohORT$',
},
@@ -27,10 +27,10 @@ describe('cohorts thunkActions', () => {
);
describe('actions dispatched on valid response', () => {
test('fetching.started, fetching.received', () => testFetch(
(resolve) => resolve({ data: responseData }),
(resolve) => resolve(responseData),
[
actions.cohorts.fetching.started(),
actions.cohorts.fetching.received(responseData.cohorts),
actions.cohorts.fetching.received(responseData.data),
],
));
});

View File

@@ -39,24 +39,23 @@ export const fetchGrades = (overrides = {}) => (
cohort,
track,
fetchOptions,
).then(response => response.data)
.then((data) => {
dispatch(grades.fetching.received({
assignmentType: (assignmentType || selectors.filters.assignmentType(getState())),
cohort,
courseId,
track,
grades: data.results.sort(sortAlphaAsc),
prev: data.previous,
next: data.next,
totalUsersCount: data.total_users_count,
filteredUsersCount: data.filtered_users_count,
}));
if (fetchOptions.showSuccess) {
dispatch(grades.banner.open());
}
dispatch(grades.fetching.finished());
})
).then(({ data }) => {
dispatch(grades.fetching.received({
assignmentType: (assignmentType || selectors.filters.assignmentType(getState())),
cohort,
courseId,
track,
grades: data.results.sort(sortAlphaAsc),
prev: data.previous,
next: data.next,
totalUsersCount: data.total_users_count,
filteredUsersCount: data.filtered_users_count,
}));
if (fetchOptions.showSuccess) {
dispatch(grades.banner.open());
}
dispatch(grades.fetching.finished());
})
.catch(() => {
dispatch(grades.fetching.error());
});
@@ -73,8 +72,7 @@ export const fetchGradesIfAssignmentGradeFiltersSet = () => (
export const fetchGradeOverrideHistory = (subsectionId, userId) => (
dispatch => lms.api.fetch.gradeOverrideHistory(subsectionId, userId)
.then(response => response.data)
.then((data) => {
.then(({ data }) => {
if (data.success) {
dispatch(grades.overrideHistory.received({
overrideHistory: formatGradeOverrideForDisplay(data.history),
@@ -147,8 +145,7 @@ export const updateGrades = () => (
const updateData = selectors.app.editUpdateData(getState());
dispatch(grades.update.request());
return lms.api.updateGradebookData(updateData)
.then(response => response.data)
.then((data) => {
.then(({ data }) => {
dispatch(grades.update.success({ data }));
dispatch(module.fetchGrades({
assignmentType: defaultAssignmentFilter,

View File

@@ -16,12 +16,11 @@ export const fetchRoles = () => (
(dispatch, getState) => {
const courseId = selectors.app.courseId(getState());
return lms.api.fetch.roles()
.then(response => response.data)
.then((response) => {
.then(({ data }) => {
const isAllowedRole = (role) => (
(role.course_id === courseId) && allowedRoles.includes(role.role)
);
const canUserViewGradebook = (response.is_staff || (response.roles.some(isAllowedRole)));
const canUserViewGradebook = (data.is_staff || (data.roles.some(isAllowedRole)));
dispatch(roles.fetching.received({ canUserViewGradebook }));
if (canUserViewGradebook) {
dispatch(fetchGrades());

View File

@@ -8,8 +8,7 @@ export const fetchTracks = () => (
(dispatch) => {
dispatch(actions.tracks.fetching.started());
return lms.api.fetch.tracks()
.then(response => response.data)
.then((data) => {
.then(({ data }) => {
dispatch(actions.tracks.fetching.received(data.course_modes));
})
.catch(() => {