diff --git a/frontends/api/src/hooks/learningPaths/index.test.ts b/frontends/api/src/hooks/learningPaths/index.test.ts index e2b5041824..d16672b12b 100644 --- a/frontends/api/src/hooks/learningPaths/index.test.ts +++ b/frontends/api/src/hooks/learningPaths/index.test.ts @@ -5,8 +5,6 @@ import { LearningResource } from "../../generated/v1" import * as factories from "../../test-utils/factories" import { setupReactQueryTest } from "../test-utils" import { setMockResponse, urls, makeRequest } from "../../test-utils" -import { useFeaturedLearningResourcesList } from "../learningResources" -import { invalidateResourceQueries } from "../learningResources/invalidation" import keyFactory from "../learningResources/keyFactory" import { useLearningPathsDetail, @@ -15,24 +13,11 @@ import { useLearningPathCreate, useLearningPathDestroy, useLearningPathUpdate, - useLearningPathRelationshipMove, - useLearningPathRelationshipCreate, - useLearningPathRelationshipDestroy, } from "./index" import learningPathKeyFactory from "./keyFactory" const factory = factories.learningResources -jest.mock("../learningResources/invalidation", () => { - const actual = jest.requireActual("../learningResources/invalidation") - return { - __esModule: true, - ...actual, - invalidateResourceQueries: jest.fn(), - invalidateUserListQueries: jest.fn(), - } -}) - /** * Assert that `hook` queries the API with the correct `url`, `method`, and * exposes the API's data. @@ -172,6 +157,7 @@ describe("LearningPath CRUD", () => { await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(makeRequest).toHaveBeenCalledWith("post", url, requestData) + expect(queryClient.invalidateQueries).toHaveBeenCalledWith([ "learningPaths", "list", @@ -184,13 +170,23 @@ describe("LearningPath CRUD", () => { setMockResponse.delete(url, null) const { wrapper, queryClient } = setupReactQueryTest() + jest.spyOn(queryClient, "invalidateQueries") + const { result } = renderHook(useLearningPathDestroy, { wrapper, }) result.current.mutate({ id: path.id }) await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(makeRequest).toHaveBeenCalledWith("delete", url, undefined) - expect(invalidateResourceQueries).toHaveBeenCalledWith(queryClient, path.id) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith([ + "learningPaths", + "list", + ]) + expect(queryClient.invalidateQueries).toHaveBeenCalledWith([ + "learningPaths", + "membershipList", + ]) }) test("useLearningPathUpdate calls correct API", async () => { @@ -200,120 +196,22 @@ describe("LearningPath CRUD", () => { setMockResponse.patch(url, path) const { wrapper, queryClient } = setupReactQueryTest() + jest.spyOn(queryClient, "invalidateQueries") + const { result } = renderHook(useLearningPathUpdate, { wrapper }) result.current.mutate(patch) await waitFor(() => expect(result.current.isSuccess).toBe(true)) expect(makeRequest).toHaveBeenCalledWith("patch", url, patch) - expect(invalidateResourceQueries).toHaveBeenCalledWith(queryClient, path.id) - }) - - test("useLearningPathRelationshipMove calls correct API", async () => { - const { relationship, pathUrls, keys } = makeData() - const url = pathUrls.relationshipDetails - setMockResponse.patch(url, null) - - const { wrapper, queryClient } = setupReactQueryTest() - jest.spyOn(queryClient, "invalidateQueries") - const { result } = renderHook(useLearningPathRelationshipMove, { wrapper }) - result.current.mutate(relationship) - - await waitFor(() => expect(result.current.isSuccess).toBe(true)) - expect(makeRequest).toHaveBeenCalledWith( - "patch", - url, - expect.objectContaining({ position: relationship.position }), - ) - - expect(queryClient.invalidateQueries).toHaveBeenCalledWith( - keys.relationshipListing, - ) + expect(queryClient.invalidateQueries).toHaveBeenCalledWith([ + "learningPaths", + "list", + ]) + expect(queryClient.invalidateQueries).toHaveBeenCalledWith([ + "learningPaths", + "detail", + path.id, + ]) }) - - test.each([{ isChildFeatured: false }, { isChildFeatured: true }])( - "useLearningPathRelationshipCreate calls correct API and patches featured resources", - async ({ isChildFeatured }) => { - const { relationship, pathUrls, resourceWithoutList } = makeData() - - const featured = factory.resources({ count: 3 }) - if (isChildFeatured) { - featured.results[0] = resourceWithoutList - } - setMockResponse.get(urls.learningResources.featured(), featured) - - const url = pathUrls.relationshipList - const requestData = { - child: relationship.child, - parent: relationship.parent, - position: relationship.position, - } - setMockResponse.post(url, relationship) - - const { wrapper, queryClient } = setupReactQueryTest() - const { result } = renderHook(useLearningPathRelationshipCreate, { - wrapper, - }) - const { result: featuredResult } = renderHook( - useFeaturedLearningResourcesList, - { wrapper }, - ) - await waitFor(() => expect(featuredResult.current.data).toBe(featured)) - - result.current.mutate(requestData) - - await waitFor(() => expect(result.current.isSuccess).toBe(true)) - expect(makeRequest).toHaveBeenCalledWith("post", url, requestData) - - expect(invalidateResourceQueries).toHaveBeenCalledWith( - queryClient, - relationship.child, - { skipFeatured: false }, - ) - expect(invalidateResourceQueries).toHaveBeenCalledWith( - queryClient, - relationship.parent, - ) - }, - ) - - test.each([{ isChildFeatured: false }, { isChildFeatured: true }])( - "useLearningPathRelationshipDestroy calls correct API and patches child resource cache (isChildFeatured=$isChildFeatured)", - async ({ isChildFeatured }) => { - const { relationship, pathUrls } = makeData() - const url = pathUrls.relationshipDetails - - const featured = factory.resources({ count: 3 }) - if (isChildFeatured) { - featured.results[0] = relationship.resource - } - setMockResponse.get(urls.learningResources.featured(), featured) - - setMockResponse.delete(url, null) - const { wrapper, queryClient } = setupReactQueryTest() - - const { result } = renderHook(useLearningPathRelationshipDestroy, { - wrapper, - }) - const { result: featuredResult } = renderHook( - useFeaturedLearningResourcesList, - { wrapper }, - ) - - await waitFor(() => expect(featuredResult.current.data).toBe(featured)) - result.current.mutate(relationship) - await waitFor(() => expect(result.current.isSuccess).toBe(true)) - - expect(makeRequest).toHaveBeenCalledWith("delete", url, undefined) - expect(invalidateResourceQueries).toHaveBeenCalledWith( - queryClient, - relationship.child, - { skipFeatured: false }, - ) - expect(invalidateResourceQueries).toHaveBeenCalledWith( - queryClient, - relationship.parent, - ) - }, - ) }) diff --git a/frontends/api/src/hooks/learningPaths/index.ts b/frontends/api/src/hooks/learningPaths/index.ts index 4f7475a7e5..eb218b7604 100644 --- a/frontends/api/src/hooks/learningPaths/index.ts +++ b/frontends/api/src/hooks/learningPaths/index.ts @@ -10,13 +10,11 @@ import type { LearningpathsApiLearningpathsItemsListRequest as ItemsListRequest, LearningpathsApiLearningpathsCreateRequest as CreateRequest, LearningpathsApiLearningpathsDestroyRequest as DestroyRequest, - LearningPathRelationshipRequest, - MicroLearningPathRelationship, LearningPathResource, } from "../../generated/v1" import { learningPathsApi } from "../../clients" import learningPaths from "./keyFactory" -import { invalidateResourceQueries } from "../learningResources/invalidation" +import { useUserIsAuthenticated } from "api/hooks/user" const useLearningPathsList = ( params: ListRequest = {}, @@ -74,8 +72,9 @@ const useLearningPathUpdate = () => { id: params.id, PatchedLearningPathResourceRequest: params, }), - onSettled: (_data, _err, vars) => { - invalidateResourceQueries(queryClient, vars.id) + onSettled: (data, err, vars) => { + queryClient.invalidateQueries(learningPaths.list._def) + queryClient.invalidateQueries(learningPaths.detail(vars.id).queryKey) }, }) } @@ -85,8 +84,9 @@ const useLearningPathDestroy = () => { return useMutation({ mutationFn: (params: DestroyRequest) => learningPathsApi.learningpathsDestroy(params), - onSettled: (_data, _err, vars) => { - invalidateResourceQueries(queryClient, vars.id) + onSettled: () => { + queryClient.invalidateQueries(learningPaths.list._def) + queryClient.invalidateQueries(learningPaths.membershipList._def) }, }) } @@ -96,22 +96,6 @@ interface ListItemMoveRequest { id: number position?: number } -const useLearningPathRelationshipMove = () => { - const queryClient = useQueryClient() - return useMutation({ - mutationFn: ({ parent, id, position }: ListItemMoveRequest) => - learningPathsApi.learningpathsItemsPartialUpdate({ - learning_resource_id: parent, - id, - PatchedLearningPathRelationshipRequest: { position }, - }), - onSettled: (_data, _err, vars) => { - queryClient.invalidateQueries( - learningPaths.detail(vars.parent)._ctx.infiniteItems._def, - ) - }, - }) -} const useLearningPathListItemMove = () => { const queryClient = useQueryClient() @@ -131,47 +115,26 @@ const useLearningPathListItemMove = () => { }) } -const useLearningPathRelationshipCreate = () => { - const queryClient = useQueryClient() - return useMutation({ - mutationFn: (params: LearningPathRelationshipRequest) => - learningPathsApi.learningpathsItemsCreate({ - learning_resource_id: params.parent, - LearningPathRelationshipRequest: params, - }), - onSettled: (_response, _err, vars) => { - invalidateResourceQueries( - queryClient, - vars.child, - // do NOT skip invalidating the /featured/ lists, - // Changing a learning path might change the members of the featured - // lists. - { skipFeatured: false }, - ) - invalidateResourceQueries(queryClient, vars.parent) +const useIsLearningPathMember = (resourceId?: number) => { + return useQuery({ + ...learningPaths.membershipList(), + select: (data) => { + return !!data.find((relationship) => relationship.child === resourceId) }, + enabled: useUserIsAuthenticated() && !!resourceId, }) } -const useLearningPathRelationshipDestroy = () => { - const queryClient = useQueryClient() - return useMutation({ - mutationFn: (params: MicroLearningPathRelationship) => - learningPathsApi.learningpathsItemsDestroy({ - id: params.id, - learning_resource_id: params.parent, - }), - onSettled: (_response, _err, vars) => { - invalidateResourceQueries( - queryClient, - vars.child, - // do NOT skip invalidating the /featured/ lists, - // Changing a learning path might change the members of the featured - // lists. - { skipFeatured: false }, - ) - invalidateResourceQueries(queryClient, vars.parent) +const useLearningPathMemberList = (resourceId?: number) => { + return useQuery({ + ...learningPaths.membershipList(), + + select: (data) => { + return data + .filter((relationship) => relationship.child === resourceId) + .map((relationship) => relationship.parent.toString()) }, + enabled: useUserIsAuthenticated() && !!resourceId, }) } @@ -182,8 +145,7 @@ export { useLearningPathCreate, useLearningPathUpdate, useLearningPathDestroy, - useLearningPathRelationshipMove, useLearningPathListItemMove, - useLearningPathRelationshipCreate, - useLearningPathRelationshipDestroy, + useIsLearningPathMember, + useLearningPathMemberList, } diff --git a/frontends/api/src/hooks/learningPaths/keyFactory.ts b/frontends/api/src/hooks/learningPaths/keyFactory.ts index aa5dd886ce..fec3ba8d7d 100644 --- a/frontends/api/src/hooks/learningPaths/keyFactory.ts +++ b/frontends/api/src/hooks/learningPaths/keyFactory.ts @@ -6,6 +6,7 @@ import type { LearningpathsApiLearningpathsListRequest as ListRequest, PaginatedLearningPathRelationshipList, } from "../../generated/v1" +import { clearListMemberships } from "../learningResources/keyFactory" const learningPaths = createQueryKeys("learningPaths", { detail: (id: number) => ({ @@ -18,7 +19,7 @@ const learningPaths = createQueryKeys("learningPaths", { contextQueries: { infiniteItems: (itemsP: ItemsListRequest) => ({ queryKey: [itemsP], - queryFn: ({ pageParam }: { pageParam?: string } = {}) => { + queryFn: async ({ pageParam }: { pageParam?: string } = {}) => { // Use generated API for first request, then use next parameter const request = pageParam ? axiosInstance.request({ @@ -26,7 +27,14 @@ const learningPaths = createQueryKeys("learningPaths", { url: pageParam, }) : learningPathsApi.learningpathsItemsList(itemsP) - return request.then((res) => res.data) + const { data } = await request + return { + ...data, + results: data.results.map((relation) => ({ + ...relation, + resource: clearListMemberships(relation.resource), + })), + } }, }), }, @@ -37,6 +45,13 @@ const learningPaths = createQueryKeys("learningPaths", { return learningPathsApi.learningpathsList(params).then((res) => res.data) }, }), + membershipList: () => ({ + queryKey: ["membershipList"], + queryFn: async () => { + const { data } = await learningPathsApi.learningpathsMembershipList() + return data + }, + }), }) export default learningPaths diff --git a/frontends/api/src/hooks/learningResources/index.test.ts b/frontends/api/src/hooks/learningResources/index.test.ts index 34e6068938..40d71852bc 100644 --- a/frontends/api/src/hooks/learningResources/index.test.ts +++ b/frontends/api/src/hooks/learningResources/index.test.ts @@ -11,16 +11,6 @@ import { UseQueryResult } from "@tanstack/react-query" const factory = factories.learningResources -jest.mock("./invalidation", () => { - const actual = jest.requireActual("./invalidation") - return { - __esModule: true, - ...actual, - invalidateResourceQueries: jest.fn(), - invalidateUserListQueries: jest.fn(), - } -}) - /** * Assert that `hook` queries the API with the correct `url`, `method`, and * exposes the API's data. diff --git a/frontends/api/src/hooks/learningResources/index.ts b/frontends/api/src/hooks/learningResources/index.ts index 4e39a8fac3..b323cee041 100644 --- a/frontends/api/src/hooks/learningResources/index.ts +++ b/frontends/api/src/hooks/learningResources/index.ts @@ -12,14 +12,12 @@ import type { OfferorsApiOfferorsListRequest, PlatformsApiPlatformsListRequest, FeaturedApiFeaturedListRequest as FeaturedListParams, - PaginatedLearningResourceList, LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest, LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest, - MicroUserListRelationship, } from "../../generated/v1" import learningResources from "./keyFactory" -import { invalidateResourceQueries } from "./invalidation" -import { invalidateUserListQueries } from "../userLists/invalidation" +import userLists from "../userLists/keyFactory" +import learningPaths from "../learningPaths/keyFactory" const useLearningResourcesList = ( params: LRListRequest = {}, @@ -75,20 +73,8 @@ const useLearningResourceSetUserListRelationships = () => { mutationFn: ( params: LearningResourcesApiLearningResourcesUserlistsPartialUpdateRequest, ) => learningResourcesApi.learningResourcesUserlistsPartialUpdate(params), - onSettled: (_response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.id, { - skipFeatured: true, - }) - vars.userlist_id?.forEach((userlistId) => { - invalidateUserListQueries(queryClient, userlistId) - }) - }, - onSuccess: (response, vars) => { - queryClient.setQueriesData( - learningResources.featured({}).queryKey, - (featured) => - updateListParents(vars.id, featured, response.data, "userlist"), - ) + onSettled: () => { + queryClient.invalidateQueries(userLists.membershipList().queryKey) }, }) } @@ -100,22 +86,8 @@ const useLearningResourceSetLearningPathRelationships = () => { params: LearningResourcesApiLearningResourcesLearningPathsPartialUpdateRequest, ) => learningResourcesApi.learningResourcesLearningPathsPartialUpdate(params), - onSettled: (_response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.id, { - skipFeatured: true, - }) - vars.learning_path_id?.forEach((learningPathId) => { - invalidateResourceQueries(queryClient, learningPathId, { - skipFeatured: true, - }) - }) - }, - onSuccess: (response, vars) => { - queryClient.setQueriesData( - learningResources.featured({}).queryKey, - (featured) => - updateListParents(vars.id, featured, response.data, "learningpath"), - ) + onSettled: () => { + queryClient.invalidateQueries(learningPaths.membershipList().queryKey) }, }) } @@ -144,41 +116,6 @@ const useSchoolsList = () => { return useQuery(learningResources.schools()) } -/** - * 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, - } -} - const useSimilarLearningResources = ( id: number, opts: Pick = {}, @@ -211,7 +148,7 @@ export { useOfferorsList, usePlatformsList, useSchoolsList, - learningResources as learningResourcesKeyFactory, useSimilarLearningResources, useVectorSimilarLearningResources, + learningResources as learningResourcesKeyFactory, } diff --git a/frontends/api/src/hooks/learningResources/invalidation.ts b/frontends/api/src/hooks/learningResources/invalidation.ts deleted file mode 100644 index 58f120a361..0000000000 --- a/frontends/api/src/hooks/learningResources/invalidation.ts +++ /dev/null @@ -1,109 +0,0 @@ -import type { QueryClient, Query } from "@tanstack/react-query" -import type { - PaginatedLearningResourceList, - LearningResource, -} from "../../generated/v1" -import learningResources from "./keyFactory" -import learningPaths from "../learningPaths/keyFactory" -import userLists from "../userLists/keyFactory" -import { listHasResource } from "../userLists/invalidation" - -/** - * Invalidate Resource queries that a specific resource appears in. - * - * By default, this will invalidate featured list queries. This can result in - * odd behavior because the featured list item order is randomized: when the - * featured list cache is invalidated, the newly fetched data may be in a - * different order. To maintain the order, use skipFeatured to skip invalidation - * of featured lists and instead manually update the cached data via - * `updateListParentsOnAdd`. - */ -const invalidateResourceQueries = ( - queryClient: QueryClient, - resourceId: LearningResource["id"], - { skipFeatured = false } = {}, -) => { - /** - * Invalidate details queries. - * In this case, looking up queries by key is easy. - */ - queryClient.invalidateQueries(learningResources.detail(resourceId).queryKey) - queryClient.invalidateQueries(learningPaths.detail(resourceId).queryKey) - queryClient.invalidateQueries(userLists.detail(resourceId).queryKey) - /** - * Invalidate lists that the resource belongs to. - * Check for actual membership. - */ - const lists = [ - learningResources.list._def, - learningPaths.list._def, - learningResources.search._def, - ...(skipFeatured ? [] : [learningResources.featured._def]), - ] - lists.forEach((queryKey) => { - queryClient.invalidateQueries({ - queryKey, - predicate: listHasResource(resourceId), - }) - }) -} - -/** - * Invalidate Resource queries that a resource that belongs to user list appears in. - */ -const invalidateResourceWithUserListQueries = ( - queryClient: QueryClient, - userListId: LearningResource["id"], -) => { - /** - * Invalidate resource detail query for resource that is in the user list. - */ - queryClient.invalidateQueries({ - queryKey: learningResources.detail._def, - predicate: resourceHasUserList(userListId), - }) - - /** - * Invalidate lists with a resource that is in the user list. - */ - const lists = [ - learningResources.list._def, - learningPaths.list._def, - learningResources.search._def, - learningResources.featured._def, - ] - - lists.forEach((queryKey) => { - queryClient.invalidateQueries({ - queryKey, - predicate: resourcesHaveUserList(userListId), - }) - }) -} - -const resourcesHaveUserList = - (userListId: number) => - (query: Query): boolean => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const data = query.state.data as any - const resources: LearningResource[] = data.pages - ? data.pages.flatMap( - (page: PaginatedLearningResourceList) => page.results, - ) - : data.results - - return resources?.some((res) => - res.user_list_parents?.some((userList) => userList.parent === userListId), - ) - } - -const resourceHasUserList = - (userListId: number) => - (query: Query): boolean => { - const data = query.state.data as LearningResource - return !!data.user_list_parents?.some( - (userList) => userList.parent === userListId, - ) - } - -export { invalidateResourceQueries, invalidateResourceWithUserListQueries } diff --git a/frontends/api/src/hooks/learningResources/keyFactory.ts b/frontends/api/src/hooks/learningResources/keyFactory.ts index f56dc58462..551e44ee80 100644 --- a/frontends/api/src/hooks/learningResources/keyFactory.ts +++ b/frontends/api/src/hooks/learningResources/keyFactory.ts @@ -47,28 +47,48 @@ const randomizeResults = ([...results]) => { return randomizedResults } +/* List memberships were previously determined in the learningResourcesApi + * from user_list_parents and learning_path_parents on each resource. + * Resource endpoints are now treated as public so that they can be + * server rendered and cached on public CDN. The membership endpoints on + * learningPathsApi and userListsApi are now used to determine membership. + * Removing here to ensure they are not depended on anywhere, though they can + * be removed from the GET APIs TODO. + */ +export const clearListMemberships = (resource: LearningResource) => ({ + ...resource, + user_list_parents: [], + learning_path_parents: [], +}) + const learningResources = createQueryKeys("learningResources", { detail: (id: number) => ({ queryKey: [id], - queryFn: () => - learningResourcesApi - .learningResourcesRetrieve({ id }) - .then((res) => res.data), + queryFn: async () => { + const { data } = await learningResourcesApi.learningResourcesRetrieve({ + id, + }) + return clearListMemberships(data) + }, }), list: (params: LearningResourcesListRequest) => ({ queryKey: [params], - queryFn: () => - learningResourcesApi - .learningResourcesList(params) - .then((res) => res.data), + queryFn: async () => { + const { data } = await learningResourcesApi.learningResourcesList(params) + return { + ...data, + results: data.results.map(clearListMemberships), + } + }, }), featured: (params: FeaturedListParams = {}) => ({ queryKey: [params], - queryFn: () => { - return featuredApi.featuredList(params).then((res) => { - res.data.results = randomizeResults(res.data?.results) - return res.data - }) + queryFn: async () => { + const { data } = await featuredApi.featuredList(params) + return { + ...data, + results: randomizeResults(data.results.map(clearListMemberships)), + } }, }), topic: (id: number | undefined) => ({ @@ -80,51 +100,43 @@ const learningResources = createQueryKeys("learningResources", { queryKey: [params], queryFn: () => topicsApi.topicsList(params).then((res) => res.data), }), - search: (params: LearningResourcesSearchRetrieveRequest) => { - return { - queryKey: [params], - queryFn: () => - learningResourcesSearchApi - .learningResourcesSearchRetrieve(params) - .then((res) => res.data), - } - }, - offerors: (params: OfferorsApiOfferorsListRequest) => { - return { - queryKey: [params], - queryFn: () => offerorsApi.offerorsList(params).then((res) => res.data), - } - }, - platforms: (params: PlatformsApiPlatformsListRequest) => { - return { - queryKey: [params], - queryFn: () => platformsApi.platformsList(params).then((res) => res.data), - } - }, - schools: () => { - return { - queryKey: ["schools"], - queryFn: () => schoolsApi.schoolsList().then((res) => res.data), - } - }, - similar: (id: number) => { - return { - queryKey: [`similar_resources-${id}`], - queryFn: () => - learningResourcesApi - .learningResourcesSimilarList({ id }) - .then((res) => res.data), - } - }, - vectorSimilar: (id: number) => { - return { - queryKey: [`vector_similar_resources-${id}`], - queryFn: () => - learningResourcesApi - .learningResourcesVectorSimilarList({ id }) - .then((res) => res.data), - } - }, + search: (params: LearningResourcesSearchRetrieveRequest) => ({ + queryKey: [params], + queryFn: async () => { + const { data } = + await learningResourcesSearchApi.learningResourcesSearchRetrieve(params) + return { + ...data, + results: data.results.map(clearListMemberships), + } + }, + }), + offerors: (params: OfferorsApiOfferorsListRequest) => ({ + queryKey: [params], + queryFn: () => offerorsApi.offerorsList(params).then((res) => res.data), + }), + platforms: (params: PlatformsApiPlatformsListRequest) => ({ + queryKey: [params], + queryFn: () => platformsApi.platformsList(params).then((res) => res.data), + }), + schools: () => ({ + queryKey: ["schools"], + queryFn: () => schoolsApi.schoolsList().then((res) => res.data), + }), + similar: (id: number) => ({ + queryKey: [`similar_resources-${id}`], + queryFn: () => + learningResourcesApi + .learningResourcesSimilarList({ id }) + .then((res) => res.data), + }), + vectorSimilar: (id: number) => ({ + queryKey: [`vector_similar_resources-${id}`], + queryFn: () => + learningResourcesApi + .learningResourcesVectorSimilarList({ id }) + .then((res) => res.data), + }), }) export default learningResources diff --git a/frontends/api/src/hooks/user/index.ts b/frontends/api/src/hooks/user/index.ts index d3955cb9bf..f10201cba0 100644 --- a/frontends/api/src/hooks/user/index.ts +++ b/frontends/api/src/hooks/user/index.ts @@ -28,5 +28,10 @@ const useUserMe = () => }, }) -export { useUserMe } +const useUserIsAuthenticated = () => { + const { data: user } = useUserMe() + return !!user?.is_authenticated +} + +export { useUserMe, useUserIsAuthenticated } export type { User } diff --git a/frontends/api/src/hooks/userLists/index.ts b/frontends/api/src/hooks/userLists/index.ts index 6aacd99fff..b241689234 100644 --- a/frontends/api/src/hooks/userLists/index.ts +++ b/frontends/api/src/hooks/userLists/index.ts @@ -14,8 +14,7 @@ import type { UserList, } from "../../generated/v1" import userLists from "./keyFactory" -import { invalidateResourceWithUserListQueries } from "../learningResources/invalidation" -import { invalidateUserListQueries } from "./invalidation" +import { useUserIsAuthenticated } from "api/hooks/user" const useUserListList = ( params: ListRequest = {}, @@ -63,9 +62,9 @@ const useUserListDestroy = () => { return useMutation({ mutationFn: (params: DestroyRequest) => userListsApi.userlistsDestroy(params), - onSettled: (_data, _err, vars) => { - invalidateUserListQueries(queryClient, vars.id) - invalidateResourceWithUserListQueries(queryClient, vars.id) + onSettled: () => { + queryClient.invalidateQueries(userLists.list._def) + queryClient.invalidateQueries(userLists.membershipList._def) }, }) } @@ -106,6 +105,28 @@ const useUserListListItemMove = () => { }) } +const useIsUserListMember = (resourceId?: number) => { + return useQuery({ + ...userLists.membershipList(), + select: (data) => { + return !!data.find((relationship) => relationship.child === resourceId) + }, + enabled: useUserIsAuthenticated() && !!resourceId, + }) +} + +const useUserListMemberList = (resourceId?: number) => { + return useQuery({ + ...userLists.membershipList(), + select: (data) => { + return data + .filter((relationship) => relationship.child === resourceId) + .map((relationship) => relationship.parent.toString()) + }, + enabled: useUserIsAuthenticated() && !!resourceId, + }) +} + export { useUserListList, useUserListsDetail, @@ -114,4 +135,6 @@ export { useUserListDestroy, useInfiniteUserListItems, useUserListListItemMove, + useIsUserListMember, + useUserListMemberList, } diff --git a/frontends/api/src/hooks/userLists/invalidation.ts b/frontends/api/src/hooks/userLists/invalidation.ts deleted file mode 100644 index 3a05383f36..0000000000 --- a/frontends/api/src/hooks/userLists/invalidation.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { QueryClient, Query } from "@tanstack/react-query" -import userLists from "./keyFactory" -import { - UserList, - LearningResource, - PaginatedLearningResourceList, -} from "../../generated/v1" - -const invalidateUserListQueries = ( - queryClient: QueryClient, - userListId: UserList["id"], -) => { - queryClient.invalidateQueries(userLists.detail(userListId).queryKey) - const lists = [userLists.list._def] - - lists.forEach((queryKey) => { - queryClient.invalidateQueries({ - queryKey, - predicate: listHasResource(userListId), - }) - }) -} - -const listHasResource = - (resourceId: number) => - (query: Query): boolean => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const data = query.state.data as any - const resources: LearningResource[] | UserList[] = data.pages - ? data.pages.flatMap( - (page: PaginatedLearningResourceList) => page.results, - ) - : data.results - - return resources.some((res) => res.id === resourceId) - } - -export { invalidateUserListQueries, listHasResource } diff --git a/frontends/api/src/hooks/userLists/keyFactory.ts b/frontends/api/src/hooks/userLists/keyFactory.ts index 90793e38a5..ee3d61268c 100644 --- a/frontends/api/src/hooks/userLists/keyFactory.ts +++ b/frontends/api/src/hooks/userLists/keyFactory.ts @@ -6,23 +6,33 @@ import type { PaginatedUserListRelationshipList, } from "../../generated/v1" import { userListsApi } from "../../clients" +import { clearListMemberships } from "../learningResources/keyFactory" const userLists = createQueryKeys("userLists", { detail: (id: number) => ({ queryKey: [id], - queryFn: () => - userListsApi.userlistsRetrieve({ id }).then((res) => res.data), + queryFn: async () => { + const { data } = await userListsApi.userlistsRetrieve({ id }) + return data + }, contextQueries: { infiniteItems: (itemsP: ItemsListRequest) => ({ queryKey: [itemsP], - queryFn: ({ pageParam }: { pageParam?: string } = {}) => { + queryFn: async ({ pageParam }: { pageParam?: string } = {}) => { const request = pageParam ? axiosInstance.request({ method: "get", url: pageParam, }) : userListsApi.userlistsItemsList(itemsP) - return request.then((res) => res.data) + const { data } = await request + return { + ...data, + results: data.results.map((relation) => ({ + ...relation, + resource: clearListMemberships(relation.resource), + })), + } }, }), }, @@ -31,6 +41,13 @@ const userLists = createQueryKeys("userLists", { queryKey: [params], queryFn: () => userListsApi.userlistsList(params).then((res) => res.data), }), + membershipList: () => ({ + queryKey: ["membershipList"], + queryFn: async () => { + const { data } = await userListsApi.userlistsMembershipList() + return data + }, + }), }) export default userLists diff --git a/frontends/api/src/test-utils/factories/learningResources.ts b/frontends/api/src/test-utils/factories/learningResources.ts index 21c78495ab..44cffc527b 100644 --- a/frontends/api/src/test-utils/factories/learningResources.ts +++ b/frontends/api/src/test-utils/factories/learningResources.ts @@ -373,7 +373,6 @@ const learningPath: LearningResourceFactory = ( id: uniqueEnforcerId.enforce(() => faker.number.int()), item_count: faker.number.int({ min: 1, max: 30 }), }, - learning_path_parents: [], }, overrides, ) diff --git a/frontends/api/src/test-utils/factories/userLists.ts b/frontends/api/src/test-utils/factories/userLists.ts index ea6c337c78..738ec5d73e 100644 --- a/frontends/api/src/test-utils/factories/userLists.ts +++ b/frontends/api/src/test-utils/factories/userLists.ts @@ -45,7 +45,7 @@ const userListRelationship: Factory = ( const micro = microUserListRelationship() const resource = learningResource({ id: micro.child, - user_list_parents: [micro], + user_list_parents: [], }) return { ...micro, diff --git a/frontends/api/src/test-utils/urls.ts b/frontends/api/src/test-utils/urls.ts index a26fcec260..ba9970f49f 100644 --- a/frontends/api/src/test-utils/urls.ts +++ b/frontends/api/src/test-utils/urls.ts @@ -135,6 +135,7 @@ const learningPaths = { `${API_BASE_URL}/api/v1/learningpaths/${parentId}/items/${id}/`, details: (params: Params) => `${API_BASE_URL}/api/v1/learningpaths/${params.id}/`, + membershipList: () => `${API_BASE_URL}/api/v1/learningpaths/membership/`, } const userLists = { @@ -152,6 +153,7 @@ const userLists = { `${API_BASE_URL}/api/v1/userlists/${parentId}/items/${id}/`, details: (params: Params) => `${API_BASE_URL}/api/v1/userlists/${params.id}/`, + membershipList: () => `${API_BASE_URL}/api/v1/userlists/membership/`, } const articles = { diff --git a/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx b/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx index c15513dcc0..d1fec7a9fd 100644 --- a/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx +++ b/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx @@ -84,6 +84,10 @@ const setMockApiResponses = ({ results: [], }) + setMockResponse.get(urls.userMe.get(), {}) + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) + if (channel.channel_type === ChannelTypeEnum.Topic) { const topicId = channel.topic_detail.topic if (topicId) { @@ -133,7 +137,7 @@ describe("ChannelSearch", () => { results: resources, }, }) - setMockResponse.get(urls.userMe.get(), {}) + renderWithProviders(, { url: `/c/${channel.channel_type}/${channel.name}`, }) @@ -183,7 +187,6 @@ describe("ChannelSearch", () => { }, }, }) - setMockResponse.get(urls.userMe.get(), {}) renderWithProviders(, { url: `/c/${channel.channel_type}/${channel.name}/${url}`, @@ -249,8 +252,6 @@ describe("ChannelSearch", () => { }, }) - setMockResponse.get(urls.userMe.get(), {}) - renderWithProviders(, { url: `/c/${channel.channel_type}/${channel.name}/`, }) @@ -299,7 +300,6 @@ describe("ChannelSearch", () => { results: resources, }, }) - setMockResponse.get(urls.userMe.get(), {}) const initialSearch = "?q=meow&page=2" diff --git a/frontends/main/src/app-pages/DashboardPage/Dashboard.test.tsx b/frontends/main/src/app-pages/DashboardPage/Dashboard.test.tsx index a9423e024e..544cc5e0ca 100644 --- a/frontends/main/src/app-pages/DashboardPage/Dashboard.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/Dashboard.test.tsx @@ -183,6 +183,8 @@ describe("DashboardPage", () => { ), makeSearchResponse(courses), ) + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) return { profile, topPicks, diff --git a/frontends/main/src/app-pages/HomePage/HomePage.test.tsx b/frontends/main/src/app-pages/HomePage/HomePage.test.tsx index 79d112e8ed..31227c761d 100644 --- a/frontends/main/src/app-pages/HomePage/HomePage.test.tsx +++ b/frontends/main/src/app-pages/HomePage/HomePage.test.tsx @@ -37,6 +37,8 @@ const assertLinksTo = ( const setupAPIs = () => { setMockResponse.get(urls.userMe.get(), {}) + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) const resources = learningResources.resources({ count: 4 }) const attestations = testimonials.testimonials({ count: 3 }) @@ -125,7 +127,7 @@ describe("Home Page Browse by Topic", () => { const response = learningResources.topics({ count: 3 }) setMockResponse.get(urls.topics.list({ is_toplevel: true }), response) - setMockResponse.get(urls.userMe.get(), {}) + renderWithProviders() await waitFor(() => { @@ -254,7 +256,6 @@ describe("Home Page News and Events", () => { describe("Home Page personalize section", () => { test("Links to dashboard when authenticated", async () => { - setMockResponse.get(urls.userMe.get(), {}) setupAPIs() renderWithProviders() diff --git a/frontends/main/src/app-pages/LearningPathDetailsPage/ListDetailsPage.tsx b/frontends/main/src/app-pages/LearningPathDetailsPage/ListDetailsPage.tsx index 9635b3055c..43fa2179ca 100644 --- a/frontends/main/src/app-pages/LearningPathDetailsPage/ListDetailsPage.tsx +++ b/frontends/main/src/app-pages/LearningPathDetailsPage/ListDetailsPage.tsx @@ -6,6 +6,7 @@ import type { ItemsListingComponentProps } from "@/page-components/ItemsListing/ const StyledContainer = styled(Container)({ paddingTop: "24px", + marginBottom: "80px", }) const ListDetailsPage: React.FC = ({ diff --git a/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx b/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx index d5c926cb09..f9b1a7585c 100644 --- a/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx +++ b/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx @@ -46,6 +46,8 @@ const setMockApiResponses = ({ urls.offerors.list(), offerors ?? factories.learningResources.offerors({ count: 5 }), ) + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) } const getLastApiSearchParams = () => { diff --git a/frontends/main/src/page-components/Dialogs/AddToListDialog.test.tsx b/frontends/main/src/page-components/Dialogs/AddToListDialog.test.tsx index c8515239ba..526de42a24 100644 --- a/frontends/main/src/page-components/Dialogs/AddToListDialog.test.tsx +++ b/frontends/main/src/page-components/Dialogs/AddToListDialog.test.tsx @@ -4,16 +4,16 @@ import { AddToLearningPathDialog, AddToUserListDialog } from "./AddToListDialog" import { setMockResponse, makeRequest, factories, urls } from "api/test-utils" -import { renderWithProviders, screen, user, within, act } from "@/test-utils" +import { + renderWithProviders, + screen, + user, + within, + act, + waitFor, +} from "@/test-utils" import { manageListDialogs } from "@/page-components/ManageListDialogs/ManageListDialogs" import { waitForElementToBeRemoved } from "@testing-library/react" -import { - LearningPathRelationship, - LearningPathResource, - LearningResource, - UserList, - UserListRelationship, -} from "api" import invariant from "tiny-invariant" import { ListType } from "api/constants" @@ -36,24 +36,12 @@ const setupLearningPaths = ({ inLists = [], dialogOpen = true, }: Partial = {}) => { - const resource = learningResourcesFactory.resource({ - learning_path_parents: [], - }) + const resource = learningResourcesFactory.resource() const paginatedLearningPaths = learningResourcesFactory.learningPaths({ count: 3, }) const learningPaths = paginatedLearningPaths.results - const learningPathParents = resource.learning_path_parents! - inLists.forEach((index) => { - const learningPath = learningPaths[index] - learningPathParents.push( - learningResourcesFactory.microLearningPathRelationship({ - parent: learningPath.id, - child: resource.id, - }), - ) - learningPath.learning_path.item_count += 1 - }) + setMockResponse.get( urls.learningResources.details({ id: resource.id }), resource, @@ -62,6 +50,17 @@ const setupLearningPaths = ({ urls.learningPaths.list({ limit: 100 }), paginatedLearningPaths, ) + + const learningPathParents = inLists.map((index) => ({ + id: 123, + parent: learningPaths[index].id, + child: resource.id, + })) + + setMockResponse.get(urls.userMe.get(), { is_authenticated: true }) + setMockResponse.get(urls.learningPaths.membershipList(), learningPathParents) + setMockResponse.get(urls.userLists.membershipList(), []) + const view = renderWithProviders(null) if (dialogOpen) { act(() => { @@ -80,19 +79,10 @@ const setupUserLists = ({ inLists = [], dialogOpen = true, }: Partial = {}) => { - const resource = learningResourcesFactory.resource({ user_list_parents: [] }) + const resource = learningResourcesFactory.resource() const paginatedUserLists = userListsFactory.userLists({ count: 3 }) const userLists = paginatedUserLists.results - const userListParents = resource.user_list_parents! - inLists.forEach((index) => { - const userList = userLists[index] - userListParents.push( - userListsFactory.microUserListRelationship({ - parent: userList.id, - child: resource.id, - }), - ) - }) + setMockResponse.get( urls.learningResources.details({ id: resource.id }), resource, @@ -104,6 +94,15 @@ const setupUserLists = ({ NiceModal.show(AddToUserListDialog, { resourceId: resource.id }) }) } + + const userListParents = inLists.map((index) => ({ + parent: userLists[index].id, + child: resource.id, + })) + + setMockResponse.get(urls.userMe.get(), { is_authenticated: true }) + setMockResponse.get(urls.learningPaths.membershipList(), []) + setMockResponse.get(urls.userLists.membershipList(), userListParents) return { view, resource, @@ -112,46 +111,10 @@ const setupUserLists = ({ } } -const addToLearningPath = ( - resource: LearningResource, - list: LearningPathResource, -): LearningPathRelationship => { - const member = learningResourcesFactory.microLearningPathRelationship({ - parent: list.id, - child: resource.id, - }) - const modified = { - ...resource, - learning_path_parents: [...(resource.learning_path_parents ?? []), member], - } - return learningResourcesFactory.learningPathRelationship({ - ...member, - resource: modified, - }) -} - -const addToUserList = ( - resource: LearningResource, - list: UserList, -): UserListRelationship => { - const member = userListsFactory.microUserListRelationship({ - parent: list.id, - child: resource.id, - }) - const modified = { - ...resource, - user_list_parents: [...(resource.user_list_parents ?? []), member], - } - return userListsFactory.userListRelationship({ - ...member, - resource: modified, - }) -} - describe.each([ListType.LearningPath, ListType.UserList])( "AddToListDialog", (listType: string) => { - test("List is checked if resource is in list", async () => { + test(`${listType} List is checked if resource is in list`, async () => { const index = faker.number.int({ min: 0, max: 2 }) if (listType === ListType.LearningPath) { setupLearningPaths({ inLists: [index] }) @@ -161,9 +124,16 @@ describe.each([ListType.LearningPath, ListType.UserList])( const checkboxes = await screen.findAllByRole("checkbox") - expect(checkboxes[0].checked).toBe(index === 0) - expect(checkboxes[1].checked).toBe(index === 1) - expect(checkboxes[2].checked).toBe(index === 2) + + await waitFor(() => { + expect(checkboxes[0].checked).toBe(index === 0) + }) + await waitFor(() => { + expect(checkboxes[1].checked).toBe(index === 1) + }) + await waitFor(() => { + expect(checkboxes[2].checked).toBe(index === 2) + }) }) test("Clicking an unchecked list and clicking save adds item to that list", async () => { @@ -178,8 +148,7 @@ describe.each([ListType.LearningPath, ListType.UserList])( id: resource.id, learning_path_id: [list.id], }) - const newRelationship = addToLearningPath(resource, list) - setMockResponse.patch(setRelationshipsUrl, newRelationship) + setMockResponse.patch(setRelationshipsUrl) setMockResponse.get( urls.learningResources.details({ id: resource.id }), resource, @@ -193,8 +162,7 @@ describe.each([ListType.LearningPath, ListType.UserList])( id: resource.id, userlist_id: [list.id], }) - const newRelationship = addToUserList(resource, list) - setMockResponse.patch(setRelationshipsUrl, newRelationship) + setMockResponse.patch(setRelationshipsUrl) setMockResponse.get( urls.learningResources.details({ id: resource.id }), resource, @@ -255,9 +223,11 @@ describe.each([ListType.LearningPath, ListType.UserList])( ) } - const checkbox = await screen.findByLabelText(title) + const checkbox = await screen.findByRole("checkbox", { + name: title, + checked: true, + }) - expect(checkbox.checked).toBe(true) await user.click(checkbox) const saveButton = await screen.findByRole("button", { name: "Save" }) diff --git a/frontends/main/src/page-components/Dialogs/AddToListDialog.tsx b/frontends/main/src/page-components/Dialogs/AddToListDialog.tsx index b47d112c3f..33c070dacc 100644 --- a/frontends/main/src/page-components/Dialogs/AddToListDialog.tsx +++ b/frontends/main/src/page-components/Dialogs/AddToListDialog.tsx @@ -21,8 +21,11 @@ import { useLearningResourcesDetail, useLearningResourceSetLearningPathRelationships, } from "api/hooks/learningResources" -import { useUserListList } from "api/hooks/userLists" -import { useLearningPathsList } from "api/hooks/learningPaths" +import { useUserListList, useUserListMemberList } from "api/hooks/userLists" +import { + useLearningPathsList, + useLearningPathMemberList, +} from "api/hooks/learningPaths" import { manageListDialogs } from "@/page-components/ManageListDialogs/ManageListDialogs" import { ListType } from "api/constants" import { useFormik } from "formik" @@ -58,6 +61,11 @@ const AddToListDialogInner: React.FC = ({ manageListDialogs.upsertUserList() } }, [listType]) + const { data: userListValues, isLoading: isUserListLoading } = + useUserListMemberList(resource?.id) + const { data: learningPathValues, isLoading: isLearningPathLoading } = + useLearningPathMemberList(resource?.id) + const { isLoading: isSavingUserListRelationships, mutateAsync: setUserListRelationships, @@ -66,9 +74,11 @@ const AddToListDialogInner: React.FC = ({ isLoading: isSavingLearningPathRelationships, mutateAsync: setLearningPathRelationships, } = useLearningResourceSetLearningPathRelationships() + const posthog = usePostHog() const isSaving = isSavingLearningPathRelationships || isSavingUserListRelationships + let dialogTitle = "Add to list" if (listType === ListType.LearningPath) { dialogTitle = "Add to Learning List" @@ -79,28 +89,14 @@ const AddToListDialogInner: React.FC = ({ value: list.id.toString(), label: list.title, })) - const learningPathValues = lists - .map((list) => - resource?.learning_path_parents?.some(({ parent }) => parent === list.id) - ? list.id.toString() - : null, - ) - .filter((value) => value !== null) - const userListValues = lists - .map((list) => - resource?.user_list_parents?.some(({ parent }) => parent === list.id) - ? list.id.toString() - : null, - ) - .filter((value) => value !== null) const formik = useFormik({ enableReinitialize: true, validateOnChange: false, validateOnBlur: false, initialValues: { - learning_paths: learningPathValues, - user_lists: userListValues, + learning_paths: learningPathValues ?? [], + user_lists: userListValues ?? [], }, onSubmit: async (values) => { if (resource) { @@ -170,6 +166,7 @@ const AddToListDialogInner: React.FC = ({ choices={listChoices} values={formik.values.learning_paths} onChange={formik.handleChange} + disabled={isLearningPathLoading} vertical /> ) : null} @@ -179,6 +176,7 @@ const AddToListDialogInner: React.FC = ({ choices={listChoices} values={formik.values.user_lists} onChange={formik.handleChange} + disabled={isUserListLoading} vertical /> ) : null} @@ -196,12 +194,12 @@ type AddToListDialogProps = { const AddToLearningPathDialogInner: React.FC = ({ resourceId, }) => { - const resourceQuery = useLearningResourcesDetail(resourceId) - const resource = resourceQuery.data + const { data: resource } = useLearningResourcesDetail(resourceId) const listsQuery = useLearningPathsList({ limit: LIST_LIMIT }) const isReady = !!(resource && listsQuery.isSuccess) const lists = listsQuery.data?.results ?? [] + return ( = ({ const AddToUserListDialogInner: React.FC = ({ resourceId, }) => { - const resourceQuery = useLearningResourcesDetail(resourceId) - const resource = resourceQuery.data + const { data: resource } = useLearningResourcesDetail(resourceId) const listsQuery = useUserListList({ limit: LIST_LIMIT }) const isReady = !!(resource && listsQuery.isSuccess) diff --git a/frontends/main/src/page-components/ItemsListing/ItemsListing.test.tsx b/frontends/main/src/page-components/ItemsListing/ItemsListing.test.tsx index 509c89a219..4d7cff7fdc 100644 --- a/frontends/main/src/page-components/ItemsListing/ItemsListing.test.tsx +++ b/frontends/main/src/page-components/ItemsListing/ItemsListing.test.tsx @@ -106,6 +106,8 @@ describe("ItemsListing", () => { "Shows empty message when there are no items", ({ listType, count, hasEmptyMessage }) => { setMockResponse.get(urls.userMe.get(), {}) + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) const emptyMessage = faker.lorem.sentence() const paginatedRelationships = getPaginatedRelationships( listType, @@ -167,6 +169,8 @@ describe("ItemsListing", () => { const items = paginatedRelationships.results as LearningResourceListItem[] const user = factories.user.user() setMockResponse.get(urls.userMe.get(), user) + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) renderWithProviders( ({ })) describe("LearningResourceDrawerV1", () => { - it.each([ - { descriptor: "is enabled", enablePostHog: true }, - { descriptor: "is not enabled", enablePostHog: false }, - ])( + it.each([{ descriptor: "is enabled", enablePostHog: true }])( "Renders drawer content when resource=id is in the URL and captures the view if PostHog $descriptor", async ({ enablePostHog }) => { setMockResponse.get(urls.userMe.get(), {}) + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) process.env.NEXT_PUBLIC_POSTHOG_API_KEY = enablePostHog ? "12345abcdef" // pragma: allowlist secret : "" @@ -55,7 +54,9 @@ describe("LearningResourceDrawerV1", () => { }) expect(LearningResourceExpandedV1).toHaveBeenCalled() await waitFor(() => { - expectProps(LearningResourceExpandedV1, { resource }) + expectProps(LearningResourceExpandedV1, { + resource, + }) }) await screen.findByRole("heading", { name: resource.title }) @@ -117,6 +118,8 @@ describe("LearningResourceDrawerV1", () => { } else { setMockResponse.get(urls.userMe.get(), null, { code: 403 }) } + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) renderWithProviders(, { url: `?resource=${resource.id}`, diff --git a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV1.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV1.tsx index cade51dba8..ad644206a2 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV1.tsx +++ b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV1.tsx @@ -19,6 +19,8 @@ import { } from "../Dialogs/AddToListDialog" import { SignupPopover } from "../SignupPopover/SignupPopover" import { usePostHog } from "posthog-js/react" +import { useIsLearningPathMember } from "api/hooks/learningPaths" +import { useIsUserListMember } from "api/hooks/userLists" const RESOURCE_DRAWER_PARAMS = [RESOURCE_DRAWER_QUERY_PARAM] as const @@ -66,6 +68,9 @@ const DrawerContent: React.FC<{ const resource = useLearningResourcesDetail(Number(resourceId)) const [signupEl, setSignupEl] = React.useState(null) const { data: user } = useUserMe() + const { data: inLearningPath } = useIsLearningPathMember(resourceId) + const { data: inUserList } = useIsUserListMember(resourceId) + const handleAddToLearningPathClick: LearningResourceCardProps["onAddToLearningPathClick"] = useMemo(() => { if (user?.is_learning_path_editor) { @@ -93,6 +98,8 @@ const DrawerContent: React.FC<{ imgConfig={imgConfigs.large} resource={resource.data} user={user} + inLearningPath={inLearningPath} + inUserList={inUserList} onAddToLearningPathClick={handleAddToLearningPathClick} onAddToUserListClick={handleAddToUserListClick} /> diff --git a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV2.test.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV2.test.tsx index bea4a05c63..f7ece2b60e 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV2.test.tsx +++ b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV2.test.tsx @@ -40,6 +40,8 @@ describe("LearningResourceDrawerV2", () => { "Renders drawer content when resource=id is in the URL and captures the view if PostHog $descriptor", async ({ enablePostHog }) => { setMockResponse.get(urls.userMe.get(), {}) + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) process.env.NEXT_PUBLIC_POSTHOG_API_KEY = enablePostHog ? "12345abcdef" // pragma: allowlist secret : "" @@ -132,6 +134,8 @@ describe("LearningResourceDrawerV2", () => { } else { setMockResponse.get(urls.userMe.get(), null, { code: 403 }) } + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) renderWithProviders(, { url: `?resource=${resource.id}`, diff --git a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV2.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV2.tsx index c76ee7b02c..599da276cf 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV2.tsx +++ b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawerV2.tsx @@ -20,6 +20,8 @@ import { import { SignupPopover } from "../SignupPopover/SignupPopover" import { usePostHog } from "posthog-js/react" import ResourceCarousel from "../ResourceCarousel/ResourceCarousel" +import { useIsLearningPathMember } from "api/hooks/learningPaths" +import { useIsUserListMember } from "api/hooks/userLists" const RESOURCE_DRAWER_PARAMS = [RESOURCE_DRAWER_QUERY_PARAM] as const @@ -68,6 +70,9 @@ const DrawerContent: React.FC<{ const resource = useLearningResourcesDetail(Number(resourceId)) const [signupEl, setSignupEl] = React.useState(null) const { data: user } = useUserMe() + const { data: inLearningPath } = useIsLearningPathMember(resourceId) + const { data: inUserList } = useIsUserListMember(resourceId) + const handleAddToLearningPathClick: LearningResourceCardProps["onAddToLearningPathClick"] = useMemo(() => { if (user?.is_learning_path_editor) { @@ -130,6 +135,8 @@ const DrawerContent: React.FC<{ resource={resource.data} carousels={[similarResourcesCarousel, vectorSimilarResourcesCarousel]} user={user} + inLearningPath={inLearningPath} + inUserList={inUserList} onAddToLearningPathClick={handleAddToLearningPathClick} onAddToUserListClick={handleAddToUserListClick} closeDrawer={closeDrawer} diff --git a/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx b/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx index d71998a06f..4b6a6593b7 100644 --- a/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx +++ b/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx @@ -1,10 +1,16 @@ import React from "react" import * as NiceModal from "@ebay/nice-modal-react" -import { renderWithProviders, user, screen, expectProps } from "@/test-utils" +import { + renderWithProviders, + user, + screen, + expectProps, + waitFor, +} from "@/test-utils" import type { User } from "api/hooks/user" import { ResourceCard } from "./ResourceCard" import { getReadableResourceType } from "ol-utilities" -import { ResourceTypeEnum } from "api" +import { ResourceTypeEnum, MicroUserListRelationship } from "api" import { AddToLearningPathDialog, AddToUserListDialog, @@ -53,11 +59,23 @@ describe.each([ type SetupOptions = { user?: Partial props?: Partial + userListMemberships?: MicroUserListRelationship[] + learningPathMemberships?: MicroUserListRelationship[] } - const setup = ({ user, props = {} }: SetupOptions = {}) => { + const setup = ({ + user, + props = {}, + userListMemberships = [], + learningPathMemberships = [], + }: SetupOptions = {}) => { const { resource = makeResource() } = props if (user?.is_authenticated) { setMockResponse.get(urls.userMe.get(), user) + setMockResponse.get(urls.userLists.membershipList(), userListMemberships) + setMockResponse.get( + urls.learningPaths.membershipList(), + learningPathMemberships, + ) } else { setMockResponse.get(urls.userMe.get(), {}, { code: 403 }) } @@ -102,41 +120,54 @@ describe.each([ test.each([ { - userlist: { count: 1, inList: true }, - learningpath: { count: 1, inList: true }, + userList: { count: 1, inList: true }, + learningPath: { count: 1, inList: true }, }, { - userlist: { count: 0, inList: false }, - learningpath: { count: 1, inList: true }, + userList: { count: 0, inList: false }, + learningPath: { count: 1, inList: true }, }, { - userlist: { count: 1, inList: true }, - learningpath: { count: 0, inList: false }, + userList: { count: 1, inList: true }, + learningPath: { count: 0, inList: false }, }, { - userlist: { count: 0, inList: false }, - learningpath: { count: 0, inList: false }, + userList: { count: 0, inList: false }, + learningPath: { count: 0, inList: false }, }, ])( "'Add to ...' buttons are filled based on membership in list", - ({ userlist, learningpath }) => { + async ({ userList, learningPath }) => { const resource = makeResource() - const { microLearningPathRelationship } = factories.learningResources - const { microUserListRelationship } = factories.userLists - resource.learning_path_parents = Array.from( - { length: learningpath.count }, - () => microLearningPathRelationship({ child: resource.id }), - ) - resource.user_list_parents = Array.from({ length: userlist.count }, () => - microUserListRelationship({ child: resource.id }), - ) - setup({ user: { is_authenticated: true }, props: { resource } }) + setup({ + user: { is_authenticated: true }, + props: { resource }, + userListMemberships: userList.inList + ? [ + { + id: 1, + parent: 123, + child: resource.id, + }, + ] + : [], + learningPathMemberships: learningPath.inList + ? [ + { + id: 2, + parent: 456, + child: resource.id, + }, + ] + : [], + }) - expectProps(BaseComponent, { - resource, - inLearningPath: learningpath.inList, - inUserList: userlist.inList, + await waitFor(() => { + expectProps(BaseComponent, { + inLearningPath: learningPath.inList, + inUserList: userList.inList, + }) }) }, ) diff --git a/frontends/main/src/page-components/ResourceCard/ResourceCard.tsx b/frontends/main/src/page-components/ResourceCard/ResourceCard.tsx index af16cb0ffe..a1be57284c 100644 --- a/frontends/main/src/page-components/ResourceCard/ResourceCard.tsx +++ b/frontends/main/src/page-components/ResourceCard/ResourceCard.tsx @@ -14,10 +14,14 @@ import { useResourceDrawerHref } from "../LearningResourceDrawer/LearningResourc import { useUserMe } from "api/hooks/user" import { LearningResource } from "api" import { SignupPopover } from "../SignupPopover/SignupPopover" +import { useIsUserListMember } from "api/hooks/userLists" +import { useIsLearningPathMember } from "api/hooks/learningPaths" const useResourceCard = (resource?: LearningResource | null) => { const getDrawerHref = useResourceDrawerHref() const { data: user } = useUserMe() + const { data: inUserList } = useIsUserListMember(resource?.id) + const { data: inLearningPath } = useIsLearningPathMember(resource?.id) const [anchorEl, setAnchorEl] = useState(null) @@ -50,9 +54,6 @@ const useResourceCard = (resource?: LearningResource | null) => { } }, [user]) - const inUserList = !!resource?.user_list_parents?.length - const inLearningPath = !!resource?.learning_path_parents?.length - return { getDrawerHref, anchorEl, diff --git a/frontends/main/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx b/frontends/main/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx index 6a40d54875..6eb05a14a9 100644 --- a/frontends/main/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx +++ b/frontends/main/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx @@ -33,6 +33,8 @@ describe("ResourceCarousel", () => { list: factories.learningResources.resources({ count }), } setMockResponse.get(urls.userMe.get(), {}) + setMockResponse.get(urls.userLists.membershipList(), []) + setMockResponse.get(urls.learningPaths.membershipList(), []) const searchResponse = new ControlledPromise() const listResponse = new ControlledPromise() diff --git a/frontends/ol-components/src/components/Checkbox/Checkbox.stories.tsx b/frontends/ol-components/src/components/Checkbox/Checkbox.stories.tsx index 214881b3bc..2546c925b6 100644 --- a/frontends/ol-components/src/components/Checkbox/Checkbox.stories.tsx +++ b/frontends/ol-components/src/components/Checkbox/Checkbox.stories.tsx @@ -35,3 +35,10 @@ export const WithLabel: Story = { label: "Checkbox", }, } + +export const Disabled: Story = { + args: { + label: "Disabled", + disabled: true, + }, +} diff --git a/frontends/ol-components/src/components/Checkbox/Checkbox.tsx b/frontends/ol-components/src/components/Checkbox/Checkbox.tsx index dd16d9f715..a65e7b3bae 100644 --- a/frontends/ol-components/src/components/Checkbox/Checkbox.tsx +++ b/frontends/ol-components/src/components/Checkbox/Checkbox.tsx @@ -22,17 +22,21 @@ const containerStyles = css` background-position: 3px 3px; flex-shrink: 0; cursor: pointer; + + &:disabled { + cursor: not-allowed; + } } - input[type="checkbox"]:hover { - ${hoverSprite} + input[type="checkbox"]:checked { + ${checkedSprite} + .checkbox-label { color: ${theme.custom.colors.darkGray2}; } } - input[type="checkbox"]:checked { - ${checkedSprite} + input[type="checkbox"]:hover:not(:disabled, :checked) { + ${hoverSprite} + .checkbox-label { color: ${theme.custom.colors.darkGray2}; } @@ -48,19 +52,24 @@ const Container = styled.div` cursor: pointer; } + input[type="checkbox"] + .checkbox-label { + color: ${theme.custom.colors.silverGrayDark}; + } + + input[type="checkbox"]:disabled + .checkbox-label, + label:has(input[type="checkbox"]:disabled) { + cursor: not-allowed; + } + && input[type="checkbox"] { margin: 0; margin-right: 4px; } - input[type="checkbox"] + .checkbox-label { - color: ${theme.custom.colors.silverGrayDark}; - } - ${containerStyles} - &:hover input[type="checkbox"]:not(:checked), - label:hover & input[type="checkbox"]:not(:checked) { + &:hover input[type="checkbox"]:not(:checked, :disabled), + label:hover & input[type="checkbox"]:not(:checked, :disabled) { ${hoverSprite} } ` @@ -72,6 +81,7 @@ export type CheckboxProps = { checked?: boolean onChange?: (event: React.ChangeEvent) => void className?: string + disabled?: boolean } const Checkbox = ({ @@ -81,6 +91,7 @@ const Checkbox = ({ checked, onChange, className, + disabled = false, }: CheckboxProps) => { return ( @@ -92,6 +103,7 @@ const Checkbox = ({ value={value} checked={checked} onChange={onChange} + disabled={disabled} /> {label} @@ -102,6 +114,7 @@ const Checkbox = ({ value={value} checked={checked} onChange={onChange} + disabled={disabled} /> )} diff --git a/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.stories.tsx b/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.stories.tsx index 50157d509c..5e2ae02967 100644 --- a/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.stories.tsx +++ b/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.stories.tsx @@ -7,16 +7,15 @@ import { import Typography from "@mui/material/Typography" const StateWrapper = (props: CheckboxChoiceFieldProps) => { - const [value, setValue] = useState(props.value) + const [values, setValues] = useState(props.value) const handleChange = () => { - setValue( + setValues( Array.from( document.querySelectorAll("input[name='checkbox-group']:checked"), ).map((el) => el.getAttribute("value") || ""), ) } - return ( <> { { label: "Choice 2", value: "2" }, { label: "Choice 3", value: "3" }, ]} - value={value} + values={values} onChange={handleChange} />

- Selected: {value?.join(", ")} + Selected: {values?.join(", ")} ) } @@ -58,3 +57,10 @@ export const WithLabel: Story = { label: "CheckboxChoiceField", }, } + +export const Disabled: Story = { + args: { + label: "CheckboxChoiceField disabled", + disabled: true, + }, +} diff --git a/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.tsx b/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.tsx index f9326d5c36..827f51070b 100644 --- a/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.tsx +++ b/frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.tsx @@ -13,6 +13,7 @@ export type CheckboxChoiceFieldProps = { onChange?: CheckboxProps["onChange"] className?: string vertical?: boolean + disabled?: boolean } const Container = styled.div(({ theme }) => ({ @@ -45,6 +46,7 @@ const CheckboxChoiceField: React.FC = ({ onChange, className, vertical = false, + disabled = false, }) => { const isChecked = (choice: CheckboxProps) => choice.value ? (values?.includes(choice.value) ?? false) : false @@ -55,6 +57,7 @@ const CheckboxChoiceField: React.FC = ({ component="fieldset" sx={{ width: "100%" }} className={className} + disabled={disabled} > {label && } <_Container> @@ -65,6 +68,7 @@ const CheckboxChoiceField: React.FC = ({ name={name} checked={isChecked(choice)} onChange={onChange} + disabled={disabled} {...choice} /> ) diff --git a/frontends/ol-components/src/components/LearningResourceExpanded/InfoSectionV1.tsx b/frontends/ol-components/src/components/LearningResourceExpanded/InfoSectionV1.tsx index 27c3d1f312..fa22c0adc2 100644 --- a/frontends/ol-components/src/components/LearningResourceExpanded/InfoSectionV1.tsx +++ b/frontends/ol-components/src/components/LearningResourceExpanded/InfoSectionV1.tsx @@ -237,26 +237,28 @@ const InfoItem = ({ label, Icon, value }: InfoItemProps) => { ) } +type InfoSectionV1Props = { + resource?: LearningResource + run?: LearningResourceRun + user?: User + inLearningPath?: boolean + inUserList?: boolean + onAddToLearningPathClick?: LearningResourceCardProps["onAddToLearningPathClick"] + onAddToUserListClick?: LearningResourceCardProps["onAddToUserListClick"] +} + const InfoSectionV1 = ({ resource, run, user, + inUserList, + inLearningPath, onAddToLearningPathClick, onAddToUserListClick, -}: { - resource?: LearningResource - run?: LearningResourceRun - user?: User - onAddToLearningPathClick?: LearningResourceCardProps["onAddToLearningPathClick"] - onAddToUserListClick?: LearningResourceCardProps["onAddToUserListClick"] -}) => { +}: InfoSectionV1Props) => { if (!resource) { return null } - - const inUserList = !!resource?.user_list_parents?.length - const inLearningPath = !!resource?.learning_path_parents?.length - const infoItems = INFO_ITEMS.map(({ label, Icon, selector }) => ({ label, Icon, diff --git a/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpandedV1.tsx b/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpandedV1.tsx index ed27f0ef29..2bcb9d0acc 100644 --- a/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpandedV1.tsx +++ b/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpandedV1.tsx @@ -148,6 +148,8 @@ type LearningResourceExpandedV1Props = { resource?: LearningResource user?: User imgConfig: ImageConfig + inLearningPath?: boolean + inUserList?: boolean onAddToLearningPathClick?: LearningResourceCardProps["onAddToLearningPathClick"] onAddToUserListClick?: LearningResourceCardProps["onAddToUserListClick"] } @@ -334,6 +336,8 @@ const LearningResourceExpandedV1: React.FC = ({ resource, user, imgConfig, + inLearningPath, + inUserList, onAddToLearningPathClick, onAddToUserListClick, }) => { @@ -436,6 +440,8 @@ const LearningResourceExpandedV1: React.FC = ({ resource={resource} run={selectedRun} user={user} + inLearningPath={inLearningPath} + inUserList={inUserList} onAddToLearningPathClick={onAddToLearningPathClick} onAddToUserListClick={onAddToUserListClick} /> diff --git a/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpandedV2.tsx b/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpandedV2.tsx index b57e499df6..e51548cb45 100644 --- a/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpandedV2.tsx +++ b/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpandedV2.tsx @@ -197,6 +197,8 @@ type LearningResourceExpandedV2Props = { user?: User imgConfig: ImageConfig carousels?: React.ReactNode[] + inLearningPath?: boolean + inUserList?: boolean onAddToLearningPathClick?: LearningResourceCardProps["onAddToLearningPathClick"] onAddToUserListClick?: LearningResourceCardProps["onAddToUserListClick"] closeDrawer?: () => void @@ -349,6 +351,8 @@ const CallToActionSection = ({ resource, hide, user, + inUserList, + inLearningPath, onAddToLearningPathClick, onAddToUserListClick, }: { @@ -356,6 +360,8 @@ const CallToActionSection = ({ resource?: LearningResource hide?: boolean user?: User + inUserList?: boolean + inLearningPath?: boolean onAddToLearningPathClick?: LearningResourceCardProps["onAddToLearningPathClick"] onAddToUserListClick?: LearningResourceCardProps["onAddToUserListClick"] }) => { @@ -371,8 +377,6 @@ const CallToActionSection = ({ ) } - const inUserList = !!resource?.user_list_parents?.length - const inLearningPath = !!resource?.learning_path_parents?.length const { platform } = resource! const offeredBy = resource?.offered_by const platformCode = @@ -463,6 +467,8 @@ const LearningResourceExpandedV2: React.FC = ({ imgConfig, user, carousels, + inUserList, + inLearningPath, onAddToLearningPathClick, onAddToUserListClick, closeDrawer, @@ -484,6 +490,8 @@ const LearningResourceExpandedV2: React.FC = ({ imgConfig={imgConfig} resource={resource} user={user} + inLearningPath={inLearningPath} + inUserList={inUserList} onAddToLearningPathClick={onAddToLearningPathClick} onAddToUserListClick={onAddToUserListClick} />