From 7bb22667901d521f05a5bc8c94a42362cdf9dd36 Mon Sep 17 00:00:00 2001 From: Michael Terry Date: Fri, 25 Mar 2022 14:13:33 -0400 Subject: [PATCH] fix: stop stripping most inaccessible sequences from navigation I had previously made a "fix" to strip all inaccessible sequences from the learning sequence outline hierarachy, as a way to filter out unreleased sequences. See commit d1f19a9. But that was too big a hammer and stripped a lot of released-but- inaccessible sequences too (e.g. prerequisites). So now, we adopt a more nuanced approach and explicitly just filter out sequences that are both inaccessible AND unreleased. AA-1219 --- .../learningSequencesOutline.factory.js | 2 ++ src/courseware/data/api.js | 36 ++++++++++--------- .../data/pact-tests/lmsPact.test.jsx | 30 ++++++++++++---- 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/courseware/data/__factories__/learningSequencesOutline.factory.js b/src/courseware/data/__factories__/learningSequencesOutline.factory.js index 92f3ad61..24bee16c 100644 --- a/src/courseware/data/__factories__/learningSequencesOutline.factory.js +++ b/src/courseware/data/__factories__/learningSequencesOutline.factory.js @@ -46,6 +46,7 @@ export function buildOutlineFromBlocks(courseBlocks) { id: block.id, title: block.display_name, start: null, + effective_start: null, sequence_ids: [...block.children], }; } else if (block.type === 'sequential') { @@ -54,6 +55,7 @@ export function buildOutlineFromBlocks(courseBlocks) { title: block.display_name, accessible: true, start: null, + effective_start: null, }; } }); diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index 7cf1e221..c1bc06ba 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -10,11 +10,17 @@ export function normalizeLearningSequencesData(learningSequencesData) { sequences: {}, }; + const now = new Date(); + function isReleased(block) { + // We check whether the backend marks this as accessible because staff users are granted access anyway. + // Note that sections don't have the `accessible` field and will just be checking `effective_start`. + return block.accessible || !block.effective_start || now >= Date.parse(block.effective_start); + } + // Sequences Object.entries(learningSequencesData.outline.sequences).forEach(([seqId, sequence]) => { - if (!sequence.accessible) { - // Skipping inaccessible sequences replicates the behavior of the legacy course blocks API - return; + if (!isReleased(sequence)) { + return; // Don't let the learner see unreleased sequences } models.sequences[seqId] = { @@ -26,39 +32,37 @@ export function normalizeLearningSequencesData(learningSequencesData) { // Sections learningSequencesData.outline.sections.forEach(section => { - // Skipping sections with only inaccessible sequences replicates the behavior of the legacy course blocks API - // (But keep it if it was already empty, again to replicate legacy blocks API.) - const accessibleSequenceIds = section.sequence_ids.filter(seqId => seqId in models.sequences); - if (section.sequence_ids.length > 0 && accessibleSequenceIds.length === 0) { + // Filter out any ignored sequences (e.g. unreleased sequences) + const availableSequenceIds = section.sequence_ids.filter(seqId => seqId in models.sequences); + + // If we are unreleased and already stripped out all our children, just don't show us at all. + // (We check both release date and children because children will exist for an unreleased section even for staff, + // so we still want to show this section.) + if (!isReleased(section) && availableSequenceIds.length === 0) { return; } models.sections[section.id] = { id: section.id, title: section.title, - sequenceIds: accessibleSequenceIds, + sequenceIds: availableSequenceIds, courseId: learningSequencesData.course_key, }; // Add back-references to this section for all child sequences. - accessibleSequenceIds.forEach(childSeqId => { + availableSequenceIds.forEach(childSeqId => { models.sequences[childSeqId].sectionId = section.id; }); }); // Course - const now = new Date(); models.courses[learningSequencesData.course_key] = { id: learningSequencesData.course_key, title: learningSequencesData.title, sectionIds: Object.entries(models.sections).map(([sectionId]) => sectionId), - // Scan through all the sequences and look for ones that aren't accessible - // to us yet because the start date has not yet passed. (Some may be - // inaccessible because the end_date has passed.) - hasScheduledContent: Object.values(learningSequencesData.outline.sequences).some( - seq => !seq.accessible && now < Date.parse(seq.effective_start), - ), + // Scan through all the sequences and look for ones that aren't released yet. + hasScheduledContent: Object.values(learningSequencesData.outline.sequences).some(seq => !isReleased(seq)), }; return models; diff --git a/src/courseware/data/pact-tests/lmsPact.test.jsx b/src/courseware/data/pact-tests/lmsPact.test.jsx index 48fc8e70..feb5ec79 100644 --- a/src/courseware/data/pact-tests/lmsPact.test.jsx +++ b/src/courseware/data/pact-tests/lmsPact.test.jsx @@ -80,9 +80,9 @@ describe('Courseware Service', () => { expect(response).toEqual(normalizedOutline); }); - it('skips inaccessible sequences', async () => { + it('skips unreleased sequences', async () => { await provider.addInteraction({ - state: `Outline exists with inaccessible sequences for course_id ${courseId}`, + state: `Outline exists with unreleased sequences for course_id ${courseId}`, uponReceiving: 'a request to get an outline', withRequest: { method: 'GET', @@ -97,18 +97,21 @@ describe('Courseware Service', () => { sections: [ { id: 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@partial', - title: 'Partially accessible', + title: 'Partially released', sequence_ids: [ 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@accessible', + 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@released', 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@nope1', ], + effective_start: null, }, { id: 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@nope', - title: 'Wholly inaccessible', + title: 'Wholly unreleased', sequence_ids: [ 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@nope2', ], + effective_start: '9999-07-01T17:00:00Z', }, ], sequences: { @@ -116,17 +119,23 @@ describe('Courseware Service', () => { id: 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@accessible', title: 'Can access', accessible: true, + effective_start: '9999-07-01T17:00:00Z', + }, + 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@released': { + id: 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@released', + title: 'Released and inaccessible', + accessible: false, effective_start: '2019-07-01T17:00:00Z', }, 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@nope1': { id: 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@nope1', - title: 'Cannot access', + title: 'Unreleased', accessible: false, effective_start: '9999-07-01T17:00:00Z', }, 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@nope2': { id: 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@nope2', - title: 'Still cannot access', + title: 'Still unreleased', accessible: false, effective_start: '9999-07-01T17:00:00Z', }, @@ -149,10 +158,11 @@ describe('Courseware Service', () => { sections: { 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@partial': { id: 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@partial', - title: 'Partially accessible', + title: 'Partially released', courseId: 'course-v1:edX+DemoX+Demo_Course', sequenceIds: [ 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@accessible', + 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@released', ], }, }, @@ -163,6 +173,12 @@ describe('Courseware Service', () => { sectionId: 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@partial', legacyWebUrl: `${getConfig().LMS_BASE_URL}/courses/course-v1:edX+DemoX+Demo_Course/jump_to/block-v1:edX+DemoX+Demo_Course+type@sequential+block@accessible?experience=legacy`, }, + 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@released': { + id: 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@released', + title: 'Released and inaccessible', + sectionId: 'block-v1:edX+DemoX+Demo_Course+type@chapter+block@partial', + legacyWebUrl: `${getConfig().LMS_BASE_URL}/courses/course-v1:edX+DemoX+Demo_Course/jump_to/block-v1:edX+DemoX+Demo_Course+type@sequential+block@released?experience=legacy`, + }, }, }; const response = await getLearningSequencesOutline(courseId);