Skip to content

Commit c636009

Browse files
aslilacaslilac
committed
feedback
1 parent 560fd4f commit c636009

File tree

7 files changed

+166
-161
lines changed

7 files changed

+166
-161
lines changed

‎enterprise/coderd/groups.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package coderd
22

33
import (
44
"database/sql"
5+
"errors"
56
"fmt"
67
"net/http"
78

@@ -170,9 +171,9 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
170171
OrganizationID: group.OrganizationID,
171172
UserID: uuid.MustParse(id),
172173
}))
173-
if xerrors.Is(err, sql.ErrNoRows) {
174+
if errors.Is(err, sql.ErrNoRows) {
174175
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
175-
Message: fmt.Sprintf("User %q must be a member of organization %q", id, group.ID),
176+
Message: fmt.Sprintf("User must be a member of organization %q", group.Name),
176177
})
177178
return
178179
}
@@ -364,7 +365,7 @@ func (api *API) group(rw http.ResponseWriter, r *http.Request) {
364365
)
365366

366367
users, err := api.Database.GetGroupMembersByGroupID(ctx, group.ID)
367-
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
368+
if err != nil && !errors.Is(err, sql.ErrNoRows) {
368369
httpapi.InternalServerError(rw, err)
369370
return
370371
}
@@ -391,7 +392,7 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) {
391392
)
392393

393394
groups, err := api.Database.GetGroupsByOrganizationID(ctx, org.ID)
394-
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
395+
if err != nil && !errors.Is(err, sql.ErrNoRows) {
395396
httpapi.InternalServerError(rw, err)
396397
return
397398
}

‎site/src/components/PageHeader/PageHeader.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,3 +107,23 @@ export const PageHeaderCaption: FC<PropsWithChildren> = ({ children }) => {
107107
</span>
108108
);
109109
};
110+
111+
interface ResourcePageHeaderProps extends Omit<PageHeaderProps, "children"> {
112+
displayName?: string;
113+
name: string;
114+
}
115+
116+
export const ResourcePageHeader: FC<ResourcePageHeaderProps> = ({
117+
displayName,
118+
name,
119+
...props
120+
}) => {
121+
const title = displayName || name;
122+
123+
return (
124+
<PageHeader {...props}>
125+
<PageHeaderTitle>{title}</PageHeaderTitle>
126+
{name !== title && <PageHeaderSubtitle>{name}</PageHeaderSubtitle>}
127+
</PageHeader>
128+
);
129+
};

‎site/src/pages/ManagementSettingsPage/GroupsPage/CreateGroupPageView.stories.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Meta, StoryObj } from "@storybook/react";
22
import { mockApiError } from "testHelpers/entities";
33
import { CreateGroupPageView } from "./CreateGroupPageView";
4+
import { userEvent, within } from "@storybook/test";
45

56
const meta: Meta<typeof CreateGroupPageView> = {
67
title: "pages/OrganizationGroupsPage/CreateGroupPageView",
@@ -18,6 +19,14 @@ export const WithError: Story = {
1819
message: "A group named new-group already exists.",
1920
validations: [{ field: "name", detail: "Group names must be unique" }],
2021
}),
21-
initialTouched: { name: true },
22+
},
23+
play: async ({ canvasElement, step }) => {
24+
const canvas = within(canvasElement);
25+
26+
await step("Enter name", async () => {
27+
const input = canvas.getByLabelText("Name");
28+
await userEvent.type(input, "new-group");
29+
input.blur();
30+
});
2231
},
2332
};

‎site/src/pages/ManagementSettingsPage/GroupsPage/CreateGroupPageView.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,12 @@ export type CreateGroupPageViewProps = {
2424
onSubmit: (data: CreateGroupRequest) => void;
2525
error?: unknown;
2626
isLoading: boolean;
27-
// Helpful to show field errors on Storybook
28-
initialTouched?: FormikTouched<CreateGroupRequest>;
2927
};
3028

3129
export const CreateGroupPageView: FC<CreateGroupPageViewProps> = ({
3230
onSubmit,
3331
error,
3432
isLoading,
35-
initialTouched,
3633
}) => {
3734
const navigate = useNavigate();
3835
const form = useFormik<CreateGroupRequest>({
@@ -44,7 +41,6 @@ export const CreateGroupPageView: FC<CreateGroupPageViewProps> = ({
4441
},
4542
validationSchema,
4643
onSubmit,
47-
initialTouched,
4844
});
4945
const getFieldHelpers = getFormHelpers<CreateGroupRequest>(form, error);
5046
const onCancel = () => navigate(-1);

‎site/src/pages/ManagementSettingsPage/GroupsPage/GroupPage.tsx

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
PageHeader,
4343
PageHeaderSubtitle,
4444
PageHeaderTitle,
45+
ResourcePageHeader,
4546
} from "components/PageHeader/PageHeader";
4647
import { Stack } from "components/Stack/Stack";
4748
import {
@@ -103,7 +104,9 @@ export const GroupPage: FC = () => {
103104
{helmet}
104105

105106
<Margins>
106-
<PageHeader
107+
<ResourcePageHeader
108+
displayName={groupData?.display_name}
109+
name={groupData?.name}
107110
actions={
108111
canUpdateGroup && (
109112
<>
@@ -127,18 +130,7 @@ export const GroupPage: FC = () => {
127130
</>
128131
)
129132
}
130-
>
131-
<PageHeaderTitle>
132-
{groupData?.display_name || groupData?.name}
133-
</PageHeaderTitle>
134-
<PageHeaderSubtitle>
135-
{/* Show the name if it differs from the display name. */}
136-
{groupData?.display_name &&
137-
groupData?.display_name !== groupData?.name
138-
? groupData?.name
139-
: ""}{" "}
140-
</PageHeaderSubtitle>
141-
</PageHeader>
133+
/>
142134

143135
<Stack spacing={1}>
144136
{canUpdateGroup && groupData && !isEveryoneGroup(groupData) && (

‎site/src/pages/ManagementSettingsPage/GroupsPage/GroupSettingsPageView.tsx

Lines changed: 72 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
} from "components/Form/Form";
1212
import { IconField } from "components/IconField/IconField";
1313
import { Loader } from "components/Loader/Loader";
14-
import { PageHeader, PageHeaderTitle } from "components/PageHeader/PageHeader";
14+
import { ResourcePageHeader } from "components/PageHeader/PageHeader";
1515
import {
1616
getFormHelpers,
1717
nameValidator,
@@ -59,72 +59,65 @@ const UpdateGroupForm: FC<UpdateGroupFormProps> = ({
5959
const getFieldHelpers = getFormHelpers<FormData>(form, errors);
6060

6161
return (
62-
<>
63-
<PageHeader css={{ paddingTop: 8 }}>
64-
<PageHeaderTitle>{group.name}</PageHeaderTitle>
65-
</PageHeader>
66-
<HorizontalForm onSubmit={form.handleSubmit}>
67-
<FormSection
68-
title="Group settings"
69-
description="Set a name and avatar for this group."
70-
>
71-
<FormFields>
72-
<TextField
73-
{...getFieldHelpers("name")}
74-
onChange={onChangeTrimmed(form)}
75-
autoComplete="name"
76-
autoFocus
77-
fullWidth
78-
label="Name"
79-
disabled={isEveryoneGroup(group)}
80-
/>
81-
{!isEveryoneGroup(group) && (
82-
<>
83-
<TextField
84-
{...getFieldHelpers("display_name", {
85-
helperText: "Optional: keep empty to default to the name.",
86-
})}
87-
autoComplete="display_name"
88-
autoFocus
89-
fullWidth
90-
label="Display Name"
91-
disabled={isEveryoneGroup(group)}
92-
/>
93-
<IconField
94-
{...getFieldHelpers("avatar_url")}
95-
onChange={onChangeTrimmed(form)}
96-
fullWidth
97-
label="Avatar URL"
98-
onPickEmoji={(value) =>
99-
form.setFieldValue("avatar_url", value)
100-
}
101-
/>
102-
</>
103-
)}
104-
</FormFields>
105-
</FormSection>
106-
<FormSection
107-
title="Quota"
108-
description="You can use quotas to restrict how many resources a user can create."
109-
>
110-
<FormFields>
111-
<TextField
112-
{...getFieldHelpers("quota_allowance", {
113-
helperText: `This group gives ${form.values.quota_allowance} quota credits to each
62+
<HorizontalForm onSubmit={form.handleSubmit}>
63+
<FormSection
64+
title="Group settings"
65+
description="Set a name and avatar for this group."
66+
>
67+
<FormFields>
68+
<TextField
69+
{...getFieldHelpers("name")}
70+
onChange={onChangeTrimmed(form)}
71+
autoComplete="name"
72+
autoFocus
73+
fullWidth
74+
label="Name"
75+
disabled={isEveryoneGroup(group)}
76+
/>
77+
{!isEveryoneGroup(group) && (
78+
<>
79+
<TextField
80+
{...getFieldHelpers("display_name", {
81+
helperText: "Optional: keep empty to default to the name.",
82+
})}
83+
autoComplete="display_name"
84+
autoFocus
85+
fullWidth
86+
label="Display Name"
87+
disabled={isEveryoneGroup(group)}
88+
/>
89+
<IconField
90+
{...getFieldHelpers("avatar_url")}
91+
onChange={onChangeTrimmed(form)}
92+
fullWidth
93+
label="Avatar URL"
94+
onPickEmoji={(value) => form.setFieldValue("avatar_url", value)}
95+
/>
96+
</>
97+
)}
98+
</FormFields>
99+
</FormSection>
100+
<FormSection
101+
title="Quota"
102+
description="You can use quotas to restrict how many resources a user can create."
103+
>
104+
<FormFields>
105+
<TextField
106+
{...getFieldHelpers("quota_allowance", {
107+
helperText: `This group gives ${form.values.quota_allowance} quota credits to each
114108
of its members.`,
115-
})}
116-
onChange={onChangeTrimmed(form)}
117-
autoFocus
118-
fullWidth
119-
type="number"
120-
label="Quota Allowance"
121-
/>
122-
</FormFields>
123-
</FormSection>
109+
})}
110+
onChange={onChangeTrimmed(form)}
111+
autoFocus
112+
fullWidth
113+
type="number"
114+
label="Quota Allowance"
115+
/>
116+
</FormFields>
117+
</FormSection>
124118

125-
<FormFooter onCancel={onCancel} isLoading={isLoading} />
126-
</HorizontalForm>
127-
</>
119+
<FormFooter onCancel={onCancel} isLoading={isLoading} />
120+
</HorizontalForm>
128121
);
129122
};
130123

@@ -150,13 +143,20 @@ const GroupSettingsPageView: FC<SettingsGroupPageViewProps> = ({
150143
}
151144

152145
return (
153-
<UpdateGroupForm
154-
group={group!}
155-
onCancel={onCancel}
156-
errors={formErrors}
157-
isLoading={isUpdating}
158-
onSubmit={onSubmit}
159-
/>
146+
<>
147+
<ResourcePageHeader
148+
displayName={group!.display_name}
149+
name={group!.name}
150+
css={{ paddingTop: 8 }}
151+
/>
152+
<UpdateGroupForm
153+
group={group!}
154+
onCancel={onCancel}
155+
errors={formErrors}
156+
isLoading={isUpdating}
157+
onSubmit={onSubmit}
158+
/>
159+
</>
160160
);
161161
};
162162

0 commit comments

Comments
 (0)