8000 chore: implement cli list organization members by Emyrk · Pull Request #13555 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

chore: implement cli list organization members #13555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 12, 2024
Merged

chore: implement cli list organization members #13555

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c5d265
chore: implement cli list organization members
Emyrk Jun 12, 2024
db4576d
chore: add unit test for custom role returned from api
Emyrk Jun 12, 2024
0012858
chore: table formatter to correctly handle slices
Emyrk Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
chore: table formatter to correctly handle slices
  • Loading branch information
@Emyrk
Emyrk committed Jun 12, 2024
commit 00128582c1347d6fb397927e13aabdf2320899ed
18 changes: 18 additions & 0 deletions cli/cliui/table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,24 @@ func renderTable(out any, sort string, headers table.Row, filterColumns []string
}
}

// Guard against nil dereferences
if v != nil {
rt := reflect.TypeOf(v)
switch rt.Kind() {
case reflect.Slice:
// By default, the behavior is '%v', which just returns a string like
// '[a b c]'. This will add commas in between each value.
strs := make([]string, 0)
vt := reflect.ValueOf(v)
for i := 0; i < vt.Len(); i++ {
strs = append(strs, fmt.Sprintf("%v", vt.Index(i).Interface()))
}
v = "[" + strings.Join(strs, ", ") + "]"
default:
// Leave it as it is
}
}

rowSlice[i] = v
}

Expand Down
28 changes: 14 additions & 14 deletions cli/cliui/table_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@ func Test_DisplayTable(t *testing.T) {
t.Parallel()

expected := `
NAME AGE ROLES SUB 1 NAME SUB 1 AGE SUB 2 NAME SUB 2 AGE SUB 3 INNER NAME SUB 3 INNER AGE SUB 4 TIME TIME PTR
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
foo 10 [a b c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
NAME AGE ROLES SUB 1 NAME SUB 1 AGE SUB 2 NAME SUB 2 AGE SUB 3 INNER NAME SUB 3 INNER AGE SUB 4 TIME TIME PTR
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
foo 10 [a, b, c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
`

// Test with non-pointer values.
Expand All @@ -165,10 +165,10 @@ foo 10 [a b c] foo1 11 foo2 12 foo3
t.Parallel()

expected := `
NAME AGE ROLES SUB 1 NAME SUB 1 AGE SUB 2 NAME SUB 2 AGE SUB 3 INNER NAME SUB 3 INNER AGE SUB 4 TIME TIME PTR
foo 10 [a b c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
NAME AGE ROLES SUB 1 NAME SUB 1 AGE SUB 2 NAME SUB 2 AGE SUB 3 INNER NAME SUB 3 INNER AGE SUB 4 TIME TIME PTR
foo 10 [a, b, c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
`

out, err := cliui.DisplayTable(in, "age", nil)
Expand Down Expand Up @@ -235,12 +235,12 @@ Alice 25
t.Run("WithSeparator", func(t *testing.T) {
t.Parallel()
expected := `
NAME AGE ROLES SUB 1 NAME SUB 1 AGE SUB 2 NAME SUB 2 AGE SUB 3 INNER NAME SUB 3 INNER AGE SUB 4 TIME TIME PTR
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
-------------------------------------------------------------------------------------------------------------------------------------------------------------
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
-------------------------------------------------------------------------------------------------------------------------------------------------------------
foo 10 [a b c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
NAME AGE ROLES SUB 1 NAME SUB 1 AGE SUB 2 NAME SUB 2 AGE SUB 3 INNER NAME SUB 3 INNER AGE SUB 4 TIME TIME PTR
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
---------------------------------------------------------------------------------------------------------------------------------------------------------------
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
---------------------------------------------------------------------------------------------------------------------------------------------------------------
foo 10 [a, b, c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
`

var inlineIn []any
Expand Down
17 changes: 12 additions & 5 deletions coderd/members.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
return
}

resp := convertOrganizationMembers(ctx, api.Database, []database.OrganizationMember{updatedUser})
resp, err := convertOrganizationMembers(ctx, api.Database, []database.OrganizationMember{updatedUser})
if err != nil {
httpapi.InternalServerError(rw, err)
return
}
if len(resp) != 1 {
httpapi.InternalServerError(rw, xerrors.Errorf("failed to serialize member to response, update still succeeded"))
return
Expand All @@ -104,7 +108,7 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {

// convertOrganizationMembers batches the role lookup to make only 1 sql call
// We
func convertOrganizationMembers(ctx context.Context, db database.Store, mems []database.OrganizationMember) []codersdk.OrganizationMember {
func convertOrganizationMembers(ctx context.Context, db database.Store, mems []database.OrganizationMember) ([]codersdk.OrganizationMember, error) {
converted := make([]codersdk.OrganizationMember, 0, len(mems))
roleLookup := make([]database.NameOrganizationPair, 0)

Expand Down Expand Up @@ -144,7 +148,7 @@ func convertOrganizationMembers(ctx context.Context, db database.Store, mems []d
if err != nil {
// We are missing the display names, but that is not absolutely required. So just
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we expecting an error here? I see we can do without it, but I wonder if we should instead of just presenting the error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll return the error 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just unfortunate we can block api calls on this kind of error. Would be nice if we could return some sort of "warning" instead. I'll return an error though as that is what we usually do for failed db calls.

// return the converted and the names will be used instead of the display names.
return converted
return converted, xerrors.Errorf("lookup custom roles: %w", err)
}

// Now map the customRoles back to the slimRoles for their display name.
Expand All @@ -161,7 +165,7 @@ func convertOrganizationMembers(ctx context.Context, db database.Store, mems []d
}
}

return converted
return converted, nil
}

func convertOrganizationMemberRows(ctx context.Context, db database.Store, rows []database.OrganizationMembersRow) ([]codersdk.OrganizationMemberWithName, error) {
Expand All @@ -170,7 +174,10 @@ func convertOrganizationMemberRows(ctx context.Context, db database.Store, rows
members = append(members, row.OrganizationMember)
}

convertedMembers := convertOrganizationMembers(ctx, db, members)
convertedMembers, err := convertOrganizationMembers(ctx, db, members)
if err != nil {
return nil, err
}
if len(convertedMembers) != len(rows) {
return nil, xerrors.Errorf("conversion failed, mismatch slice lengths")
}
Expand Down
10 changes: 7 additions & 3 deletions enterprise/cli/organizationmembers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import (
"github.com/coder/coder/v2/testutil"
)

func TestEnterpriseListOrganization(t *testing.T) {
func TestEnterpriseListOrganizationMembers(t *testing.T) {
t.Parallel()

t.Run("CustomRole", func(t *testing.T) {
t.Parallel()

Expand All @@ -29,9 +31,11 @@ func TestEnterpriseListOrganization(t *testing.T) {
Features: license.Features{
codersdk.FeatureCustomRoles: 1,
},
}})
},
})

ctx := testutil.Context(t, testutil.WaitMedium)
//nolint:gocritic // only owners can patch roles
customRole, err := ownerClient.PatchOrganizationRole(ctx, owner.OrganizationID, codersdk.Role{
Name: "custom",
OrganizationID: owner.OrganizationID.String(),
Expand All @@ -47,7 +51,7 @@ func TestEnterpriseListOrganization(t *testing.T) {
client, user := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleUserAdmin(), rbac.RoleIdentifier{
Name: customRole.Name,
OrganizationID: owner.OrganizationID,
})
}, rbac.ScopedRoleOrgAdmin(owner.OrganizationID))

inv, root := clitest.New(t, "organization", "members", "-c", "user_id,username,organization_roles")
clitest.SetupConfig(t, client, root)
Expand Down
Loading
0