From db89ac4134088d1558c80284735043c211d75009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 14 Feb 2018 11:50:58 +0100 Subject: [PATCH 01/77] initial fixes for dashboard permission acl list query, fixes #10864 --- pkg/services/sqlstore/dashboard_acl.go | 62 +++++++-------------- pkg/services/sqlstore/dashboard_acl_test.go | 17 ++++++ pkg/services/sqlstore/org_test.go | 15 ++++- 3 files changed, 49 insertions(+), 45 deletions(-) diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index 829182a8195..a1a308d6497 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -1,7 +1,6 @@ package sqlstore import ( - "fmt" "time" "github.com/grafana/grafana/pkg/bus" @@ -40,7 +39,7 @@ func UpdateDashboardAcl(cmd *m.UpdateDashboardAclCommand) error { // Update dashboard HasAcl flag dashboard := m.Dashboard{HasAcl: true} - if _, err := sess.Cols("has_acl").Where("id=? OR folder_id=?", cmd.DashboardId, cmd.DashboardId).Update(&dashboard); err != nil { + if _, err := sess.Cols("has_acl").Where("id=?", cmd.DashboardId).Update(&dashboard); err != nil { return err } return nil @@ -134,6 +133,8 @@ func RemoveDashboardAcl(cmd *m.RemoveDashboardAclCommand) error { func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error { var err error + falseStr := dialect.BooleanStr(false) + if query.DashboardId == 0 { sql := `SELECT da.id, @@ -151,18 +152,13 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error { '' as title, '' as slug, '' as uid,` + - dialect.BooleanStr(false) + ` AS is_folder + falseStr + ` AS is_folder FROM dashboard_acl as da WHERE da.dashboard_id = -1` query.Result = make([]*m.DashboardAclInfoDTO, 0) err = x.SQL(sql).Find(&query.Result) } else { - dashboardFilter := fmt.Sprintf(`IN ( - SELECT %d - UNION - SELECT folder_id from dashboard where id = %d - )`, query.DashboardId, query.DashboardId) rawSQL := ` -- get permissions for the dashboard and its parent folder @@ -183,41 +179,21 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error { d.slug, d.uid, d.is_folder - FROM` + dialect.Quote("dashboard_acl") + ` as da - LEFT OUTER JOIN ` + dialect.Quote("user") + ` AS u ON u.id = da.user_id - LEFT OUTER JOIN team ug on ug.id = da.team_id - LEFT OUTER JOIN dashboard d on da.dashboard_id = d.id - WHERE dashboard_id ` + dashboardFilter + ` AND da.org_id = ? - - -- Also include default permissions if folder or dashboard field "has_acl" is false - - UNION - SELECT - da.id, - da.org_id, - da.dashboard_id, - da.user_id, - da.team_id, - da.permission, - da.role, - da.created, - da.updated, - '' as user_login, - '' as user_email, - '' as team, - folder.title, - folder.slug, - folder.uid, - folder.is_folder - FROM dashboard_acl as da, - dashboard as dash - LEFT OUTER JOIN dashboard folder on dash.folder_id = folder.id - WHERE - dash.id = ? AND ( - dash.has_acl = ` + dialect.BooleanStr(false) + ` or - folder.has_acl = ` + dialect.BooleanStr(false) + ` - ) AND - da.dashboard_id = -1 + FROM dashboard as d + LEFT JOIN dashboard folder on folder.id = d.folder_id + LEFT JOIN dashboard_acl AS da ON + da.dashboard_id = d.id OR + da.dashboard_id = d.folder_id OR + ( + -- include default permissions --> + da.org_id = -1 AND ( + (folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR + (folder.id IS NULL AND d.has_acl = ` + falseStr + `) + ) + ) + LEFT JOIN ` + dialect.Quote("user") + ` AS u ON u.id = da.user_id + LEFT JOIN team ug on ug.id = da.team_id + WHERE d.org_id = ? AND d.id = ? AND da.id IS NOT NULL ORDER BY 1 ASC ` diff --git a/pkg/services/sqlstore/dashboard_acl_test.go b/pkg/services/sqlstore/dashboard_acl_test.go index 8b712c73ece..8d4af9544d9 100644 --- a/pkg/services/sqlstore/dashboard_acl_test.go +++ b/pkg/services/sqlstore/dashboard_acl_test.go @@ -41,6 +41,23 @@ func TestDashboardAclDataAccess(t *testing.T) { }) }) + Convey("Given dashboard folder with removed default permissions", func() { + err := UpdateDashboardAcl(&m.UpdateDashboardAclCommand{ + DashboardId: savedFolder.Id, + Items: []*m.DashboardAcl{}, + }) + So(err, ShouldBeNil) + + Convey("When reading dashboard acl should return no acl items", func() { + query := m.GetDashboardAclInfoListQuery{DashboardId: childDash.Id, OrgId: 1} + + err := GetDashboardAclInfoList(&query) + So(err, ShouldBeNil) + + So(len(query.Result), ShouldEqual, 0) + }) + }) + Convey("Given dashboard folder permission", func() { err := SetDashboardAcl(&m.SetDashboardAclCommand{ OrgId: 1, diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 5322dfd4748..c57d15a48d5 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -199,10 +199,13 @@ func TestAccountDataAccess(t *testing.T) { So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 3) - err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 1, OrgId: ac1.OrgId, UserId: ac3.Id, Permission: m.PERMISSION_EDIT}) + dash1 := insertTestDashboard("1 test dash", ac1.OrgId, 0, false, "prod", "webapp") + dash2 := insertTestDashboard("2 test dash", ac3.OrgId, 0, false, "prod", "webapp") + + err = testHelperUpdateDashboardAcl(dash1.Id, m.DashboardAcl{DashboardId: dash1.Id, OrgId: ac1.OrgId, UserId: ac3.Id, Permission: m.PERMISSION_EDIT}) So(err, ShouldBeNil) - err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 2, OrgId: ac3.OrgId, UserId: ac3.Id, Permission: m.PERMISSION_EDIT}) + err = testHelperUpdateDashboardAcl(dash2.Id, m.DashboardAcl{DashboardId: dash2.Id, OrgId: ac3.OrgId, UserId: ac3.Id, Permission: m.PERMISSION_EDIT}) So(err, ShouldBeNil) Convey("When org user is deleted", func() { @@ -234,3 +237,11 @@ func TestAccountDataAccess(t *testing.T) { }) }) } + +func testHelperUpdateDashboardAcl(dashboardId int64, items ...m.DashboardAcl) error { + cmd := m.UpdateDashboardAclCommand{DashboardId: dashboardId} + for _, item := range items { + cmd.Items = append(cmd.Items, &item) + } + return UpdateDashboardAcl(&cmd) +} From ec6f0f94b80c10e30226d2bdccb8d9db5c885b86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 14 Feb 2018 14:31:20 +0100 Subject: [PATCH 02/77] permissions: refactoring of acl api and query --- pkg/api/api.go | 1 - pkg/api/dashboard_acl.go | 30 ----- pkg/api/dashboard_acl_test.go | 104 ++---------------- pkg/api/dashboard_test.go | 8 +- pkg/models/dashboard_acl.go | 16 --- pkg/services/guardian/guardian.go | 20 ---- pkg/services/sqlstore/dashboard.go | 32 +----- pkg/services/sqlstore/dashboard_acl.go | 85 +------------- pkg/services/sqlstore/dashboard_acl_test.go | 74 ++----------- .../sqlstore/dashboard_folder_test.go | 29 ++--- pkg/services/sqlstore/dashboard_test.go | 19 ---- pkg/services/sqlstore/team_test.go | 2 +- pkg/services/sqlstore/user_test.go | 2 +- .../PermissionsStore/PermissionsStoreItem.ts | 3 +- 14 files changed, 40 insertions(+), 385 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index c03bf7963b8..1320663f630 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -269,7 +269,6 @@ func (hs *HttpServer) registerRoutes() { dashIdRoute.Group("/acl", func(aclRoute RouteRegister) { aclRoute.Get("/", wrap(GetDashboardAclList)) aclRoute.Post("/", bind(dtos.UpdateDashboardAclCommand{}), wrap(UpdateDashboardAcl)) - aclRoute.Delete("/:aclId", wrap(DeleteDashboardAcl)) }) }) }) diff --git a/pkg/api/dashboard_acl.go b/pkg/api/dashboard_acl.go index 45f121dd0d0..32b75e80cc0 100644 --- a/pkg/api/dashboard_acl.go +++ b/pkg/api/dashboard_acl.go @@ -84,33 +84,3 @@ func UpdateDashboardAcl(c *middleware.Context, apiCmd dtos.UpdateDashboardAclCom return ApiSuccess("Dashboard acl updated") } - -func DeleteDashboardAcl(c *middleware.Context) Response { - dashId := c.ParamsInt64(":dashboardId") - aclId := c.ParamsInt64(":aclId") - - _, rsp := getDashboardHelper(c.OrgId, "", dashId, "") - if rsp != nil { - return rsp - } - - guardian := guardian.NewDashboardGuardian(dashId, c.OrgId, c.SignedInUser) - if canAdmin, err := guardian.CanAdmin(); err != nil || !canAdmin { - return dashboardGuardianResponse(err) - } - - if okToDelete, err := guardian.CheckPermissionBeforeRemove(m.PERMISSION_ADMIN, aclId); err != nil || !okToDelete { - if err != nil { - return ApiError(500, "Error while checking dashboard permissions", err) - } - - return ApiError(403, "Cannot remove own admin permission for a folder", nil) - } - - cmd := m.RemoveDashboardAclCommand{OrgId: c.OrgId, AclId: aclId} - if err := bus.Dispatch(&cmd); err != nil { - return ApiError(500, "Failed to delete permission for user", err) - } - - return Json(200, "") -} diff --git a/pkg/api/dashboard_acl_test.go b/pkg/api/dashboard_acl_test.go index e43e57ed5c0..d6b7e305daf 100644 --- a/pkg/api/dashboard_acl_test.go +++ b/pkg/api/dashboard_acl_test.go @@ -15,11 +15,11 @@ import ( func TestDashboardAclApiEndpoint(t *testing.T) { Convey("Given a dashboard acl", t, func() { mockResult := []*m.DashboardAclInfoDTO{ - {Id: 1, OrgId: 1, DashboardId: 1, UserId: 2, Permission: m.PERMISSION_VIEW}, - {Id: 2, OrgId: 1, DashboardId: 1, UserId: 3, Permission: m.PERMISSION_EDIT}, - {Id: 3, OrgId: 1, DashboardId: 1, UserId: 4, Permission: m.PERMISSION_ADMIN}, - {Id: 4, OrgId: 1, DashboardId: 1, TeamId: 1, Permission: m.PERMISSION_VIEW}, - {Id: 5, OrgId: 1, DashboardId: 1, TeamId: 2, Permission: m.PERMISSION_ADMIN}, + {OrgId: 1, DashboardId: 1, UserId: 2, Permission: m.PERMISSION_VIEW}, + {OrgId: 1, DashboardId: 1, UserId: 3, Permission: m.PERMISSION_EDIT}, + {OrgId: 1, DashboardId: 1, UserId: 4, Permission: m.PERMISSION_ADMIN}, + {OrgId: 1, DashboardId: 1, TeamId: 1, Permission: m.PERMISSION_VIEW}, + {OrgId: 1, DashboardId: 1, TeamId: 2, Permission: m.PERMISSION_ADMIN}, } dtoRes := transformDashboardAclsToDTOs(mockResult) @@ -92,21 +92,11 @@ func TestDashboardAclApiEndpoint(t *testing.T) { So(sc.resp.Code, ShouldEqual, 404) }) }) - - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/2/acl/6", "/api/dashboards/id/:dashboardId/acl/:aclId", m.ROLE_ADMIN, func(sc *scenarioContext) { - getDashboardNotFoundError = m.ErrDashboardNotFound - sc.handlerFunc = DeleteDashboardAcl - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - - Convey("Should not be able to delete non-existing dashboard", func() { - So(sc.resp.Code, ShouldEqual, 404) - }) - }) }) Convey("When user is org editor and has admin permission in the ACL", func() { loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", m.ROLE_EDITOR, func(sc *scenarioContext) { - mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) + mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) Convey("Should be able to access ACL", func() { sc.handlerFunc = GetDashboardAclList @@ -116,36 +106,6 @@ func TestDashboardAclApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/1", "/api/dashboards/id/:dashboardId/acl/:aclId", m.ROLE_EDITOR, func(sc *scenarioContext) { - mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) - - bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error { - return nil - }) - - Convey("Should be able to delete permission", func() { - sc.handlerFunc = DeleteDashboardAcl - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - - So(sc.resp.Code, ShouldEqual, 200) - }) - }) - - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/6", "/api/dashboards/id/:dashboardId/acl/:aclId", m.ROLE_EDITOR, func(sc *scenarioContext) { - mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) - - bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error { - return nil - }) - - Convey("Should not be able to delete their own Admin permission", func() { - sc.handlerFunc = DeleteDashboardAcl - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - - So(sc.resp.Code, ShouldEqual, 403) - }) - }) - Convey("Should not be able to downgrade their own Admin permission", func() { cmd := dtos.UpdateDashboardAclCommand{ Items: []dtos.DashboardAclUpdateItem{ @@ -154,7 +114,7 @@ func TestDashboardAclApiEndpoint(t *testing.T) { } postAclScenario("When calling POST on", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", m.ROLE_EDITOR, cmd, func(sc *scenarioContext) { - mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) + mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) CallPostAcl(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -170,34 +130,18 @@ func TestDashboardAclApiEndpoint(t *testing.T) { } postAclScenario("When calling POST on", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", m.ROLE_EDITOR, cmd, func(sc *scenarioContext) { - mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 6, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) + mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_ADMIN}) CallPostAcl(sc) So(sc.resp.Code, ShouldEqual, 200) }) }) - Convey("When user is a member of a team in the ACL with admin permission", func() { - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/1", "/api/dashboards/id/:dashboardsId/acl/:aclId", m.ROLE_EDITOR, func(sc *scenarioContext) { - teamResp = append(teamResp, &m.Team{Id: 2, OrgId: 1, Name: "UG2"}) - - bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error { - return nil - }) - - Convey("Should be able to delete permission", func() { - sc.handlerFunc = DeleteDashboardAcl - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - - So(sc.resp.Code, ShouldEqual, 200) - }) - }) - }) }) Convey("When user is org viewer and has edit permission in the ACL", func() { loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/1/acl", "/api/dashboards/id/:dashboardId/acl", m.ROLE_VIEWER, func(sc *scenarioContext) { - mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_EDIT}) + mockResult = append(mockResult, &m.DashboardAclInfoDTO{OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_EDIT}) // Getting the permissions is an Admin permission Convey("Should not be able to get list of permissions from ACL", func() { @@ -207,21 +151,6 @@ func TestDashboardAclApiEndpoint(t *testing.T) { So(sc.resp.Code, ShouldEqual, 403) }) }) - - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/1", "/api/dashboards/id/:dashboardId/acl/:aclId", m.ROLE_VIEWER, func(sc *scenarioContext) { - mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_EDIT}) - - bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error { - return nil - }) - - Convey("Should be not be able to delete permission", func() { - sc.handlerFunc = DeleteDashboardAcl - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - - So(sc.resp.Code, ShouldEqual, 403) - }) - }) }) Convey("When user is org editor and not in the ACL", func() { @@ -234,20 +163,6 @@ func TestDashboardAclApiEndpoint(t *testing.T) { So(sc.resp.Code, ShouldEqual, 403) }) }) - - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/id/1/acl/user/1", "/api/dashboards/id/:dashboardsId/acl/user/:userId", m.ROLE_EDITOR, func(sc *scenarioContext) { - mockResult = append(mockResult, &m.DashboardAclInfoDTO{Id: 1, OrgId: 1, DashboardId: 1, UserId: 1, Permission: m.PERMISSION_VIEW}) - bus.AddHandler("test3", func(cmd *m.RemoveDashboardAclCommand) error { - return nil - }) - - Convey("Should be not be able to delete permission", func() { - sc.handlerFunc = DeleteDashboardAcl - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - - So(sc.resp.Code, ShouldEqual, 403) - }) - }) }) }) } @@ -257,7 +172,6 @@ func transformDashboardAclsToDTOs(acls []*m.DashboardAclInfoDTO) []*m.DashboardA for _, acl := range acls { dto := &m.DashboardAclInfoDTO{ - Id: acl.Id, OrgId: acl.OrgId, DashboardId: acl.DashboardId, Permission: acl.Permission, diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index e80b3cad4dc..4a45c561d57 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -431,7 +431,7 @@ func TestDashboardApiEndpoint(t *testing.T) { role := m.ROLE_VIEWER mockResult := []*m.DashboardAclInfoDTO{ - {Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_EDIT}, + {OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_EDIT}, } bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { @@ -505,7 +505,7 @@ func TestDashboardApiEndpoint(t *testing.T) { setting.ViewersCanEdit = true mockResult := []*m.DashboardAclInfoDTO{ - {Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW}, + {OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW}, } bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { @@ -564,7 +564,7 @@ func TestDashboardApiEndpoint(t *testing.T) { role := m.ROLE_VIEWER mockResult := []*m.DashboardAclInfoDTO{ - {Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_ADMIN}, + {OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_ADMIN}, } bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { @@ -637,7 +637,7 @@ func TestDashboardApiEndpoint(t *testing.T) { role := m.ROLE_EDITOR mockResult := []*m.DashboardAclInfoDTO{ - {Id: 1, OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW}, + {OrgId: 1, DashboardId: 2, UserId: 1, Permission: m.PERMISSION_VIEW}, } bus.AddHandler("test", func(query *m.GetDashboardAclInfoListQuery) error { diff --git a/pkg/models/dashboard_acl.go b/pkg/models/dashboard_acl.go index 933487650e3..202b519207d 100644 --- a/pkg/models/dashboard_acl.go +++ b/pkg/models/dashboard_acl.go @@ -44,7 +44,6 @@ type DashboardAcl struct { } type DashboardAclInfoDTO struct { - Id int64 `json:"id"` OrgId int64 `json:"-"` DashboardId int64 `json:"dashboardId"` @@ -75,21 +74,6 @@ type UpdateDashboardAclCommand struct { Items []*DashboardAcl } -type SetDashboardAclCommand struct { - DashboardId int64 - OrgId int64 - UserId int64 - TeamId int64 - Permission PermissionType - - Result DashboardAcl -} - -type RemoveDashboardAclCommand struct { - AclId int64 - OrgId int64 -} - // // QUERIES // diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index b448561494d..05795b7f2df 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -106,26 +106,6 @@ func (g *DashboardGuardian) checkAcl(permission m.PermissionType, acl []*m.Dashb return false, nil } -func (g *DashboardGuardian) CheckPermissionBeforeRemove(permission m.PermissionType, aclIdToRemove int64) (bool, error) { - if g.user.OrgRole == m.ROLE_ADMIN { - return true, nil - } - - acl, err := g.GetAcl() - if err != nil { - return false, err - } - - for i, p := range acl { - if p.Id == aclIdToRemove { - acl = append(acl[:i], acl[i+1:]...) - break - } - } - - return g.checkAcl(permission, acl) -} - func (g *DashboardGuardian) CheckPermissionBeforeUpdate(permission m.PermissionType, updatePermissions []*m.DashboardAcl) (bool, error) { if g.user.OrgRole == m.ROLE_ADMIN { return true, nil diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index f3fd81ebbe2..42c83da8810 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -79,11 +79,6 @@ func saveDashboard(sess *DBSession, cmd *m.SaveDashboardCommand) error { dash.Data.Set("uid", uid) } - err = setHasAcl(sess, dash) - if err != nil { - return err - } - parentVersion := dash.Version affectedRows := int64(0) @@ -100,7 +95,7 @@ func saveDashboard(sess *DBSession, cmd *m.SaveDashboardCommand) error { dash.Updated = cmd.UpdatedAt } - affectedRows, err = sess.MustCols("folder_id", "has_acl").ID(dash.Id).Update(dash) + affectedRows, err = sess.MustCols("folder_id").ID(dash.Id).Update(dash) } if err != nil { @@ -233,31 +228,6 @@ func generateNewDashboardUid(sess *DBSession, orgId int64) (string, error) { return "", m.ErrDashboardFailedGenerateUniqueUid } -func setHasAcl(sess *DBSession, dash *m.Dashboard) error { - // check if parent has acl - if dash.FolderId > 0 { - var parent m.Dashboard - if hasParent, err := sess.Where("folder_id=?", dash.FolderId).Get(&parent); err != nil { - return err - } else if hasParent && parent.HasAcl { - dash.HasAcl = true - } - } - - // check if dash has its own acl - if dash.Id > 0 { - if res, err := sess.Query("SELECT 1 from dashboard_acl WHERE dashboard_id =?", dash.Id); err != nil { - return err - } else { - if len(res) > 0 { - dash.HasAcl = true - } - } - } - - return nil -} - func GetDashboard(query *m.GetDashboardQuery) error { dashboard := m.Dashboard{Slug: query.Slug, OrgId: query.OrgId, Id: query.Id, Uid: query.Uid} has, err := x.Get(&dashboard) diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index a1a308d6497..ae91d1d41f3 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -1,16 +1,12 @@ package sqlstore import ( - "time" - "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" ) func init() { - bus.AddHandler("sql", SetDashboardAcl) bus.AddHandler("sql", UpdateDashboardAcl) - bus.AddHandler("sql", RemoveDashboardAcl) bus.AddHandler("sql", GetDashboardAclInfoList) } @@ -23,7 +19,7 @@ func UpdateDashboardAcl(cmd *m.UpdateDashboardAclCommand) error { } for _, item := range cmd.Items { - if item.UserId == 0 && item.TeamId == 0 && !item.Role.IsValid() { + if item.UserId == 0 && item.TeamId == 0 && (item.Role == nil || !item.Role.IsValid()) { return m.ErrDashboardAclInfoMissing } @@ -46,85 +42,6 @@ func UpdateDashboardAcl(cmd *m.UpdateDashboardAclCommand) error { }) } -func SetDashboardAcl(cmd *m.SetDashboardAclCommand) error { - return inTransaction(func(sess *DBSession) error { - if cmd.UserId == 0 && cmd.TeamId == 0 { - return m.ErrDashboardAclInfoMissing - } - - if cmd.DashboardId == 0 { - return m.ErrDashboardPermissionDashboardEmpty - } - - if res, err := sess.Query("SELECT 1 from "+dialect.Quote("dashboard_acl")+" WHERE dashboard_id =? and (team_id=? or user_id=?)", cmd.DashboardId, cmd.TeamId, cmd.UserId); err != nil { - return err - } else if len(res) == 1 { - - entity := m.DashboardAcl{ - Permission: cmd.Permission, - Updated: time.Now(), - } - - if _, err := sess.Cols("updated", "permission").Where("dashboard_id =? and (team_id=? or user_id=?)", cmd.DashboardId, cmd.TeamId, cmd.UserId).Update(&entity); err != nil { - return err - } - - return nil - } - - entity := m.DashboardAcl{ - OrgId: cmd.OrgId, - TeamId: cmd.TeamId, - UserId: cmd.UserId, - Created: time.Now(), - Updated: time.Now(), - DashboardId: cmd.DashboardId, - Permission: cmd.Permission, - } - - cols := []string{"org_id", "created", "updated", "dashboard_id", "permission"} - - if cmd.UserId != 0 { - cols = append(cols, "user_id") - } - - if cmd.TeamId != 0 { - cols = append(cols, "team_id") - } - - _, err := sess.Cols(cols...).Insert(&entity) - if err != nil { - return err - } - - cmd.Result = entity - - // Update dashboard HasAcl flag - dashboard := m.Dashboard{ - HasAcl: true, - } - - if _, err := sess.Cols("has_acl").Where("id=? OR folder_id=?", cmd.DashboardId, cmd.DashboardId).Update(&dashboard); err != nil { - return err - } - - return nil - }) -} - -// RemoveDashboardAcl removes a specified permission from the dashboard acl -func RemoveDashboardAcl(cmd *m.RemoveDashboardAclCommand) error { - return inTransaction(func(sess *DBSession) error { - var rawSQL = "DELETE FROM " + dialect.Quote("dashboard_acl") + " WHERE org_id =? and id=?" - _, err := sess.Exec(rawSQL, cmd.OrgId, cmd.AclId) - if err != nil { - return err - } - - return err - }) -} - // GetDashboardAclInfoList returns a list of permissions for a dashboard. They can be fetched from three // different places. // 1) Permissions for the dashboard diff --git a/pkg/services/sqlstore/dashboard_acl_test.go b/pkg/services/sqlstore/dashboard_acl_test.go index 8d4af9544d9..8fbb9c0d813 100644 --- a/pkg/services/sqlstore/dashboard_acl_test.go +++ b/pkg/services/sqlstore/dashboard_acl_test.go @@ -17,7 +17,7 @@ func TestDashboardAclDataAccess(t *testing.T) { childDash := insertTestDashboard("2 test dash", 1, savedFolder.Id, false, "prod", "webapp") Convey("When adding dashboard permission with userId and teamId set to 0", func() { - err := SetDashboardAcl(&m.SetDashboardAclCommand{ + err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{ OrgId: 1, DashboardId: savedFolder.Id, Permission: m.PERMISSION_EDIT, @@ -59,7 +59,7 @@ func TestDashboardAclDataAccess(t *testing.T) { }) Convey("Given dashboard folder permission", func() { - err := SetDashboardAcl(&m.SetDashboardAclCommand{ + err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{ OrgId: 1, UserId: currentUser.Id, DashboardId: savedFolder.Id, @@ -78,7 +78,7 @@ func TestDashboardAclDataAccess(t *testing.T) { }) Convey("Given child dashboard permission", func() { - err := SetDashboardAcl(&m.SetDashboardAclCommand{ + err := testHelperUpdateDashboardAcl(childDash.Id, m.DashboardAcl{ OrgId: 1, UserId: currentUser.Id, DashboardId: childDash.Id, @@ -100,7 +100,7 @@ func TestDashboardAclDataAccess(t *testing.T) { }) Convey("Given child dashboard permission in folder with no permissions", func() { - err := SetDashboardAcl(&m.SetDashboardAclCommand{ + err := testHelperUpdateDashboardAcl(childDash.Id, m.DashboardAcl{ OrgId: 1, UserId: currentUser.Id, DashboardId: childDash.Id, @@ -125,17 +125,12 @@ func TestDashboardAclDataAccess(t *testing.T) { }) Convey("Should be able to add dashboard permission", func() { - setDashAclCmd := m.SetDashboardAclCommand{ + err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{ OrgId: 1, UserId: currentUser.Id, DashboardId: savedFolder.Id, Permission: m.PERMISSION_EDIT, - } - - err := SetDashboardAcl(&setDashAclCmd) - So(err, ShouldBeNil) - - So(setDashAclCmd.Result.Id, ShouldEqual, 3) + }) q1 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1} err = GetDashboardAclInfoList(q1) @@ -147,42 +142,9 @@ func TestDashboardAclDataAccess(t *testing.T) { So(q1.Result[0].UserId, ShouldEqual, currentUser.Id) So(q1.Result[0].UserLogin, ShouldEqual, currentUser.Login) So(q1.Result[0].UserEmail, ShouldEqual, currentUser.Email) - So(q1.Result[0].Id, ShouldEqual, setDashAclCmd.Result.Id) - - Convey("Should update hasAcl field to true for dashboard folder and its children", func() { - q2 := &m.GetDashboardsQuery{DashboardIds: []int64{savedFolder.Id, childDash.Id}} - err := GetDashboards(q2) - So(err, ShouldBeNil) - So(q2.Result[0].HasAcl, ShouldBeTrue) - So(q2.Result[1].HasAcl, ShouldBeTrue) - }) - - Convey("Should be able to update an existing permission", func() { - err := SetDashboardAcl(&m.SetDashboardAclCommand{ - OrgId: 1, - UserId: 1, - DashboardId: savedFolder.Id, - Permission: m.PERMISSION_ADMIN, - }) - - So(err, ShouldBeNil) - - q3 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1} - err = GetDashboardAclInfoList(q3) - So(err, ShouldBeNil) - So(len(q3.Result), ShouldEqual, 1) - So(q3.Result[0].DashboardId, ShouldEqual, savedFolder.Id) - So(q3.Result[0].Permission, ShouldEqual, m.PERMISSION_ADMIN) - So(q3.Result[0].UserId, ShouldEqual, 1) - - }) Convey("Should be able to delete an existing permission", func() { - err := RemoveDashboardAcl(&m.RemoveDashboardAclCommand{ - OrgId: 1, - AclId: setDashAclCmd.Result.Id, - }) - + err := testHelperUpdateDashboardAcl(savedFolder.Id) So(err, ShouldBeNil) q3 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1} @@ -198,14 +160,12 @@ func TestDashboardAclDataAccess(t *testing.T) { So(err, ShouldBeNil) Convey("Should be able to add a user permission for a team", func() { - setDashAclCmd := m.SetDashboardAclCommand{ + err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{ OrgId: 1, TeamId: group1.Result.Id, DashboardId: savedFolder.Id, Permission: m.PERMISSION_EDIT, - } - - err := SetDashboardAcl(&setDashAclCmd) + }) So(err, ShouldBeNil) q1 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1} @@ -214,23 +174,10 @@ func TestDashboardAclDataAccess(t *testing.T) { So(q1.Result[0].DashboardId, ShouldEqual, savedFolder.Id) So(q1.Result[0].Permission, ShouldEqual, m.PERMISSION_EDIT) So(q1.Result[0].TeamId, ShouldEqual, group1.Result.Id) - - Convey("Should be able to delete an existing permission for a team", func() { - err := RemoveDashboardAcl(&m.RemoveDashboardAclCommand{ - OrgId: 1, - AclId: setDashAclCmd.Result.Id, - }) - - So(err, ShouldBeNil) - q3 := &m.GetDashboardAclInfoListQuery{DashboardId: savedFolder.Id, OrgId: 1} - err = GetDashboardAclInfoList(q3) - So(err, ShouldBeNil) - So(len(q3.Result), ShouldEqual, 0) - }) }) Convey("Should be able to update an existing permission for a team", func() { - err := SetDashboardAcl(&m.SetDashboardAclCommand{ + err := testHelperUpdateDashboardAcl(savedFolder.Id, m.DashboardAcl{ OrgId: 1, TeamId: group1.Result.Id, DashboardId: savedFolder.Id, @@ -246,7 +193,6 @@ func TestDashboardAclDataAccess(t *testing.T) { So(q3.Result[0].Permission, ShouldEqual, m.PERMISSION_ADMIN) So(q3.Result[0].TeamId, ShouldEqual, group1.Result.Id) }) - }) }) diff --git a/pkg/services/sqlstore/dashboard_folder_test.go b/pkg/services/sqlstore/dashboard_folder_test.go index b32a4dfed1d..40d6cf5bcb2 100644 --- a/pkg/services/sqlstore/dashboard_folder_test.go +++ b/pkg/services/sqlstore/dashboard_folder_test.go @@ -41,7 +41,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("and acl is set for dashboard folder", func() { var otherUser int64 = 999 - updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) + testHelperUpdateDashboardAcl(folder.Id, m.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: otherUser, Permission: m.PERMISSION_EDIT}) Convey("should not return folder", func() { query := &search.FindPersistedDashboardsQuery{ @@ -55,7 +55,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { }) Convey("when the user is given permission", func() { - updateTestDashboardWithAcl(folder.Id, currentUser.Id, m.PERMISSION_EDIT) + testHelperUpdateDashboardAcl(folder.Id, m.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: currentUser.Id, Permission: m.PERMISSION_EDIT}) Convey("should be able to access folder", func() { query := &search.FindPersistedDashboardsQuery{ @@ -93,9 +93,8 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("and acl is set for dashboard child and folder has all permissions removed", func() { var otherUser int64 = 999 - aclId := updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) - removeAcl(aclId) - updateTestDashboardWithAcl(childDash.Id, otherUser, m.PERMISSION_EDIT) + testHelperUpdateDashboardAcl(folder.Id) + testHelperUpdateDashboardAcl(childDash.Id, m.DashboardAcl{DashboardId: folder.Id, OrgId: 1, UserId: otherUser, Permission: m.PERMISSION_EDIT}) Convey("should not return folder or child", func() { query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} @@ -106,7 +105,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { }) Convey("when the user is given permission to child", func() { - updateTestDashboardWithAcl(childDash.Id, currentUser.Id, m.PERMISSION_EDIT) + testHelperUpdateDashboardAcl(childDash.Id, m.DashboardAcl{DashboardId: childDash.Id, OrgId: 1, UserId: currentUser.Id, Permission: m.PERMISSION_EDIT}) Convey("should be able to search for child dashboard but not folder", func() { query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1, OrgRole: m.ROLE_VIEWER}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} @@ -165,11 +164,10 @@ func TestDashboardFolderDataAccess(t *testing.T) { Convey("and acl is set for one dashboard folder", func() { var otherUser int64 = 999 - updateTestDashboardWithAcl(folder1.Id, otherUser, m.PERMISSION_EDIT) + testHelperUpdateDashboardAcl(folder1.Id, m.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: otherUser, Permission: m.PERMISSION_EDIT}) Convey("and a dashboard is moved from folder without acl to the folder with an acl", func() { - movedDash := moveDashboard(1, childDash2.Data, folder1.Id) - So(movedDash.HasAcl, ShouldBeTrue) + moveDashboard(1, childDash2.Data, folder1.Id) Convey("should not return folder with acl or its children", func() { query := &search.FindPersistedDashboardsQuery{ @@ -184,9 +182,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { }) }) Convey("and a dashboard is moved from folder with acl to the folder without an acl", func() { - - movedDash := moveDashboard(1, childDash1.Data, folder2.Id) - So(movedDash.HasAcl, ShouldBeFalse) + moveDashboard(1, childDash1.Data, folder2.Id) Convey("should return folder without acl and its children", func() { query := &search.FindPersistedDashboardsQuery{ @@ -205,9 +201,8 @@ func TestDashboardFolderDataAccess(t *testing.T) { }) Convey("and a dashboard with an acl is moved to the folder without an acl", func() { - updateTestDashboardWithAcl(childDash1.Id, otherUser, m.PERMISSION_EDIT) - movedDash := moveDashboard(1, childDash1.Data, folder2.Id) - So(movedDash.HasAcl, ShouldBeTrue) + testHelperUpdateDashboardAcl(childDash1.Id, m.DashboardAcl{DashboardId: childDash1.Id, OrgId: 1, UserId: otherUser, Permission: m.PERMISSION_EDIT}) + moveDashboard(1, childDash1.Data, folder2.Id) Convey("should return folder without acl but not the dashboard with acl", func() { query := &search.FindPersistedDashboardsQuery{ @@ -308,7 +303,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { }) Convey("Should have write access to one dashboard folder if default role changed to view for one folder", func() { - updateTestDashboardWithAcl(folder1.Id, editorUser.Id, m.PERMISSION_VIEW) + testHelperUpdateDashboardAcl(folder1.Id, m.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: editorUser.Id, Permission: m.PERMISSION_VIEW}) err := SearchDashboards(&query) So(err, ShouldBeNil) @@ -352,7 +347,7 @@ func TestDashboardFolderDataAccess(t *testing.T) { }) Convey("Should be able to get one dashboard folder if default role changed to edit for one folder", func() { - updateTestDashboardWithAcl(folder1.Id, viewerUser.Id, m.PERMISSION_EDIT) + testHelperUpdateDashboardAcl(folder1.Id, m.DashboardAcl{DashboardId: folder1.Id, OrgId: 1, UserId: viewerUser.Id, Permission: m.PERMISSION_EDIT}) err := SearchDashboards(&query) So(err, ShouldBeNil) diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index de7cdf19927..7de4c5f5701 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -663,25 +663,6 @@ func createUser(name string, role string, isAdmin bool) m.User { return currentUserCmd.Result } -func updateTestDashboardWithAcl(dashId int64, userId int64, permissions m.PermissionType) int64 { - cmd := &m.SetDashboardAclCommand{ - OrgId: 1, - UserId: userId, - DashboardId: dashId, - Permission: permissions, - } - - err := SetDashboardAcl(cmd) - So(err, ShouldBeNil) - - return cmd.Result.Id -} - -func removeAcl(aclId int64) { - err := RemoveDashboardAcl(&m.RemoveDashboardAclCommand{AclId: aclId, OrgId: 1}) - So(err, ShouldBeNil) -} - func moveDashboard(orgId int64, dashboard *simplejson.Json, newFolderId int64) *m.Dashboard { cmd := m.SaveDashboardCommand{ OrgId: orgId, diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index bebe59f4238..fb76c3fa9d6 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -99,7 +99,7 @@ func TestTeamCommandsAndQueries(t *testing.T) { So(err, ShouldBeNil) err = AddTeamMember(&m.AddTeamMemberCommand{OrgId: testOrgId, TeamId: groupId, UserId: userIds[2]}) So(err, ShouldBeNil) - err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 1, OrgId: testOrgId, Permission: m.PERMISSION_EDIT, TeamId: groupId}) + err = testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: testOrgId, Permission: m.PERMISSION_EDIT, TeamId: groupId}) err = DeleteTeam(&m.DeleteTeamCommand{OrgId: testOrgId, Id: groupId}) So(err, ShouldBeNil) diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index a65b7226eb6..2830733c96a 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -99,7 +99,7 @@ func TestUserDataAccess(t *testing.T) { err = AddOrgUser(&m.AddOrgUserCommand{LoginOrEmail: users[0].Login, Role: m.ROLE_VIEWER, OrgId: users[0].OrgId}) So(err, ShouldBeNil) - err = SetDashboardAcl(&m.SetDashboardAclCommand{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[0].Id, Permission: m.PERMISSION_EDIT}) + testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[0].Id, Permission: m.PERMISSION_EDIT}) So(err, ShouldBeNil) err = SavePreferences(&m.SavePreferencesCommand{UserId: users[0].Id, OrgId: users[0].OrgId, HomeDashboardId: 1, Theme: "dark"}) diff --git a/public/app/stores/PermissionsStore/PermissionsStoreItem.ts b/public/app/stores/PermissionsStore/PermissionsStoreItem.ts index 74769891256..92dca0220ca 100644 --- a/public/app/stores/PermissionsStore/PermissionsStoreItem.ts +++ b/public/app/stores/PermissionsStore/PermissionsStoreItem.ts @@ -1,9 +1,8 @@ -import { types } from 'mobx-state-tree'; +import { types } from 'mobx-state-tree'; export const PermissionsStoreItem = types .model('PermissionsStoreItem', { dashboardId: types.optional(types.number, -1), - id: types.maybe(types.number), permission: types.number, permissionName: types.maybe(types.string), role: types.maybe(types.string), From 73eaba076e4de50289c2403e8ab87a1a4485b213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 14 Feb 2018 15:02:42 +0100 Subject: [PATCH 03/77] wip: dashboard acl ux2, #10747 --- pkg/api/dashboard_acl.go | 2 ++ pkg/models/dashboard_acl.go | 3 +++ pkg/services/sqlstore/dashboard_acl.go | 1 + .../DisabledPermissionsListItem.tsx | 6 +++--- .../components/Permissions/Permissions.tsx | 3 +-- .../Permissions/PermissionsList.tsx | 4 ++-- .../Permissions/PermissionsListItem.tsx | 19 +++++++++++++++---- .../PermissionsStore/PermissionsStore.ts | 11 +++-------- .../PermissionsStore/PermissionsStoreItem.ts | 5 +++-- 9 files changed, 33 insertions(+), 21 deletions(-) diff --git a/pkg/api/dashboard_acl.go b/pkg/api/dashboard_acl.go index 32b75e80cc0..d15a575a05e 100644 --- a/pkg/api/dashboard_acl.go +++ b/pkg/api/dashboard_acl.go @@ -30,6 +30,8 @@ func GetDashboardAclList(c *middleware.Context) Response { } for _, perm := range acl { + perm.UserAvatarUrl = dtos.GetGravatarUrl(perm.UserEmail) + perm.TeamAvatarUrl = dtos.GetGravatarUrl(perm.TeamEmail) if perm.Slug != "" { perm.Url = m.GetDashboardFolderUrl(perm.IsFolder, perm.Uid, perm.Slug) } diff --git a/pkg/models/dashboard_acl.go b/pkg/models/dashboard_acl.go index 202b519207d..0e14a3bfd71 100644 --- a/pkg/models/dashboard_acl.go +++ b/pkg/models/dashboard_acl.go @@ -53,7 +53,10 @@ type DashboardAclInfoDTO struct { UserId int64 `json:"userId"` UserLogin string `json:"userLogin"` UserEmail string `json:"userEmail"` + UserAvatarUrl string `json:"userAvatarUrl"` TeamId int64 `json:"teamId"` + TeamEmail string `json:"teamEmail"` + TeamAvatarUrl string `json:"teamAvatarUrl"` Team string `json:"team"` Role *RoleType `json:"role,omitempty"` Permission PermissionType `json:"permission"` diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index ae91d1d41f3..6e7175335f3 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -92,6 +92,7 @@ func GetDashboardAclInfoList(query *m.GetDashboardAclInfoListQuery) error { u.login AS user_login, u.email AS user_email, ug.name AS team, + ug.email AS team_email, d.title, d.slug, d.uid, diff --git a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx index db45714136e..e3f3ee56d75 100644 --- a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx +++ b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { Component } from 'react'; import DescriptionPicker from 'app/core/components/Picker/DescriptionPicker'; import { permissionOptions } from 'app/stores/PermissionsStore/PermissionsStore'; @@ -12,10 +12,10 @@ export default class DisabledPermissionListItem extends Component { return ( - + - + {item.name} Can diff --git a/public/app/core/components/Permissions/Permissions.tsx b/public/app/core/components/Permissions/Permissions.tsx index 0a0572ed86e..dbdc1682f6b 100644 --- a/public/app/core/components/Permissions/Permissions.tsx +++ b/public/app/core/components/Permissions/Permissions.tsx @@ -15,9 +15,8 @@ export interface DashboardAcl { permissionName?: string; role?: string; icon?: string; - nameHtml?: string; + name?: string; inherited?: boolean; - sortName?: string; sortRank?: number; } diff --git a/public/app/core/components/Permissions/PermissionsList.tsx b/public/app/core/components/Permissions/PermissionsList.tsx index b215dad2391..a77235ecc30 100644 --- a/public/app/core/components/Permissions/PermissionsList.tsx +++ b/public/app/core/components/Permissions/PermissionsList.tsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { Component } from 'react'; import PermissionsListItem from './PermissionsListItem'; import DisabledPermissionsListItem from './DisabledPermissionsListItem'; import { observer } from 'mobx-react'; @@ -23,7 +23,7 @@ class PermissionsList extends Component { Admin Role', + name: 'Admin', permission: 4, icon: 'fa fa-fw fa-street-view', }} diff --git a/public/app/core/components/Permissions/PermissionsListItem.tsx b/public/app/core/components/Permissions/PermissionsListItem.tsx index 3140b8fcc0c..2ab5b948440 100644 --- a/public/app/core/components/Permissions/PermissionsListItem.tsx +++ b/public/app/core/components/Permissions/PermissionsListItem.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React from 'react'; import { observer } from 'mobx-react'; import DescriptionPicker from 'app/core/components/Picker/DescriptionPicker'; import { permissionOptions } from 'app/stores/PermissionsStore/PermissionsStore'; @@ -7,6 +7,16 @@ const setClassNameHelper = inherited => { return inherited ? 'gf-form-disabled' : ''; }; +function ItemAvatar({ item }) { + if (item.userAvatarUrl) { + return ; + } + if (item.teamAvatarUrl) { + return ; + } + return ; +} + export default observer(({ item, removeItem, permissionChanged, itemIndex, folderInfo }) => { const handleRemoveItem = evt => { evt.preventDefault(); @@ -18,13 +28,14 @@ export default observer(({ item, removeItem, permissionChanged, itemIndex, folde }; const inheritedFromRoot = item.dashboardId === -1 && folderInfo && folderInfo.id === 0; + console.log(item.name); return ( - - - + + + {item.name} {item.inherited && folderInfo && ( diff --git a/public/app/stores/PermissionsStore/PermissionsStore.ts b/public/app/stores/PermissionsStore/PermissionsStore.ts index a7c90d13da0..7838744c541 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.ts @@ -231,19 +231,14 @@ const prepareItem = (item, dashboardId: number, isFolder: boolean, isInRoot: boo item.sortRank = 0; if (item.userId > 0) { - item.icon = 'fa fa-fw fa-user'; - item.nameHtml = item.userLogin; - item.sortName = item.userLogin; + item.name = item.userLogin; item.sortRank = 10; } else if (item.teamId > 0) { - item.icon = 'fa fa-fw fa-users'; - item.nameHtml = item.team; - item.sortName = item.team; + item.name = item.team; item.sortRank = 20; } else if (item.role) { item.icon = 'fa fa-fw fa-street-view'; - item.nameHtml = `Everyone with ${item.role} Role`; - item.sortName = item.role; + item.name = item.role; item.sortRank = 30; if (item.role === 'Viewer') { item.sortRank += 1; diff --git a/public/app/stores/PermissionsStore/PermissionsStoreItem.ts b/public/app/stores/PermissionsStore/PermissionsStoreItem.ts index 92dca0220ca..c4873cb9c01 100644 --- a/public/app/stores/PermissionsStore/PermissionsStoreItem.ts +++ b/public/app/stores/PermissionsStore/PermissionsStoreItem.ts @@ -14,8 +14,9 @@ export const PermissionsStoreItem = types inherited: types.maybe(types.boolean), sortRank: types.maybe(types.number), icon: types.maybe(types.string), - nameHtml: types.maybe(types.string), - sortName: types.maybe(types.string), + name: types.maybe(types.string), + teamAvatarUrl: types.maybe(types.string), + userAvatarUrl: types.maybe(types.string), }) .actions(self => ({ updateRole: role => { From e037ef21f790f6ea4aea3c3e0ee29e66375e636c Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 26 Feb 2018 10:21:24 +0100 Subject: [PATCH 04/77] added admin icon and permission member definitions(role,team,user) --- .../Permissions/DisabledPermissionsListItem.tsx | 7 +++++-- .../Permissions/PermissionsListItem.tsx | 16 ++++++++++++++-- public/sass/components/_filter-table.scss | 4 ++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx index e3f3ee56d75..adc2bec3d81 100644 --- a/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx +++ b/public/app/core/components/Permissions/DisabledPermissionsListItem.tsx @@ -13,9 +13,12 @@ export default class DisabledPermissionListItem extends Component { return ( - + + + + {item.name} + (Role) - {item.name} Can diff --git a/public/app/core/components/Permissions/PermissionsListItem.tsx b/public/app/core/components/Permissions/PermissionsListItem.tsx index 2ab5b948440..1bec7003f1f 100644 --- a/public/app/core/components/Permissions/PermissionsListItem.tsx +++ b/public/app/core/components/Permissions/PermissionsListItem.tsx @@ -14,7 +14,17 @@ function ItemAvatar({ item }) { if (item.teamAvatarUrl) { return ; } - return ; + return ; +} + +function ItemDescription({ item }) { + if (item.userId) { + return (User); + } + if (item.teamId) { + return (Team); + } + return (Role); } export default observer(({ item, removeItem, permissionChanged, itemIndex, folderInfo }) => { @@ -35,7 +45,9 @@ export default observer(({ item, removeItem, permissionChanged, itemIndex, folde - {item.name} + + {item.name} + {item.inherited && folderInfo && ( diff --git a/public/sass/components/_filter-table.scss b/public/sass/components/_filter-table.scss index 00f9b93dcfd..bfa9fbbbc5a 100644 --- a/public/sass/components/_filter-table.scss +++ b/public/sass/components/_filter-table.scss @@ -85,3 +85,7 @@ } } } +.filter-table__weak-italic { + font-style: italic; + color: $text-color-weak; +} From d6faa3d06f606410070b38ae78fc6665836358eb Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 27 Feb 2018 18:51:04 +0100 Subject: [PATCH 05/77] provisioning: improve UX when saving provisioned dashboards --- pkg/api/dashboard.go | 7 ++ pkg/api/dtos/dashboard.go | 1 + pkg/models/dashboards.go | 6 ++ pkg/services/provisioning/dashboards/types.go | 3 - .../sqlstore/dashboard_provisioning.go | 13 ++++ .../sqlstore/dashboard_provisioning_test.go | 10 +++ public/app/features/dashboard/all.ts | 1 + .../app/features/dashboard/dashboard_srv.ts | 11 +++ .../features/dashboard/dashnav/dashnav.html | 2 +- .../dashboard/save_provisioned_modal.ts | 74 +++++++++++++++++++ .../specs/save_provisioned_modal.jest.ts | 28 +++++++ 11 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 public/app/features/dashboard/save_provisioned_modal.ts create mode 100644 public/app/features/dashboard/specs/save_provisioned_modal.jest.ts diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 11a028cdd29..e5e4fc560e1 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -102,6 +102,13 @@ func GetDashboard(c *m.ReqContext) Response { meta.FolderUrl = query.Result.GetUrl() } + dpQuery := &m.GetProvisionedDashboardByDashboardId{DashboardId: dash.Id} + err = bus.Dispatch(dpQuery) + if dpQuery.Result != nil { + meta.CanEdit = true + meta.Provisioned = true + } + // make sure db version is in sync with json model version dash.Data.Set("version", dash.Version) diff --git a/pkg/api/dtos/dashboard.go b/pkg/api/dtos/dashboard.go index e4c66aebbda..39a6dca580d 100644 --- a/pkg/api/dtos/dashboard.go +++ b/pkg/api/dtos/dashboard.go @@ -28,6 +28,7 @@ type DashboardMeta struct { FolderId int64 `json:"folderId"` FolderTitle string `json:"folderTitle"` FolderUrl string `json:"folderUrl"` + Provisioned bool `json:"provisioned"` } type DashboardFullWithMeta struct { diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 4b771038df6..e4f0758fc19 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -317,6 +317,12 @@ type GetDashboardSlugByIdQuery struct { Result string } +type GetProvisionedDashboardByDashboardId struct { + DashboardId int64 + + Result *DashboardProvisioning +} + type GetProvisionedDashboardDataQuery struct { Name string diff --git a/pkg/services/provisioning/dashboards/types.go b/pkg/services/provisioning/dashboards/types.go index f742b321552..4a55351d3e4 100644 --- a/pkg/services/provisioning/dashboards/types.go +++ b/pkg/services/provisioning/dashboards/types.go @@ -55,9 +55,6 @@ func createDashboardJson(data *simplejson.Json, lastModified time.Time, cfg *Das dash.OrgId = cfg.OrgId dash.Dashboard.OrgId = cfg.OrgId dash.Dashboard.FolderId = folderId - if !cfg.Editable { - dash.Dashboard.Data.Set("editable", cfg.Editable) - } if dash.Dashboard.Title == "" { return nil, models.ErrDashboardTitleEmpty diff --git a/pkg/services/sqlstore/dashboard_provisioning.go b/pkg/services/sqlstore/dashboard_provisioning.go index 69409c3b873..99178d38f9c 100644 --- a/pkg/services/sqlstore/dashboard_provisioning.go +++ b/pkg/services/sqlstore/dashboard_provisioning.go @@ -8,6 +8,7 @@ import ( func init() { bus.AddHandler("sql", GetProvisionedDashboardDataQuery) bus.AddHandler("sql", SaveProvisionedDashboard) + bus.AddHandler("sql", GetProvisionedDataByDashboardId) } type DashboardExtras struct { @@ -17,6 +18,18 @@ type DashboardExtras struct { Value string } +func GetProvisionedDataByDashboardId(cmd *models.GetProvisionedDashboardByDashboardId) error { + result := &models.DashboardProvisioning{} + + _, err := x.Where("dashboard_id = ?", cmd.DashboardId).Get(result) + if err != nil { + return err + } + + cmd.Result = result + return nil +} + func SaveProvisionedDashboard(cmd *models.SaveProvisionedDashboardCommand) error { return inTransaction(func(sess *DBSession) error { err := saveDashboard(sess, cmd.DashboardCmd) diff --git a/pkg/services/sqlstore/dashboard_provisioning_test.go b/pkg/services/sqlstore/dashboard_provisioning_test.go index b752173b67d..89b3451a3ac 100644 --- a/pkg/services/sqlstore/dashboard_provisioning_test.go +++ b/pkg/services/sqlstore/dashboard_provisioning_test.go @@ -50,6 +50,16 @@ func TestDashboardProvisioningTest(t *testing.T) { So(query.Result[0].DashboardId, ShouldEqual, dashId) So(query.Result[0].Updated, ShouldEqual, now.Unix()) }) + + Convey("Can query for one provisioned dashboard", func() { + query := &models.GetProvisionedDashboardByDashboardId{DashboardId: cmd.Result.Id} + + err := GetProvisionedDataByDashboardId(query) + So(err, ShouldBeNil) + + So(query.Result.DashboardId, ShouldEqual, cmd.Result.Id) + So(query.Result.Updated, ShouldEqual, now.Unix()) + }) }) }) } diff --git a/public/app/features/dashboard/all.ts b/public/app/features/dashboard/all.ts index f2e2e3dcdc0..a8f491f3ddd 100644 --- a/public/app/features/dashboard/all.ts +++ b/public/app/features/dashboard/all.ts @@ -6,6 +6,7 @@ import './dashnav/dashnav'; import './submenu/submenu'; import './save_as_modal'; import './save_modal'; +import './save_provisioned_modal'; import './shareModalCtrl'; import './share_snapshot_ctrl'; import './dashboard_srv'; diff --git a/public/app/features/dashboard/dashboard_srv.ts b/public/app/features/dashboard/dashboard_srv.ts index 9d766fdfc3f..3aa7ca118fb 100644 --- a/public/app/features/dashboard/dashboard_srv.ts +++ b/public/app/features/dashboard/dashboard_srv.ts @@ -105,6 +105,10 @@ export class DashboardSrv { this.setCurrent(this.create(clone, this.dash.meta)); } + if (this.dash.meta.provisioned) { + return this.showDashboardProvisionedModal(); + } + if (!this.dash.meta.canSave && options.makeEditable !== true) { return Promise.resolve(); } @@ -120,6 +124,13 @@ export class DashboardSrv { return this.save(this.dash.getSaveModelClone(), options); } + showDashboardProvisionedModal() { + this.$rootScope.appEvent('show-modal', { + templateHtml: '', + modalClass: 'modal--narrow', + }); + } + showSaveAsModal() { this.$rootScope.appEvent('show-modal', { templateHtml: '', diff --git a/public/app/features/dashboard/dashnav/dashnav.html b/public/app/features/dashboard/dashnav/dashnav.html index 269d4b0bada..0c3f949ed7c 100644 --- a/public/app/features/dashboard/dashnav/dashnav.html +++ b/public/app/features/dashboard/dashnav/dashnav.html @@ -17,7 +17,7 @@ +

PostgreSQL details

+ +
+
+ + Version + + This option controls what functions are used when expanding grafana macros. + + + + + +
+
+
User Permission
From 9b61ffb48ad9544b52b35f5870bfca1db098ae47 Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Sun, 15 Apr 2018 18:38:20 +0200 Subject: [PATCH 31/77] make timefilter macro aware of pg version --- pkg/tsdb/postgres/macros.go | 12 +++++- pkg/tsdb/postgres/macros_test.go | 65 +++++++++++++++++++++++++++++--- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/pkg/tsdb/postgres/macros.go b/pkg/tsdb/postgres/macros.go index bd0ac0cc620..b9a7580b3ce 100644 --- a/pkg/tsdb/postgres/macros.go +++ b/pkg/tsdb/postgres/macros.go @@ -79,11 +79,19 @@ func (m *PostgresMacroEngine) evaluateMacro(name string, args []string) (string, } return fmt.Sprintf("extract(epoch from %s) as \"time\"", args[0]), nil case "__timeFilter": - // dont use to_timestamp in this macro for redshift compatibility #9566 if len(args) == 0 { return "", fmt.Errorf("missing time column argument for macro %v", name) } - return fmt.Sprintf("extract(epoch from %s) BETWEEN %d AND %d", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil + + pg_version := m.Query.DataSource.JsonData.Get("postgresVersion").MustInt(0) + if pg_version >= 80100 { + // postgres has to_timestamp(double) starting with 8.1 + return fmt.Sprintf("%s BETWEEN to_timestamp(%d) AND to_timestamp(%d)", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil + } + + // dont use to_timestamp in this macro for redshift compatibility #9566 + return fmt.Sprintf("%s BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil + case "__timeFrom": return fmt.Sprintf("to_timestamp(%d)", m.TimeRange.GetFromAsSecondsEpoch()), nil case "__timeTo": diff --git a/pkg/tsdb/postgres/macros_test.go b/pkg/tsdb/postgres/macros_test.go index f441690a429..b4ee043a87d 100644 --- a/pkg/tsdb/postgres/macros_test.go +++ b/pkg/tsdb/postgres/macros_test.go @@ -6,14 +6,27 @@ import ( "testing" "time" + "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/tsdb" . "github.com/smartystreets/goconvey/convey" ) func TestMacroEngine(t *testing.T) { Convey("MacroEngine", t, func() { - engine := &PostgresMacroEngine{} - query := &tsdb.Query{} + engine := NewPostgresMacroEngine() + // datasource with no pg version specified + ds := &models.DataSource{Id: 1, Type: "postgres", JsonData: simplejson.New()} + // datasource with postgres 8.0 configured + ds_80 := &models.DataSource{Id: 2, Type: "postgres", JsonData: simplejson.New()} + ds_80.JsonData.Set("postgresVersion", 80000) + // datasource with postgres 8.1 configured + ds_81 := &models.DataSource{Id: 3, Type: "postgres", JsonData: simplejson.New()} + ds_81.JsonData.Set("postgresVersion", 80100) + + query := &tsdb.Query{RefId: "A", DataSource: ds} + query_80 := &tsdb.Query{RefId: "A", DataSource: ds_80} + query_81 := &tsdb.Query{RefId: "A", DataSource: ds_81} Convey("Given a time range between 2018-04-12 00:00 and 2018-04-12 00:05", func() { from := time.Date(2018, 4, 12, 18, 0, 0, 0, time.UTC) @@ -38,7 +51,21 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE extract(epoch from time_column) BETWEEN %d AND %d", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for postgres 8.0", func() { + sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for postgres 8.1", func() { + sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) }) Convey("interpolate __timeFrom function", func() { @@ -102,7 +129,21 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE extract(epoch from time_column) BETWEEN %d AND %d", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for 8.0", func() { + sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for 8.1", func() { + sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) }) Convey("interpolate __timeFrom function", func() { @@ -150,7 +191,21 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE extract(epoch from time_column) BETWEEN %d AND %d", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for 8.0", func() { + sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) + }) + + Convey("interpolate __timeFilter function for 8.1", func() { + sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") + So(err, ShouldBeNil) + + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) }) Convey("interpolate __timeFrom function", func() { From 6d3da9a73df9f3cc74648d2913555437d9bbea1a Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Sun, 15 Apr 2018 22:14:13 +0200 Subject: [PATCH 32/77] remove postgresversion and convert unix timestamp in go --- pkg/tsdb/postgres/macros.go | 10 +-- pkg/tsdb/postgres/macros_test.go | 63 ++----------------- .../app/plugins/datasource/postgres/module.ts | 5 -- .../datasource/postgres/partials/config.html | 16 ----- 4 files changed, 5 insertions(+), 89 deletions(-) diff --git a/pkg/tsdb/postgres/macros.go b/pkg/tsdb/postgres/macros.go index b9a7580b3ce..a13912f0e1d 100644 --- a/pkg/tsdb/postgres/macros.go +++ b/pkg/tsdb/postgres/macros.go @@ -83,15 +83,7 @@ func (m *PostgresMacroEngine) evaluateMacro(name string, args []string) (string, return "", fmt.Errorf("missing time column argument for macro %v", name) } - pg_version := m.Query.DataSource.JsonData.Get("postgresVersion").MustInt(0) - if pg_version >= 80100 { - // postgres has to_timestamp(double) starting with 8.1 - return fmt.Sprintf("%s BETWEEN to_timestamp(%d) AND to_timestamp(%d)", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil - } - - // dont use to_timestamp in this macro for redshift compatibility #9566 - return fmt.Sprintf("%s BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", args[0], m.TimeRange.GetFromAsSecondsEpoch(), m.TimeRange.GetToAsSecondsEpoch()), nil - + return fmt.Sprintf("%s BETWEEN '%s' AND '%s'", args[0], m.TimeRange.MustGetFrom().UTC().Format(time.RFC3339), m.TimeRange.MustGetTo().UTC().Format(time.RFC3339)), nil case "__timeFrom": return fmt.Sprintf("to_timestamp(%d)", m.TimeRange.GetFromAsSecondsEpoch()), nil case "__timeTo": diff --git a/pkg/tsdb/postgres/macros_test.go b/pkg/tsdb/postgres/macros_test.go index b4ee043a87d..d1bcaff796d 100644 --- a/pkg/tsdb/postgres/macros_test.go +++ b/pkg/tsdb/postgres/macros_test.go @@ -6,8 +6,6 @@ import ( "testing" "time" - "github.com/grafana/grafana/pkg/components/simplejson" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/tsdb" . "github.com/smartystreets/goconvey/convey" ) @@ -15,18 +13,7 @@ import ( func TestMacroEngine(t *testing.T) { Convey("MacroEngine", t, func() { engine := NewPostgresMacroEngine() - // datasource with no pg version specified - ds := &models.DataSource{Id: 1, Type: "postgres", JsonData: simplejson.New()} - // datasource with postgres 8.0 configured - ds_80 := &models.DataSource{Id: 2, Type: "postgres", JsonData: simplejson.New()} - ds_80.JsonData.Set("postgresVersion", 80000) - // datasource with postgres 8.1 configured - ds_81 := &models.DataSource{Id: 3, Type: "postgres", JsonData: simplejson.New()} - ds_81.JsonData.Set("postgresVersion", 80100) - - query := &tsdb.Query{RefId: "A", DataSource: ds} - query_80 := &tsdb.Query{RefId: "A", DataSource: ds_80} - query_81 := &tsdb.Query{RefId: "A", DataSource: ds_81} + query := &tsdb.Query{} Convey("Given a time range between 2018-04-12 00:00 and 2018-04-12 00:05", func() { from := time.Date(2018, 4, 12, 18, 0, 0, 0, time.UTC) @@ -51,21 +38,7 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for postgres 8.0", func() { - sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for postgres 8.1", func() { - sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN '%s' AND '%s'", from.Format(time.RFC3339), to.Format(time.RFC3339))) }) Convey("interpolate __timeFrom function", func() { @@ -129,21 +102,7 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for 8.0", func() { - sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for 8.1", func() { - sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN '%s' AND '%s'", from.Format(time.RFC3339), to.Format(time.RFC3339))) }) Convey("interpolate __timeFrom function", func() { @@ -191,21 +150,7 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "WHERE $__timeFilter(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for 8.0", func() { - sql, err := engine.Interpolate(query_80, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN 'epoch'::timestamptz + %d * '1s'::interval AND 'epoch'::timestamptz + %d * '1s'::interval", from.Unix(), to.Unix())) - }) - - Convey("interpolate __timeFilter function for 8.1", func() { - sql, err := engine.Interpolate(query_81, timeRange, "WHERE $__timeFilter(time_column)") - So(err, ShouldBeNil) - - So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN to_timestamp(%d) AND to_timestamp(%d)", from.Unix(), to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("WHERE time_column BETWEEN '%s' AND '%s'", from.Format(time.RFC3339), to.Format(time.RFC3339))) }) Convey("interpolate __timeFrom function", func() { diff --git a/public/app/plugins/datasource/postgres/module.ts b/public/app/plugins/datasource/postgres/module.ts index 766e7b2ec37..9deb7909167 100644 --- a/public/app/plugins/datasource/postgres/module.ts +++ b/public/app/plugins/datasource/postgres/module.ts @@ -11,11 +11,6 @@ class PostgresConfigCtrl { this.current.jsonData.sslmode = this.current.jsonData.sslmode || 'verify-full'; } - /* the values are chosen to be equivalent to `select current_setting('server_version_num');` */ - postgresVersions = [ - { name: '8.0+', value: 80000 }, - { name: '8.1+', value: 80100 }, - ]; } const defaultQuery = `SELECT diff --git a/public/app/plugins/datasource/postgres/partials/config.html b/public/app/plugins/datasource/postgres/partials/config.html index 51fd66d7ed6..77f0dcfa4a5 100644 --- a/public/app/plugins/datasource/postgres/partials/config.html +++ b/public/app/plugins/datasource/postgres/partials/config.html @@ -38,22 +38,6 @@
-

PostgreSQL details

- -
-
- - Version - - This option controls what functions are used when expanding grafana macros. - - - - - -
-
-
User Permission
From 6b4ef7f5981811651e010bce19346b2500f78a05 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 16 Apr 2018 10:42:39 +0200 Subject: [PATCH 33/77] wip: writing tests for permission sorting --- .../PermissionsStore/PermissionsStore.jest.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index c3bc6016e50..2bb2f1b6a0c 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -17,6 +17,22 @@ describe('PermissionsStore', () => { teamId: 1, teamName: 'MyTestTeam', }, + { + id: 5, + dashboardId: 10, + permission: 1, + permissionName: 'View', + userId: 1, + userName: 'MyTestUser', + }, + { + id: 6, + dashboardId: 10, + permission: 1, + permissionName: 'Edit', + teamId: 2, + teamName: 'MyTestTeam2', + }, ]) ); @@ -32,7 +48,10 @@ describe('PermissionsStore', () => { } ); + console.log(store); + await store.load(1, false, false); + console.log(store); }); it('should save update on permission change', async () => { From aff336d4e7c104391033184d6117a2d9f77e6e62 Mon Sep 17 00:00:00 2001 From: Mario Trangoni Date: Mon, 16 Apr 2018 11:44:50 +0200 Subject: [PATCH 34/77] add codespell to circleci --- .circleci/config.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index cfa8b762e49..fc04e69af6e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,6 +1,18 @@ version: 2 jobs: + codespell: + docker: + - image: circleci/python + steps: + - checkout + - run: + name: install codespell + command: 'sudo pip install codespell' + - run: + name: check documentation spelling errors + command: 'codespell -x docs/sources/project/building_from_source.md docs/' + test-frontend: docker: - image: circleci/node:6.11.4 @@ -103,6 +115,10 @@ workflows: version: 2 test-and-build: jobs: + - codespell: + filters: + tags: + only: /.*/ - build: filters: tags: From ce3dcadfeffbd12b02b014ebf1314e033720fe36 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 16 Apr 2018 14:30:50 +0200 Subject: [PATCH 35/77] addeds test for sort order --- .../PermissionsStore/PermissionsStore.jest.ts | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index 2bb2f1b6a0c..0bfcadb4874 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -11,27 +11,27 @@ describe('PermissionsStore', () => { { id: 3, dashboardId: 1, role: 'Editor', permission: 1, permissionName: 'Edit' }, { id: 4, - dashboardId: 10, + dashboardId: 1, permission: 1, permissionName: 'View', teamId: 1, - teamName: 'MyTestTeam', + team: 'MyTestTeam', }, { id: 5, - dashboardId: 10, + dashboardId: 1, permission: 1, permissionName: 'View', userId: 1, - userName: 'MyTestUser', + userLogin: 'MyTestUser', }, { id: 6, - dashboardId: 10, + dashboardId: 1, permission: 1, permissionName: 'Edit', teamId: 2, - teamName: 'MyTestTeam2', + team: 'MyTestTeam2', }, ]) ); @@ -48,15 +48,12 @@ describe('PermissionsStore', () => { } ); - console.log(store); - await store.load(1, false, false); - console.log(store); }); it('should save update on permission change', async () => { expect(store.items[0].permission).toBe(1); - expect(store.items[0].permissionName).toBe('View'); + expect(store.items[0].permissionName).toBe('Edit'); await store.updatePermissionOnIndex(0, 2, 'Edit'); @@ -67,15 +64,20 @@ describe('PermissionsStore', () => { }); it('should save removed permissions automatically', async () => { - expect(store.items.length).toBe(3); + expect(store.items.length).toBe(5); await store.removeStoreItem(2); - expect(store.items.length).toBe(2); + expect(store.items.length).toBe(4); expect(backendSrv.post.mock.calls.length).toBe(1); expect(backendSrv.post.mock.calls[0][0]).toBe('/api/dashboards/id/1/permissions'); }); + it('should be sorted by sort rank and alphabetically', async () => { + expect(store.items[3].name).toBe('MyTestTeam2'); + expect(store.items[4].name).toBe('MyTestUser'); + }); + describe('when one inherited and one not inherited team permission are added', () => { beforeEach(async () => { const overridingItemForChildDashboard = { @@ -92,7 +94,7 @@ describe('PermissionsStore', () => { }); it('should add new overriding permission', () => { - expect(store.items.length).toBe(4); + expect(store.items.length).toBe(6); }); }); }); From 5200196092091972983c0e3dbed2c5c263e98d51 Mon Sep 17 00:00:00 2001 From: Patrick O'Carroll Date: Mon, 16 Apr 2018 14:49:47 +0200 Subject: [PATCH 36/77] added fix for test --- public/app/stores/PermissionsStore/PermissionsStore.jest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index 0bfcadb4874..24f1705367a 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -11,7 +11,7 @@ describe('PermissionsStore', () => { { id: 3, dashboardId: 1, role: 'Editor', permission: 1, permissionName: 'Edit' }, { id: 4, - dashboardId: 1, + dashboardId: 10, permission: 1, permissionName: 'View', teamId: 1, @@ -53,7 +53,7 @@ describe('PermissionsStore', () => { it('should save update on permission change', async () => { expect(store.items[0].permission).toBe(1); - expect(store.items[0].permissionName).toBe('Edit'); + expect(store.items[0].permissionName).toBe('View'); await store.updatePermissionOnIndex(0, 2, 'Edit'); From 9ca1a94580cc5280939a6769013b5ca4dab3dabc Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Mon, 16 Apr 2018 16:52:44 +0200 Subject: [PATCH 37/77] bra should use the proper build script * `bra run` is used during development, but it was not using build.go which sets a couple of globals, e.g., the commit (currently set to "NA") * This will help in reporting the commit that someone is basing their screenshot on. --- .bra.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.bra.toml b/.bra.toml index 36421a3c5a6..053ae78150a 100644 --- a/.bra.toml +++ b/.bra.toml @@ -1,6 +1,6 @@ [run] init_cmds = [ - ["go", "build", "-o", "./bin/grafana-server", "./pkg/cmd/grafana-server"], + ["go", "run", "build.go", "build"], ["./bin/grafana-server", "cfg:app_mode=development"] ] watch_all = true @@ -12,6 +12,6 @@ watch_dirs = [ watch_exts = [".go", ".ini", ".toml"] build_delay = 1500 cmds = [ - ["go", "build", "-o", "./bin/grafana-server", "./pkg/cmd/grafana-server"], + ["go", "run", "build.go", "build"], ["./bin/grafana-server", "cfg:app_mode=development"] ] From 9573bc43ceb584f3b1fbf1b5cb81bb40531af20f Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 16 Apr 2018 17:16:35 +0200 Subject: [PATCH 38/77] dashboard: better size and alignment of settings icons --- .../sass/components/_dashboard_settings.scss | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/public/sass/components/_dashboard_settings.scss b/public/sass/components/_dashboard_settings.scss index 6be7fab53e5..2c709f1ddf9 100644 --- a/public/sass/components/_dashboard_settings.scss +++ b/public/sass/components/_dashboard_settings.scss @@ -3,20 +3,6 @@ height: 100%; display: flex; flex-direction: row; - - //fix for fontawesome icons - .fa-lock { - &::before { - font-size: 20px; - text-align: center; - padding-left: 1px; - } - } - .fa-history { - &::before { - font-size: 17px; - } - } } .dashboard-page--settings-opening { @@ -78,8 +64,13 @@ background: $page-bg; } - i { - padding-right: 5px; + .gicon { + margin-bottom: 2px; + } + + .fa { + font-size: 17px; + width: 16px; } } From e8e554b3da65aeb0007ca6fe93ae07eb22102362 Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Mon, 16 Apr 2018 19:14:25 +0200 Subject: [PATCH 39/77] remove changes to module.ts from this branch --- public/app/plugins/datasource/postgres/module.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/public/app/plugins/datasource/postgres/module.ts b/public/app/plugins/datasource/postgres/module.ts index 9deb7909167..acd23318b6d 100644 --- a/public/app/plugins/datasource/postgres/module.ts +++ b/public/app/plugins/datasource/postgres/module.ts @@ -8,9 +8,8 @@ class PostgresConfigCtrl { /** @ngInject **/ constructor($scope) { - this.current.jsonData.sslmode = this.current.jsonData.sslmode || 'verify-full'; + this.current.jsonData.sslmode = this.current.jsonData.sslmode || 'require'; } - } const defaultQuery = `SELECT From a2a7d3d436aa3e3da4d38aeef06d880f1e25c888 Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Mon, 16 Apr 2018 19:38:35 +0200 Subject: [PATCH 40/77] add GetFromAsTimeUTC and GetToAsTimeUTC and use them in timeFilter macro --- pkg/tsdb/postgres/macros.go | 2 +- pkg/tsdb/time_range.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/tsdb/postgres/macros.go b/pkg/tsdb/postgres/macros.go index a13912f0e1d..e666d20ae06 100644 --- a/pkg/tsdb/postgres/macros.go +++ b/pkg/tsdb/postgres/macros.go @@ -83,7 +83,7 @@ func (m *PostgresMacroEngine) evaluateMacro(name string, args []string) (string, return "", fmt.Errorf("missing time column argument for macro %v", name) } - return fmt.Sprintf("%s BETWEEN '%s' AND '%s'", args[0], m.TimeRange.MustGetFrom().UTC().Format(time.RFC3339), m.TimeRange.MustGetTo().UTC().Format(time.RFC3339)), nil + return fmt.Sprintf("%s BETWEEN '%s' AND '%s'", args[0], m.TimeRange.GetFromAsTimeUTC().Format(time.RFC3339), m.TimeRange.GetToAsTimeUTC().Format(time.RFC3339)), nil case "__timeFrom": return fmt.Sprintf("to_timestamp(%d)", m.TimeRange.GetFromAsSecondsEpoch()), nil case "__timeTo": diff --git a/pkg/tsdb/time_range.go b/pkg/tsdb/time_range.go index 3bd4e228999..777fd15907e 100644 --- a/pkg/tsdb/time_range.go +++ b/pkg/tsdb/time_range.go @@ -37,6 +37,10 @@ func (tr *TimeRange) GetFromAsSecondsEpoch() int64 { return tr.GetFromAsMsEpoch() / 1000 } +func (tr *TimeRange) GetFromAsTimeUTC() time.Time { + return tr.MustGetFrom().UTC() +} + func (tr *TimeRange) GetToAsMsEpoch() int64 { return tr.MustGetTo().UnixNano() / int64(time.Millisecond) } @@ -45,6 +49,10 @@ func (tr *TimeRange) GetToAsSecondsEpoch() int64 { return tr.GetToAsMsEpoch() / 1000 } +func (tr *TimeRange) GetToAsTimeUTC() time.Time { + return tr.MustGetTo().UTC() +} + func (tr *TimeRange) MustGetFrom() time.Time { if res, err := tr.ParseFrom(); err != nil { return time.Unix(0, 0) From ce941a004d6445f1ff809451301d237756798818 Mon Sep 17 00:00:00 2001 From: Mario Trangoni Date: Mon, 16 Apr 2018 20:04:58 +0200 Subject: [PATCH 41/77] fix unconvert issues --- pkg/cmd/grafana-cli/services/services.go | 2 +- pkg/components/apikeygen/apikeygen.go | 4 ++-- pkg/components/imguploader/azureblobuploader.go | 2 +- pkg/components/null/float.go | 2 +- pkg/metrics/metrics.go | 2 +- pkg/models/datasource_cache.go | 2 +- pkg/plugins/models.go | 2 +- pkg/plugins/update_checker.go | 2 +- pkg/services/alerting/notifiers/line.go | 2 +- pkg/tsdb/cloudwatch/annotation_query.go | 4 ++-- pkg/tsdb/prometheus/prometheus.go | 4 ++-- 11 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/grafana-cli/services/services.go b/pkg/cmd/grafana-cli/services/services.go index d13e90d6a2f..3745dbff90e 100644 --- a/pkg/cmd/grafana-cli/services/services.go +++ b/pkg/cmd/grafana-cli/services/services.go @@ -42,7 +42,7 @@ func Init(version string, skipTLSVerify bool) { } HttpClient = http.Client{ - Timeout: time.Duration(10 * time.Second), + Timeout: 10 * time.Second, Transport: tr, } } diff --git a/pkg/components/apikeygen/apikeygen.go b/pkg/components/apikeygen/apikeygen.go index 310188a80ef..7824cf7667f 100644 --- a/pkg/components/apikeygen/apikeygen.go +++ b/pkg/components/apikeygen/apikeygen.go @@ -33,7 +33,7 @@ func New(orgId int64, name string) KeyGenResult { jsonString, _ := json.Marshal(jsonKey) - result.ClientSecret = base64.StdEncoding.EncodeToString([]byte(jsonString)) + result.ClientSecret = base64.StdEncoding.EncodeToString(jsonString) return result } @@ -44,7 +44,7 @@ func Decode(keyString string) (*ApiKeyJson, error) { } var keyObj ApiKeyJson - err = json.Unmarshal([]byte(jsonString), &keyObj) + err = json.Unmarshal(jsonString, &keyObj) if err != nil { return nil, ErrInvalidApiKey } diff --git a/pkg/components/imguploader/azureblobuploader.go b/pkg/components/imguploader/azureblobuploader.go index 40d2de836be..3c0ac5b8884 100644 --- a/pkg/components/imguploader/azureblobuploader.go +++ b/pkg/components/imguploader/azureblobuploader.go @@ -225,7 +225,7 @@ func (a *Auth) SignRequest(req *http.Request) { ) decodedKey, _ := base64.StdEncoding.DecodeString(a.Key) - sha256 := hmac.New(sha256.New, []byte(decodedKey)) + sha256 := hmac.New(sha256.New, decodedKey) sha256.Write([]byte(strToSign)) signature := base64.StdEncoding.EncodeToString(sha256.Sum(nil)) diff --git a/pkg/components/null/float.go b/pkg/components/null/float.go index 1e78946e878..caf4d8b677c 100644 --- a/pkg/components/null/float.go +++ b/pkg/components/null/float.go @@ -50,7 +50,7 @@ func (f *Float) UnmarshalJSON(data []byte) error { } switch x := v.(type) { case float64: - f.Float64 = float64(x) + f.Float64 = x case map[string]interface{}: err = json.Unmarshal(data, &f.NullFloat64) case nil: diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 4d4a11d0faa..4cefe12d6e5 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -403,6 +403,6 @@ func sendUsageStats() { out, _ := json.MarshalIndent(report, "", " ") data := bytes.NewBuffer(out) - client := http.Client{Timeout: time.Duration(5 * time.Second)} + client := http.Client{Timeout: 5 * time.Second} go client.Post("https://stats.grafana.org/grafana-usage-report", "application/json", data) } diff --git a/pkg/models/datasource_cache.go b/pkg/models/datasource_cache.go index b4a4e7f8a4d..66ba66e4d39 100644 --- a/pkg/models/datasource_cache.go +++ b/pkg/models/datasource_cache.go @@ -33,7 +33,7 @@ func (ds *DataSource) GetHttpClient() (*http.Client, error) { } return &http.Client{ - Timeout: time.Duration(30 * time.Second), + Timeout: 30 * time.Second, Transport: transport, }, nil } diff --git a/pkg/plugins/models.go b/pkg/plugins/models.go index 541b37c8a8a..9677c21ef04 100644 --- a/pkg/plugins/models.go +++ b/pkg/plugins/models.go @@ -69,7 +69,7 @@ func (pb *PluginBase) registerPlugin(pluginDir string) error { for _, include := range pb.Includes { if include.Role == "" { - include.Role = m.RoleType(m.ROLE_VIEWER) + include.Role = m.ROLE_VIEWER } } diff --git a/pkg/plugins/update_checker.go b/pkg/plugins/update_checker.go index 68ccdeaf840..946d215b1c2 100644 --- a/pkg/plugins/update_checker.go +++ b/pkg/plugins/update_checker.go @@ -13,7 +13,7 @@ import ( ) var ( - httpClient http.Client = http.Client{Timeout: time.Duration(10 * time.Second)} + httpClient http.Client = http.Client{Timeout: 10 * time.Second} ) type GrafanaNetPlugin struct { diff --git a/pkg/services/alerting/notifiers/line.go b/pkg/services/alerting/notifiers/line.go index 4fbaa2d543e..4814662f3a9 100644 --- a/pkg/services/alerting/notifiers/line.go +++ b/pkg/services/alerting/notifiers/line.go @@ -90,7 +90,7 @@ func (this *LineNotifier) createAlert(evalContext *alerting.EvalContext) error { } if err := bus.DispatchCtx(evalContext.Ctx, cmd); err != nil { - this.log.Error("Failed to send notification to LINE", "error", err, "body", string(body)) + this.log.Error("Failed to send notification to LINE", "error", err, "body", body) return err } diff --git a/pkg/tsdb/cloudwatch/annotation_query.go b/pkg/tsdb/cloudwatch/annotation_query.go index 287f4e770ef..e0d9158435e 100644 --- a/pkg/tsdb/cloudwatch/annotation_query.go +++ b/pkg/tsdb/cloudwatch/annotation_query.go @@ -72,7 +72,7 @@ func (e *CloudWatchExecutor) executeAnnotationQuery(ctx context.Context, queryCo MetricName: aws.String(metricName), Dimensions: qd, Statistic: aws.String(s), - Period: aws.Int64(int64(period)), + Period: aws.Int64(period), } resp, err := svc.DescribeAlarmsForMetric(params) if err != nil { @@ -88,7 +88,7 @@ func (e *CloudWatchExecutor) executeAnnotationQuery(ctx context.Context, queryCo MetricName: aws.String(metricName), Dimensions: qd, ExtendedStatistic: aws.String(s), - Period: aws.Int64(int64(period)), + Period: aws.Int64(period), } resp, err := svc.DescribeAlarmsForMetric(params) if err != nil { diff --git a/pkg/tsdb/prometheus/prometheus.go b/pkg/tsdb/prometheus/prometheus.go index 1186fccbbf9..bf9fe9f152c 100644 --- a/pkg/tsdb/prometheus/prometheus.go +++ b/pkg/tsdb/prometheus/prometheus.go @@ -108,8 +108,8 @@ func (e *PrometheusExecutor) Query(ctx context.Context, dsInfo *models.DataSourc span, ctx := opentracing.StartSpanFromContext(ctx, "alerting.prometheus") span.SetTag("expr", query.Expr) - span.SetTag("start_unixnano", int64(query.Start.UnixNano())) - span.SetTag("stop_unixnano", int64(query.End.UnixNano())) + span.SetTag("start_unixnano", query.Start.UnixNano()) + span.SetTag("stop_unixnano", query.End.UnixNano()) defer span.Finish() value, err := client.QueryRange(ctx, query.Expr, timeRange) From c5a09a3dc1b4c0ffa4483229357892b445d26cda Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 19:43:15 +0200 Subject: [PATCH 42/77] Remove redundant break statements (gosimple) This fixes: pkg/components/dynmap/dynmap.go:588:3: redundant break statement (S1023) pkg/components/dynmap/dynmap.go:610:3: redundant break statement (S1023) pkg/components/dynmap/dynmap.go:641:3: redundant break statement (S1023) pkg/components/dynmap/dynmap.go:690:3: redundant break statement (S1023) pkg/components/dynmap/dynmap.go:712:3: redundant break statement (S1023) pkg/components/dynmap/dynmap.go:749:3: redundant break statement (S1023) pkg/components/dynmap/dynmap.go:785:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/cloudwatch.go:74:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/cloudwatch.go:77:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/cloudwatch.go:82:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/metric_find_query.go:178:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/metric_find_query.go:181:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/metric_find_query.go:184:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/metric_find_query.go:187:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/metric_find_query.go:190:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/metric_find_query.go:193:3: redundant break statement (S1023) pkg/tsdb/cloudwatch/metric_find_query.go:196:3: redundant break statement (S1023) --- pkg/components/dynmap/dynmap.go | 7 ------- pkg/tsdb/cloudwatch/cloudwatch.go | 3 --- pkg/tsdb/cloudwatch/metric_find_query.go | 7 ------- 3 files changed, 17 deletions(-) diff --git a/pkg/components/dynmap/dynmap.go b/pkg/components/dynmap/dynmap.go index 797694845cd..2b86ce384eb 100644 --- a/pkg/components/dynmap/dynmap.go +++ b/pkg/components/dynmap/dynmap.go @@ -585,7 +585,6 @@ func (v *Value) Null() error { switch v.data.(type) { case nil: valid = v.exists // Valid only if j also exists, since other values could possibly also be nil - break } if valid { @@ -607,7 +606,6 @@ func (v *Value) Array() ([]*Value, error) { switch v.data.(type) { case []interface{}: valid = true - break } // Unsure if this is a good way to use slices, it's probably not @@ -638,7 +636,6 @@ func (v *Value) Number() (json.Number, error) { switch v.data.(type) { case json.Number: valid = true - break } if valid { @@ -687,7 +684,6 @@ func (v *Value) Boolean() (bool, error) { switch v.data.(type) { case bool: valid = true - break } if valid { @@ -709,7 +705,6 @@ func (v *Value) Object() (*Object, error) { switch v.data.(type) { case map[string]interface{}: valid = true - break } if valid { @@ -746,7 +741,6 @@ func (v *Value) ObjectArray() ([]*Object, error) { switch v.data.(type) { case []interface{}: valid = true - break } // Unsure if this is a good way to use slices, it's probably not @@ -782,7 +776,6 @@ func (v *Value) String() (string, error) { switch v.data.(type) { case string: valid = true - break } if valid { diff --git a/pkg/tsdb/cloudwatch/cloudwatch.go b/pkg/tsdb/cloudwatch/cloudwatch.go index 3879dce4ea6..d98805c661d 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch.go +++ b/pkg/tsdb/cloudwatch/cloudwatch.go @@ -71,15 +71,12 @@ func (e *CloudWatchExecutor) Query(ctx context.Context, dsInfo *models.DataSourc switch queryType { case "metricFindQuery": result, err = e.executeMetricFindQuery(ctx, queryContext) - break case "annotationQuery": result, err = e.executeAnnotationQuery(ctx, queryContext) - break case "timeSeriesQuery": fallthrough default: result, err = e.executeTimeSeriesQuery(ctx, queryContext) - break } return result, err diff --git a/pkg/tsdb/cloudwatch/metric_find_query.go b/pkg/tsdb/cloudwatch/metric_find_query.go index c82cff390c3..23b69f243fa 100644 --- a/pkg/tsdb/cloudwatch/metric_find_query.go +++ b/pkg/tsdb/cloudwatch/metric_find_query.go @@ -175,25 +175,18 @@ func (e *CloudWatchExecutor) executeMetricFindQuery(ctx context.Context, queryCo switch subType { case "regions": data, err = e.handleGetRegions(ctx, parameters, queryContext) - break case "namespaces": data, err = e.handleGetNamespaces(ctx, parameters, queryContext) - break case "metrics": data, err = e.handleGetMetrics(ctx, parameters, queryContext) - break case "dimension_keys": data, err = e.handleGetDimensions(ctx, parameters, queryContext) - break case "dimension_values": data, err = e.handleGetDimensionValues(ctx, parameters, queryContext) - break case "ebs_volume_ids": data, err = e.handleGetEbsVolumeIds(ctx, parameters, queryContext) - break case "ec2_instance_attribute": data, err = e.handleGetEc2InstanceAttribute(ctx, parameters, queryContext) - break } transformToTable(data, queryResult) From be2d635078b12bd3aa6cf54255d2dad9faa0be5d Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 19:54:23 +0200 Subject: [PATCH 43/77] Simplify error returns (gosimple) This fixes: pkg/api/avatar/avatar.go:261:2: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/log/file.go:102:2: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/metrics/graphitebridge/graphite.go:298:2: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/provisioning/provisioning.go:23:2: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/alert_notification.go:27:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/alert_notification.go:27:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/annotation.go:105:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/annotation.go:105:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/dashboard.go:351:2: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/dashboard.go:435:2: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/dashboard_acl.go:38:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/dashboard_acl.go:38:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/stats.go:22:2: 'if err != nil { return err }; return err' can be simplified to 'return err' (S1013) pkg/services/sqlstore/team.go:213:2: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/user.go:256:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/user.go:256:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/user.go:274:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/user.go:274:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/user.go:482:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) pkg/services/sqlstore/user.go:482:3: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) --- pkg/api/avatar/avatar.go | 7 ++----- pkg/log/file.go | 5 +---- pkg/metrics/graphitebridge/graphite.go | 6 +----- pkg/services/provisioning/provisioning.go | 6 +----- pkg/services/sqlstore/alert_notification.go | 7 +------ pkg/services/sqlstore/annotation.go | 7 ++----- pkg/services/sqlstore/dashboard.go | 14 ++------------ pkg/services/sqlstore/dashboard_acl.go | 6 ++---- pkg/services/sqlstore/stats.go | 4 ---- pkg/services/sqlstore/team.go | 6 +----- pkg/services/sqlstore/user.go | 21 ++++++--------------- pkg/tsdb/cloudwatch/metric_find_query.go | 6 +----- 12 files changed, 20 insertions(+), 75 deletions(-) diff --git a/pkg/api/avatar/avatar.go b/pkg/api/avatar/avatar.go index ce9da1e8790..9f282794076 100644 --- a/pkg/api/avatar/avatar.go +++ b/pkg/api/avatar/avatar.go @@ -258,9 +258,6 @@ func (this *thunderTask) fetch() error { this.Avatar.data = &bytes.Buffer{} writer := bufio.NewWriter(this.Avatar.data) - if _, err = io.Copy(writer, resp.Body); err != nil { - return err - } - - return nil + _, err = io.Copy(writer, resp.Body) + return err } diff --git a/pkg/log/file.go b/pkg/log/file.go index 721db1e55b3..d137adbf3de 100644 --- a/pkg/log/file.go +++ b/pkg/log/file.go @@ -99,10 +99,7 @@ func (w *FileLogWriter) StartLogger() error { return err } w.mw.SetFd(fd) - if err = w.initFd(); err != nil { - return err - } - return nil + return w.initFd() } func (w *FileLogWriter) docheck(size int) { diff --git a/pkg/metrics/graphitebridge/graphite.go b/pkg/metrics/graphitebridge/graphite.go index 68fb544fc7c..670636cedce 100644 --- a/pkg/metrics/graphitebridge/graphite.go +++ b/pkg/metrics/graphitebridge/graphite.go @@ -295,11 +295,7 @@ func writeMetric(buf *bufio.Writer, m model.Metric, mf *dto.MetricFamily) error } } - if err = addExtentionConventionForRollups(buf, mf, m); err != nil { - return err - } - - return nil + return addExtentionConventionForRollups(buf, mf, m) } func addExtentionConventionForRollups(buf *bufio.Writer, mf *dto.MetricFamily, m model.Metric) error { diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index b41ec37b797..71385994bb5 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -20,11 +20,7 @@ func Init(ctx context.Context, homePath string, cfg *ini.File) error { dashboardPath := path.Join(provisioningPath, "dashboards") _, err := dashboards.Provision(ctx, dashboardPath) - if err != nil { - return err - } - - return nil + return err } func makeAbsolute(path string, root string) string { diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index ae691c7166c..651241f7714 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -23,12 +23,7 @@ func DeleteAlertNotification(cmd *m.DeleteAlertNotificationCommand) error { return inTransaction(func(sess *DBSession) error { sql := "DELETE FROM alert_notification WHERE alert_notification.org_id = ? AND alert_notification.id = ?" _, err := sess.Exec(sql, cmd.OrgId, cmd.Id) - - if err != nil { - return err - } - - return nil + return err }) } diff --git a/pkg/services/sqlstore/annotation.go b/pkg/services/sqlstore/annotation.go index 18794281e33..076c0e250b6 100644 --- a/pkg/services/sqlstore/annotation.go +++ b/pkg/services/sqlstore/annotation.go @@ -102,11 +102,8 @@ func (r *SqlAnnotationRepo) Update(item *annotations.Item) error { existing.Tags = item.Tags - if _, err := sess.Table("annotation").Id(existing.Id).Cols("epoch", "text", "region_id", "tags").Update(existing); err != nil { - return err - } - - return nil + _, err = sess.Table("annotation").Id(existing.Id).Cols("epoch", "text", "region_id", "tags").Update(existing) + return err }) } diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 8a89c3d942c..8d90d348a94 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -347,12 +347,7 @@ func GetDashboards(query *m.GetDashboardsQuery) error { err := x.In("id", query.DashboardIds).Find(&dashboards) query.Result = dashboards - - if err != nil { - return err - } - - return nil + return err } // GetDashboardPermissionsForUser returns the maximum permission the specified user has for a dashboard(s) @@ -431,12 +426,7 @@ func GetDashboardsByPluginId(query *m.GetDashboardsByPluginIdQuery) error { err := x.Where(whereExpr, query.OrgId, query.PluginId).Find(&dashboards) query.Result = dashboards - - if err != nil { - return err - } - - return nil + return err } type DashboardSlugDTO struct { diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index ae91d1d41f3..814dc03f9f7 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -35,10 +35,8 @@ func UpdateDashboardAcl(cmd *m.UpdateDashboardAclCommand) error { // Update dashboard HasAcl flag dashboard := m.Dashboard{HasAcl: true} - if _, err := sess.Cols("has_acl").Where("id=?", cmd.DashboardId).Update(&dashboard); err != nil { - return err - } - return nil + _, err = sess.Cols("has_acl").Where("id=?", cmd.DashboardId).Update(&dashboard) + return err }) } diff --git a/pkg/services/sqlstore/stats.go b/pkg/services/sqlstore/stats.go index cfe2d88c82c..6929c1fb9b2 100644 --- a/pkg/services/sqlstore/stats.go +++ b/pkg/services/sqlstore/stats.go @@ -19,10 +19,6 @@ func GetDataSourceStats(query *m.GetDataSourceStatsQuery) error { var rawSql = `SELECT COUNT(*) as count, type FROM data_source GROUP BY type` query.Result = make([]*m.DataSourceStats, 0) err := x.SQL(rawSql).Find(&query.Result) - if err != nil { - return err - } - return err } diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index d238301c7ce..b3ff4c81e7c 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -210,11 +210,7 @@ func GetTeamsByUser(query *m.GetTeamsByUserQuery) error { sess.Where("team.org_id=? and team_member.user_id=?", query.OrgId, query.UserId) err := sess.Find(&query.Result) - if err != nil { - return err - } - - return nil + return err } // AddTeamMember adds a user to a team diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index db7e851435c..bf64bc65602 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -253,11 +253,8 @@ func ChangeUserPassword(cmd *m.ChangeUserPasswordCommand) error { Updated: time.Now(), } - if _, err := sess.Id(cmd.UserId).Update(&user); err != nil { - return err - } - - return nil + _, err := sess.Id(cmd.UserId).Update(&user) + return err }) } @@ -271,11 +268,8 @@ func UpdateUserLastSeenAt(cmd *m.UpdateUserLastSeenAtCommand) error { LastSeenAt: time.Now(), } - if _, err := sess.Id(cmd.UserId).Update(&user); err != nil { - return err - } - - return nil + _, err := sess.Id(cmd.UserId).Update(&user) + return err }) } @@ -479,10 +473,7 @@ func SetUserHelpFlag(cmd *m.SetUserHelpFlagCommand) error { Updated: time.Now(), } - if _, err := sess.Id(cmd.UserId).Cols("help_flags1").Update(&user); err != nil { - return err - } - - return nil + _, err := sess.Id(cmd.UserId).Cols("help_flags1").Update(&user) + return err }) } diff --git a/pkg/tsdb/cloudwatch/metric_find_query.go b/pkg/tsdb/cloudwatch/metric_find_query.go index 23b69f243fa..8864819271e 100644 --- a/pkg/tsdb/cloudwatch/metric_find_query.go +++ b/pkg/tsdb/cloudwatch/metric_find_query.go @@ -566,11 +566,7 @@ func getAllMetrics(cwData *DatasourceInfo) (cloudwatch.ListMetricsOutput, error) } return !lastPage }) - if err != nil { - return resp, err - } - - return resp, nil + return resp, err } var metricsCacheLock sync.Mutex From f61e69ce753cf79404dede251786ec161bd4dc38 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 20:12:59 +0200 Subject: [PATCH 44/77] Simplify comparison to bool constant (gosimple) This fixes: build.go:553:6: should omit comparison to bool constant, can be simplified to !strings.Contains(path, ".sha256") (S1002) pkg/cmd/grafana-cli/commands/ls_command.go:27:5: should omit comparison to bool constant, can be simplified to !pluginDirInfo.IsDir() (S1002) pkg/components/dynmap/dynmap_test.go:24:5: should omit comparison to bool constant, can be simplified to !value (S1002) pkg/components/dynmap/dynmap_test.go:122:14: should omit comparison to bool constant, can be simplified to b (S1002) pkg/components/dynmap/dynmap_test.go:125:14: should omit comparison to bool constant, can be simplified to !b (S1002) pkg/components/dynmap/dynmap_test.go:128:14: should omit comparison to bool constant, can be simplified to !b (S1002) pkg/models/org_user.go:51:5: should omit comparison to bool constant, can be simplified to !(*r).IsValid() (S1002) pkg/plugins/datasource/wrapper/datasource_plugin_wrapper_test.go:77:12: should omit comparison to bool constant, can be simplified to !haveBool (S1002) pkg/services/alerting/conditions/evaluator.go:23:9: should omit comparison to bool constant, can be simplified to !reducedValue.Valid (S1002) pkg/services/alerting/conditions/evaluator.go:48:5: should omit comparison to bool constant, can be simplified to !reducedValue.Valid (S1002) pkg/services/alerting/conditions/evaluator.go:91:5: should omit comparison to bool constant, can be simplified to !reducedValue.Valid (S1002) pkg/services/alerting/conditions/query.go:56:6: should omit comparison to bool constant, can be simplified to !reducedValue.Valid (S1002) pkg/services/alerting/extractor.go:107:20: should omit comparison to bool constant, can be simplified to !enabled.MustBool() (S1002) pkg/services/alerting/notifiers/telegram.go:222:41: should omit comparison to bool constant, can be simplified to this.UploadImage (S1002) pkg/services/sqlstore/apikey.go:58:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/apikey.go:72:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/dashboard.go:66:33: should omit comparison to bool constant, can be simplified to !cmd.Overwrite (S1002) pkg/services/sqlstore/dashboard.go:175:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/dashboard.go:311:13: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/dashboard.go:444:12: should omit comparison to bool constant, can be simplified to !exists (S1002) pkg/services/sqlstore/dashboard.go:472:12: should omit comparison to bool constant, can be simplified to !exists (S1002) pkg/services/sqlstore/dashboard.go:554:32: should omit comparison to bool constant, can be simplified to !cmd.Overwrite (S1002) pkg/services/sqlstore/dashboard_snapshot.go:83:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/plugin_setting.go:39:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/quota.go:34:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/quota.go:111:6: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/quota.go:136:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/quota.go:213:6: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/temp_user.go:129:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/user.go:157:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/user.go:182:5: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/user.go:191:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/user.go:212:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/services/sqlstore/user.go:307:12: should omit comparison to bool constant, can be simplified to !has (S1002) pkg/social/generic_oauth.go:185:5: should omit comparison to bool constant, can be simplified to !s.extractToken(&data, token) (S1002) pkg/tsdb/mssql/mssql.go:148:39: should omit comparison to bool constant, can be simplified to ok (S1002) pkg/tsdb/mssql/mssql.go:212:6: should omit comparison to bool constant, can be simplified to !query.Model.Get("fillNull").MustBool(false) (S1002) pkg/tsdb/mssql/mssql.go:247:56: should omit comparison to bool constant, can be simplified to ok (S1002) pkg/tsdb/mssql/mssql.go:274:7: should omit comparison to bool constant, can be simplified to !exist (S1002) pkg/tsdb/mssql/mssql.go:282:8: should omit comparison to bool constant, can be simplified to !exist (S1002) pkg/tsdb/mysql/mysql.go:221:6: should omit comparison to bool constant, can be simplified to !query.Model.Get("fillNull").MustBool(false) (S1002) pkg/tsdb/mysql/mysql.go:256:56: should omit comparison to bool constant, can be simplified to ok (S1002) pkg/tsdb/mysql/mysql.go:283:7: should omit comparison to bool constant, can be simplified to !exist (S1002) pkg/tsdb/mysql/mysql.go:291:8: should omit comparison to bool constant, can be simplified to !exist (S1002) pkg/tsdb/postgres/postgres.go:134:39: should omit comparison to bool constant, can be simplified to ok (S1002) pkg/tsdb/postgres/postgres.go:201:6: should omit comparison to bool constant, can be simplified to !query.Model.Get("fillNull").MustBool(false) (S1002) pkg/tsdb/postgres/postgres.go:236:56: should omit comparison to bool constant, can be simplified to ok (S1002) pkg/tsdb/postgres/postgres.go:263:7: should omit comparison to bool constant, can be simplified to !exist (S1002) pkg/tsdb/postgres/postgres.go:271:8: should omit comparison to bool constant, can be simplified to !exist (S1002) --- build.go | 2 +- pkg/cmd/grafana-cli/commands/ls_command.go | 2 +- pkg/components/dynmap/dynmap_test.go | 8 ++++---- pkg/models/org_user.go | 2 +- .../wrapper/datasource_plugin_wrapper_test.go | 2 +- pkg/services/alerting/conditions/evaluator.go | 6 +++--- pkg/services/alerting/conditions/query.go | 2 +- pkg/services/alerting/extractor.go | 2 +- pkg/services/alerting/notifiers/telegram.go | 2 +- pkg/services/sqlstore/apikey.go | 4 ++-- pkg/services/sqlstore/dashboard.go | 12 ++++++------ pkg/services/sqlstore/dashboard_snapshot.go | 2 +- pkg/services/sqlstore/plugin_setting.go | 2 +- pkg/services/sqlstore/quota.go | 8 ++++---- pkg/services/sqlstore/temp_user.go | 2 +- pkg/services/sqlstore/user.go | 10 +++++----- pkg/social/generic_oauth.go | 2 +- pkg/tsdb/mssql/mssql.go | 10 +++++----- pkg/tsdb/mysql/mysql.go | 8 ++++---- pkg/tsdb/postgres/postgres.go | 10 +++++----- 20 files changed, 49 insertions(+), 49 deletions(-) diff --git a/build.go b/build.go index c38c452f61f..2497017a873 100644 --- a/build.go +++ b/build.go @@ -550,7 +550,7 @@ func shaFilesInDist() { return nil } - if strings.Contains(path, ".sha256") == false { + if !strings.Contains(path, ".sha256") { err := shaFile(path) if err != nil { log.Printf("Failed to create sha file. error: %v\n", err) diff --git a/pkg/cmd/grafana-cli/commands/ls_command.go b/pkg/cmd/grafana-cli/commands/ls_command.go index 7dcecb9d725..30745ce3172 100644 --- a/pkg/cmd/grafana-cli/commands/ls_command.go +++ b/pkg/cmd/grafana-cli/commands/ls_command.go @@ -24,7 +24,7 @@ var validateLsCommand = func(pluginDir string) error { return fmt.Errorf("error: %s", err) } - if pluginDirInfo.IsDir() == false { + if !pluginDirInfo.IsDir() { return errors.New("plugin path is not a directory") } diff --git a/pkg/components/dynmap/dynmap_test.go b/pkg/components/dynmap/dynmap_test.go index 1dacee163f1..fa5f73c3719 100644 --- a/pkg/components/dynmap/dynmap_test.go +++ b/pkg/components/dynmap/dynmap_test.go @@ -21,7 +21,7 @@ func NewAssert(t *testing.T) *Assert { } func (assert *Assert) True(value bool, message string) { - if value == false { + if !value { log.Panicln("Assert: ", message) } } @@ -119,13 +119,13 @@ func TestFirst(t *testing.T) { assert.True(s == "" && err != nil, "nonexistent string fail") b, err := j.GetBoolean("true") - assert.True(b == true && err == nil, "bool true test") + assert.True(b && err == nil, "bool true test") b, err = j.GetBoolean("false") - assert.True(b == false && err == nil, "bool false test") + assert.True(!b && err == nil, "bool false test") b, err = j.GetBoolean("invalid_field") - assert.True(b == false && err != nil, "bool invalid test") + assert.True(!b && err != nil, "bool invalid test") list, err := j.GetValueArray("list") assert.True(list != nil && err == nil, "list should be an array") diff --git a/pkg/models/org_user.go b/pkg/models/org_user.go index ca32cc50060..98cc4cb3997 100644 --- a/pkg/models/org_user.go +++ b/pkg/models/org_user.go @@ -48,7 +48,7 @@ func (r *RoleType) UnmarshalJSON(data []byte) error { *r = RoleType(str) - if (*r).IsValid() == false { + if !(*r).IsValid() { if (*r) != "" { return errors.New(fmt.Sprintf("JSON validation error: invalid role value: %s", *r)) } diff --git a/pkg/plugins/datasource/wrapper/datasource_plugin_wrapper_test.go b/pkg/plugins/datasource/wrapper/datasource_plugin_wrapper_test.go index 834e8238e3a..7ada6fb6b03 100644 --- a/pkg/plugins/datasource/wrapper/datasource_plugin_wrapper_test.go +++ b/pkg/plugins/datasource/wrapper/datasource_plugin_wrapper_test.go @@ -74,7 +74,7 @@ func TestMappingRowValue(t *testing.T) { boolRowValue, _ := dpw.mapRowValue(&datasource.RowValue{Kind: datasource.RowValue_TYPE_BOOL, BoolValue: true}) haveBool, ok := boolRowValue.(bool) - if !ok || haveBool != true { + if !ok || !haveBool { t.Fatalf("Expected true, was %v", haveBool) } diff --git a/pkg/services/alerting/conditions/evaluator.go b/pkg/services/alerting/conditions/evaluator.go index 1b8fb952f65..dfc058940cf 100644 --- a/pkg/services/alerting/conditions/evaluator.go +++ b/pkg/services/alerting/conditions/evaluator.go @@ -20,7 +20,7 @@ type AlertEvaluator interface { type NoValueEvaluator struct{} func (e *NoValueEvaluator) Eval(reducedValue null.Float) bool { - return reducedValue.Valid == false + return !reducedValue.Valid } type ThresholdEvaluator struct { @@ -45,7 +45,7 @@ func newThresholdEvaluator(typ string, model *simplejson.Json) (*ThresholdEvalua } func (e *ThresholdEvaluator) Eval(reducedValue null.Float) bool { - if reducedValue.Valid == false { + if !reducedValue.Valid { return false } @@ -88,7 +88,7 @@ func newRangedEvaluator(typ string, model *simplejson.Json) (*RangedEvaluator, e } func (e *RangedEvaluator) Eval(reducedValue null.Float) bool { - if reducedValue.Valid == false { + if !reducedValue.Valid { return false } diff --git a/pkg/services/alerting/conditions/query.go b/pkg/services/alerting/conditions/query.go index d499c5e8532..7d1a276c42e 100644 --- a/pkg/services/alerting/conditions/query.go +++ b/pkg/services/alerting/conditions/query.go @@ -53,7 +53,7 @@ func (c *QueryCondition) Eval(context *alerting.EvalContext) (*alerting.Conditio reducedValue := c.Reducer.Reduce(series) evalMatch := c.Evaluator.Eval(reducedValue) - if reducedValue.Valid == false { + if !reducedValue.Valid { emptySerieCount++ } diff --git a/pkg/services/alerting/extractor.go b/pkg/services/alerting/extractor.go index edd872b8fce..e1c1bfacb2e 100644 --- a/pkg/services/alerting/extractor.go +++ b/pkg/services/alerting/extractor.go @@ -104,7 +104,7 @@ func (e *DashAlertExtractor) getAlertFromPanels(jsonWithPanels *simplejson.Json, // backward compatibility check, can be removed later enabled, hasEnabled := jsonAlert.CheckGet("enabled") - if hasEnabled && enabled.MustBool() == false { + if hasEnabled && !enabled.MustBool() { continue } diff --git a/pkg/services/alerting/notifiers/telegram.go b/pkg/services/alerting/notifiers/telegram.go index 5cbdad60906..0b8efe808d5 100644 --- a/pkg/services/alerting/notifiers/telegram.go +++ b/pkg/services/alerting/notifiers/telegram.go @@ -219,7 +219,7 @@ func appendIfPossible(message string, extra string, sizeLimit int) string { func (this *TelegramNotifier) Notify(evalContext *alerting.EvalContext) error { var cmd *m.SendWebhookSync - if evalContext.ImagePublicUrl == "" && this.UploadImage == true { + if evalContext.ImagePublicUrl == "" && this.UploadImage { cmd = this.buildMessage(evalContext, true) } else { cmd = this.buildMessage(evalContext, false) diff --git a/pkg/services/sqlstore/apikey.go b/pkg/services/sqlstore/apikey.go index 0532f636625..9d41b5c809e 100644 --- a/pkg/services/sqlstore/apikey.go +++ b/pkg/services/sqlstore/apikey.go @@ -55,7 +55,7 @@ func GetApiKeyById(query *m.GetApiKeyByIdQuery) error { if err != nil { return err - } else if has == false { + } else if !has { return m.ErrInvalidApiKey } @@ -69,7 +69,7 @@ func GetApiKeyByName(query *m.GetApiKeyByNameQuery) error { if err != nil { return err - } else if has == false { + } else if !has { return m.ErrInvalidApiKey } diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 8d90d348a94..c0e8ead6945 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -63,7 +63,7 @@ func saveDashboard(sess *DBSession, cmd *m.SaveDashboardCommand) error { } // do not allow plugin dashboard updates without overwrite flag - if existing.PluginId != "" && cmd.Overwrite == false { + if existing.PluginId != "" && !cmd.Overwrite { return m.UpdatePluginDashboardError{PluginId: existing.PluginId} } } @@ -172,7 +172,7 @@ func GetDashboard(query *m.GetDashboardQuery) error { if err != nil { return err - } else if has == false { + } else if !has { return m.ErrDashboardNotFound } @@ -308,7 +308,7 @@ func DeleteDashboard(cmd *m.DeleteDashboardCommand) error { has, err := sess.Get(&dashboard) if err != nil { return err - } else if has == false { + } else if !has { return m.ErrDashboardNotFound } @@ -441,7 +441,7 @@ func GetDashboardSlugById(query *m.GetDashboardSlugByIdQuery) error { if err != nil { return err - } else if exists == false { + } else if !exists { return m.ErrDashboardNotFound } @@ -469,7 +469,7 @@ func GetDashboardUIDById(query *m.GetDashboardRefByIdQuery) error { if err != nil { return err - } else if exists == false { + } else if !exists { return m.ErrDashboardNotFound } @@ -551,7 +551,7 @@ func getExistingDashboardByIdOrUidForUpdate(sess *DBSession, cmd *m.ValidateDash } // do not allow plugin dashboard updates without overwrite flag - if existing.PluginId != "" && cmd.Overwrite == false { + if existing.PluginId != "" && !cmd.Overwrite { return m.UpdatePluginDashboardError{PluginId: existing.PluginId} } diff --git a/pkg/services/sqlstore/dashboard_snapshot.go b/pkg/services/sqlstore/dashboard_snapshot.go index 9e82bbb2c83..2e2ea8a4783 100644 --- a/pkg/services/sqlstore/dashboard_snapshot.go +++ b/pkg/services/sqlstore/dashboard_snapshot.go @@ -80,7 +80,7 @@ func GetDashboardSnapshot(query *m.GetDashboardSnapshotQuery) error { if err != nil { return err - } else if has == false { + } else if !has { return m.ErrDashboardSnapshotNotFound } diff --git a/pkg/services/sqlstore/plugin_setting.go b/pkg/services/sqlstore/plugin_setting.go index 172995872eb..312a3bda523 100644 --- a/pkg/services/sqlstore/plugin_setting.go +++ b/pkg/services/sqlstore/plugin_setting.go @@ -36,7 +36,7 @@ func GetPluginSettingById(query *m.GetPluginSettingByIdQuery) error { has, err := x.Get(&pluginSetting) if err != nil { return err - } else if has == false { + } else if !has { return m.ErrPluginSettingNotFound } query.Result = &pluginSetting diff --git a/pkg/services/sqlstore/quota.go b/pkg/services/sqlstore/quota.go index 3db3fc2657e..539555ddc50 100644 --- a/pkg/services/sqlstore/quota.go +++ b/pkg/services/sqlstore/quota.go @@ -31,7 +31,7 @@ func GetOrgQuotaByTarget(query *m.GetOrgQuotaByTargetQuery) error { has, err := x.Get("a) if err != nil { return err - } else if has == false { + } else if !has { quota.Limit = query.Default } @@ -108,7 +108,7 @@ func UpdateOrgQuota(cmd *m.UpdateOrgQuotaCmd) error { return err } quota.Limit = cmd.Limit - if has == false { + if !has { quota.Created = time.Now() //No quota in the DB for this target, so create a new one. if _, err := sess.Insert("a); err != nil { @@ -133,7 +133,7 @@ func GetUserQuotaByTarget(query *m.GetUserQuotaByTargetQuery) error { has, err := x.Get("a) if err != nil { return err - } else if has == false { + } else if !has { quota.Limit = query.Default } @@ -210,7 +210,7 @@ func UpdateUserQuota(cmd *m.UpdateUserQuotaCmd) error { return err } quota.Limit = cmd.Limit - if has == false { + if !has { quota.Created = time.Now() //No quota in the DB for this target, so create a new one. if _, err := sess.Insert("a); err != nil { diff --git a/pkg/services/sqlstore/temp_user.go b/pkg/services/sqlstore/temp_user.go index 43e1f027057..e93ba2fd641 100644 --- a/pkg/services/sqlstore/temp_user.go +++ b/pkg/services/sqlstore/temp_user.go @@ -126,7 +126,7 @@ func GetTempUserByCode(query *m.GetTempUserByCodeQuery) error { if err != nil { return err - } else if has == false { + } else if !has { return m.ErrTempUserNotFound } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index bf64bc65602..6d8bd0c5279 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -154,7 +154,7 @@ func GetUserById(query *m.GetUserByIdQuery) error { if err != nil { return err - } else if has == false { + } else if !has { return m.ErrUserNotFound } @@ -179,7 +179,7 @@ func GetUserByLogin(query *m.GetUserByLoginQuery) error { return err } - if has == false && strings.Contains(query.LoginOrEmail, "@") { + if !has && strings.Contains(query.LoginOrEmail, "@") { // If the user wasn't found, and it contains an "@" fallback to finding the // user by email. user = &m.User{Email: query.LoginOrEmail} @@ -188,7 +188,7 @@ func GetUserByLogin(query *m.GetUserByLoginQuery) error { if err != nil { return err - } else if has == false { + } else if !has { return m.ErrUserNotFound } @@ -209,7 +209,7 @@ func GetUserByEmail(query *m.GetUserByEmailQuery) error { if err != nil { return err - } else if has == false { + } else if !has { return m.ErrUserNotFound } @@ -304,7 +304,7 @@ func GetUserProfile(query *m.GetUserProfileQuery) error { if err != nil { return err - } else if has == false { + } else if !has { return m.ErrUserNotFound } diff --git a/pkg/social/generic_oauth.go b/pkg/social/generic_oauth.go index b92d64ad9fc..8c02076096d 100644 --- a/pkg/social/generic_oauth.go +++ b/pkg/social/generic_oauth.go @@ -182,7 +182,7 @@ func (s *SocialGenericOAuth) UserInfo(client *http.Client, token *oauth2.Token) var data UserInfoJson var err error - if s.extractToken(&data, token) != true { + if !s.extractToken(&data, token) { response, err := HttpGet(client, s.apiUrl) if err != nil { return nil, fmt.Errorf("Error getting user info: %s", err) diff --git a/pkg/tsdb/mssql/mssql.go b/pkg/tsdb/mssql/mssql.go index 3a440859f9f..a598b7239ed 100644 --- a/pkg/tsdb/mssql/mssql.go +++ b/pkg/tsdb/mssql/mssql.go @@ -145,7 +145,7 @@ func (e MssqlQueryEndpoint) getTypedRowData(types []*sql.ColumnType, rows *core. // convert types not handled by denisenkom/go-mssqldb // unhandled types are returned as []byte for i := 0; i < len(types); i++ { - if value, ok := values[i].([]byte); ok == true { + if value, ok := values[i].([]byte); ok { switch types[i].DatabaseTypeName() { case "MONEY", "SMALLMONEY", "DECIMAL": if v, err := strconv.ParseFloat(string(value), 64); err == nil { @@ -209,7 +209,7 @@ func (e MssqlQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *core. fillValue := null.Float{} if fillMissing { fillInterval = query.Model.Get("fillInterval").MustFloat64() * 1000 - if query.Model.Get("fillNull").MustBool(false) == false { + if !query.Model.Get("fillNull").MustBool(false) { fillValue.Float64 = query.Model.Get("fillValue").MustFloat64() fillValue.Valid = true } @@ -244,7 +244,7 @@ func (e MssqlQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *core. } if metricIndex >= 0 { - if columnValue, ok := values[metricIndex].(string); ok == true { + if columnValue, ok := values[metricIndex].(string); ok { metric = columnValue } else { return fmt.Errorf("Column metric must be of type CHAR, VARCHAR, NCHAR or NVARCHAR. metric column name: %s type: %s but datatype is %T", columnNames[metricIndex], columnTypes[metricIndex].DatabaseTypeName(), values[metricIndex]) @@ -271,7 +271,7 @@ func (e MssqlQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *core. } series, exist := pointsBySeries[metric] - if exist == false { + if !exist { series = &tsdb.TimeSeries{Name: metric} pointsBySeries[metric] = series seriesByQueryOrder.PushBack(metric) @@ -279,7 +279,7 @@ func (e MssqlQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *core. if fillMissing { var intervalStart float64 - if exist == false { + if !exist { intervalStart = float64(tsdbQuery.TimeRange.MustGetFrom().UnixNano() / 1e6) } else { intervalStart = series.Points[len(series.Points)-1][1].Float64 + fillInterval diff --git a/pkg/tsdb/mysql/mysql.go b/pkg/tsdb/mysql/mysql.go index 83027d4b210..4f5cd1b0784 100644 --- a/pkg/tsdb/mysql/mysql.go +++ b/pkg/tsdb/mysql/mysql.go @@ -218,7 +218,7 @@ func (e MysqlQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *core. fillValue := null.Float{} if fillMissing { fillInterval = query.Model.Get("fillInterval").MustFloat64() * 1000 - if query.Model.Get("fillNull").MustBool(false) == false { + if !query.Model.Get("fillNull").MustBool(false) { fillValue.Float64 = query.Model.Get("fillValue").MustFloat64() fillValue.Valid = true } @@ -253,7 +253,7 @@ func (e MysqlQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *core. } if metricIndex >= 0 { - if columnValue, ok := values[metricIndex].(string); ok == true { + if columnValue, ok := values[metricIndex].(string); ok { metric = columnValue } else { return fmt.Errorf("Column metric must be of type char,varchar or text, got: %T %v", values[metricIndex], values[metricIndex]) @@ -280,7 +280,7 @@ func (e MysqlQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *core. } series, exist := pointsBySeries[metric] - if exist == false { + if !exist { series = &tsdb.TimeSeries{Name: metric} pointsBySeries[metric] = series seriesByQueryOrder.PushBack(metric) @@ -288,7 +288,7 @@ func (e MysqlQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *core. if fillMissing { var intervalStart float64 - if exist == false { + if !exist { intervalStart = float64(tsdbQuery.TimeRange.MustGetFrom().UnixNano() / 1e6) } else { intervalStart = series.Points[len(series.Points)-1][1].Float64 + fillInterval diff --git a/pkg/tsdb/postgres/postgres.go b/pkg/tsdb/postgres/postgres.go index e17cc783f38..72d50b32d04 100644 --- a/pkg/tsdb/postgres/postgres.go +++ b/pkg/tsdb/postgres/postgres.go @@ -131,7 +131,7 @@ func (e PostgresQueryEndpoint) getTypedRowData(rows *core.Rows) (tsdb.RowValues, // convert types not handled by lib/pq // unhandled types are returned as []byte for i := 0; i < len(types); i++ { - if value, ok := values[i].([]byte); ok == true { + if value, ok := values[i].([]byte); ok { switch types[i].DatabaseTypeName() { case "NUMERIC": if v, err := strconv.ParseFloat(string(value), 64); err == nil { @@ -198,7 +198,7 @@ func (e PostgresQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *co fillValue := null.Float{} if fillMissing { fillInterval = query.Model.Get("fillInterval").MustFloat64() * 1000 - if query.Model.Get("fillNull").MustBool(false) == false { + if !query.Model.Get("fillNull").MustBool(false) { fillValue.Float64 = query.Model.Get("fillValue").MustFloat64() fillValue.Valid = true } @@ -233,7 +233,7 @@ func (e PostgresQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *co } if metricIndex >= 0 { - if columnValue, ok := values[metricIndex].(string); ok == true { + if columnValue, ok := values[metricIndex].(string); ok { metric = columnValue } else { return fmt.Errorf("Column metric must be of type char,varchar or text, got: %T %v", values[metricIndex], values[metricIndex]) @@ -260,7 +260,7 @@ func (e PostgresQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *co } series, exist := pointsBySeries[metric] - if exist == false { + if !exist { series = &tsdb.TimeSeries{Name: metric} pointsBySeries[metric] = series seriesByQueryOrder.PushBack(metric) @@ -268,7 +268,7 @@ func (e PostgresQueryEndpoint) transformToTimeSeries(query *tsdb.Query, rows *co if fillMissing { var intervalStart float64 - if exist == false { + if !exist { intervalStart = float64(tsdbQuery.TimeRange.MustGetFrom().UnixNano() / 1e6) } else { intervalStart = series.Points[len(series.Points)-1][1].Float64 + fillInterval From da5654ad04e45316b36ec7ce360c8e34a965b4a0 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 20:17:04 +0200 Subject: [PATCH 45/77] Simplify if expression (gosimple) pkg/util/shortid_generator.go:20:2: should use 'return ' instead of 'if { return }; return ' (S1008) --- pkg/util/shortid_generator.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/util/shortid_generator.go b/pkg/util/shortid_generator.go index d87b6f70fe6..f900cb8275e 100644 --- a/pkg/util/shortid_generator.go +++ b/pkg/util/shortid_generator.go @@ -17,11 +17,7 @@ func init() { // IsValidShortUid checks if short unique identifier contains valid characters func IsValidShortUid(uid string) bool { - if !validUidPattern(uid) { - return false - } - - return true + return validUidPattern(uid) } // GenerateShortUid generates a short unique identifier. From cb3b9f8b66f38ff1aa0763d031b9b1d9108942f7 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 20:19:19 +0200 Subject: [PATCH 46/77] Use raw strings to avoid double escapes (gosimple) This fixes: pkg/services/alerting/rule.go:58:21: should use raw string (`...`) with regexp.MustCompile to avoid having to escape twice (S1007\) pkg/services/alerting/rule.go:59:21: should use raw string (`...`) with regexp.MustCompile to avoid having to escape twice (S1007) --- pkg/services/alerting/rule.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/services/alerting/rule.go b/pkg/services/alerting/rule.go index bdf53798e34..027ff96d6c0 100644 --- a/pkg/services/alerting/rule.go +++ b/pkg/services/alerting/rule.go @@ -55,8 +55,8 @@ func (e ValidationError) Error() string { } var ( - ValueFormatRegex = regexp.MustCompile("^\\d+") - UnitFormatRegex = regexp.MustCompile("\\w{1}$") + ValueFormatRegex = regexp.MustCompile(`^\d+`) + UnitFormatRegex = regexp.MustCompile(`\w{1}$`) ) var unitMultiplier = map[string]int{ From 4d87cb03c543a7e808209de8cc3e8ade43ecfa64 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 20:22:12 +0200 Subject: [PATCH 47/77] Simplify make() (gosimple) This fixes: pkg/api/http_server.go:142:89: should use make(map[string]func(*http.Server, *tls.Conn, http.Handler)) instead (S1019) pkg/services/alerting/scheduler.go:18:30: should use make(map[int64]*Job) instead (S1019) pkg/services/alerting/scheduler.go:26:31: should use make(map[int64]*Job) instead (S1019) --- pkg/api/http_server.go | 2 +- pkg/services/alerting/scheduler.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 387a543ca89..8e01e869329 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -139,7 +139,7 @@ func (hs *HTTPServer) listenAndServeTLS(certfile, keyfile string) error { } hs.httpSrv.TLSConfig = tlsCfg - hs.httpSrv.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler), 0) + hs.httpSrv.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler)) return hs.httpSrv.ListenAndServeTLS(setting.CertFile, setting.KeyFile) } diff --git a/pkg/services/alerting/scheduler.go b/pkg/services/alerting/scheduler.go index b0a3f8303c4..b7555ae8d89 100644 --- a/pkg/services/alerting/scheduler.go +++ b/pkg/services/alerting/scheduler.go @@ -15,7 +15,7 @@ type SchedulerImpl struct { func NewScheduler() Scheduler { return &SchedulerImpl{ - jobs: make(map[int64]*Job, 0), + jobs: make(map[int64]*Job), log: log.New("alerting.scheduler"), } } @@ -23,7 +23,7 @@ func NewScheduler() Scheduler { func (s *SchedulerImpl) Update(rules []*Rule) { s.log.Debug("Scheduling update", "ruleCount", len(rules)) - jobs := make(map[int64]*Job, 0) + jobs := make(map[int64]*Job) for i, rule := range rules { var job *Job From 4d6386e97bb30138c270828b9f290ab5b2ac53a4 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 20:25:48 +0200 Subject: [PATCH 48/77] Use fmt.Errorf() (gosimple) This fixes: pkg/cmd/grafana-cli/commands/install_command.go:36:11: should use fmt.Errorf(...) instead of errors.New(fmt.Sprintf(...)) (S1028) pkg/models/org_user.go:53:11: should use fmt.Errorf(...) instead of errors.New(fmt.Sprintf(...)) (S1028) pkg/services/notifications/mailer.go:138:16: should use fmt.Errorf(...) instead of errors.New(fmt.Sprintf(...)) (S1028) --- pkg/cmd/grafana-cli/commands/install_command.go | 2 +- pkg/models/org_user.go | 2 +- pkg/services/notifications/mailer.go | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/grafana-cli/commands/install_command.go b/pkg/cmd/grafana-cli/commands/install_command.go index 6f6849ccddf..9bdb73a5858 100644 --- a/pkg/cmd/grafana-cli/commands/install_command.go +++ b/pkg/cmd/grafana-cli/commands/install_command.go @@ -33,7 +33,7 @@ func validateInput(c CommandLine, pluginFolder string) error { fileInfo, err := os.Stat(pluginsDir) if err != nil { if err = os.MkdirAll(pluginsDir, os.ModePerm); err != nil { - return errors.New(fmt.Sprintf("pluginsDir (%s) is not a writable directory", pluginsDir)) + return fmt.Errorf("pluginsDir (%s) is not a writable directory", pluginsDir) } return nil } diff --git a/pkg/models/org_user.go b/pkg/models/org_user.go index 98cc4cb3997..9231d18cfd6 100644 --- a/pkg/models/org_user.go +++ b/pkg/models/org_user.go @@ -50,7 +50,7 @@ func (r *RoleType) UnmarshalJSON(data []byte) error { if !(*r).IsValid() { if (*r) != "" { - return errors.New(fmt.Sprintf("JSON validation error: invalid role value: %s", *r)) + return fmt.Errorf("JSON validation error: invalid role value: %s", *r) } *r = ROLE_VIEWER diff --git a/pkg/services/notifications/mailer.go b/pkg/services/notifications/mailer.go index 05c2e53c748..1bac5025244 100644 --- a/pkg/services/notifications/mailer.go +++ b/pkg/services/notifications/mailer.go @@ -7,7 +7,6 @@ package notifications import ( "bytes" "crypto/tls" - "errors" "fmt" "html/template" "net" @@ -135,7 +134,7 @@ func buildEmailMessage(cmd *m.SendEmailCommand) (*Message, error) { subjectText, hasSubject := subjectData["value"] if !hasSubject { - return nil, errors.New(fmt.Sprintf("Missing subject in Template %s", cmd.Template)) + return nil, fmt.Errorf("Missing subject in Template %s", cmd.Template) } subjectTmpl, err := template.New("subject").Parse(subjectText.(string)) From 5d95601720d2a742b94d88ca6c958d6a679810af Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 20:33:59 +0200 Subject: [PATCH 49/77] Merge variable declaration with assignment (gosimple) This fixes: pkg/cmd/grafana-cli/commands/upgrade_all_command.go:56:3: should merge variable declaration with assignment on next line (S1021) pkg/login/ldap.go:406:4: should merge variable declaration with assignment on next line (S1021) pkg/services/sqlstore/migrator/dialect.go:87:2: should merge variable declaration with assignment on next line (S1021) pkg/services/sqlstore/migrator/dialect.go:165:2: should merge variable declaration with assignment on next line (S1021) pkg/tsdb/cloudwatch/metric_find_query_test.go:185:2: should merge variable declaration with assignment on next line (S1021) --- pkg/cmd/grafana-cli/commands/upgrade_all_command.go | 3 +-- pkg/login/ldap.go | 3 +-- pkg/services/sqlstore/migrator/dialect.go | 6 ++---- pkg/tsdb/cloudwatch/metric_find_query_test.go | 5 +---- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/grafana-cli/commands/upgrade_all_command.go b/pkg/cmd/grafana-cli/commands/upgrade_all_command.go index 636292cce11..e01df2dab60 100644 --- a/pkg/cmd/grafana-cli/commands/upgrade_all_command.go +++ b/pkg/cmd/grafana-cli/commands/upgrade_all_command.go @@ -53,8 +53,7 @@ func upgradeAllCommand(c CommandLine) error { for _, p := range pluginsToUpgrade { logger.Infof("Updating %v \n", p.Id) - var err error - err = s.RemoveInstalledPlugin(pluginsDir, p.Id) + err := s.RemoveInstalledPlugin(pluginsDir, p.Id) if err != nil { return err } diff --git a/pkg/login/ldap.go b/pkg/login/ldap.go index be3babac02e..9f2338d653c 100644 --- a/pkg/login/ldap.go +++ b/pkg/login/ldap.go @@ -403,8 +403,7 @@ func (a *ldapAuther) searchForUser(username string) (*LdapUserInfo, error) { // If we are using a POSIX LDAP schema it won't support memberOf, so we manually search the groups var groupSearchResult *ldap.SearchResult for _, groupSearchBase := range a.server.GroupSearchBaseDNs { - var filter_replace string - filter_replace = getLdapAttr(a.server.GroupSearchFilterUserAttribute, searchResult) + filter_replace := getLdapAttr(a.server.GroupSearchFilterUserAttribute, searchResult) if a.server.GroupSearchFilterUserAttribute == "" { filter_replace = getLdapAttr(a.server.Attr.Username, searchResult) } diff --git a/pkg/services/sqlstore/migrator/dialect.go b/pkg/services/sqlstore/migrator/dialect.go index 064b5981063..dadc7248844 100644 --- a/pkg/services/sqlstore/migrator/dialect.go +++ b/pkg/services/sqlstore/migrator/dialect.go @@ -84,8 +84,7 @@ func (db *BaseDialect) DateTimeFunc(value string) string { } func (b *BaseDialect) CreateTableSql(table *Table) string { - var sql string - sql = "CREATE TABLE IF NOT EXISTS " + sql := "CREATE TABLE IF NOT EXISTS " sql += b.dialect.Quote(table.Name) + " (\n" pkList := table.PrimaryKeys @@ -162,8 +161,7 @@ func (db *BaseDialect) RenameTable(oldName string, newName string) string { func (db *BaseDialect) DropIndexSql(tableName string, index *Index) string { quote := db.dialect.Quote - var name string - name = index.XName(tableName) + name := index.XName(tableName) return fmt.Sprintf("DROP INDEX %v ON %s", quote(name), quote(tableName)) } diff --git a/pkg/tsdb/cloudwatch/metric_find_query_test.go b/pkg/tsdb/cloudwatch/metric_find_query_test.go index bf87e7b7d41..e3903e8027e 100644 --- a/pkg/tsdb/cloudwatch/metric_find_query_test.go +++ b/pkg/tsdb/cloudwatch/metric_find_query_test.go @@ -181,10 +181,7 @@ func TestCloudWatchMetrics(t *testing.T) { } func TestParseMultiSelectValue(t *testing.T) { - - var values []string - - values = parseMultiSelectValue(" i-someInstance ") + values := parseMultiSelectValue(" i-someInstance ") assert.Equal(t, []string{"i-someInstance"}, values) values = parseMultiSelectValue("{i-05}") From de084e589175fd718c4c248e64a92a03e3445877 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 20:40:19 +0200 Subject: [PATCH 50/77] Remove unnecessary fmt.Sprintf() calls (gosimple) This fixes: pkg/services/sqlstore/migrator/migrations.go:116:18: the argument is already a string, there's no need to use fmt.Sprintf (S1025) pkg/services/sqlstore/migrator/types.go:49:16: the argument is already a string, there's no need to use fmt.Sprintf (S1025) --- pkg/services/sqlstore/migrator/migrations.go | 3 +-- pkg/services/sqlstore/migrator/types.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/services/sqlstore/migrator/migrations.go b/pkg/services/sqlstore/migrator/migrations.go index 2fec8825fa4..5232e65d62b 100644 --- a/pkg/services/sqlstore/migrator/migrations.go +++ b/pkg/services/sqlstore/migrator/migrations.go @@ -1,7 +1,6 @@ package migrator import ( - "fmt" "strings" ) @@ -113,7 +112,7 @@ func NewDropIndexMigration(table Table, index *Index) *DropIndexMigration { func (m *DropIndexMigration) Sql(dialect Dialect) string { if m.index.Name == "" { - m.index.Name = fmt.Sprintf("%s", strings.Join(m.index.Cols, "_")) + m.index.Name = strings.Join(m.index.Cols, "_") } return dialect.DropIndexSql(m.tableName, m.index) } diff --git a/pkg/services/sqlstore/migrator/types.go b/pkg/services/sqlstore/migrator/types.go index d42eba0f58a..62ec74e7b9f 100644 --- a/pkg/services/sqlstore/migrator/types.go +++ b/pkg/services/sqlstore/migrator/types.go @@ -46,7 +46,7 @@ type Index struct { func (index *Index) XName(tableName string) string { if index.Name == "" { - index.Name = fmt.Sprintf("%s", strings.Join(index.Cols, "_")) + index.Name = strings.Join(index.Cols, "_") } if !strings.HasPrefix(index.Name, "UQE_") && From 25bb0b556961b161d2b6accfde93151aab855c75 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 20:42:17 +0200 Subject: [PATCH 51/77] Remove unused return value assignment (gosimple) This fixes: pkg/tsdb/sql_engine.go:54:6: should write version := engineCache.versions[dsInfo.Id] instead of version, _ := engineCache.versions[dsInfo.Id] (S1005) --- pkg/tsdb/sql_engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tsdb/sql_engine.go b/pkg/tsdb/sql_engine.go index 0f35cadf4d6..56ed2cd3cb6 100644 --- a/pkg/tsdb/sql_engine.go +++ b/pkg/tsdb/sql_engine.go @@ -51,7 +51,7 @@ func (e *DefaultSqlEngine) InitEngine(driverName string, dsInfo *models.DataSour defer engineCache.Unlock() if engine, present := engineCache.cache[dsInfo.Id]; present { - if version, _ := engineCache.versions[dsInfo.Id]; version == dsInfo.Version { + if version := engineCache.versions[dsInfo.Id]; version == dsInfo.Version { e.XormEngine = engine return nil } From ed46efa0811d39d706a90b4a24998305c45f4cd7 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 16 Apr 2018 20:45:59 +0200 Subject: [PATCH 52/77] Use sort.Strings() (gosimple) This fixes: pkg/tsdb/cloudwatch/metric_find_query.go:257:2: should use sort.Strings(...) instead of sort.Sort(sort.StringSlice(...)) (S1032) pkg/tsdb/cloudwatch/metric_find_query.go:286:2: should use sort.Strings(...) instead of sort.Sort(sort.StringSlice(...)) (S1032) pkg/tsdb/cloudwatch/metric_find_query.go:315:2: should use sort.Strings(...) instead of sort.Sort(sort.StringSlice(...)) (S1032) --- pkg/tsdb/cloudwatch/metric_find_query.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/tsdb/cloudwatch/metric_find_query.go b/pkg/tsdb/cloudwatch/metric_find_query.go index 8864819271e..d73516ca88f 100644 --- a/pkg/tsdb/cloudwatch/metric_find_query.go +++ b/pkg/tsdb/cloudwatch/metric_find_query.go @@ -254,7 +254,7 @@ func (e *CloudWatchExecutor) handleGetNamespaces(ctx context.Context, parameters keys = append(keys, strings.Split(customNamespaces, ",")...) } - sort.Sort(sort.StringSlice(keys)) + sort.Strings(keys) result := make([]suggestData, 0) for _, key := range keys { @@ -283,7 +283,7 @@ func (e *CloudWatchExecutor) handleGetMetrics(ctx context.Context, parameters *s return nil, errors.New("Unable to call AWS API") } } - sort.Sort(sort.StringSlice(namespaceMetrics)) + sort.Strings(namespaceMetrics) result := make([]suggestData, 0) for _, name := range namespaceMetrics { @@ -312,7 +312,7 @@ func (e *CloudWatchExecutor) handleGetDimensions(ctx context.Context, parameters return nil, errors.New("Unable to call AWS API") } } - sort.Sort(sort.StringSlice(dimensionValues)) + sort.Strings(dimensionValues) result := make([]suggestData, 0) for _, name := range dimensionValues { From 58e6b11809e6e1b3115e88e7dd4a80d423cfa956 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 16 Apr 2018 21:51:53 +0200 Subject: [PATCH 53/77] add some more sort order asserts for permissions store tests --- .../stores/PermissionsStore/PermissionsStore.jest.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index 24f1705367a..f1713542be5 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -74,6 +74,9 @@ describe('PermissionsStore', () => { }); it('should be sorted by sort rank and alphabetically', async () => { + expect(store.items[0].name).toBe('MyTestTeam'); + expect(store.items[1].name).toBe('Editor'); + expect(store.items[2].name).toBe('Viewer'); expect(store.items[3].name).toBe('MyTestTeam2'); expect(store.items[4].name).toBe('MyTestUser'); }); @@ -96,5 +99,14 @@ describe('PermissionsStore', () => { it('should add new overriding permission', () => { expect(store.items.length).toBe(6); }); + + it('should be sorted by sort rank and alphabetically', async () => { + expect(store.items[0].name).toBe('MyTestTeam'); + expect(store.items[1].name).toBe('Editor'); + expect(store.items[2].name).toBe('Viewer'); + expect(store.items[3].name).toBe('MyTestTeam'); + expect(store.items[4].name).toBe('MyTestTeam2'); + expect(store.items[5].name).toBe('MyTestUser'); + }); }); }); From 5dc36afe71de80ec2311b942ca137c1c3dec5b70 Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Mon, 16 Apr 2018 23:02:24 +0200 Subject: [PATCH 54/77] calculate datetime for timeFrom and timeTo macro in go --- pkg/tsdb/postgres/macros.go | 4 ++-- pkg/tsdb/postgres/macros_test.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/tsdb/postgres/macros.go b/pkg/tsdb/postgres/macros.go index e666d20ae06..05e39f2c762 100644 --- a/pkg/tsdb/postgres/macros.go +++ b/pkg/tsdb/postgres/macros.go @@ -85,9 +85,9 @@ func (m *PostgresMacroEngine) evaluateMacro(name string, args []string) (string, return fmt.Sprintf("%s BETWEEN '%s' AND '%s'", args[0], m.TimeRange.GetFromAsTimeUTC().Format(time.RFC3339), m.TimeRange.GetToAsTimeUTC().Format(time.RFC3339)), nil case "__timeFrom": - return fmt.Sprintf("to_timestamp(%d)", m.TimeRange.GetFromAsSecondsEpoch()), nil + return fmt.Sprintf("'%s'", m.TimeRange.GetFromAsTimeUTC().Format(time.RFC3339)), nil case "__timeTo": - return fmt.Sprintf("to_timestamp(%d)", m.TimeRange.GetToAsSecondsEpoch()), nil + return fmt.Sprintf("'%s'", m.TimeRange.GetToAsTimeUTC().Format(time.RFC3339)), nil case "__timeGroup": if len(args) < 2 { return "", fmt.Errorf("macro %v needs time column and interval and optional fill value", name) diff --git a/pkg/tsdb/postgres/macros_test.go b/pkg/tsdb/postgres/macros_test.go index d1bcaff796d..c3c15691e42 100644 --- a/pkg/tsdb/postgres/macros_test.go +++ b/pkg/tsdb/postgres/macros_test.go @@ -45,7 +45,7 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "select $__timeFrom(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("select to_timestamp(%d)", from.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("select '%s'", from.Format(time.RFC3339))) }) Convey("interpolate __timeGroup function", func() { @@ -68,7 +68,7 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "select $__timeTo(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("select to_timestamp(%d)", to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("select '%s'", to.Format(time.RFC3339))) }) Convey("interpolate __unixEpochFilter function", func() { @@ -109,14 +109,14 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "select $__timeFrom(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("select to_timestamp(%d)", from.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("select '%s'", from.Format(time.RFC3339))) }) Convey("interpolate __timeTo function", func() { sql, err := engine.Interpolate(query, timeRange, "select $__timeTo(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("select to_timestamp(%d)", to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("select '%s'", to.Format(time.RFC3339))) }) Convey("interpolate __unixEpochFilter function", func() { @@ -157,14 +157,14 @@ func TestMacroEngine(t *testing.T) { sql, err := engine.Interpolate(query, timeRange, "select $__timeFrom(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("select to_timestamp(%d)", from.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("select '%s'", from.Format(time.RFC3339))) }) Convey("interpolate __timeTo function", func() { sql, err := engine.Interpolate(query, timeRange, "select $__timeTo(time_column)") So(err, ShouldBeNil) - So(sql, ShouldEqual, fmt.Sprintf("select to_timestamp(%d)", to.Unix())) + So(sql, ShouldEqual, fmt.Sprintf("select '%s'", to.Format(time.RFC3339))) }) Convey("interpolate __unixEpochFilter function", func() { From 69d1330ae9f45074b1e34aac768571ec5b167fee Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 16 Apr 2018 23:46:26 +0200 Subject: [PATCH 55/77] changelog: notes about closing #11578 [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6df0746e436..f433b102342 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ * **Prometheus**: tooltip for legend format not showing properly [#11516](https://github.com/grafana/grafana/issues/11516), thx [@svenklemm](https://github.com/svenklemm) * **Playlist**: Empty playlists cannot be deleted [#11133](https://github.com/grafana/grafana/issues/11133), thx [@kichristensen](https://github.com/kichristensen) * **Switch Orgs**: Alphabetic order in Switch Organization modal [#11556](https://github.com/grafana/grafana/issues/11556) +* **Postgres**: improve `$__timeFilter` macro [#11578](https://github.com/grafana/grafana/issues/11578), thx [@svenklemm](https://github.com/svenklemm) ### Tech * Migrated JavaScript files to TypeScript From 3a5d1f459466d6929033df198b17ede98e333e44 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 16 Apr 2018 23:56:14 +0200 Subject: [PATCH 56/77] tsdb: update query and annotation editor help texts for postgres In addition to closing #11578 --- .../datasource/postgres/partials/annotations.editor.html | 8 ++++---- .../datasource/postgres/partials/query.editor.html | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/public/app/plugins/datasource/postgres/partials/annotations.editor.html b/public/app/plugins/datasource/postgres/partials/annotations.editor.html index 907b1b10be4..b83f5a14832 100644 --- a/public/app/plugins/datasource/postgres/partials/annotations.editor.html +++ b/public/app/plugins/datasource/postgres/partials/annotations.editor.html @@ -28,12 +28,12 @@ An annotation is an event that is overlaid on top of graphs. The query can have Macros: - $__time(column) -> column as "time" - $__timeEpoch -> extract(epoch from column) as "time" -- $__timeFilter(column) -> column ≥ to_timestamp(1492750877) AND column ≤ to_timestamp(1492750877) -- $__unixEpochFilter(column) -> column > 1492750877 AND column < 1492750877 +- $__timeFilter(column) -> column BETWEEN '2017-04-21T05:01:17Z' AND '2017-04-21T05:01:17Z' +- $__unixEpochFilter(column) -> column >= 1492750877 AND column <= 1492750877 Or build your own conditionals using these macros which just return the values: -- $__timeFrom() -> to_timestamp(1492750877) -- $__timeTo() -> to_timestamp(1492750877) +- $__timeFrom() -> '2017-04-21T05:01:17Z' +- $__timeTo() -> '2017-04-21T05:01:17Z' - $__unixEpochFrom() -> 1492750877 - $__unixEpochTo() -> 1492750877 diff --git a/public/app/plugins/datasource/postgres/partials/query.editor.html b/public/app/plugins/datasource/postgres/partials/query.editor.html index 163970a9ad5..26392c17356 100644 --- a/public/app/plugins/datasource/postgres/partials/query.editor.html +++ b/public/app/plugins/datasource/postgres/partials/query.editor.html @@ -48,8 +48,8 @@ Table: Macros: - $__time(column) -> column as "time" - $__timeEpoch -> extract(epoch from column) as "time" -- $__timeFilter(column) -> extract(epoch from column) BETWEEN 1492750877 AND 1492750877 -- $__unixEpochFilter(column) -> column > 1492750877 AND column < 1492750877 +- $__timeFilter(column) -> column BETWEEN '2017-04-21T05:01:17Z' AND '2017-04-21T05:01:17Z' +- $__unixEpochFilter(column) -> column >= 1492750877 AND column <= 1492750877 - $__timeGroup(column,'5m') -> (extract(epoch from column)/300)::bigint*300 AS time Example of group by and order by with $__timeGroup: @@ -61,8 +61,8 @@ GROUP BY time ORDER BY time Or build your own conditionals using these macros which just return the values: -- $__timeFrom() -> to_timestamp(1492750877) -- $__timeTo() -> to_timestamp(1492750877) +- $__timeFrom() -> '2017-04-21T05:01:17Z' +- $__timeTo() -> '2017-04-21T05:01:17Z' - $__unixEpochFrom() -> 1492750877 - $__unixEpochTo() -> 1492750877 From b6b152d9ea41384e87af5d98f211a3bbba598b0b Mon Sep 17 00:00:00 2001 From: Carl Bergquist Date: Tue, 17 Apr 2018 09:00:39 +0200 Subject: [PATCH 57/77] Revert "removes codecov from frontend tests" --- Gruntfile.js | 1 + codecov.yml | 13 +++++++++++++ package.json | 1 + scripts/circle-test-frontend.sh | 9 +++++++-- scripts/grunt/options/exec.js | 7 ++++++- 5 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 codecov.yml diff --git a/Gruntfile.js b/Gruntfile.js index 03f70565b57..a0607ef49dc 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -22,6 +22,7 @@ module.exports = function (grunt) { } } + config.coverage = grunt.option('coverage'); config.phjs = grunt.option('phjsToRelease'); config.pkg.version = grunt.option('pkgVer') || config.pkg.version; diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 00000000000..82a86e0232b --- /dev/null +++ b/codecov.yml @@ -0,0 +1,13 @@ +coverage: + precision: 2 + round: down + range: "50...100" + + status: + project: yes + patch: yes + changes: no + +comment: + layout: "diff" + behavior: "once" diff --git a/package.json b/package.json index b74d23f33b2..ce861a25f7b 100644 --- a/package.json +++ b/package.json @@ -102,6 +102,7 @@ "watch": "webpack --progress --colors --watch --config scripts/webpack/webpack.dev.js", "build": "grunt build", "test": "grunt test", + "test:coverage": "grunt test --coverage=true", "lint": "tslint -c tslint.json --project tsconfig.json --type-check", "karma": "grunt karma:dev", "jest": "jest --notify --watch", diff --git a/scripts/circle-test-frontend.sh b/scripts/circle-test-frontend.sh index 325c24ae7a9..9857e00f70d 100755 --- a/scripts/circle-test-frontend.sh +++ b/scripts/circle-test-frontend.sh @@ -10,5 +10,10 @@ function exit_if_fail { fi } -exit_if_fail npm run test -exit_if_fail npm run build \ No newline at end of file +exit_if_fail npm run test:coverage +exit_if_fail npm run build + +# publish code coverage +echo "Publishing javascript code coverage" +bash <(curl -s https://codecov.io/bash) -cF javascript +rm -rf coverage diff --git a/scripts/grunt/options/exec.js b/scripts/grunt/options/exec.js index be163581bf6..e22d060ea04 100644 --- a/scripts/grunt/options/exec.js +++ b/scripts/grunt/options/exec.js @@ -1,9 +1,14 @@ module.exports = function(config, grunt) { 'use strict'; + var coverage = ''; + if (config.coverage) { + coverage = '--coverage --maxWorkers 2'; + } + return { tslint: 'node ./node_modules/tslint/lib/tslint-cli.js -c tslint.json --project ./tsconfig.json', - jest: 'node ./node_modules/jest-cli/bin/jest.js --maxWorkers 2', + jest: 'node ./node_modules/jest-cli/bin/jest.js ' + coverage, webpack: 'node ./node_modules/webpack/bin/webpack.js --config scripts/webpack/webpack.prod.js', }; }; From 450a3b4a0080149c2fcf6a59c6b48bbdeff00241 Mon Sep 17 00:00:00 2001 From: Carl Bergquist Date: Tue, 17 Apr 2018 09:00:55 +0200 Subject: [PATCH 58/77] Revert "build: remove code cov" --- scripts/circle-test-backend.sh | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/scripts/circle-test-backend.sh b/scripts/circle-test-backend.sh index 71fc598b609..a63d6354fa6 100755 --- a/scripts/circle-test-backend.sh +++ b/scripts/circle-test-backend.sh @@ -20,4 +20,17 @@ echo "building backend with install to cache pkgs" exit_if_fail time go install ./pkg/cmd/grafana-server echo "running go test" -go test ./pkg/... + +set -e +echo "" > coverage.txt + +time for d in $(go list ./pkg/...); do + exit_if_fail go test -coverprofile=profile.out -covermode=atomic $d + if [ -f profile.out ]; then + cat profile.out >> coverage.txt + rm profile.out + fi +done + +echo "Publishing go code coverage" +bash <(curl -s https://codecov.io/bash) -cF go From a01e6fc9c7d221385467fe4d4e2b7ccd2715b18b Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 17 Apr 2018 09:19:17 +0200 Subject: [PATCH 59/77] add some more sort order asserts for permissions store tests --- public/app/stores/PermissionsStore/PermissionsStore.jest.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts index f1713542be5..d6a20e25846 100644 --- a/public/app/stores/PermissionsStore/PermissionsStore.jest.ts +++ b/public/app/stores/PermissionsStore/PermissionsStore.jest.ts @@ -75,6 +75,7 @@ describe('PermissionsStore', () => { it('should be sorted by sort rank and alphabetically', async () => { expect(store.items[0].name).toBe('MyTestTeam'); + expect(store.items[0].dashboardId).toBe(10); expect(store.items[1].name).toBe('Editor'); expect(store.items[2].name).toBe('Viewer'); expect(store.items[3].name).toBe('MyTestTeam2'); @@ -102,9 +103,11 @@ describe('PermissionsStore', () => { it('should be sorted by sort rank and alphabetically', async () => { expect(store.items[0].name).toBe('MyTestTeam'); + expect(store.items[0].dashboardId).toBe(10); expect(store.items[1].name).toBe('Editor'); expect(store.items[2].name).toBe('Viewer'); expect(store.items[3].name).toBe('MyTestTeam'); + expect(store.items[3].dashboardId).toBe(1); expect(store.items[4].name).toBe('MyTestTeam2'); expect(store.items[5].name).toBe('MyTestUser'); }); From 9ec89c1848e65bd079e69bdc26c4fade9591b95e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 17 Apr 2018 09:20:43 +0200 Subject: [PATCH 60/77] disable codecov comments --- codecov.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/codecov.yml b/codecov.yml index 82a86e0232b..b2a839365ac 100644 --- a/codecov.yml +++ b/codecov.yml @@ -8,6 +8,4 @@ coverage: patch: yes changes: no -comment: - layout: "diff" - behavior: "once" +comment: off From 85121e55c9ee2bf3401416b52ff6e4aeb0a70d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 17 Apr 2018 09:41:07 +0200 Subject: [PATCH 61/77] fix: Row state is now ignored when looking for dashboard changes (#11608) * fix: Row state is now ignored when looking for dashboawrd changes, had to fix bug in expand logic to make fix work. Also moved the ChangeTracker to it's own file and moved tests to jest, fixes #11208 * removed commented out log calls --- .../app/features/dashboard/change_tracker.ts | 186 +++++++++++++++ .../app/features/dashboard/dashboard_model.ts | 3 +- .../dashboard/specs/change_tracker.jest.ts | 99 ++++++++ .../dashboard/specs/dashboard_model.jest.ts | 14 +- .../specs/unsaved_changes_srv_specs.ts | 95 -------- .../features/dashboard/unsaved_changes_srv.ts | 211 +----------------- .../templating/specs/query_variable.jest.ts | 1 - 7 files changed, 296 insertions(+), 313 deletions(-) create mode 100644 public/app/features/dashboard/change_tracker.ts create mode 100644 public/app/features/dashboard/specs/change_tracker.jest.ts delete mode 100644 public/app/features/dashboard/specs/unsaved_changes_srv_specs.ts diff --git a/public/app/features/dashboard/change_tracker.ts b/public/app/features/dashboard/change_tracker.ts new file mode 100644 index 00000000000..745b76ce347 --- /dev/null +++ b/public/app/features/dashboard/change_tracker.ts @@ -0,0 +1,186 @@ +import angular from 'angular'; +import _ from 'lodash'; +import { DashboardModel } from './dashboard_model'; + +export class ChangeTracker { + current: any; + originalPath: any; + scope: any; + original: any; + next: any; + $window: any; + + /** @ngInject */ + constructor( + dashboard, + scope, + originalCopyDelay, + private $location, + $window, + private $timeout, + private contextSrv, + private $rootScope + ) { + this.$location = $location; + this.$window = $window; + + this.current = dashboard; + this.originalPath = $location.path(); + this.scope = scope; + + // register events + scope.onAppEvent('dashboard-saved', () => { + this.original = this.current.getSaveModelClone(); + this.originalPath = $location.path(); + }); + + $window.onbeforeunload = () => { + if (this.ignoreChanges()) { + return undefined; + } + if (this.hasChanges()) { + return 'There are unsaved changes to this dashboard'; + } + return undefined; + }; + + scope.$on('$locationChangeStart', (event, next) => { + // check if we should look for changes + if (this.originalPath === $location.path()) { + return true; + } + if (this.ignoreChanges()) { + return true; + } + + if (this.hasChanges()) { + event.preventDefault(); + this.next = next; + + this.$timeout(() => { + this.open_modal(); + }); + } + return false; + }); + + if (originalCopyDelay) { + this.$timeout(() => { + // wait for different services to patch the dashboard (missing properties) + this.original = dashboard.getSaveModelClone(); + }, originalCopyDelay); + } else { + this.original = dashboard.getSaveModelClone(); + } + } + + // for some dashboards and users + // changes should be ignored + ignoreChanges() { + if (!this.original) { + return true; + } + if (!this.contextSrv.isEditor) { + return true; + } + if (!this.current || !this.current.meta) { + return true; + } + + var meta = this.current.meta; + return !meta.canSave || meta.fromScript || meta.fromFile; + } + + // remove stuff that should not count in diff + cleanDashboardFromIgnoredChanges(dashData) { + // need to new up the domain model class to get access to expand / collapse row logic + let model = new DashboardModel(dashData); + + // Expand all rows before making comparison. This is required because row expand / collapse + // change order of panel array and panel positions. + model.expandRows(); + + let dash = model.getSaveModelClone(); + + // ignore time and refresh + dash.time = 0; + dash.refresh = 0; + dash.schemaVersion = 0; + + // ignore iteration property + delete dash.iteration; + + dash.panels = _.filter(dash.panels, panel => { + if (panel.repeatPanelId) { + return false; + } + + // remove scopedVars + panel.scopedVars = null; + + // ignore panel legend sort + if (panel.legend) { + delete panel.legend.sort; + delete panel.legend.sortDesc; + } + + return true; + }); + + // ignore template variable values + _.each(dash.templating.list, function(value) { + value.current = null; + value.options = null; + value.filters = null; + }); + + return dash; + } + + hasChanges() { + let current = this.cleanDashboardFromIgnoredChanges(this.current.getSaveModelClone()); + let original = this.cleanDashboardFromIgnoredChanges(this.original); + + var currentTimepicker = _.find(current.nav, { type: 'timepicker' }); + var originalTimepicker = _.find(original.nav, { type: 'timepicker' }); + + if (currentTimepicker && originalTimepicker) { + currentTimepicker.now = originalTimepicker.now; + } + + var currentJson = angular.toJson(current, true); + var originalJson = angular.toJson(original, true); + + return currentJson !== originalJson; + } + + discardChanges() { + this.original = null; + this.gotoNext(); + } + + open_modal() { + this.$rootScope.appEvent('show-modal', { + templateHtml: '', + modalClass: 'modal--narrow confirm-modal', + }); + } + + saveChanges() { + var self = this; + var cancel = this.$rootScope.$on('dashboard-saved', () => { + cancel(); + this.$timeout(() => { + self.gotoNext(); + }); + }); + + this.$rootScope.appEvent('save-dashboard'); + } + + gotoNext() { + var baseLen = this.$location.absUrl().length - this.$location.url().length; + var nextUrl = this.next.substring(baseLen); + this.$location.url(nextUrl); + } +} diff --git a/public/app/features/dashboard/dashboard_model.ts b/public/app/features/dashboard/dashboard_model.ts index 9130cb7e806..8a300a80341 100644 --- a/public/app/features/dashboard/dashboard_model.ts +++ b/public/app/features/dashboard/dashboard_model.ts @@ -649,6 +649,7 @@ export class DashboardModel { for (let panel of row.panels) { // make sure y is adjusted (in case row moved while collapsed) + // console.log('yDiff', yDiff); panel.gridPos.y -= yDiff; // insert after row this.panels.splice(insertPos, 0, new PanelModel(panel)); @@ -657,7 +658,7 @@ export class DashboardModel { yMax = Math.max(yMax, panel.gridPos.y + panel.gridPos.h); } - const pushDownAmount = yMax - row.gridPos.y; + const pushDownAmount = yMax - row.gridPos.y - 1; // push panels below down for (let panelIndex = insertPos; panelIndex < this.panels.length; panelIndex++) { diff --git a/public/app/features/dashboard/specs/change_tracker.jest.ts b/public/app/features/dashboard/specs/change_tracker.jest.ts new file mode 100644 index 00000000000..5ec84aadbd0 --- /dev/null +++ b/public/app/features/dashboard/specs/change_tracker.jest.ts @@ -0,0 +1,99 @@ +import { ChangeTracker } from 'app/features/dashboard/change_tracker'; +import { contextSrv } from 'app/core/services/context_srv'; +import { DashboardModel } from '../dashboard_model'; +import { PanelModel } from '../panel_model'; + +jest.mock('app/core/services/context_srv', () => ({ + contextSrv: { + user: { orgId: 1 }, + }, +})); + +describe('ChangeTracker', () => { + let rootScope; + let location; + let timeout; + let tracker: ChangeTracker; + let dash; + let scope; + + beforeEach(() => { + dash = new DashboardModel({ + refresh: false, + panels: [ + { + id: 1, + type: 'graph', + gridPos: { x: 0, y: 0, w: 24, h: 6 }, + legend: { sortDesc: false }, + }, + { + id: 2, + type: 'row', + gridPos: { x: 0, y: 6, w: 24, h: 2 }, + collapsed: true, + panels: [ + { id: 3, type: 'graph', gridPos: { x: 0, y: 6, w: 12, h: 2 } }, + { id: 4, type: 'graph', gridPos: { x: 12, y: 6, w: 12, h: 2 } }, + ], + }, + { id: 5, type: 'row', gridPos: { x: 0, y: 6, w: 1, h: 1 } }, + ], + }); + + scope = { + appEvent: jest.fn(), + onAppEvent: jest.fn(), + $on: jest.fn(), + }; + + rootScope = { + appEvent: jest.fn(), + onAppEvent: jest.fn(), + $on: jest.fn(), + }; + + location = { + path: jest.fn(), + }; + + tracker = new ChangeTracker(dash, scope, undefined, location, window, timeout, contextSrv, rootScope); + }); + + it('No changes should not have changes', () => { + expect(tracker.hasChanges()).toBe(false); + }); + + it('Simple change should be registered', () => { + dash.title = 'google'; + expect(tracker.hasChanges()).toBe(true); + }); + + it('Should ignore a lot of changes', () => { + dash.time = { from: '1h' }; + dash.refresh = true; + dash.schemaVersion = 10; + expect(tracker.hasChanges()).toBe(false); + }); + + it('Should ignore .iteration changes', () => { + dash.iteration = new Date().getTime() + 1; + expect(tracker.hasChanges()).toBe(false); + }); + + it('Should ignore row collapse change', () => { + dash.toggleRow(dash.panels[1]); + expect(tracker.hasChanges()).toBe(false); + }); + + it('Should ignore panel legend changes', () => { + dash.panels[0].legend.sortDesc = true; + dash.panels[0].legend.sort = 'avg'; + expect(tracker.hasChanges()).toBe(false); + }); + + it('Should ignore panel repeats', () => { + dash.panels.push(new PanelModel({ repeatPanelId: 10 })); + expect(tracker.hasChanges()).toBe(false); + }); +}); diff --git a/public/app/features/dashboard/specs/dashboard_model.jest.ts b/public/app/features/dashboard/specs/dashboard_model.jest.ts index 99fe727c49d..feede679018 100644 --- a/public/app/features/dashboard/specs/dashboard_model.jest.ts +++ b/public/app/features/dashboard/specs/dashboard_model.jest.ts @@ -374,14 +374,14 @@ describe('DashboardModel', function() { { id: 2, type: 'row', - gridPos: { x: 0, y: 6, w: 24, h: 2 }, + gridPos: { x: 0, y: 6, w: 24, h: 1 }, collapsed: true, panels: [ - { id: 3, type: 'graph', gridPos: { x: 0, y: 2, w: 12, h: 2 } }, - { id: 4, type: 'graph', gridPos: { x: 12, y: 2, w: 12, h: 2 } }, + { id: 3, type: 'graph', gridPos: { x: 0, y: 7, w: 12, h: 2 } }, + { id: 4, type: 'graph', gridPos: { x: 12, y: 7, w: 12, h: 2 } }, ], }, - { id: 5, type: 'row', gridPos: { x: 0, y: 6, w: 1, h: 1 } }, + { id: 5, type: 'row', gridPos: { x: 0, y: 7, w: 1, h: 1 } }, ], }); dashboard.toggleRow(dashboard.panels[1]); @@ -399,16 +399,16 @@ describe('DashboardModel', function() { it('should position them below row', function() { expect(dashboard.panels[2].gridPos).toMatchObject({ x: 0, - y: 8, + y: 7, w: 12, h: 2, }); }); - it('should move panels below down', function() { + it.only('should move panels below down', function() { expect(dashboard.panels[4].gridPos).toMatchObject({ x: 0, - y: 10, + y: 9, w: 1, h: 1, }); diff --git a/public/app/features/dashboard/specs/unsaved_changes_srv_specs.ts b/public/app/features/dashboard/specs/unsaved_changes_srv_specs.ts deleted file mode 100644 index 8bd639de681..00000000000 --- a/public/app/features/dashboard/specs/unsaved_changes_srv_specs.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { describe, beforeEach, it, expect, sinon, angularMocks } from 'test/lib/common'; -import { Tracker } from 'app/features/dashboard/unsaved_changes_srv'; -import 'app/features/dashboard/dashboard_srv'; -import { contextSrv } from 'app/core/core'; - -describe('unsavedChangesSrv', function() { - var _dashboardSrv; - var _contextSrvStub = { isEditor: true }; - var _rootScope; - var _location; - var _timeout; - var _window; - var tracker; - var dash; - var scope; - - beforeEach(angularMocks.module('grafana.core')); - beforeEach(angularMocks.module('grafana.services')); - beforeEach( - angularMocks.module(function($provide) { - $provide.value('contextSrv', _contextSrvStub); - $provide.value('$window', {}); - }) - ); - - beforeEach( - angularMocks.inject(function($location, $rootScope, dashboardSrv, $timeout, $window) { - _dashboardSrv = dashboardSrv; - _rootScope = $rootScope; - _location = $location; - _timeout = $timeout; - _window = $window; - }) - ); - - beforeEach(function() { - dash = _dashboardSrv.create({ - refresh: false, - panels: [{ test: 'asd', legend: {} }], - rows: [ - { - panels: [{ test: 'asd', legend: {} }], - }, - ], - }); - scope = _rootScope.$new(); - scope.appEvent = sinon.spy(); - scope.onAppEvent = sinon.spy(); - - tracker = new Tracker(dash, scope, undefined, _location, _window, _timeout, contextSrv, _rootScope); - }); - - it('No changes should not have changes', function() { - expect(tracker.hasChanges()).to.be(false); - }); - - it('Simple change should be registered', function() { - dash.property = 'google'; - expect(tracker.hasChanges()).to.be(true); - }); - - it('Should ignore a lot of changes', function() { - dash.time = { from: '1h' }; - dash.refresh = true; - dash.schemaVersion = 10; - expect(tracker.hasChanges()).to.be(false); - }); - - it('Should ignore .iteration changes', () => { - dash.iteration = new Date().getTime() + 1; - expect(tracker.hasChanges()).to.be(false); - }); - - it.skip('Should ignore row collapse change', function() { - dash.rows[0].collapse = true; - expect(tracker.hasChanges()).to.be(false); - }); - - it('Should ignore panel legend changes', function() { - dash.panels[0].legend.sortDesc = true; - dash.panels[0].legend.sort = 'avg'; - expect(tracker.hasChanges()).to.be(false); - }); - - it.skip('Should ignore panel repeats', function() { - dash.rows[0].panels.push({ repeatPanelId: 10 }); - expect(tracker.hasChanges()).to.be(false); - }); - - it.skip('Should ignore row repeats', function() { - dash.addEmptyRow(); - dash.rows[1].repeatRowId = 10; - expect(tracker.hasChanges()).to.be(false); - }); -}); diff --git a/public/app/features/dashboard/unsaved_changes_srv.ts b/public/app/features/dashboard/unsaved_changes_srv.ts index d4c12b8bcd6..0406e6a55d7 100644 --- a/public/app/features/dashboard/unsaved_changes_srv.ts +++ b/public/app/features/dashboard/unsaved_changes_srv.ts @@ -1,217 +1,10 @@ import angular from 'angular'; -import _ from 'lodash'; - -export class Tracker { - current: any; - originalPath: any; - scope: any; - original: any; - next: any; - $window: any; - - /** @ngInject */ - constructor( - dashboard, - scope, - originalCopyDelay, - private $location, - $window, - private $timeout, - private contextSrv, - private $rootScope - ) { - this.$location = $location; - this.$window = $window; - - this.current = dashboard; - this.originalPath = $location.path(); - this.scope = scope; - - // register events - scope.onAppEvent('dashboard-saved', () => { - this.original = this.current.getSaveModelClone(); - this.originalPath = $location.path(); - }); - - $window.onbeforeunload = () => { - if (this.ignoreChanges()) { - return undefined; - } - if (this.hasChanges()) { - return 'There are unsaved changes to this dashboard'; - } - return undefined; - }; - - scope.$on('$locationChangeStart', (event, next) => { - // check if we should look for changes - if (this.originalPath === $location.path()) { - return true; - } - if (this.ignoreChanges()) { - return true; - } - - if (this.hasChanges()) { - event.preventDefault(); - this.next = next; - - this.$timeout(() => { - this.open_modal(); - }); - } - return false; - }); - - if (originalCopyDelay) { - this.$timeout(() => { - // wait for different services to patch the dashboard (missing properties) - this.original = dashboard.getSaveModelClone(); - }, originalCopyDelay); - } else { - this.original = dashboard.getSaveModelClone(); - } - } - - // for some dashboards and users - // changes should be ignored - ignoreChanges() { - if (!this.original) { - return true; - } - if (!this.contextSrv.isEditor) { - return true; - } - if (!this.current || !this.current.meta) { - return true; - } - - var meta = this.current.meta; - return !meta.canSave || meta.fromScript || meta.fromFile; - } - - // remove stuff that should not count in diff - cleanDashboardFromIgnoredChanges(dash) { - // ignore time and refresh - dash.time = 0; - dash.refresh = 0; - dash.schemaVersion = 0; - - // ignore iteration property - delete dash.iteration; - - // filter row and panels properties that should be ignored - dash.rows = _.filter(dash.rows, function(row) { - if (row.repeatRowId) { - return false; - } - - row.panels = _.filter(row.panels, function(panel) { - if (panel.repeatPanelId) { - return false; - } - - // remove scopedVars - panel.scopedVars = null; - - // ignore span changes - panel.span = null; - - // ignore panel legend sort - if (panel.legend) { - delete panel.legend.sort; - delete panel.legend.sortDesc; - } - - return true; - }); - - // ignore collapse state - row.collapse = false; - return true; - }); - - dash.panels = _.filter(dash.panels, panel => { - if (panel.repeatPanelId) { - return false; - } - - // remove scopedVars - panel.scopedVars = null; - - // ignore panel legend sort - if (panel.legend) { - delete panel.legend.sort; - delete panel.legend.sortDesc; - } - - return true; - }); - - // ignore template variable values - _.each(dash.templating.list, function(value) { - value.current = null; - value.options = null; - value.filters = null; - }); - } - - hasChanges() { - var current = this.current.getSaveModelClone(); - var original = this.original; - - this.cleanDashboardFromIgnoredChanges(current); - this.cleanDashboardFromIgnoredChanges(original); - - var currentTimepicker = _.find(current.nav, { type: 'timepicker' }); - var originalTimepicker = _.find(original.nav, { type: 'timepicker' }); - - if (currentTimepicker && originalTimepicker) { - currentTimepicker.now = originalTimepicker.now; - } - - var currentJson = angular.toJson(current); - var originalJson = angular.toJson(original); - - return currentJson !== originalJson; - } - - discardChanges() { - this.original = null; - this.gotoNext(); - } - - open_modal() { - this.$rootScope.appEvent('show-modal', { - templateHtml: '', - modalClass: 'modal--narrow confirm-modal', - }); - } - - saveChanges() { - var self = this; - var cancel = this.$rootScope.$on('dashboard-saved', () => { - cancel(); - this.$timeout(() => { - self.gotoNext(); - }); - }); - - this.$rootScope.appEvent('save-dashboard'); - } - - gotoNext() { - var baseLen = this.$location.absUrl().length - this.$location.url().length; - var nextUrl = this.next.substring(baseLen); - this.$location.url(nextUrl); - } -} +import { ChangeTracker } from './change_tracker'; /** @ngInject */ export function unsavedChangesSrv($rootScope, $q, $location, $timeout, contextSrv, dashboardSrv, $window) { - this.Tracker = Tracker; this.init = function(dashboard, scope) { - this.tracker = new Tracker(dashboard, scope, 1000, $location, $window, $timeout, contextSrv, $rootScope); + this.tracker = new ChangeTracker(dashboard, scope, 1000, $location, $window, $timeout, contextSrv, $rootScope); return this.tracker; }; } diff --git a/public/app/features/templating/specs/query_variable.jest.ts b/public/app/features/templating/specs/query_variable.jest.ts index ce753a4b205..39c51874586 100644 --- a/public/app/features/templating/specs/query_variable.jest.ts +++ b/public/app/features/templating/specs/query_variable.jest.ts @@ -91,7 +91,6 @@ describe('QueryVariable', () => { it('should return in same order', () => { var i = 0; - console.log(result); expect(result.length).toBe(11); expect(result[i++].text).toBe(''); expect(result[i++].text).toBe('0'); From ffe9b426d4a7871078c81c5254a85115f1506e9b Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 17 Apr 2018 10:00:09 +0200 Subject: [PATCH 62/77] changelog: notes about closing #10747 [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f433b102342..81059a4d58f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ * **Playlist**: Empty playlists cannot be deleted [#11133](https://github.com/grafana/grafana/issues/11133), thx [@kichristensen](https://github.com/kichristensen) * **Switch Orgs**: Alphabetic order in Switch Organization modal [#11556](https://github.com/grafana/grafana/issues/11556) * **Postgres**: improve `$__timeFilter` macro [#11578](https://github.com/grafana/grafana/issues/11578), thx [@svenklemm](https://github.com/svenklemm) +* **Permission list**: Improved ux [#10747](https://github.com/grafana/grafana/issues/10747) ### Tech * Migrated JavaScript files to TypeScript From e07de80b75266dc64e01d47d34e3137d82d2f684 Mon Sep 17 00:00:00 2001 From: Dan Cech Date: Tue, 17 Apr 2018 04:20:01 -0400 Subject: [PATCH 63/77] Fix issues with metric reporting (#11518) * report active users in graphite stats * use bus to publish system stats * metrics: avoid using events unless we have to this commit also changes the default interval for updating the stats gauges. Seems like the old values was a product of previous metrics implementation --- pkg/metrics/metrics.go | 42 ++++++++++++++++++++-------------- pkg/services/sqlstore/stats.go | 1 + 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 4cefe12d6e5..e3640378f7e 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -54,6 +54,7 @@ var ( M_Alerting_Active_Alerts prometheus.Gauge M_StatTotal_Dashboards prometheus.Gauge M_StatTotal_Users prometheus.Gauge + M_StatActive_Users prometheus.Gauge M_StatTotal_Orgs prometheus.Gauge M_StatTotal_Playlists prometheus.Gauge M_Grafana_Version *prometheus.GaugeVec @@ -253,6 +254,12 @@ func init() { Namespace: exporterName, }) + M_StatActive_Users = prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "stat_active_users", + Help: "number of active users", + Namespace: exporterName, + }) + M_StatTotal_Orgs = prometheus.NewGauge(prometheus.GaugeOpts{ Name: "stat_total_orgs", Help: "total amount of orgs", @@ -270,7 +277,6 @@ func init() { Help: "Information about the Grafana", Namespace: exporterName, }, []string{"version"}) - } func initMetricVars(settings *MetricSettings) { @@ -305,6 +311,7 @@ func initMetricVars(settings *MetricSettings) { M_Alerting_Active_Alerts, M_StatTotal_Dashboards, M_StatTotal_Users, + M_StatActive_Users, M_StatTotal_Orgs, M_StatTotal_Playlists, M_Grafana_Version) @@ -315,35 +322,36 @@ func initMetricVars(settings *MetricSettings) { func instrumentationLoop(settings *MetricSettings) chan struct{} { M_Instance_Start.Inc() + // set the total stats gauges before we publishing metrics + updateTotalStats() + onceEveryDayTick := time.NewTicker(time.Hour * 24) - secondTicker := time.NewTicker(time.Second * time.Duration(settings.IntervalSeconds)) + everyMinuteTicker := time.NewTicker(time.Minute) + defer onceEveryDayTick.Stop() + defer everyMinuteTicker.Stop() for { select { case <-onceEveryDayTick.C: sendUsageStats() - case <-secondTicker.C: + case <-everyMinuteTicker.C: updateTotalStats() } } } -var metricPublishCounter int64 = 0 - func updateTotalStats() { - metricPublishCounter++ - if metricPublishCounter == 1 || metricPublishCounter%10 == 0 { - statsQuery := models.GetSystemStatsQuery{} - if err := bus.Dispatch(&statsQuery); err != nil { - metricsLogger.Error("Failed to get system stats", "error", err) - return - } - - M_StatTotal_Dashboards.Set(float64(statsQuery.Result.Dashboards)) - M_StatTotal_Users.Set(float64(statsQuery.Result.Users)) - M_StatTotal_Playlists.Set(float64(statsQuery.Result.Playlists)) - M_StatTotal_Orgs.Set(float64(statsQuery.Result.Orgs)) + statsQuery := models.GetSystemStatsQuery{} + if err := bus.Dispatch(&statsQuery); err != nil { + metricsLogger.Error("Failed to get system stats", "error", err) + return } + + M_StatTotal_Dashboards.Set(float64(statsQuery.Result.Dashboards)) + M_StatTotal_Users.Set(float64(statsQuery.Result.Users)) + M_StatActive_Users.Set(float64(statsQuery.Result.ActiveUsers)) + M_StatTotal_Playlists.Set(float64(statsQuery.Result.Playlists)) + M_StatTotal_Orgs.Set(float64(statsQuery.Result.Orgs)) } func sendUsageStats() { diff --git a/pkg/services/sqlstore/stats.go b/pkg/services/sqlstore/stats.go index cfe2d88c82c..0138b7f283d 100644 --- a/pkg/services/sqlstore/stats.go +++ b/pkg/services/sqlstore/stats.go @@ -68,6 +68,7 @@ func GetSystemStats(query *m.GetSystemStatsQuery) error { } query.Result = &stats + return err } From a3eff6cf25ae3afe7ff0d85363cc599e6e6adf00 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 17 Apr 2018 10:27:12 +0200 Subject: [PATCH 64/77] changelog: notes about closing #11572 [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81059a4d58f..951e80c3557 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ * **Switch Orgs**: Alphabetic order in Switch Organization modal [#11556](https://github.com/grafana/grafana/issues/11556) * **Postgres**: improve `$__timeFilter` macro [#11578](https://github.com/grafana/grafana/issues/11578), thx [@svenklemm](https://github.com/svenklemm) * **Permission list**: Improved ux [#10747](https://github.com/grafana/grafana/issues/10747) +* **Dashboard**: sizing and positioning of settings menu icons [#11572](https://github.com/grafana/grafana/pull/11572) ### Tech * Migrated JavaScript files to TypeScript From e21d5748145afb94075e664aba663a35597fd8d5 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 17 Apr 2018 10:28:16 +0200 Subject: [PATCH 65/77] changelog: fix typo [skip ci] --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 951e80c3557..45881e602bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,7 +51,7 @@ * **Switch Orgs**: Alphabetic order in Switch Organization modal [#11556](https://github.com/grafana/grafana/issues/11556) * **Postgres**: improve `$__timeFilter` macro [#11578](https://github.com/grafana/grafana/issues/11578), thx [@svenklemm](https://github.com/svenklemm) * **Permission list**: Improved ux [#10747](https://github.com/grafana/grafana/issues/10747) -* **Dashboard**: sizing and positioning of settings menu icons [#11572](https://github.com/grafana/grafana/pull/11572) +* **Dashboard**: Sizing and positioning of settings menu icons [#11572](https://github.com/grafana/grafana/pull/11572) ### Tech * Migrated JavaScript files to TypeScript From 2fc1558daf86d3c786398b4e4503fb0c677510d0 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 17 Apr 2018 11:03:46 +0200 Subject: [PATCH 66/77] revert changes of add panel button to require save permission This resolves an issue where the add panel button was shown on the default home dashboard --- public/app/features/dashboard/dashnav/dashnav.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/features/dashboard/dashnav/dashnav.html b/public/app/features/dashboard/dashnav/dashnav.html index 0c3f949ed7c..269d4b0bada 100644 --- a/public/app/features/dashboard/dashnav/dashnav.html +++ b/public/app/features/dashboard/dashnav/dashnav.html @@ -17,7 +17,7 @@