Tracking analytics for onClick events in the Course Tool widget.
Extra: Fixed intl error in the Course Dates widget.
Co-authored-by: Carla Duarte <cduarte@edx.org>
* fix: Use reselect’s defaultMemoize instead of lodash.memoize
Lodash memoize doesn’t examine all parameters when deciding to memoize, apparently, meaning it doesn’t re-call the function if any parameter except the first changes.
More here: https://dev.to/nioufe/you-should-not-use-lodash-for-memoization-3441
* Fixing test setup. Improper use of sequenceMetadata factory!
Two problems:
One, we weren’t properly passing the courseId into our sequenceMetadata factories, so it was differing than the one defined in courseMetadata. This didn’t manifest until now because we weren’t using the one from sequenceMetadata until this memoization fix!
Two, I’d updated the options for sequenceMetadata to have a “unitBlocks” option, but didn’t update all the usages of the old “unitBlock” option. This meant it was manually creating its own unit instead of inheriting the one from courseBlocks, resulting in a different ID.
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. 🥳
We were inconsistently using “position” - a 1-indexed value - in JS arrays which are 0-indexed. We had an existing, normalized property called “activeUnitIndex” which we now use everywhere instead. The value is modified back to 1-indexed before being returned to the server.
* Adding testing-library dependencies, and bumping frontend-build to be compatible with them.
* Adding a function to initialize the redux store
We need to use it in a few places. Seems worth not-repeating, since they can easily get out of sync. In general, tests should only test the parts of the store they care about, as well.
* Adding function to initialize a mock application.
Ultimately I’d like to move this to frontend-platform as an alternative to ‘initialize’ for tests. ‘initialize’ is an async function which complicates matters.
* Using more explicit assertions for courseware reducer fields.
This removes the need for the snapshot file, and ensures our test is more resilient to unrelated changes in the store.
Also added a few more stages of assertions to some of the tests, showing that they have the right values over time.
* Adding a helper to build a simple course blocks response.
We can use this in the courseware data tests, and shortly in the tests for CoursewareContainer.
* Modifying sequenceMetadata factory to allow multiple units.
This will help us test sequence navigation’s behavior more fully by having multiple units in a sequence.
* A little linting and cleanup.
* Adding first round of tests for CoursewareContainer.
Tests loading, sequence navigation/unit rendering, and ‘denied’ states.
Subsequent tests will add tests for handlers.
* Do not redirect when the sequenceId is not valid
That is, if firstSequenceId is null or undefined. This prevents the url
becoming bogus but does cause the course contents display to become stuck
with the loading message.
* Detect invalid sequence when loading
If the course has no sections or the first section has no sub-sections,
then sequence will be null. Before the redirection fix, this would cause an
error, but after, the sequenceStatus never leaves the loading state. Thus,
if still loading and the sequence is null, return the no.content message.
* Check sequenceId instead of sequence
From David Joy:
During initial page load, I expect there's a period of time before the
course blocks API and the sequence metadata API come back where the
sequenceStatus is loading but the sequence is still false, meaning that
we'll see a flash of this 'no content' messaging for a moment before the
data comes in.
If we instead check whether sequenceId is null here, that may give us a
more accurate condition. The sequenceId in redux is only populated when we
begin to request a sequence (fetchSequence thunk). If we have no sequence
ID in the URL route, then fetchSequence never happens and the sequenceId in
redux stays null.
* Fix up some additional errors Piotr found
This fixes errors caused by deleting units or subsections.
* Move test for unit validity to SequenceContent
After further conversation, we decide that
we shouldn't just treat HTML blocks as units.
We've been making the assumption that Verticals
occupy the Unit-level, and we shouldn't break that
assumption for this specific case.
Reverts part of e04f588d1f.
However, the error handling changes remain.
In some cases (user schedule is close to or past the course end),
we don't actually have dates for a course, even if we would normally
have them.
When that happens, let's not show a banner saying we made a
schedule for the learner.
* Add test for fetchCourse
* Add tests for fetchDatesTab, fetchOutlineTab, fetchSequence and resetDeadlines
* Implement fetch tabs tests
* Add fail test case for fetchSequence
* Add success test for fetchSequence
* Add test for resetDeadlines
* Update test group name
* Add empty tests for courseware and bookmarks
* Fix wrong field in saveSequencePosition thunk
* Add tests for courseware data layer
* Temporary commit
* Split tests after rebase
* Revert "Fix wrong field in saveSequencePosition thunk"
This reverts commit 4394d363c58ad929f81e587ce4da2241528494b5.
* Fix test for position
* Move executeThunk into utils
* Add test for all reducers
* Add expect statements for logs
* Remove redundant snapshot tests and add some specific checks
* Polishing
* Remove redundant checks
* Fix bug in normalizer and update test
* Upgrade @edx/frontend-platform dependency
* Utilize MockAuthService instead of manual auth package mocking
* Update tests after breaking changes in master
* Remove redundant snapshot check
Fixes bug where courses with html-type XBlocks as children
of sequences would ignore those children instead of treating them
as units. This caused the app to later just give up and redirect
to the course home in the old experience.
Also, handle that scenario where we have sections/sequences
with children of unexpected block types more gracefully by
logging an error instead of crashing.
TNL-7305
* Moving model-store into “generic” sub-directory.
Also adding a README.md to explain what belongs in “generic”
* Moving user-messages into “generic” sub-directory.
* Moving PageLoading into “generic” sub-directory.
* Moving “tabs” module into “generic” sub-directory.
* Moving InstructorToolbar and MasqueradeWidget up to instructor-toolbar.
The masquerade widget is a sub-module of instructor-toolbar.
* Co-locating celebration APIs with celebration utils.
Also adding an ADR about thunk/API naming conventions and making some other areas of the code adhere to it.
* Moving courseware data (thunks, api) into the courseware module.
Note that cousre-home/data/api still uses normalizeBlocks - this should be fixed so it’s not reaching across. Maybe we pull that particular API up top.
This PR includes a few TODOs for things I saw, as well as a tiny bit of whitespace cleanup.
- Pulled Course Home specific components into `course-home`
- Created a courseHome reducer (and all necessary data files - api, thunks, slice)
- Removed Course Home logic from Courseware's data files (api, thunks, slice, etc.)
- Renamed Outline Tab URL to end in `/home` rather than `/outline` again (per Product)
Co-authored-by: Carla Duarte <cduarte@edx.org>
* BB-2569: Use faVideo instead of faFilm for video units; Set page title based on section, sequence, and course titles
* Add CourseLicense component with styling
* Reorder the pageTitleBreadCrumbs that are used for setting the page title
* Revert "Add CourseLicense component with styling"
This reverts commit 8d154998de.
* Fix package-lock.json so that only new changes for react-helmet are included
Including upcoming dates and a link to dates tab. Gives user ability to
look at any important upcoming dates for their course and to navigate
to upcoming assignments.
Co-authored-by: Daphne Li-Chen <daphneli-chen@MacBook-Pro.local>
The sequence.lmsWebUrl variable is loaded as part of the course blocks API. The status of that API’s request is stored in courseStatus.
The useEffect hook in useExamRedirect didn’t ensure that courseStatus was equal to “loaded”. This meant that if the sequence loaded first, it might attempt to redirect to sequence.lmsWebUrl even though that variable is still undefined.
When global.location.assign() is given `undefined` as a value, it tacks it onto the end of the URL and calls it a day. After that, we’ve got a badly formed URL.
- Updated the Outline Tab to fetch course blocks from the Outline API.
- Changed naming conventions to more accurately portray the tab naming scheme
(ex. Outline Tab, Dates Tab, etc.)
- Removed logic from `fetchCourses` that was specific to the Outline Tab
The useAlert hook was being given a new payload object every time it was called, defeating any memoization happening inside.
It was also re-calling it’s useEffect hook when alertId changed, which it was changing itself. That’s a no-no.
* Normalizing “courseInfo” back into “course”
Splitting it out denormalizes the data and introduces potential data inconsistencies.
* Name component JSX files with the name of the component.
* Normalizing some module exports/naming.
* Moving alerts into a sub-directory.
* DRYing up alert hook creation into reusable useAlert hook.
* Adding some comments about ‘courses’ hydration.
- Drop mock data, call real API instead
- Call course metadata API for general info, not the dates API
- Mark text as translatable
- Add badges and timeline dots, group same-day items
AA-116
