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.
This commit is contained in:
Konstantin Schaper
2021-10-20 13:15:17 +02:00
committed by GitHub
parent 6a881b3d98
commit d41b293109
6 changed files with 83 additions and 73 deletions

View File

@@ -0,0 +1,2 @@
- type: fixed
descripion: remove query keys when deleting individual entities ([#1832](https://github.com/scm-manager/scm-manager/pull/1832))

View File

@@ -48,11 +48,11 @@ export const useGroups = (request?: UseGroupsRequest): ApiResult<GroupCollection
return useQuery<GroupCollection, Error>( return useQuery<GroupCollection, Error>(
["groups", request?.search || "", request?.page || 0], ["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) => { onSuccess: (groups: GroupCollection) => {
groups._embedded.groups.forEach((group: Group) => queryClient.setQueryData(["group", group.name], group)); groups._embedded.groups.forEach((group: Group) => queryClient.setQueryData(["group", group.name], group));
} },
} }
); );
}; };
@@ -60,7 +60,7 @@ export const useGroups = (request?: UseGroupsRequest): ApiResult<GroupCollection
export const useGroup = (name: string): ApiResult<Group> => { export const useGroup = (name: string): ApiResult<Group> => {
const indexLink = useRequiredIndexLink("groups"); const indexLink = useRequiredIndexLink("groups");
return useQuery<Group, Error>(["group", name], () => return useQuery<Group, Error>(["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 (group: GroupCreation) => {
return apiClient return apiClient
.post(link, group, "application/vnd.scmm-group+json;v=2") .post(link, group, "application/vnd.scmm-group+json;v=2")
.then(response => { .then((response) => {
const location = response.headers.get("Location"); const location = response.headers.get("Location");
if (!location) { if (!location) {
throw new Error("Server does not return required Location header"); throw new Error("Server does not return required Location header");
} }
return apiClient.get(location); return apiClient.get(location);
}) })
.then(response => response.json()); .then((response) => response.json());
}; };
}; };
@@ -83,23 +83,23 @@ export const useCreateGroup = () => {
const queryClient = useQueryClient(); const queryClient = useQueryClient();
const link = useRequiredIndexLink("groups"); const link = useRequiredIndexLink("groups");
const { mutate, data, isLoading, error } = useMutation<Group, Error, GroupCreation>(createGroup(link), { const { mutate, data, isLoading, error } = useMutation<Group, Error, GroupCreation>(createGroup(link), {
onSuccess: group => { onSuccess: (group) => {
queryClient.setQueryData(["group", group.name], group); queryClient.setQueryData(["group", group.name], group);
return queryClient.invalidateQueries(["groups"]); return queryClient.invalidateQueries(["groups"]);
} },
}); });
return { return {
create: (group: GroupCreation) => mutate(group), create: (group: GroupCreation) => mutate(group),
isLoading, isLoading,
error, error,
group: data group: data,
}; };
}; };
export const useUpdateGroup = () => { export const useUpdateGroup = () => {
const queryClient = useQueryClient(); const queryClient = useQueryClient();
const { mutate, isLoading, error, data } = useMutation<unknown, Error, Group>( const { mutate, isLoading, error, data } = useMutation<unknown, Error, Group>(
group => { (group) => {
const updateUrl = (group._links.update as Link).href; const updateUrl = (group._links.update as Link).href;
return apiClient.put(updateUrl, group, "application/vnd.scmm-group+json;v=2"); return apiClient.put(updateUrl, group, "application/vnd.scmm-group+json;v=2");
}, },
@@ -107,35 +107,35 @@ export const useUpdateGroup = () => {
onSuccess: async (_, group) => { onSuccess: async (_, group) => {
await queryClient.invalidateQueries(["group", group.name]); await queryClient.invalidateQueries(["group", group.name]);
await queryClient.invalidateQueries(["groups"]); await queryClient.invalidateQueries(["groups"]);
} },
} }
); );
return { return {
update: (group: Group) => mutate(group), update: (group: Group) => mutate(group),
isLoading, isLoading,
error, error,
isUpdated: !!data isUpdated: !!data,
}; };
}; };
export const useDeleteGroup = () => { export const useDeleteGroup = () => {
const queryClient = useQueryClient(); const queryClient = useQueryClient();
const { mutate, isLoading, error, data } = useMutation<unknown, Error, Group>( const { mutate, isLoading, error, data } = useMutation<unknown, Error, Group>(
group => { (group) => {
const deleteUrl = (group._links.delete as Link).href; const deleteUrl = (group._links.delete as Link).href;
return apiClient.delete(deleteUrl); return apiClient.delete(deleteUrl);
}, },
{ {
onSuccess: async (_, name) => { onSuccess: async (_, name) => {
await queryClient.invalidateQueries(["group", name]); await queryClient.removeQueries(["group", name]);
await queryClient.invalidateQueries(["groups"]); await queryClient.invalidateQueries(["groups"]);
} },
} }
); );
return { return {
remove: (group: Group) => mutate(group), remove: (group: Group) => mutate(group),
isLoading, isLoading,
error, error,
isDeleted: !!data isDeleted: !!data,
}; };
}; };

View File

@@ -37,7 +37,7 @@ import {
useRepository, useRepository,
useRepositoryTypes, useRepositoryTypes,
useUnarchiveRepository, useUnarchiveRepository,
useUpdateRepository useUpdateRepository,
} from "./repositories"; } from "./repositories";
import { Repository } from "@scm-manager/ui-types"; import { Repository } from "@scm-manager/ui-types";
import { QueryClient } from "react-query"; import { QueryClient } from "react-query";
@@ -50,25 +50,25 @@ describe("Test repository hooks", () => {
type: "git", type: "git",
_links: { _links: {
delete: { delete: {
href: "/r/spaceships/heartOfGold" href: "/r/spaceships/heartOfGold",
}, },
update: { update: {
href: "/r/spaceships/heartOfGold" href: "/r/spaceships/heartOfGold",
}, },
archive: { archive: {
href: "/r/spaceships/heartOfGold/archive" href: "/r/spaceships/heartOfGold/archive",
}, },
unarchive: { unarchive: {
href: "/r/spaceships/heartOfGold/unarchive" href: "/r/spaceships/heartOfGold/unarchive",
} },
} },
}; };
const repositoryCollection = { const repositoryCollection = {
_embedded: { _embedded: {
repositories: [heartOfGold] repositories: [heartOfGold],
}, },
_links: {} _links: {},
}; };
afterEach(() => { afterEach(() => {
@@ -78,7 +78,7 @@ describe("Test repository hooks", () => {
describe("useRepositories tests", () => { describe("useRepositories tests", () => {
const expectCollection = async (queryClient: QueryClient, request?: UseRepositoriesRequest) => { const expectCollection = async (queryClient: QueryClient, request?: UseRepositoriesRequest) => {
const { result, waitFor } = renderHook(() => useRepositories(request), { const { result, waitFor } = renderHook(() => useRepositories(request), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
await waitFor(() => { await waitFor(() => {
return !!result.current.data; return !!result.current.data;
@@ -91,8 +91,8 @@ describe("Test repository hooks", () => {
setIndexLink(queryClient, "repositories", "/repos"); setIndexLink(queryClient, "repositories", "/repos");
fetchMock.get("/api/v2/repos", repositoryCollection, { fetchMock.get("/api/v2/repos", repositoryCollection, {
query: { query: {
sortBy: "namespaceAndName" sortBy: "namespaceAndName",
} },
}); });
await expectCollection(queryClient); await expectCollection(queryClient);
@@ -104,12 +104,12 @@ describe("Test repository hooks", () => {
fetchMock.get("/api/v2/repos", repositoryCollection, { fetchMock.get("/api/v2/repos", repositoryCollection, {
query: { query: {
sortBy: "namespaceAndName", sortBy: "namespaceAndName",
page: "42" page: "42",
} },
}); });
await expectCollection(queryClient, { await expectCollection(queryClient, {
page: 42 page: 42,
}); });
}); });
@@ -118,8 +118,8 @@ describe("Test repository hooks", () => {
setIndexLink(queryClient, "repositories", "/repos"); setIndexLink(queryClient, "repositories", "/repos");
fetchMock.get("/api/v2/spaceships", repositoryCollection, { fetchMock.get("/api/v2/spaceships", repositoryCollection, {
query: { query: {
sortBy: "namespaceAndName" sortBy: "namespaceAndName",
} },
}); });
await expectCollection(queryClient, { await expectCollection(queryClient, {
@@ -127,10 +127,10 @@ describe("Test repository hooks", () => {
namespace: "spaceships", namespace: "spaceships",
_links: { _links: {
repositories: { repositories: {
href: "/spaceships" href: "/spaceships",
} },
} },
} },
}); });
}); });
@@ -140,12 +140,12 @@ describe("Test repository hooks", () => {
fetchMock.get("/api/v2/repos", repositoryCollection, { fetchMock.get("/api/v2/repos", repositoryCollection, {
query: { query: {
sortBy: "namespaceAndName", sortBy: "namespaceAndName",
q: "heart" q: "heart",
} },
}); });
await expectCollection(queryClient, { await expectCollection(queryClient, {
search: "heart" search: "heart",
}); });
}); });
@@ -154,8 +154,8 @@ describe("Test repository hooks", () => {
setIndexLink(queryClient, "repositories", "/repos"); setIndexLink(queryClient, "repositories", "/repos");
fetchMock.get("/api/v2/repos", repositoryCollection, { fetchMock.get("/api/v2/repos", repositoryCollection, {
query: { query: {
sortBy: "namespaceAndName" sortBy: "namespaceAndName",
} },
}); });
await expectCollection(queryClient); await expectCollection(queryClient);
@@ -168,7 +168,7 @@ describe("Test repository hooks", () => {
const queryClient = createInfiniteCachingClient(); const queryClient = createInfiniteCachingClient();
setIndexLink(queryClient, "repositories", "/repos"); setIndexLink(queryClient, "repositories", "/repos");
const { result } = renderHook(() => useRepositories({ disabled: true }), { const { result } = renderHook(() => useRepositories({ disabled: true }), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
expect(result.current.isLoading).toBe(false); expect(result.current.isLoading).toBe(false);
@@ -185,19 +185,19 @@ describe("Test repository hooks", () => {
fetchMock.postOnce("/api/v2/r", { fetchMock.postOnce("/api/v2/r", {
status: 201, status: 201,
headers: { headers: {
Location: "/r/spaceships/heartOfGold" Location: "/r/spaceships/heartOfGold",
} },
}); });
fetchMock.getOnce("/api/v2/r/spaceships/heartOfGold", heartOfGold); fetchMock.getOnce("/api/v2/r/spaceships/heartOfGold", heartOfGold);
const { result, waitForNextUpdate } = renderHook(() => useCreateRepository(), { const { result, waitForNextUpdate } = renderHook(() => useCreateRepository(), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
const repository = { const repository = {
...heartOfGold, ...heartOfGold,
contextEntries: [] contextEntries: [],
}; };
await act(() => { await act(() => {
@@ -216,19 +216,19 @@ describe("Test repository hooks", () => {
fetchMock.postOnce("/api/v2/r?initialize=true", { fetchMock.postOnce("/api/v2/r?initialize=true", {
status: 201, status: 201,
headers: { headers: {
Location: "/r/spaceships/heartOfGold" Location: "/r/spaceships/heartOfGold",
} },
}); });
fetchMock.getOnce("/api/v2/r/spaceships/heartOfGold", heartOfGold); fetchMock.getOnce("/api/v2/r/spaceships/heartOfGold", heartOfGold);
const { result, waitForNextUpdate } = renderHook(() => useCreateRepository(), { const { result, waitForNextUpdate } = renderHook(() => useCreateRepository(), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
const repository = { const repository = {
...heartOfGold, ...heartOfGold,
contextEntries: [] contextEntries: [],
}; };
await act(() => { await act(() => {
@@ -245,16 +245,16 @@ describe("Test repository hooks", () => {
setIndexLink(queryClient, "repositories", "/r"); setIndexLink(queryClient, "repositories", "/r");
fetchMock.postOnce("/api/v2/r", { fetchMock.postOnce("/api/v2/r", {
status: 201 status: 201,
}); });
const { result, waitForNextUpdate } = renderHook(() => useCreateRepository(), { const { result, waitForNextUpdate } = renderHook(() => useCreateRepository(), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
const repository = { const repository = {
...heartOfGold, ...heartOfGold,
contextEntries: [] contextEntries: [],
}; };
await act(() => { await act(() => {
@@ -274,7 +274,7 @@ describe("Test repository hooks", () => {
fetchMock.get("/api/v2/r/spaceships/heartOfGold", heartOfGold); fetchMock.get("/api/v2/r/spaceships/heartOfGold", heartOfGold);
const { result, waitFor } = renderHook(() => useRepository("spaceships", "heartOfGold"), { const { result, waitFor } = renderHook(() => useRepository("spaceships", "heartOfGold"), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
await waitFor(() => { await waitFor(() => {
return !!result.current.data; return !!result.current.data;
@@ -293,15 +293,15 @@ describe("Test repository hooks", () => {
{ {
name: "git", name: "git",
displayName: "Git", displayName: "Git",
_links: {} _links: {},
} },
] ],
}, },
_links: {} _links: {},
}); });
const { result, waitFor } = renderHook(() => useRepositoryTypes(), { const { result, waitFor } = renderHook(() => useRepositoryTypes(), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
await waitFor(() => { await waitFor(() => {
return !!result.current.data; return !!result.current.data;
@@ -322,11 +322,11 @@ describe("Test repository hooks", () => {
const deleteRepository = async (options?: UseDeleteRepositoryOptions) => { const deleteRepository = async (options?: UseDeleteRepositoryOptions) => {
fetchMock.deleteOnce("/api/v2/r/spaceships/heartOfGold", { fetchMock.deleteOnce("/api/v2/r/spaceships/heartOfGold", {
status: 204 status: 204,
}); });
const { result, waitForNextUpdate } = renderHook(() => useDeleteRepository(options), { const { result, waitForNextUpdate } = renderHook(() => useDeleteRepository(options), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
await act(() => { await act(() => {
@@ -338,6 +338,14 @@ describe("Test repository hooks", () => {
return result.current; 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) => { const shouldInvalidateQuery = async (queryKey: string[], data: unknown) => {
queryClient.setQueryData(queryKey, data); queryClient.setQueryData(queryKey, data);
await deleteRepository(); await deleteRepository();
@@ -353,7 +361,7 @@ describe("Test repository hooks", () => {
}); });
it("should invalidate repository cache", async () => { 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 () => { it("should invalidate repository collection cache", async () => {
@@ -363,9 +371,9 @@ describe("Test repository hooks", () => {
it("should call onSuccess callback", async () => { it("should call onSuccess callback", async () => {
let repo; let repo;
await deleteRepository({ await deleteRepository({
onSuccess: repository => { onSuccess: (repository) => {
repo = repository; repo = repository;
} },
}); });
expect(repo).toEqual(heartOfGold); expect(repo).toEqual(heartOfGold);
}); });
@@ -380,11 +388,11 @@ describe("Test repository hooks", () => {
const updateRepository = async () => { const updateRepository = async () => {
fetchMock.putOnce("/api/v2/r/spaceships/heartOfGold", { fetchMock.putOnce("/api/v2/r/spaceships/heartOfGold", {
status: 204 status: 204,
}); });
const { result, waitForNextUpdate } = renderHook(() => useUpdateRepository(), { const { result, waitForNextUpdate } = renderHook(() => useUpdateRepository(), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
await act(() => { await act(() => {
@@ -428,11 +436,11 @@ describe("Test repository hooks", () => {
const archiveRepository = async () => { const archiveRepository = async () => {
fetchMock.postOnce("/api/v2/r/spaceships/heartOfGold/archive", { fetchMock.postOnce("/api/v2/r/spaceships/heartOfGold/archive", {
status: 204 status: 204,
}); });
const { result, waitForNextUpdate } = renderHook(() => useArchiveRepository(), { const { result, waitForNextUpdate } = renderHook(() => useArchiveRepository(), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
await act(() => { await act(() => {
@@ -476,11 +484,11 @@ describe("Test repository hooks", () => {
const unarchiveRepository = async () => { const unarchiveRepository = async () => {
fetchMock.postOnce("/api/v2/r/spaceships/heartOfGold/unarchive", { fetchMock.postOnce("/api/v2/r/spaceships/heartOfGold/unarchive", {
status: 204 status: 204,
}); });
const { result, waitForNextUpdate } = renderHook(() => useUnarchiveRepository(), { const { result, waitForNextUpdate } = renderHook(() => useUnarchiveRepository(), {
wrapper: createWrapper(undefined, queryClient) wrapper: createWrapper(undefined, queryClient),
}); });
await act(() => { await act(() => {

View File

@@ -153,7 +153,7 @@ export const useDeleteRepository = (options?: UseDeleteRepositoryOptions) => {
if (options?.onSuccess) { if (options?.onSuccess) {
options.onSuccess(repository); options.onSuccess(repository);
} }
await queryClient.invalidateQueries(repoQueryKey(repository)); await queryClient.removeQueries(repoQueryKey(repository));
await queryClient.invalidateQueries(["repositories"]); await queryClient.invalidateQueries(["repositories"]);
}, },
} }

View File

@@ -127,7 +127,7 @@ export const useDeleteRepositoryRole = () => {
}, },
{ {
onSuccess: async (_, name) => { onSuccess: async (_, name) => {
await queryClient.invalidateQueries(["repositoryRole", name]); await queryClient.removeQueries(["repositoryRole", name]);
await queryClient.invalidateQueries(["repositoryRoles"]); await queryClient.invalidateQueries(["repositoryRoles"]);
}, },
} }

View File

@@ -128,7 +128,7 @@ export const useDeleteUser = () => {
}, },
{ {
onSuccess: async (_, name) => { onSuccess: async (_, name) => {
await queryClient.invalidateQueries(["user", name]); await queryClient.removeQueries(["user", name]);
await queryClient.invalidateQueries(["users"]); await queryClient.invalidateQueries(["users"]);
}, },
} }