Skip to content

Commit

Permalink
prevent featured course carousel from re-randomizing (#1673)
Browse files Browse the repository at this point in the history
* use skipFeatured when invalidating userlists / learning paths after update

* add a new function for updating query data for user_list_parents and learning_path_parents on the featured courses list when either is edited without re-randomizing

* fix learning path invalidations

* fix test
  • Loading branch information
gumaerc authored Oct 10, 2024
1 parent fb1744f commit f8a5b1e
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 10 deletions.
11 changes: 7 additions & 4 deletions frontends/api/src/hooks/learningResources/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ describe("LearningPath CRUD", () => {
}

test("useLearningpathCreate calls correct API", async () => {
const { path, pathUrls, keys } = makeData()
const { path, pathUrls } = makeData()
const url = pathUrls.list

const requestData = { title: path.title }
Expand All @@ -226,9 +226,12 @@ describe("LearningPath CRUD", () => {

await waitFor(() => expect(result.current.isSuccess).toBe(true))
expect(makeRequest).toHaveBeenCalledWith("post", url, requestData)
expect(queryClient.invalidateQueries).toHaveBeenCalledWith(
keys.learningResources,
)
expect(queryClient.invalidateQueries).toHaveBeenCalledWith([
"learningResources",
"learningpaths",
"learning_paths",
"list",
])
})

test("useLearningpathDestroy calls correct API", async () => {
Expand Down
27 changes: 21 additions & 6 deletions frontends/api/src/hooks/learningResources/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import learningResources, {
invalidateResourceWithUserListQueries,
updateListParentsOnAdd,
updateListParentsOnDestroy,
updateListParents,
} from "./keyFactory"
import { ListType } from "../../common/constants"

Expand Down Expand Up @@ -123,9 +124,9 @@ const useLearningpathCreate = () => {
LearningPathResourceRequest: params,
}),
onSettled: () => {
// Invalidate everything: this is over-aggressive, but the new resource
// could appear in most lists
queryClient.invalidateQueries(learningResources._def)
queryClient.invalidateQueries(
learningResources.learningpaths._ctx.list._def,
)
},
})
}
Expand Down Expand Up @@ -343,12 +344,19 @@ const useLearningResourceSetUserListRelationships = () => {
) => learningResourcesApi.learningResourcesUserlistsPartialUpdate(params),
onSettled: (_response, _err, vars) => {
invalidateResourceQueries(queryClient, vars.id, {
skipFeatured: false,
skipFeatured: true,
})
vars.userlist_id?.forEach((userlistId) => {
invalidateUserListQueries(queryClient, userlistId)
})
},
onSuccess: (response, vars) => {
queryClient.setQueriesData<PaginatedLearningResourceList>(
learningResources.featured({}).queryKey,
(featured) =>
updateListParents(vars.id, featured, response.data, "userlist"),
)
},
})
}

Expand All @@ -361,14 +369,21 @@ const useLearningResourceSetLearningPathRelationships = () => {
learningResourcesApi.learningResourcesLearningPathsPartialUpdate(params),
onSettled: (_response, _err, vars) => {
invalidateResourceQueries(queryClient, vars.id, {
skipFeatured: false,
skipFeatured: true,
})
vars.learning_path_id?.forEach((learningPathId) => {
invalidateResourceQueries(queryClient, learningPathId, {
skipFeatured: false,
skipFeatured: true,
})
})
},
onSuccess: (response, vars) => {
queryClient.setQueriesData<PaginatedLearningResourceList>(
learningResources.featured({}).queryKey,
(featured) =>
updateListParents(vars.id, featured, response.data, "learningpath"),
)
},
})
}

Expand Down
36 changes: 36 additions & 0 deletions frontends/api/src/hooks/learningResources/keyFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,11 +371,47 @@ const updateListParentsOnDestroy = (
}
}

/**
* Given
* - a LearningResource ID
* - a paginated list of current resources
* - a list of new relationships
* - the type of list
* Update the resources' user_list_parents field to include the new relationships
*/
const updateListParents = (
resourceId: number,
staleResources?: PaginatedLearningResourceList,
newRelationships?: MicroUserListRelationship[],
listType?: "userlist" | "learningpath",
) => {
if (!resourceId || !staleResources || !newRelationships || !listType)
return staleResources
const matchIndex = staleResources.results.findIndex(
(res) => res.id === resourceId,
)
if (matchIndex === -1) return staleResources
const updatedResults = [...staleResources.results]
const newResource = { ...updatedResults[matchIndex] }
if (listType === "userlist") {
newResource.user_list_parents = newRelationships
}
if (listType === "learningpath") {
newResource.learning_path_parents = newRelationships
}
updatedResults[matchIndex] = newResource
return {
...staleResources,
results: updatedResults,
}
}

export default learningResources
export {
invalidateResourceQueries,
invalidateUserListQueries,
invalidateResourceWithUserListQueries,
updateListParentsOnAdd,
updateListParentsOnDestroy,
updateListParents,
}

0 comments on commit f8a5b1e

Please sign in to comment.