fix: [AA-1018] api refactor

This is the first step toward clearing out the redundant metadata from the
coursewareMetadata and getting it from a common source - the courseHomeMetadata.

remove username from coursewareMetadata
Remove courseAccess from coursewareMetadata.

Fix all unit tests
Modify classes that use metadataModel to use courseHomeMetadata for common data.
metadataModel still exists as a mechanism to distinguish if a component is under
courseware or courseHome, and it will be renamed or removed in a later refactor.
This commit is contained in:
Chris Deery
2022-01-14 08:54:07 -05:00
parent 6db421eade
commit 3fe5bb1733
25 changed files with 127 additions and 62 deletions

View File

@@ -63,5 +63,4 @@ Factory.define('courseMetadata')
related_programs: null,
user_needs_integrity_signature: false,
recommendations: null,
username: 'MockUser',
});

View File

@@ -83,9 +83,13 @@ export default function buildSimpleCourseAndSequenceMetadata(options = {}) {
courseId, unitBlocks, sequenceBlock: block,
},
));
// need to synchronize the id with the courseMetadata
const courseHomeMetadataOptions = (options.courseHomeMetadata ? options.courseHomeMetadata : {});
const courseHomeMetadata = Factory.build('courseHomeMetadata', courseHomeMetadataOptions);
return {
...simpleCourseBlocks,
courseMetadata,
sequenceMetadata,
courseHomeMetadata,
};
}

View File

@@ -210,7 +210,6 @@ function normalizeMetadata(metadata) {
start: data.start,
enrollmentMode: data.enrollment.mode,
isEnrolled: data.enrollment.is_active,
courseAccess: camelCaseObject(data.course_access),
canViewLegacyCourseware: data.can_view_legacy_courseware,
originalUserIsStaff: data.original_user_is_staff,
isStaff: data.is_staff,
@@ -233,7 +232,6 @@ function normalizeMetadata(metadata) {
isIntegritySignatureEnabled: data.is_integrity_signature_enabled,
userNeedsIntegritySignature: data.user_needs_integrity_signature,
isMasquerading: data.original_user_is_staff && !data.is_staff,
username: data.username,
};
}

View File

@@ -368,14 +368,6 @@ describe('Courseware Service', () => {
start: '2013-02-05T05:00:00Z',
enrollmentMode: 'audit',
isEnrolled: true,
courseAccess: {
hasAccess: true,
errorCode: null,
developerMessage: null,
userMessage: null,
additionalContextUserMessage: null,
userFragment: null,
},
canViewLegacyCourseware: true,
originalUserIsStaff: true,
isStaff: true,

View File

@@ -26,6 +26,7 @@ describe('Data layer integration tests', () => {
// building minimum set of api responses to test all thunks
const courseMetadata = Factory.build('courseMetadata');
const courseId = courseMetadata.id;
const courseHomeMetadata = Factory.build('courseHomeMetadata');
const { courseBlocks, unitBlocks, sequenceBlocks } = buildSimpleCourseBlocks(courseId);
const sequenceMetadata = Factory.build(
'sequenceMetadata',
@@ -37,6 +38,9 @@ describe('Data layer integration tests', () => {
let courseUrl = `${courseBaseUrl}/${courseId}`;
courseUrl = appendBrowserTimezoneToUrl(courseUrl);
const courseHomeMetadataUrl = appendBrowserTimezoneToUrl(
`${getConfig().LMS_BASE_URL}/api/course_home/course_metadata/${courseId}`,
);
const sequenceUrl = `${sequenceBaseUrl}/${sequenceMetadata.item_id}`;
const sequenceId = sequenceBlocks[0].id;
const unitId = unitBlocks[0].id;
@@ -66,17 +70,22 @@ describe('Data layer integration tests', () => {
});
it('Should fetch, normalize, and save metadata, but with denied status', async () => {
const forbiddenCourseMetadata = Factory.build('courseMetadata', {
const forbiddenCourseMetadata = Factory.build('courseMetadata');
const forbiddenCourseHomeMetadata = Factory.build('courseHomeMetadata', {
course_access: {
has_access: false,
},
});
const forbiddenCourseHomeUrl = appendBrowserTimezoneToUrl(
`${getConfig().LMS_BASE_URL}/api/course_home/course_metadata/${courseId}`,
);
const forbiddenCourseBlocks = Factory.build('courseBlocks', {
courseId: forbiddenCourseMetadata.id,
});
let forbiddenCourseUrl = `${courseBaseUrl}/${forbiddenCourseMetadata.id}`;
forbiddenCourseUrl = appendBrowserTimezoneToUrl(forbiddenCourseUrl);
axiosMock.onGet(forbiddenCourseHomeUrl).reply(200, forbiddenCourseHomeMetadata);
axiosMock.onGet(forbiddenCourseUrl).reply(200, forbiddenCourseMetadata);
axiosMock.onGet(courseBlocksUrlRegExp).reply(200, forbiddenCourseBlocks);
axiosMock.onGet(learningSequencesUrlRegExp).reply(403, {});
@@ -88,10 +97,11 @@ describe('Data layer integration tests', () => {
expect(state.courseware.courseStatus).toEqual('denied');
// check that at least one key camel cased, thus course data normalized
expect(state.models.coursewareMeta[forbiddenCourseMetadata.id].courseAccess).not.toBeUndefined();
expect(state.models.courseHomeMeta[forbiddenCourseMetadata.id].courseAccess).not.toBeUndefined();
});
it('Should fetch, normalize, and save metadata', async () => {
axiosMock.onGet(courseHomeMetadataUrl).reply(200, courseHomeMetadata);
axiosMock.onGet(courseUrl).reply(200, courseMetadata);
axiosMock.onGet(courseBlocksUrlRegExp).reply(200, courseBlocks);
axiosMock.onGet(learningSequencesUrlRegExp).reply(403, {});
@@ -106,12 +116,13 @@ describe('Data layer integration tests', () => {
expect(state.courseware.sequenceId).toEqual(null);
// check that at least one key camel cased, thus course data normalized
expect(state.models.coursewareMeta[courseId].courseAccess).not.toBeUndefined();
expect(state.models.coursewareMeta[courseId].verifiedMode).not.toBeUndefined();
});
it('Should fetch, normalize, and save metadata; filtering has no effect', async () => {
// Very similar to previous test, but pass back an outline for filtering
// (even though it won't actually filter down in this case).
axiosMock.onGet(courseHomeMetadataUrl).reply(200, courseHomeMetadata);
axiosMock.onGet(courseUrl).reply(200, courseMetadata);
axiosMock.onGet(courseBlocksUrlRegExp).reply(200, courseBlocks);
axiosMock.onGet(learningSequencesUrlRegExp).reply(200, simpleOutline);
@@ -126,7 +137,7 @@ describe('Data layer integration tests', () => {
expect(state.courseware.sequenceId).toEqual(null);
// check that at least one key camel cased, thus course data normalized
expect(state.models.coursewareMeta[courseId].courseAccess).not.toBeUndefined();
expect(state.models.coursewareMeta[courseId].verifiedMode).not.toBeUndefined();
expect(state.models.sequences.length === 1);
Object.values(state.models.sections).forEach(section => expect(section.sequenceIds.length === 1));
@@ -135,6 +146,7 @@ describe('Data layer integration tests', () => {
it('Should fetch, normalize, and save metadata; filtering removes sequence', async () => {
// Very similar to previous test, but pass back an outline for filtering
// (even though it won't actually filter down in this case).
axiosMock.onGet(courseHomeMetadataUrl).reply(200, courseHomeMetadata);
axiosMock.onGet(courseUrl).reply(200, courseMetadata);
axiosMock.onGet(courseBlocksUrlRegExp).reply(200, courseBlocks);
@@ -153,7 +165,7 @@ describe('Data layer integration tests', () => {
expect(state.courseware.sequenceId).toEqual(null);
// check that at least one key camel cased, thus course data normalized
expect(state.models.coursewareMeta[courseId].courseAccess).not.toBeUndefined();
expect(state.models.coursewareMeta[courseId].verifiedMode).not.toBeUndefined();
expect(state.models.sequences === null);
Object.values(state.models.sections).forEach(section => expect(section.sequenceIds.length === 0));
@@ -187,6 +199,7 @@ describe('Data layer integration tests', () => {
});
it('Should fetch and normalize metadata, and then update existing models with sequence metadata', async () => {
axiosMock.onGet(courseHomeMetadataUrl).reply(200, courseHomeMetadata);
axiosMock.onGet(courseUrl).reply(200, courseMetadata);
axiosMock.onGet(courseBlocksUrlRegExp).reply(200, courseBlocks);
axiosMock.onGet(learningSequencesUrlRegExp).reply(403, {});

View File

@@ -1,13 +1,14 @@
import { logError, logInfo } from '@edx/frontend-platform/logging';
import {
getBlockCompletion,
postSequencePosition,
getCourseMetadata,
getCourseBlocks,
getCourseMetadata,
getLearningSequencesOutline,
getSequenceMetadata,
postIntegritySignature,
postSequencePosition,
} from './api';
import { getCourseHomeCourseMetadata } from '../../course-home/data/api';
import {
updateModel, addModel, updateModelsMap, addModelsMap, updateModels,
} from '../../generic/model-store';
@@ -130,14 +131,34 @@ export function fetchCourse(courseId) {
getCourseMetadata(courseId),
getCourseBlocks(courseId),
getLearningSequencesOutline(courseId),
]).then(([courseMetadataResult, courseBlocksResult, learningSequencesOutlineResult]) => {
getCourseHomeCourseMetadata(courseId),
]).then(([
courseMetadataResult,
courseBlocksResult,
learningSequencesOutlineResult,
courseHomeMetadataResult]) => {
if (courseMetadataResult.status === 'fulfilled') {
dispatch(addModel({
modelType: 'coursewareMeta',
model: courseMetadataResult.value,
model: {
id: courseId,
...courseMetadataResult.value,
},
}));
}
if (courseHomeMetadataResult.status === 'fulfilled') {
dispatch(addModel({
modelType: 'courseHomeMeta',
model: {
id: courseId,
...courseHomeMetadataResult.value,
},
}));
} else {
logError(courseHomeMetadataResult.reason);
}
if (courseBlocksResult.status === 'fulfilled') {
const {
courses, sections, sequences, units,
@@ -167,6 +188,7 @@ export function fetchCourse(courseId) {
}
const fetchedMetadata = courseMetadataResult.status === 'fulfilled';
const fetchedCourseHomeMetadata = courseHomeMetadataResult.status === 'fulfilled';
const fetchedBlocks = courseBlocksResult.status === 'fulfilled';
// Log errors for each request if needed. Course block failures may occur
@@ -185,8 +207,8 @@ export function fetchCourse(courseId) {
logError(courseMetadataResult.reason);
}
if (fetchedMetadata) {
if (courseMetadataResult.value.courseAccess.hasAccess && fetchedBlocks) {
if (fetchedMetadata && fetchedCourseHomeMetadata) {
if (courseHomeMetadataResult.value.courseAccess.hasAccess && fetchedBlocks) {
// User has access
dispatch(fetchCourseSuccess({ courseId }));
return;