diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 65043d2412302..757616774c6c7 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -2784,6 +2784,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 } @@ -3135,6 +3136,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/coderd/database/dump.sql b/coderd/database/dump.sql index b946a1130e0c8..36e1bd583a15c 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -181,7 +181,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/000062_group_avatars.down.sql b/coderd/database/migrations/000062_group_avatars.down.sql new file mode 100644 index 0000000000000..eb15f354383fc --- /dev/null +++ b/coderd/database/migrations/000062_group_avatars.down.sql @@ -0,0 +1,5 @@ +BEGIN; + +ALTER TABLE groups DROP COLUMN avatar_url; + +COMMIT; diff --git a/coderd/database/migrations/000062_group_avatars.up.sql b/coderd/database/migrations/000062_group_avatars.up.sql new file mode 100644 index 0000000000000..b7f033874ba68 --- /dev/null +++ b/coderd/database/migrations/000062_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 53e074984ac11..7e398552de93e 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -439,6 +439,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 3621050bc0096..cb4b43591a41e 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -928,7 +928,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 @@ -940,13 +940,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 @@ -965,7 +970,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 } @@ -1024,7 +1034,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 @@ -1042,7 +1052,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) @@ -1058,7 +1073,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 @@ -1078,7 +1093,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) @@ -1099,7 +1119,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 @@ -1108,7 +1128,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 } @@ -1116,22 +1141,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 } @@ -1157,21 +1194,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..7c1cb907062d9 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{ @@ -81,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...) @@ -109,7 +116,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { return } } - if req.Name != "" { + if req.Name != "" && req.Name != group.Name { _, err := api.Database.GetGroupByOrgAndName(ctx, database.GetGroupByOrgAndNameParams{ OrganizationID: group.OrganizationID, Name: req.Name, @@ -123,16 +130,29 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { } err := api.Database.InTx(func(tx database.Store) error { + var err 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: group.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 +296,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/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index eae51b0dfdc3f..8019b956d4b04 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{ + TemplateRBAC: 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{ + TemplateRBAC: 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() diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d6305b3dbc8c0..0a709f8927a2a 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -169,6 +169,7 @@ export interface CreateFirstUserResponse { // From codersdk/groups.go export interface CreateGroupRequest { readonly name: string + readonly avatar_url: string } // From codersdk/users.go @@ -376,6 +377,7 @@ export interface Group { readonly name: string readonly organization_id: string readonly members: User[] + readonly avatar_url: string } // From codersdk/workspaceapps.go @@ -491,6 +493,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..ab9762050ab27 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)} + {firstLetter(name)} ) } diff --git a/site/src/pages/GroupsPage/CreateGroupPageView.tsx b/site/src/pages/GroupsPage/CreateGroupPageView.tsx index 8fd863815c59f..44e87e3d53b74 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,14 @@ 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/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 8d0358bc5826e..f2c9836990495 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -929,6 +929,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], } 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 => { 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,