Skip to content

Commit f7668c1

Browse files
EmyrkEmyrk
committed
chore: add unit test for custom role returned from api
1 parent b347db1 commit f7668c1

File tree

4 files changed

+169
-21
lines changed

4 files changed

+169
-21
lines changed

‎coderd/members.go

Lines changed: 92 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
package coderd
22

33
import (
4+
"context"
45
"net/http"
56

67
"github.com/google/uuid"
7-
8-
"github.com/coder/coder/v2/coderd/database/db2sdk"
9-
"github.com/coder/coder/v2/coderd/rbac"
8+
"golang.org/x/xerrors"
109

1110
"github.com/coder/coder/v2/coderd/database"
11+
"github.com/coder/coder/v2/coderd/database/db2sdk"
1212
"github.com/coder/coder/v2/coderd/httpapi"
1313
"github.com/coder/coder/v2/coderd/httpmw"
14+
"github.com/coder/coder/v2/coderd/rbac"
1415
"github.com/coder/coder/v2/codersdk"
1516
)
1617

@@ -41,7 +42,13 @@ func (api *API) listMembers(rw http.ResponseWriter, r *http.Request) {
4142
return
4243
}
4344

44-
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.List(members, convertOrganizationMemberRow))
45+
resp, err := convertOrganizationMemberRows(ctx, api.Database, members)
46+
if err != nil {
47+
httpapi.InternalServerError(rw, err)
48+
return
49+
}
50+
51+
httpapi.Write(ctx, rw, http.StatusOK, resp)
4552
}
4653

4754
// @Summary Assign role to organization member
@@ -87,30 +94,94 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
8794
return
8895
}
8996

90-
httpapi.Write(ctx, rw, http.StatusOK, convertOrganizationMember(updatedUser))
97+
resp := convertOrganizationMembers(ctx, api.Database, []database.OrganizationMember{updatedUser})
98+
if len(resp) != 1 {
99+
httpapi.InternalServerError(rw, xerrors.Errorf("failed to serialize member to response, update still succeeded"))
100+
return
101+
}
102+
httpapi.Write(ctx, rw, http.StatusOK, resp[0])
91103
}
92104

93-
func convertOrganizationMember(mem database.OrganizationMember) codersdk.OrganizationMember {
94-
convertedMember := codersdk.OrganizationMember{
95-
UserID: mem.UserID,
96-
OrganizationID: mem.OrganizationID,
97-
CreatedAt: mem.CreatedAt,
98-
UpdatedAt: mem.UpdatedAt,
99-
Roles: make([]codersdk.SlimRole, 0, len(mem.Roles)),
105+
// convertOrganizationMembers batches the role lookup to make only 1 sql call
106+
// We
107+
func convertOrganizationMembers(ctx context.Context, db database.Store, mems []database.OrganizationMember) []codersdk.OrganizationMember {
108+
converted := make([]codersdk.OrganizationMember, 0, len(mems))
109+
roleLookup := make([]database.NameOrganizationPair, 0)
110+
111+
for _, m := range mems {
112+
converted = append(converted, codersdk.OrganizationMember{
113+
UserID: m.UserID,
114+
OrganizationID: m.OrganizationID,
115+
CreatedAt: m.CreatedAt,
116+
UpdatedAt: m.UpdatedAt,
117+
Roles: db2sdk.List(m.Roles, func(r string) codersdk.SlimRole {
118+
// If it is a built-in role, no lookups are needed.
119+
rbacRole, err := rbac.RoleByName(rbac.RoleIdentifier{Name: r, OrganizationID: m.OrganizationID})
120+
if err == nil {
121+
return db2sdk.SlimRole(rbacRole)
122+
}
123+
124+
// We know the role name and the organization ID. We are missing the
125+
// display name. Append the lookup parameter, so we can get the display name
126+
roleLookup = append(roleLookup, database.NameOrganizationPair{
127+
Name: r,
128+
OrganizationID: m.OrganizationID,
129+
})
130+
return codersdk.SlimRole{
131+
Name: r,
132+
DisplayName: "",
133+
OrganizationID: m.OrganizationID.String(),
134+
}
135+
}),
136+
})
100137
}
101138

102-
for _, roleName := range mem.Roles {
103-
rbacRole, _ := rbac.RoleByName(rbac.RoleIdentifier{Name: roleName, OrganizationID: mem.OrganizationID})
104-
convertedMember.Roles = append(convertedMember.Roles, db2sdk.SlimRole(rbacRole))
139+
customRoles, err := db.CustomRoles(ctx, database.CustomRolesParams{
140+
LookupRoles: roleLookup,
141+
ExcludeOrgRoles: false,
142+
OrganizationID: uuid.UUID{},
143+
})
144+
if err != nil {
145+
// We are missing the display names, but that is not absolutely required. So just
146+
// return the converted and the names will be used instead of the display names.
147+
return converted
148+
}
149+
150+
// Now map the customRoles back to the slimRoles for their display name.
151+
customRolesMap := make(map[string]database.CustomRole)
152+
for _, role := range customRoles {
153+
customRolesMap[role.RoleIdentifier().UniqueName()] = role
154+
}
155+
156+
for i := range converted {
157+
for j, role := range converted[i].Roles {
158+
if cr, ok := customRolesMap[role.UniqueName()]; ok {
159+
converted[i].Roles[j].DisplayName = cr.DisplayName
160+
}
161+
}
105162
}
106-
return convertedMember
163+
164+
return converted
107165
}
108166

109-
func convertOrganizationMemberRow(row database.OrganizationMembersRow) codersdk.OrganizationMemberWithName {
110-
convertedMember := codersdk.OrganizationMemberWithName{
111-
Username: row.Username,
112-
OrganizationMember: convertOrganizationMember(row.OrganizationMember),
167+
func convertOrganizationMemberRows(ctx context.Context, db database.Store, rows []database.OrganizationMembersRow) ([]codersdk.OrganizationMemberWithName, error) {
168+
members := make([]database.OrganizationMember, 0)
169+
for _, row := range rows {
170+
members = append(members, row.OrganizationMember)
171+
}
172+
173+
convertedMembers := convertOrganizationMembers(ctx, db, members)
174+
if len(convertedMembers) != len(rows) {
175+
return nil, xerrors.Errorf("conversion failed, mismatch slice lengths")
176+
}
177+
178+
converted := make([]codersdk.OrganizationMemberWithName, 0)
179+
for i := range convertedMembers {
180+
converted = append(converted, codersdk.OrganizationMemberWithName{
181+
Username: rows[i].Username,
182+
OrganizationMember: convertedMembers[i],
183+
})
113184
}
114185

115-
return convertedMember
186+
return converted, nil
116187
}

‎coderd/rbac/roles.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ func (r RoleIdentifier) String() string {
9696
return r.Name
9797
}
9898

99+
func (r RoleIdentifier) UniqueName() string {
100+
return r.String()
101+
}
102+
99103
func (r *RoleIdentifier) MarshalJSON() ([]byte, error) {
100104
return json.Marshal(r.String())
101105
}

‎codersdk/roles.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ func (s SlimRole) String() string {
2626
return s.Name
2727
}
2828

29+
// UniqueName concatenates the organization ID to create a globally unique
30+
// string name for the role.
31+
func (s SlimRole) UniqueName() string {
32+
if s.OrganizationID != "" {
33+
return s.Name + ":" + s.OrganizationID
34+
}
35+
return s.Name
36+
}
37+
2938
type AssignableRoles struct {
3039
Role `table:"r,recursive_inline"`
3140
Assignable bool `json:"assignable" table:"assignable"`

‎enterprise/cli/organizationmembers_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package cli_test
2+
3+
import (
4+
"bytes"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
9+
"github.com/coder/coder/v2/cli/clitest"
10+
"github.com/coder/coder/v2/coderd/coderdtest"
11+
"github.com/coder/coder/v2/coderd/rbac"
12+
"github.com/coder/coder/v2/codersdk"
13+
"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
14+
"github.com/coder/coder/v2/enterprise/coderd/license"
15+
"github.com/coder/coder/v2/testutil"
16+
)
17+
18+
func TestEnterpriseListOrganization(t *testing.T) {
19+
t.Run("CustomRole", func(t *testing.T) {
20+
t.Parallel()
21+
22+
dv := coderdtest.DeploymentValues(t)
23+
dv.Experiments = []string{string(codersdk.ExperimentCustomRoles)}
24+
ownerClient, owner := coderdenttest.New(t, &coderdenttest.Options{
25+
Options: &coderdtest.Options{
26+
DeploymentValues: dv,
27+
},
28+
LicenseOptions: &coderdenttest.LicenseOptions{
29+
Features: license.Features{
30+
codersdk.FeatureCustomRoles: 1,
31+
},
32+
}})
33+
34+
ctx := testutil.Context(t, testutil.WaitMedium)
35+
customRole, err := ownerClient.PatchOrganizationRole(ctx, owner.OrganizationID, codersdk.Role{
36+
Name: "custom",
37+
OrganizationID: owner.OrganizationID.String(),
38+
DisplayName: "Custom Role",
39+
SitePermissions: nil,
40+
OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
41+
codersdk.ResourceWorkspace: {codersdk.ActionRead},
42+
}),
43+
UserPermissions: nil,
44+
})
45+
require.NoError(t, err)
46+
47+
client, user := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleUserAdmin(), rbac.RoleIdentifier{
48+
Name: customRole.Name,
49+
OrganizationID: owner.OrganizationID,
50+
})
51+
52+
inv, root := clitest.New(t, "organization", "members", "-c", "user_id,username,organization_roles")
53+
clitest.SetupConfig(t, client, root)
54+
55+
buf := new(bytes.Buffer)
56+
inv.Stdout = buf
57+
err = inv.WithContext(ctx).Run()
58+
require.NoError(t, err)
59+
require.Contains(t, buf.String(), user.Username)
60+
require.Contains(t, buf.String(), owner.UserID.String())
61+
// Check the display name is the value in the cli list
62+
require.Contains(t, buf.String(), customRole.DisplayName)
63+
})
64+
}

0 commit comments

Comments
 (0)