I find it much more legible this way.
Some thoughts… as part of refactoring it, I made some of the redux selectors more formal, and made use of reselect more thoroughly. this resulted in a reduction in re-renders from 16 to 12 on your average page load. It’s also a bit more verbose, accounting for some of the increased line count.
I hadn’t tried it before, but found the memoize method of comparing previous props/state to current props/state to be very, very nice. Much easier than manually comparing props, and much clearer to me than using react hooks’ dependency arrays.
The lack of dependency arrays feels really freeing in general to me. They’ve been such a source of hard-to-track-down bugs, and the hooks linter does not always suggest the right solution for what belongs in and out of the array.
Function names are nice. We had a ton of custom hooks in there so that we could put names to otherwise anonymous bits of functionality.
Also note: this component has a test suite. It passed without any changes. 🥳
Fixes TNL-7175: Redirect to course home if a user is not unenrolled and the course is private.
- Require authentication to use the app while course blocks api requires it
- Gracefully handle course blocks api request failures allowing app to proceed to it redirection logic
Notable changes:
- selectors related to sequences are more resilient to missing models. In the case the course blocks api returns successfully but empty (in this case of enrolled but course not yet started).
- `fetchCourse` thunk handles failures for fetchCourseMeta and fetchCourseBlocks separately using `Promise.allSettled` instead of `Promise.all`
- `denied` is a new `courseStatus`
- Access denied redirect is done using a component at a new route `redirect/course-home/:courseId`
Now handles cases
- User is unauthenticated > redirect to login
- User is authenticated but not enrolled > redirects to lms course home
- When an enrolled user attempts to access courseware before the course start date they will load the sequence (but unable to load the vertical block). This behavior should be fixed in an update to edx-platform
* Extensive refactor of application data management.
- “course-blocks” and “course-meta” are replaced with “courseware” module. This obscures the difference between the two from the application itself.
- a generic “model-store” module is used to store all course, section, sequence, and unit data in a normalized way, agnostic to the metadata vs. blocks APIs.
- SequenceContainer has been removed, and it’s work is just done in CourseContainer instead.
- UI components are - in general - more responsible for deciding their own behavior during data loading. If they want to show a spinner or nothing, it’s up to their discretion.
- The API layer is responsible for normalizing data into a form the app will want to use, prior to putting it into the model store.
* Organizing into some more sub-modules.
- Bookmarks becomes it’s own module.
- SequenceNavigation becomes another one.
* More modularization of data directories.
- Moving model-store up to the top.
- Moving fetchCourse and fetchSequence up to the top-level data directory, since they’re used by both courseware and outline.
- Moving getBlockCompletion and updateSequencePosition into the courseware/data directory, since they pertain to that page.
* Normalizing on using the word “title”
* Using history.replace instead of history.push
This fixes TNL-7125
* Allowing sub-components to use hooks and redux
This reduces the amount of data we need to pass around, and lets us move some complexity to more natural modules.
* Fixing bug where enrollment alert is shown for undefined isEnrolled
The enrollment alert would inadvertently be shown if a user navigated from the outline to the course. This was because it interpreted an undefined “isEnrolled” flag as false. Instead, we should wait for the isEnrolled flag to be explicitly true or false.
* Organizing modules.
- Renaming “outline” to “course-home”.
- Moving sequence and sequence-navigation modules under the course module.
* Some final application organization and ADR write-ups.
* Final refactoring
- Favoring passing data by ID and looking it up in the store with useModel.
- Moving headers into course-header directory.
* Updating ADRs. Splitting model-store information out into its own ADR.
