diff --git a/.eslintrc.js b/.eslintrc.js index 3072c76..58d6ce8 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,6 +1,13 @@ const { createConfig } = require('@edx/frontend-build'); -const config = createConfig('eslint'); +const config = createConfig('eslint', { + rules: { + 'import/no-named-as-default': 'off', + 'import/no-named-as-default-member': 'off', + 'import/no-self-import': 'off', + 'spaced-comment': ['error', 'always', { 'block': { 'exceptions': ['*'] } }], + }, +}); config.settings = { "import/resolver": { diff --git a/package.json b/package.json index d9be651..d29feaa 100755 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@edx/frontend-app-gradebook", - "version": "1.4.39", + "version": "1.4.40", "description": "edx editable gradebook-ui to manipulate grade overrides on subsections", "repository": { "type": "git", diff --git a/src/components/BulkManagementTab/HistoryTable.jsx b/src/components/BulkManagementTab/HistoryTable.jsx index 82cdefc..2b0dd50 100644 --- a/src/components/BulkManagementTab/HistoryTable.jsx +++ b/src/components/BulkManagementTab/HistoryTable.jsx @@ -55,7 +55,6 @@ HistoryTable.propTypes = { timeUploaded: PropTypes.string.isRequired, resultsSummary: PropTypes.shape({ rowId: PropTypes.number.isRequired, - courseId: PropTypes.string.isRequired, text: PropTypes.string.isRequired, }), })), diff --git a/src/components/BulkManagementTab/ResultsSummary.jsx b/src/components/BulkManagementTab/ResultsSummary.jsx index 7a36b3c..7c29180 100644 --- a/src/components/BulkManagementTab/ResultsSummary.jsx +++ b/src/components/BulkManagementTab/ResultsSummary.jsx @@ -5,7 +5,7 @@ import PropTypes from 'prop-types'; import { Hyperlink, Icon } from '@edx/paragon'; import { Download } from '@edx/paragon/icons'; -import { bulkGradesUrlByCourseAndRow } from 'data/constants/api'; +import lms from 'data/services/lms'; /** * @@ -15,12 +15,11 @@ import { bulkGradesUrlByCourseAndRow } from 'data/constants/api'; * @param {string} text - summary string */ const ResultsSummary = ({ - courseId, rowId, text, }) => ( ({ @@ -14,13 +14,14 @@ jest.mock('@edx/paragon', () => ({ jest.mock('@edx/paragon/icons', () => ({ Download: 'DownloadIcon', })); -jest.mock('data/constants/api', () => ({ - bulkGradesUrlByCourseAndRow: jest.fn((courseId, rowId) => ({ url: { courseId, rowId } })), +jest.mock('data/services/lms', () => ({ + urls: { + bulkGradesUrlByRow: jest.fn((rowId) => ({ url: { rowId } })), + }, })); describe('ResultsSummary component', () => { const props = { - courseId: 'classy', rowId: 42, text: 'texty', }; @@ -41,7 +42,7 @@ describe('ResultsSummary component', () => { expect(el.props().rel).toEqual('noopener noreferrer'); }); test('Hyperlink has href to bulkGradesUrl', () => { - expect(el.props().href).toEqual(api.bulkGradesUrlByCourseAndRow(props.courseId, props.rowId)); + expect(el.props().href).toEqual(lms.urls.bulkGradesUrlByRow(props.rowId)); }); test('displays Download Icon and text', () => { const icon = el.childAt(0); diff --git a/src/components/BulkManagementTab/__snapshots__/ResultsSummary.test.jsx.snap b/src/components/BulkManagementTab/__snapshots__/ResultsSummary.test.jsx.snap index 07ded56..43fbe6f 100644 --- a/src/components/BulkManagementTab/__snapshots__/ResultsSummary.test.jsx.snap +++ b/src/components/BulkManagementTab/__snapshots__/ResultsSummary.test.jsx.snap @@ -6,7 +6,6 @@ exports[`ResultsSummary component snapshot - safe hyperlink with bulkGradesUrl w href={ Object { "url": Object { - "courseId": "classy", "rowId": 42, }, } diff --git a/src/components/GradesTab/ScoreViewInput.jsx b/src/components/GradesTab/ScoreViewInput.jsx index 363e57a..b1e34a0 100644 --- a/src/components/GradesTab/ScoreViewInput.jsx +++ b/src/components/GradesTab/ScoreViewInput.jsx @@ -24,8 +24,11 @@ export const ScoreViewInput = ({ format, toggleFormat }) => ( ); +ScoreViewInput.defaultProps = { + format: 'percent', +}; ScoreViewInput.propTypes = { - format: PropTypes.string.isRequired, + format: PropTypes.string, toggleFormat: PropTypes.func.isRequired, }; @@ -37,4 +40,4 @@ export const mapDispatchToProps = { toggleFormat: actions.grades.toggleGradeFormat, }; -export default connect(() => ({}), mapDispatchToProps)(ScoreViewInput); +export default connect(mapStateToProps, mapDispatchToProps)(ScoreViewInput); diff --git a/src/data/constants/api.js b/src/data/constants/api.js deleted file mode 100644 index e883953..0000000 --- a/src/data/constants/api.js +++ /dev/null @@ -1,14 +0,0 @@ -import { configuration } from 'config'; - -export const baseUrl = `${configuration.LMS_BASE_URL}/api`; - -/** - * bulkGradesUrlByCourseAndRow(courseId, rowId) - * returns the bulkGrades url with the given courseId and rowId. - * @param {string} courseId - course identifier - * @param {string} rowId - row/error identifier - * @return {string} - bulk grades fetch url - */ -export const bulkGradesUrlByCourseAndRow = (courseId, rowId) => ( - `${baseUrl}/bulkGrades/course/${courseId}/?error_id=${rowId}` -); diff --git a/src/data/constants/filters.js b/src/data/constants/filters.js index 1499637..afac9bb 100644 --- a/src/data/constants/filters.js +++ b/src/data/constants/filters.js @@ -37,8 +37,8 @@ export const filterConfig = StrictDict({ }, [filters.assignmentGrade]: { displayName: 'Assignment Grade', - filterOrder: ['courseGradeMin', 'courseGradeMax'], - connectedFilters: ['courseGradeMax', 'courseGradeMin'], + filterOrder: ['assignmentGradeMin', 'assignmentGradeMax'], + connectedFilters: ['assignmentGradeMax', 'assignmentGradeMin'], }, [filters.cohort]: { displayName: 'Cohort', diff --git a/src/data/selectors/grades.js b/src/data/selectors/grades.js index d7271cc..a8bf471 100644 --- a/src/data/selectors/grades.js +++ b/src/data/selectors/grades.js @@ -133,7 +133,6 @@ export const transformHistoryEntry = ({ originalFilename, resultsSummary: { rowId: id, - courseId, text: module.getRowsProcessed(data), }, ...rest, diff --git a/src/data/selectors/grades.test.js b/src/data/selectors/grades.test.js index bd63bdb..acda34b 100644 --- a/src/data/selectors/grades.test.js +++ b/src/data/selectors/grades.test.js @@ -200,7 +200,6 @@ describe('grades selectors', () => { it('summarizes processed rows', () => { expect(output.resultsSummary).toEqual({ text: selectors.getRowsProcessed(rawEntry.data), - courseId: rawEntry.unique_id, rowId: rawEntry.id, }); }); diff --git a/src/data/selectors/index.js b/src/data/selectors/index.js index 911fd61..c4be8f7 100644 --- a/src/data/selectors/index.js +++ b/src/data/selectors/index.js @@ -1,7 +1,6 @@ -/* eslint-disable import/no-named-as-default-member, import/no-self-import */ import { StrictDict } from 'utils'; -import LmsApiService from 'data/services/LmsApiService'; +import lms from 'data/services/lms'; import * as filterConstants from 'data/constants/filters'; import * as module from '.'; @@ -122,13 +121,13 @@ export const getHeadings = (state) => grades.headingMapper( /** * gradeExportUrl(state, options) - * Returns the output of getGradeExportCsvUrl, applying the current includeCourseRoleMembers + * Returns the output of getGradeCsvUrl, applying the current includeCourseRoleMembers * filter. * @param {object} state - redux state * @return {string} - generated grade export url */ export const gradeExportUrl = (state) => ( - LmsApiService.getGradeExportCsvUrl(app.courseId(state), { + lms.urls.gradeCsvUrl({ ...module.lmsApiServiceArgs(state), excludeCourseRoles: filters.includeCourseRoleMembers(state) ? '' : 'all', }) @@ -141,8 +140,7 @@ export const gradeExportUrl = (state) => ( * @return {string} - generated intervention export url */ export const interventionExportUrl = (state) => ( - LmsApiService.getInterventionExportCsvUrl( - app.courseId(state), + lms.urls.interventionExportCsvUrl( module.lmsApiServiceArgs(state), ) ); diff --git a/src/data/selectors/index.test.js b/src/data/selectors/index.test.js index 00a8c9d..5692bc3 100644 --- a/src/data/selectors/index.test.js +++ b/src/data/selectors/index.test.js @@ -1,17 +1,16 @@ -/* eslint-disable import/no-named-as-default-member */ +/* eslint-disable import/no-named-as-default-member, import/no-named-as-default */ import * as filterConstants from '../constants/filters'; import selectors from '.'; import * as moduleSelectors from '.'; import { minGrade, maxGrade } from './grades'; -jest.mock('../services/LmsApiService', () => ({ - __esModule: true, - default: { - getGradeExportCsvUrl: jest.fn( - (...args) => ({ getGradeExportCsvUrl: { args } }), +jest.mock('data/services/lms', () => ({ + urls: { + gradeCsvUrl: jest.fn( + (...args) => ({ gradeCsvUrl: { args } }), ), - getInterventionExportCsvUrl: jest.fn( - (...args) => ({ getInterventionExportCsvUrl: { args } }), + interventionExportCsvUrl: jest.fn( + (...args) => ({ interventionExportCsvUrl: { args } }), ), }, })); @@ -344,8 +343,8 @@ describe('root selectors', () => { it('calls the API service with the right args, excluding all course roles', () => { selectors.filters.includeCourseRoleMembers.mockReturnValue(undefined); expect(selector(testState)).toEqual({ - getGradeExportCsvUrl: { - args: [testCourseId, { lmsArgs: testState, excludeCourseRoles: 'all' }], + gradeCsvUrl: { + args: [{ lmsArgs: testState, excludeCourseRoles: 'all' }], }, }); }); @@ -354,8 +353,8 @@ describe('root selectors', () => { it('calls the API service with the right args, including course roles', () => { selectors.filters.includeCourseRoleMembers.mockReturnValue(true); expect(selector(testState)).toEqual({ - getGradeExportCsvUrl: { - args: [testCourseId, { lmsArgs: testState, excludeCourseRoles: '' }], + gradeCsvUrl: { + args: [{ lmsArgs: testState, excludeCourseRoles: '' }], }, }); }); @@ -369,8 +368,8 @@ describe('root selectors', () => { expect( moduleSelectors.interventionExportUrl(testState), ).toEqual({ - getInterventionExportCsvUrl: { - args: [testCourseId, { lmsArgs: testState }], + interventionExportCsvUrl: { + args: [{ lmsArgs: testState }], }, }); moduleSelectors.lmsApiServiceArgs = lmsApiServiceArgs; diff --git a/src/data/services/LmsApiService.js b/src/data/services/LmsApiService.js deleted file mode 100644 index 347ba80..0000000 --- a/src/data/services/LmsApiService.js +++ /dev/null @@ -1,143 +0,0 @@ -import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { configuration } from '../../config'; - -class LmsApiService { - static baseUrl = configuration.LMS_BASE_URL; - - static pageSize = 25 - - static fetchGradebookData(courseId, searchText, cohort, track, options = {}) { - const queryParams = {}; - queryParams.page_size = LmsApiService.pageSize; - if (searchText) { - queryParams.user_contains = searchText; - } - if (cohort) { - queryParams.cohort_id = cohort; - } - if (track) { - queryParams.enrollment_mode = track; - } - if (options.assignmentGradeMax || options.assignmentGradeMin) { - if (!options.assignment) { - throw new Error('Gradebook LMS API requires assignment to be set to filter by min/max assig. grade'); - } - queryParams.assignment = options.assignment; - if (options.assignmentGradeMin) { - queryParams.assignment_grade_min = options.assignmentGradeMin; - } - if (options.assignmentGradeMax) { - queryParams.assignment_grade_max = options.assignmentGradeMax; - } - } - if (options.courseGradeMin) { - queryParams.course_grade_min = options.courseGradeMin; - } - if (options.courseGradeMax) { - queryParams.course_grade_max = options.courseGradeMax; - } - if (!options.includeCourseRoleMembers) { - queryParams.excluded_course_roles = ['all']; - } - - const queryParamString = Object.keys(queryParams) - .map(attr => `${attr}=${encodeURIComponent(queryParams[attr])}`) - .join('&'); - - const gradebookUrl = `${LmsApiService.baseUrl}/api/grades/v1/gradebook/${courseId}/?${queryParamString}`; - - return getAuthenticatedHttpClient().get(gradebookUrl); - } - - static updateGradebookData(courseId, updateData) { - /* - updateData is expected to be a list of objects with the keys 'user_id' (an integer), - 'usage_id' (a string) and 'grade', which is an object with the keys: - 'earned_all_override', - 'possible_all_override', - 'earned_graded_override', - and 'possible_graded_override', - each of which should be an integer. - Example: - [ - { - "user_id": 9, - "usage_id": "block-v1:edX+DemoX+Demo_Course+type@sequential+block@basic_questions", - "grade": { - "earned_all_override": 11, - "possible_all_override": 11, - "earned_graded_override": 11, - "possible_graded_override": 11, - "comment": "reason for override" - } - } - ] - */ - const gradebookUrl = `${LmsApiService.baseUrl}/api/grades/v1/gradebook/${courseId}/bulk-update`; - return getAuthenticatedHttpClient().post(gradebookUrl, updateData); - } - - static fetchTracks(courseId) { - const trackUrl = `${LmsApiService.baseUrl}/api/enrollment/v1/course/${courseId}?include_expired=1`; - return getAuthenticatedHttpClient().get(trackUrl); - } - - static fetchCohorts(courseId) { - const cohortsUrl = `${LmsApiService.baseUrl}/courses/${courseId}/cohorts/`; - return getAuthenticatedHttpClient().get(cohortsUrl); - } - - static fetchAssignmentTypes(courseId) { - const assignmentTypesUrl = `${LmsApiService.baseUrl}/api/grades/v1/gradebook/${courseId}/grading-info?graded_only=true`; - return getAuthenticatedHttpClient().get(assignmentTypesUrl); - } - - static fetchUserRoles(courseId) { - const rolesUrl = `${LmsApiService.baseUrl}/api/enrollment/v1/roles/?course_id=${encodeURIComponent(courseId)}`; - return getAuthenticatedHttpClient().get(rolesUrl); - } - - static getGradeExportCsvUrl(courseId, options = {}) { - const queryParams = ['track', 'cohort', 'assignment', 'assignmentType', 'assignmentGradeMax', - 'assignmentGradeMin', 'courseGradeMin', 'courseGradeMax', 'excludedCourseRoles'] - .filter(opt => options[opt] - && options[opt] !== 'All') - .map(opt => `${opt}=${encodeURIComponent(options[opt])}`) - .join('&'); - return `${LmsApiService.baseUrl}/api/bulk_grades/course/${courseId}/?${queryParams}`; - } - - static getInterventionExportCsvUrl(courseId, options = {}) { - const queryParams = ['track', 'cohort', 'assignment', 'assignmentType', 'assignmentGradeMax', - 'assignmentGradeMin', 'courseGradeMin', 'courseGradeMax'] - .filter(opt => options[opt] - && options[opt] !== 'All') - .map(opt => `${opt}=${encodeURIComponent(options[opt])}`) - .join('&'); - return `${LmsApiService.baseUrl}/api/bulk_grades/course/${courseId}/intervention?${queryParams}`; - } - - static getGradeImportCsvUrl = LmsApiService.getGradeExportCsvUrl; - - static uploadGradeCsv(courseId, formData) { - const fileUploadUrl = LmsApiService.getGradeImportCsvUrl(courseId); - return getAuthenticatedHttpClient().post(fileUploadUrl, formData).then((result) => { - if (result.status === 200 && !result.data.error_messages.length) { - return result.data; - } - return Promise.reject(result); - }); - } - - static fetchGradeBulkOperationHistory(courseId) { - const url = `${LmsApiService.baseUrl}/api/bulk_grades/course/${courseId}/history/`; - return getAuthenticatedHttpClient().get(url).then(response => response.data).catch(() => Promise.reject(Error('unhandled response error'))); - } - - static fetchGradeOverrideHistory(subsectionId, userId) { - const historyUrl = `${LmsApiService.baseUrl}/api/grades/v1/subsection/${subsectionId}/?user_id=${userId}&history_record_limit=5`; - return getAuthenticatedHttpClient().get(historyUrl); - } -} - -export default LmsApiService; diff --git a/src/data/services/lms/api.js b/src/data/services/lms/api.js new file mode 100644 index 0000000..9127c27 --- /dev/null +++ b/src/data/services/lms/api.js @@ -0,0 +1,120 @@ +import { StrictDict } from 'utils'; + +import urls, { + gradeCsvUrl, + sectionOverrideHistoryUrl, +} from './urls'; +import { pageSize, paramKeys } from './constants'; +import messages from './messages'; + +import * as utils from './utils'; + +const { get, post, stringifyUrl } = utils; + +/********************************************************************************* + * GET Actions + *********************************************************************************/ +const assignmentTypes = () => get(urls.assignmentTypes); +const cohorts = () => get(urls.cohorts); +const roles = () => get(urls.roles); +const tracks = () => get(urls.tracks); + +/** + * fetch.gradebookData(searchText, cohort, track, options) + * fetches updated gradebook data based on current filter selections. + * Raises an error if assignment grade limits are set, but not assignment. + * @param {string} searchText - search text filter + * @param {nunber} cohort - selected cohort filter + * @param {string} track - selected track filter + * @param {object} options - additional optional filter values + * @return {Promise} - get response + */ +const gradebookData = (searchText, cohort, track, options = {}) => { + if ((options.assignmentGradeMax || options.assignmentGradeMin) && !options.assignment) { + throw new Error(messages.errors.missingAssignment); + } + const queryParams = { + [paramKeys.pageSize]: pageSize, + [paramKeys.userContains]: searchText, + [paramKeys.cohortId]: cohort, + [paramKeys.enrollmentMode]: track, + [paramKeys.courseGradeMax]: options.courseGradeMax, + [paramKeys.courseGradeMin]: options.courseGradeMin, + [paramKeys.excludedCourseRoles]: options.includeCourseRoleMembers ? null : ['all'], + [paramKeys.assignment]: options.assignment, + [paramKeys.assignmentGradeMax]: options.assignmentGradeMax, + [paramKeys.assignmentGradeMin]: options.assignmentGradeMin, + }; + return get(stringifyUrl(urls.gradebook, queryParams)); +}; + +/** + * fetch.gradeBulkOperationHistory() + * fetches bulk operation history and raises an error if the operation fails + * @return {Promise} - get response + */ +const gradeBulkOperationHistory = () => get(urls.bulkHistory) + .then(response => response.data) + .catch(() => Promise.reject(Error(messages.errors.unhandledResponse))); + +/** + * fetch.gradeOverrideHistory(subsectionId, userId) + * fetches grade override history for a given user on a given subsection + * @param {string} subsectionId - subsection identifier + * @param {string} userId - user identifier + * @return {Promise} - get response + */ +const gradeOverrideHistory = (subsectionId, userId) => ( + get(sectionOverrideHistoryUrl(subsectionId, userId)) +); + +/********************************************************************************* + * POST Actions + *********************************************************************************/ +/** + * updateGradebookData(updateData) + * sends an update message with new grades overrides + * @param {object[]} updateData + * { + * user_id: , + * usage_id: + * grade: { + * earned_all_override: + * possible_all_override: + * earned_graded_override: + * possible_graded_override: + * } + * } + * @return {Promise} - post response + */ +const updateGradebookData = (updateData) => post(urls.bulkUpdate, updateData); + +/** + * uploadGradeCsv(formData) + * Posts form data to grade csv url. On success, forwards response data. + * Reject promise with result on failure. + * @param {object} formData - new grade data + * @return {Promise} - post response + */ +const uploadGradeCsv = (formData) => ( + post(gradeCsvUrl(), formData).then((result) => { + if (result.status === 200 && !result.data.error_messages.length) { + return result.data; + } + return Promise.reject(result); + }) +); + +export default StrictDict({ + fetch: StrictDict({ + assignmentTypes, + cohorts, + gradebookData, + gradeBulkOperationHistory, + gradeOverrideHistory, + roles, + tracks, + }), + updateGradebookData, + uploadGradeCsv, +}); diff --git a/src/data/services/lms/api.test.js b/src/data/services/lms/api.test.js new file mode 100644 index 0000000..4b90ffc --- /dev/null +++ b/src/data/services/lms/api.test.js @@ -0,0 +1,234 @@ +import api from './api'; +import { pageSize, paramKeys } from './constants'; +import messages from './messages'; +import urls, { gradeCsvUrl, sectionOverrideHistoryUrl } from './urls'; +import * as utils from './utils'; + +jest.mock('./urls', () => ({ + __esModule: true, + default: jest.requireActual('./urls').default, + gradeCsvUrl: (...args) => ({ gradeCsvUrl: args }), + sectionOverrideHistoryUrl: (...args) => `sectionOverrideHistoryUrl(${args})`, +})); + +jest.mock('./utils', () => ({ + get: jest.fn(), + post: jest.fn(), + stringifyUrl: jest.fn(), +})); + +describe('lms service api', () => { + describe('get actions', () => { + const mockGet = promiseFn => { + jest.spyOn(utils, 'get').mockImplementation( + url => new Promise(promiseFn(url)), + ); + }; + const resolveFn = (url) => (resolve) => resolve({ data: url }); + const rejectFn = (url) => (resolve, reject) => reject(url); + const testSimpleFetch = (method, expectedUrl, description) => { + mockGet(resolveFn); + test(description, () => ( + method().then(({ data }) => { expect(data).toEqual(expectedUrl); }) + )); + }; + describe('fetch.assignmentTypes', () => { + testSimpleFetch( + api.fetch.assignmentTypes, + urls.assignmentTypes, + 'fetches from urls.assignmentTypes', + ); + }); + describe('fetch.cohorts', () => { + testSimpleFetch( + api.fetch.cohorts, + urls.cohorts, + 'fetches from urls.cohorts', + ); + }); + describe('fetch.roles', () => { + testSimpleFetch( + api.fetch.roles, + urls.roles, + 'fetches from urls.roles', + ); + }); + describe('fetch.tracks', () => { + testSimpleFetch( + api.fetch.tracks, + urls.tracks, + 'fetches from urls.tracks', + ); + }); + describe('fetch.gradebookData', () => { + const searchText = 'some user'; + const cohort = 2; + const track = 'masters'; + const options = { + courseGradeMax: 90, + courseGradeMin: 10, + includeCourseRoleMembers: true, + assignment: 'some work', + assignmentGradeMax: 95, + assignmentGradeMin: 5, + }; + + it('throws an error if either assignmentGrade limit is set, but no assignment', () => { + mockGet(resolveFn); + expect(() => { + api.fetch.gradebookData( + searchText, + cohort, + track, + { ...options, assignmentGradeMax: null, assignment: null }, + ); + }).toThrow(Error(messages.errors.missingAssignment)); + expect(() => { + api.fetch.gradebookData( + searchText, + cohort, + track, + { ...options, assignmentGradeMin: null, assignment: null }, + ); + }).toThrow(Error(messages.errors.missingAssignment)); + }); + describe('fetches from urls.gradebook with queryParams loaded from options', () => { + beforeEach(() => { + mockGet(resolveFn); + }); + test('loads only passed values if options is empty', () => ( + api.fetch.gradebookData(searchText, cohort, track).then(({ data }) => { + expect(data).toEqual(utils.stringifyUrl(urls.gradebook, { + [paramKeys.pageSize]: pageSize, + [paramKeys.userContains]: searchText, + [paramKeys.cohortId]: cohort, + [paramKeys.enrollmentMode]: track, + [paramKeys.courseGradeMax]: undefined, + [paramKeys.courseGradeMin]: undefined, + [paramKeys.excludedCourseRoles]: undefined, + [paramKeys.assignment]: undefined, + [paramKeys.assignmentGradeMax]: undefined, + [paramKeys.assignmentGradeMin]: undefined, + })); + }) + )); + test('loads ["all"] for excludedCorseRoles if not includeCourseRoles', () => ( + api.fetch.gradebookData(searchText, cohort, track, options).then(({ data }) => { + expect(data).toEqual(utils.stringifyUrl(urls.gradebook, { + [paramKeys.pageSize]: pageSize, + [paramKeys.userContains]: searchText, + [paramKeys.cohortId]: cohort, + [paramKeys.enrollmentMode]: track, + [paramKeys.courseGradeMax]: options.courseGradeMax, + [paramKeys.courseGradeMin]: options.courseGradeMin, + [paramKeys.excludedCourseRoles]: ['all'], + [paramKeys.assignment]: options.assignment, + [paramKeys.assignmentGradeMax]: options.assignmentGradeMax, + [paramKeys.assignmentGradeMin]: options.assignmentGradeMin, + })); + }) + )); + test('loads null for excludedCorseRoles if includeCourseRoles', () => ( + api.fetch.gradebookData(searchText, cohort, track, options).then(({ data }) => { + expect(data).toEqual(utils.stringifyUrl(urls.gradebook, { + [paramKeys.pageSize]: pageSize, + [paramKeys.userContains]: searchText, + [paramKeys.cohortId]: cohort, + [paramKeys.enrollmentMode]: track, + [paramKeys.courseGradeMax]: options.courseGradeMax, + [paramKeys.courseGradeMin]: options.courseGradeMin, + [paramKeys.excludedCourseRoles]: null, + [paramKeys.assignment]: options.assignment, + [paramKeys.assignmentGradeMax]: options.assignmentGradeMax, + [paramKeys.assignmentGradeMin]: options.assignmentGradeMin, + })); + }) + )); + }); + }); + describe('gradeBulkOperationHistory', () => { + describe('success', () => { + beforeEach(() => { + mockGet(resolveFn); + }); + it('fetches from urls.bulkHistory and returns the data', () => ( + api.fetch.gradeBulkOperationHistory().then(url => { + expect(url).toEqual(urls.bulkHistory); + }) + )); + }); + describe('failure', () => { + beforeEach(() => { + mockGet(rejectFn); + }); + it('rejects with unhandledResponse Error', () => ( + api.fetch.gradeBulkOperationHistory().catch(error => { + expect(error).toEqual(Error(messages.errors.unhandledResponse)); + }) + )); + }); + }); + describe('gradeOverrideHistory', () => { + const subsectionId = 'a subsection'; + const userId = 'Thomas'; + beforeEach(() => { + mockGet(resolveFn); + }); + test('gets from urls.sectionOverrideHistoryUrl with passed subseciton and user ids', () => ( + api.fetch.gradeOverrideHistory(subsectionId, userId).then(({ data }) => { + expect(data).toEqual(sectionOverrideHistoryUrl(subsectionId, userId)); + }) + )); + }); + }); + describe('post actions', () => { + const mockPost = promiseFn => { + jest.spyOn(utils, 'post').mockImplementation( + (url, callback) => new Promise(promiseFn(url, callback)), + ); + }; + const resolveFn = (url, data) => (resolve) => resolve({ data: { url, data } }); + describe('updateGradebookData', () => { + const updateData = { some: 'update data' }; + beforeEach(() => { + mockPost(resolveFn); + }); + test('posts to urls.bulkUpdate with passed data', () => ( + api.updateGradebookData(updateData).then(({ data }) => { + expect(data).toEqual({ url: urls.bulkUpdate, data: updateData }); + }) + )); + }); + describe('uploadGradeCsv', () => { + describe('200 status with no error_messages', () => { + const response = { + status: 200, + data: { + error_messages: [], + other: 'data', + }, + }; + const formData = { some: 'form Data' }; + beforeEach(() => { + mockPost(() => (resolve) => { resolve(response); }); + }); + it('posts formData to gradeCsvUrl and returns the data from response', () => ( + api.uploadGradeCsv(formData).then(result => { + expect(result).toEqual(response.data); + }) + )); + }); + describe('non-200 status', () => { + const formData = { some: 'form Data' }; + beforeEach(() => { + mockPost((url, data) => (resolve) => { resolve({ url, data }); }); + }); + it('posts formData to gradeCsvUrl and returns the data from response', () => ( + api.uploadGradeCsv(formData).catch(result => { + expect(result).toEqual({ url: gradeCsvUrl(), data: formData }); + }) + )); + }); + }); + }); +}); diff --git a/src/data/services/lms/constants.js b/src/data/services/lms/constants.js new file mode 100644 index 0000000..eb3d223 --- /dev/null +++ b/src/data/services/lms/constants.js @@ -0,0 +1,17 @@ +import { StrictDict } from 'utils'; + +export const pageSize = 25; +export const historyRecordLimit = 5; + +export const paramKeys = StrictDict({ + cohortId: 'cohort_id', + pageSize: 'page_size', + userContains: 'user_contains', + enrollmentMode: 'enrollment_mode', + assignment: 'assignment', + assignmentGradeMin: 'assignment_grade_min', + assignmentGradeMax: 'assignment_grade_max', + courseGradeMin: 'course_grade_min', + courseGradeMax: 'course_grade_max', + excludedCourseRoles: 'excluded_course_roles', +}); diff --git a/src/data/services/lms/index.js b/src/data/services/lms/index.js new file mode 100644 index 0000000..c79aa5f --- /dev/null +++ b/src/data/services/lms/index.js @@ -0,0 +1,8 @@ +import { StrictDict } from 'utils'; +import api from './api'; +import urls from './urls'; + +export default StrictDict({ + api, + urls, +}); diff --git a/src/data/services/lms/messages.js b/src/data/services/lms/messages.js new file mode 100644 index 0000000..7ace8bc --- /dev/null +++ b/src/data/services/lms/messages.js @@ -0,0 +1,10 @@ +import { StrictDict } from 'utils'; + +export default StrictDict({ + errors: { + missingAssignment: ( + 'Gradebook LMS API requires assignment to be set to filter by min/max assig. grade' + ), + unhandledResponse: 'unhandled response error', + }, +}); diff --git a/src/data/services/lms/urls.js b/src/data/services/lms/urls.js new file mode 100644 index 0000000..3d13ddb --- /dev/null +++ b/src/data/services/lms/urls.js @@ -0,0 +1,61 @@ +import { StrictDict } from 'utils'; +import { configuration } from 'config'; +import { historyRecordLimit } from './constants'; +import { filterQuery, stringifyUrl } from './utils'; + +const baseUrl = `${configuration.LMS_BASE_URL}`; + +const courseId = window.location.pathname.slice(1); + +const api = `${baseUrl}/api/`; +const bulkGrades = `${api}bulk_grades/course/${courseId}/`; +const enrollment = `${api}enrollment/v1/`; +const grades = `${api}grades/v1/`; +const gradebook = `${grades}gradebook/${courseId}/`; +const bulkUpdate = `${gradebook}bulk-update`; +const intervention = `${bulkGrades}intervention/`; + +const cohorts = `${baseUrl}courses/${courseId}/cohorts/`; +const tracks = `${enrollment}course/${courseId}?include_expired=1`; +const bulkHistory = `${bulkGrades}history/`; + +const assignmentTypes = stringifyUrl(`${gradebook}grading-info`, { graded_only: true }); +const roles = stringifyUrl(`${enrollment}roles/`, { courseId }); + +/** + * bulkGradesUrlByCourseAndRow(courseId, rowId) + * returns the bulkGrades url with the given rowId. + * @param {string} rowId - row/error identifier + * @return {string} - bulk grades fetch url + */ +export const bulkGradesUrlByRow = (rowId) => stringifyUrl(bulkGrades, { error_id: rowId }); + +export const gradeCsvUrl = (options = {}) => stringifyUrl(bulkGrades, filterQuery(options)); + +export const interventionExportCsvUrl = (options = {}) => ( + stringifyUrl(intervention, filterQuery(options)) +); + +export const sectionOverrideHistoryUrl = (subsectionId, userId) => stringifyUrl( + `${grades}subsection/${subsectionId}/`, + { user_id: userId, history_record_limit: historyRecordLimit }, +); + +export default StrictDict({ + assignmentTypes, + bulkGrades, + bulkHistory, + bulkUpdate, + cohorts, + enrollment, + grades, + gradebook, + intervention, + roles, + tracks, + + bulkGradesUrlByRow, + gradeCsvUrl, + interventionExportCsvUrl, + sectionOverrideHistoryUrl, +}); diff --git a/src/data/services/lms/urls.test.js b/src/data/services/lms/urls.test.js new file mode 100644 index 0000000..9c509cd --- /dev/null +++ b/src/data/services/lms/urls.test.js @@ -0,0 +1,62 @@ +import { historyRecordLimit } from './constants'; +import * as utils from './utils'; +import urls, { + bulkGradesUrlByRow, + gradeCsvUrl, + interventionExportCsvUrl, + sectionOverrideHistoryUrl, +} from './urls'; + +jest.mock('./utils', () => ({ + filterQuery: jest.fn(options => ({ filterQuery: options })), + stringifyUrl: jest.fn((url, query) => ({ url, query })), +})); + +describe('lms api url methods', () => { + describe('bulkGradesUrlByRow', () => { + it('returns bulkGrades url with error_id', () => { + const id = 'heyo'; + expect(bulkGradesUrlByRow(id)).toEqual( + utils.stringifyUrl(urls.bulkGrades, { error_id: id }), + ); + }); + }); + describe('gradeCsvUrl', () => { + it('returns bulkGrades with filterQuery-loaded options as query', () => { + const options = { some: 'fun', query: 'options' }; + expect(gradeCsvUrl(options)).toEqual( + utils.stringifyUrl(urls.bulkGrades, utils.filterQuery(options)), + ); + }); + it('defaults options to empty object', () => { + expect(gradeCsvUrl()).toEqual( + utils.stringifyUrl(urls.bulkGrades, utils.filterQuery({})), + ); + }); + }); + describe('interventionExportCsvUrl', () => { + it('returns intervention url with filterQuery-loaded options as query', () => { + const options = { some: 'fun', query: 'options' }; + expect(interventionExportCsvUrl(options)).toEqual( + utils.stringifyUrl(urls.intervention, utils.filterQuery(options)), + ); + }); + it('defaults options to empty object', () => { + expect(interventionExportCsvUrl()).toEqual( + utils.stringifyUrl(urls.intervention, utils.filterQuery({})), + ); + }); + }); + describe('sectionOverrideHistoryUrl', () => { + it('returns grades url with subsection id, and user_id/history_record_limit query', () => { + const subsectionId = 'a sub section'; + const userId = 'Tom'; + expect(sectionOverrideHistoryUrl(subsectionId, userId)).toEqual( + utils.stringifyUrl( + `${urls.grades}subsection/${subsectionId}/`, + { user_id: userId, history_record_limit: historyRecordLimit }, + ), + ); + }); + }); +}); diff --git a/src/data/services/lms/utils.js b/src/data/services/lms/utils.js new file mode 100644 index 0000000..562da3d --- /dev/null +++ b/src/data/services/lms/utils.js @@ -0,0 +1,42 @@ +import queryString from 'query-string'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import { filters } from 'data/constants/filters'; + +/** + * get(url) + * simple wrapper providing an authenticated Http client get action + * @param {string} url - target url + */ +export const get = (...args) => getAuthenticatedHttpClient().get(...args); +/** + * post(url, data) + * simple wrapper providing an authenticated Http client post action + * @param {string} url - target url + * @param {object|string} data - post payload + */ +export const post = (...args) => getAuthenticatedHttpClient().post(...args); + +/** + * stringifyUrl(url, query) + * simple wrapper around queryString.stringifyUrl that sets skip behavior + * @param {string} url - base url string + * @param {object} query - query parameters + */ +export const stringifyUrl = (url, query) => queryString.stringifyUrl( + { url, query }, + { skipNull: true, skipEmptyString: true }, +); + +/** + * filterQuery(options) + * Takes current filter object and returns it with only valid filters that are + * set and have non-'All' values + * @param {object} options - filter values + * @return {object} - valid filters that are set and do not equal 'All' + */ +export const filterQuery = (options) => Object.values(filters) + .filter(filter => options[filter] && options[filter] !== 'All') + .reduce( + (obj, filter) => ({ ...obj, [filter]: options[filter] }), + {}, + ); diff --git a/src/data/services/lms/utils.test.js b/src/data/services/lms/utils.test.js new file mode 100644 index 0000000..7aec18c --- /dev/null +++ b/src/data/services/lms/utils.test.js @@ -0,0 +1,97 @@ +import queryString from 'query-string'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import { filters } from 'data/constants/filters'; +import * as utils from './utils'; + +jest.mock('query-string', () => ({ + stringifyUrl: jest.fn((url, options) => ({ url, options })), +})); +jest.mock('@edx/frontend-platform/auth', () => ({ + getAuthenticatedHttpClient: jest.fn(), +})); + +describe('lms service utils', () => { + describe('get', () => { + it('forwards arguments to authenticatedHttpClient().get', () => { + const get = jest.fn((...args) => ({ get: args })); + getAuthenticatedHttpClient.mockReturnValue({ get }); + const args = ['some', 'args', 'for', 'the', 'test']; + expect(utils.get(...args)).toEqual(get(...args)); + }); + }); + describe('post', () => { + it('forwards arguments to authenticatedHttpClient().post', () => { + const post = jest.fn((...args) => ({ post: args })); + getAuthenticatedHttpClient.mockReturnValue({ post }); + const args = ['some', 'args', 'for', 'the', 'test']; + expect(utils.post(...args)).toEqual(post(...args)); + }); + }); + describe('stringifyUrl', () => { + it('forwards url and query to stringifyUrl with options to skip null and ""', () => { + const url = 'here.com'; + const query = { some: 'set', of: 'queryParams' }; + const options = { skipNull: true, skipEmptyString: true }; + expect(utils.stringifyUrl(url, query)).toEqual( + queryString.stringifyUrl({ url, query }, options), + ); + }); + }); + describe('filterQuery', () => { + it('returns all filters included in validated list that are not "All"', () => { + const goodOptions = { + [filters.assignmentType]: 'quiz', + [filters.courseGradeMax]: 100, + [filters.courseGradeMin]: 1, + }; + const extraOptions = { + fake: 'option', + another: 'fake one', + }; + expect(utils.filterQuery({ + ...goodOptions, + ...extraOptions, + [filters.includeCourseRoleMembers]: 'All', + })).toEqual(goodOptions); + }); + }); +}); + +/** + * get(url) + * simple wrapper providing an authenticated Http client get action + * @param {string} url - target url + */ +export const get = (...args) => getAuthenticatedHttpClient().get(...args); +/** + * post(url, data) + * simple wrapper providing an authenticated Http client post action + * @param {string} url - target url + * @param {object|string} data - post payload + */ +export const post = (...args) => getAuthenticatedHttpClient().post(...args); + +/** + * stringifyUrl(url, query) + * simple wrapper around queryString.stringifyUrl that sets skip behavior + * @param {string} url - base url string + * @param {object} query - query parameters + */ +export const stringifyUrl = (url, query) => queryString.stringifyUrl( + { url, query }, + { skipNull: true, skipEmptyString: true }, +); + +/** + * filterQuery(options) + * Takes current filter object and returns it with only valid filters that are + * set and have non-'All' values + * @param {object} options - filter values + * @return {object} - valid filters that are set and do not equal 'All' + */ +export const filterQuery = (options) => Object.values(filters) + .filter(filter => options[filter] && options[filter] !== 'All') + .reduce( + (obj, filter) => ({ ...obj, [filter]: options[filter] }), + {}, + ); diff --git a/src/data/thunkActions/assignmentTypes.js b/src/data/thunkActions/assignmentTypes.js index a43eebe..b0c9494 100644 --- a/src/data/thunkActions/assignmentTypes.js +++ b/src/data/thunkActions/assignmentTypes.js @@ -1,16 +1,15 @@ /* eslint-disable import/prefer-default-export */ import { StrictDict } from 'utils'; import actions from 'data/actions'; -import selectors from 'data/selectors'; -import LmsApiService from 'data/services/LmsApiService'; +import lms from 'data/services/lms'; const { fetching, gotGradesFrozen } = actions.assignmentTypes; const { gotBulkManagementConfig } = actions.config; export const fetchAssignmentTypes = () => ( - (dispatch, getState) => { + (dispatch) => { dispatch(fetching.started()); - return LmsApiService.fetchAssignmentTypes(selectors.app.courseId(getState())) + return lms.api.fetch.assignmentTypes() .then(response => response.data) .then((data) => { dispatch(fetching.received(Object.keys(data.assignment_types))); diff --git a/src/data/thunkActions/assignmentTypes.test.js b/src/data/thunkActions/assignmentTypes.test.js index 7c19eb4..3cfd27e 100644 --- a/src/data/thunkActions/assignmentTypes.test.js +++ b/src/data/thunkActions/assignmentTypes.test.js @@ -1,12 +1,15 @@ -import LmsApiService from '../services/LmsApiService'; +import lms from '../services/lms'; import actions from '../actions'; -import selectors from '../selectors'; import * as thunkActions from './assignmentTypes'; import { createTestFetcher } from './testUtils'; -jest.mock('../services/LmsApiService', () => ({ - fetchAssignmentTypes: jest.fn(), +jest.mock('data/services/lms', () => ({ + api: { + fetch: { + assignmentTypes: jest.fn(), + }, + }, })); const responseData = { @@ -19,16 +22,12 @@ const responseData = { }; describe('assignmentType thunkActions', () => { - const courseId = 'course-v1:edX+DemoX+Demo_Course'; - beforeEach(() => { - selectors.app.courseId = jest.fn(() => courseId); - }); describe('fetchAssignmentTypes', () => { const testFetch = createTestFetcher( - LmsApiService.fetchAssignmentTypes, + lms.api.fetch.assignmentTypes, thunkActions.fetchAssignmentTypes, [], - () => expect(LmsApiService.fetchAssignmentTypes).toHaveBeenCalledWith(courseId), + () => expect(lms.api.fetch.assignmentTypes).toHaveBeenCalledWith(), ); describe('actions dispatched on valid response', () => { const actionNames = [ diff --git a/src/data/thunkActions/cohorts.js b/src/data/thunkActions/cohorts.js index 0a5da57..c32cfa4 100644 --- a/src/data/thunkActions/cohorts.js +++ b/src/data/thunkActions/cohorts.js @@ -1,13 +1,12 @@ /* eslint-disable import/prefer-default-export */ import { StrictDict } from 'utils'; import actions from 'data/actions'; -import selectors from 'data/selectors'; -import LmsApiService from 'data/services/LmsApiService'; +import lms from 'data/services/lms'; export const fetchCohorts = () => ( - (dispatch, getState) => { + (dispatch) => { dispatch(actions.cohorts.fetching.started()); - return LmsApiService.fetchCohorts(selectors.app.courseId(getState())) + return lms.api.fetch.cohorts() .then(response => response.data) .then((data) => { dispatch(actions.cohorts.fetching.received(data.cohorts)); diff --git a/src/data/thunkActions/cohorts.test.js b/src/data/thunkActions/cohorts.test.js index 3858dce..05da1db 100644 --- a/src/data/thunkActions/cohorts.test.js +++ b/src/data/thunkActions/cohorts.test.js @@ -1,12 +1,13 @@ -import LmsApiService from '../services/LmsApiService'; +import lms from 'data/services/lms'; -import actions from '../actions'; -import selectors from '../selectors'; +import actions from 'data/actions'; import * as thunkActions from './cohorts'; import { createTestFetcher } from './testUtils'; -jest.mock('../services/LmsApiService', () => ({ - fetchCohorts: jest.fn(), +jest.mock('data/services/lms', () => ({ + api: { + fetch: { cohorts: jest.fn() }, + }, })); const responseData = { @@ -17,16 +18,12 @@ const responseData = { }; describe('cohorts thunkActions', () => { - const courseId = 'course-v1:edX+DemoX+Demo_Course'; - beforeEach(() => { - selectors.app.courseId = jest.fn(() => courseId); - }); describe('fetchCohorts', () => { const testFetch = createTestFetcher( - LmsApiService.fetchCohorts, + lms.api.fetch.cohorts, thunkActions.fetchCohorts, [], - () => expect(LmsApiService.fetchCohorts).toHaveBeenCalledWith(courseId), + () => expect(lms.api.fetch.cohorts).toHaveBeenCalledWith(), ); describe('actions dispatched on valid response', () => { test('fetching.started, fetching.received', () => testFetch( diff --git a/src/data/thunkActions/grades.js b/src/data/thunkActions/grades.js index 6a67f8f..1087857 100644 --- a/src/data/thunkActions/grades.js +++ b/src/data/thunkActions/grades.js @@ -1,4 +1,4 @@ -/* eslint-disable import/no-self-import */ +/* eslint-disable import/no-self-import, import/no-named-as-default-member */ import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { StrictDict } from 'utils'; @@ -8,7 +8,7 @@ import GRADE_OVERRIDE_HISTORY_ERROR_DEFAULT_MSG from 'data/constants/errors'; import grades from 'data/actions/grades'; import { sortAlphaAsc } from 'data/actions/utils'; import selectors from 'data/selectors'; -import LmsApiService from 'data/services/LmsApiService'; +import lms from 'data/services/lms'; import * as module from './grades'; @@ -16,13 +16,12 @@ const { formatGradeOverrideForDisplay } = selectors.grades; export const defaultAssignmentFilter = 'All'; -export const fetchBulkUpgradeHistory = () => (dispatch, getState) => { +export const fetchBulkUpgradeHistory = () => (dispatch) => ( // todo add loading effect - const courseId = selectors.app.courseId(getState()); - return LmsApiService.fetchGradeBulkOperationHistory(courseId).then( + lms.api.fetch.gradeBulkOperationHistory().then( (response) => { dispatch(grades.bulkHistory.received(response)); }, - ).catch(() => dispatch(grades.bulkHistory.error())); -}; + ).catch(() => dispatch(grades.bulkHistory.error())) +); export const fetchGrades = (overrides = {}) => ( (dispatch, getState) => { @@ -35,8 +34,7 @@ export const fetchGrades = (overrides = {}) => ( ...selectors.root.localFilters(getState()), ...options, }; - return LmsApiService.fetchGradebookData( - courseId, + return lms.api.fetch.gradebookData( fetchOptions.searchText || null, cohort, track, @@ -74,7 +72,7 @@ export const fetchGradesIfAssignmentGradeFiltersSet = () => ( ); export const fetchGradeOverrideHistory = (subsectionId, userId) => ( - dispatch => LmsApiService.fetchGradeOverrideHistory(subsectionId, userId) + dispatch => lms.api.fetch.gradeOverrideHistory(subsectionId, userId) .then(response => response.data) .then((data) => { if (data.success) { @@ -130,7 +128,7 @@ export const submitFileUploadFormData = (formData) => ( (dispatch, getState) => { const courseId = selectors.app.courseId(getState()); dispatch(grades.csvUpload.started()); - return LmsApiService.uploadGradeCsv(courseId, formData).then(() => { + return lms.api.uploadGradeCsv(formData).then(() => { dispatch(grades.csvUpload.finished()); dispatch(grades.uploadOverride.success(courseId)); }).catch((error) => { @@ -146,10 +144,9 @@ export const submitFileUploadFormData = (formData) => ( export const updateGrades = () => ( (dispatch, getState) => { - const courseId = selectors.app.courseId(getState()); const updateData = selectors.app.editUpdateData(getState()); dispatch(grades.update.request()); - return LmsApiService.updateGradebookData(courseId, updateData) + return lms.api.updateGradebookData(updateData) .then(response => response.data) .then((data) => { dispatch(grades.update.success({ data })); diff --git a/src/data/thunkActions/grades.test.js b/src/data/thunkActions/grades.test.js index be6d9da..ee6f15f 100644 --- a/src/data/thunkActions/grades.test.js +++ b/src/data/thunkActions/grades.test.js @@ -2,12 +2,13 @@ import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import * as auth from '@edx/frontend-platform/auth'; + +import GRADE_OVERRIDE_HISTORY_ERROR_DEFAULT_MSG from 'data/constants/errors'; +import actions from 'data/actions'; +import { sortAlphaAsc } from 'data/actions/utils'; +import lms from 'data/services/lms'; +import selectors from 'data/selectors'; import * as thunkActions from './grades'; -import actions from '../actions'; -import GRADE_OVERRIDE_HISTORY_ERROR_DEFAULT_MSG from '../constants/errors'; -import { sortAlphaAsc } from '../actions/utils'; -import LmsApiService from '../services/LmsApiService'; -import selectors from '../selectors'; import { createTestFetcher } from './testUtils'; @@ -68,19 +69,25 @@ const allFilters = { const testVal = 'A Test VALue'; -jest.mock('../services/LmsApiService', () => ({ - fetchGradebookData: jest.fn(), - fetchGradeBulkOperationHistory: jest.fn(), - fetchGradeOverrideHistory: jest.fn(), - fetchPrevNextGrades: jest.fn(), - updateGradebookData: jest.fn(), - updateGrades: jest.fn(), - uploadGradeCsv: jest.fn(), +jest.mock('data/services/lms', () => ({ + __esModule: true, + default: { + api: { + fetch: { + gradebookData: jest.fn(), + gradeBulkOperationHistory: jest.fn(), + gradeOverrideHistory: jest.fn(), + prevNextGrades: jest.fn(), + }, + updateGradebookData: jest.fn(), + uploadGradeCsv: jest.fn(), + }, + }, })); jest.mock('@edx/frontend-platform/auth', () => ({ getAuthenticatedHttpClient: jest.fn(), })); -jest.mock('../selectors', () => ({ +jest.mock('data/selectors', () => ({ __esModule: true, default: { grades: { @@ -94,7 +101,9 @@ jest.mock('../selectors', () => ({ assignmentGradeMin: jest.fn(), assignmentGradeMax: jest.fn(), }, - app: {}, + app: { + courseId: jest.fn(), + }, root: {}, }, })); @@ -103,11 +112,10 @@ selectors.filters.allFilters.mockReturnValue(allFilters); describe('grades thunkActions', () => { beforeEach(() => { - LmsApiService.fetchGradebookData.mockClear(); - LmsApiService.fetchGradeBulkOperationHistory.mockClear(); - LmsApiService.fetchGradeOverrideHistory.mockClear(); - LmsApiService.fetchPrevNextGrades.mockClear(); - LmsApiService.updateGrades.mockClear(); + lms.api.fetch.gradebookData.mockClear(); + lms.api.fetch.gradeBulkOperationHistory.mockClear(); + lms.api.fetch.gradeOverrideHistory.mockClear(); + lms.api.fetch.prevNextGrades.mockClear(); selectors.app.courseId = jest.fn(() => courseId); selectors.filters.cohort = jest.fn(() => cohort); selectors.filters.track = jest.fn(() => track); @@ -115,10 +123,10 @@ describe('grades thunkActions', () => { }); describe('fetchBulkUpgradeHistory', () => { const testFetch = createTestFetcher( - LmsApiService.fetchGradeBulkOperationHistory, + lms.api.fetch.gradeBulkOperationHistory, thunkActions.fetchBulkUpgradeHistory, [], - () => expect(LmsApiService.fetchGradeBulkOperationHistory).toHaveBeenCalledWith(courseId), + () => expect(lms.api.fetch.gradeBulkOperationHistory).toHaveBeenCalledWith(), ); describe('valid response', () => { it('dispatches bulkHistory.received with response', () => { @@ -154,13 +162,13 @@ describe('grades thunkActions', () => { }; const testFetch = ({ apiArgs, overrides }) => createTestFetcher( - LmsApiService.fetchGradebookData, + lms.api.fetch.gradebookData, thunkActions.fetchGrades, [overrides], () => { if (apiArgs !== undefined) { expect( - LmsApiService.fetchGradebookData, + lms.api.fetch.gradebookData, ).toHaveBeenCalledWith(...apiArgs); } }, @@ -179,10 +187,10 @@ describe('grades thunkActions', () => { describe('fetchGradebookData args', () => { describe('searchText is not empty', () => { const options = { to: 'be', or: 'not', searchText: '' }; - test('courseId, searchText, cohort, track, and merged localFilters and options', () => ( + test('searchText, cohort, track, and merged localFilters and options', () => ( testFetch({ overrides: { options }, - apiArgs: [courseId, null, cohort, track, { ...localFilters, ...options }], + apiArgs: [null, cohort, track, { ...localFilters, ...options }], })(resolve => resolve({ data: responseData })) )); }); @@ -190,7 +198,7 @@ describe('grades thunkActions', () => { const options = { to: 'be', or: 'not', searchText: '' }; test('null searchText', () => testFetch({ overrides: { options }, - apiArgs: [courseId, null, cohort, track, { ...localFilters, ...options }], + apiArgs: [null, cohort, track, { ...localFilters, ...options }], })(resolve => resolve({ data: responseData }))); }); }); @@ -270,12 +278,12 @@ describe('grades thunkActions', () => { const history = { some: 'history' }; const testFetch = createTestFetcher( - LmsApiService.fetchGradeOverrideHistory, + lms.api.fetch.gradeOverrideHistory, thunkActions.fetchGradeOverrideHistory, [subsectionId, userId], () => { expect( - LmsApiService.fetchGradeOverrideHistory, + lms.api.fetch.gradeOverrideHistory, ).toHaveBeenCalledWith(subsectionId, userId); }, ); @@ -402,12 +410,12 @@ describe('grades thunkActions', () => { const updateData = { some: 'data' }; const gradebookData = { data: { OTher: 'DATA' } }; const testFetch = createTestFetcher( - LmsApiService.updateGradebookData, + lms.api.updateGradebookData, thunkActions.updateGrades, [], () => expect( - LmsApiService.updateGradebookData, - ).toHaveBeenCalledWith(courseId, updateData), + lms.api.updateGradebookData, + ).toHaveBeenCalledWith(updateData), ); beforeEach(() => { selectors.app.editUpdateData = jest.fn(() => updateData); @@ -511,10 +519,10 @@ describe('grades thunkActions', () => { describe('submitFileUploadFormData', () => { const formData = { form: 'data' }; const testFetch = createTestFetcher( - LmsApiService.uploadGradeCsv, + lms.api.uploadGradeCsv, thunkActions.submitFileUploadFormData, [formData], - () => expect(LmsApiService.uploadGradeCsv).toHaveBeenCalledWith(courseId, formData), + () => expect(lms.api.uploadGradeCsv).toHaveBeenCalledWith(formData), ); const { csvUpload, uploadOverride } = actions.grades; describe('valid data', () => { diff --git a/src/data/thunkActions/roles.js b/src/data/thunkActions/roles.js index 7c86a5b..84777c5 100644 --- a/src/data/thunkActions/roles.js +++ b/src/data/thunkActions/roles.js @@ -3,19 +3,19 @@ import { StrictDict } from 'utils'; import roles from 'data/actions/roles'; import selectors from 'data/selectors'; +import lms from 'data/services/lms'; + import { fetchCohorts } from './cohorts'; import { fetchGrades } from './grades'; import { fetchTracks } from './tracks'; import { fetchAssignmentTypes } from './assignmentTypes'; -import LmsApiService from '../services/LmsApiService'; - export const allowedRoles = ['staff', 'instructor', 'support']; export const fetchRoles = () => ( (dispatch, getState) => { const courseId = selectors.app.courseId(getState()); - return LmsApiService.fetchUserRoles(courseId) + return lms.api.fetch.roles() .then(response => response.data) .then((response) => { const isAllowedRole = (role) => ( diff --git a/src/data/thunkActions/roles.test.js b/src/data/thunkActions/roles.test.js index 1d2bbed..bae4967 100644 --- a/src/data/thunkActions/roles.test.js +++ b/src/data/thunkActions/roles.test.js @@ -1,17 +1,15 @@ -import { createTestFetcher } from './testUtils'; - -import LmsApiService from '../services/LmsApiService'; -import actions from '../actions'; -import selectors from '../selectors'; +import lms from 'data/services/lms'; +import actions from 'data/actions'; +import selectors from 'data/selectors'; import { fetchAssignmentTypes } from './assignmentTypes'; import { fetchCohorts } from './cohorts'; import { fetchGrades } from './grades'; import { fetchTracks } from './tracks'; - import { allowedRoles, fetchRoles } from './roles'; +import { createTestFetcher } from './testUtils'; -jest.mock('../selectors', () => ({ +jest.mock('data/selectors', () => ({ __esModule: true, default: { filters: { @@ -20,8 +18,10 @@ jest.mock('../selectors', () => ({ app: {}, }, })); -jest.mock('../services/LmsApiService', () => ({ - fetchUserRoles: jest.fn(), +jest.mock('data/services/lms', () => ({ + api: { + fetch: { roles: jest.fn() }, + }, })); jest.mock('./assignmentTypes', () => ({ fetchAssignmentTypes: jest.fn((...args) => ({ type: 'fetchAssignmentTypes', args })), @@ -58,10 +58,10 @@ describe('roles thunkActions', () => { }); describe('fetchRoles', () => { const testFetch = createTestFetcher( - LmsApiService.fetchUserRoles, + lms.api.fetch.roles, fetchRoles, [], - () => expect(LmsApiService.fetchUserRoles).toHaveBeenCalledWith(courseId), + () => expect(lms.api.fetch.roles).toHaveBeenCalledWith(), ); describe('valid response', () => { describe('cannot view gradebook (not is_staff, and no allowed roles)', () => { diff --git a/src/data/thunkActions/tracks.js b/src/data/thunkActions/tracks.js index 188e58c..4bccb86 100644 --- a/src/data/thunkActions/tracks.js +++ b/src/data/thunkActions/tracks.js @@ -1,27 +1,25 @@ /* eslint-disable import/prefer-default-export */ import { StrictDict } from 'utils'; -import tracks from '../actions/tracks'; -import selectors from '../selectors'; +import lms from 'data/services/lms'; +import actions from 'data/actions'; +import selectors from 'data/selectors'; import { fetchBulkUpgradeHistory } from './grades'; -import LmsApiService from '../services/LmsApiService'; - export const fetchTracks = () => ( - (dispatch, getState) => { - const courseId = selectors.app.courseId(getState()); - dispatch(tracks.fetching.started()); - return LmsApiService.fetchTracks(courseId) + (dispatch) => { + dispatch(actions.tracks.fetching.started()); + return lms.api.fetch.tracks() .then(response => response.data) .then((data) => { - dispatch(tracks.fetching.received(data.course_modes)); + dispatch(actions.tracks.fetching.received(data.course_modes)); if (selectors.tracks.hasMastersTrack(data.course_modes)) { - dispatch(fetchBulkUpgradeHistory(courseId)); + dispatch(fetchBulkUpgradeHistory()); } }) .catch(() => { - dispatch(tracks.fetching.error()); + dispatch(actions.tracks.fetching.error()); }); } ); diff --git a/src/data/thunkActions/tracks.test.js b/src/data/thunkActions/tracks.test.js index 7c70e9d..f4fd07b 100644 --- a/src/data/thunkActions/tracks.test.js +++ b/src/data/thunkActions/tracks.test.js @@ -1,41 +1,38 @@ -import { createTestFetcher } from './testUtils'; +import lms from 'data/services/lms'; +import actions from 'data/actions'; +import selectors from 'data/selectors'; -import LmsApiService from '../services/LmsApiService'; -import actions from '../actions'; -import selectors from '../selectors'; +import { createTestFetcher } from './testUtils'; import { fetchBulkUpgradeHistory } from './grades'; import { fetchTracks } from './tracks'; -jest.mock('../services/LmsApiService', () => ({ - fetchTracks: jest.fn(), +jest.mock('data/services/lms', () => ({ + api: { + fetch: { tracks: jest.fn() }, + }, })); -jest.mock('../selectors', () => ({ +jest.mock('data/selectors', () => ({ __esModule: true, default: { tracks: { hasMastersTrack: jest.fn(() => false) }, - app: { }, }, })); jest.mock('./grades', () => ({ fetchBulkUpgradeHistory: jest.fn((...args) => ({ type: 'fetchBulkUpgradeHistory', args })), })); -const courseId = 'course-v1:edX+DemoX+Demo_Course'; const responseData = { couse_modes: ['some', 'course', 'modes'], }; -describe('tracjs thunkActions', () => { - beforeEach(() => { - selectors.app.courseId = jest.fn(() => courseId); - }); +describe('tracks thunkActions', () => { describe('fetchTracks', () => { const testFetch = createTestFetcher( - LmsApiService.fetchTracks, + lms.api.fetch.tracks, fetchTracks, [], - () => expect(LmsApiService.fetchTracks).toHaveBeenCalledWith(courseId), + () => expect(lms.api.fetch.tracks).toHaveBeenCalledWith(), ); describe('valid response', () => { describe('if not hasMastersTrack(data.course_modes)', () => { @@ -64,14 +61,14 @@ describe('tracjs thunkActions', () => { const expectedActions = [ 'fetching.started', 'fetching.received with course_modes', - 'fetchBulkUpgradeHistory thunkAction with courseId', + 'fetchBulkUpgradeHistory thunkAction', ]; test(`[${expectedActions.join(', ')}]`, () => testFetch( (resolve) => resolve({ data: responseData }), [ actions.tracks.fetching.started(), actions.tracks.fetching.received(responseData.course_modes), - fetchBulkUpgradeHistory(courseId), + fetchBulkUpgradeHistory(), ], )); });