From 88d7181a478181dd423685b76e099f246bd79145 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 26 Aug 2024 19:13:34 +0000 Subject: [PATCH] fix: filter "add group member" by organization (#14404) This is accomplished by using the members endpoint instead of the users endpoint, and to that end the UserAutocomplete component has been reworked to support either endpoint as separate components with a shared base. * Add Storybook for groups page This ensures it is using the right endpoint for the add member dropdown. * Add ability to mock react-query errors --- site/.storybook/preview.jsx | 15 +- site/src/api/queries/groups.ts | 10 +- site/src/api/queries/organizations.ts | 8 +- .../MemberAutocomplete.stories.tsx | 26 ++++ .../UserAutocomplete.stories.tsx | 2 +- .../UserAutocomplete/UserAutocomplete.tsx | 145 ++++++++++++------ .../GroupsPage/GroupPage.stories.tsx | 131 ++++++++++++++++ .../GroupsPage/GroupPage.tsx | 32 ++-- 8 files changed, 308 insertions(+), 61 deletions(-) create mode 100644 site/src/components/UserAutocomplete/MemberAutocomplete.stories.tsx create mode 100644 site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx diff --git a/site/.storybook/preview.jsx b/site/.storybook/preview.jsx index 6e2f17343e..d4eeafec72 100644 --- a/site/.storybook/preview.jsx +++ b/site/.storybook/preview.jsx @@ -7,7 +7,7 @@ import { ThemeProvider as EmotionThemeProvider } from "@emotion/react"; import { DecoratorHelpers } from "@storybook/addon-themes"; import { withRouter } from "storybook-addon-remix-react-router"; import { StrictMode } from "react"; -import { QueryClient, QueryClientProvider } from "react-query"; +import { parseQueryArgs, QueryClient, QueryClientProvider } from "react-query"; import { HelmetProvider } from "react-helmet-async"; import themes from "theme"; import "theme/globalFonts"; @@ -93,7 +93,18 @@ function withQuery(Story, { parameters }) { if (parameters.queries) { parameters.queries.forEach((query) => { - queryClient.setQueryData(query.key, query.data); + if (query.data instanceof Error) { + // This is copied from setQueryData() but sets the error. + const cache = queryClient.getQueryCache(); + const parsedOptions = parseQueryArgs(query.key) + const defaultedOptions = queryClient.defaultQueryOptions(parsedOptions) + const cachedQuery = cache.build(queryClient, defaultedOptions); + // Set manual data so react-query will not try to refetch. + cachedQuery.setData(undefined, { manual: true }); + cachedQuery.setState({ error: query.data }); + } else { + queryClient.setQueryData(query.key, query.data); + } }); } diff --git a/site/src/api/queries/groups.ts b/site/src/api/queries/groups.ts index e42ba57566..7202b8ffd1 100644 --- a/site/src/api/queries/groups.ts +++ b/site/src/api/queries/groups.ts @@ -21,7 +21,7 @@ export const groups = (organization: string) => { } satisfies UseQueryOptions; }; -const getGroupQueryKey = (organization: string, groupName: string) => [ +export const getGroupQueryKey = (organization: string, groupName: string) => [ "organization", organization, "group", @@ -77,9 +77,15 @@ export function groupsForUser(organization: string, userId: string) { } as const satisfies UseQueryOptions; } +export const groupPermissionsKey = (groupId: string) => [ + "group", + groupId, + "permissions", +]; + export const groupPermissions = (groupId: string) => { return { - queryKey: ["group", groupId, "permissions"], + queryKey: groupPermissionsKey(groupId), queryFn: () => API.checkAuthorization({ checks: { diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts index 54ff0ca883..d8386679dd 100644 --- a/site/src/api/queries/organizations.ts +++ b/site/src/api/queries/organizations.ts @@ -47,10 +47,16 @@ export const deleteOrganization = (queryClient: QueryClient) => { }; }; +export const organizationMembersKey = (id: string) => [ + "organization", + id, + "members", +]; + export const organizationMembers = (id: string) => { return { queryFn: () => API.getOrganizationMembers(id), - queryKey: ["organization", id, "members"], + queryKey: organizationMembersKey(id), }; }; diff --git a/site/src/components/UserAutocomplete/MemberAutocomplete.stories.tsx b/site/src/components/UserAutocomplete/MemberAutocomplete.stories.tsx new file mode 100644 index 0000000000..9e2c78e41d --- /dev/null +++ b/site/src/components/UserAutocomplete/MemberAutocomplete.stories.tsx @@ -0,0 +1,26 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import { MockOrganizationMember } from "testHelpers/entities"; +import { MemberAutocomplete } from "./UserAutocomplete"; + +const meta: Meta = { + title: "components/MemberAutocomplete", + component: MemberAutocomplete, +}; + +export default meta; +type Story = StoryObj; + +export const WithLabel: Story = { + args: { + value: MockOrganizationMember, + organizationId: MockOrganizationMember.organization_id, + label: "Member", + }, +}; + +export const NoLabel: Story = { + args: { + value: MockOrganizationMember, + organizationId: MockOrganizationMember.organization_id, + }, +}; diff --git a/site/src/components/UserAutocomplete/UserAutocomplete.stories.tsx b/site/src/components/UserAutocomplete/UserAutocomplete.stories.tsx index 212578bc8e..eee96b248f 100644 --- a/site/src/components/UserAutocomplete/UserAutocomplete.stories.tsx +++ b/site/src/components/UserAutocomplete/UserAutocomplete.stories.tsx @@ -10,7 +10,7 @@ const meta: Meta = { export default meta; type Story = StoryObj; -export const Example: Story = { +export const WithLabel: Story = { args: { value: MockUser, label: "User", diff --git a/site/src/components/UserAutocomplete/UserAutocomplete.tsx b/site/src/components/UserAutocomplete/UserAutocomplete.tsx index 9581aaf505..d80773cb4d 100644 --- a/site/src/components/UserAutocomplete/UserAutocomplete.tsx +++ b/site/src/components/UserAutocomplete/UserAutocomplete.tsx @@ -2,8 +2,10 @@ import { css } from "@emotion/css"; import Autocomplete from "@mui/material/Autocomplete"; import CircularProgress from "@mui/material/CircularProgress"; import TextField from "@mui/material/TextField"; +import { getErrorMessage } from "api/errors"; +import { organizationMembers } from "api/queries/organizations"; import { users } from "api/queries/users"; -import type { User } from "api/typesGenerated"; +import type { OrganizationMemberWithUserData, User } from "api/typesGenerated"; import { Avatar } from "components/Avatar/Avatar"; import { AvatarData } from "components/AvatarData/AvatarData"; import { useDebouncedFunction } from "hooks/debounce"; @@ -16,71 +18,128 @@ import { import { useQuery } from "react-query"; import { prepareQuery } from "utils/filters"; -export type UserAutocompleteProps = { - value: User | null; - onChange: (user: User | null) => void; - label?: string; - className?: string; - size?: ComponentProps["size"]; - required?: boolean; +// The common properties between users and org members that we need. +export type SelectedUser = { + avatar_url: string; + email: string; + username: string; }; -export const UserAutocomplete: FC = ({ - value, - onChange, - label, - className, - size = "small", - required, -}) => { - const [autoComplete, setAutoComplete] = useState<{ - value: string; - open: boolean; - }>({ - value: value?.email ?? "", - open: false, - }); +export type CommonAutocompleteProps = { + className?: string; + label?: string; + onChange: (user: T | null) => void; + required?: boolean; + size?: ComponentProps["size"]; + value: T | null; +}; + +export type UserAutocompleteProps = CommonAutocompleteProps; + +export const UserAutocomplete: FC = (props) => { + const [filter, setFilter] = useState(); + const usersQuery = useQuery({ ...users({ - q: prepareQuery(encodeURI(autoComplete.value)), + q: prepareQuery(encodeURI(filter ?? "")), limit: 25, }), - enabled: autoComplete.open, + enabled: filter !== undefined, keepPreviousData: true, }); + return ( + + error={usersQuery.error} + isFetching={usersQuery.isFetching} + setFilter={setFilter} + users={usersQuery.data?.users} + {...props} + /> + ); +}; + +export type MemberAutocompleteProps = + CommonAutocompleteProps & { + organizationId: string; + }; + +export const MemberAutocomplete: FC = ({ + organizationId, + ...props +}) => { + const [filter, setFilter] = useState(); + + // Currently this queries all members, as there is no pagination. + const membersQuery = useQuery({ + ...organizationMembers(organizationId), + enabled: filter !== undefined, + keepPreviousData: true, + }); + return ( + + error={membersQuery.error} + isFetching={membersQuery.isFetching} + setFilter={setFilter} + users={membersQuery.data} + {...props} + /> + ); +}; + +type InnerAutocompleteProps = + CommonAutocompleteProps & { + /** The error is null if not loaded or no error. */ + error: unknown; + isFetching: boolean; + /** Filter is undefined if the autocomplete is closed. */ + setFilter: (filter: string | undefined) => void; + /** Users are undefined if not loaded or errored. */ + users: readonly T[] | undefined; + }; + +const InnerAutocomplete = ({ + className, + error, + isFetching, + label, + onChange, + required, + setFilter, + size = "small", + users, + value, +}: InnerAutocompleteProps) => { + const [open, setOpen] = useState(false); const { debounced: debouncedInputOnChange } = useDebouncedFunction( (event: ChangeEvent) => { - setAutoComplete((state) => ({ - ...state, - value: event.target.value, - })); + setFilter(event.target.value ?? ""); }, 750, ); return ( a.username === b.username} getOptionLabel={(option) => option.email} onOpen={() => { - setAutoComplete((state) => ({ - ...state, - open: true, - })); + setOpen(true); + setFilter(value?.email ?? ""); }} onClose={() => { - setAutoComplete({ - value: value?.email ?? "", - open: false, - }); + setOpen(false); + setFilter(undefined); }} onChange={(_, newValue) => { onChange(newValue); @@ -117,9 +176,7 @@ export const UserAutocomplete: FC = ({ ), endAdornment: ( <> - {usersQuery.isFetching && autoComplete.open && ( - - )} + {isFetching && open && } {params.InputProps.endAdornment} ), diff --git a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx new file mode 100644 index 0000000000..b7b75fc134 --- /dev/null +++ b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx @@ -0,0 +1,131 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import { userEvent, within } from "@storybook/test"; +import { getGroupQueryKey, groupPermissionsKey } from "api/queries/groups"; +import { organizationMembersKey } from "api/queries/organizations"; +import { reactRouterParameters } from "storybook-addon-remix-react-router"; +import { + MockDefaultOrganization, + MockGroup, + MockOrganizationMember, + MockOrganizationMember2, +} from "testHelpers/entities"; +import GroupPage from "./GroupPage"; + +const meta: Meta = { + title: "pages/OrganizationGroupsPage/GroupPage", + component: GroupPage, + parameters: { + reactRouter: reactRouterParameters({ + location: { + pathParams: { + organization: MockDefaultOrganization.name, + groupName: MockGroup.name, + }, + }, + routing: { path: "/organizations/:organization/groups/:groupName" }, + }), + }, +}; + +const groupQuery = (data: unknown) => ({ + key: getGroupQueryKey(MockDefaultOrganization.name, MockGroup.name), + data, +}); + +const permissionsQuery = (data: unknown, id?: string) => ({ + key: groupPermissionsKey(id ?? MockGroup.id), + data, +}); + +const membersQuery = (data: unknown) => ({ + key: organizationMembersKey(MockDefaultOrganization.id), + data, +}); + +export default meta; +type Story = StoryObj; + +export const LoadingGroup: Story = { + parameters: { + queries: [groupQuery(null), permissionsQuery({})], + }, +}; + +export const GroupError: Story = { + parameters: { + queries: [groupQuery(new Error("test group error")), permissionsQuery({})], + }, +}; + +export const LoadingPermissions: Story = { + parameters: { + queries: [groupQuery(MockGroup), permissionsQuery(null)], + }, +}; + +export const NoUpdatePermission: Story = { + parameters: { + queries: [ + groupQuery(MockGroup), + permissionsQuery({ canUpdateGroup: false }), + ], + }, +}; + +export const EveryoneGroup: Story = { + parameters: { + queries: [ + groupQuery({ + ...MockGroup, + // The everyone group has the same ID as the organization. + id: MockDefaultOrganization.id, + }), + permissionsQuery({ canUpdateGroup: true }, MockDefaultOrganization.id), + ], + }, +}; + +export const MembersError: Story = { + parameters: { + queries: [ + groupQuery(MockGroup), + permissionsQuery({ canUpdateGroup: true }), + membersQuery(new Error("test members error")), + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Open" })); + }, +}; + +export const NoMembers: Story = { + parameters: { + queries: [ + groupQuery({ + ...MockGroup, + members: [], + }), + permissionsQuery({ canUpdateGroup: true }), + membersQuery([]), + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Open" })); + }, +}; + +export const FiltersByMembers: Story = { + parameters: { + queries: [ + groupQuery(MockGroup), + permissionsQuery({ canUpdateGroup: true }), + membersQuery([MockOrganizationMember, MockOrganizationMember2]), + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Open" })); + }, +}; diff --git a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.tsx b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.tsx index 50beb7f8ce..8a90404644 100644 --- a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.tsx +++ b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.tsx @@ -18,7 +18,11 @@ import { groupPermissions, removeMember, } from "api/queries/groups"; -import type { Group, ReducedUser, User } from "api/typesGenerated"; +import type { + Group, + OrganizationMemberWithUserData, + ReducedUser, +} from "api/typesGenerated"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { AvatarData } from "components/AvatarData/AvatarData"; import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"; @@ -39,7 +43,7 @@ import { PaginationStatus, TableToolbar, } from "components/TableToolbar/TableToolbar"; -import { UserAutocomplete } from "components/UserAutocomplete/UserAutocomplete"; +import { MemberAutocomplete } from "components/UserAutocomplete/UserAutocomplete"; import { UserAvatar } from "components/UserAvatar/UserAvatar"; import { type FC, useState } from "react"; import { Helmet } from "react-helmet-async"; @@ -58,9 +62,7 @@ export const GroupPage: FC = () => { const groupQuery = useQuery(group(organization, groupName)); const groupData = groupQuery.data; const { data: permissions } = useQuery( - groupData !== undefined - ? groupPermissions(groupData.id) - : { enabled: false }, + groupData ? groupPermissions(groupData.id) : { enabled: false }, ); const addMemberMutation = useMutation(addMember(queryClient)); const removeMemberMutation = useMutation(removeMember(queryClient)); @@ -133,11 +135,12 @@ export const GroupPage: FC = () => { {canUpdateGroup && groupData && !isEveryoneGroup(groupData) && ( { + organizationId={groupData.organization_id} + onSubmit={async (member, reset) => { try { await addMemberMutation.mutateAsync({ groupId, - userId: user.id, + userId: member.user_id, }); reset(); await groupQuery.refetch(); @@ -231,11 +234,17 @@ export const GroupPage: FC = () => { interface AddGroupMemberProps { isLoading: boolean; - onSubmit: (user: User, reset: () => void) => void; + onSubmit: (user: OrganizationMemberWithUserData, reset: () => void) => void; + organizationId: string; } -const AddGroupMember: FC = ({ isLoading, onSubmit }) => { - const [selectedUser, setSelectedUser] = useState(null); +const AddGroupMember: FC = ({ + isLoading, + onSubmit, + organizationId, +}) => { + const [selectedUser, setSelectedUser] = + useState(null); const resetValues = () => { setSelectedUser(null); @@ -252,9 +261,10 @@ const AddGroupMember: FC = ({ isLoading, onSubmit }) => { }} > - { setSelectedUser(newValue); }}