feat: add avatar urls to groups (#4525)

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)

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 (

coderd/database/migrations/000062_group_avatars.down.sql

@@ -0,0 +1,5 @@
+BEGIN;
+
+ALTER TABLE groups DROP COLUMN avatar_url;
+
+COMMIT;

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;

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 {

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
+	id = $3
-RETURNING id, name, organization_id
+RETURNING id, name, organization_id, avatar_url
 `
 
 type UpdateGroupByIDParams struct {
-	Name string    `db:"name" json:"name"`
+	Name      string    `db:"name" json:"name"`
-	ID   uuid.UUID `db:"id" json:"id"`
+	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
 }
 

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

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) {

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 {
-		if req.Name != "" {
+		var err error
-			var err error
+		group, err = tx.GetGroupByID(ctx, group.ID)
-			group, err = tx.UpdateGroupByID(ctx, database.UpdateGroupByIDParams{
+		if err != nil {
-				ID:   group.ID,
+			return xerrors.Errorf("get group by ID: %w", err)
-				Name: req.Name,
-			})
-			if err != nil {
-				return xerrors.Errorf("update group by ID: %w", err)
-			}
 		}
+
+		// TODO: Do we care about validating this?
+		if req.AvatarURL != nil {
+			group.AvatarURL = *req.AvatarURL
+		}
+		if req.Name != "" {
+			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),
 	}
 }

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,35 @@ 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)
+		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",
+			AvatarURL: "https://example.com",
+		})
+		require.NoError(t, err)
+
+		group, err = client.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{
+			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)
@@ -99,10 +130,10 @@ func TestPatchGroup(t *testing.T) {
 		require.NoError(t, err)
 
 		group, err = client.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{
-			Name: "bye",
+			Name: "hi",
 		})
 		require.NoError(t, err)
-		require.Equal(t, "bye", group.Name)
+		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()
 

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

site/src/components/GroupAvatar/GroupAvatar.stories.tsx

@@ -11,4 +11,5 @@ const Template: Story<GroupAvatarProps> = (args) => <GroupAvatar {...args} />
 export const Example = Template.bind({})
 Example.args = {
   name: "My Group",
+  avatarURL: "",
 }

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<GroupAvatarProps> = ({ name }) => {
+export const GroupAvatar: FC<GroupAvatarProps> = ({ name, avatarURL }) => {
   return (
     <StyledBadge
       overlap="circular"
@@ -37,7 +38,7 @@ export const GroupAvatar: FC<GroupAvatarProps> = ({ name }) => {
       }}
       badgeContent={<Group />}
     >
-      <Avatar>{firstLetter(name)}</Avatar>
+      <Avatar src={avatarURL}>{firstLetter(name)}</Avatar>
     </StyledBadge>
   )
 }

site/src/pages/GroupsPage/CreateGroupPageView.tsx

@@ -28,6 +28,7 @@ export const CreateGroupPageView: React.FC<CreateGroupPageViewProps> = ({
   const form = useFormik<CreateGroupRequest>({
     initialValues: {
       name: "",
+      avatar_url: "",
     },
     validationSchema,
     onSubmit,
@@ -48,6 +49,14 @@ export const CreateGroupPageView: React.FC<CreateGroupPageViewProps> = ({
             label="Name"
             variant="outlined"
           />
+          <TextField
+            {...getFieldHelpers("avatar_url")}
+            onChange={onChangeTrimmed(form)}
+            autoComplete="avatar url"
+            fullWidth
+            label="Avatar URL"
+            variant="outlined"
+          />
           <FormFooter onCancel={onCancel} isLoading={isLoading} />
         </form>
       </FullPageForm>

site/src/pages/GroupsPage/GroupsPageView.tsx

@@ -136,7 +136,12 @@ export const GroupsPageView: React.FC<GroupsPageViewProps> = ({
                         >
                           <TableCell>
                             <AvatarData
-                              avatar={<GroupAvatar name={group.name} />}
+                              avatar={
+                                <GroupAvatar
+                                  name={group.name}
+                                  avatarURL={group.avatar_url}
+                                />
+                              }
                               title={group.name}
                               subtitle={`${group.members.length} members`}
                               highlightTitle

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<FormData>({
     initialValues: {
       name: group.name,
+      avatar_url: group.avatar_url,
     },
     validationSchema,
     onSubmit,
@@ -46,6 +48,14 @@ const UpdateGroupForm: React.FC<{
           label="Name"
           variant="outlined"
         />
+        <TextField
+          {...getFieldHelpers("avatar_url")}
+          onChange={onChangeTrimmed(form)}
+          autoFocus
+          fullWidth
+          label="Avatar URL"
+          variant="outlined"
+        />
         <FormFooter onCancel={onCancel} isLoading={isLoading} />
       </form>
     </FullPageForm>

site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx

@@ -241,7 +241,12 @@ export const TemplatePermissionsPageView: FC<
                   <TableRow key={group.id}>
                     <TableCell>
                       <AvatarData
-                        avatar={<GroupAvatar name={group.name} />}
+                        avatar={
+                          <GroupAvatar
+                            name={group.name}
+                            avatarURL={group.avatar_url}
+                          />
+                        }
                         title={group.name}
                         subtitle={getGroupSubtitle(group)}
                         highlightTitle

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],
 }

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 => {

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,