From 1c1d579cbd8c3d728dfdccbdeef2c0738383bed2 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 12 Oct 2022 23:59:25 +0000 Subject: [PATCH 01/14] feat: add avatar urls to groups --- coderd/database/dump.sql | 3 +- .../migrations/000059_group_avatars.down.sql | 5 ++ .../migrations/000059_group_avatars.up.sql | 5 ++ coderd/database/models.go | 1 + coderd/database/queries.sql.go | 86 ++++++++++++++----- coderd/database/queries/groups.sql | 10 ++- codersdk/groups.go | 5 +- enterprise/coderd/groups.go | 30 +++++-- site/src/api/typesGenerated.ts | 3 + .../GroupAvatar/GroupAvatar.stories.tsx | 1 + .../components/GroupAvatar/GroupAvatar.tsx | 11 ++- .../pages/GroupsPage/CreateGroupPageView.tsx | 10 +++ site/src/pages/GroupsPage/GroupsPageView.tsx | 7 +- .../GroupsPage/SettingsGroupPageView.tsx | 10 +++ .../TemplatePermissionsPageView.tsx | 7 +- .../src/xServices/groups/editGroupXService.ts | 2 +- 16 files changed, 156 insertions(+), 40 deletions(-) create mode 100644 coderd/database/migrations/000059_group_avatars.down.sql create mode 100644 coderd/database/migrations/000059_group_avatars.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index eb16074e90525..c79f088f02dbd 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -170,7 +170,8 @@ CREATE TABLE group_members ( CREATE TABLE groups ( id uuid NOT NULL, name text NOT NULL, - organization_id uuid NOT NULL + organization_id uuid NOT NULL, + avatar_url text DEFAULT ''::text NOT NULL ); CREATE TABLE licenses ( diff --git a/coderd/database/migrations/000059_group_avatars.down.sql b/coderd/database/migrations/000059_group_avatars.down.sql new file mode 100644 index 0000000000000..eb15f354383fc --- /dev/null +++ b/coderd/database/migrations/000059_group_avatars.down.sql @@ -0,0 +1,5 @@ +BEGIN; + +ALTER TABLE groups DROP COLUMN avatar_url; + +COMMIT; diff --git a/coderd/database/migrations/000059_group_avatars.up.sql b/coderd/database/migrations/000059_group_avatars.up.sql new file mode 100644 index 0000000000000..b7f033874ba68 --- /dev/null +++ b/coderd/database/migrations/000059_group_avatars.up.sql @@ -0,0 +1,5 @@ +BEGIN; + +ALTER TABLE groups ADD COLUMN avatar_url text NOT NULL DEFAULT ''; + +COMMIT; diff --git a/coderd/database/models.go b/coderd/database/models.go index f669b5e618138..d1e99af98a914 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -418,6 +418,7 @@ type Group struct { ID uuid.UUID `db:"id" json:"id"` Name string `db:"name" json:"name"` OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + AvatarURL string `db:"avatar_url" json:"avatar_url"` } type GroupMember struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 16613ee7d5ef5..4e45a3694193d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -882,7 +882,7 @@ func (q *sqlQuerier) GetAllOrganizationMembers(ctx context.Context, organization const getGroupByID = `-- name: GetGroupByID :one SELECT - id, name, organization_id + id, name, organization_id, avatar_url FROM groups WHERE @@ -894,13 +894,18 @@ LIMIT func (q *sqlQuerier) GetGroupByID(ctx context.Context, id uuid.UUID) (Group, error) { row := q.db.QueryRowContext(ctx, getGroupByID, id) var i Group - err := row.Scan(&i.ID, &i.Name, &i.OrganizationID) + err := row.Scan( + &i.ID, + &i.Name, + &i.OrganizationID, + &i.AvatarURL, + ) return i, err } const getGroupByOrgAndName = `-- name: GetGroupByOrgAndName :one SELECT - id, name, organization_id + id, name, organization_id, avatar_url FROM groups WHERE @@ -919,7 +924,12 @@ type GetGroupByOrgAndNameParams struct { func (q *sqlQuerier) GetGroupByOrgAndName(ctx context.Context, arg GetGroupByOrgAndNameParams) (Group, error) { row := q.db.QueryRowContext(ctx, getGroupByOrgAndName, arg.OrganizationID, arg.Name) var i Group - err := row.Scan(&i.ID, &i.Name, &i.OrganizationID) + err := row.Scan( + &i.ID, + &i.Name, + &i.OrganizationID, + &i.AvatarURL, + ) return i, err } @@ -978,7 +988,7 @@ func (q *sqlQuerier) GetGroupMembers(ctx context.Context, groupID uuid.UUID) ([] const getGroupsByOrganizationID = `-- name: GetGroupsByOrganizationID :many SELECT - id, name, organization_id + id, name, organization_id, avatar_url FROM groups WHERE @@ -996,7 +1006,12 @@ func (q *sqlQuerier) GetGroupsByOrganizationID(ctx context.Context, organization var items []Group for rows.Next() { var i Group - if err := rows.Scan(&i.ID, &i.Name, &i.OrganizationID); err != nil { + if err := rows.Scan( + &i.ID, + &i.Name, + &i.OrganizationID, + &i.AvatarURL, + ); err != nil { return nil, err } items = append(items, i) @@ -1012,7 +1027,7 @@ func (q *sqlQuerier) GetGroupsByOrganizationID(ctx context.Context, organization const getUserGroups = `-- name: GetUserGroups :many SELECT - groups.id, groups.name, groups.organization_id + groups.id, groups.name, groups.organization_id, groups.avatar_url FROM groups JOIN @@ -1032,7 +1047,12 @@ func (q *sqlQuerier) GetUserGroups(ctx context.Context, userID uuid.UUID) ([]Gro var items []Group for rows.Next() { var i Group - if err := rows.Scan(&i.ID, &i.Name, &i.OrganizationID); err != nil { + if err := rows.Scan( + &i.ID, + &i.Name, + &i.OrganizationID, + &i.AvatarURL, + ); err != nil { return nil, err } items = append(items, i) @@ -1053,7 +1073,7 @@ INSERT INTO groups ( organization_id ) VALUES - ( $1, 'Everyone', $1) RETURNING id, name, organization_id + ( $1, 'Everyone', $1) RETURNING id, name, organization_id, avatar_url ` // We use the organization_id as the id @@ -1062,7 +1082,12 @@ VALUES func (q *sqlQuerier) InsertAllUsersGroup(ctx context.Context, organizationID uuid.UUID) (Group, error) { row := q.db.QueryRowContext(ctx, insertAllUsersGroup, organizationID) var i Group - err := row.Scan(&i.ID, &i.Name, &i.OrganizationID) + err := row.Scan( + &i.ID, + &i.Name, + &i.OrganizationID, + &i.AvatarURL, + ) return i, err } @@ -1070,22 +1095,34 @@ const insertGroup = `-- name: InsertGroup :one INSERT INTO groups ( id, name, - organization_id + organization_id, + avatar_url ) VALUES - ( $1, $2, $3) RETURNING id, name, organization_id + ( $1, $2, $3, $4) RETURNING id, name, organization_id, avatar_url ` type InsertGroupParams struct { ID uuid.UUID `db:"id" json:"id"` Name string `db:"name" json:"name"` OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + AvatarURL string `db:"avatar_url" json:"avatar_url"` } func (q *sqlQuerier) InsertGroup(ctx context.Context, arg InsertGroupParams) (Group, error) { - row := q.db.QueryRowContext(ctx, insertGroup, arg.ID, arg.Name, arg.OrganizationID) + row := q.db.QueryRowContext(ctx, insertGroup, + arg.ID, + arg.Name, + arg.OrganizationID, + arg.AvatarURL, + ) var i Group - err := row.Scan(&i.ID, &i.Name, &i.OrganizationID) + err := row.Scan( + &i.ID, + &i.Name, + &i.OrganizationID, + &i.AvatarURL, + ) return i, err } @@ -1111,21 +1148,28 @@ const updateGroupByID = `-- name: UpdateGroupByID :one UPDATE groups SET - name = $1 + name = $1, + avatar_url = $2 WHERE - id = $2 -RETURNING id, name, organization_id + id = $3 +RETURNING id, name, organization_id, avatar_url ` type UpdateGroupByIDParams struct { - Name string `db:"name" json:"name"` - ID uuid.UUID `db:"id" json:"id"` + Name string `db:"name" json:"name"` + AvatarURL string `db:"avatar_url" json:"avatar_url"` + ID uuid.UUID `db:"id" json:"id"` } func (q *sqlQuerier) UpdateGroupByID(ctx context.Context, arg UpdateGroupByIDParams) (Group, error) { - row := q.db.QueryRowContext(ctx, updateGroupByID, arg.Name, arg.ID) + row := q.db.QueryRowContext(ctx, updateGroupByID, arg.Name, arg.AvatarURL, arg.ID) var i Group - err := row.Scan(&i.ID, &i.Name, &i.OrganizationID) + err := row.Scan( + &i.ID, + &i.Name, + &i.OrganizationID, + &i.AvatarURL, + ) return i, err } diff --git a/coderd/database/queries/groups.sql b/coderd/database/queries/groups.sql index 137bf3040a127..45c1b8d03c405 100644 --- a/coderd/database/queries/groups.sql +++ b/coderd/database/queries/groups.sql @@ -74,10 +74,11 @@ AND INSERT INTO groups ( id, name, - organization_id + organization_id, + avatar_url ) VALUES - ( $1, $2, $3) RETURNING *; + ( $1, $2, $3, $4) RETURNING *; -- We use the organization_id as the id -- for simplicity since all users is @@ -95,9 +96,10 @@ VALUES UPDATE groups SET - name = $1 + name = $1, + avatar_url = $2 WHERE - id = $2 + id = $3 RETURNING *; -- name: InsertGroupMember :exec diff --git a/codersdk/groups.go b/codersdk/groups.go index b4b9759a0295d..a84f8560b2440 100644 --- a/codersdk/groups.go +++ b/codersdk/groups.go @@ -11,7 +11,8 @@ import ( ) type CreateGroupRequest struct { - Name string `json:"name"` + Name string `json:"name"` + AvatarURL string `json:"avatar_url"` } type Group struct { @@ -19,6 +20,7 @@ type Group struct { Name string `json:"name"` OrganizationID uuid.UUID `json:"organization_id"` Members []User `json:"members"` + AvatarURL string `json:"avatar_url"` } func (c *Client) CreateGroup(ctx context.Context, orgID uuid.UUID, req CreateGroupRequest) (Group, error) { @@ -77,6 +79,7 @@ type PatchGroupRequest struct { AddUsers []string `json:"add_users"` RemoveUsers []string `json:"remove_users"` Name string `json:"name"` + AvatarURL *string `json:"avatar_url"` } func (c *Client) PatchGroup(ctx context.Context, group uuid.UUID, req PatchGroupRequest) (Group, error) { diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index 4c81c4a5efaf9..3874ade2fe498 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -43,6 +43,7 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request) ID: uuid.New(), Name: req.Name, OrganizationID: org.ID, + AvatarURL: req.AvatarURL, }) if database.IsUniqueViolation(err) { httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ @@ -123,16 +124,28 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { } err := api.Database.InTx(func(tx database.Store) error { + group, err := tx.GetGroupByID(ctx, group.ID) + if err != nil { + return xerrors.Errorf("get group by ID: %w", err) + } + + // TODO: Do we care about validating this? + if req.AvatarURL != nil { + group.AvatarURL = *req.AvatarURL + } if req.Name != "" { - var err error - group, err = tx.UpdateGroupByID(ctx, database.UpdateGroupByIDParams{ - ID: group.ID, - Name: req.Name, - }) - if err != nil { - return xerrors.Errorf("update group by ID: %w", err) - } + group.Name = req.Name + } + + group, err = tx.UpdateGroupByID(ctx, database.UpdateGroupByIDParams{ + ID: group.ID, + Name: req.Name, + AvatarURL: group.AvatarURL, + }) + if err != nil { + return xerrors.Errorf("update group by ID: %w", err) } + for _, id := range req.AddUsers { err := tx.InsertGroupMember(ctx, database.InsertGroupMemberParams{ GroupID: group.ID, @@ -276,6 +289,7 @@ func convertGroup(g database.Group, users []database.User) codersdk.Group { ID: g.ID, Name: g.Name, OrganizationID: g.OrganizationID, + AvatarURL: g.AvatarURL, Members: convertUsers(users, orgs), } } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 92088b978f77a..339421d845f4c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -168,6 +168,7 @@ export interface CreateFirstUserResponse { // From codersdk/groups.go export interface CreateGroupRequest { readonly name: string + readonly avatar_url: string } // From codersdk/users.go @@ -368,6 +369,7 @@ export interface Group { readonly name: string readonly organization_id: string readonly members: User[] + readonly avatar_url: string } // From codersdk/workspaceapps.go @@ -483,6 +485,7 @@ export interface PatchGroupRequest { readonly add_users: string[] readonly remove_users: string[] readonly name: string + readonly avatar_url?: string } // From codersdk/provisionerdaemons.go diff --git a/site/src/components/GroupAvatar/GroupAvatar.stories.tsx b/site/src/components/GroupAvatar/GroupAvatar.stories.tsx index 315a179c3ca28..8fda557362fce 100644 --- a/site/src/components/GroupAvatar/GroupAvatar.stories.tsx +++ b/site/src/components/GroupAvatar/GroupAvatar.stories.tsx @@ -11,4 +11,5 @@ const Template: Story = (args) => export const Example = Template.bind({}) Example.args = { name: "My Group", + avatarURL: "", } diff --git a/site/src/components/GroupAvatar/GroupAvatar.tsx b/site/src/components/GroupAvatar/GroupAvatar.tsx index 6f3b043db080d..86ab501a24136 100644 --- a/site/src/components/GroupAvatar/GroupAvatar.tsx +++ b/site/src/components/GroupAvatar/GroupAvatar.tsx @@ -25,9 +25,10 @@ const StyledBadge = withStyles((theme) => ({ export type GroupAvatarProps = { name: string + avatarURL?: string } -export const GroupAvatar: FC = ({ name }) => { +export const GroupAvatar: FC = ({ name, avatarURL }) => { return ( = ({ name }) => { }} badgeContent={} > - {firstLetter(name)} + + {avatarURL ? ( + {`${name}'s + ) : ( + firstLetter(name) + )} + ) } diff --git a/site/src/pages/GroupsPage/CreateGroupPageView.tsx b/site/src/pages/GroupsPage/CreateGroupPageView.tsx index 8fd863815c59f..7bb2077afe0d9 100644 --- a/site/src/pages/GroupsPage/CreateGroupPageView.tsx +++ b/site/src/pages/GroupsPage/CreateGroupPageView.tsx @@ -28,6 +28,7 @@ export const CreateGroupPageView: React.FC = ({ const form = useFormik({ initialValues: { name: "", + avatar_url: "", }, validationSchema, onSubmit, @@ -48,6 +49,15 @@ export const CreateGroupPageView: React.FC = ({ label="Name" variant="outlined" /> + diff --git a/site/src/pages/GroupsPage/GroupsPageView.tsx b/site/src/pages/GroupsPage/GroupsPageView.tsx index 83c090fda9bb6..19bc2a88517ae 100644 --- a/site/src/pages/GroupsPage/GroupsPageView.tsx +++ b/site/src/pages/GroupsPage/GroupsPageView.tsx @@ -136,7 +136,12 @@ export const GroupsPageView: React.FC = ({ > } + avatar={ + + } title={group.name} subtitle={`${group.members.length} members`} highlightTitle diff --git a/site/src/pages/GroupsPage/SettingsGroupPageView.tsx b/site/src/pages/GroupsPage/SettingsGroupPageView.tsx index c1b63915345ad..55c59ccd93fc9 100644 --- a/site/src/pages/GroupsPage/SettingsGroupPageView.tsx +++ b/site/src/pages/GroupsPage/SettingsGroupPageView.tsx @@ -12,6 +12,7 @@ import * as Yup from "yup" type FormData = { name: string + avatar_url: string } const validationSchema = Yup.object({ @@ -28,6 +29,7 @@ const UpdateGroupForm: React.FC<{ const form = useFormik({ initialValues: { name: group.name, + avatar_url: group.avatar_url, }, validationSchema, onSubmit, @@ -46,6 +48,14 @@ const UpdateGroupForm: React.FC<{ label="Name" variant="outlined" /> + diff --git a/site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx b/site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx index 0188f0de11a2f..d9c4c74001beb 100644 --- a/site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx +++ b/site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx @@ -241,7 +241,12 @@ export const TemplatePermissionsPageView: FC< } + avatar={ + + } title={group.name} subtitle={getGroupSubtitle(group)} highlightTitle diff --git a/site/src/xServices/groups/editGroupXService.ts b/site/src/xServices/groups/editGroupXService.ts index 78d4b1a14370b..ae06e937998b1 100644 --- a/site/src/xServices/groups/editGroupXService.ts +++ b/site/src/xServices/groups/editGroupXService.ts @@ -29,7 +29,7 @@ export const editGroupMachine = createMachine( }, events: {} as { type: "UPDATE" - data: { name: string } + data: { name: string; avatar_url: string } }, }, tsTypes: {} as import("./editGroupXService.typegen").Typegen0, From e682e134a98698d8fc86b0f4377728c254b6414f Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 00:08:12 +0000 Subject: [PATCH 02/14] fix issue where update doesn't work when submitting the existing name --- enterprise/coderd/groups.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index 3874ade2fe498..bf5964465067b 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -111,15 +111,21 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { } } if req.Name != "" { - _, err := api.Database.GetGroupByOrgAndName(ctx, database.GetGroupByOrgAndNameParams{ + existingGroup, err := api.Database.GetGroupByOrgAndName(ctx, database.GetGroupByOrgAndNameParams{ OrganizationID: group.OrganizationID, Name: req.Name, }) if err == nil { - httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ - Message: fmt.Sprintf("A group with name %q already exists.", req.Name), - }) - return + // We may have just queried ourself. This should really + // go in the tx. + if existingGroup.ID != group.ID { + httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ + Message: fmt.Sprintf("A group with name %q already exists.", req.Name), + }) + return + } + // If we queried ourself then we don't want to update the name. + req.Name = "" } } @@ -139,7 +145,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { group, err = tx.UpdateGroupByID(ctx, database.UpdateGroupByIDParams{ ID: group.ID, - Name: req.Name, + Name: group.Name, AvatarURL: group.AvatarURL, }) if err != nil { From 8d1393cec738034f1a548a9bdd7da2aa5dd1d4df Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 00:24:39 +0000 Subject: [PATCH 03/14] add some tests --- coderd/database/databasefake/databasefake.go | 2 + enterprise/coderd/groups.go | 3 +- enterprise/coderd/groups_test.go | 70 ++++++++++++++++++-- 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 5a652164d3a6f..301ba84962164 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -2701,6 +2701,7 @@ func (q *fakeQuerier) UpdateGroupByID(_ context.Context, arg database.UpdateGrou for i, group := range q.groups { if group.ID == arg.ID { group.Name = arg.Name + group.AvatarURL = arg.AvatarURL q.groups[i] = group return group, nil } @@ -3037,6 +3038,7 @@ func (q *fakeQuerier) InsertGroup(_ context.Context, arg database.InsertGroupPar ID: arg.ID, Name: arg.Name, OrganizationID: arg.OrganizationID, + AvatarURL: arg.AvatarURL, } q.groups = append(q.groups, group) diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index bf5964465067b..9019f7ca565c4 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -130,7 +130,8 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { } err := api.Database.InTx(func(tx database.Store) error { - group, err := tx.GetGroupByID(ctx, group.ID) + var err error + group, err = tx.GetGroupByID(ctx, group.ID) if err != nil { return xerrors.Errorf("get group by ID: %w", err) } diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 2661da6bcc29f..a23c563b1f563 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -6,6 +6,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" + "k8s.io/utils/pointer" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/database" @@ -28,10 +29,12 @@ func TestCreateGroup(t *testing.T) { }) ctx, _ := testutil.Context(t) group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ - Name: "hi", + Name: "hi", + AvatarURL: "https://example.com", }) require.NoError(t, err) require.Equal(t, "hi", group.Name) + require.Equal(t, "https://example.com", group.AvatarURL) require.Empty(t, group.Members) require.NotEqual(t, uuid.Nil.String(), group.ID.String()) }) @@ -83,7 +86,7 @@ func TestCreateGroup(t *testing.T) { func TestPatchGroup(t *testing.T) { t.Parallel() - t.Run("Name", func(t *testing.T) { + t.Run("OK", func(t *testing.T) { t.Parallel() client := coderdenttest.New(t, nil) @@ -94,15 +97,43 @@ func TestPatchGroup(t *testing.T) { }) ctx, _ := testutil.Context(t) group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ - Name: "hi", + Name: "hi", + AvatarURL: "https://example.com", }) require.NoError(t, err) group, err = client.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{ - Name: "bye", + Name: "bye", + AvatarURL: pointer.String("https://google.com"), }) require.NoError(t, err) require.Equal(t, "bye", group.Name) + require.Equal(t, "https://google.com", group.AvatarURL) + }) + + // The FE sends a request from the edit page where the old name == new name. + // This should pass since it's not really an error to update a group name + // to itself. + t.Run("SameNameOK", func(t *testing.T) { + t.Parallel() + + client := coderdenttest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + + _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + TemplateRBACEnabled: true, + }) + ctx, _ := testutil.Context(t) + group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ + Name: "hi", + }) + require.NoError(t, err) + + group, err = client.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{ + Name: "hi", + }) + require.NoError(t, err) + require.Equal(t, "hi", group.Name) }) t.Run("AddUsers", func(t *testing.T) { @@ -166,6 +197,37 @@ func TestPatchGroup(t *testing.T) { require.Contains(t, group.Members, user4) }) + t.Run("NameConflict", func(t *testing.T) { + t.Parallel() + + client := coderdenttest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + + _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ + TemplateRBACEnabled: true, + }) + ctx, _ := testutil.Context(t) + group1, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ + Name: "hi", + AvatarURL: "https://example.com", + }) + require.NoError(t, err) + + group2, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ + Name: "bye", + }) + require.NoError(t, err) + + group1, err = client.PatchGroup(ctx, group1.ID, codersdk.PatchGroupRequest{ + Name: group2.Name, + AvatarURL: pointer.String("https://google.com"), + }) + require.Error(t, err) + cerr, ok := codersdk.AsError(err) + require.True(t, ok) + require.Equal(t, http.StatusConflict, cerr.StatusCode()) + }) + t.Run("UserNotExist", func(t *testing.T) { t.Parallel() From 223182f7bc7050a255f931d35ce32c5d4fe577d1 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 00:29:25 +0000 Subject: [PATCH 04/14] fix some dumb shit --- enterprise/coderd/groups.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index 9019f7ca565c4..7c1cb907062d9 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -82,6 +82,12 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { return } + // If the name matches the existing group name pretend we aren't + // updating the name at all. + if req.Name == group.Name { + req.Name = "" + } + users := make([]string, 0, len(req.AddUsers)+len(req.RemoveUsers)) users = append(users, req.AddUsers...) users = append(users, req.RemoveUsers...) @@ -110,22 +116,16 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { return } } - if req.Name != "" { - existingGroup, err := api.Database.GetGroupByOrgAndName(ctx, database.GetGroupByOrgAndNameParams{ + if req.Name != "" && req.Name != group.Name { + _, err := api.Database.GetGroupByOrgAndName(ctx, database.GetGroupByOrgAndNameParams{ OrganizationID: group.OrganizationID, Name: req.Name, }) if err == nil { - // We may have just queried ourself. This should really - // go in the tx. - if existingGroup.ID != group.ID { - httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ - Message: fmt.Sprintf("A group with name %q already exists.", req.Name), - }) - return - } - // If we queried ourself then we don't want to update the name. - req.Name = "" + httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ + Message: fmt.Sprintf("A group with name %q already exists.", req.Name), + }) + return } } From ef0e7d5cadbbd62cf81115bc90d530bebb1b8109 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 00:34:22 +0000 Subject: [PATCH 05/14] fix test/js --- site/src/testHelpers/entities.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index fe779f3789bdf..3f5f435676af6 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -914,6 +914,7 @@ export const MockWorkspaceQuota: TypesGen.WorkspaceQuota = { export const MockGroup: TypesGen.Group = { id: "fbd2116a-8961-4954-87ae-e4575bd29ce0", name: "Front-End", + avatar_url: "https://example.com", organization_id: MockOrganization.id, members: [MockUser, MockUser2], } From 6728e8c7aff320e5c931ae69d92984bfe78bd82b Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 00:39:30 +0000 Subject: [PATCH 06/14] fix test/js AGAIN --- site/src/util/groups.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/site/src/util/groups.ts b/site/src/util/groups.ts index 6f6f211a41cee..a5140bb7b4762 100644 --- a/site/src/util/groups.ts +++ b/site/src/util/groups.ts @@ -5,6 +5,7 @@ export const everyOneGroup = (organizationId: string): Group => ({ name: "Everyone", organization_id: organizationId, members: [], + avatar_url: "", }) export const getGroupSubtitle = (group: Group): string => { From c26ae3a662a68b70e99c4a875a07516e580ebfd3 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 00:43:29 +0000 Subject: [PATCH 07/14] don't autofocus --- site/src/pages/GroupsPage/CreateGroupPageView.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/pages/GroupsPage/CreateGroupPageView.tsx b/site/src/pages/GroupsPage/CreateGroupPageView.tsx index 7bb2077afe0d9..44e87e3d53b74 100644 --- a/site/src/pages/GroupsPage/CreateGroupPageView.tsx +++ b/site/src/pages/GroupsPage/CreateGroupPageView.tsx @@ -53,7 +53,6 @@ export const CreateGroupPageView: React.FC = ({ {...getFieldHelpers("avatar_url")} onChange={onChangeTrimmed(form)} autoComplete="avatar url" - autoFocus fullWidth label="Avatar URL" variant="outlined" From 76dbe3c0a8565c6bbe29db283694d3bb12489fb6 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 13 Oct 2022 23:08:49 +0000 Subject: [PATCH 08/14] update migration file --- ...00059_group_avatars.down.sql => 000060_group_avatars.down.sql} | 0 .../{000059_group_avatars.up.sql => 000060_group_avatars.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000059_group_avatars.down.sql => 000060_group_avatars.down.sql} (100%) rename coderd/database/migrations/{000059_group_avatars.up.sql => 000060_group_avatars.up.sql} (100%) diff --git a/coderd/database/migrations/000059_group_avatars.down.sql b/coderd/database/migrations/000060_group_avatars.down.sql similarity index 100% rename from coderd/database/migrations/000059_group_avatars.down.sql rename to coderd/database/migrations/000060_group_avatars.down.sql diff --git a/coderd/database/migrations/000059_group_avatars.up.sql b/coderd/database/migrations/000060_group_avatars.up.sql similarity index 100% rename from coderd/database/migrations/000059_group_avatars.up.sql rename to coderd/database/migrations/000060_group_avatars.up.sql From cf1243b34047dede16c163e1be6154bb19b59751 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Fri, 14 Oct 2022 00:23:06 +0000 Subject: [PATCH 09/14] use a better alt for avatars --- site/src/components/GroupAvatar/GroupAvatar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/GroupAvatar/GroupAvatar.tsx b/site/src/components/GroupAvatar/GroupAvatar.tsx index 86ab501a24136..1a1dfd581a656 100644 --- a/site/src/components/GroupAvatar/GroupAvatar.tsx +++ b/site/src/components/GroupAvatar/GroupAvatar.tsx @@ -40,7 +40,7 @@ export const GroupAvatar: FC = ({ name, avatarURL }) => { > {avatarURL ? ( - {`${name}'s + {firstLetter(name)} ) : ( firstLetter(name) )} From e3af511b64c991c93e3677f695ec1c6d8274a6e3 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Fri, 14 Oct 2022 02:39:48 +0000 Subject: [PATCH 10/14] use avatar attr --- site/src/components/GroupAvatar/GroupAvatar.tsx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/site/src/components/GroupAvatar/GroupAvatar.tsx b/site/src/components/GroupAvatar/GroupAvatar.tsx index 1a1dfd581a656..75d01e97638b2 100644 --- a/site/src/components/GroupAvatar/GroupAvatar.tsx +++ b/site/src/components/GroupAvatar/GroupAvatar.tsx @@ -38,13 +38,7 @@ export const GroupAvatar: FC = ({ name, avatarURL }) => { }} badgeContent={} > - - {avatarURL ? ( - {firstLetter(name)} - ) : ( - firstLetter(name) - )} - + ) } From 867bae9492c07b81b5a3a2df63cf2aaed7262083 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Fri, 14 Oct 2022 03:11:38 +0000 Subject: [PATCH 11/14] use firstletter --- site/src/components/GroupAvatar/GroupAvatar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/GroupAvatar/GroupAvatar.tsx b/site/src/components/GroupAvatar/GroupAvatar.tsx index 75d01e97638b2..ff2428c61e163 100644 --- a/site/src/components/GroupAvatar/GroupAvatar.tsx +++ b/site/src/components/GroupAvatar/GroupAvatar.tsx @@ -38,7 +38,7 @@ export const GroupAvatar: FC = ({ name, avatarURL }) => { }} badgeContent={} > - + ) } From b622487a23dd70405702c2601725eaaf97366e33 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Mon, 17 Oct 2022 20:41:50 +0000 Subject: [PATCH 12/14] update migration file --- ...00060_group_avatars.down.sql => 000062_group_avatars.down.sql} | 0 .../{000060_group_avatars.up.sql => 000062_group_avatars.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000060_group_avatars.down.sql => 000062_group_avatars.down.sql} (100%) rename coderd/database/migrations/{000060_group_avatars.up.sql => 000062_group_avatars.up.sql} (100%) diff --git a/coderd/database/migrations/000060_group_avatars.down.sql b/coderd/database/migrations/000062_group_avatars.down.sql similarity index 100% rename from coderd/database/migrations/000060_group_avatars.down.sql rename to coderd/database/migrations/000062_group_avatars.down.sql diff --git a/coderd/database/migrations/000060_group_avatars.up.sql b/coderd/database/migrations/000062_group_avatars.up.sql similarity index 100% rename from coderd/database/migrations/000060_group_avatars.up.sql rename to coderd/database/migrations/000062_group_avatars.up.sql From 16c0ed6788cec605cf9d59e2ea992c8d1dcd4422 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Mon, 17 Oct 2022 21:39:12 +0000 Subject: [PATCH 13/14] fix a little avatar bug --- site/src/components/GroupAvatar/GroupAvatar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/GroupAvatar/GroupAvatar.tsx b/site/src/components/GroupAvatar/GroupAvatar.tsx index ff2428c61e163..ab9762050ab27 100644 --- a/site/src/components/GroupAvatar/GroupAvatar.tsx +++ b/site/src/components/GroupAvatar/GroupAvatar.tsx @@ -38,7 +38,7 @@ export const GroupAvatar: FC = ({ name, avatarURL }) => { }} badgeContent={} > - + {firstLetter(name)} ) } From 5e66b1586b29352491fdd2d891150c6cb24764a8 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Mon, 17 Oct 2022 21:41:47 +0000 Subject: [PATCH 14/14] fix merge conflict --- enterprise/coderd/groups_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 51f6c9f69d6d0..8019b956d4b04 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -121,7 +121,7 @@ func TestPatchGroup(t *testing.T) { user := coderdtest.CreateFirstUser(t, client) _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ - TemplateRBACEnabled: true, + TemplateRBAC: true, }) ctx, _ := testutil.Context(t) group, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{ @@ -204,7 +204,7 @@ func TestPatchGroup(t *testing.T) { user := coderdtest.CreateFirstUser(t, client) _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ - TemplateRBACEnabled: true, + TemplateRBAC: true, }) ctx, _ := testutil.Context(t) group1, err := client.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{