8000 refactor: Add roles into the user response by BrunoQuaresma · Pull Request #1347 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

refactor: Add roles into the user response #1347

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 9 commits into from
May 9, 2022
Merged

refactor: Add roles into the user response #1347

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b27a5ed
refactor: Add roles into the user response
BrunoQuaresma May 9, 2022
b1bb55f
Fix return type of UpdateOrganizationMemberRoles
BrunoQuaresma May 9, 2022
87c474b
Move convertRole(s) to coderd
BrunoQuaresma May 9, 2022
a9c8def
Fix TS types
BrunoQuaresma May 9, 2022
6ff75cc
Fix missed ConvertRole
BrunoQuaresma May 9, 2022
8588cf8
Fix JS test user mock
BrunoQuaresma May 9, 2022
440308f
Fix random story
BrunoQuaresma May 9, 2022
dda5809
Do not export convert functions
BrunoQuaresma May 9, 2022
27e14bc
Fix missed ConvertRole
BrunoQuaresma May 9, 2022
File filter

Filter by extension

Filter by extension
8000
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Move convertRole(s) to coderd
  • Loading branch information
@BrunoQuaresma
BrunoQuaresma committed May 9, 2022
commit 87c474b6b503651f36c576818e9a175629d9da70
19 changes: 17 additions & 2 deletions coderd/roles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func (*api) assignableSiteRoles(rw http.ResponseWriter, _ *http.Request) {
// TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the
// role of the user.
roles := rbac.SiteRoles()
httpapi.Write(rw, http.StatusOK, codersdk.ConvertRoles(roles))
httpapi.Write(rw, http.StatusOK, ConvertRoles(roles))
}

// assignableSiteRoles returns all site wide roles that can be assigned.
Expand All @@ -24,5 +24,20 @@ func (*api) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
// role of the user.
organization := httpmw.OrganizationParam(r)
roles := rbac.OrganizationRoles(organization.ID)
httpapi.Write(rw, http.StatusOK, codersdk.ConvertRoles(roles))
httpapi.Write(rw, http.StatusOK, ConvertRoles(roles))
}

func ConvertRole(role rbac.Role) codersdk.Role {
return codersdk.Role{
DisplayName: role.DisplayName,
Name: role.Name,
}
}

func ConvertRoles(roles []rbac.Role) []codersdk.Role {
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be exported either!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We are using it for some tests here

coder/coderd/roles_test.go

Line 88 in 87c474b

ExpectedRoles: coderd.ConvertRoles(rbac.OrganizationRoles(admin.OrganizationID)),

Copy link
Member

Choose a reason for hiding this comment

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

We should never need to use internal coderd APIs to check API responses, otherwise, our customers will have to as well!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thinking on:

  • Do not export the convert functions
  • Copy the convert functions into the role tests
  • See if we need restructure the tests at all

converted := make([]codersdk.Role, 0, len(roles))
for _, role := range roles {
converted = append(converted, ConvertRole(role))
}
return converted
}
7 changes: 4 additions & 3 deletions coderd/roles_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/stretchr/testify/require"

"github.com/coder/coder/coderd"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/codersdk"
Expand Down Expand Up @@ -84,7 +85,7 @@ func TestListRoles(t *testing.T) {
APICall: func() ([]codersdk.Role, error) {
return orgAdmin.ListOrganizationRoles(ctx, admin.OrganizationID)
},
ExpectedRoles: codersdk.ConvertRoles(rbac.OrganizationRoles(admin.OrganizationID)),
ExpectedRoles: coderd.ConvertRoles(rbac.OrganizationRoles(admin.OrganizationID)),
},
{
Name: "OrgAdminListOtherOrg",
Expand All @@ -99,14 +100,14 @@ func TestListRoles(t *testing.T) {
APICall: func() ([]codersdk.Role, error) {
return client.ListSiteRoles(ctx)
},
ExpectedRoles: codersdk.ConvertRoles(rbac.SiteRoles()),
ExpectedRoles: coderd.ConvertRoles(rbac.SiteRoles()),
},
{
Name: "AdminListOrg",
APICall: func() ([]codersdk.Role, error) {
return client.ListOrganizationRoles(ctx, admin.OrganizationID)
},
ExpectedRoles: codersdk.ConvertRoles(rbac.OrganizationRoles(admin.OrganizationID)),
ExpectedRoles: coderd.ConvertRoles(rbac.OrganizationRoles(admin.OrganizationID)),
},
}

Expand Down
16 changes: 0 additions & 16 deletions codersdk/roles.go
View file
Open in desktop
50FE
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"net/http"

"github.com/coder/coder/coderd/rbac"
"github.com/google/uuid"
)

Expand Down Expand Up @@ -44,18 +43,3 @@ func (c *Client) ListOrganizationRoles(ctx context.Context, org uuid.UUID) ([]Ro
var roles []Role
return roles, json.NewDecoder(res.Body).Decode(&roles)
}

func ConvertRole(role rbac.Role) Role {
return Role{
DisplayName: role.DisplayName,
Name: role.Name,
}
}

func ConvertRoles(roles []rbac.Role) []Role {
converted := make([]Role, 0, len(roles))
for _, role := range roles {
converted = append(converted, ConvertRole(role))
}
return converted
}
0