From cdec99d910b86b794bfb26d6020cc947af5fede1 Mon Sep 17 00:00:00 2001 From: Ghassan Maslamani Date: Tue, 29 Nov 2022 09:44:19 +0200 Subject: [PATCH] feat: ensure lms api synced with latest value in config This change make it possible if LMS url to be changed, that the last value will be picked. This is simlair openedx/frontend-app-course-authoring/pull/389 which issue overhangio/tutor-mfe/issues/86, the fixes is needed so that dynamic config would work with tutor: overhangio/tutor-mfe/pull/69 --- package-lock.json | 4 +- src/data/services/lms/api.js | 14 +++---- src/data/services/lms/api.test.js | 18 ++++----- src/data/services/lms/urls.js | 61 ++++++++++++++---------------- src/data/services/lms/urls.test.js | 12 +++--- 5 files changed, 52 insertions(+), 57 deletions(-) diff --git a/package-lock.json b/package-lock.json index 20b4ee5c..12f5fabb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@edx/frontend-app-gradebook", - "version": "1.5.0", + "version": "1.6.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@edx/frontend-app-gradebook", - "version": "1.5.0", + "version": "1.6.0", "license": "AGPL-3.0", "dependencies": { "@edx/brand": "npm:@edx/brand-edx.org@^1.3.2", diff --git a/src/data/services/lms/api.js b/src/data/services/lms/api.js index 9127c27b..993243d0 100644 --- a/src/data/services/lms/api.js +++ b/src/data/services/lms/api.js @@ -14,10 +14,10 @@ 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); +const assignmentTypes = () => get(urls.getAssignmentTypesUrl()); +const cohorts = () => get(urls.getCohortsUrl()); +const roles = () => get(urls.getRolesUrl()); +const tracks = () => get(urls.getTracksUrl()); /** * fetch.gradebookData(searchText, cohort, track, options) @@ -45,7 +45,7 @@ const gradebookData = (searchText, cohort, track, options = {}) => { [paramKeys.assignmentGradeMax]: options.assignmentGradeMax, [paramKeys.assignmentGradeMin]: options.assignmentGradeMin, }; - return get(stringifyUrl(urls.gradebook, queryParams)); + return get(stringifyUrl(urls.getGradebookUrl(), queryParams)); }; /** @@ -53,7 +53,7 @@ const gradebookData = (searchText, cohort, track, options = {}) => { * fetches bulk operation history and raises an error if the operation fails * @return {Promise} - get response */ -const gradeBulkOperationHistory = () => get(urls.bulkHistory) +const gradeBulkOperationHistory = () => get(urls.getBulkHistoryUrl()) .then(response => response.data) .catch(() => Promise.reject(Error(messages.errors.unhandledResponse))); @@ -87,7 +87,7 @@ const gradeOverrideHistory = (subsectionId, userId) => ( * } * @return {Promise} - post response */ -const updateGradebookData = (updateData) => post(urls.bulkUpdate, updateData); +const updateGradebookData = (updateData) => post(urls.getBulkUpdateUrl(), updateData); /** * uploadGradeCsv(formData) diff --git a/src/data/services/lms/api.test.js b/src/data/services/lms/api.test.js index 4b90ffce..d08fedd6 100644 --- a/src/data/services/lms/api.test.js +++ b/src/data/services/lms/api.test.js @@ -35,28 +35,28 @@ describe('lms service api', () => { describe('fetch.assignmentTypes', () => { testSimpleFetch( api.fetch.assignmentTypes, - urls.assignmentTypes, + urls.getAssignmentTypesUrl(), 'fetches from urls.assignmentTypes', ); }); describe('fetch.cohorts', () => { testSimpleFetch( api.fetch.cohorts, - urls.cohorts, + urls.getCohortsUrl(), 'fetches from urls.cohorts', ); }); describe('fetch.roles', () => { testSimpleFetch( api.fetch.roles, - urls.roles, + urls.getRolesUrl(), 'fetches from urls.roles', ); }); describe('fetch.tracks', () => { testSimpleFetch( api.fetch.tracks, - urls.tracks, + urls.getTracksUrl(), 'fetches from urls.tracks', ); }); @@ -98,7 +98,7 @@ describe('lms service api', () => { }); test('loads only passed values if options is empty', () => ( api.fetch.gradebookData(searchText, cohort, track).then(({ data }) => { - expect(data).toEqual(utils.stringifyUrl(urls.gradebook, { + expect(data).toEqual(utils.stringifyUrl(urls.getGradebookUrl(), { [paramKeys.pageSize]: pageSize, [paramKeys.userContains]: searchText, [paramKeys.cohortId]: cohort, @@ -114,7 +114,7 @@ describe('lms service api', () => { )); test('loads ["all"] for excludedCorseRoles if not includeCourseRoles', () => ( api.fetch.gradebookData(searchText, cohort, track, options).then(({ data }) => { - expect(data).toEqual(utils.stringifyUrl(urls.gradebook, { + expect(data).toEqual(utils.stringifyUrl(urls.getGradebookUrl(), { [paramKeys.pageSize]: pageSize, [paramKeys.userContains]: searchText, [paramKeys.cohortId]: cohort, @@ -130,7 +130,7 @@ describe('lms service api', () => { )); test('loads null for excludedCorseRoles if includeCourseRoles', () => ( api.fetch.gradebookData(searchText, cohort, track, options).then(({ data }) => { - expect(data).toEqual(utils.stringifyUrl(urls.gradebook, { + expect(data).toEqual(utils.stringifyUrl(urls.getGradebookUrl(), { [paramKeys.pageSize]: pageSize, [paramKeys.userContains]: searchText, [paramKeys.cohortId]: cohort, @@ -153,7 +153,7 @@ describe('lms service api', () => { }); it('fetches from urls.bulkHistory and returns the data', () => ( api.fetch.gradeBulkOperationHistory().then(url => { - expect(url).toEqual(urls.bulkHistory); + expect(url).toEqual(urls.getBulkHistoryUrl()); }) )); }); @@ -195,7 +195,7 @@ describe('lms service api', () => { }); test('posts to urls.bulkUpdate with passed data', () => ( api.updateGradebookData(updateData).then(({ data }) => { - expect(data).toEqual({ url: urls.bulkUpdate, data: updateData }); + expect(data).toEqual({ url: urls.getBulkUpdateUrl(), data: updateData }); }) )); }); diff --git a/src/data/services/lms/urls.js b/src/data/services/lms/urls.js index 843dc99a..cb62197e 100644 --- a/src/data/services/lms/urls.js +++ b/src/data/services/lms/urls.js @@ -3,57 +3,52 @@ import { StrictDict } from 'utils'; import { historyRecordLimit } from './constants'; import { filterQuery, stringifyUrl } from './utils'; -const baseUrl = `${getConfig().LMS_BASE_URL}`; - const courseId = window.location.pathname.split('/').filter(Boolean).pop() || ''; -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 }); - +export const getUrlPrefix = () => `${getConfig().LMS_BASE_URL}/api/`; +export const getBulkGradesUrl = () => `${getUrlPrefix()}bulk_grades/course/${courseId}/`; +export const getEnrollmentUrl = () => `${getUrlPrefix()}enrollment/v1/`; +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 getTracksUrl = () => `${getEnrollmentUrl()}course/${courseId}?include_expired=1`; +export const getBulkHistoryUrl = () => `${getBulkUpdateUrl()}history/`; +export const getAssignmentTypesUrl = () => stringifyUrl(`${getGradebookUrl()}grading-info`, { graded_only: true }); +export const getRolesUrl = () => stringifyUrl(`${getEnrollmentUrl()}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 bulkGradesUrlByRow = (rowId) => stringifyUrl(getBulkGradesUrl(), { error_id: rowId }); -export const gradeCsvUrl = (options = {}) => stringifyUrl(bulkGrades, filterQuery(options)); +export const gradeCsvUrl = (options = {}) => stringifyUrl(getBulkGradesUrl(), filterQuery(options)); export const interventionExportCsvUrl = (options = {}) => ( - stringifyUrl(intervention, filterQuery(options)) + stringifyUrl(getInterventionUrl(), filterQuery(options)) ); export const sectionOverrideHistoryUrl = (subsectionId, userId) => stringifyUrl( - `${grades}subsection/${subsectionId}/`, + `${getGradesUrl()}subsection/${subsectionId}/`, { user_id: userId, history_record_limit: historyRecordLimit }, ); export default StrictDict({ - assignmentTypes, - bulkGrades, - bulkHistory, - bulkUpdate, - cohorts, - enrollment, - grades, - gradebook, - intervention, - roles, - tracks, - + getUrlPrefix, + getBulkGradesUrl, + getEnrollmentUrl, + getGradesUrl, + getGradebookUrl, + getBulkUpdateUrl, + getInterventionUrl, + getCohortsUrl, + getTracksUrl, + getBulkHistoryUrl, + getAssignmentTypesUrl, + getRolesUrl, bulkGradesUrlByRow, gradeCsvUrl, interventionExportCsvUrl, diff --git a/src/data/services/lms/urls.test.js b/src/data/services/lms/urls.test.js index 9c509cd4..936839d8 100644 --- a/src/data/services/lms/urls.test.js +++ b/src/data/services/lms/urls.test.js @@ -17,7 +17,7 @@ describe('lms api url methods', () => { it('returns bulkGrades url with error_id', () => { const id = 'heyo'; expect(bulkGradesUrlByRow(id)).toEqual( - utils.stringifyUrl(urls.bulkGrades, { error_id: id }), + utils.stringifyUrl(urls.getBulkGradesUrl(), { error_id: id }), ); }); }); @@ -25,12 +25,12 @@ describe('lms api url methods', () => { 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)), + utils.stringifyUrl(urls.getBulkGradesUrl(), utils.filterQuery(options)), ); }); it('defaults options to empty object', () => { expect(gradeCsvUrl()).toEqual( - utils.stringifyUrl(urls.bulkGrades, utils.filterQuery({})), + utils.stringifyUrl(urls.getBulkGradesUrl(), utils.filterQuery({})), ); }); }); @@ -38,12 +38,12 @@ describe('lms api url methods', () => { 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)), + utils.stringifyUrl(urls.getInterventionUrl(), utils.filterQuery(options)), ); }); it('defaults options to empty object', () => { expect(interventionExportCsvUrl()).toEqual( - utils.stringifyUrl(urls.intervention, utils.filterQuery({})), + utils.stringifyUrl(urls.getInterventionUrl(), utils.filterQuery({})), ); }); }); @@ -53,7 +53,7 @@ describe('lms api url methods', () => { const userId = 'Tom'; expect(sectionOverrideHistoryUrl(subsectionId, userId)).toEqual( utils.stringifyUrl( - `${urls.grades}subsection/${subsectionId}/`, + `${urls.getGradesUrl()}subsection/${subsectionId}/`, { user_id: userId, history_record_limit: historyRecordLimit }, ), );