Skip to content

Commit 0012858

Browse files
EmyrkEmyrk
committed
chore: table formatter to correctly handle slices
1 parent db4576d commit 0012858

File tree

4 files changed

+51
-22
lines changed

4 files changed

+51
-22
lines changed

‎cli/cliui/table.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,24 @@ func renderTable(out any, sort string, headers table.Row, filterColumns []string
205205
}
206206
}
207207

208+
// Guard against nil dereferences
209+
if v != nil {
210+
rt := reflect.TypeOf(v)
211+
switch rt.Kind() {
212+
case reflect.Slice:
213+
// By default, the behavior is '%v', which just returns a string like
214+
// '[a b c]'. This will add commas in between each value.
215+
strs := make([]string, 0)
216+
vt := reflect.ValueOf(v)
217+
for i := 0; i < vt.Len(); i++ {
218+
strs = append(strs, fmt.Sprintf("%v", vt.Index(i).Interface()))
219+
}
220+
v = "[" + strings.Join(strs, ", ") + "]"
221+
default:
222+
// Leave it as it is
223+
}
224+
}
225+
208226
rowSlice[i] = v
209227
}
210228

‎cli/cliui/table_test.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,10 @@ func Test_DisplayTable(t *testing.T) {
138138
t.Parallel()
139139

140140
expected := `
141-
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
142-
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
143-
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
144-
foo 10 [a b c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
141+
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
142+
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
143+
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
144+
foo 10 [a, b, c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
145145
`
146146

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

167167
expected := `
168-
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
169-
foo 10 [a b c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
170-
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
171-
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
168+
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
169+
foo 10 [a, b, c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
170+
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
171+
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
172172
`
173173

174174
out, err := cliui.DisplayTable(in, "age", nil)
@@ -235,12 +235,12 @@ Alice 25
235235
t.Run("WithSeparator", func(t *testing.T) {
236236
t.Parallel()
237237
expected := `
238-
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
239-
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
240-
-------------------------------------------------------------------------------------------------------------------------------------------------------------
241-
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
242-
-------------------------------------------------------------------------------------------------------------------------------------------------------------
243-
foo 10 [a b c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
238+
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
239+
bar 20 [a] bar1 21 <nil> <nil> bar3 23 {bar4 24 } 2022-08-02T15:49:10Z <nil>
240+
---------------------------------------------------------------------------------------------------------------------------------------------------------------
241+
baz 30 [] baz1 31 <nil> <nil> baz3 33 {baz4 34 } 2022-08-02T15:49:10Z <nil>
242+
---------------------------------------------------------------------------------------------------------------------------------------------------------------
243+
foo 10 [a, b, c] foo1 11 foo2 12 foo3 13 {foo4 14 } 2022-08-02T15:49:10Z 2022-08-02T15:49:10Z
244244
`
245245

246246
var inlineIn []any

‎coderd/members.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,11 @@ func (api *API) putMemberRoles(rw http.ResponseWriter, r *http.Request) {
9494
return
9595
}
9696

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

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

@@ -144,7 +148,7 @@ func convertOrganizationMembers(ctx context.Context, db database.Store, mems []d
144148
if err != nil {
145149
// We are missing the display names, but that is not absolutely required. So just
146150
// return the converted and the names will be used instead of the display names.
147-
return converted
151+
return converted, xerrors.Errorf("lookup custom roles: %w", err)
148152
}
149153

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

164-
return converted
168+
return converted, nil
165169
}
166170

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

173-
convertedMembers := convertOrganizationMembers(ctx, db, members)
177+
convertedMembers, err := convertOrganizationMembers(ctx, db, members)
178+
if err != nil {
179+
return nil, err
180+
}
174181
if len(convertedMembers) != len(rows) {
175182
return nil, xerrors.Errorf("conversion failed, mismatch slice lengths")
176183
}

‎enterprise/cli/organizationmembers_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import (
1515
"github.com/coder/coder/v2/testutil"
1616
)
1717

18-
func TestEnterpriseListOrganization(t *testing.T) {
18+
func TestEnterpriseListOrganizationMembers(t *testing.T) {
19+
t.Parallel()
20+
1921
t.Run("CustomRole", func(t *testing.T) {
2022
t.Parallel()
2123

@@ -29,9 +31,11 @@ func TestEnterpriseListOrganization(t *testing.T) {
2931
Features: license.Features{
3032
codersdk.FeatureCustomRoles: 1,
3133
},
32-
}})
34+
},
35+
})
3336

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

5256
inv, root := clitest.New(t, "organization", "members", "-c", "user_id,username,organization_roles")
5357
clitest.SetupConfig(t, client, root)

0 commit comments

Comments
 (0)