From 60e165c9255770b00f235e1330481aa1c1c9b998 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 22 Aug 2024 12:30:55 -0800 Subject: [PATCH 1/3] fix: filter add group member by organization 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. --- .../MemberAutocomplete.stories.tsx | 26 ++++ .../UserAutocomplete.stories.tsx | 2 +- .../UserAutocomplete/UserAutocomplete.tsx | 141 ++++++++++++------ .../GroupsPage/GroupPage.tsx | 28 +++- 4 files changed, 146 insertions(+), 51 deletions(-) create mode 100644 site/src/components/UserAutocomplete/MemberAutocomplete.stories.tsx diff --git a/site/src/components/UserAutocomplete/MemberAutocomplete.stories.tsx b/site/src/components/UserAutocomplete/MemberAutocomplete.stories.tsx new file mode 100644 index 0000000000000..9e2c78e41d8df --- /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 212578bc8e196..eee96b248f52b 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 9581aaf505f61..96d53ed05b65c 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; +// The common properties between users and org members that we need. +export type SelectedUser = { + avatar_url: string; + email: string; + username: string; +}; + +export type CommonAutocompleteProps = { className?: string; - size?: ComponentProps["size"]; + label?: string; + onChange: (user: T | null) => void; required?: boolean; + size?: ComponentProps["size"]; + value: T | null; }; -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 UserAutocompleteProps = CommonAutocompleteProps; + +export const UserAutocomplete: FC = (props) => { + const [filter, setFilter] = useState(); + const usersQuery = useQuery({ ...users({ - q: prepareQuery(encodeURI(autoComplete.value)), + q: filter !== undefined ? 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.tsx b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.tsx index 50beb7f8ce276..b307adabc4a50 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"; @@ -133,11 +137,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 +236,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 +263,10 @@ const AddGroupMember: FC = ({ isLoading, onSubmit }) => { }} > - { setSelectedUser(newValue); }} From fd02c23fb689db47bbce07ba40630f836b9b9403 Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 23 Aug 2024 14:32:09 -0800 Subject: [PATCH 2/3] Add Storybook for groups page This ensures it is using the right endpoint for the add member dropdown. --- site/src/api/queries/groups.ts | 10 +- site/src/api/queries/organizations.ts | 8 +- .../UserAutocomplete/UserAutocomplete.tsx | 2 +- .../GroupsPage/GroupPage.stories.tsx | 132 ++++++++++++++++++ .../GroupsPage/GroupPage.tsx | 4 +- 5 files changed, 149 insertions(+), 7 deletions(-) create mode 100644 site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx diff --git a/site/src/api/queries/groups.ts b/site/src/api/queries/groups.ts index e42ba57566990..7202b8ffd19dd 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 54ff0ca8832dc..d8386679dd7b7 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/UserAutocomplete.tsx b/site/src/components/UserAutocomplete/UserAutocomplete.tsx index 96d53ed05b65c..d80773cb4de16 100644 --- a/site/src/components/UserAutocomplete/UserAutocomplete.tsx +++ b/site/src/components/UserAutocomplete/UserAutocomplete.tsx @@ -41,7 +41,7 @@ export const UserAutocomplete: FC = (props) => { const usersQuery = useQuery({ ...users({ - q: filter !== undefined ? prepareQuery(encodeURI(filter)) : "", + q: prepareQuery(encodeURI(filter ?? "")), limit: 25, }), enabled: filter !== undefined, 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 0000000000000..535328e061151 --- /dev/null +++ b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx @@ -0,0 +1,132 @@ +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({})], + }, +}; + +// Will show a 404 because the query is not mocked. +export const GroupError: Story = { + parameters: { + queries: [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), + ], + }, +}; + +// Will show a 404 because the query is not mocked. +export const MembersError: Story = { + parameters: { + queries: [ + groupQuery(MockGroup), + permissionsQuery({ canUpdateGroup: true }), + ], + }, + 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 b307adabc4a50..8a90404644b19 100644 --- a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.tsx +++ b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.tsx @@ -62,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)); From a04365393a1291fcee9ca6b3eb061a8347a153f5 Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 23 Aug 2024 15:35:00 -0800 Subject: [PATCH 3/3] Add ability to mock react-query errors --- site/.storybook/preview.jsx | 15 +++++++++++++-- .../GroupsPage/GroupPage.stories.tsx | 5 ++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/site/.storybook/preview.jsx b/site/.storybook/preview.jsx index 6e2f17343e19e..d4eeafec72737 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/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx index 535328e061151..b7b75fc134fae 100644 --- a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.stories.tsx @@ -51,10 +51,9 @@ export const LoadingGroup: Story = { }, }; -// Will show a 404 because the query is not mocked. export const GroupError: Story = { parameters: { - queries: [permissionsQuery({})], + queries: [groupQuery(new Error("test group error")), permissionsQuery({})], }, }; @@ -86,12 +85,12 @@ export const EveryoneGroup: Story = { }, }; -// Will show a 404 because the query is not mocked. export const MembersError: Story = { parameters: { queries: [ groupQuery(MockGroup), permissionsQuery({ canUpdateGroup: true }), + membersQuery(new Error("test members error")), ], }, play: async ({ canvasElement }) => {