From d41b2931094a6f556110b08b462b21599a71aa2e Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Wed, 20 Oct 2021 13:15:17 +0200 Subject: [PATCH] remove query keys when deleting individual entities (#1832) When deleting individual entities, their query keys should be removed, not only invalidated. This is to prevent situations, where an entity is deleted via the web interface and react-query attempts a re-fetch before a redirect to the collection view can occur. This could lead to a not found error. --- gradle/changelog/remove_query_keys.yaml | 2 + scm-ui/ui-api/src/groups.ts | 30 +++--- scm-ui/ui-api/src/repositories.test.ts | 118 +++++++++++++----------- scm-ui/ui-api/src/repositories.ts | 2 +- scm-ui/ui-api/src/repository-roles.ts | 2 +- scm-ui/ui-api/src/users.ts | 2 +- 6 files changed, 83 insertions(+), 73 deletions(-) create mode 100644 gradle/changelog/remove_query_keys.yaml diff --git a/gradle/changelog/remove_query_keys.yaml b/gradle/changelog/remove_query_keys.yaml new file mode 100644 index 0000000000..cffbec54f5 --- /dev/null +++ b/gradle/changelog/remove_query_keys.yaml @@ -0,0 +1,2 @@ +- type: fixed + descripion: remove query keys when deleting individual entities ([#1832](https://github.com/scm-manager/scm-manager/pull/1832)) diff --git a/scm-ui/ui-api/src/groups.ts b/scm-ui/ui-api/src/groups.ts index 515b6e4a54..1b604ca6fd 100644 --- a/scm-ui/ui-api/src/groups.ts +++ b/scm-ui/ui-api/src/groups.ts @@ -48,11 +48,11 @@ export const useGroups = (request?: UseGroupsRequest): ApiResult( ["groups", request?.search || "", request?.page || 0], - () => apiClient.get(`${indexLink}?${createQueryString(queryParams)}`).then(response => response.json()), + () => apiClient.get(`${indexLink}?${createQueryString(queryParams)}`).then((response) => response.json()), { onSuccess: (groups: GroupCollection) => { groups._embedded.groups.forEach((group: Group) => queryClient.setQueryData(["group", group.name], group)); - } + }, } ); }; @@ -60,7 +60,7 @@ export const useGroups = (request?: UseGroupsRequest): ApiResult => { const indexLink = useRequiredIndexLink("groups"); return useQuery(["group", name], () => - apiClient.get(concat(indexLink, name)).then(response => response.json()) + apiClient.get(concat(indexLink, name)).then((response) => response.json()) ); }; @@ -68,14 +68,14 @@ const createGroup = (link: string) => { return (group: GroupCreation) => { return apiClient .post(link, group, "application/vnd.scmm-group+json;v=2") - .then(response => { + .then((response) => { const location = response.headers.get("Location"); if (!location) { throw new Error("Server does not return required Location header"); } return apiClient.get(location); }) - .then(response => response.json()); + .then((response) => response.json()); }; }; @@ -83,23 +83,23 @@ export const useCreateGroup = () => { const queryClient = useQueryClient(); const link = useRequiredIndexLink("groups"); const { mutate, data, isLoading, error } = useMutation(createGroup(link), { - onSuccess: group => { + onSuccess: (group) => { queryClient.setQueryData(["group", group.name], group); return queryClient.invalidateQueries(["groups"]); - } + }, }); return { create: (group: GroupCreation) => mutate(group), isLoading, error, - group: data + group: data, }; }; export const useUpdateGroup = () => { const queryClient = useQueryClient(); const { mutate, isLoading, error, data } = useMutation( - group => { + (group) => { const updateUrl = (group._links.update as Link).href; return apiClient.put(updateUrl, group, "application/vnd.scmm-group+json;v=2"); }, @@ -107,35 +107,35 @@ export const useUpdateGroup = () => { onSuccess: async (_, group) => { await queryClient.invalidateQueries(["group", group.name]); await queryClient.invalidateQueries(["groups"]); - } + }, } ); return { update: (group: Group) => mutate(group), isLoading, error, - isUpdated: !!data + isUpdated: !!data, }; }; export const useDeleteGroup = () => { const queryClient = useQueryClient(); const { mutate, isLoading, error, data } = useMutation( - group => { + (group) => { const deleteUrl = (group._links.delete as Link).href; return apiClient.delete(deleteUrl); }, { onSuccess: async (_, name) => { - await queryClient.invalidateQueries(["group", name]); + await queryClient.removeQueries(["group", name]); await queryClient.invalidateQueries(["groups"]); - } + }, } ); return { remove: (group: Group) => mutate(group), isLoading, error, - isDeleted: !!data + isDeleted: !!data, }; }; diff --git a/scm-ui/ui-api/src/repositories.test.ts b/scm-ui/ui-api/src/repositories.test.ts index e9aebe43c8..98ab014a33 100644 --- a/scm-ui/ui-api/src/repositories.test.ts +++ b/scm-ui/ui-api/src/repositories.test.ts @@ -37,7 +37,7 @@ import { useRepository, useRepositoryTypes, useUnarchiveRepository, - useUpdateRepository + useUpdateRepository, } from "./repositories"; import { Repository } from "@scm-manager/ui-types"; import { QueryClient } from "react-query"; @@ -50,25 +50,25 @@ describe("Test repository hooks", () => { type: "git", _links: { delete: { - href: "/r/spaceships/heartOfGold" + href: "/r/spaceships/heartOfGold", }, update: { - href: "/r/spaceships/heartOfGold" + href: "/r/spaceships/heartOfGold", }, archive: { - href: "/r/spaceships/heartOfGold/archive" + href: "/r/spaceships/heartOfGold/archive", }, unarchive: { - href: "/r/spaceships/heartOfGold/unarchive" - } - } + href: "/r/spaceships/heartOfGold/unarchive", + }, + }, }; const repositoryCollection = { _embedded: { - repositories: [heartOfGold] + repositories: [heartOfGold], }, - _links: {} + _links: {}, }; afterEach(() => { @@ -78,7 +78,7 @@ describe("Test repository hooks", () => { describe("useRepositories tests", () => { const expectCollection = async (queryClient: QueryClient, request?: UseRepositoriesRequest) => { const { result, waitFor } = renderHook(() => useRepositories(request), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); await waitFor(() => { return !!result.current.data; @@ -91,8 +91,8 @@ describe("Test repository hooks", () => { setIndexLink(queryClient, "repositories", "/repos"); fetchMock.get("/api/v2/repos", repositoryCollection, { query: { - sortBy: "namespaceAndName" - } + sortBy: "namespaceAndName", + }, }); await expectCollection(queryClient); @@ -104,12 +104,12 @@ describe("Test repository hooks", () => { fetchMock.get("/api/v2/repos", repositoryCollection, { query: { sortBy: "namespaceAndName", - page: "42" - } + page: "42", + }, }); await expectCollection(queryClient, { - page: 42 + page: 42, }); }); @@ -118,8 +118,8 @@ describe("Test repository hooks", () => { setIndexLink(queryClient, "repositories", "/repos"); fetchMock.get("/api/v2/spaceships", repositoryCollection, { query: { - sortBy: "namespaceAndName" - } + sortBy: "namespaceAndName", + }, }); await expectCollection(queryClient, { @@ -127,10 +127,10 @@ describe("Test repository hooks", () => { namespace: "spaceships", _links: { repositories: { - href: "/spaceships" - } - } - } + href: "/spaceships", + }, + }, + }, }); }); @@ -140,12 +140,12 @@ describe("Test repository hooks", () => { fetchMock.get("/api/v2/repos", repositoryCollection, { query: { sortBy: "namespaceAndName", - q: "heart" - } + q: "heart", + }, }); await expectCollection(queryClient, { - search: "heart" + search: "heart", }); }); @@ -154,8 +154,8 @@ describe("Test repository hooks", () => { setIndexLink(queryClient, "repositories", "/repos"); fetchMock.get("/api/v2/repos", repositoryCollection, { query: { - sortBy: "namespaceAndName" - } + sortBy: "namespaceAndName", + }, }); await expectCollection(queryClient); @@ -168,7 +168,7 @@ describe("Test repository hooks", () => { const queryClient = createInfiniteCachingClient(); setIndexLink(queryClient, "repositories", "/repos"); const { result } = renderHook(() => useRepositories({ disabled: true }), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); expect(result.current.isLoading).toBe(false); @@ -185,19 +185,19 @@ describe("Test repository hooks", () => { fetchMock.postOnce("/api/v2/r", { status: 201, headers: { - Location: "/r/spaceships/heartOfGold" - } + Location: "/r/spaceships/heartOfGold", + }, }); fetchMock.getOnce("/api/v2/r/spaceships/heartOfGold", heartOfGold); const { result, waitForNextUpdate } = renderHook(() => useCreateRepository(), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); const repository = { ...heartOfGold, - contextEntries: [] + contextEntries: [], }; await act(() => { @@ -216,19 +216,19 @@ describe("Test repository hooks", () => { fetchMock.postOnce("/api/v2/r?initialize=true", { status: 201, headers: { - Location: "/r/spaceships/heartOfGold" - } + Location: "/r/spaceships/heartOfGold", + }, }); fetchMock.getOnce("/api/v2/r/spaceships/heartOfGold", heartOfGold); const { result, waitForNextUpdate } = renderHook(() => useCreateRepository(), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); const repository = { ...heartOfGold, - contextEntries: [] + contextEntries: [], }; await act(() => { @@ -245,16 +245,16 @@ describe("Test repository hooks", () => { setIndexLink(queryClient, "repositories", "/r"); fetchMock.postOnce("/api/v2/r", { - status: 201 + status: 201, }); const { result, waitForNextUpdate } = renderHook(() => useCreateRepository(), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); const repository = { ...heartOfGold, - contextEntries: [] + contextEntries: [], }; await act(() => { @@ -274,7 +274,7 @@ describe("Test repository hooks", () => { fetchMock.get("/api/v2/r/spaceships/heartOfGold", heartOfGold); const { result, waitFor } = renderHook(() => useRepository("spaceships", "heartOfGold"), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); await waitFor(() => { return !!result.current.data; @@ -293,15 +293,15 @@ describe("Test repository hooks", () => { { name: "git", displayName: "Git", - _links: {} - } - ] + _links: {}, + }, + ], }, - _links: {} + _links: {}, }); const { result, waitFor } = renderHook(() => useRepositoryTypes(), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); await waitFor(() => { return !!result.current.data; @@ -322,11 +322,11 @@ describe("Test repository hooks", () => { const deleteRepository = async (options?: UseDeleteRepositoryOptions) => { fetchMock.deleteOnce("/api/v2/r/spaceships/heartOfGold", { - status: 204 + status: 204, }); const { result, waitForNextUpdate } = renderHook(() => useDeleteRepository(options), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); await act(() => { @@ -338,6 +338,14 @@ describe("Test repository hooks", () => { return result.current; }; + const shouldRemoveQuery = async (queryKey: string[], data: unknown) => { + queryClient.setQueryData(queryKey, data); + await deleteRepository(); + + const queryState = queryClient.getQueryState(queryKey); + expect(queryState).toBeUndefined(); + }; + const shouldInvalidateQuery = async (queryKey: string[], data: unknown) => { queryClient.setQueryData(queryKey, data); await deleteRepository(); @@ -353,7 +361,7 @@ describe("Test repository hooks", () => { }); it("should invalidate repository cache", async () => { - await shouldInvalidateQuery(["repository", "spaceships", "heartOfGold"], heartOfGold); + await shouldRemoveQuery(["repository", "spaceships", "heartOfGold"], heartOfGold); }); it("should invalidate repository collection cache", async () => { @@ -363,9 +371,9 @@ describe("Test repository hooks", () => { it("should call onSuccess callback", async () => { let repo; await deleteRepository({ - onSuccess: repository => { + onSuccess: (repository) => { repo = repository; - } + }, }); expect(repo).toEqual(heartOfGold); }); @@ -380,11 +388,11 @@ describe("Test repository hooks", () => { const updateRepository = async () => { fetchMock.putOnce("/api/v2/r/spaceships/heartOfGold", { - status: 204 + status: 204, }); const { result, waitForNextUpdate } = renderHook(() => useUpdateRepository(), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); await act(() => { @@ -428,11 +436,11 @@ describe("Test repository hooks", () => { const archiveRepository = async () => { fetchMock.postOnce("/api/v2/r/spaceships/heartOfGold/archive", { - status: 204 + status: 204, }); const { result, waitForNextUpdate } = renderHook(() => useArchiveRepository(), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); await act(() => { @@ -476,11 +484,11 @@ describe("Test repository hooks", () => { const unarchiveRepository = async () => { fetchMock.postOnce("/api/v2/r/spaceships/heartOfGold/unarchive", { - status: 204 + status: 204, }); const { result, waitForNextUpdate } = renderHook(() => useUnarchiveRepository(), { - wrapper: createWrapper(undefined, queryClient) + wrapper: createWrapper(undefined, queryClient), }); await act(() => { diff --git a/scm-ui/ui-api/src/repositories.ts b/scm-ui/ui-api/src/repositories.ts index 31ad3fa6ee..c28fb74e98 100644 --- a/scm-ui/ui-api/src/repositories.ts +++ b/scm-ui/ui-api/src/repositories.ts @@ -153,7 +153,7 @@ export const useDeleteRepository = (options?: UseDeleteRepositoryOptions) => { if (options?.onSuccess) { options.onSuccess(repository); } - await queryClient.invalidateQueries(repoQueryKey(repository)); + await queryClient.removeQueries(repoQueryKey(repository)); await queryClient.invalidateQueries(["repositories"]); }, } diff --git a/scm-ui/ui-api/src/repository-roles.ts b/scm-ui/ui-api/src/repository-roles.ts index 85bb4a2c6a..a3940a6cd3 100644 --- a/scm-ui/ui-api/src/repository-roles.ts +++ b/scm-ui/ui-api/src/repository-roles.ts @@ -127,7 +127,7 @@ export const useDeleteRepositoryRole = () => { }, { onSuccess: async (_, name) => { - await queryClient.invalidateQueries(["repositoryRole", name]); + await queryClient.removeQueries(["repositoryRole", name]); await queryClient.invalidateQueries(["repositoryRoles"]); }, } diff --git a/scm-ui/ui-api/src/users.ts b/scm-ui/ui-api/src/users.ts index b69350371b..ca736ef080 100644 --- a/scm-ui/ui-api/src/users.ts +++ b/scm-ui/ui-api/src/users.ts @@ -128,7 +128,7 @@ export const useDeleteUser = () => { }, { onSuccess: async (_, name) => { - await queryClient.invalidateQueries(["user", name]); + await queryClient.removeQueries(["user", name]); await queryClient.invalidateQueries(["users"]); }, }