Query membership endpoints for resources belonging to user lists and learning paths #1846
Conversation
…Replace resource cache invalidation logic for membership keys
| const checkbox = await screen.findByLabelText<HTMLInputElement>(title) | ||
|
|
||
| expect(checkbox.checked).toBe(true) | ||
| await waitFor(() => { | ||
| expect(checkbox.checked).toBe(true) | ||
| }) | ||
|
|
There was a problem hiding this comment.
Suggestion: Drop the waitFor, and change the query to
const checkbox = await screen.findByRole<HTMLInputElement>("checkbox", {
name: title,
checked: true,
})(findBy whats, and if you switch it to findByRole you can add the checked option)
| await setLearningPathRelationships({ | ||
| id: resource.id, | ||
| learning_path_id: newParents, | ||
| }) | ||
| } else if (listType === ListType.UserList) { | ||
| const newParents = values.user_lists.map((id) => parseInt(id)) | ||
| const newParents = values.user_lists!.map((id) => parseInt(id)) |
There was a problem hiding this comment.
Requested change: Here and above, let's get rid of the non-null assertion. My rec instead would be to set defaults on the user_lists and learning_paths:
const formik = useFormik({
enableReinitialize: true,
initialValues: {
learning_paths: learningPathValues ?? [],
user_lists: userListValues ?? [],
},
// etc ...
})Opinion: We should not use obj!.prop. This has caused UI crashes in the past (New Resource Card: null image crashes page#4530) that would have been caught by TS had we not used a non-null assertion.
Additionally
I think the checkboxes should be disabled while membership info is still loading. This should be as simple as saying
<CheckboxChoiceField disabled={
!userListValues /* or maybe grab isLoading off the query*/
} />
but apparently CheckboxChoiceField doesn't have a disabled prop. Adding one should be easy—just a matter of passing the prop to the underlying html checkbox.
There was a problem hiding this comment.
I was reluctant to disable checkboxes here as these lists can never not be loaded by the time we open the dialog - the membership lists will have been fetched to fill the resource card action buttons; these hooks just select the value for the resource. I added for good measure as we could in theory use the dialog from some other entrypoint in the future, plus the checkboxes do need to be disableable (is that a word?) generally for shared use.
There was a problem hiding this comment.
as these lists can never not be loaded by the time
In principle they can be—if the membership endpoint response is slower than others for some reason. And there's no disadvantage (as far as I can tell) to disabling them while the data is loading.
plus the checkboxes do need to be disableable
👍 👍
There was a problem hiding this comment.
🎉🎉🎉🎉🎉🎉🎉🎉 x 100
…learning paths (#1846) * Retrieve user list and learning path memberships from new endpoints. Replace resource cache invalidation logic for membership keys * Remove commented dead code * Update Learning path hook and AddToListDialog tests * Null list memeberships from results * Pass in list booleans to the resource drawer * Remove redundant mock patch body * Fetch memeberships only when authenticated. Update tests. * Mock responses for tests * Clear list memberships from items list endpoints * Replace wait in test * Checkbox disabled state * Disable checkboxes if memberships not loaded
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/6032
Description (What does it do?)
Provides hooks for components to determine a resource's User List and Learning Path memberships. This replaces relationships being specified on resources in their
user_list_parentsandlearning_path_parentsarrays, so we have been able to remove all the query cache invalidation logic that determined which resources were stale when change were made to the parent lists. We now just invalidate the respective membershipList caches (on userLists and learningPaths).How can this be tested?
Resource card and learning drawer action buttons should highlight according to User List and Learning Path memberships and should be responsive to changes made in the Add to List dialogs.
Changes made in the Add to List dialogs in one screen should be reflected in other screens while navigating without page refresh (Homepage, Search, User List (/dashboard/my-lists/1) and Learning Path details (/learningpaths/1) pages.
If the only User List or Learning Path that a Learning Resource is a member of is deleted, the action button should no longer be highlighted.
Additional Context
The membership lists previously lived on
user_list_parentsandlearning_path_parentson learning resources, however these are specific to a user so cannot be cached on public CDN. Membership lists endpoints have been added to the API with #1808 so that they can be queried for independently of the learning resources.These do still exist on the API and can be cleaned out, but for now are overriden to
[]in the query hooks to ensure no remaining stray dependency.Also fixes (by bypass) an issue in Prod where resources are not bookmarked in the User List details screen as
user_list_parentson /api/v1/userlists/{userlist_id}/items/ is always empty.