Chore: Replace Command dispatches by explicit calls (#32131)

* Clean up GetProvisionedDashboardDataByIdQuery

* Clean up SaveProvisionedDashboardCommand

* Clean up & fix AddTeamMemberCommand usages

* Clean up & fix UpdateUserPermissionsCommand usages

* Lint imports
This commit is contained in:
Joan López de la Franca Beltran 2021-03-19 09:14:14 +01:00 committed by GitHub
parent f8d5388758
commit 5c07df9f4b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 38 additions and 85 deletions

View File

@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/util"
)
@ -86,15 +87,11 @@ func AdminUpdateUserPassword(c *models.ReqContext, form dtos.AdminUpdateUserPass
}
// PUT /api/admin/users/:id/permissions
func AdminUpdateUserPermissions(c *models.ReqContext, form dtos.AdminUpdateUserPermissionsForm) response.Response {
func (hs *HTTPServer) AdminUpdateUserPermissions(c *models.ReqContext, form dtos.AdminUpdateUserPermissionsForm) response.Response {
userID := c.ParamsInt64(":id")
cmd := models.UpdateUserPermissionsCommand{
UserId: userID,
IsGrafanaAdmin: form.IsGrafanaAdmin,
}
if err := bus.Dispatch(&cmd); err != nil {
err := updateUserPermissions(hs.SQLStore, userID, form.IsGrafanaAdmin)
if err != nil {
if errors.Is(err, models.ErrLastGrafanaAdmin) {
return response.Error(400, models.ErrLastGrafanaAdmin.Error(), nil)
}
@ -189,3 +186,10 @@ func (hs *HTTPServer) AdminRevokeUserAuthToken(c *models.ReqContext, cmd models.
userID := c.ParamsInt64(":id")
return hs.revokeUserAuthTokenInternal(c, userID, cmd)
}
// updateUserPermissions updates the user's permissions.
//
// Stubbable by tests.
var updateUserPermissions = func(sqlStore *sqlstore.SQLStore, userID int64, isAdmin bool) error {
return sqlStore.UpdateUserPermissions(userID, isAdmin)
}

View File

@ -13,6 +13,8 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/auth"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -34,10 +36,16 @@ func TestAdminAPIEndpoint(t *testing.T) {
putAdminScenario(t, "When calling PUT on", "/api/admin/users/1/permissions",
"/api/admin/users/:id/permissions", role, updateCmd, func(sc *scenarioContext) {
bus.AddHandler("test", func(cmd *models.UpdateUserPermissionsCommand) error {
return models.ErrLastGrafanaAdmin
// TODO: Use a fake SQLStore when it's represented by an interface
origUpdateUserPermissions := updateUserPermissions
t.Cleanup(func() {
updateUserPermissions = origUpdateUserPermissions
})
updateUserPermissions = func(sqlStore *sqlstore.SQLStore, userID int64, isAdmin bool) error {
return models.ErrLastGrafanaAdmin
}
sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec()
assert.Equal(t, 400, sc.resp.Code)
})
@ -300,6 +308,10 @@ func putAdminScenario(t *testing.T, desc string, url string, routePattern string
t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
t.Cleanup(bus.ClearBusHandlers)
hs := &HTTPServer{
Cfg: setting.NewCfg(),
}
sc := setupScenarioContext(t, url)
sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
sc.context = c
@ -307,7 +319,7 @@ func putAdminScenario(t *testing.T, desc string, url string, routePattern string
sc.context.OrgId = testOrgID
sc.context.OrgRole = role
return AdminUpdateUserPermissions(c, cmd)
return hs.AdminUpdateUserPermissions(c, cmd)
})
sc.m.Put(routePattern, sc.defaultHandler)

View File

@ -406,7 +406,7 @@ func (hs *HTTPServer) registerRoutes() {
adminRoute.Get("/settings", routing.Wrap(AdminGetSettings))
adminRoute.Post("/users", bind(dtos.AdminCreateUserForm{}), routing.Wrap(hs.AdminCreateUser))
adminRoute.Put("/users/:id/password", bind(dtos.AdminUpdateUserPasswordForm{}), routing.Wrap(AdminUpdateUserPassword))
adminRoute.Put("/users/:id/permissions", bind(dtos.AdminUpdateUserPermissionsForm{}), routing.Wrap(AdminUpdateUserPermissions))
adminRoute.Put("/users/:id/permissions", bind(dtos.AdminUpdateUserPermissionsForm{}), routing.Wrap(hs.AdminUpdateUserPermissions))
adminRoute.Delete("/users/:id", routing.Wrap(AdminDeleteUser))
adminRoute.Post("/users/:id/disable", routing.Wrap(hs.AdminDisableUser))
adminRoute.Post("/users/:id/enable", routing.Wrap(AdminEnableUser))

View File

@ -176,11 +176,3 @@ func (hs *HTTPServer) UpdateTeamPreferences(c *models.ReqContext, dtoCmd dtos.Up
var createTeam = func(sqlStore *sqlstore.SQLStore, name, email string, orgID int64) (models.Team, error) {
return sqlStore.CreateTeam(name, email, orgID)
}
// addTeamMember adds a team member.
//
// Stubbable by tests.
var addTeamMember = func(sqlStore *sqlstore.SQLStore, userID, orgID, teamID int64, isExternal bool,
permission models.PermissionType) error {
return sqlStore.AddTeamMember(userID, orgID, teamID, isExternal, permission)
}

View File

@ -7,6 +7,7 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/teamguardian"
"github.com/grafana/grafana/pkg/util"
)
@ -48,7 +49,8 @@ func (hs *HTTPServer) AddTeamMember(c *models.ReqContext, cmd models.AddTeamMemb
return response.Error(403, "Not allowed to add team member", err)
}
if err := hs.Bus.Dispatch(&cmd); err != nil {
err := addTeamMember(hs.SQLStore, cmd.UserId, cmd.OrgId, cmd.TeamId, cmd.External, cmd.Permission)
if err != nil {
if errors.Is(err, models.ErrTeamNotFound) {
return response.Error(404, "Team not found", nil)
}
@ -119,3 +121,11 @@ func (hs *HTTPServer) RemoveTeamMember(c *models.ReqContext) response.Response {
}
return response.Success("Team Member removed")
}
// addTeamMember adds a team member.
//
// Stubbable by tests.
var addTeamMember = func(sqlStore *sqlstore.SQLStore, userID, orgID, teamID int64, isExternal bool,
permission models.PermissionType) error {
return sqlStore.AddTeamMember(userID, orgID, teamID, isExternal, permission)
}

View File

@ -355,13 +355,6 @@ type DashboardProvisioning struct {
Updated int64
}
type SaveProvisionedDashboardCommand struct {
DashboardCmd *SaveDashboardCommand
DashboardProvisioning *DashboardProvisioning
Result *Dashboard
}
type DeleteDashboardCommand struct {
Id int64
OrgId int64
@ -418,11 +411,6 @@ type GetDashboardSlugByIdQuery struct {
Result string
}
type GetProvisionedDashboardDataByIdQuery struct {
DashboardId int64
Result *DashboardProvisioning
}
type GetDashboardsBySlugQuery struct {
OrgId int64
Slug string

View File

@ -87,11 +87,6 @@ type ChangeUserPasswordCommand struct {
UserId int64 `json:"-"`
}
type UpdateUserPermissionsCommand struct {
IsGrafanaAdmin bool
UserId int64 `json:"-"`
}
type DisableUserCommand struct {
UserId int64
IsDisabled bool

View File

@ -107,13 +107,6 @@ func TestDashboardService(t *testing.T) {
})
Convey("Should not return validation error if dashboard is provisioned but UI updates allowed", func() {
provisioningValidated := false
bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardDataByIdQuery) error {
provisioningValidated = true
cmd.Result = &models.DashboardProvisioning{}
return nil
})
origValidateAlerts := validateAlerts
t.Cleanup(func() {
validateAlerts = origValidateAlerts
@ -126,16 +119,10 @@ func TestDashboardService(t *testing.T) {
dto.Dashboard.SetId(3)
dto.User = &models.SignedInUser{UserId: 1}
_, err := service.SaveDashboard(dto, true)
So(provisioningValidated, ShouldBeFalse)
So(err, ShouldBeNil)
})
Convey("Should return validation error if alert data is invalid", func() {
bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardDataByIdQuery) error {
cmd.Result = nil
return nil
})
origValidateAlerts := validateAlerts
t.Cleanup(func() {
validateAlerts = origValidateAlerts
@ -154,13 +141,6 @@ func TestDashboardService(t *testing.T) {
dto := &SaveDashboardDTO{}
Convey("Should not return validation error if dashboard is provisioned", func() {
provisioningValidated := false
bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardDataByIdQuery) error {
provisioningValidated = true
cmd.Result = &models.DashboardProvisioning{}
return nil
})
origUpdateAlerting := UpdateAlerting
t.Cleanup(func() {
UpdateAlerting = origUpdateAlerting
@ -178,16 +158,11 @@ func TestDashboardService(t *testing.T) {
return nil
}
bus.AddHandler("test", func(cmd *models.SaveProvisionedDashboardCommand) error {
return nil
})
dto.Dashboard = models.NewDashboard("Dash")
dto.Dashboard.SetId(3)
dto.User = &models.SignedInUser{UserId: 1}
_, err := service.SaveProvisionedDashboard(dto, nil)
So(err, ShouldBeNil)
So(provisioningValidated, ShouldBeFalse)
})
Convey("Should override invalid refresh interval if dashboard is provisioned", func() {
@ -195,11 +170,6 @@ func TestDashboardService(t *testing.T) {
setting.MinRefreshInterval = "5m"
defer func() { setting.MinRefreshInterval = oldRefreshInterval }()
bus.AddHandler("test", func(cmd *models.GetProvisionedDashboardDataByIdQuery) error {
cmd.Result = &models.DashboardProvisioning{}
return nil
})
origValidateAlerts := validateAlerts
t.Cleanup(func() {
validateAlerts = origValidateAlerts
@ -208,10 +178,6 @@ func TestDashboardService(t *testing.T) {
return nil
}
bus.AddHandler("test", func(cmd *models.SaveProvisionedDashboardCommand) error {
return nil
})
origUpdateAlerting := UpdateAlerting
t.Cleanup(func() {
UpdateAlerting = origUpdateAlerting
@ -248,10 +214,6 @@ func TestDashboardService(t *testing.T) {
return nil
}
bus.AddHandler("test", func(cmd *models.SaveProvisionedDashboardCommand) error {
return nil
})
origUpdateAlerting := UpdateAlerting
t.Cleanup(func() {
UpdateAlerting = origUpdateAlerting

View File

@ -102,18 +102,9 @@ func TestFolderService(t *testing.T) {
return nil
})
provisioningValidated := false
bus.AddHandler("test", func(query *models.GetProvisionedDashboardDataByIdQuery) error {
provisioningValidated = true
query.Result = nil
return nil
})
Convey("When creating folder should not return access denied error", func() {
_, err := service.CreateFolder("Folder", "")
So(err, ShouldBeNil)
So(provisioningValidated, ShouldBeFalse)
})
Convey("When updating folder should not return access denied error", func() {
@ -122,7 +113,6 @@ func TestFolderService(t *testing.T) {
Title: "Folder",
})
So(err, ShouldBeNil)
So(provisioningValidated, ShouldBeFalse)
})
Convey("When deleting folder by uid should not return access denied error", func() {