From 8970352cdd739910d03890b836cfb74876e31b14 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Tue, 13 Jul 2021 13:15:11 -0400 Subject: [PATCH 01/60] Update path --- src/courseware/CoursewareContainer.jsx | 33 ++++++++++++++------------ src/courseware/data/api.js | 5 ++-- src/index.jsx | 14 +++++------ 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index 05982cc10d..7cf710ca75 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -25,9 +25,9 @@ const checkResumeRedirect = memoize((courseStatus, courseId, sequenceId, firstSe getResumeBlock(courseId).then((data) => { // This is a replace because we don't want this change saved in the browser's history. if (data.sectionId && data.unitId) { - history.replace(`/course/${courseId}/${data.sectionId}/${data.unitId}`); + history.replace(`/c/${courseId}/${data.sectionId}/${data.unitId}`); } else if (firstSequenceId) { - history.replace(`/course/${courseId}/${firstSequenceId}`); + history.replace(`/c/${courseId}/${firstSequenceId}`); } }); } @@ -35,7 +35,7 @@ const checkResumeRedirect = memoize((courseStatus, courseId, sequenceId, firstSe const checkSectionUnitToUnitRedirect = memoize((courseStatus, courseId, sequenceStatus, section, unitId) => { if (courseStatus === 'loaded' && sequenceStatus === 'failed' && section && unitId) { - history.replace(`/course/${courseId}/${unitId}`); + history.replace(`/c/${courseId}/${unitId}`); } }); @@ -43,10 +43,10 @@ const checkSectionToSequenceRedirect = memoize((courseStatus, courseId, sequence if (courseStatus === 'loaded' && sequenceStatus === 'failed' && section && !unitId) { // If the section is non-empty, redirect to its first sequence. if (section.sequenceIds && section.sequenceIds[0]) { - history.replace(`/course/${courseId}/${section.sequenceIds[0]}`); + history.replace(`/c/${courseId}/${section.sequenceIds[0]}`); // Otherwise, just go to the course root, letting the resume redirect take care of things. } else { - history.replace(`/course/${courseId}`); + history.replace(`/c/${courseId}`); } } }); @@ -55,7 +55,7 @@ const checkUnitToSequenceUnitRedirect = memoize((courseStatus, courseId, sequenc if (courseStatus === 'loaded' && sequenceStatus === 'failed' && unit) { // If the sequence failed to load as a sequence, but it *did* load as a unit, then // insert the unit's parent sequenceId into the URL. - history.replace(`/course/${courseId}/${unit.sequenceId}/${unit.id}`); + history.replace(`/c/${courseId}/${unit.sequenceId}/${unit.id}`); } }); @@ -72,7 +72,7 @@ const checkSequenceToSequenceUnitRedirect = memoize((courseId, sequenceStatus, s if (sequence.unitIds !== undefined && sequence.unitIds.length > 0) { const nextUnitId = sequence.unitIds[sequence.activeUnitIndex]; // This is a replace because we don't want this change saved in the browser's history. - history.replace(`/course/${courseId}/${sequence.id}/${nextUnitId}`); + history.replace(`/c/${courseId}/${sequence.id}/${nextUnitId}`); } } }); @@ -106,13 +106,17 @@ class CoursewareContainer extends Component { match: { params: { courseId: routeCourseId, - sequenceId: routeSequenceId, + // sequenceId: routeSequenceId, + sequenceId: routeSequenceHash, }, }, } = this.props; + // const routeSequenceId = decodeURIComponent(routeSequenceHash.replace(/\+/g, ' ')); + // console.log(routeSequenceId); // Load data whenever the course or sequence ID changes. this.checkFetchCourse(routeCourseId); - this.checkFetchSequence(routeSequenceId); + // this.checkFetchSequence(routeSequenceId); + this.checkFetchSequence(routeSequenceHash); } componentDidUpdate() { @@ -135,7 +139,6 @@ class CoursewareContainer extends Component { }, }, } = this.props; - // Load data whenever the course or sequence ID changes. this.checkFetchCourse(routeCourseId); this.checkFetchSequence(routeSequenceId); @@ -205,7 +208,7 @@ class CoursewareContainer extends Component { } = this.props; this.props.checkBlockCompletion(courseId, sequenceId, routeUnitId); - history.push(`/course/${courseId}/${sequenceId}/${nextUnitId}`); + history.push(`/c/${courseId}/${sequenceId}/${nextUnitId}`); } handleNextSequenceClick = () => { @@ -221,10 +224,10 @@ class CoursewareContainer extends Component { let nextUnitId = null; if (nextSequence.unitIds.length > 0) { [nextUnitId] = nextSequence.unitIds; - history.push(`/course/${courseId}/${nextSequence.id}/${nextUnitId}`); + history.push(`/c/${courseId}/${nextSequence.id}/${nextUnitId}`); } else { // Some sequences have no units. This will show a blank page with prev/next buttons. - history.push(`/course/${courseId}/${nextSequence.id}`); + history.push(`/c/${courseId}/${nextSequence.id}`); } const celebrateFirstSection = course && course.celebrations && course.celebrations.firstSection; @@ -239,10 +242,10 @@ class CoursewareContainer extends Component { if (previousSequence !== null) { if (previousSequence.unitIds.length > 0) { const previousUnitId = previousSequence.unitIds[previousSequence.unitIds.length - 1]; - history.push(`/course/${courseId}/${previousSequence.id}/${previousUnitId}`); + history.push(`/c/${courseId}/${previousSequence.id}/${previousUnitId}`); } else { // Some sequences have no units. This will show a blank page with prev/next buttons. - history.push(`/course/${courseId}/${previousSequence.id}`); + history.push(`/c/${courseId}/${previousSequence.id}`); } } } diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index 495d3cd7f7..aafe078f8c 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -67,7 +67,6 @@ export function normalizeBlocks(courseId, blocks) { }); } }); - Object.values(models.sections).forEach(section => { if (Array.isArray(section.sequenceIds)) { section.sequenceIds.forEach(sequenceId => { @@ -219,6 +218,7 @@ function normalizeSequenceMetadata(sequence) { saveUnitPosition: sequence.save_position, showCompletion: sequence.show_completion, allowProctoringOptOut: sequence.allow_proctoring_opt_out, + hash_key: sequence.hash_key, }, units: sequence.items.map(unit => ({ id: unit.id, @@ -229,14 +229,13 @@ function normalizeSequenceMetadata(sequence) { contentType: unit.type, graded: unit.graded, containsContentTypeGatedContent: unit.contains_content_type_gated_content, + hash_key: unit.hash_key, })), }; } - export async function getSequenceMetadata(sequenceId) { const { data } = await getAuthenticatedHttpClient() .get(`${getConfig().LMS_BASE_URL}/api/courseware/sequence/${sequenceId}`, {}); - return normalizeSequenceMetadata(data); } diff --git a/src/index.jsx b/src/index.jsx index dc2809ca24..a40cc9fbaa 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -34,31 +34,31 @@ subscribe(APP_READY, () => { - + - + - + - + From b23a6330f195f973c67178e753d475a34da54014 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Tue, 13 Jul 2021 14:50:40 -0400 Subject: [PATCH 02/60] Add ADR --- .../0009-courseware-url-shortening.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 docs/decisions/0009-courseware-url-shortening.md diff --git a/docs/decisions/0009-courseware-url-shortening.md b/docs/decisions/0009-courseware-url-shortening.md new file mode 100644 index 0000000000..027a9e8889 --- /dev/null +++ b/docs/decisions/0009-courseware-url-shortening.md @@ -0,0 +1,47 @@ +# Courseware URL shortening + +## Status + +Accepted + +_This updates some of the content in [ADR #8: Liberal courseware path handling](./0008-liberal-courseware-path-handling.md)._ + +## Context + +The current URL is not human-readable. The URL is composed of the UsageKeys for the current sequence and unit. We can't make UsageKeys themselves more readable because they're tied to student state. + +This is what the URLs currently look like: + +``` + +https://learning.edx.org/course/course-v1:edX+DemoX.1+2T2019/block-v1:edX+DemoX.1+2T2019+type@sequential+block@e0a61b3d5a2046949e76d12cac5df493/block-v1:edX+DemoX.1+2T2019+type@vertical+block@52dbad5a28e140f291a476f92ce11996 + +``` + +After exploring different URL patterns and possible redundancies in the current URL format. The course, run, and organization is stated in every portion of the URL. We also do not need the URL to tell us the type of block since it has been determined that all URLs will follow the path` /course/:courseId/:sequenceId/:unitId`. + +## Decision + +The courseware URL will format to the following structure: + +``` + +https://learning.edx.org/c/:courseId/:sequenceHash/:unitHash/:sectionSlug/:sequenceSlug/:unitSlug/ +``` + +The fields definition and requirements ar as follows: + +* :courseId (required) - same as the previous `courseId`. +* :sequenceHash (required) - a `urlsafe_b64encode` version of the `sequenceId`. +* :unitHash (required) - a `urlsafe_b64encode` version of the `unitId`. +* :sectionSlug (optional) - `display_name` of the current sequence's parent section. +* :sequenceSlug (optional) - `display_name` of the current sequence. +* :unitSlug (optional) - `display_name` of the current unit + +## Consequences + +If old URLs are not properly routed then the content and those links will no longer be accessible to the user. The old URLs could include, but not limited to, bookmarks and exams. + +## Further work + +At some point, we may decide to further extend the URL shortening to the entire platform. At the moment, the hashes for the sequences and units are generated when the sequences and units are being called. In the future, it would be better if the hashes would be generated and stored when the sequences and units are originally created. This would require `learning_sequences` to include a class for unit storage, which is not being stored at the moment. From 832107f08412d807bae3e6101062ad9ab7c81b82 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Tue, 13 Jul 2021 14:51:33 -0400 Subject: [PATCH 03/60] Add reference to ADR 009 --- docs/decisions/0008-liberal-courseware-path-handling.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/decisions/0008-liberal-courseware-path-handling.md b/docs/decisions/0008-liberal-courseware-path-handling.md index addd3511be..dfc607a8fc 100644 --- a/docs/decisions/0008-liberal-courseware-path-handling.md +++ b/docs/decisions/0008-liberal-courseware-path-handling.md @@ -88,3 +88,6 @@ And more like: ``` https://learning.edx.org/course/course-v1:edX+DemoX.1+2T2019/Being_Social/Teams ``` + +_This further work has been expanded upon in +[ADR #9: Courseware URL shortening](./0009-courseware-url-shortening.md)._ From d38c07a206081ee02dd737b8046384dee2c19c3f Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Tue, 13 Jul 2021 15:33:12 -0400 Subject: [PATCH 04/60] Update 0009-courseware-url-shortening.md --- docs/decisions/0009-courseware-url-shortening.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/decisions/0009-courseware-url-shortening.md b/docs/decisions/0009-courseware-url-shortening.md index 027a9e8889..ac21641d1a 100644 --- a/docs/decisions/0009-courseware-url-shortening.md +++ b/docs/decisions/0009-courseware-url-shortening.md @@ -38,6 +38,8 @@ The fields definition and requirements ar as follows: * :sequenceSlug (optional) - `display_name` of the current sequence. * :unitSlug (optional) - `display_name` of the current unit +The slugs based on `display_name` are optional because not all blocks have an associated `display_name` attributes, most likely to occur in OLX imports. + ## Consequences If old URLs are not properly routed then the content and those links will no longer be accessible to the user. The old URLs could include, but not limited to, bookmarks and exams. From 19f318679f40e8026de0f774748513f1c1e8f4cb Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Tue, 13 Jul 2021 16:28:45 -0400 Subject: [PATCH 05/60] Add example url --- docs/decisions/0009-courseware-url-shortening.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/decisions/0009-courseware-url-shortening.md b/docs/decisions/0009-courseware-url-shortening.md index ac21641d1a..8a52f62c15 100644 --- a/docs/decisions/0009-courseware-url-shortening.md +++ b/docs/decisions/0009-courseware-url-shortening.md @@ -27,6 +27,15 @@ The courseware URL will format to the following structure: ``` https://learning.edx.org/c/:courseId/:sequenceHash/:unitHash/:sectionSlug/:sequenceSlug/:unitSlug/ + +``` + +Example URL: + +``` + +https://learning.edx.org/c/course-v1:edX+DemoX.1+2T2019/YmxvY2/njuRCq/Optional_Example_Problem_Types/STEM_Problems/Code_Grader + ``` The fields definition and requirements ar as follows: From 3b2f91cd32efe9433137477da0aefb1f98c797f1 Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Thu, 15 Jul 2021 14:44:12 -0400 Subject: [PATCH 06/60] Update 0009-courseware-url-shortening.md --- docs/decisions/0009-courseware-url-shortening.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/decisions/0009-courseware-url-shortening.md b/docs/decisions/0009-courseware-url-shortening.md index 8a52f62c15..5a550a7258 100644 --- a/docs/decisions/0009-courseware-url-shortening.md +++ b/docs/decisions/0009-courseware-url-shortening.md @@ -47,7 +47,7 @@ The fields definition and requirements ar as follows: * :sequenceSlug (optional) - `display_name` of the current sequence. * :unitSlug (optional) - `display_name` of the current unit -The slugs based on `display_name` are optional because not all blocks have an associated `display_name` attributes, most likely to occur in OLX imports. +The slugs based on `display_name` are optional because not all blocks have an associated `display_name` attributes, most likely to occur in OLX imports. The `sequenceHash` and `unitHash` will shorten their respective ids using `hashlib.blake2b`. `Blake2b` will reduce the length of the id so the encoded version can also be short. Hashing will be handled by `blake2b` because it is the fastest hashing function in the `hashlib` library and is the most compatible with `urlsafe_b64encode`. The hash will be generated and mapped in LMS. ## Consequences From 457dc4b279021b94f04101fc1ec430368556ae32 Mon Sep 17 00:00:00 2001 From: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com> Date: Thu, 15 Jul 2021 14:52:48 -0400 Subject: [PATCH 07/60] Update 0009-courseware-url-shortening.md --- docs/decisions/0009-courseware-url-shortening.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/decisions/0009-courseware-url-shortening.md b/docs/decisions/0009-courseware-url-shortening.md index 5a550a7258..5134daa6ac 100644 --- a/docs/decisions/0009-courseware-url-shortening.md +++ b/docs/decisions/0009-courseware-url-shortening.md @@ -47,7 +47,7 @@ The fields definition and requirements ar as follows: * :sequenceSlug (optional) - `display_name` of the current sequence. * :unitSlug (optional) - `display_name` of the current unit -The slugs based on `display_name` are optional because not all blocks have an associated `display_name` attributes, most likely to occur in OLX imports. The `sequenceHash` and `unitHash` will shorten their respective ids using `hashlib.blake2b`. `Blake2b` will reduce the length of the id so the encoded version can also be short. Hashing will be handled by `blake2b` because it is the fastest hashing function in the `hashlib` library and is the most compatible with `urlsafe_b64encode`. The hash will be generated and mapped in LMS. +The slugs based on `display_name` are optional because not all blocks have an associated `display_name` attributes, most likely to occur in OLX imports. The `sequenceHash` and `unitHash` will shorten their respective ids using `hashlib.blake2b`. `Blake2b` will reduce the length of the id so the encoded version can also be short. Hashing will be handled by `blake2b` because it is the fastest hashing function in the `hashlib` library. The hash will be generated and mapped in LMS. ## Consequences From abac174e2efef33f19b623cd2187323f55b87c6b Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 21 Jul 2021 09:18:04 -0400 Subject: [PATCH 08/60] Update model to base storage off hash_key for sequence and unit --- src/courseware/data/api.js | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index aafe078f8c..8bfaf053a5 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -35,21 +35,23 @@ export function normalizeBlocks(courseId, blocks) { break; case 'sequential': - models.sequences[block.id] = { + models.sequences[block.hash_key] = { effortActivities: block.effort_activities, effortTime: block.effort_time, id: block.id, title: block.display_name, legacyWebUrl: block.legacy_web_url, unitIds: block.children || [], + hash_key: block.hash_key, }; break; case 'vertical': - models.units[block.id] = { + models.units[block.hash_key] = { graded: block.graded, id: block.id, title: block.display_name, legacyWebUrl: block.legacy_web_url, + hash_key: block.hash_key, }; break; default: @@ -67,11 +69,19 @@ export function normalizeBlocks(courseId, blocks) { }); } }); + Object.values(models.sections).forEach(section => { if (Array.isArray(section.sequenceIds)) { section.sequenceIds.forEach(sequenceId => { - if (sequenceId in models.sequences) { - models.sequences[sequenceId].sectionId = section.id; + const modelSequenceIds = {}; + Object.values(models.sequences).forEach(sequence => { + if (sequenceId === sequence.id) { + modelSequenceIds[sequenceId] = sequence.hash_key; + } + }); + if (sequenceId in modelSequenceIds) { + const sequence = modelSequenceIds[sequenceId]; + models.sequences.[sequence].sectionId = section.id; } else { logInfo(`Section ${section.id} has child block ${sequenceId}, but that block is not in the list of sequences.`); } @@ -82,15 +92,21 @@ export function normalizeBlocks(courseId, blocks) { Object.values(models.sequences).forEach(sequence => { if (Array.isArray(sequence.unitIds)) { sequence.unitIds.forEach(unitId => { - if (unitId in models.units) { - models.units[unitId].sequenceId = sequence.id; + const modelUnitIds = {}; + Object.values(models.units).forEach(unit => { + if (unitId === unit.id) { + modelUnitIds[unitId] = unit.hash_key; + } + }); + if (unitId in modelUnitIds) { + const unit = modelUnitIds[unitId]; + models.units.[unit].sequenceId = sequence.id; } else { logInfo(`Sequence ${sequence.id} has child block ${unitId}, but that block is not in the list of units.`); } }); } }); - return models; } @@ -195,7 +211,7 @@ export async function getCourseMetadata(courseId) { function normalizeSequenceMetadata(sequence) { return { sequence: { - id: sequence.item_id, + id: sequence.hash_key, blockType: sequence.tag, unitIds: sequence.items.map(unit => unit.id), bannerText: sequence.banner_text, @@ -218,7 +234,6 @@ function normalizeSequenceMetadata(sequence) { saveUnitPosition: sequence.save_position, showCompletion: sequence.show_completion, allowProctoringOptOut: sequence.allow_proctoring_opt_out, - hash_key: sequence.hash_key, }, units: sequence.items.map(unit => ({ id: unit.id, @@ -229,7 +244,7 @@ function normalizeSequenceMetadata(sequence) { contentType: unit.type, graded: unit.graded, containsContentTypeGatedContent: unit.contains_content_type_gated_content, - hash_key: unit.hash_key, + decoded_id: unit.decoded_id, })), }; } From fcda48513a14c2c0ed26c3d04722132a1ad129f8 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 21 Jul 2021 09:22:16 -0400 Subject: [PATCH 09/60] Remove commented out code --- src/courseware/CoursewareContainer.jsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index b03b4bf555..b64c3ed0b0 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -108,16 +108,12 @@ class CoursewareContainer extends Component { match: { params: { courseId: routeCourseId, - // sequenceId: routeSequenceId, sequenceId: routeSequenceHash, }, }, } = this.props; - // const routeSequenceId = decodeURIComponent(routeSequenceHash.replace(/\+/g, ' ')); - // console.log(routeSequenceId); // Load data whenever the course or sequence ID changes. this.checkFetchCourse(routeCourseId); - // this.checkFetchSequence(routeSequenceId); this.checkFetchSequence(routeSequenceHash); } @@ -136,14 +132,14 @@ class CoursewareContainer extends Component { match: { params: { courseId: routeCourseId, - sequenceId: routeSequenceId, + sequenceId: routeSequenceHash, unitId: routeUnitId, }, }, } = this.props; // Load data whenever the course or sequence ID changes. this.checkFetchCourse(routeCourseId); - this.checkFetchSequence(routeSequenceId); + this.checkFetchSequence(routeSequenceHash); // All courseware URLs should normalize to the format /course/:courseId/:sequenceId/:unitId // via the series of redirection rules below. From e96d885114c0f4d3afea5dd1a3cb3019b25b4934 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 26 Jul 2021 15:09:38 -0400 Subject: [PATCH 10/60] Update model to store sequence based on hash_key --- src/course-home/data/api.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/course-home/data/api.js b/src/course-home/data/api.js index f1f5906022..5b9d868481 100644 --- a/src/course-home/data/api.js +++ b/src/course-home/data/api.js @@ -133,7 +133,7 @@ export function normalizeOutlineBlocks(courseId, blocks) { break; case 'sequential': - models.sequences[block.id] = { + models.sequences[block.hash_key] = { complete: block.complete, description: block.description, due: block.due, @@ -148,6 +148,7 @@ export function normalizeOutlineBlocks(courseId, blocks) { // link to the MFE ourselves). showLink: !!block.legacy_web_url, title: block.display_name, + hash_key: block.hash_key, }; break; @@ -170,8 +171,15 @@ export function normalizeOutlineBlocks(courseId, blocks) { Object.values(models.sections).forEach(section => { if (Array.isArray(section.sequenceIds)) { section.sequenceIds.forEach(sequenceId => { - if (sequenceId in models.sequences) { - models.sequences[sequenceId].sectionId = section.id; + const modelSequenceIds = {}; + Object.values(models.sequences).forEach(sequence => { + if (sequenceId === sequence.id) { + modelSequenceIds[sequenceId] = sequence.hash_key; + } + }); + if (sequenceId in modelSequenceIds) { + const sequence = modelSequenceIds[sequenceId]; + models.sequences.[sequence].sectionId = section.id; } else { logInfo(`Section ${section.id} has child block ${sequenceId}, but that block is not in the list of sequences.`); } From e4ec845bd45cbad85534bdc0bb8a4e3139cbe70f Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 26 Jul 2021 15:11:07 -0400 Subject: [PATCH 11/60] Update link path to match new format --- src/course-home/outline-tab/SequenceLink.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/course-home/outline-tab/SequenceLink.jsx b/src/course-home/outline-tab/SequenceLink.jsx index 0aa5d5a157..42920e717b 100644 --- a/src/course-home/outline-tab/SequenceLink.jsx +++ b/src/course-home/outline-tab/SequenceLink.jsx @@ -46,7 +46,7 @@ function SequenceLink({ // canLoadCourseware is true if the Courseware MFE is enabled, false otherwise const coursewareUrl = ( canLoadCourseware - ? {title} + ? {title} : {title} ); const displayTitle = showLink ? coursewareUrl : title; From 388b9dfe59600a8922032f120dc3694bcf0075f7 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 26 Jul 2021 16:35:03 -0400 Subject: [PATCH 12/60] Fix undefined error in sequence metadata --- src/courseware/data/api.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index 8bfaf053a5..54062d3244 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -211,7 +211,7 @@ export async function getCourseMetadata(courseId) { function normalizeSequenceMetadata(sequence) { return { sequence: { - id: sequence.hash_key, + id: sequence.item_id, blockType: sequence.tag, unitIds: sequence.items.map(unit => unit.id), bannerText: sequence.banner_text, @@ -234,6 +234,7 @@ function normalizeSequenceMetadata(sequence) { saveUnitPosition: sequence.save_position, showCompletion: sequence.show_completion, allowProctoringOptOut: sequence.allow_proctoring_opt_out, + decoded_id: sequence.decoded_id, }, units: sequence.items.map(unit => ({ id: unit.id, From 174be4adc7579a91780bc5ed4c4409226a829607 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 26 Jul 2021 16:39:14 -0400 Subject: [PATCH 13/60] Update iframeUrl to use decoded_id instead of id --- src/courseware/course/sequence/Unit.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/courseware/course/sequence/Unit.jsx b/src/courseware/course/sequence/Unit.jsx index 6c08d7cce5..98c88c371d 100644 --- a/src/courseware/course/sequence/Unit.jsx +++ b/src/courseware/course/sequence/Unit.jsx @@ -88,9 +88,10 @@ function Unit({ /** [MM-P2P] Experiment */ mmp2p, }) { + const unit = useModel('units', id); const { authenticatedUser } = useContext(AppContext); const view = authenticatedUser ? 'student_view' : 'public_view'; - let iframeUrl = `${getConfig().LMS_BASE_URL}/xblock/${id}?show_title=0&show_bookmark_button=0&recheck_access=1&view=${view}`; + let iframeUrl = `${getConfig().LMS_BASE_URL}/xblock/${unit.decoded_id}?show_title=0&show_bookmark_button=0&recheck_access=1&view=${view}`; if (format) { iframeUrl += `&format=${format}`; } @@ -100,7 +101,6 @@ function Unit({ const [modalOptions, setModalOptions] = useState({ open: false }); const [shouldDisplayHonorCode, setShouldDisplayHonorCode] = useState(false); - const unit = useModel('units', id); const course = useModel('coursewareMeta', courseId); const { contentTypeGatingEnabled, From 4be725b4c2e540d52658d9c6f4d06591ede67431 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 29 Jul 2021 14:09:51 -0400 Subject: [PATCH 14/60] Update iframe url parameter variables --- src/courseware/course/sequence/Unit.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/courseware/course/sequence/Unit.jsx b/src/courseware/course/sequence/Unit.jsx index d761f307d3..a8580c80b3 100644 --- a/src/courseware/course/sequence/Unit.jsx +++ b/src/courseware/course/sequence/Unit.jsx @@ -92,7 +92,7 @@ function Unit({ const unit = useModel('units', id); const { authenticatedUser } = useContext(AppContext); const view = authenticatedUser ? 'student_view' : 'public_view'; - let iframeUrl = `${getConfig().LMS_BASE_URL}/xblock/${unit.decoded_id}?show_title=0&show_bookmark_button=0&recheck_access=1&view=${view}`; + let iframeUrl = `${getConfig().LMS_BASE_URL}/xblock/${(unit.decoded_id || id)}?show_title=0&show_bookmark_button=0&recheck_access=1&view=${view}`; if (format) { iframeUrl += `&format=${format}`; } From 29a24aa62e8b18af53b92fe6dc603539580bd93f Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 29 Jul 2021 14:17:08 -0400 Subject: [PATCH 15/60] Fix broken api calls --- src/courseware/data/thunks.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/courseware/data/thunks.js b/src/courseware/data/thunks.js index 55767d3c22..5808bfa111 100644 --- a/src/courseware/data/thunks.js +++ b/src/courseware/data/thunks.js @@ -174,7 +174,8 @@ export function checkBlockCompletion(courseId, sequenceId, unitId) { } try { - const isComplete = await getBlockCompletion(courseId, sequenceId, unitId); + const isComplete = await getBlockCompletion(courseId, models.sequences[sequenceId].decoded_id, + models.units[unitId].decoded_id); dispatch(updateModel({ modelType: 'units', model: { @@ -201,7 +202,7 @@ export function saveSequencePosition(courseId, sequenceId, activeUnitIndex) { }, })); try { - await postSequencePosition(courseId, sequenceId, activeUnitIndex); + await postSequencePosition(courseId, models.sequences[sequenceId].decoded_id, activeUnitIndex); // Update again under the assumption that the above call succeeded, since it doesn't return a // meaningful response. dispatch(updateModel({ From aeca68fd56803ba9460120a5728b809ad472a637 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 29 Jul 2021 14:17:51 -0400 Subject: [PATCH 16/60] Update block id variables --- src/courseware/data/api.js | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index d4c47ee218..5dfd6c1a91 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -26,7 +26,7 @@ export function normalizeBlocks(courseId, blocks) { models.sections[block.id] = { id: block.id, title: block.display_name, - sequenceIds: block.children || [], + sequenceIds: block.hash_children || block.children || [], }; break; @@ -34,17 +34,17 @@ export function normalizeBlocks(courseId, blocks) { models.sequences[block.hash_key] = { effortActivities: block.effort_activities, effortTime: block.effort_time, - id: block.id, + id: block.hash_key || block.id, title: block.display_name, legacyWebUrl: block.legacy_web_url, - unitIds: block.children || [], + unitIds: block.hash_children || block.children || [], hash_key: block.hash_key, }; break; case 'vertical': models.units[block.hash_key] = { graded: block.graded, - id: block.id, + id: block.hash_key || block.id, title: block.display_name, legacyWebUrl: block.legacy_web_url, hash_key: block.hash_key, @@ -69,15 +69,8 @@ export function normalizeBlocks(courseId, blocks) { Object.values(models.sections).forEach(section => { if (Array.isArray(section.sequenceIds)) { section.sequenceIds.forEach(sequenceId => { - const modelSequenceIds = {}; - Object.values(models.sequences).forEach(sequence => { - if (sequenceId === sequence.id) { - modelSequenceIds[sequenceId] = sequence.hash_key; - } - }); - if (sequenceId in modelSequenceIds) { - const sequence = modelSequenceIds[sequenceId]; - models.sequences.[sequence].sectionId = section.id; + if (sequenceId in models.sequences) { + models.sequences.[sequenceId].sectionId = section.id; } else { logInfo(`Section ${section.id} has child block ${sequenceId}, but that block is not in the list of sequences.`); } @@ -88,15 +81,8 @@ export function normalizeBlocks(courseId, blocks) { Object.values(models.sequences).forEach(sequence => { if (Array.isArray(sequence.unitIds)) { sequence.unitIds.forEach(unitId => { - const modelUnitIds = {}; - Object.values(models.units).forEach(unit => { - if (unitId === unit.id) { - modelUnitIds[unitId] = unit.hash_key; - } - }); - if (unitId in modelUnitIds) { - const unit = modelUnitIds[unitId]; - models.units.[unit].sequenceId = sequence.id; + if (unitId in models.units) { + models.units.[unitId].sequenceId = sequence.id; } else { logInfo(`Sequence ${sequence.id} has child block ${unitId}, but that block is not in the list of units.`); } From 43ff07af3ec23a99fc5af3d540d3bc96dd8fbd41 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 29 Jul 2021 15:17:12 -0400 Subject: [PATCH 17/60] Update course exit url --- src/courseware/course/sequence/Sequence.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/courseware/course/sequence/Sequence.jsx b/src/courseware/course/sequence/Sequence.jsx index c274a65bf9..aa0454f2b8 100644 --- a/src/courseware/course/sequence/Sequence.jsx +++ b/src/courseware/course/sequence/Sequence.jsx @@ -161,7 +161,7 @@ function Sequence({ const gated = sequence && sequence.gatedContent !== undefined && sequence.gatedContent.gated; const goToCourseExitPage = () => { - history.push(`/course/${courseId}/course-end`); + history.push(`/c/${courseId}/course-end`); }; const defaultContent = ( From d517f94c497da043465394bec5cc6c08d160a69b Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Fri, 30 Jul 2021 15:18:31 -0400 Subject: [PATCH 18/60] Add more information about the url changes --- docs/decisions/0009-courseware-url-shortening.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/decisions/0009-courseware-url-shortening.md b/docs/decisions/0009-courseware-url-shortening.md index 5134daa6ac..a2034ad048 100644 --- a/docs/decisions/0009-courseware-url-shortening.md +++ b/docs/decisions/0009-courseware-url-shortening.md @@ -34,7 +34,7 @@ Example URL: ``` -https://learning.edx.org/c/course-v1:edX+DemoX.1+2T2019/YmxvY2/njuRCq/Optional_Example_Problem_Types/STEM_Problems/Code_Grader +https://learning.edx.org/c/course-v1:edX+DemoX.1+2T2019/YmxvY2/njuRCq/optional-example-problem-types/stem-problems/code-grader ``` @@ -47,7 +47,7 @@ The fields definition and requirements ar as follows: * :sequenceSlug (optional) - `display_name` of the current sequence. * :unitSlug (optional) - `display_name` of the current unit -The slugs based on `display_name` are optional because not all blocks have an associated `display_name` attributes, most likely to occur in OLX imports. The `sequenceHash` and `unitHash` will shorten their respective ids using `hashlib.blake2b`. `Blake2b` will reduce the length of the id so the encoded version can also be short. Hashing will be handled by `blake2b` because it is the fastest hashing function in the `hashlib` library. The hash will be generated and mapped in LMS. +Partial paths will update with the required parameters as dicussed in [ADR #8: Liberal courseware path handling](./0008-liberal-courseware-path-handling.md). The `sequenceHash` and `unitHash` will shorten their respective ids using `hashlib.blake2b` with `digest_size` of 6 bytes. `Blake2b` will reduce the length of the id so the encoded version can also be short. Hashing will be handled by `blake2b` because it is the fastest hashing function in the `hashlib` library. The hash will be generated and mapped in LMS. The slugs based on `display_name` are optional because not all blocks have an associated `display_name` attributes, most likely to occur in OLX imports. The `display_name` will be pulled from the current section, sequence, and unit attribute, and if there is not an attribute `display`, the url will use the attribute `display_name_with_default`. The `display_name` will be formatted safely for a url using Django's [slugify](https://docs.djangoproject.com/en/3.2/ref/utils/#django.utils.text.slugify). Slugify allows unicode identifiers in the slug. If the slugs are omitted, it will redirect to the canonical version without the slugs. ## Consequences From 4baf78c79e7c01d74e97df10bcd82714bf22a1c2 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Fri, 30 Jul 2021 15:20:01 -0400 Subject: [PATCH 19/60] Update links to match the new pattern --- src/course-home/dates-tab/DatesTab.test.jsx | 6 ++-- .../outline-tab/OutlineTab.test.jsx | 2 +- src/courseware/CoursewareContainer.test.jsx | 28 ++++++++-------- .../course/course-exit/CourseExit.test.jsx | 2 +- .../sequence/content-lock/ContentLock.jsx | 2 +- .../content-lock/ContentLock.test.jsx | 2 +- .../course/sequence/honor-code/HonorCode.jsx | 2 +- .../sequence/honor-code/HonorCode.test.jsx | 2 +- .../__factories__/sequenceMetadata.factory.js | 7 +++- src/setupTest.js | 2 +- .../data/__factories__/block.factory.js | 32 ++++++++++++++----- src/tab-page/TabContainer.test.jsx | 8 ++--- 12 files changed, 58 insertions(+), 37 deletions(-) diff --git a/src/course-home/dates-tab/DatesTab.test.jsx b/src/course-home/dates-tab/DatesTab.test.jsx index d093777284..c2ce6d326c 100644 --- a/src/course-home/dates-tab/DatesTab.test.jsx +++ b/src/course-home/dates-tab/DatesTab.test.jsx @@ -81,7 +81,7 @@ describe('DatesTab', () => { beforeEach(() => { axiosMock.onGet(courseMetadataUrl).reply(200, courseMetadata); axiosMock.onGet(datesUrl).reply(200, datesTabData); - history.push(`/course/${courseId}/dates`); // so tab can pull course id from url + history.push(`/c/${courseId}/dates`); // so tab can pull course id from url render(component); }); @@ -147,7 +147,7 @@ describe('DatesTab', () => { describe('Suggested schedule messaging', () => { beforeEach(() => { setMetadata({ is_self_paced: true, is_enrolled: true }); - history.push(`/course/${courseId}/dates`); + history.push(`/c/${courseId}/dates`); }); it('renders SuggestedScheduleHeader', async () => { @@ -316,7 +316,7 @@ describe('DatesTab', () => { beforeEach(() => { axiosMock.onGet(datesUrl).reply(200, datesTabData); - history.push(`/course/${courseId}/dates`); // so tab can pull course id from url + history.push(`/c/${courseId}/dates`); // so tab can pull course id from url }); it('redirects to course survey for a survey_required error code', async () => { diff --git a/src/course-home/outline-tab/OutlineTab.test.jsx b/src/course-home/outline-tab/OutlineTab.test.jsx index 040f3ce094..acf0f820a4 100644 --- a/src/course-home/outline-tab/OutlineTab.test.jsx +++ b/src/course-home/outline-tab/OutlineTab.test.jsx @@ -156,7 +156,7 @@ describe('Outline Tab', () => { await fetchAndRender(); const sequenceLink = screen.getByText('Title of Sequence'); - expect(sequenceLink.getAttribute('href')).toContain(`/course/${courseId}`); + expect(sequenceLink.getAttribute('href')).toContain(`/c/${courseId}`); }); }); diff --git a/src/courseware/CoursewareContainer.test.jsx b/src/courseware/CoursewareContainer.test.jsx index e598969db4..0cb111e676 100644 --- a/src/courseware/CoursewareContainer.test.jsx +++ b/src/courseware/CoursewareContainer.test.jsx @@ -85,9 +85,9 @@ describe('CoursewareContainer', () => { @@ -241,7 +241,7 @@ describe('CoursewareContainer', () => { } function assertLocation(container, sequenceId, unitId) { - const expectedUrl = `http://localhost/course/${courseId}/${sequenceId}/${unitId}`; + const expectedUrl = `http://localhost/c/${courseId}/${sequenceId}/${unitId}`; expect(global.location.href).toEqual(expectedUrl); expect(container.querySelector('.fake-unit')).toHaveTextContent(unitId); } @@ -293,14 +293,14 @@ describe('CoursewareContainer', () => { it('should ignore the section ID and instead redirect to the course root', async () => { setUrl(sectionTree[1].id); await loadContainer(); - expect(global.location.href).toEqual(`http://localhost/course/${courseId}`); + expect(global.location.href).toEqual(`http://localhost/c/${courseId}`); }); it('should ignore the section and unit IDs and instead to the course root', async () => { // Specific unit ID used here shouldn't matter; is ignored due to empty section. setUrl(sectionTree[1].id, unitTree[0][0][0]); await loadContainer(); - expect(global.location.href).toEqual(`http://localhost/course/${courseId}`); + expect(global.location.href).toEqual(`http://localhost/c/${courseId}`); }); }); }); @@ -314,13 +314,13 @@ describe('CoursewareContainer', () => { it('should insert the sequence ID into the URL', async () => { const unit = unitTree[1][0][1]; - history.push(`/course/${courseId}/${unit.id}`); + history.push(`/c/${courseId}/${unit.id}`); const container = await loadContainer(); assertLoadedHeader(container); assertSequenceNavigation(container, 2); const expectedSequenceId = sequenceTree[1][0].id; - const expectedUrl = `http://localhost/course/${courseId}/${expectedSequenceId}/${unit.id}`; + const expectedUrl = `http://localhost/c/${courseId}/${expectedSequenceId}/${unit.id}`; expect(global.location.href).toEqual(expectedUrl); expect(container.querySelector('.fake-unit')).toHaveTextContent(unit.id); }); @@ -331,7 +331,7 @@ describe('CoursewareContainer', () => { const unitBlocks = defaultUnitBlocks; it('should pick the first unit if position was not defined (activeUnitIndex becomes 0)', async () => { - history.push(`/course/${courseId}/${sequenceBlock.id}`); + history.push(`/c/${courseId}/${sequenceBlock.id}`); const container = await loadContainer(); assertLoadedHeader(container); @@ -350,7 +350,7 @@ describe('CoursewareContainer', () => { ); setUpMockRequests({ sequenceMetadatas: [sequenceMetadata] }); - history.push(`/course/${courseId}/${sequenceBlock.id}`); + history.push(`/c/${courseId}/${sequenceBlock.id}`); const container = await loadContainer(); assertLoadedHeader(container); @@ -367,7 +367,7 @@ describe('CoursewareContainer', () => { const unitBlocks = defaultUnitBlocks; it('should load the specified unit', async () => { - history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[2].id}`); + history.push(`/c/${courseId}/${sequenceBlock.id}/${unitBlocks[2].id}`); const container = await loadContainer(); assertLoadedHeader(container); @@ -391,7 +391,7 @@ describe('CoursewareContainer', () => { expect(sequenceNextButton).toHaveTextContent('Next'); fireEvent.click(sequenceNavButtons[4]); - expect(global.location.href).toEqual(`http://localhost/course/${courseId}/${sequenceBlock.id}/${unitBlocks[1].id}`); + expect(global.location.href).toEqual(`http://localhost/c/${courseId}/${sequenceBlock.id}/${unitBlocks[1].id}`); }); }); @@ -419,7 +419,7 @@ describe('CoursewareContainer', () => { ); setUpMockRequests({ sequenceMetadatas: [sequenceMetadata] }); - history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[2].id}`); + history.push(`/c/${courseId}/${sequenceBlock.id}/${unitBlocks[2].id}`); await loadContainer(); expect(global.location.assign).toHaveBeenCalledWith(sequenceBlock.legacy_web_url); @@ -440,7 +440,7 @@ describe('CoursewareContainer', () => { const { courseBlocks, sequenceBlocks, unitBlocks } = buildSimpleCourseBlocks(courseId, courseMetadata.name); setUpMockRequests({ courseBlocks, courseMetadata }); - history.push(`/course/${courseId}/${sequenceBlocks[0].id}/${unitBlocks[0].id}`); + history.push(`/c/${courseId}/${sequenceBlocks[0].id}/${unitBlocks[0].id}`); return { courseMetadata, unitBlocks }; } diff --git a/src/courseware/course/course-exit/CourseExit.test.jsx b/src/courseware/course/course-exit/CourseExit.test.jsx index 0a1e2c2e4f..f5654979d3 100644 --- a/src/courseware/course/course-exit/CourseExit.test.jsx +++ b/src/courseware/course/course-exit/CourseExit.test.jsx @@ -94,7 +94,7 @@ describe('Course Exit Pages', () => { }, }); await fetchAndRender(); - expect(global.location.href).toEqual(`http://localhost/course/${defaultMetadata.id}`); + expect(global.location.href).toEqual(`http://localhost/c/${defaultMetadata.id}`); }); }); diff --git a/src/courseware/course/sequence/content-lock/ContentLock.jsx b/src/courseware/course/sequence/content-lock/ContentLock.jsx index 2ec0f00b69..29a6e5517c 100644 --- a/src/courseware/course/sequence/content-lock/ContentLock.jsx +++ b/src/courseware/course/sequence/content-lock/ContentLock.jsx @@ -12,7 +12,7 @@ function ContentLock({ intl, courseId, prereqSectionName, prereqId, sequenceTitle, }) { const handleClick = useCallback(() => { - history.push(`/course/${courseId}/${prereqId}`); + history.push(`/c/${courseId}/${prereqId}`); }); return ( diff --git a/src/courseware/course/sequence/content-lock/ContentLock.test.jsx b/src/courseware/course/sequence/content-lock/ContentLock.test.jsx index 500b507866..39da5c83fe 100644 --- a/src/courseware/course/sequence/content-lock/ContentLock.test.jsx +++ b/src/courseware/course/sequence/content-lock/ContentLock.test.jsx @@ -38,6 +38,6 @@ describe('Content Lock', () => { render(); fireEvent.click(screen.getByRole('button')); - expect(history.push).toHaveBeenCalledWith(`/course/${mockData.courseId}/${mockData.prereqId}`); + expect(history.push).toHaveBeenCalledWith(`/c/${mockData.courseId}/${mockData.prereqId}`); }); }); diff --git a/src/courseware/course/sequence/honor-code/HonorCode.jsx b/src/courseware/course/sequence/honor-code/HonorCode.jsx index 1d9f399911..2c0df28507 100644 --- a/src/courseware/course/sequence/honor-code/HonorCode.jsx +++ b/src/courseware/course/sequence/honor-code/HonorCode.jsx @@ -13,7 +13,7 @@ function HonorCode({ intl, courseId }) { const siteName = getConfig().SITE_NAME; const honorCodeUrl = `${process.env.TERMS_OF_SERVICE_URL}#honor-code`; - const handleCancel = () => history.push(`/course/${courseId}/home`); + const handleCancel = () => history.push(`/c/${courseId}/home`); const handleAgree = () => { dispatch(saveIntegritySignature(courseId)); diff --git a/src/courseware/course/sequence/honor-code/HonorCode.test.jsx b/src/courseware/course/sequence/honor-code/HonorCode.test.jsx index 2708dcbea1..3a5b902a6d 100644 --- a/src/courseware/course/sequence/honor-code/HonorCode.test.jsx +++ b/src/courseware/course/sequence/honor-code/HonorCode.test.jsx @@ -28,6 +28,6 @@ describe('Honor Code', () => { const cancelButton = screen.getByText('Cancel'); fireEvent.click(cancelButton); - expect(history.push).toHaveBeenCalledWith(`/course/${mockData.courseId}/home`); + expect(history.push).toHaveBeenCalledWith(`/c/${mockData.courseId}/home`); }); }); diff --git a/src/courseware/data/__factories__/sequenceMetadata.factory.js b/src/courseware/data/__factories__/sequenceMetadata.factory.js index 3ec46b8e4d..788611fa98 100644 --- a/src/courseware/data/__factories__/sequenceMetadata.factory.js +++ b/src/courseware/data/__factories__/sequenceMetadata.factory.js @@ -27,7 +27,7 @@ Factory.define('sequenceMetadata') ), ) .attr('element_id', ['sequenceBlock'], sequenceBlock => sequenceBlock.block_id) - .attr('item_id', ['sequenceBlock'], sequenceBlock => sequenceBlock.id) + .attr('item_id', ['sequenceBlock'], sequenceBlock => (sequenceBlock.hash_key || sequenceBlock.id)) .attr('display_name', ['sequenceBlock'], sequenceBlock => sequenceBlock.display_name) .attr('gated_content', ['sequenceBlock'], sequenceBlock => ({ gated: false, @@ -36,6 +36,9 @@ Factory.define('sequenceMetadata') prereq_section_name: `${sequenceBlock.display_name}-prereq`, gated_section_name: sequenceBlock.display_name, })) + + .attr('decoded_id', ['sequenceBlock'], sequenceBlock => sequenceBlock.decoded_id) + .attr('hash_key', ['sequenceBlock'], sequenceBlock => sequenceBlock.hash_key) .attr('items', ['unitBlocks', 'sequenceBlock'], (unitBlocks, sequenceBlock) => unitBlocks.map( unitBlock => ({ href: '', @@ -44,10 +47,12 @@ Factory.define('sequenceMetadata') bookmarked: unitBlock.bookmarked || false, path: `Chapter Display Name > ${sequenceBlock.display_name} > ${unitBlock.display_name}`, type: unitBlock.type, + hash_key: unitBlock.hash_key, complete: unitBlock.complete || null, content: '', page_title: unitBlock.display_name, contains_content_type_gated_content: unitBlock.contains_content_type_gated_content, + decoded_id: unitBlock.decoded_id, }), )) .attrs({ diff --git a/src/setupTest.js b/src/setupTest.js index 4fb05c6e8a..19e4d0ae45 100755 --- a/src/setupTest.js +++ b/src/setupTest.js @@ -158,7 +158,7 @@ export async function initializeTestStore(options = {}, overrideStore = true) { axiosMock.onGet(courseBlocksUrlRegExp).reply(200, courseBlocks); axiosMock.onGet(learningSequencesUrlRegExp).reply(403, {}); sequenceMetadata.forEach(metadata => { - const sequenceMetadataUrl = `${getConfig().LMS_BASE_URL}/api/courseware/sequence/${metadata.item_id}`; + const sequenceMetadataUrl = `${getConfig().LMS_BASE_URL}/api/courseware/sequence/${metadata.decoded_id}`; axiosMock.onGet(sequenceMetadataUrl).reply(200, metadata); const proctoredExamApiUrl = `${getConfig().LMS_BASE_URL}/api/edx_proctoring/v1/proctored_exam/attempt/course_id/${courseMetadata.id}/content_id/${sequenceMetadata.item_id}?is_learning_mfe=true`; axiosMock.onGet(proctoredExamApiUrl).reply(200, { exam: {}, active_attempt: {} }); diff --git a/src/shared/data/__factories__/block.factory.js b/src/shared/data/__factories__/block.factory.js index f037c022fc..b37272ea9d 100644 --- a/src/shared/data/__factories__/block.factory.js +++ b/src/shared/data/__factories__/block.factory.js @@ -22,10 +22,18 @@ Factory.define('block') return blockId; }) + .attr( + 'hash_key', ['hash_key'], + () => (Math.random().toString(36).substring(2, 15)), + ) .attr( 'id', - ['id', 'block_id', 'type', 'courseId'], - (id, blockId, type, courseId) => { + ['id', 'block_id', 'type', 'courseId', 'hash_key'], + (id, blockId, type, courseId, hashKey) => { + if (hashKey) { + return hashKey; + } + if (id) { return id; } @@ -35,25 +43,33 @@ Factory.define('block') return `block-v1:${courseInfo}+type@${type}+block@${blockId}`; }, ) + .attr( + 'decoded_id', ['block_id', 'type', 'courseId'], + (blockId, type, courseId) => { + const courseInfo = courseId.split(':')[1]; + + return `block-v1:${courseInfo}+type@${type}+block@${blockId}`; + }, + ) .attr( 'student_view_url', - ['student_view_url', 'host', 'id'], - (url, host, id) => { + ['student_view_url', 'host', 'decoded_id'], + (url, host, decodedId) => { if (url) { return url; } - return `${host}/xblock/${id}`; + return `${host}/xblock/${decodedId}`; }, ) .attr( 'legacy_web_url', - ['legacy_web_url', 'host', 'courseId', 'id'], - (url, host, courseId, id) => { + ['legacy_web_url', 'host', 'courseId', 'decoded_id'], + (url, host, courseId, decodedId) => { if (url) { return url; } - return `${host}/courses/${courseId}/jump_to/${id}?experience=legacy`; + return `${host}/courses/${courseId}/jump_to/${decodedId}?experience=legacy`; }, ); diff --git a/src/tab-page/TabContainer.test.jsx b/src/tab-page/TabContainer.test.jsx index 55c81fa9f9..cd43ec6cdd 100644 --- a/src/tab-page/TabContainer.test.jsx +++ b/src/tab-page/TabContainer.test.jsx @@ -30,9 +30,9 @@ describe('Tab Container', () => { }); it('renders correctly', () => { - history.push(`/course/${courseId}`); + history.push(`/c/${courseId}`); render( - + , ); @@ -48,11 +48,11 @@ describe('Tab Container', () => { it('Should handle passing in a targetUserId', () => { const targetUserId = '1'; - history.push(`/course/${courseId}/progress/${targetUserId}/`); + history.push(`/c/${courseId}/progress/${targetUserId}/`); render( ( mockFetch(match.params.courseId, match.params.targetUserId)} From f2d7e119a52d02e14df5ac2973bbbda31f6678f9 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Fri, 30 Jul 2021 15:22:07 -0400 Subject: [PATCH 20/60] Update API call urls --- src/courseware/data/redux.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/courseware/data/redux.test.js b/src/courseware/data/redux.test.js index d3bea9048b..88db77f78d 100644 --- a/src/courseware/data/redux.test.js +++ b/src/courseware/data/redux.test.js @@ -38,7 +38,7 @@ describe('Data layer integration tests', () => { let courseUrl = `${courseBaseUrl}/${courseId}`; courseUrl = appendBrowserTimezoneToUrl(courseUrl); - const sequenceUrl = `${sequenceBaseUrl}/${sequenceMetadata.item_id}`; + const sequenceUrl = `${sequenceBaseUrl}/${sequenceMetadata.decoded_id}`; const sequenceId = sequenceBlocks[0].id; const unitId = unitBlocks[0].id; @@ -249,8 +249,7 @@ describe('Data layer integration tests', () => { }); describe('Test checkBlockCompletion', () => { - const getCompletionURL = `${getConfig().LMS_BASE_URL}/courses/${courseId}/xblock/${sequenceId}/handler/get_completion`; - + const getCompletionURL = `${getConfig().LMS_BASE_URL}/courses/${courseId}/xblock/${sequenceMetadata.decoded_id}/handler/get_completion`; it('Should fail to check completion and log error', async () => { axiosMock.onPost(getCompletionURL).networkError(); @@ -278,7 +277,7 @@ describe('Data layer integration tests', () => { }); describe('Test saveSequencePosition', () => { - const gotoPositionURL = `${getConfig().LMS_BASE_URL}/courses/${courseId}/xblock/${sequenceId}/handler/goto_position`; + const gotoPositionURL = `${getConfig().LMS_BASE_URL}/courses/${courseId}/xblock/${sequenceMetadata.decoded_id}/handler/goto_position`; it('Should change and revert sequence model activeUnitIndex in case of error', async () => { axiosMock.onPost(gotoPositionURL).networkError(); From 7b45c8b6fac7166499ab997dd15c017a38d3cafe Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Fri, 30 Jul 2021 16:24:52 -0400 Subject: [PATCH 21/60] Fix broken redirect --- src/courseware/course/course-exit/CourseExit.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/courseware/course/course-exit/CourseExit.jsx b/src/courseware/course/course-exit/CourseExit.jsx index 37b28cf135..a320b3d9c4 100644 --- a/src/courseware/course/course-exit/CourseExit.jsx +++ b/src/courseware/course/course-exit/CourseExit.jsx @@ -40,7 +40,7 @@ function CourseExit({ intl }) { } else if (mode === COURSE_EXIT_MODES.celebration) { body = (); } else { - return (); + return (); } return ( From 229692255f517933ab4e8ade9735c4abdedd59d6 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 2 Aug 2021 15:03:14 -0400 Subject: [PATCH 22/60] Fix broken sequence metadata URL --- src/setupTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/setupTest.js b/src/setupTest.js index 19e4d0ae45..4fb05c6e8a 100755 --- a/src/setupTest.js +++ b/src/setupTest.js @@ -158,7 +158,7 @@ export async function initializeTestStore(options = {}, overrideStore = true) { axiosMock.onGet(courseBlocksUrlRegExp).reply(200, courseBlocks); axiosMock.onGet(learningSequencesUrlRegExp).reply(403, {}); sequenceMetadata.forEach(metadata => { - const sequenceMetadataUrl = `${getConfig().LMS_BASE_URL}/api/courseware/sequence/${metadata.decoded_id}`; + const sequenceMetadataUrl = `${getConfig().LMS_BASE_URL}/api/courseware/sequence/${metadata.item_id}`; axiosMock.onGet(sequenceMetadataUrl).reply(200, metadata); const proctoredExamApiUrl = `${getConfig().LMS_BASE_URL}/api/edx_proctoring/v1/proctored_exam/attempt/course_id/${courseMetadata.id}/content_id/${sequenceMetadata.item_id}?is_learning_mfe=true`; axiosMock.onGet(proctoredExamApiUrl).reply(200, { exam: {}, active_attempt: {} }); From 7242583f1363d7b8a448fa3c1e420a67d31e96d5 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 2 Aug 2021 15:12:05 -0400 Subject: [PATCH 23/60] Fix variable in API call --- src/courseware/course/sequence/Unit.test.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/courseware/course/sequence/Unit.test.jsx b/src/courseware/course/sequence/Unit.test.jsx index 20bb9d560b..cf2d68a7a6 100644 --- a/src/courseware/course/sequence/Unit.test.jsx +++ b/src/courseware/course/sequence/Unit.test.jsx @@ -44,6 +44,7 @@ describe('Unit', () => { id: unit.id, courseId: courseMetadata.id, format: 'Homework', + decoded_id: unit.decoded_id, }; }); @@ -53,7 +54,7 @@ describe('Unit', () => { const renderedUnit = screen.getByTitle(unit.display_name); expect(renderedUnit).toHaveAttribute('height', String(0)); expect(renderedUnit).toHaveAttribute( - 'src', `http://localhost:18000/xblock/${mockData.id}?show_title=0&show_bookmark_button=0&recheck_access=1&view=student_view&format=${mockData.format}`, + 'src', `http://localhost:18000/xblock/${mockData.decoded_id}?show_title=0&show_bookmark_button=0&recheck_access=1&view=student_view&format=${mockData.format}`, ); }); From 9b33f20eaac88e24f3e5560ee743f243a1d42198 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 2 Aug 2021 15:23:25 -0400 Subject: [PATCH 24/60] Fix route path bug --- src/course-home/dates-tab/DatesTab.test.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/course-home/dates-tab/DatesTab.test.jsx b/src/course-home/dates-tab/DatesTab.test.jsx index 1d42904948..5c06487121 100644 --- a/src/course-home/dates-tab/DatesTab.test.jsx +++ b/src/course-home/dates-tab/DatesTab.test.jsx @@ -32,7 +32,7 @@ describe('DatesTab', () => { component = ( - + From 7f2df8b88637437a78968c3c27bcc011b2e6ffb9 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 2 Aug 2021 15:36:15 -0400 Subject: [PATCH 25/60] Update snapshot to reflect new storage of blocks by hash key --- .../data/__snapshots__/redux.test.js.snap | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/course-home/data/__snapshots__/redux.test.js.snap b/src/course-home/data/__snapshots__/redux.test.js.snap index e5aa549ac9..f5b0ea79d8 100644 --- a/src/course-home/data/__snapshots__/redux.test.js.snap +++ b/src/course-home/data/__snapshots__/redux.test.js.snap @@ -395,38 +395,39 @@ Object { }, "courseBlocks": Object { "courses": Object { - "block-v1:edX+DemoX+Demo_Course+type@course+block@bcdabcdabcdabcdabcdabcdabcdabcd3": Object { + "3iwl6mf8i6h": Object { "hasScheduledContent": false, "id": "course-v1:edX+DemoX+Demo_Course_1", "sectionIds": Array [ - "block-v1:edX+DemoX+Demo_Course+type@chapter+block@bcdabcdabcdabcdabcdabcdabcdabcd2", + "e42svlz653b", ], "title": "bcdabcdabcdabcdabcdabcdabcdabcd3", }, }, "sections": Object { - "block-v1:edX+DemoX+Demo_Course+type@chapter+block@bcdabcdabcdabcdabcdabcdabcdabcd2": Object { + "e42svlz653b": Object { "complete": false, "courseId": "course-v1:edX+DemoX+Demo_Course_1", - "id": "block-v1:edX+DemoX+Demo_Course+type@chapter+block@bcdabcdabcdabcdabcdabcdabcdabcd2", + "id": "e42svlz653b", "resumeBlock": false, "sequenceIds": Array [ - "block-v1:edX+DemoX+Demo_Course+type@sequential+block@bcdabcdabcdabcdabcdabcdabcdabcd1", + "g3boz4y8ntm", ], "title": "Title of Section", }, }, "sequences": Object { - "block-v1:edX+DemoX+Demo_Course+type@sequential+block@bcdabcdabcdabcdabcdabcdabcdabcd1": Object { + "g3boz4y8ntm": Object { "complete": false, "description": null, "due": null, "effortActivities": 2, "effortTime": 15, + "hash_key": "g3boz4y8ntm", "icon": null, - "id": "block-v1:edX+DemoX+Demo_Course+type@sequential+block@bcdabcdabcdabcdabcdabcdabcdabcd1", + "id": "g3boz4y8ntm", "legacyWebUrl": "http://localhost:18000/courses/course-v1:edX+DemoX+Demo_Course/jump_to/block-v1:edX+DemoX+Demo_Course+type@sequential+block@bcdabcdabcdabcdabcdabcdabcdabcd1?experience=legacy", - "sectionId": "block-v1:edX+DemoX+Demo_Course+type@chapter+block@bcdabcdabcdabcdabcdabcdabcdabcd2", + "sectionId": "e42svlz653b", "showLink": true, "title": "Title of Sequence", }, From f0fab488a5f53e52039324ce9e4173d891346d2f Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 2 Aug 2021 15:48:48 -0400 Subject: [PATCH 26/60] Fix broken urls --- src/courseware/CoursewareContainer.test.jsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/courseware/CoursewareContainer.test.jsx b/src/courseware/CoursewareContainer.test.jsx index 0cb111e676..aab37ef916 100644 --- a/src/courseware/CoursewareContainer.test.jsx +++ b/src/courseware/CoursewareContainer.test.jsx @@ -144,7 +144,7 @@ describe('CoursewareContainer', () => { } it('should initialize to show a spinner', () => { - history.push('/course/abc123'); + history.push('/c/abc123'); render(component); const spinner = screen.getByRole('status'); @@ -194,7 +194,7 @@ describe('CoursewareContainer', () => { unitId: unitBlocks[1].id, }); - history.push(`/course/${courseId}`); + history.push(`/c/${courseId}`); const container = await loadContainer(); assertLoadedHeader(container); @@ -217,7 +217,7 @@ describe('CoursewareContainer', () => { // Note how there is no sectionId/unitId returned in this mock response! axiosMock.onGet(`${getConfig().LMS_BASE_URL}/api/courseware/resume/${courseId}`).reply(200, {}); - history.push(`/course/${courseId}`); + history.push(`/c/${courseId}`); const container = await loadContainer(); assertLoadedHeader(container); @@ -237,7 +237,7 @@ describe('CoursewareContainer', () => { ); function setUrl(urlSequenceId, urlUnitId = null) { - history.push(`/course/${courseId}/${urlSequenceId}/${urlUnitId || ''}`); + history.push(`/c/${courseId}/${urlSequenceId}/${urlUnitId || ''}`); } function assertLocation(container, sequenceId, unitId) { @@ -379,11 +379,11 @@ describe('CoursewareContainer', () => { }); it('should navigate between units and check block completion', async () => { - axiosMock.onPost(`${courseId}/xblock/${sequenceBlock.id}/handler/get_completion`).reply(200, { + axiosMock.onPost(`${courseId}/xblock/${sequenceBlock.decoded_id}/handler/get_completion`).reply(200, { complete: true, }); - history.push(`/course/${courseId}/${sequenceBlock.id}/${unitBlocks[0].id}`); + history.push(`/c/${courseId}/${sequenceBlock.id}/${unitBlocks[0].id}`); const container = await loadContainer(); const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation button'); From 40ea41996f6ac3dbe196196c2a45d1855ba12e53 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 2 Aug 2021 15:54:26 -0400 Subject: [PATCH 27/60] Fix broken sequence URL variable --- src/courseware/data/redux.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/courseware/data/redux.test.js b/src/courseware/data/redux.test.js index feb4938279..b65ce71b50 100644 --- a/src/courseware/data/redux.test.js +++ b/src/courseware/data/redux.test.js @@ -37,7 +37,7 @@ describe('Data layer integration tests', () => { let courseUrl = `${courseBaseUrl}/${courseId}`; courseUrl = appendBrowserTimezoneToUrl(courseUrl); - const sequenceUrl = `${sequenceBaseUrl}/${sequenceMetadata.decoded_id}`; + const sequenceUrl = `${sequenceBaseUrl}/${sequenceMetadata.item_id}`; const sequenceId = sequenceBlocks[0].id; const unitId = unitBlocks[0].id; From 20935e78602770fbaac7d2ce0ebe49998cff665c Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 2 Aug 2021 16:20:32 -0400 Subject: [PATCH 28/60] Update snapshot to reflect new block hash keys --- .../data/__snapshots__/redux.test.js.snap | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/course-home/data/__snapshots__/redux.test.js.snap b/src/course-home/data/__snapshots__/redux.test.js.snap index f5b0ea79d8..c573eb84ae 100644 --- a/src/course-home/data/__snapshots__/redux.test.js.snap +++ b/src/course-home/data/__snapshots__/redux.test.js.snap @@ -395,39 +395,39 @@ Object { }, "courseBlocks": Object { "courses": Object { - "3iwl6mf8i6h": Object { + "abcdabcd3": Object { "hasScheduledContent": false, "id": "course-v1:edX+DemoX+Demo_Course_1", "sectionIds": Array [ - "e42svlz653b", + "abcdabcd2", ], "title": "bcdabcdabcdabcdabcdabcdabcdabcd3", }, }, "sections": Object { - "e42svlz653b": Object { + "abcdabcd2": Object { "complete": false, "courseId": "course-v1:edX+DemoX+Demo_Course_1", - "id": "e42svlz653b", + "id": "abcdabcd2", "resumeBlock": false, "sequenceIds": Array [ - "g3boz4y8ntm", + "abcdabcd1", ], "title": "Title of Section", }, }, "sequences": Object { - "g3boz4y8ntm": Object { + "abcdabcd1": Object { "complete": false, "description": null, "due": null, "effortActivities": 2, "effortTime": 15, - "hash_key": "g3boz4y8ntm", + "hash_key": "abcdabcd1", "icon": null, - "id": "g3boz4y8ntm", + "id": "abcdabcd1", "legacyWebUrl": "http://localhost:18000/courses/course-v1:edX+DemoX+Demo_Course/jump_to/block-v1:edX+DemoX+Demo_Course+type@sequential+block@bcdabcdabcdabcdabcdabcdabcdabcd1?experience=legacy", - "sectionId": "e42svlz653b", + "sectionId": "abcdabcd2", "showLink": true, "title": "Title of Sequence", }, From 5f0968e3489edb0252088d0de49e0fd0794113ec Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Mon, 2 Aug 2021 16:22:33 -0400 Subject: [PATCH 29/60] Update hash key to be consistently generated for snapshots --- src/shared/data/__factories__/block.factory.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/shared/data/__factories__/block.factory.js b/src/shared/data/__factories__/block.factory.js index b37272ea9d..0a8a65e731 100644 --- a/src/shared/data/__factories__/block.factory.js +++ b/src/shared/data/__factories__/block.factory.js @@ -23,8 +23,11 @@ Factory.define('block') return blockId; }) .attr( - 'hash_key', ['hash_key'], - () => (Math.random().toString(36).substring(2, 15)), + 'hash_key', ['block_id'], + (blockId) => { + const len = blockId.length; + return blockId.substring(23, len); + }, ) .attr( 'id', From 7fde146edd04a4c15c0fbc18f0e7c1387121866c Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Tue, 3 Aug 2021 10:05:55 -0400 Subject: [PATCH 30/60] Remove unused parameters --- src/courseware/data/__factories__/sequenceMetadata.factory.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/courseware/data/__factories__/sequenceMetadata.factory.js b/src/courseware/data/__factories__/sequenceMetadata.factory.js index 788611fa98..c5b3de3e37 100644 --- a/src/courseware/data/__factories__/sequenceMetadata.factory.js +++ b/src/courseware/data/__factories__/sequenceMetadata.factory.js @@ -27,12 +27,12 @@ Factory.define('sequenceMetadata') ), ) .attr('element_id', ['sequenceBlock'], sequenceBlock => sequenceBlock.block_id) - .attr('item_id', ['sequenceBlock'], sequenceBlock => (sequenceBlock.hash_key || sequenceBlock.id)) + .attr('item_id', ['sequenceBlock'], sequenceBlock => sequenceBlock.id) .attr('display_name', ['sequenceBlock'], sequenceBlock => sequenceBlock.display_name) .attr('gated_content', ['sequenceBlock'], sequenceBlock => ({ gated: false, prereq_url: null, - prereq_id: `${sequenceBlock.id}-prereq`, + prereq_id: `${sequenceBlock.decode_id}-prereq`, prereq_section_name: `${sequenceBlock.display_name}-prereq`, gated_section_name: sequenceBlock.display_name, })) From 2d1a13ab0a9b9bb111ed299c28e9f503228afe11 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Tue, 3 Aug 2021 10:09:11 -0400 Subject: [PATCH 31/60] Remove unused definitions --- src/shared/data/__factories__/block.factory.js | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/shared/data/__factories__/block.factory.js b/src/shared/data/__factories__/block.factory.js index 0a8a65e731..93269fe6a1 100644 --- a/src/shared/data/__factories__/block.factory.js +++ b/src/shared/data/__factories__/block.factory.js @@ -31,20 +31,8 @@ Factory.define('block') ) .attr( 'id', - ['id', 'block_id', 'type', 'courseId', 'hash_key'], - (id, blockId, type, courseId, hashKey) => { - if (hashKey) { - return hashKey; - } - - if (id) { - return id; - } - - const courseInfo = courseId.split(':')[1]; - - return `block-v1:${courseInfo}+type@${type}+block@${blockId}`; - }, + ['hash_key'], + (hashKey) => (hashKey), ) .attr( 'decoded_id', ['block_id', 'type', 'courseId'], From 07042d99087d7f103f64af737c52108c606eced2 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 4 Aug 2021 11:10:29 -0400 Subject: [PATCH 32/60] Update object key for unit and sequence to store by id if no hash_key --- src/course-home/data/api.js | 13 +++---------- src/courseware/data/api.js | 4 ++-- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/course-home/data/api.js b/src/course-home/data/api.js index b0ef3cf32e..a1ce673368 100644 --- a/src/course-home/data/api.js +++ b/src/course-home/data/api.js @@ -129,7 +129,7 @@ export function normalizeOutlineBlocks(courseId, blocks) { break; case 'sequential': - models.sequences[block.hash_key] = { + models.sequences[(block.hash_key || block.id)] = { complete: block.complete, description: block.description, due: block.due, @@ -167,15 +167,8 @@ export function normalizeOutlineBlocks(courseId, blocks) { Object.values(models.sections).forEach(section => { if (Array.isArray(section.sequenceIds)) { section.sequenceIds.forEach(sequenceId => { - const modelSequenceIds = {}; - Object.values(models.sequences).forEach(sequence => { - if (sequenceId === sequence.id) { - modelSequenceIds[sequenceId] = sequence.hash_key; - } - }); - if (sequenceId in modelSequenceIds) { - const sequence = modelSequenceIds[sequenceId]; - models.sequences.[sequence].sectionId = section.id; + if (sequenceId in models.sequences) { + models.sequences.[sequenceId].sectionId = section.id; } else { logInfo(`Section ${section.id} has child block ${sequenceId}, but that block is not in the list of sequences.`); } diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index 0d4e0e4485..afc87e1d22 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -31,7 +31,7 @@ export function normalizeBlocks(courseId, blocks) { break; case 'sequential': - models.sequences[block.hash_key] = { + models.sequences[(block.hash_key || block.id)] = { effortActivities: block.effort_activities, effortTime: block.effort_time, id: block.hash_key || block.id, @@ -42,7 +42,7 @@ export function normalizeBlocks(courseId, blocks) { }; break; case 'vertical': - models.units[block.hash_key] = { + models.units[(block.hash_key || block.id)] = { graded: block.graded, id: block.hash_key || block.id, title: block.display_name, From 75b195bdc0c90188f4c166d65b4def45fec16bfd Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 5 Aug 2021 09:29:58 -0400 Subject: [PATCH 33/60] Fix typos --- docs/decisions/0009-courseware-url-shortening.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/decisions/0009-courseware-url-shortening.md b/docs/decisions/0009-courseware-url-shortening.md index a2034ad048..da7a3e953d 100644 --- a/docs/decisions/0009-courseware-url-shortening.md +++ b/docs/decisions/0009-courseware-url-shortening.md @@ -18,7 +18,7 @@ https://learning.edx.org/course/course-v1:edX+DemoX.1+2T2019/block-v1:edX+DemoX. ``` -After exploring different URL patterns and possible redundancies in the current URL format. The course, run, and organization is stated in every portion of the URL. We also do not need the URL to tell us the type of block since it has been determined that all URLs will follow the path` /course/:courseId/:sequenceId/:unitId`. +After exploring different URL patterns and possible redundancies in the current URL format, the following key points were noticed. The course, run, and organization are stated in every portion of the URL. We also do not need the URL to tell us the type of block since it has been determined that all URLs will follow the path` /course/:courseId/:sequenceId/:unitId`. ## Decision @@ -41,8 +41,8 @@ https://learning.edx.org/c/course-v1:edX+DemoX.1+2T2019/YmxvY2/njuRCq/optional-e The fields definition and requirements ar as follows: * :courseId (required) - same as the previous `courseId`. -* :sequenceHash (required) - a `urlsafe_b64encode` version of the `sequenceId`. -* :unitHash (required) - a `urlsafe_b64encode` version of the `unitId`. +* :sequenceHash (required) - a `blake2b` version of the `sequenceId`'s `urlsafe_b64encode` . +* :unitHash (required) - a `blake2b` version of the `unitId`'s `urlsafe_b64encode`. * :sectionSlug (optional) - `display_name` of the current sequence's parent section. * :sequenceSlug (optional) - `display_name` of the current sequence. * :unitSlug (optional) - `display_name` of the current unit From 544e11b6284e1c860b1bfa96228079c085b8161d Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 5 Aug 2021 09:32:29 -0400 Subject: [PATCH 34/60] Add new flag to courseMetadata --- src/courseware/data/api.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index afc87e1d22..ae9f18789f 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -70,7 +70,7 @@ export function normalizeBlocks(courseId, blocks) { if (Array.isArray(section.sequenceIds)) { section.sequenceIds.forEach(sequenceId => { if (sequenceId in models.sequences) { - models.sequences.[sequenceId].sectionId = section.id; + models.sequences[sequenceId].sectionId = section.id; } else { logInfo(`Section ${section.id} has child block ${sequenceId}, but that block is not in the list of sequences.`); } @@ -82,7 +82,7 @@ export function normalizeBlocks(courseId, blocks) { if (Array.isArray(sequence.unitIds)) { sequence.unitIds.forEach(unitId => { if (unitId in models.units) { - models.units.[unitId].sequenceId = sequence.id; + models.units[unitId].sequenceId = sequence.id; } else { logInfo(`Sequence ${sequence.id} has child block ${unitId}, but that block is not in the list of units.`); } @@ -223,6 +223,7 @@ function normalizeMetadata(metadata) { specialExamsEnabledWaffleFlag: data.is_mfe_special_exams_enabled, proctoredExamsEnabledWaffleFlag: data.is_mfe_proctored_exams_enabled, isMasquerading: data.original_user_is_staff && !data.is_staff, + shortLinkFeatureFlag: data.mfe_short_url_is_active, }; } From 296607fb76da70759c57f6058d77ffbac76faef5 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 5 Aug 2021 14:57:18 -0400 Subject: [PATCH 35/60] Add new flag declarations --- src/course-home/data/__snapshots__/redux.test.js.snap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/course-home/data/__snapshots__/redux.test.js.snap b/src/course-home/data/__snapshots__/redux.test.js.snap index c573eb84ae..9c5a1584e7 100644 --- a/src/course-home/data/__snapshots__/redux.test.js.snap +++ b/src/course-home/data/__snapshots__/redux.test.js.snap @@ -16,6 +16,7 @@ Object { "proctoredExamsEnabledWaffleFlag": false, "sequenceId": null, "sequenceStatus": "loading", + "shortLinkFeatureFlag": false, "specialExamsEnabledWaffleFlag": false, }, "models": Object { @@ -321,6 +322,7 @@ Object { "proctoredExamsEnabledWaffleFlag": false, "sequenceId": null, "sequenceStatus": "loading", + "shortLinkFeatureFlag": false, "specialExamsEnabledWaffleFlag": false, }, "models": Object { @@ -508,6 +510,7 @@ Object { "proctoredExamsEnabledWaffleFlag": false, "sequenceId": null, "sequenceStatus": "loading", + "shortLinkFeatureFlag": false, "specialExamsEnabledWaffleFlag": false, }, "models": Object { From 4e136d9c55b533afe61ab13dcba412089353fdd6 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 5 Aug 2021 14:57:50 -0400 Subject: [PATCH 36/60] Add dispatch for new feature flag --- src/courseware/data/thunks.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/courseware/data/thunks.js b/src/courseware/data/thunks.js index 954290743f..66d83fbf56 100644 --- a/src/courseware/data/thunks.js +++ b/src/courseware/data/thunks.js @@ -14,6 +14,7 @@ import { import { setsSpecialExamsEnabled, setsProctoredExamsEnabled, + setsShortLinkFeatureFlag, fetchCourseRequest, fetchCourseSuccess, fetchCourseFailure, @@ -151,6 +152,9 @@ export function fetchCourse(courseId) { dispatch(setsProctoredExamsEnabled({ proctoredExamsEnabledWaffleFlag: courseMetadataResult.value.proctoredExamsEnabledWaffleFlag, })); + dispatch(setsShortLinkFeatureFlag({ + shortLinkFeatureFlag: courseMetadataResult.value.shortLinkFeatureFlag, + })); } if (courseBlocksResult.status === 'fulfilled') { From 24de9d7adda2753527985feb6bcee333ecc910f8 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 5 Aug 2021 14:58:24 -0400 Subject: [PATCH 37/60] Add new feature flag --- src/courseware/data/slice.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/courseware/data/slice.js b/src/courseware/data/slice.js index 8a515741a2..8933aca1b3 100644 --- a/src/courseware/data/slice.js +++ b/src/courseware/data/slice.js @@ -15,6 +15,7 @@ const slice = createSlice({ sequenceId: null, specialExamsEnabledWaffleFlag: false, proctoredExamsEnabledWaffleFlag: false, + shortLinkFeatureFlag: false, }, reducers: { setsSpecialExamsEnabled: (state, { payload }) => { @@ -23,6 +24,9 @@ const slice = createSlice({ setsProctoredExamsEnabled: (state, { payload }) => { state.proctoredExamsEnabledWaffleFlag = payload.proctoredExamsEnabledWaffleFlag; }, + setsShortLinkFeatureFlag: (state, { payload }) => { + state.shortLinkFeatureFlag = payload.shortLinkFeatureFlag; + }, fetchCourseRequest: (state, { payload }) => { state.courseId = payload.courseId; state.courseStatus = LOADING; @@ -57,6 +61,7 @@ const slice = createSlice({ export const { setsSpecialExamsEnabled, setsProctoredExamsEnabled, + setsShortLinkFeatureFlag, fetchCourseRequest, fetchCourseSuccess, fetchCourseFailure, From e05428e01d86720ed450fc56ca8912dd605a0e41 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 5 Aug 2021 14:59:10 -0400 Subject: [PATCH 38/60] Add new flag and function for old urls --- src/courseware/CoursewareContainer.jsx | 49 +++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index 28ef133394..7da50f1883 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -17,6 +17,12 @@ import { TabPage } from '../tab-page'; import Course from './course'; import { handleNextSectionCelebration } from './course/celebration'; +const checkUrlLength = memoize((shortLinkFeatureFlag, courseStatus, courseId, sequence) => { + if (shortLinkFeatureFlag && courseStatus === 'loaded' && sequence) { + history.replace(`/c/${courseId}/${sequence.id}/${sequence.unitIds[sequence.activeUnitIndex]}`); + } +}); + const checkResumeRedirect = memoize((courseStatus, courseId, sequenceId, firstSequenceId) => { if (courseStatus === 'loaded' && !sequenceId) { // Note that getResumeBlock is just an API call, not a redux thunk. @@ -127,6 +133,7 @@ class CoursewareContainer extends Component { firstSequenceId, unitViaSequenceId, sectionViaSequenceId, + shortLinkFeatureFlag, match: { params: { courseId: routeCourseId, @@ -135,10 +142,15 @@ class CoursewareContainer extends Component { }, }, } = this.props; + // Load data whenever the course or sequence ID changes. this.checkFetchCourse(routeCourseId); this.checkFetchSequence(routeSequenceHash); + if (sequence && routeSequenceHash.includes('block')) { + checkUrlLength(shortLinkFeatureFlag, courseStatus, courseId, sequence); + } + // All courseware URLs should normalize to the format /course/:courseId/:sequenceId/:unitId // via the series of redirection rules below. // See docs/decisions/0008-liberal-courseware-path-handling.md for more context. @@ -211,7 +223,6 @@ class CoursewareContainer extends Component { sequence, sequenceId, } = this.props; - if (nextSequence !== null) { let nextUnitId = null; if (nextSequence.unitIds.length > 0) { @@ -247,6 +258,8 @@ class CoursewareContainer extends Component { courseStatus, courseId, sequenceId, + sequence, + shortLinkFeatureFlag, match: { params: { unitId: routeUnitId, @@ -254,18 +267,30 @@ class CoursewareContainer extends Component { }, } = this.props; + // This helps process old URLS that still use a blocks usage key in the URL. + let updatedSequenceId; + let updatedUnitId; + if (sequence) { + if (sequenceId.includes('block') && shortLinkFeatureFlag) { + updatedSequenceId = sequence.id; + } + if (routeUnitId.includes('block') && shortLinkFeatureFlag) { + updatedUnitId = sequence.unitIds[sequence.activeUnitIndex]; + } + } + return ( state.models.sequences || {}, (state) => state.courseware.sequenceId, - (sequencesById, sequenceId) => (sequencesById[sequenceId] ? sequencesById[sequenceId] : null), + (sequencesById, sequenceId) => { + if (!sequencesById[sequenceId] && Object.keys(sequencesById).length > 0) { + for (let i = 0; i < Object.values(sequencesById).length; i++) { + const sequence = Object.values(sequencesById)[i]; + if (sequence.decoded_id === sequenceId) { + return sequence; + } + } + return null; + } + return sequencesById[sequenceId]; + }, ); const sequenceIdsSelector = createSelector( @@ -430,6 +467,7 @@ const mapStateToProps = (state) => { sequenceStatus, specialExamsEnabledWaffleFlag, proctoredExamsEnabledWaffleFlag, + shortLinkFeatureFlag, } = state.courseware; return { @@ -439,6 +477,7 @@ const mapStateToProps = (state) => { sequenceStatus, specialExamsEnabledWaffleFlag, proctoredExamsEnabledWaffleFlag, + shortLinkFeatureFlag, course: currentCourseSelector(state), sequence: currentSequenceSelector(state), previousSequence: previousSequenceSelector(state), From ea93aea4dde99898faa918cccef0c2ddb80b8234 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Fri, 6 Aug 2021 09:52:19 -0400 Subject: [PATCH 39/60] Add check for routeUnitId --- src/courseware/CoursewareContainer.jsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index 7da50f1883..809e7de6af 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -270,11 +270,11 @@ class CoursewareContainer extends Component { // This helps process old URLS that still use a blocks usage key in the URL. let updatedSequenceId; let updatedUnitId; - if (sequence) { - if (sequenceId.includes('block') && shortLinkFeatureFlag) { + if (shortLinkFeatureFlag) { + if (sequence && sequenceId.includes('block')) { updatedSequenceId = sequence.id; } - if (routeUnitId.includes('block') && shortLinkFeatureFlag) { + if (routeUnitId && routeUnitId.includes('block')) { updatedUnitId = sequence.unitIds[sequence.activeUnitIndex]; } } From 367c8ad0df2d899e943b55afef22f863f0907787 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 11 Aug 2021 10:33:15 -0400 Subject: [PATCH 40/60] Add new feature flag --- .../__factories__/outlineTabData.factory.js | 1 + .../data/__snapshots__/redux.test.js.snap | 17 +++++++++-------- src/course-home/data/api.js | 6 ++++-- .../__factories__/courseMetadata.factory.js | 1 + 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/course-home/data/__factories__/outlineTabData.factory.js b/src/course-home/data/__factories__/outlineTabData.factory.js index 0a740c3dc5..16245f1771 100644 --- a/src/course-home/data/__factories__/outlineTabData.factory.js +++ b/src/course-home/data/__factories__/outlineTabData.factory.js @@ -66,4 +66,5 @@ Factory.define('outlineTabData') handouts_html: '
  • Handout 1
', offer: null, welcome_message_html: '

Welcome to this course!

', + mfe_short_url_is_active: true, }); diff --git a/src/course-home/data/__snapshots__/redux.test.js.snap b/src/course-home/data/__snapshots__/redux.test.js.snap index 9c5a1584e7..a29039ab95 100644 --- a/src/course-home/data/__snapshots__/redux.test.js.snap +++ b/src/course-home/data/__snapshots__/redux.test.js.snap @@ -397,29 +397,29 @@ Object { }, "courseBlocks": Object { "courses": Object { - "abcdabcd3": Object { + "block-v1:edX+DemoX+Demo_Course+type@course+block@bcdabcdabcdabcdabcdabcdabcdabcd3": Object { "hasScheduledContent": false, "id": "course-v1:edX+DemoX+Demo_Course_1", "sectionIds": Array [ - "abcdabcd2", + "block-v1:edX+DemoX+Demo_Course+type@chapter+block@bcdabcdabcdabcdabcdabcdabcdabcd2", ], "title": "bcdabcdabcdabcdabcdabcdabcdabcd3", }, }, "sections": Object { - "abcdabcd2": Object { + "block-v1:edX+DemoX+Demo_Course+type@chapter+block@bcdabcdabcdabcdabcdabcdabcdabcd2": Object { "complete": false, "courseId": "course-v1:edX+DemoX+Demo_Course_1", - "id": "abcdabcd2", + "id": "block-v1:edX+DemoX+Demo_Course+type@chapter+block@bcdabcdabcdabcdabcdabcdabcdabcd2", "resumeBlock": false, "sequenceIds": Array [ - "abcdabcd1", + "block-v1:edX+DemoX+Demo_Course+type@sequential+block@bcdabcdabcdabcdabcdabcdabcdabcd1", ], "title": "Title of Section", }, }, "sequences": Object { - "abcdabcd1": Object { + "block-v1:edX+DemoX+Demo_Course+type@sequential+block@bcdabcdabcdabcdabcdabcdabcdabcd1": Object { "complete": false, "description": null, "due": null, @@ -427,9 +427,9 @@ Object { "effortTime": 15, "hash_key": "abcdabcd1", "icon": null, - "id": "abcdabcd1", + "id": "block-v1:edX+DemoX+Demo_Course+type@sequential+block@bcdabcdabcdabcdabcdabcdabcdabcd1", "legacyWebUrl": "http://localhost:18000/courses/course-v1:edX+DemoX+Demo_Course/jump_to/block-v1:edX+DemoX+Demo_Course+type@sequential+block@bcdabcdabcdabcdabcdabcdabcdabcd1?experience=legacy", - "sectionId": "abcdabcd2", + "sectionId": "block-v1:edX+DemoX+Demo_Course+type@chapter+block@bcdabcdabcdabcdabcdabcdabcdabcd2", "showLink": true, "title": "Title of Sequence", }, @@ -474,6 +474,7 @@ Object { "hasVisitedCourse": false, "url": "http://localhost:18000/courses/course-v1:edX+DemoX+Demo_Course/jump_to/block-v1:edX+Test+Block@12345abcde", }, + "shortLinkFeatureFlag": true, "timeOffsetMillis": 0, "userHasPassingGrade": undefined, "verifiedMode": Object { diff --git a/src/course-home/data/api.js b/src/course-home/data/api.js index a1ce673368..b13c7df364 100644 --- a/src/course-home/data/api.js +++ b/src/course-home/data/api.js @@ -129,7 +129,7 @@ export function normalizeOutlineBlocks(courseId, blocks) { break; case 'sequential': - models.sequences[(block.hash_key || block.id)] = { + models.sequences[block.id] = { complete: block.complete, description: block.description, due: block.due, @@ -168,7 +168,7 @@ export function normalizeOutlineBlocks(courseId, blocks) { if (Array.isArray(section.sequenceIds)) { section.sequenceIds.forEach(sequenceId => { if (sequenceId in models.sequences) { - models.sequences.[sequenceId].sectionId = section.id; + models.sequences[sequenceId].sectionId = section.id; } else { logInfo(`Section ${section.id} has child block ${sequenceId}, but that block is not in the list of sequences.`); } @@ -354,6 +354,7 @@ export async function getOutlineTabData(courseId) { const userHasPassingGrade = data.user_has_passing_grade; const verifiedMode = camelCaseObject(data.verified_mode); const welcomeMessageHtml = data.welcome_message_html; + const shortLinkFeatureFlag = data.mfe_short_url_is_active; return { accessExpiration, @@ -375,6 +376,7 @@ export async function getOutlineTabData(courseId) { userHasPassingGrade, verifiedMode, welcomeMessageHtml, + shortLinkFeatureFlag, }; } diff --git a/src/courseware/data/__factories__/courseMetadata.factory.js b/src/courseware/data/__factories__/courseMetadata.factory.js index 55bd3b3417..d2819374c5 100644 --- a/src/courseware/data/__factories__/courseMetadata.factory.js +++ b/src/courseware/data/__factories__/courseMetadata.factory.js @@ -58,4 +58,5 @@ Factory.define('courseMetadata') is_mfe_special_exams_enabled: false, is_mfe_proctored_exams_enabled: false, recommendations: null, + mfe_short_url_is_active: true, }); From a32a58019d7ca25cee5f7d4ab40b2b2d8f2758e8 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 11 Aug 2021 10:33:54 -0400 Subject: [PATCH 41/60] Add id to hash key mapping for sequences and units --- src/generic/model-store/slice.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/generic/model-store/slice.js b/src/generic/model-store/slice.js index 0bf587ff12..1769f2caed 100644 --- a/src/generic/model-store/slice.js +++ b/src/generic/model-store/slice.js @@ -6,6 +6,9 @@ function add(state, modelType, model) { if (state[modelType] === undefined) { state[modelType] = {}; } + if (modelType.includes('IdToHashKeyMap')) { + state[modelType][model.hash_key] = id; + } state[modelType][id] = model; } @@ -13,6 +16,9 @@ function update(state, modelType, model) { if (state[modelType] === undefined) { state[modelType] = {}; } + if (modelType.includes('IdToHashKeyMap')) { + state[modelType][model.hash_key] = model.id; + } state[modelType][model.id] = { ...state[modelType][model.id], ...model }; } From 0ba9ed7d314295f8939fee906fc56037b70d33a0 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 11 Aug 2021 10:34:32 -0400 Subject: [PATCH 42/60] Add hash key and mapping model --- src/courseware/data/thunks.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/courseware/data/thunks.js b/src/courseware/data/thunks.js index 66d83fbf56..917b0c44a0 100644 --- a/src/courseware/data/thunks.js +++ b/src/courseware/data/thunks.js @@ -101,6 +101,7 @@ function mergeLearningSequencesWithCourseBlocks(learningSequencesModels, courseB effortActivities: blocksSequence.effortActivities, effortTime: blocksSequence.effortTime, legacyWebUrl: blocksSequence.legacyWebUrl, + hash_key: blocksSequence.hash_key, unitIds: blocksSequence.unitIds, }; @@ -180,10 +181,18 @@ export function fetchCourse(courseId) { modelType: 'sequences', modelsMap: sequences, })); + dispatch(addModelsMap({ + modelType: 'sequenceIdToHashKeyMap', + modelsMap: sequences, + })); dispatch(updateModelsMap({ modelType: 'units', modelsMap: units, })); + dispatch(addModelsMap({ + modelType: 'unitIdToHashKeyMap', + modelsMap: units, + })); } const fetchedMetadata = courseMetadataResult.status === 'fulfilled'; @@ -235,10 +244,18 @@ export function fetchSequence(sequenceId) { modelType: 'sequences', model: sequence, })); + dispatch(updateModel({ + modelType: 'sequenceIdToHashKeyMap', + model: sequence, + })); dispatch(updateModels({ modelType: 'units', models: units, })); + dispatch(updateModels({ + modelType: 'unitIdToHashKeyMap', + models: units, + })); dispatch(fetchSequenceSuccess({ sequenceId })); } } catch (error) { From 5efc22220f541e457ff5d5964b455622008ad1c8 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 11 Aug 2021 10:35:17 -0400 Subject: [PATCH 43/60] Update model to store by id instead of hash key --- src/courseware/data/api.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index ae9f18789f..3eb9e99f7e 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -26,25 +26,25 @@ export function normalizeBlocks(courseId, blocks) { models.sections[block.id] = { id: block.id, title: block.display_name, - sequenceIds: block.hash_children || block.children || [], + sequenceIds: block.children || [], }; break; case 'sequential': - models.sequences[(block.hash_key || block.id)] = { + models.sequences[block.id] = { effortActivities: block.effort_activities, effortTime: block.effort_time, - id: block.hash_key || block.id, + id: block.id, title: block.display_name, legacyWebUrl: block.legacy_web_url, - unitIds: block.hash_children || block.children || [], + unitIds: block.children || [], hash_key: block.hash_key, }; break; case 'vertical': - models.units[(block.hash_key || block.id)] = { + models.units[block.id] = { graded: block.graded, - id: block.hash_key || block.id, + id: block.id, title: block.display_name, legacyWebUrl: block.legacy_web_url, hash_key: block.hash_key, @@ -260,6 +260,7 @@ function normalizeSequenceMetadata(sequence) { saveUnitPosition: sequence.save_position, showCompletion: sequence.show_completion, allowProctoringOptOut: sequence.allow_proctoring_opt_out, + hash_key: sequence.hash_key, decoded_id: sequence.decoded_id, }, units: sequence.items.map(unit => ({ @@ -272,6 +273,7 @@ function normalizeSequenceMetadata(sequence) { graded: unit.graded, containsContentTypeGatedContent: unit.contains_content_type_gated_content, decoded_id: unit.decoded_id, + hash_key: unit.hash_key, })), }; } From b4c83a38aad404a99a37da1bfe6ce61342c70d1c Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 11 Aug 2021 10:36:52 -0400 Subject: [PATCH 44/60] Update conditional checks to be based on hash_key instead of id --- src/courseware/CoursewareContainer.jsx | 70 ++++++++++++++++++-------- 1 file changed, 48 insertions(+), 22 deletions(-) diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index 809e7de6af..a85d8b1c0f 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -19,7 +19,7 @@ import { handleNextSectionCelebration } from './course/celebration'; const checkUrlLength = memoize((shortLinkFeatureFlag, courseStatus, courseId, sequence) => { if (shortLinkFeatureFlag && courseStatus === 'loaded' && sequence) { - history.replace(`/c/${courseId}/${sequence.id}/${sequence.unitIds[sequence.activeUnitIndex]}`); + history.replace(`/c/${courseId}/${sequence.hash_key}/${sequence.unitIds[sequence.activeUnitIndex]}`); } }); @@ -222,15 +222,20 @@ class CoursewareContainer extends Component { nextSequence, sequence, sequenceId, + shortLinkFeatureFlag, } = this.props; if (nextSequence !== null) { + let nextSequenceParam = nextSequence.id; + if (shortLinkFeatureFlag) { + nextSequenceParam = nextSequence.hash_key; + } let nextUnitId = null; if (nextSequence.unitIds.length > 0) { [nextUnitId] = nextSequence.unitIds; - history.push(`/c/${courseId}/${nextSequence.id}/${nextUnitId}`); + history.push(`/c/${courseId}/${nextSequenceParam}/${nextUnitId}`); } else { // Some sequences have no units. This will show a blank page with prev/next buttons. - history.push(`/c/${courseId}/${nextSequence.id}`); + history.push(`/c/${courseId}/${nextSequenceParam}`); } const celebrateFirstSection = course && course.celebrations && course.celebrations.firstSection; @@ -241,14 +246,22 @@ class CoursewareContainer extends Component { } handlePreviousSequenceClick = () => { - const { previousSequence, courseId } = this.props; + const { + previousSequence, + courseId, + shortLinkFeatureFlag, + } = this.props; if (previousSequence !== null) { + let previousSequenceParam = previousSequence.id; + if (shortLinkFeatureFlag) { + previousSequenceParam = previousSequence.hash_key; + } if (previousSequence.unitIds.length > 0) { const previousUnitId = previousSequence.unitIds[previousSequence.unitIds.length - 1]; - history.push(`/c/${courseId}/${previousSequence.id}/${previousUnitId}`); + history.push(`/c/${courseId}/${previousSequenceParam}/${previousUnitId}`); } else { // Some sequences have no units. This will show a blank page with prev/next buttons. - history.push(`/c/${courseId}/${previousSequence.id}`); + history.push(`/c/${courseId}/${previousSequenceParam}`); } } } @@ -270,11 +283,11 @@ class CoursewareContainer extends Component { // This helps process old URLS that still use a blocks usage key in the URL. let updatedSequenceId; let updatedUnitId; - if (shortLinkFeatureFlag) { - if (sequence && sequenceId.includes('block')) { + if (shortLinkFeatureFlag && sequence) { + if (!sequenceId.includes('block')) { updatedSequenceId = sequence.id; } - if (routeUnitId && routeUnitId.includes('block')) { + if (routeUnitId && !routeUnitId.includes('block')) { updatedUnitId = sequence.unitIds[sequence.activeUnitIndex]; } } @@ -309,6 +322,7 @@ const sequenceShape = PropTypes.shape({ id: PropTypes.string.isRequired, unitIds: PropTypes.arrayOf(PropTypes.string).isRequired, sectionId: PropTypes.string.isRequired, + hash_key: PropTypes.string.isRequired, isTimeLimited: PropTypes.bool, isProctored: PropTypes.bool, legacyWebUrl: PropTypes.string, @@ -374,17 +388,15 @@ const currentCourseSelector = createSelector( const currentSequenceSelector = createSelector( (state) => state.models.sequences || {}, (state) => state.courseware.sequenceId, - (sequencesById, sequenceId) => { - if (!sequencesById[sequenceId] && Object.keys(sequencesById).length > 0) { - for (let i = 0; i < Object.values(sequencesById).length; i++) { - const sequence = Object.values(sequencesById)[i]; - if (sequence.decoded_id === sequenceId) { - return sequence; - } + (state) => state.models.sequenceIdToHashKeyMap, + (sequencesById, sequenceId, sequenceMap) => { + if (!sequencesById[sequenceId] && Object.keys(sequencesById).length > 0 && sequenceMap) { + if (sequenceId in sequenceMap) { + const updatedSequenceId = sequenceMap[sequenceId]; + return sequencesById[updatedSequenceId]; } - return null; } - return sequencesById[sequenceId]; + return sequencesById[sequenceId] ? sequencesById[sequenceId] : null; }, ); @@ -405,11 +417,18 @@ const previousSequenceSelector = createSelector( sequenceIdsSelector, (state) => state.models.sequences || {}, (state) => state.courseware.sequenceId, - (sequenceIds, sequencesById, sequenceId) => { + (state) => state.models.sequenceIdToHashKeyMap, + (sequenceIds, sequencesById, sequenceId, sequenceMap) => { if (!sequenceId || sequenceIds.length === 0) { return null; } - const sequenceIndex = sequenceIds.indexOf(sequenceId); + let sequenceIndex = sequenceIds.indexOf(sequenceId); + if (!sequencesById[sequenceId] && Object.keys(sequencesById).length > 0 && sequenceMap) { + if (sequenceId in sequenceMap) { + const updatedSequenceId = sequenceMap[sequenceId]; + sequenceIndex = sequenceIds.indexOf(updatedSequenceId); + } + } const previousSequenceId = sequenceIndex > 0 ? sequenceIds[sequenceIndex - 1] : null; return previousSequenceId !== null ? sequencesById[previousSequenceId] : null; }, @@ -419,11 +438,18 @@ const nextSequenceSelector = createSelector( sequenceIdsSelector, (state) => state.models.sequences || {}, (state) => state.courseware.sequenceId, - (sequenceIds, sequencesById, sequenceId) => { + (state) => state.models.sequenceIdToHashKeyMap, + (sequenceIds, sequencesById, sequenceId, sequenceMap) => { if (!sequenceId || sequenceIds.length === 0) { return null; } - const sequenceIndex = sequenceIds.indexOf(sequenceId); + let sequenceIndex = sequenceIds.indexOf(sequenceId); + if (!sequencesById[sequenceId] && Object.keys(sequencesById).length > 0 && sequenceMap) { + if (sequenceId in sequenceMap) { + const updatedSequenceId = sequenceMap[sequenceId]; + sequenceIndex = sequenceIds.indexOf(updatedSequenceId); + } + } const nextSequenceId = sequenceIndex < sequenceIds.length - 1 ? sequenceIds[sequenceIndex + 1] : null; return nextSequenceId !== null ? sequencesById[nextSequenceId] : null; }, From 9c2190980ec046fa68a98cce2c5b1b252e042e72 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 11 Aug 2021 10:37:36 -0400 Subject: [PATCH 45/60] Add condition to know how to render sequence links --- src/course-home/outline-tab/Section.jsx | 33 ++++++++++++++++++------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/course-home/outline-tab/Section.jsx b/src/course-home/outline-tab/Section.jsx index b50781267f..45cf134948 100644 --- a/src/course-home/outline-tab/Section.jsx +++ b/src/course-home/outline-tab/Section.jsx @@ -28,6 +28,7 @@ function Section({ courseBlocks: { sequences, }, + shortLinkFeatureFlag, } = useModel('outline', courseId); const [open, setOpen] = useState(defaultOpen); @@ -39,6 +40,28 @@ function Section({ useEffect(() => { setOpen(defaultOpen); }, []); + let sequenceLinks; + if (shortLinkFeatureFlag) { + sequenceLinks = sequenceIds.map((sequenceId, index) => ( + + )); + } else { + sequenceLinks = sequenceIds.map((sequenceId, index) => ( + + )); + } const sectionTitle = (
@@ -96,15 +119,7 @@ function Section({ )} >
    - {sequenceIds.map((sequenceId, index) => ( - - ))} + {sequenceLinks}
From 7b57b06ed5f062a4d3ac3620a23584151303a8bc Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Wed, 11 Aug 2021 12:08:41 -0400 Subject: [PATCH 46/60] Update id generation --- src/shared/data/__factories__/block.factory.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/shared/data/__factories__/block.factory.js b/src/shared/data/__factories__/block.factory.js index 93269fe6a1..7eab3680b1 100644 --- a/src/shared/data/__factories__/block.factory.js +++ b/src/shared/data/__factories__/block.factory.js @@ -31,8 +31,16 @@ Factory.define('block') ) .attr( 'id', - ['hash_key'], - (hashKey) => (hashKey), + ['id', 'block_id', 'type', 'courseId'], + (id, blockId, type, courseId) => { + if (id) { + return id; + } + + const courseInfo = courseId.split(':')[1]; + + return `block-v1:${courseInfo}+type@${type}+block@${blockId}`; + }, ) .attr( 'decoded_id', ['block_id', 'type', 'courseId'], From 38db0ebfe1dbc1948a36f7b9a967da7885dcf228 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 09:50:06 -0400 Subject: [PATCH 47/60] Add page route and redirect to handle old urls --- src/courseware/CoursewareRedirectLandingPage.jsx | 6 ++++++ src/index.jsx | 1 + 2 files changed, 7 insertions(+) diff --git a/src/courseware/CoursewareRedirectLandingPage.jsx b/src/courseware/CoursewareRedirectLandingPage.jsx index f63ce4afbe..46b942be02 100644 --- a/src/courseware/CoursewareRedirectLandingPage.jsx +++ b/src/courseware/CoursewareRedirectLandingPage.jsx @@ -25,6 +25,12 @@ export default () => { path={`${path}/courseware/:courseId/unit/:unitId`} component={CoursewareRedirect} /> + { + global.location.assign(`/c/${match.params.courseId}/${match.params.sequenceId}/${match.params.unitId}`) + }} + /> { diff --git a/src/index.jsx b/src/index.jsx index 0ad2f25dae..54ebdc2c6e 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -34,6 +34,7 @@ subscribe(APP_READY, () => { + From 847cdfa0bd2c3efd5d2e582fba1154cbf1472344 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 09:50:51 -0400 Subject: [PATCH 48/60] Fix misplace semicolon --- src/courseware/CoursewareRedirectLandingPage.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/courseware/CoursewareRedirectLandingPage.jsx b/src/courseware/CoursewareRedirectLandingPage.jsx index 46b942be02..815e855527 100644 --- a/src/courseware/CoursewareRedirectLandingPage.jsx +++ b/src/courseware/CoursewareRedirectLandingPage.jsx @@ -28,7 +28,7 @@ export default () => { { - global.location.assign(`/c/${match.params.courseId}/${match.params.sequenceId}/${match.params.unitId}`) + global.location.assign(`/c/${match.params.courseId}/${match.params.sequenceId}/${match.params.unitId}`); }} /> Date: Thu, 12 Aug 2021 09:59:27 -0400 Subject: [PATCH 49/60] Update prereq_id --- src/courseware/data/__factories__/sequenceMetadata.factory.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/courseware/data/__factories__/sequenceMetadata.factory.js b/src/courseware/data/__factories__/sequenceMetadata.factory.js index c5b3de3e37..aadd5dca62 100644 --- a/src/courseware/data/__factories__/sequenceMetadata.factory.js +++ b/src/courseware/data/__factories__/sequenceMetadata.factory.js @@ -32,7 +32,7 @@ Factory.define('sequenceMetadata') .attr('gated_content', ['sequenceBlock'], sequenceBlock => ({ gated: false, prereq_url: null, - prereq_id: `${sequenceBlock.decode_id}-prereq`, + prereq_id: `${sequenceBlock.id}-prereq`, prereq_section_name: `${sequenceBlock.display_name}-prereq`, gated_section_name: sequenceBlock.display_name, })) From 6f2281c1a44e520d86a4b4b02a4168a51fc53bc3 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 10:01:31 -0400 Subject: [PATCH 50/60] Add hash_key to id translations --- src/courseware/data/thunks.js | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/courseware/data/thunks.js b/src/courseware/data/thunks.js index 917b0c44a0..995f47a51c 100644 --- a/src/courseware/data/thunks.js +++ b/src/courseware/data/thunks.js @@ -268,17 +268,25 @@ export function fetchSequence(sequenceId) { export function checkBlockCompletion(courseId, sequenceId, unitId) { return async (dispatch, getState) => { const { models } = getState(); + let modelsUnitId; if (models.units[unitId].complete) { + modelsUnitId = models.unitIdToHashKeyMap[unitId]; + } + let modelSequenceId = models.sequences[sequenceId]; + if (!modelSequenceId) { + modelSequenceId = models.sequenceIdToHashKeyMap[sequenceId]; + } + if (modelsUnitId.complete) { return; // do nothing. Things don't get uncompleted after they are completed. } try { - const isComplete = await getBlockCompletion(courseId, models.sequences[sequenceId].decoded_id, - models.units[unitId].decoded_id); + const isComplete = await getBlockCompletion(courseId, modelSequenceId, + modelsUnitId); dispatch(updateModel({ modelType: 'units', model: { - id: unitId, + id: modelsUnitId, complete: isComplete, }, })); @@ -291,23 +299,27 @@ export function checkBlockCompletion(courseId, sequenceId, unitId) { export function saveSequencePosition(courseId, sequenceId, activeUnitIndex) { return async (dispatch, getState) => { const { models } = getState(); - const initialActiveUnitIndex = models.sequences[sequenceId].activeUnitIndex; + let modelSequenceId = models.sequences[sequenceId]; + if (!modelSequenceId) { + modelSequenceId = models.sequenceIdToHashKeyMap[sequenceId]; + } + const initialActiveUnitIndex = modelSequenceId.activeUnitIndex; // Optimistically update the position. dispatch(updateModel({ modelType: 'sequences', model: { - id: sequenceId, + id: modelSequenceId, activeUnitIndex, }, })); try { - await postSequencePosition(courseId, models.sequences[sequenceId].decoded_id, activeUnitIndex); + await postSequencePosition(courseId, modelSequenceId, activeUnitIndex); // Update again under the assumption that the above call succeeded, since it doesn't return a // meaningful response. dispatch(updateModel({ modelType: 'sequences', model: { - id: sequenceId, + id: modelSequenceId, activeUnitIndex, }, })); @@ -316,7 +328,7 @@ export function saveSequencePosition(courseId, sequenceId, activeUnitIndex) { dispatch(updateModel({ modelType: 'sequences', model: { - id: sequenceId, + id: modelSequenceId, activeUnitIndex: initialActiveUnitIndex, }, })); From 43eb58974a44c524e99d740d190b1328f2d536f3 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 11:00:25 -0400 Subject: [PATCH 51/60] Update url length function --- src/courseware/CoursewareContainer.jsx | 30 +++++++++++++++++++------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index a85d8b1c0f..c89dfccc15 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -17,9 +17,9 @@ import { TabPage } from '../tab-page'; import Course from './course'; import { handleNextSectionCelebration } from './course/celebration'; -const checkUrlLength = memoize((shortLinkFeatureFlag, courseStatus, courseId, sequence) => { - if (shortLinkFeatureFlag && courseStatus === 'loaded' && sequence) { - history.replace(`/c/${courseId}/${sequence.hash_key}/${sequence.unitIds[sequence.activeUnitIndex]}`); +const checkUrlLength = memoize((shortLinkFeatureFlag, courseStatus, courseId, sequence, unitHashKey) => { + if (shortLinkFeatureFlag && courseStatus === 'loaded' && sequence && unitHashKey) { + history.replace(`/c/${courseId}/${sequence.hash_key}/${unitHashKey}`); } }); @@ -133,6 +133,7 @@ class CoursewareContainer extends Component { firstSequenceId, unitViaSequenceId, sectionViaSequenceId, + unitIdHashKeyMap, shortLinkFeatureFlag, match: { params: { @@ -142,13 +143,17 @@ class CoursewareContainer extends Component { }, }, } = this.props; - // Load data whenever the course or sequence ID changes. this.checkFetchCourse(routeCourseId); this.checkFetchSequence(routeSequenceHash); - - if (sequence && routeSequenceHash.includes('block')) { - checkUrlLength(shortLinkFeatureFlag, courseStatus, courseId, sequence); + if (sequence && routeSequenceHash.includes('block') && unitIdHashKeyMap) { + let unitHashKey; + Object.values(unitIdHashKeyMap).forEach(id => { + if (id === routeUnitId) { + unitHashKey = Object.keys(unitIdHashKeyMap).find(key => unitIdHashKeyMap[key] === id); + } + }); + checkUrlLength(shortLinkFeatureFlag, courseStatus, courseId, sequence, unitHashKey); } // All courseware URLs should normalize to the format /course/:courseId/:sequenceId/:unitId @@ -273,6 +278,7 @@ class CoursewareContainer extends Component { sequenceId, sequence, shortLinkFeatureFlag, + unitIdHashKeyMap, match: { params: { unitId: routeUnitId, @@ -288,7 +294,7 @@ class CoursewareContainer extends Component { updatedSequenceId = sequence.id; } if (routeUnitId && !routeUnitId.includes('block')) { - updatedUnitId = sequence.unitIds[sequence.activeUnitIndex]; + updatedUnitId = unitIdHashKeyMap[routeUnitId]; } } @@ -356,6 +362,7 @@ CoursewareContainer.propTypes = { previousSequence: sequenceShape, unitViaSequenceId: unitShape, sectionViaSequenceId: sectionShape, + unitIdHashKeyMap: unitShape, course: courseShape, sequence: sequenceShape, saveSequencePosition: PropTypes.func.isRequired, @@ -377,6 +384,7 @@ CoursewareContainer.defaultProps = { sectionViaSequenceId: null, course: null, sequence: null, + unitIdHashKeyMap: null, }; const currentCourseSelector = createSelector( @@ -485,6 +493,11 @@ const unitViaSequenceIdSelector = createSelector( (unitsById, sequenceId) => (unitsById[sequenceId] ? unitsById[sequenceId] : null), ); +const unitIdHashKeyMapSelector = createSelector( + (state) => state.models.unitIdToHashKeyMap, + (unitIdToHashKeyMap) => (unitIdToHashKeyMap), +); + const mapStateToProps = (state) => { const { courseId, @@ -511,6 +524,7 @@ const mapStateToProps = (state) => { firstSequenceId: firstSequenceIdSelector(state), sectionViaSequenceId: sectionViaSequenceIdSelector(state), unitViaSequenceId: unitViaSequenceIdSelector(state), + unitIdHashKeyMap: unitIdHashKeyMapSelector(state), }; }; From 569b628961bc17313a6e8e2fbcdee5c70f046d07 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 11:09:43 -0400 Subject: [PATCH 52/60] Update sequenceMetadata response --- src/courseware/CoursewareContainer.test.jsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/courseware/CoursewareContainer.test.jsx b/src/courseware/CoursewareContainer.test.jsx index aab37ef916..d87b454ae9 100644 --- a/src/courseware/CoursewareContainer.test.jsx +++ b/src/courseware/CoursewareContainer.test.jsx @@ -128,8 +128,10 @@ describe('CoursewareContainer', () => { axiosMock.onGet(courseMetadataUrl).reply(200, courseMetadata); sequenceMetadatas.forEach(sequenceMetadata => { - const sequenceMetadataUrl = `${getConfig().LMS_BASE_URL}/api/courseware/sequence/${sequenceMetadata.item_id}`; + const sequenceMetadataUrl = `${getConfig().LMS_BASE_URL}/api/courseware/sequence/${sequenceMetadata.hash_key}`; axiosMock.onGet(sequenceMetadataUrl).reply(200, sequenceMetadata); + const sequenceMetadataUrlFull = `${getConfig().LMS_BASE_URL}/api/courseware/sequence/${sequenceMetadata.item_id}`; + axiosMock.onGet(sequenceMetadataUrlFull).reply(200, sequenceMetadata); const proctoredExamApiUrl = `${getConfig().LMS_BASE_URL}/api/edx_proctoring/v1/proctored_exam/attempt/course_id/${courseId}/content_id/${sequenceMetadata.item_id}?is_learning_mfe=true`; axiosMock.onGet(proctoredExamApiUrl).reply(200, { exam: {}, active_attempt: {} }); }); From 5fa33e4015714603ac739504b62d90ec750ec970 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 14:30:25 -0400 Subject: [PATCH 53/60] Update checkBlockCompletion function unitID --- src/courseware/data/thunks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/courseware/data/thunks.js b/src/courseware/data/thunks.js index 995f47a51c..7a86c79d55 100644 --- a/src/courseware/data/thunks.js +++ b/src/courseware/data/thunks.js @@ -268,8 +268,8 @@ export function fetchSequence(sequenceId) { export function checkBlockCompletion(courseId, sequenceId, unitId) { return async (dispatch, getState) => { const { models } = getState(); - let modelsUnitId; - if (models.units[unitId].complete) { + let modelsUnitId = models.units[unitId]; + if (!modelsUnitId) { modelsUnitId = models.unitIdToHashKeyMap[unitId]; } let modelSequenceId = models.sequences[sequenceId]; From ea02b2f70f45c1c1233d14fa0d1486632a72f0af Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 14:32:02 -0400 Subject: [PATCH 54/60] Update links --- src/courseware/CoursewareContainer.test.jsx | 42 ++++++++++----------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/courseware/CoursewareContainer.test.jsx b/src/courseware/CoursewareContainer.test.jsx index d87b454ae9..4263dd1e19 100644 --- a/src/courseware/CoursewareContainer.test.jsx +++ b/src/courseware/CoursewareContainer.test.jsx @@ -192,8 +192,8 @@ describe('CoursewareContainer', () => { it('should use the resume block repsonse to pick a unit if it contains one', async () => { axiosMock.onGet(`${getConfig().LMS_BASE_URL}/api/courseware/resume/${courseId}`).reply(200, { - sectionId: sequenceBlock.id, - unitId: unitBlocks[1].id, + sectionId: sequenceBlock.hash_key, + unitId: unitBlocks[1].hash_key, }); history.push(`/c/${courseId}`); @@ -204,7 +204,7 @@ describe('CoursewareContainer', () => { expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents'); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); - expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[1].id); + expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[1].hash_key); }); it('should use the first sequence ID and activeUnitIndex if the resume block response is empty', async () => { @@ -255,11 +255,11 @@ describe('CoursewareContainer', () => { describe('when the URL contains a unit ID', () => { it('should ignore the section ID and redirect based on the unit ID', async () => { const urlUnit = unitTree[1][1][1]; - setUrl(sectionTree[1].id, urlUnit.id); + setUrl(sectionTree[1].id, urlUnit.hash_key); const container = await loadContainer(); assertLoadedHeader(container); assertSequenceNavigation(container, 2); - assertLocation(container, sequenceTree[1][1].id, urlUnit.id); + assertLocation(container, sequenceTree[1][1].hash_key, urlUnit.hash_key); }); }); @@ -269,7 +269,7 @@ describe('CoursewareContainer', () => { const container = await loadContainer(); assertLoadedHeader(container); assertSequenceNavigation(container, 2); - assertLocation(container, sequenceTree[1][0].id, unitTree[1][0][0].id); + assertLocation(container, sequenceTree[1][0].hash_key, unitTree[1][0][0].hash_key); }); }); @@ -316,15 +316,15 @@ describe('CoursewareContainer', () => { it('should insert the sequence ID into the URL', async () => { const unit = unitTree[1][0][1]; - history.push(`/c/${courseId}/${unit.id}`); + history.push(`/c/${courseId}/${unit.hash_key}`); const container = await loadContainer(); assertLoadedHeader(container); assertSequenceNavigation(container, 2); - const expectedSequenceId = sequenceTree[1][0].id; - const expectedUrl = `http://localhost/c/${courseId}/${expectedSequenceId}/${unit.id}`; + const expectedSequenceId = sequenceTree[1][0].hash_key; + const expectedUrl = `http://localhost/c/${courseId}/${expectedSequenceId}/${unit.hash_key}`; expect(global.location.href).toEqual(expectedUrl); - expect(container.querySelector('.fake-unit')).toHaveTextContent(unit.id); + expect(container.querySelector('.fake-unit')).toHaveTextContent(unit.hash_key); }); }); @@ -333,7 +333,7 @@ describe('CoursewareContainer', () => { const unitBlocks = defaultUnitBlocks; it('should pick the first unit if position was not defined (activeUnitIndex becomes 0)', async () => { - history.push(`/c/${courseId}/${sequenceBlock.id}`); + history.push(`/c/${courseId}/${sequenceBlock.hash_key}`); const container = await loadContainer(); assertLoadedHeader(container); @@ -341,7 +341,7 @@ describe('CoursewareContainer', () => { expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents'); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); - expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[0].id); + expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[0].hash_key); }); it('should use activeUnitIndex to pick a unit from the sequence', async () => { @@ -352,7 +352,7 @@ describe('CoursewareContainer', () => { ); setUpMockRequests({ sequenceMetadatas: [sequenceMetadata] }); - history.push(`/c/${courseId}/${sequenceBlock.id}`); + history.push(`/c/${courseId}/${sequenceBlock.hash_key}`); const container = await loadContainer(); assertLoadedHeader(container); @@ -360,7 +360,7 @@ describe('CoursewareContainer', () => { expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents'); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); - expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[2].id); + expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[2].hash_key); }); }); @@ -369,7 +369,7 @@ describe('CoursewareContainer', () => { const unitBlocks = defaultUnitBlocks; it('should load the specified unit', async () => { - history.push(`/c/${courseId}/${sequenceBlock.id}/${unitBlocks[2].id}`); + history.push(`/c/${courseId}/${sequenceBlock.hash_key}/${unitBlocks[2].hash_key}`); const container = await loadContainer(); assertLoadedHeader(container); @@ -377,15 +377,15 @@ describe('CoursewareContainer', () => { expect(container.querySelector('.fake-unit')).toHaveTextContent('Unit Contents'); expect(container.querySelector('.fake-unit')).toHaveTextContent(courseId); - expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[2].id); + expect(container.querySelector('.fake-unit')).toHaveTextContent(unitBlocks[2].hash_key); }); it('should navigate between units and check block completion', async () => { - axiosMock.onPost(`${courseId}/xblock/${sequenceBlock.decoded_id}/handler/get_completion`).reply(200, { + axiosMock.onPost(`${courseId}/xblock/${sequenceBlock.id}/handler/get_completion`).reply(200, { complete: true, }); - history.push(`/c/${courseId}/${sequenceBlock.id}/${unitBlocks[0].id}`); + history.push(`/c/${courseId}/${sequenceBlock.hash_key}/${unitBlocks[0].id}`); const container = await loadContainer(); const sequenceNavButtons = container.querySelectorAll('nav.sequence-navigation button'); @@ -393,7 +393,7 @@ describe('CoursewareContainer', () => { expect(sequenceNextButton).toHaveTextContent('Next'); fireEvent.click(sequenceNavButtons[4]); - expect(global.location.href).toEqual(`http://localhost/c/${courseId}/${sequenceBlock.id}/${unitBlocks[1].id}`); + expect(global.location.href).toEqual(`http://localhost/c/${courseId}/${sequenceBlock.hash_key}/${unitBlocks[1].id}`); }); }); @@ -421,7 +421,7 @@ describe('CoursewareContainer', () => { ); setUpMockRequests({ sequenceMetadatas: [sequenceMetadata] }); - history.push(`/c/${courseId}/${sequenceBlock.id}/${unitBlocks[2].id}`); + history.push(`/c/${courseId}/${sequenceBlock.hash_key}/${unitBlocks[2].hash_key}`); await loadContainer(); expect(global.location.assign).toHaveBeenCalledWith(sequenceBlock.legacy_web_url); @@ -442,7 +442,7 @@ describe('CoursewareContainer', () => { const { courseBlocks, sequenceBlocks, unitBlocks } = buildSimpleCourseBlocks(courseId, courseMetadata.name); setUpMockRequests({ courseBlocks, courseMetadata }); - history.push(`/c/${courseId}/${sequenceBlocks[0].id}/${unitBlocks[0].id}`); + history.push(`/c/${courseId}/${sequenceBlocks[0].hash_key}/${unitBlocks[0].hash_key}`); return { courseMetadata, unitBlocks }; } From a17e2a1a15bf6e8aa0f898f38ba095232464ab50 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 14:35:11 -0400 Subject: [PATCH 55/60] Update unitViaSequenceId to check if id is not block id --- src/courseware/CoursewareContainer.jsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/courseware/CoursewareContainer.jsx b/src/courseware/CoursewareContainer.jsx index c89dfccc15..1439e109e0 100644 --- a/src/courseware/CoursewareContainer.jsx +++ b/src/courseware/CoursewareContainer.jsx @@ -490,7 +490,16 @@ const sectionViaSequenceIdSelector = createSelector( const unitViaSequenceIdSelector = createSelector( (state) => state.models.units || {}, (state) => state.courseware.sequenceId, - (unitsById, sequenceId) => (unitsById[sequenceId] ? unitsById[sequenceId] : null), + (state) => state.models.unitIdHashKeyMap, + (unitsById, sequenceId, unitMap) => { + if (!unitsById[sequenceId] && Object.keys(unitsById).length > 0 && unitMap) { + if (sequenceId in unitMap) { + const updatedSequenceId = unitMap[sequenceId]; + return unitsById[updatedSequenceId]; + } + } + return unitsById[sequenceId] ? unitsById[sequenceId] : null; + }, ); const unitIdHashKeyMapSelector = createSelector( From 9ca5c6108864be7c7747b4de0d58f8fd2dcc709b Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 15:07:23 -0400 Subject: [PATCH 56/60] Fix id references --- src/courseware/CoursewareContainer.test.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/courseware/CoursewareContainer.test.jsx b/src/courseware/CoursewareContainer.test.jsx index 4263dd1e19..16b16516d4 100644 --- a/src/courseware/CoursewareContainer.test.jsx +++ b/src/courseware/CoursewareContainer.test.jsx @@ -255,7 +255,7 @@ describe('CoursewareContainer', () => { describe('when the URL contains a unit ID', () => { it('should ignore the section ID and redirect based on the unit ID', async () => { const urlUnit = unitTree[1][1][1]; - setUrl(sectionTree[1].id, urlUnit.hash_key); + setUrl(sectionTree[1].id, urlUnit.id); const container = await loadContainer(); assertLoadedHeader(container); assertSequenceNavigation(container, 2); @@ -316,7 +316,7 @@ describe('CoursewareContainer', () => { it('should insert the sequence ID into the URL', async () => { const unit = unitTree[1][0][1]; - history.push(`/c/${courseId}/${unit.hash_key}`); + history.push(`/c/${courseId}/${unit.id}`); const container = await loadContainer(); assertLoadedHeader(container); From c09ba486156079fec1176db07441022502ff4da7 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 15:33:35 -0400 Subject: [PATCH 57/60] Update sequence id for goto links --- src/courseware/data/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index 3eb9e99f7e..db70b21b83 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -283,7 +283,7 @@ export async function getSequenceMetadata(sequenceId) { return normalizeSequenceMetadata(data); } -const getSequenceHandlerUrl = (courseId, sequenceId) => `${getConfig().LMS_BASE_URL}/courses/${courseId}/xblock/${sequenceId}/handler`; +const getSequenceHandlerUrl = (courseId, sequenceId) => `${getConfig().LMS_BASE_URL}/courses/${courseId}/xblock/${sequenceId.decoded_id}/handler`; export async function getBlockCompletion(courseId, sequenceId, usageKey) { const { data } = await getAuthenticatedHttpClient().post( From fe4680646e7d27f3a2d8e254e957856b4e727c58 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 16:18:40 -0400 Subject: [PATCH 58/60] Update modelsSequenceId and modelsUnitId definitions --- src/courseware/data/thunks.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/courseware/data/thunks.js b/src/courseware/data/thunks.js index 7a86c79d55..ea249c061e 100644 --- a/src/courseware/data/thunks.js +++ b/src/courseware/data/thunks.js @@ -268,21 +268,20 @@ export function fetchSequence(sequenceId) { export function checkBlockCompletion(courseId, sequenceId, unitId) { return async (dispatch, getState) => { const { models } = getState(); - let modelsUnitId = models.units[unitId]; - if (!modelsUnitId) { + let modelsUnitId = unitId; + let modelsSequenceId = sequenceId; + if (!models.units[unitId]) { modelsUnitId = models.unitIdToHashKeyMap[unitId]; } - let modelSequenceId = models.sequences[sequenceId]; - if (!modelSequenceId) { - modelSequenceId = models.sequenceIdToHashKeyMap[sequenceId]; + if (!models.sequences[sequenceId]) { + modelsSequenceId = models.sequenceIdToHashKeyMap[sequenceId]; } - if (modelsUnitId.complete) { + if (models.units[modelsUnitId].complete) { return; // do nothing. Things don't get uncompleted after they are completed. } try { - const isComplete = await getBlockCompletion(courseId, modelSequenceId, - modelsUnitId); + const isComplete = await getBlockCompletion(courseId, modelSequenceId, modelsUnitId); dispatch(updateModel({ modelType: 'units', model: { @@ -299,11 +298,11 @@ export function checkBlockCompletion(courseId, sequenceId, unitId) { export function saveSequencePosition(courseId, sequenceId, activeUnitIndex) { return async (dispatch, getState) => { const { models } = getState(); - let modelSequenceId = models.sequences[sequenceId]; - if (!modelSequenceId) { + let modelSequenceId = sequenceId; + if (!models.sequences[sequenceId]) { modelSequenceId = models.sequenceIdToHashKeyMap[sequenceId]; } - const initialActiveUnitIndex = modelSequenceId.activeUnitIndex; + const initialActiveUnitIndex = models.sequences[modelSequenceId].activeUnitIndex; // Optimistically update the position. dispatch(updateModel({ modelType: 'sequences', From c6578d4e2ec90fee7b96477e3eb6bfc31e5a0db3 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 16:20:07 -0400 Subject: [PATCH 59/60] Update url to take direct sequence id --- src/courseware/data/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/courseware/data/api.js b/src/courseware/data/api.js index db70b21b83..3eb9e99f7e 100644 --- a/src/courseware/data/api.js +++ b/src/courseware/data/api.js @@ -283,7 +283,7 @@ export async function getSequenceMetadata(sequenceId) { return normalizeSequenceMetadata(data); } -const getSequenceHandlerUrl = (courseId, sequenceId) => `${getConfig().LMS_BASE_URL}/courses/${courseId}/xblock/${sequenceId.decoded_id}/handler`; +const getSequenceHandlerUrl = (courseId, sequenceId) => `${getConfig().LMS_BASE_URL}/courses/${courseId}/xblock/${sequenceId}/handler`; export async function getBlockCompletion(courseId, sequenceId, usageKey) { const { data } = await getAuthenticatedHttpClient().post( From 8882026a01f52fea2457ed505fb1a122b3b4b546 Mon Sep 17 00:00:00 2001 From: Kristin Aoki Date: Thu, 12 Aug 2021 16:24:23 -0400 Subject: [PATCH 60/60] Fix misspelled variable names --- src/courseware/data/thunks.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/courseware/data/thunks.js b/src/courseware/data/thunks.js index ea249c061e..785ed7f419 100644 --- a/src/courseware/data/thunks.js +++ b/src/courseware/data/thunks.js @@ -281,7 +281,7 @@ export function checkBlockCompletion(courseId, sequenceId, unitId) { } try { - const isComplete = await getBlockCompletion(courseId, modelSequenceId, modelsUnitId); + const isComplete = await getBlockCompletion(courseId, modelsSequenceId, modelsUnitId); dispatch(updateModel({ modelType: 'units', model: { @@ -298,27 +298,27 @@ export function checkBlockCompletion(courseId, sequenceId, unitId) { export function saveSequencePosition(courseId, sequenceId, activeUnitIndex) { return async (dispatch, getState) => { const { models } = getState(); - let modelSequenceId = sequenceId; + let modelsSequenceId = sequenceId; if (!models.sequences[sequenceId]) { - modelSequenceId = models.sequenceIdToHashKeyMap[sequenceId]; + modelsSequenceId = models.sequenceIdToHashKeyMap[sequenceId]; } - const initialActiveUnitIndex = models.sequences[modelSequenceId].activeUnitIndex; + const initialActiveUnitIndex = models.sequences[modelsSequenceId].activeUnitIndex; // Optimistically update the position. dispatch(updateModel({ modelType: 'sequences', model: { - id: modelSequenceId, + id: modelsSequenceId, activeUnitIndex, }, })); try { - await postSequencePosition(courseId, modelSequenceId, activeUnitIndex); + await postSequencePosition(courseId, modelsSequenceId, activeUnitIndex); // Update again under the assumption that the above call succeeded, since it doesn't return a // meaningful response. dispatch(updateModel({ modelType: 'sequences', model: { - id: modelSequenceId, + id: modelsSequenceId, activeUnitIndex, }, })); @@ -327,7 +327,7 @@ export function saveSequencePosition(courseId, sequenceId, activeUnitIndex) { dispatch(updateModel({ modelType: 'sequences', model: { - id: modelSequenceId, + id: modelsSequenceId, activeUnitIndex: initialActiveUnitIndex, }, }));