From faf3b9bc69a2722166a1ef1b8c06b017c60c1192 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Wed, 26 Jun 2024 22:56:48 +0000 Subject: [PATCH 1/6] feat: route groups by name instead of id --- site/e2e/tests/groups/addMembers.spec.ts | 2 +- site/e2e/tests/groups/removeGroup.spec.ts | 2 +- site/e2e/tests/groups/removeMember.spec.ts | 2 +- site/src/api/api.ts | 6 ++-- site/src/api/queries/groups.ts | 8 ++--- site/src/pages/GroupsPage/CreateGroupPage.tsx | 2 +- site/src/pages/GroupsPage/GroupPage.tsx | 12 ++++++-- site/src/pages/GroupsPage/GroupsPageView.tsx | 2 +- .../pages/GroupsPage/SettingsGroupPage.tsx | 29 +++++++++++++++---- site/src/router.tsx | 4 +-- 10 files changed, 47 insertions(+), 22 deletions(-) diff --git a/site/e2e/tests/groups/addMembers.spec.ts b/site/e2e/tests/groups/addMembers.spec.ts index f9532733d86dd..5967dc80bfb60 100644 --- a/site/e2e/tests/groups/addMembers.spec.ts +++ b/site/e2e/tests/groups/addMembers.spec.ts @@ -20,7 +20,7 @@ test("add members", async ({ page, baseURL }) => { Array.from({ length: numberOfMembers }, () => createUser(orgId)), ); - await page.goto(`${baseURL}/groups/${group.id}`, { + await page.goto(`${baseURL}/groups/${group.name}`, { waitUntil: "domcontentloaded", }); await expect(page).toHaveTitle(`${group.display_name} - Coder`); diff --git a/site/e2e/tests/groups/removeGroup.spec.ts b/site/e2e/tests/groups/removeGroup.spec.ts index 9011ecbb7147a..eeea0afa22eef 100644 --- a/site/e2e/tests/groups/removeGroup.spec.ts +++ b/site/e2e/tests/groups/removeGroup.spec.ts @@ -11,7 +11,7 @@ test("remove group", async ({ page, baseURL }) => { const orgId = await getCurrentOrgId(); const group = await createGroup(orgId); - await page.goto(`${baseURL}/groups/${group.id}`, { + await page.goto(`${baseURL}/groups/${group.name}`, { waitUntil: "domcontentloaded", }); await expect(page).toHaveTitle(`${group.display_name} - Coder`); diff --git a/site/e2e/tests/groups/removeMember.spec.ts b/site/e2e/tests/groups/removeMember.spec.ts index 468d9d4851441..ba2856d578ae5 100644 --- a/site/e2e/tests/groups/removeMember.spec.ts +++ b/site/e2e/tests/groups/removeMember.spec.ts @@ -21,7 +21,7 @@ test("remove member", async ({ page, baseURL }) => { ]); await API.addMember(group.id, member.id); - await page.goto(`${baseURL}/groups/${group.id}`, { + await page.goto(`${baseURL}/groups/${group.name}`, { waitUntil: "domcontentloaded", }); await expect(page).toHaveTitle(`${group.display_name} - Coder`); diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 01b4ccec76b9d..75a476adcf559 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -1455,8 +1455,10 @@ class ApiMethods { return response.data; }; - getGroup = async (groupId: string): Promise => { - const response = await this.axios.get(`/api/v2/groups/${groupId}`); + getGroup = async (groupName: string): Promise => { + const response = await this.axios.get( + `/api/v2/organizations/default/groups/${groupName}`, + ); return response.data; }; diff --git a/site/src/api/queries/groups.ts b/site/src/api/queries/groups.ts index 5c34758df069f..feeeb2335b16b 100644 --- a/site/src/api/queries/groups.ts +++ b/site/src/api/queries/groups.ts @@ -9,7 +9,7 @@ import type { const GROUPS_QUERY_KEY = ["groups"]; type GroupSortOrder = "asc" | "desc"; -const getGroupQueryKey = (groupId: string) => ["group", groupId]; +const getGroupQueryKey = (groupName: string) => ["group", groupName]; export const groups = (organizationId: string) => { return { @@ -18,10 +18,10 @@ export const groups = (organizationId: string) => { } satisfies UseQueryOptions; }; -export const group = (groupId: string) => { +export const group = (groupName: string) => { return { - queryKey: getGroupQueryKey(groupId), - queryFn: () => API.getGroup(groupId), + queryKey: getGroupQueryKey(groupName), + queryFn: () => API.getGroup(groupName), }; }; diff --git a/site/src/pages/GroupsPage/CreateGroupPage.tsx b/site/src/pages/GroupsPage/CreateGroupPage.tsx index d5fd2c1f73c01..16b75cb7bbb0d 100644 --- a/site/src/pages/GroupsPage/CreateGroupPage.tsx +++ b/site/src/pages/GroupsPage/CreateGroupPage.tsx @@ -24,7 +24,7 @@ export const CreateGroupPage: FC = () => { organizationId, ...data, }); - navigate(`/groups/${newGroup.id}`); + navigate(`/groups/${newGroup.name}`); }} error={createGroupMutation.error} isLoading={createGroupMutation.isLoading} diff --git a/site/src/pages/GroupsPage/GroupPage.tsx b/site/src/pages/GroupsPage/GroupPage.tsx index 01e8dc250b13b..56e84a91d713d 100644 --- a/site/src/pages/GroupsPage/GroupPage.tsx +++ b/site/src/pages/GroupsPage/GroupPage.tsx @@ -53,12 +53,16 @@ import { isEveryoneGroup } from "utils/groups"; import { pageTitle } from "utils/page"; export const GroupPage: FC = () => { - const { groupId } = useParams() as { groupId: string }; + const { groupName } = useParams() as { groupName: string }; const queryClient = useQueryClient(); const navigate = useNavigate(); - const groupQuery = useQuery(group(groupId)); + const groupQuery = useQuery(group(groupName)); const groupData = groupQuery.data; - const { data: permissions } = useQuery(groupPermissions(groupId)); + const { data: permissions } = useQuery( + groupData !== undefined + ? groupPermissions(groupData.id) + : { enabled: false }, + ); const addMemberMutation = useMutation(addMember(queryClient)); const deleteGroupMutation = useMutation(deleteGroup(queryClient)); const [isDeletingGroup, setIsDeletingGroup] = useState(false); @@ -83,6 +87,7 @@ export const GroupPage: FC = () => { ); } + const groupId = groupData.id; return ( <> @@ -137,6 +142,7 @@ export const GroupPage: FC = () => { userId: user.id, }); reset(); + await groupQuery.refetch(); } catch (error) { displayError(getErrorMessage(error, "Failed to add member.")); } diff --git a/site/src/pages/GroupsPage/GroupsPageView.tsx b/site/src/pages/GroupsPage/GroupsPageView.tsx index da87bb6e07580..4f167be339eef 100644 --- a/site/src/pages/GroupsPage/GroupsPageView.tsx +++ b/site/src/pages/GroupsPage/GroupsPageView.tsx @@ -96,7 +96,7 @@ export const GroupsPageView: FC = ({ {groups?.map((group) => { - const groupPageLink = `/groups/${group.id}`; + const groupPageLink = `/groups/${group.name}`; return ( { - const { groupId } = useParams() as { groupId: string }; + const { groupName } = useParams() as { groupName: string }; const queryClient = useQueryClient(); - const groupQuery = useQuery(group(groupId)); + const groupQuery = useQuery(group(groupName)); const patchGroupMutation = useMutation(patchGroup(queryClient)); const navigate = useNavigate(); + const groupData = groupQuery.data; + const isLoading = !groupData; const navigateToGroup = () => { - navigate(`/groups/${groupId}`); + navigate(`/groups/${groupName}`); }; + const helmet = ( + + {pageTitle("Settings Group")} + + ); + + if (isLoading) { + return ( + <> + {helmet} + + + ); + } + const groupId = groupData.id; + return ( <> - - {pageTitle("Settings Group")} - + {helmet} } /> - } /> - } /> + } /> + } /> } /> From 2126a1a829aa0b8c5cf82c226ccd012038a43a88 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 27 Jun 2024 19:27:11 +0000 Subject: [PATCH 2/6] fix: update group navigation when name changes --- site/src/pages/GroupsPage/SettingsGroupPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/GroupsPage/SettingsGroupPage.tsx b/site/src/pages/GroupsPage/SettingsGroupPage.tsx index 618747e307376..caa1cd70d103e 100644 --- a/site/src/pages/GroupsPage/SettingsGroupPage.tsx +++ b/site/src/pages/GroupsPage/SettingsGroupPage.tsx @@ -52,7 +52,7 @@ export const SettingsGroupPage: FC = () => { add_users: [], remove_users: [], }); - navigateToGroup(); + navigate(`/groups/${data.name}`); } catch (error) { displayError(getErrorMessage(error, "Failed to update group")); } From d22ada5a7f2a91d57f90e21c1cdb733862b94478 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 27 Jun 2024 20:45:32 +0000 Subject: [PATCH 3/6] fix: update isLoading and error checking --- site/src/pages/GroupsPage/SettingsGroupPage.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/site/src/pages/GroupsPage/SettingsGroupPage.tsx b/site/src/pages/GroupsPage/SettingsGroupPage.tsx index caa1cd70d103e..515e12520d549 100644 --- a/site/src/pages/GroupsPage/SettingsGroupPage.tsx +++ b/site/src/pages/GroupsPage/SettingsGroupPage.tsx @@ -4,6 +4,7 @@ import { useMutation, useQuery, useQueryClient } from "react-query"; import { useNavigate, useParams } from "react-router-dom"; import { getErrorMessage } from "api/errors"; import { group, patchGroup } from "api/queries/groups"; +import { ErrorAlert } from "components/Alert/ErrorAlert"; import { displayError } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { pageTitle } from "utils/page"; @@ -16,7 +17,7 @@ export const SettingsGroupPage: FC = () => { const patchGroupMutation = useMutation(patchGroup(queryClient)); const navigate = useNavigate(); const groupData = groupQuery.data; - const isLoading = !groupData; + const { isLoading, error} = groupQuery const navigateToGroup = () => { navigate(`/groups/${groupName}`); @@ -28,7 +29,11 @@ export const SettingsGroupPage: FC = () => { ); - if (isLoading) { + if (error) { + return ; + } + + if (isLoading || !groupData) { return ( <> {helmet} @@ -52,7 +57,7 @@ export const SettingsGroupPage: FC = () => { add_users: [], remove_users: [], }); - navigate(`/groups/${data.name}`); + navigate(`/groups/${data.name}`, { replace: true }); } catch (error) { displayError(getErrorMessage(error, "Failed to update group")); } From b0f2ae0e4365a1224cdc78a39518b9be99b2c74d Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 27 Jun 2024 20:54:18 +0000 Subject: [PATCH 4/6] fix: fix format --- site/src/pages/GroupsPage/SettingsGroupPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/GroupsPage/SettingsGroupPage.tsx b/site/src/pages/GroupsPage/SettingsGroupPage.tsx index 515e12520d549..9157f2511bb25 100644 --- a/site/src/pages/GroupsPage/SettingsGroupPage.tsx +++ b/site/src/pages/GroupsPage/SettingsGroupPage.tsx @@ -17,7 +17,7 @@ export const SettingsGroupPage: FC = () => { const patchGroupMutation = useMutation(patchGroup(queryClient)); const navigate = useNavigate(); const groupData = groupQuery.data; - const { isLoading, error} = groupQuery + const { isLoading, error } = groupQuery; const navigateToGroup = () => { navigate(`/groups/${groupName}`); From fcf6630a884243ab507d67920e08f5b21e4b6b9a Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 27 Jun 2024 21:02:50 +0000 Subject: [PATCH 5/6] fix: update isLoading and error --- site/src/pages/GroupsPage/GroupPage.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/site/src/pages/GroupsPage/GroupPage.tsx b/site/src/pages/GroupsPage/GroupPage.tsx index 56e84a91d713d..b36f1f9c1cde0 100644 --- a/site/src/pages/GroupsPage/GroupPage.tsx +++ b/site/src/pages/GroupsPage/GroupPage.tsx @@ -23,6 +23,7 @@ import { removeMember, } from "api/queries/groups"; import type { Group, ReducedUser, User } from "api/typesGenerated"; +import { ErrorAlert } from "components/Alert/ErrorAlert"; import { AvatarData } from "components/AvatarData/AvatarData"; import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"; import { EmptyState } from "components/EmptyState/EmptyState"; @@ -66,7 +67,7 @@ export const GroupPage: FC = () => { const addMemberMutation = useMutation(addMember(queryClient)); const deleteGroupMutation = useMutation(deleteGroup(queryClient)); const [isDeletingGroup, setIsDeletingGroup] = useState(false); - const isLoading = !groupData || !permissions; + const isLoading = groupQuery.isLoading || !groupData || !permissions; const canUpdateGroup = permissions ? permissions.canUpdateGroup : false; const helmet = ( @@ -79,6 +80,10 @@ export const GroupPage: FC = () => { ); + if (groupQuery.error) { + return ; + } + if (isLoading) { return ( <> From b3d4d7fb0b632872c6a123c367f39078bbf78846 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 28 Jun 2024 20:03:56 +0000 Subject: [PATCH 6/6] fix: cleanup --- site/src/pages/GroupsPage/SettingsGroupPage.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/site/src/pages/GroupsPage/SettingsGroupPage.tsx b/site/src/pages/GroupsPage/SettingsGroupPage.tsx index 9157f2511bb25..efb7fadbce29e 100644 --- a/site/src/pages/GroupsPage/SettingsGroupPage.tsx +++ b/site/src/pages/GroupsPage/SettingsGroupPage.tsx @@ -14,10 +14,9 @@ export const SettingsGroupPage: FC = () => { const { groupName } = useParams() as { groupName: string }; const queryClient = useQueryClient(); const groupQuery = useQuery(group(groupName)); + const { data: groupData, isLoading, error } = useQuery(group(groupName)); const patchGroupMutation = useMutation(patchGroup(queryClient)); const navigate = useNavigate(); - const groupData = groupQuery.data; - const { isLoading, error } = groupQuery; const navigateToGroup = () => { navigate(`/groups/${groupName}`);