From c3a7b13690071b8f22da64483b56d4d1a56f3b27 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 21 Feb 2024 15:58:11 -0600 Subject: [PATCH] chore: remove organization requirement from convertGroup() (#12195) * feat: convertGroups() no longer requires organization info Removing role information from some users in the api. This info is excessive and not required. It is costly to always include --- cli/clitest/golden.go | 6 +- cli/cliui/output.go | 2 +- cli/cliui/table.go | 21 ++++- cli/cliui/table_test.go | 4 +- cli/root_test.go | 4 + cli/testdata/coder_users_list.golden | 3 + .../coder_users_list_--output_json.golden | 16 ++-- coderd/apidoc/docs.go | 58 +++++++++++- coderd/apidoc/swagger.json | 50 ++++++++++- coderd/audit.go | 18 ++-- coderd/database/db2sdk/db2sdk.go | 76 +++++++++++----- codersdk/groups.go | 16 ++-- codersdk/templates.go | 4 +- codersdk/users.go | 27 +++--- codersdk/workspaceproxy.go | 7 +- docs/api/enterprise.md | 64 -------------- docs/api/schemas.md | 88 +++++++++++-------- enterprise/coderd/groups.go | 69 ++------------- enterprise/coderd/groups_test.go | 39 ++++---- enterprise/coderd/templates.go | 10 +-- enterprise/coderd/userauth_test.go | 2 +- site/src/api/typesGenerated.ts | 27 +++--- site/src/pages/GroupsPage/GroupPage.tsx | 4 +- .../TemplatePermissionsPageView.tsx | 9 +- .../UserOrGroupAutocomplete.tsx | 4 +- 25 files changed, 350 insertions(+), 278 deletions(-) create mode 100644 cli/testdata/coder_users_list.golden diff --git a/cli/clitest/golden.go b/cli/clitest/golden.go index 2a3ad2dc60..cfd253613c 100644 --- a/cli/clitest/golden.go +++ b/cli/clitest/golden.go @@ -167,7 +167,11 @@ func prepareTestData(t *testing.T) (*codersdk.Client, map[string]string) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - db, pubsub := dbtestutil.NewDB(t) + // This needs to be a fixed timezone because timezones increase the length + // of timestamp strings. The increased length can pad table formatting's + // and differ the table header spacings. + //nolint:gocritic + db, pubsub := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC")) rootClient := coderdtest.New(t, &coderdtest.Options{ Database: db, Pubsub: pubsub, diff --git a/cli/cliui/output.go b/cli/cliui/output.go index 63a4d4ee5d..b78149572d 100644 --- a/cli/cliui/output.go +++ b/cli/cliui/output.go @@ -106,7 +106,7 @@ func TableFormat(out any, defaultColumns []string) OutputFormat { } // Get the list of table column headers. - headers, defaultSort, err := typeToTableHeaders(v.Type().Elem()) + headers, defaultSort, err := typeToTableHeaders(v.Type().Elem(), true) if err != nil { panic("parse table headers: " + err.Error()) } diff --git a/cli/cliui/table.go b/cli/cliui/table.go index b4b00e8759..9962678be9 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -70,7 +70,7 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error) } // Get the list of table column headers. - headersRaw, defaultSort, err := typeToTableHeaders(v.Type().Elem()) + headersRaw, defaultSort, err := typeToTableHeaders(v.Type().Elem(), true) if err != nil { return "", xerrors.Errorf("get table headers recursively for type %q: %w", v.Type().Elem().String(), err) } @@ -230,7 +230,11 @@ func isStructOrStructPointer(t reflect.Type) bool { // typeToTableHeaders converts a type to a slice of column names. If the given // type is invalid (not a struct or a pointer to a struct, has invalid table // tags, etc.), an error is returned. -func typeToTableHeaders(t reflect.Type) ([]string, string, error) { +// +// requireDefault is only needed for the root call. This is recursive, so nested +// structs do not need the default sort name. +// nolint:revive +func typeToTableHeaders(t reflect.Type, requireDefault bool) ([]string, string, error) { if !isStructOrStructPointer(t) { return nil, "", xerrors.Errorf("typeToTableHeaders called with a non-struct or a non-pointer-to-a-struct type") } @@ -246,6 +250,12 @@ func typeToTableHeaders(t reflect.Type) ([]string, string, error) { if err != nil { return nil, "", xerrors.Errorf("parse struct tags for field %q in type %q: %w", field.Name, t.String(), err) } + + if name == "" && (recursive && skip) { + return nil, "", xerrors.Errorf("a name is required for the field %q. "+ + "recursive_line will ensure this is never shown to the user, but is still needed", field.Name) + } + // If recurse and skip is set, the name is intentionally empty. if name == "" { continue } @@ -262,7 +272,7 @@ func typeToTableHeaders(t reflect.Type) ([]string, string, error) { return nil, "", xerrors.Errorf("field %q in type %q is marked as recursive but does not contain a struct or a pointer to a struct", field.Name, t.String()) } - childNames, _, err := typeToTableHeaders(fieldType) + childNames, defaultSort, err := typeToTableHeaders(fieldType, false) if err != nil { return nil, "", xerrors.Errorf("get child field header names for field %q in type %q: %w", field.Name, fieldType.String(), err) } @@ -273,13 +283,16 @@ func typeToTableHeaders(t reflect.Type) ([]string, string, error) { } headers = append(headers, fullName) } + if defaultSortName == "" { + defaultSortName = defaultSort + } continue } headers = append(headers, name) } - if defaultSortName == "" { + if defaultSortName == "" && requireDefault { return nil, "", xerrors.Errorf("no field marked as default_sort in type %q", t.String()) } diff --git a/cli/cliui/table_test.go b/cli/cliui/table_test.go index 32159abb9f..bb0b6c658f 100644 --- a/cli/cliui/table_test.go +++ b/cli/cliui/table_test.go @@ -46,12 +46,12 @@ type tableTest2 struct { type tableTest3 struct { NotIncluded string // no table tag - Sub tableTest2 `table:"inner,recursive,default_sort"` + Sub tableTest2 `table:"inner,recursive"` } type tableTest4 struct { Inline tableTest2 `table:"ignored,recursive_inline"` - SortField string `table:"sort_field,default_sort"` + SortField string `table:"sort_field"` } func Test_DisplayTable(t *testing.T) { diff --git a/cli/root_test.go b/cli/root_test.go index ff564e5858..3a5702d4c0 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -51,6 +51,10 @@ func TestCommandHelp(t *testing.T) { Name: "coder users list --output json", Cmd: []string{"users", "list", "--output", "json"}, }, + clitest.CommandHelpCase{ + Name: "coder users list", + Cmd: []string{"users", "list"}, + }, )) } diff --git a/cli/testdata/coder_users_list.golden b/cli/testdata/coder_users_list.golden new file mode 100644 index 0000000000..0d2f2e30c9 --- /dev/null +++ b/cli/testdata/coder_users_list.golden @@ -0,0 +1,3 @@ +USERNAME EMAIL CREATED AT STATUS +testuser testuser@coder.com [timestamp] active +testuser2 testuser2@coder.com [timestamp] dormant diff --git a/cli/testdata/coder_users_list_--output_json.golden b/cli/testdata/coder_users_list_--output_json.golden index 1ec8f37cb5..b62ce00992 100644 --- a/cli/testdata/coder_users_list_--output_json.golden +++ b/cli/testdata/coder_users_list_--output_json.golden @@ -2,11 +2,14 @@ { "id": "[first user ID]", "username": "testuser", + "avatar_url": "", "name": "", "email": "testuser@coder.com", "created_at": "[timestamp]", "last_seen_at": "[timestamp]", "status": "active", + "login_type": "password", + "theme_preference": "", "organization_ids": [ "[first org ID]" ], @@ -15,25 +18,22 @@ "name": "owner", "display_name": "Owner" } - ], - "avatar_url": "", - "login_type": "password", - "theme_preference": "" + ] }, { "id": "[second user ID]", "username": "testuser2", + "avatar_url": "", "name": "", "email": "testuser2@coder.com", "created_at": "[timestamp]", "last_seen_at": "[timestamp]", "status": "dormant", + "login_type": "password", + "theme_preference": "", "organization_ids": [ "[first org ID]" ], - "roles": [], - "avatar_url": "", - "login_type": "password", - "theme_preference": "" + "roles": [] } ] diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 1a520a5645..b733846963 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -8308,7 +8308,7 @@ const docTemplate = `{ "users": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.User" + "$ref": "#/definitions/codersdk.ReducedUser" } } } @@ -9946,7 +9946,7 @@ const docTemplate = `{ "members": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.User" + "$ref": "#/definitions/codersdk.ReducedUser" } }, "name": { @@ -10996,6 +10996,60 @@ const docTemplate = `{ } } }, + "codersdk.ReducedUser": { + "type": "object", + "required": [ + "created_at", + "email", + "id", + "username" + ], + "properties": { + "avatar_url": { + "type": "string", + "format": "uri" + }, + "created_at": { + "type": "string", + "format": "date-time" + }, + "email": { + "type": "string", + "format": "email" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "last_seen_at": { + "type": "string", + "format": "date-time" + }, + "login_type": { + "$ref": "#/definitions/codersdk.LoginType" + }, + "name": { + "type": "string" + }, + "status": { + "enum": [ + "active", + "suspended" + ], + "allOf": [ + { + "$ref": "#/definitions/codersdk.UserStatus" + } + ] + }, + "theme_preference": { + "type": "string" + }, + "username": { + "type": "string" + } + } + }, "codersdk.Region": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 35378c3cb0..a3f00910e7 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -7393,7 +7393,7 @@ "users": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.User" + "$ref": "#/definitions/codersdk.ReducedUser" } } } @@ -8939,7 +8939,7 @@ "members": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.User" + "$ref": "#/definitions/codersdk.ReducedUser" } }, "name": { @@ -9902,6 +9902,52 @@ } } }, + "codersdk.ReducedUser": { + "type": "object", + "required": ["created_at", "email", "id", "username"], + "properties": { + "avatar_url": { + "type": "string", + "format": "uri" + }, + "created_at": { + "type": "string", + "format": "date-time" + }, + "email": { + "type": "string", + "format": "email" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "last_seen_at": { + "type": "string", + "format": "date-time" + }, + "login_type": { + "$ref": "#/definitions/codersdk.LoginType" + }, + "name": { + "type": "string" + }, + "status": { + "enum": ["active", "suspended"], + "allOf": [ + { + "$ref": "#/definitions/codersdk.UserStatus" + } + ] + }, + "theme_preference": { + "type": "string" + }, + "username": { + "type": "string" + } + } + }, "codersdk.Region": { "type": "object", "properties": { diff --git a/coderd/audit.go b/coderd/audit.go index 00e4c520c3..d78fba6116 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -186,13 +186,17 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs if dblog.UserUsername.Valid { user = &codersdk.User{ - ID: dblog.UserID, - Username: dblog.UserUsername.String, - Email: dblog.UserEmail.String, - CreatedAt: dblog.UserCreatedAt.Time, - Status: codersdk.UserStatus(dblog.UserStatus.UserStatus), - Roles: []codersdk.Role{}, - AvatarURL: dblog.UserAvatarUrl.String, + ReducedUser: codersdk.ReducedUser{ + MinimalUser: codersdk.MinimalUser{ + ID: dblog.UserID, + Username: dblog.UserUsername.String, + AvatarURL: dblog.UserAvatarUrl.String, + }, + Email: dblog.UserEmail.String, + CreatedAt: dblog.UserCreatedAt.Time, + Status: codersdk.UserStatus(dblog.UserStatus.UserStatus), + }, + Roles: []codersdk.Role{}, } for _, roleName := range dblog.UserRoles { diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 5389f263ab..14a24e6631 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -24,6 +24,17 @@ import ( "github.com/coder/coder/v2/tailnet" ) +// List is a helper function to reduce boilerplate when converting slices of +// database types to slices of codersdk types. +// Only works if the function takes a single argument. +func List[F any, T any](list []F, convert func(F) T) []T { + into := make([]T, 0, len(list)) + for _, item := range list { + into = append(into, convert(item)) + } + return into +} + type ExternalAuthMeta struct { Authenticated bool ValidateError string @@ -49,14 +60,6 @@ func ExternalAuth(auth database.ExternalAuthLink, meta ExternalAuthMeta) codersd } } -func WorkspaceBuildParameters(params []database.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter { - out := make([]codersdk.WorkspaceBuildParameter, len(params)) - for i, p := range params { - out[i] = WorkspaceBuildParameter(p) - } - return out -} - func WorkspaceBuildParameter(p database.WorkspaceBuildParameter) codersdk.WorkspaceBuildParameter { return codersdk.WorkspaceBuildParameter{ Name: p.Name, @@ -64,6 +67,10 @@ func WorkspaceBuildParameter(p database.WorkspaceBuildParameter) codersdk.Worksp } } +func WorkspaceBuildParameters(params []database.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter { + return List(params, WorkspaceBuildParameter) +} + func TemplateVersionParameters(params []database.TemplateVersionParameter) ([]codersdk.TemplateVersionParameter, error) { out := make([]codersdk.TemplateVersionParameter, len(params)) var err error @@ -118,21 +125,33 @@ func TemplateVersionParameter(param database.TemplateVersionParameter) (codersdk }, nil } -func User(user database.User, organizationIDs []uuid.UUID) codersdk.User { - convertedUser := codersdk.User{ - ID: user.ID, +func ReducedUser(user database.User) codersdk.ReducedUser { + return codersdk.ReducedUser{ + MinimalUser: codersdk.MinimalUser{ + ID: user.ID, + Username: user.Username, + AvatarURL: user.AvatarURL, + }, Email: user.Email, Name: user.Name, CreatedAt: user.CreatedAt, LastSeenAt: user.LastSeenAt, - Username: user.Username, Status: codersdk.UserStatus(user.Status), - OrganizationIDs: organizationIDs, - Roles: make([]codersdk.Role, 0, len(user.RBACRoles)), - AvatarURL: user.AvatarURL, LoginType: codersdk.LoginType(user.LoginType), ThemePreference: user.ThemePreference, } +} + +func ReducedUsers(users []database.User) []codersdk.ReducedUser { + return List(users, ReducedUser) +} + +func User(user database.User, organizationIDs []uuid.UUID) codersdk.User { + convertedUser := codersdk.User{ + ReducedUser: ReducedUser(user), + OrganizationIDs: organizationIDs, + Roles: make([]codersdk.Role, 0, len(user.RBACRoles)), + } for _, roleName := range user.RBACRoles { rbacRole, _ := rbac.RoleByName(roleName) @@ -142,6 +161,25 @@ func User(user database.User, organizationIDs []uuid.UUID) codersdk.User { return convertedUser } +func Users(users []database.User, organizationIDs map[uuid.UUID][]uuid.UUID) []codersdk.User { + return List(users, func(user database.User) codersdk.User { + return User(user, organizationIDs[user.ID]) + }) +} + +func Group(group database.Group, members []database.User) codersdk.Group { + return codersdk.Group{ + ID: group.ID, + Name: group.Name, + DisplayName: group.DisplayName, + OrganizationID: group.OrganizationID, + AvatarURL: group.AvatarURL, + Members: ReducedUsers(members), + QuotaAllowance: int(group.QuotaAllowance), + Source: codersdk.GroupSource(group.Source), + } +} + func Role(role rbac.Role) codersdk.Role { return codersdk.Role{ DisplayName: role.DisplayName, @@ -248,11 +286,9 @@ func OAuth2ProviderApp(accessURL *url.URL, dbApp database.OAuth2ProviderApp) cod } func OAuth2ProviderApps(accessURL *url.URL, dbApps []database.OAuth2ProviderApp) []codersdk.OAuth2ProviderApp { - apps := []codersdk.OAuth2ProviderApp{} - for _, dbApp := range dbApps { - apps = append(apps, OAuth2ProviderApp(accessURL, dbApp)) - } - return apps + return List(dbApps, func(dbApp database.OAuth2ProviderApp) codersdk.OAuth2ProviderApp { + return OAuth2ProviderApp(accessURL, dbApp) + }) } func convertDisplayApps(apps []database.DisplayApp) []codersdk.DisplayApp { diff --git a/codersdk/groups.go b/codersdk/groups.go index 2796a776a9..eb76902b01 100644 --- a/codersdk/groups.go +++ b/codersdk/groups.go @@ -25,14 +25,14 @@ type CreateGroupRequest struct { } type Group struct { - ID uuid.UUID `json:"id" format:"uuid"` - Name string `json:"name"` - DisplayName string `json:"display_name"` - OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` - Members []User `json:"members"` - AvatarURL string `json:"avatar_url"` - QuotaAllowance int `json:"quota_allowance"` - Source GroupSource `json:"source"` + ID uuid.UUID `json:"id" format:"uuid"` + Name string `json:"name"` + DisplayName string `json:"display_name"` + OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` + Members []ReducedUser `json:"members"` + AvatarURL string `json:"avatar_url"` + QuotaAllowance int `json:"quota_allowance"` + Source GroupSource `json:"source"` } func (g Group) IsEveryone() bool { diff --git a/codersdk/templates.go b/codersdk/templates.go index c23c3cd46c..76fe9f4a4e 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -200,8 +200,8 @@ type UpdateTemplateACL struct { // ACLAvailable is a list of users and groups that can be added to a template // ACL. type ACLAvailable struct { - Users []User `json:"users"` - Groups []Group `json:"groups"` + Users []ReducedUser `json:"users"` + Groups []Group `json:"groups"` } type UpdateTemplateMeta struct { diff --git a/codersdk/users.go b/codersdk/users.go index 1bd8852cf4..b42eee38af 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -42,21 +42,28 @@ type MinimalUser struct { AvatarURL string `json:"avatar_url" format:"uri"` } +// ReducedUser omits role and organization information. Roles are deduced from +// the user's site and organization roles. This requires fetching the user's +// organizational memberships. Fetching that is more expensive, and not usually +// required by the frontend. +type ReducedUser struct { + MinimalUser `table:"m,recursive_inline"` + Name string `json:"name"` + Email string `json:"email" validate:"required" table:"email" format:"email"` + CreatedAt time.Time `json:"created_at" validate:"required" table:"created at" format:"date-time"` + LastSeenAt time.Time `json:"last_seen_at" format:"date-time"` + + Status UserStatus `json:"status" table:"status" enums:"active,suspended"` + LoginType LoginType `json:"login_type"` + ThemePreference string `json:"theme_preference"` +} + // User represents a user in Coder. type User struct { - ID uuid.UUID `json:"id" validate:"required" table:"id" format:"uuid"` - Username string `json:"username" validate:"required" table:"username,default_sort"` - Name string `json:"name"` - Email string `json:"email" validate:"required" table:"email" format:"email"` - CreatedAt time.Time `json:"created_at" validate:"required" table:"created at" format:"date-time"` - LastSeenAt time.Time `json:"last_seen_at" format:"date-time"` + ReducedUser `table:"r,recursive_inline"` - Status UserStatus `json:"status" table:"status" enums:"active,suspended"` OrganizationIDs []uuid.UUID `json:"organization_ids" format:"uuid"` Roles []Role `json:"roles"` - AvatarURL string `json:"avatar_url" format:"uri"` - LoginType LoginType `json:"login_type"` - ThemePreference string `json:"theme_preference"` } type GetUsersResponse struct { diff --git a/codersdk/workspaceproxy.go b/codersdk/workspaceproxy.go index 0340e2f23f..268486be53 100644 --- a/codersdk/workspaceproxy.go +++ b/codersdk/workspaceproxy.go @@ -56,7 +56,7 @@ type WorkspaceProxy struct { // and ready to use. Status WorkspaceProxyStatus `json:"status,omitempty" table:"proxy,recursive"` - CreatedAt time.Time `json:"created_at" format:"date-time" table:"created_at,default_sort"` + CreatedAt time.Time `json:"created_at" format:"date-time" table:"created_at"` UpdatedAt time.Time `json:"updated_at" format:"date-time" table:"updated_at"` Deleted bool `json:"deleted" table:"deleted"` Version string `json:"version" table:"version"` @@ -69,9 +69,8 @@ type CreateWorkspaceProxyRequest struct { } type UpdateWorkspaceProxyResponse struct { - Proxy WorkspaceProxy `json:"proxy" table:"proxy,recursive"` - // The recursive table sort is not working very well. - ProxyToken string `json:"proxy_token" table:"proxy token,default_sort"` + Proxy WorkspaceProxy `json:"proxy" table:"p,recursive_inline"` + ProxyToken string `json:"proxy_token" table:"proxy token"` } func (c *Client) CreateWorkspaceProxy(ctx context.Context, req CreateWorkspaceProxyRequest) (UpdateWorkspaceProxyResponse, error) { diff --git a/docs/api/enterprise.md b/docs/api/enterprise.md index 88b728421c..0b05a9fffe 100644 --- a/docs/api/enterprise.md +++ b/docs/api/enterprise.md @@ -189,13 +189,6 @@ curl -X GET http://coder-server:8080/api/v2/groups/{group} \ "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -253,13 +246,6 @@ curl -X DELETE http://coder-server:8080/api/v2/groups/{group} \ "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -332,13 +318,6 @@ curl -X PATCH http://coder-server:8080/api/v2/groups/{group} \ "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -1069,13 +1048,6 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -1113,10 +1085,6 @@ Status Code **200** | `»» last_seen_at` | string(date-time) | false | | | | `»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | | `»» name` | string | false | | | -| `»» organization_ids` | array | false | | | -| `»» roles` | array | false | | | -| `»»» display_name` | string | false | | | -| `»»» name` | string | false | | | | `»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | | `»» theme_preference` | string | false | | | | `»» username` | string | true | | | @@ -1192,13 +1160,6 @@ curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/groups "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -1257,13 +1218,6 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups/ "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -1837,13 +1791,6 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/acl/available \ "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -1864,13 +1811,6 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/acl/available \ "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -1905,10 +1845,6 @@ Status Code **200** | `»»» last_seen_at` | string(date-time) | false | | | | `»»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | | `»»» name` | string | false | | | -| `»»» organization_ids` | array | false | | | -| `»»» roles` | array | false | | | -| `»»»» display_name` | string | false | | | -| `»»»» name` | string | false | | | | `»»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | | `»»» theme_preference` | string | false | | | | `»»» username` | string | true | | | diff --git a/docs/api/schemas.md b/docs/api/schemas.md index c95d58a935..c4ab8128fa 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -862,13 +862,6 @@ _None_ "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -889,13 +882,6 @@ _None_ "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -906,10 +892,10 @@ _None_ ### Properties -| Name | Type | Required | Restrictions | Description | -| -------- | ----------------------------------------- | -------- | ------------ | ----------- | -| `groups` | array of [codersdk.Group](#codersdkgroup) | false | | | -| `users` | array of [codersdk.User](#codersdkuser) | false | | | +| Name | Type | Required | Restrictions | Description | +| -------- | ----------------------------------------------------- | -------- | ------------ | ----------- | +| `groups` | array of [codersdk.Group](#codersdkgroup) | false | | | +| `users` | array of [codersdk.ReducedUser](#codersdkreduceduser) | false | | | ## codersdk.APIKey @@ -3207,13 +3193,6 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "last_seen_at": "2019-08-24T14:15:22Z", "login_type": "", "name": "string", - "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], - "roles": [ - { - "display_name": "string", - "name": "string" - } - ], "status": "active", "theme_preference": "string", "username": "string" @@ -3228,16 +3207,16 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------------- | -------------------------------------------- | -------- | ------------ | ----------- | -| `avatar_url` | string | false | | | -| `display_name` | string | false | | | -| `id` | string | false | | | -| `members` | array of [codersdk.User](#codersdkuser) | false | | | -| `name` | string | false | | | -| `organization_id` | string | false | | | -| `quota_allowance` | integer | false | | | -| `source` | [codersdk.GroupSource](#codersdkgroupsource) | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------------- | ----------------------------------------------------- | -------- | ------------ | ----------- | +| `avatar_url` | string | false | | | +| `display_name` | string | false | | | +| `id` | string | false | | | +| `members` | array of [codersdk.ReducedUser](#codersdkreduceduser) | false | | | +| `name` | string | false | | | +| `organization_id` | string | false | | | +| `quota_allowance` | integer | false | | | +| `source` | [codersdk.GroupSource](#codersdkgroupsource) | false | | | ## codersdk.GroupSource @@ -4287,6 +4266,45 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `api` | integer | false | | | | `disable_all` | boolean | false | | | +## codersdk.ReducedUser + +```json +{ + "avatar_url": "http://example.com", + "created_at": "2019-08-24T14:15:22Z", + "email": "user@example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "", + "name": "string", + "status": "active", + "theme_preference": "string", + "username": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------------------ | ------------------------------------------ | -------- | ------------ | ----------- | +| `avatar_url` | string | false | | | +| `created_at` | string | true | | | +| `email` | string | true | | | +| `id` | string | true | | | +| `last_seen_at` | string | false | | | +| `login_type` | [codersdk.LoginType](#codersdklogintype) | false | | | +| `name` | string | false | | | +| `status` | [codersdk.UserStatus](#codersdkuserstatus) | false | | | +| `theme_preference` | string | false | | | +| `username` | string | true | | | + +#### Enumerated Values + +| Property | Value | +| -------- | ----------- | +| `status` | `active` | +| `status` | `suspended` | + ## codersdk.Region ```json diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index b1330993b1..e9ae63c176 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -11,6 +11,7 @@ import ( "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/rbac" @@ -77,7 +78,7 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request) var emptyUsers []database.User aReq.New = group.Auditable(emptyUsers) - httpapi.Write(ctx, rw, http.StatusCreated, convertGroup(group, nil)) + httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.Group(group, nil)) } // @Summary Update group by name @@ -281,7 +282,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { aReq.New = group.Auditable(patchedMembers) - httpapi.Write(ctx, rw, http.StatusOK, convertGroup(group, patchedMembers)) + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, patchedMembers)) } // @Summary Delete group by name @@ -365,7 +366,7 @@ func (api *API) group(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(ctx, rw, http.StatusOK, convertGroup(group, users)) + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, users)) } // @Summary Get groups by organization @@ -410,68 +411,8 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) { return } - resp = append(resp, convertGroup(group, members)) + resp = append(resp, db2sdk.Group(group, members)) } httpapi.Write(ctx, rw, http.StatusOK, resp) } - -func convertGroup(g database.Group, users []database.User) codersdk.Group { - // It's ridiculous to query all the orgs of a user here - // especially since as of the writing of this comment there - // is only one org. So we pretend everyone is only part of - // the group's organization. - orgs := make(map[uuid.UUID][]uuid.UUID) - for _, user := range users { - orgs[user.ID] = []uuid.UUID{g.OrganizationID} - } - - return codersdk.Group{ - ID: g.ID, - Name: g.Name, - DisplayName: g.DisplayName, - OrganizationID: g.OrganizationID, - AvatarURL: g.AvatarURL, - QuotaAllowance: int(g.QuotaAllowance), - Members: convertUsers(users, orgs), - Source: codersdk.GroupSource(g.Source), - } -} - -func convertUser(user database.User, organizationIDs []uuid.UUID) codersdk.User { - convertedUser := codersdk.User{ - ID: user.ID, - Email: user.Email, - CreatedAt: user.CreatedAt, - LastSeenAt: user.LastSeenAt, - Username: user.Username, - Status: codersdk.UserStatus(user.Status), - OrganizationIDs: organizationIDs, - Roles: make([]codersdk.Role, 0, len(user.RBACRoles)), - AvatarURL: user.AvatarURL, - LoginType: codersdk.LoginType(user.LoginType), - } - - for _, roleName := range user.RBACRoles { - rbacRole, _ := rbac.RoleByName(roleName) - convertedUser.Roles = append(convertedUser.Roles, convertRole(rbacRole)) - } - - return convertedUser -} - -func convertUsers(users []database.User, organizationIDsByUserID map[uuid.UUID][]uuid.UUID) []codersdk.User { - converted := make([]codersdk.User, 0, len(users)) - for _, u := range users { - userOrganizationIDs := organizationIDsByUserID[u.ID] - converted = append(converted, convertUser(u, userOrganizationIDs)) - } - return converted -} - -func convertRole(role rbac.Role) codersdk.Role { - return codersdk.Role{ - DisplayName: role.DisplayName, - Name: role.Name, - } -} diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 90d52ea92b..5efb2e5361 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -236,8 +236,8 @@ func TestPatchGroup(t *testing.T) { AddUsers: []string{user2.ID.String(), user3.ID.String()}, }) require.NoError(t, err) - require.Contains(t, group.Members, user2) - require.Contains(t, group.Members, user3) + require.Contains(t, group.Members, user2.ReducedUser) + require.Contains(t, group.Members, user3.ReducedUser) }) t.Run("RemoveUsers", func(t *testing.T) { @@ -263,16 +263,16 @@ func TestPatchGroup(t *testing.T) { AddUsers: []string{user2.ID.String(), user3.ID.String(), user4.ID.String()}, }) require.NoError(t, err) - require.Contains(t, group.Members, user2) - require.Contains(t, group.Members, user3) + require.Contains(t, group.Members, user2.ReducedUser) + require.Contains(t, group.Members, user3.ReducedUser) group, err = userAdminClient.PatchGroup(ctx, group.ID, codersdk.PatchGroupRequest{ RemoveUsers: []string{user2.ID.String(), user3.ID.String()}, }) require.NoError(t, err) - require.NotContains(t, group.Members, user2) - require.NotContains(t, group.Members, user3) - require.Contains(t, group.Members, user4) + require.NotContains(t, group.Members, user2.ReducedUser) + require.NotContains(t, group.Members, user3.ReducedUser) + require.Contains(t, group.Members, user4.ReducedUser) }) t.Run("Audit", func(t *testing.T) { @@ -613,8 +613,8 @@ func TestGroup(t *testing.T) { AddUsers: []string{user2.ID.String(), user3.ID.String()}, }) require.NoError(t, err) - require.Contains(t, group.Members, user2) - require.Contains(t, group.Members, user3) + require.Contains(t, group.Members, user2.ReducedUser) + require.Contains(t, group.Members, user3.ReducedUser) ggroup, err := userAdminClient.Group(ctx, group.ID) require.NoError(t, err) @@ -665,15 +665,15 @@ func TestGroup(t *testing.T) { AddUsers: []string{user1.ID.String(), user2.ID.String()}, }) require.NoError(t, err) - require.Contains(t, group.Members, user1) - require.Contains(t, group.Members, user2) + require.Contains(t, group.Members, user1.ReducedUser) + require.Contains(t, group.Members, user2.ReducedUser) err = userAdminClient.DeleteUser(ctx, user1.ID) require.NoError(t, err) group, err = userAdminClient.Group(ctx, group.ID) require.NoError(t, err) - require.NotContains(t, group.Members, user1) + require.NotContains(t, group.Members, user1.ReducedUser) }) t.Run("IncludeSuspendedAndDormantUsers", func(t *testing.T) { @@ -700,8 +700,8 @@ func TestGroup(t *testing.T) { }) require.NoError(t, err) require.Len(t, group.Members, 2) - require.Contains(t, group.Members, user1) - require.Contains(t, group.Members, user2) + require.Contains(t, group.Members, user1.ReducedUser) + require.Contains(t, group.Members, user2.ReducedUser) user1, err = userAdminClient.UpdateUserStatus(ctx, user1.ID.String(), codersdk.UserStatusSuspended) require.NoError(t, err) @@ -709,8 +709,8 @@ func TestGroup(t *testing.T) { group, err = userAdminClient.Group(ctx, group.ID) require.NoError(t, err) require.Len(t, group.Members, 2) - require.Contains(t, group.Members, user1) - require.Contains(t, group.Members, user2) + require.Contains(t, group.Members, user1.ReducedUser) + require.Contains(t, group.Members, user2.ReducedUser) // cannot explicitly set a dormant user status so must create a new user anotherUser, err := userAdminClient.CreateUser(ctx, codersdk.CreateUserRequest{ @@ -731,8 +731,8 @@ func TestGroup(t *testing.T) { group, err = userAdminClient.Group(ctx, group.ID) require.NoError(t, err) require.Len(t, group.Members, 3) - require.Contains(t, group.Members, user1) - require.Contains(t, group.Members, user2) + require.Contains(t, group.Members, user1.ReducedUser) + require.Contains(t, group.Members, user2.ReducedUser) }) t.Run("everyoneGroupReturnsEmpty", func(t *testing.T) { @@ -754,7 +754,8 @@ func TestGroup(t *testing.T) { require.Len(t, group.Members, 4) require.Equal(t, "Everyone", group.Name) require.Equal(t, user.OrganizationID, group.OrganizationID) - require.Contains(t, group.Members, user1, user2) + require.Contains(t, group.Members, user1.ReducedUser) + require.Contains(t, group.Members, user2.ReducedUser) }) } diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 00b914046b..5ce27b8f80 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -11,6 +11,7 @@ import ( "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" @@ -64,15 +65,14 @@ func (api *API) templateAvailablePermissions(rw http.ResponseWriter, r *http.Req return } - sdkGroups = append(sdkGroups, convertGroup(group, members)) + sdkGroups = append(sdkGroups, db2sdk.Group(group, members)) } httpapi.Write(ctx, rw, http.StatusOK, codersdk.ACLAvailable{ - // No need to pass organization info here. // TODO: @emyrk we should return a MinimalUser here instead of a full user. // The FE requires the `email` field, so this cannot be done without // a UI change. - Users: convertUsers(users, map[uuid.UUID][]uuid.UUID{}), + Users: db2sdk.ReducedUsers(users), Groups: sdkGroups, }) } @@ -134,7 +134,7 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) { return } groups = append(groups, codersdk.TemplateGroup{ - Group: convertGroup(group.Group, members), + Group: db2sdk.Group(group.Group, members), Role: convertToTemplateRole(group.Actions), }) } @@ -287,7 +287,7 @@ func convertTemplateUsers(tus []database.TemplateUser, orgIDsByUserIDs map[uuid. for _, tu := range tus { users = append(users, codersdk.TemplateUser{ - User: convertUser(tu.User, orgIDsByUserIDs[tu.User.ID]), + User: db2sdk.User(tu.User, orgIDsByUserIDs[tu.User.ID]), Role: convertToTemplateRole(tu.Actions), }) } diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index bb3b37e943..36e0d69cbd 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -668,7 +668,7 @@ func TestGroupSync(t *testing.T) { } for _, group := range orgGroups { - userInGroup := slice.ContainsCompare(group.Members, codersdk.User{Email: user.Email}, func(a, b codersdk.User) bool { + userInGroup := slice.ContainsCompare(group.Members, codersdk.ReducedUser{Email: user.Email}, func(a, b codersdk.ReducedUser) bool { return a.Email == b.Email }) if group.IsEveryone() { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4b460df498..731b7b3a41 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -4,7 +4,7 @@ // From codersdk/templates.go export interface ACLAvailable { - readonly users: User[]; + readonly users: ReducedUser[]; readonly groups: Group[]; } @@ -581,7 +581,7 @@ export interface Group { readonly name: string; readonly display_name: string; readonly organization_id: string; - readonly members: User[]; + readonly members: ReducedUser[]; readonly avatar_url: string; readonly quota_allowance: number; readonly source: GroupSource; @@ -911,6 +911,17 @@ export interface RateLimitConfig { readonly api: number; } +// From codersdk/users.go +export interface ReducedUser extends MinimalUser { + readonly name: string; + readonly email: string; + readonly created_at: string; + readonly last_seen_at: string; + readonly status: UserStatus; + readonly login_type: LoginType; + readonly theme_preference: string; +} + // From codersdk/workspaceproxy.go export interface Region { readonly id: string; @@ -1385,19 +1396,9 @@ export interface UpsertWorkspaceAgentPortShareRequest { } // From codersdk/users.go -export interface User { - readonly id: string; - readonly username: string; - readonly name: string; - readonly email: string; - readonly created_at: string; - readonly last_seen_at: string; - readonly status: UserStatus; +export interface User extends ReducedUser { readonly organization_ids: string[]; readonly roles: Role[]; - readonly avatar_url: string; - readonly login_type: LoginType; - readonly theme_preference: string; } // From codersdk/insights.go diff --git a/site/src/pages/GroupsPage/GroupPage.tsx b/site/src/pages/GroupsPage/GroupPage.tsx index 6fe81b09e4..a27241f370 100644 --- a/site/src/pages/GroupsPage/GroupPage.tsx +++ b/site/src/pages/GroupsPage/GroupPage.tsx @@ -8,7 +8,7 @@ import TableRow from "@mui/material/TableRow"; import DeleteOutline from "@mui/icons-material/DeleteOutline"; import PersonAdd from "@mui/icons-material/PersonAdd"; import SettingsOutlined from "@mui/icons-material/SettingsOutlined"; -import type { Group, User } from "api/typesGenerated"; +import type { Group, ReducedUser, User } from "api/typesGenerated"; import { AvatarData } from "components/AvatarData/AvatarData"; import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"; import { EmptyState } from "components/EmptyState/EmptyState"; @@ -257,7 +257,7 @@ const AddGroupMember: FC = ({ isLoading, onSubmit }) => { }; interface GroupMemberRowProps { - member: User; + member: ReducedUser; group: Group; canUpdate: boolean; } diff --git a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx index 0e2fb9b216..5adc97e678 100644 --- a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx @@ -12,6 +12,7 @@ import { type Interpolation, type Theme } from "@emotion/react"; import { type FC, useState } from "react"; import type { Group, + ReducedUser, TemplateACL, TemplateGroup, TemplateRole, @@ -43,7 +44,11 @@ type AddTemplateUserOrGroupProps = { isLoading: boolean; templateACL: TemplateACL | undefined; onSubmit: ( - userOrGroup: TemplateUser | TemplateGroup, + userOrGroup: + | TemplateUser + | TemplateGroup + // Reduce user is returned by the groups. + | ({ role: TemplateRole } & ReducedUser), role: TemplateRole, reset: () => void, ) => void; @@ -160,7 +165,7 @@ export interface TemplatePermissionsPageViewProps { canUpdatePermissions: boolean; // User onAddUser: ( - user: TemplateUser, + user: TemplateUser | ({ role: TemplateRole } & ReducedUser), role: TemplateRole, reset: () => void, ) => void; diff --git a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/UserOrGroupAutocomplete.tsx b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/UserOrGroupAutocomplete.tsx index dac80188af..b6e332b11b 100644 --- a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/UserOrGroupAutocomplete.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/UserOrGroupAutocomplete.tsx @@ -3,7 +3,7 @@ import TextField from "@mui/material/TextField"; import Autocomplete from "@mui/material/Autocomplete"; import { type ChangeEvent, type FC, useState } from "react"; import { css } from "@emotion/react"; -import type { Group, User } from "api/typesGenerated"; +import type { Group, ReducedUser } from "api/typesGenerated"; import { AvatarData } from "components/AvatarData/AvatarData"; import { getGroupSubtitle } from "utils/groups"; import { useDebouncedFunction } from "hooks/debounce"; @@ -11,7 +11,7 @@ import { useQuery } from "react-query"; import { templaceACLAvailable } from "api/queries/templates"; import { prepareQuery } from "utils/filters"; -export type UserOrGroupAutocompleteValue = User | Group | null; +export type UserOrGroupAutocompleteValue = ReducedUser | Group | null; export type UserOrGroupAutocompleteProps = { value: UserOrGroupAutocompleteValue;