From 84ff62d77372d110bd6cc80dd1efb597d1416a8d Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Fri, 16 Jun 2017 21:14:42 +0200 Subject: [PATCH 1/5] WIP: remove dashboard children on delete --- pkg/services/sqlstore/dashboard.go | 1 + pkg/services/sqlstore/dashboard_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 21baa63f2b0..489cc7022e6 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -371,6 +371,7 @@ func DeleteDashboard(cmd *m.DeleteDashboardCommand) error { "DELETE FROM dashboard WHERE id = ?", "DELETE FROM playlist_item WHERE type = 'dashboard_by_id' AND value = ?", "DELETE FROM dashboard_version WHERE dashboard_id = ?", + "DELETE FROM dashboard WHERE parent_id = ?", } for _, sql := range deletes { diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 98439cb7178..9a8d2312e5d 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -247,6 +247,22 @@ func TestDashboardDataAccess(t *testing.T) { So(query.Result.ParentId, ShouldEqual, 0) }) + Convey("Should be able to delete a dashboard folder and its children", func() { + deleteCmd := &m.DeleteDashboardCommand{Slug: savedFolder.Slug} + err := DeleteDashboard(deleteCmd) + So(err, ShouldBeNil) + + query := search.FindPersistedDashboardsQuery{ + OrgId: 1, + ParentId: savedFolder.Id, + } + + err = SearchDashboards(&query) + So(err, ShouldBeNil) + + So(len(query.Result), ShouldEqual, 0) + }) + Convey("Should be able to get dashboard tags", func() { query := m.GetDashboardTagsQuery{OrgId: 1} From 5cf40cbd125e36f43c0542350af9de1f9c5acc4a Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Sat, 17 Jun 2017 02:33:53 +0200 Subject: [PATCH 2/5] WIP: move guardian logic for search into the sql query --- pkg/api/playlist.go | 2 +- pkg/api/playlist_play.go | 18 ++-- pkg/api/search.go | 6 +- pkg/services/guardian/guardian.go | 23 ----- pkg/services/search/handlers.go | 34 +------ pkg/services/search/handlers_test.go | 2 +- pkg/services/search/models.go | 9 +- pkg/services/sqlstore/dashboard.go | 26 ++++- pkg/services/sqlstore/dashboard_test.go | 123 +++++++++++++++++++++--- pkg/services/sqlstore/guardian.go | 2 +- pkg/services/sqlstore/guardian_test.go | 26 ----- 11 files changed, 153 insertions(+), 118 deletions(-) diff --git a/pkg/api/playlist.go b/pkg/api/playlist.go index a6c2da26dd8..040aef0474e 100644 --- a/pkg/api/playlist.go +++ b/pkg/api/playlist.go @@ -130,7 +130,7 @@ func GetPlaylistItems(c *middleware.Context) Response { func GetPlaylistDashboards(c *middleware.Context) Response { playlistId := c.ParamsInt64(":id") - playlists, err := LoadPlaylistDashboards(c.OrgId, c.UserId, playlistId) + playlists, err := LoadPlaylistDashboards(c.OrgId, c.SignedInUser, playlistId) if err != nil { return ApiError(500, "Could not load dashboards", err) } diff --git a/pkg/api/playlist_play.go b/pkg/api/playlist_play.go index 29a806ce23d..1d059e06be5 100644 --- a/pkg/api/playlist_play.go +++ b/pkg/api/playlist_play.go @@ -34,18 +34,18 @@ func populateDashboardsById(dashboardByIds []int64, dashboardIdOrder map[int64]i return result, nil } -func populateDashboardsByTag(orgId, userId int64, dashboardByTag []string, dashboardTagOrder map[string]int) dtos.PlaylistDashboardsSlice { +func populateDashboardsByTag(orgId int64, signedInUser *m.SignedInUser, dashboardByTag []string, dashboardTagOrder map[string]int) dtos.PlaylistDashboardsSlice { result := make(dtos.PlaylistDashboardsSlice, 0) if len(dashboardByTag) > 0 { for _, tag := range dashboardByTag { searchQuery := search.Query{ - Title: "", - Tags: []string{tag}, - UserId: userId, - Limit: 100, - IsStarred: false, - OrgId: orgId, + Title: "", + Tags: []string{tag}, + SignedInUser: signedInUser, + Limit: 100, + IsStarred: false, + OrgId: orgId, } if err := bus.Dispatch(&searchQuery); err == nil { @@ -64,7 +64,7 @@ func populateDashboardsByTag(orgId, userId int64, dashboardByTag []string, dashb return result } -func LoadPlaylistDashboards(orgId, userId, playlistId int64) (dtos.PlaylistDashboardsSlice, error) { +func LoadPlaylistDashboards(orgId int64, signedInUser *m.SignedInUser, playlistId int64) (dtos.PlaylistDashboardsSlice, error) { playlistItems, _ := LoadPlaylistItems(playlistId) dashboardByIds := make([]int64, 0) @@ -89,7 +89,7 @@ func LoadPlaylistDashboards(orgId, userId, playlistId int64) (dtos.PlaylistDashb var k, _ = populateDashboardsById(dashboardByIds, dashboardIdOrder) result = append(result, k...) - result = append(result, populateDashboardsByTag(orgId, userId, dashboardByTag, dashboardTagOrder)...) + result = append(result, populateDashboardsByTag(orgId, signedInUser, dashboardByTag, dashboardTagOrder)...) sort.Sort(result) return result, nil diff --git a/pkg/api/search.go b/pkg/api/search.go index bf6e5ad91fe..17b1ac0da61 100644 --- a/pkg/api/search.go +++ b/pkg/api/search.go @@ -26,9 +26,9 @@ func Search(c *middleware.Context) { mode = "list" } - dbids := make([]int, 0) + dbids := make([]int64, 0) for _, id := range c.QueryStrings("dashboardIds") { - dashboardId, err := strconv.Atoi(id) + dashboardId, err := strconv.ParseInt(id, 10, 64) if err == nil { dbids = append(dbids, dashboardId) } @@ -37,7 +37,7 @@ func Search(c *middleware.Context) { searchQuery := search.Query{ Title: query, Tags: tags, - UserId: c.UserId, + SignedInUser: c.SignedInUser, Limit: limit, IsStarred: starred == "true", OrgId: c.OrgId, diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 3ed65c85465..6340efee05f 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -5,22 +5,6 @@ import ( m "github.com/grafana/grafana/pkg/models" ) -// FilterRestrictedDashboards filters out dashboards from the list that the user does have access to -func FilterRestrictedDashboards(dashList []int64, orgId int64, userId int64) ([]int64, error) { - user, err := getUser(userId) - if err != nil { - return nil, err - } - - if user.IsGrafanaAdmin || user.OrgRole == m.ROLE_ADMIN { - return dashList, nil - } - - filteredList, err := getAllowedDashboards(dashList, orgId, userId) - - return filteredList, err -} - // CanViewAcl determines if a user has permission to view a dashboard's ACL func CanViewAcl(dashboardId int64, role m.RoleType, isGrafanaAdmin bool, orgId int64, userId int64) (bool, error) { if role == m.ROLE_ADMIN || isGrafanaAdmin { @@ -115,13 +99,6 @@ func checkPermission(minimumPermission m.PermissionType, permissions []*m.Dashbo return false, nil } -func getUser(userId int64) (*m.SignedInUser, error) { - query := m.GetSignedInUserQuery{UserId: userId} - err := bus.Dispatch(&query) - - return query.Result, err -} - func getAllowedDashboards(dashList []int64, orgId int64, userId int64) ([]int64, error) { query := m.GetAllowedDashboardsQuery{UserId: userId, OrgId: orgId, DashList: dashList} err := bus.Dispatch(&query) diff --git a/pkg/services/search/handlers.go b/pkg/services/search/handlers.go index e2ad56e7363..1b7eab79720 100644 --- a/pkg/services/search/handlers.go +++ b/pkg/services/search/handlers.go @@ -7,7 +7,6 @@ import ( "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/setting" ) @@ -41,9 +40,8 @@ func searchHandler(query *Query) error { dashQuery := FindPersistedDashboardsQuery{ Title: query.Title, - UserId: query.UserId, + SignedInUser: query.SignedInUser, IsStarred: query.IsStarred, - OrgId: query.OrgId, DashboardIds: query.DashboardIds, Type: query.Type, ParentId: query.FolderId, @@ -76,11 +74,6 @@ func searchHandler(query *Query) error { hits = filtered } - hits, err := removeRestrictedDashboardsFromList(hits, query) - if err != nil { - return err - } - // sort main result array sort.Sort(hits) @@ -94,7 +87,7 @@ func searchHandler(query *Query) error { } // add isStarred info - if err := setIsStarredFlagOnSearchResults(query.UserId, hits); err != nil { + if err := setIsStarredFlagOnSearchResults(query.SignedInUser.UserId, hits); err != nil { return err } @@ -102,29 +95,6 @@ func searchHandler(query *Query) error { return nil } -func removeRestrictedDashboardsFromList(hits HitList, query *Query) (HitList, error) { - var dashboardIds = []int64{} - for _, hit := range hits { - dashboardIds = append(dashboardIds, hit.Id) - } - - filteredHits, err := guardian.FilterRestrictedDashboards(dashboardIds, query.OrgId, query.UserId) - if err != nil { - return nil, err - } - - filtered := HitList{} - for _, hit := range hits { - for _, dashId := range filteredHits { - if hit.Id == dashId { - filtered = append(filtered, hit) - } - } - } - - return filtered, nil -} - func stringInSlice(a string, list []string) bool { for _, b := range list { if b == a { diff --git a/pkg/services/search/handlers_test.go b/pkg/services/search/handlers_test.go index 3fd7801a323..18b7453271e 100644 --- a/pkg/services/search/handlers_test.go +++ b/pkg/services/search/handlers_test.go @@ -12,7 +12,7 @@ func TestSearch(t *testing.T) { Convey("Given search query", t, func() { jsonDashIndex = NewJsonDashIndex("../../../public/dashboards/") - query := Query{Limit: 2000} + query := Query{Limit: 2000, SignedInUser: &m.SignedInUser{IsGrafanaAdmin: true}} bus.AddHandler("test", func(query *FindPersistedDashboardsQuery) error { query.Result = HitList{ diff --git a/pkg/services/search/models.go b/pkg/services/search/models.go index d0468ac4576..8f6a42ac40a 100644 --- a/pkg/services/search/models.go +++ b/pkg/services/search/models.go @@ -1,6 +1,7 @@ package search import "strings" +import "github.com/grafana/grafana/pkg/models" type HitType string @@ -43,11 +44,11 @@ type Query struct { Title string Tags []string OrgId int64 - UserId int64 + SignedInUser *models.SignedInUser Limit int IsStarred bool Type string - DashboardIds []int + DashboardIds []int64 FolderId int64 Mode string @@ -57,9 +58,9 @@ type Query struct { type FindPersistedDashboardsQuery struct { Title string OrgId int64 - UserId int64 + SignedInUser *models.SignedInUser IsStarred bool - DashboardIds []int + DashboardIds []int64 Type string ParentId int64 Mode string diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 489cc7022e6..169edbf6214 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -166,8 +166,10 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear dashboard.slug, dashboard_tag.term, dashboard.is_folder, - dashboard.parent_id + dashboard.parent_id, + pd.title as folder_title FROM dashboard + LEFT OUTER JOIN dashboard pd on pd.id = dashboard.parent_id LEFT OUTER JOIN dashboard_tag on dashboard_tag.dashboard_id = dashboard.id`) if query.IsStarred { @@ -175,12 +177,11 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear } sql.WriteString(` WHERE dashboard.org_id=?`) - - params = append(params, query.OrgId) + params = append(params, query.SignedInUser.OrgId) if query.IsStarred { sql.WriteString(` AND star.user_id=?`) - params = append(params, query.UserId) + params = append(params, query.SignedInUser.UserId) } if len(query.DashboardIds) > 0 { @@ -196,6 +197,23 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear sql.WriteString(")") } + if query.SignedInUser.OrgRole != m.ROLE_ADMIN { + allowedDashboardsSubQuery := ` AND (dashboard.has_acl = 0 OR dashboard.id in ( + SELECT distinct d.id AS DashboardId + FROM dashboard AS d + LEFT JOIN dashboard AS df ON d.parent_id = df.id + LEFT JOIN dashboard_acl as dfa on d.parent_id = dfa.dashboard_id or d.id = dfa.dashboard_id + LEFT JOIN user_group_member as ugm on ugm.user_group_id = dfa.user_group_id + WHERE + d.has_acl = 1 and + (dfa.user_id = ? or ugm.user_id = ?) + and d.org_id = ? + ))` + + sql.WriteString(allowedDashboardsSubQuery) + params = append(params, query.SignedInUser.UserId, query.SignedInUser.UserId, query.SignedInUser.OrgId) + } + if len(query.Title) > 0 { sql.WriteString(" AND dashboard.title " + dialect.LikeStr() + " ?") params = append(params, "%"+query.Title+"%") diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 9a8d2312e5d..6ba4d5c0a13 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/search" + "github.com/grafana/grafana/pkg/setting" ) func insertTestDashboard(title string, orgId int64, parentId int64, isFolder bool, tags ...interface{}) *m.Dashboard { @@ -116,9 +117,10 @@ func TestDashboardDataAccess(t *testing.T) { Convey("Should be able to search for dashboard and return in folder hierarchy", func() { query := search.FindPersistedDashboardsQuery{ - Title: "test dash 23", - OrgId: 1, - Mode: "tree", + Title: "test dash 23", + OrgId: 1, + Mode: "tree", + SignedInUser: &m.SignedInUser{OrgId: 1}, } err := SearchDashboards(&query) @@ -135,8 +137,9 @@ func TestDashboardDataAccess(t *testing.T) { Convey("Should be able to search for dashboard folder", func() { query := search.FindPersistedDashboardsQuery{ - Title: "1 test dash folder", - OrgId: 1, + Title: "1 test dash folder", + OrgId: 1, + SignedInUser: &m.SignedInUser{OrgId: 1}, } err := SearchDashboards(&query) @@ -149,8 +152,9 @@ func TestDashboardDataAccess(t *testing.T) { Convey("Should be able to search for a dashboard folder's children", func() { query := search.FindPersistedDashboardsQuery{ - OrgId: 1, - ParentId: savedFolder.Id, + OrgId: 1, + ParentId: savedFolder.Id, + SignedInUser: &m.SignedInUser{OrgId: 1}, } err := SearchDashboards(&query) @@ -164,9 +168,9 @@ func TestDashboardDataAccess(t *testing.T) { Convey("Should be able to search for dashboard by dashboard ids", func() { Convey("should be able to find two dashboards by id", func() { query := search.FindPersistedDashboardsQuery{ - DashboardIds: []int{2, 3}, - OrgId: 1, + DashboardIds: []int64{2, 3}, Mode: "tree", + SignedInUser: &m.SignedInUser{OrgId: 1}, } err := SearchDashboards(&query) @@ -183,8 +187,8 @@ func TestDashboardDataAccess(t *testing.T) { Convey("DashboardIds that does not exists should not cause errors", func() { query := search.FindPersistedDashboardsQuery{ - DashboardIds: []int{1000}, - OrgId: 1, + DashboardIds: []int64{1000}, + SignedInUser: &m.SignedInUser{OrgId: 1}, } err := SearchDashboards(&query) @@ -253,8 +257,9 @@ func TestDashboardDataAccess(t *testing.T) { So(err, ShouldBeNil) query := search.FindPersistedDashboardsQuery{ - OrgId: 1, - ParentId: savedFolder.Id, + OrgId: 1, + ParentId: savedFolder.Id, + SignedInUser: &m.SignedInUser{}, } err = SearchDashboards(&query) @@ -285,7 +290,7 @@ func TestDashboardDataAccess(t *testing.T) { }) Convey("Should be able to search for starred dashboards", func() { - query := search.FindPersistedDashboardsQuery{OrgId: 1, UserId: 10, IsStarred: true} + query := search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: 10, OrgId: 1}, IsStarred: true} err := SearchDashboards(&query) So(err, ShouldBeNil) @@ -294,5 +299,95 @@ func TestDashboardDataAccess(t *testing.T) { }) }) }) + + Convey("Given one dashboard folder with two dashboard and one dashboard in the root folder", func() { + folder := insertTestDashboard("1 test dash folder", 1, 0, true, "prod", "webapp") + dashInRoot := insertTestDashboard("test dash 67", 1, 0, false, "prod", "webapp") + insertTestDashboard("test dash 23", 1, folder.Id, false, "prod", "webapp") + insertTestDashboard("test dash 45", 1, folder.Id, false, "prod") + + currentUser := createUser("viewer", "Viewer", false) + + Convey("and no acls are set", func() { + Convey("should return all dashboards", func() { + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 2) + So(query.Result[0].Id, ShouldEqual, folder.Id) + So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) + }) + }) + + Convey("and acl is set for dashboard folder", func() { + var otherUser int64 = 999 + updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) + + Convey("should not return folder", func() { + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 1) + So(query.Result[0].Id, ShouldEqual, dashInRoot.Id) + }) + + Convey("when the user is given permission", func() { + updateTestDashboardWithAcl(folder.Id, currentUser.Id, m.PERMISSION_EDIT) + + Convey("should be able to access folder", func() { + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 2) + So(query.Result[0].Id, ShouldEqual, folder.Id) + So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) + }) + }) + + Convey("when the user is an admin", func() { + Convey("should be able to access folder", func() { + query := &search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{ + UserId: currentUser.Id, + OrgId: 1, + OrgRole: m.ROLE_ADMIN, + }, + OrgId: 1, + DashboardIds: []int64{folder.Id, dashInRoot.Id}, + } + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 2) + So(query.Result[0].Id, ShouldEqual, folder.Id) + So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) + }) + }) + }) + }) }) } + +func createUser(name string, role string, isAdmin bool) m.User { + setting.AutoAssignOrg = true + setting.AutoAssignOrgRole = role + + currentUserCmd := m.CreateUserCommand{Login: name, Email: name + "@test.com", Name: "a " + name, IsAdmin: isAdmin} + err := CreateUser(¤tUserCmd) + So(err, ShouldBeNil) + + q1 := m.GetUserOrgListQuery{UserId: currentUserCmd.Result.Id} + GetUserOrgList(&q1) + So(q1.Result[0].Role, ShouldEqual, role) + + return currentUserCmd.Result +} + +func updateTestDashboardWithAcl(dashId int64, userId int64, permissionType m.PermissionType) { + err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{ + OrgId: 1, + UserId: userId, + DashboardId: dashId, + PermissionType: permissionType, + }) + So(err, ShouldBeNil) +} diff --git a/pkg/services/sqlstore/guardian.go b/pkg/services/sqlstore/guardian.go index f3e5d45e707..5e907434d40 100644 --- a/pkg/services/sqlstore/guardian.go +++ b/pkg/services/sqlstore/guardian.go @@ -21,7 +21,7 @@ from dashboard as d left join dashboard_acl as dfa on d.parent_id = dfa.dashboard_id or d.id = dfa.dashboard_id left join user_group_member as ugm on ugm.user_group_id = dfa.user_group_id where ( - (d.has_acl = 1 and (dfa.user_id = ? or ugm.user_id = ? or df.created_by = ? or (d.is_folder = 1 and d.created_by = ?))) + (d.has_acl = 1 and (dfa.user_id = ? or ugm.user_id = ?)) or d.has_acl = 0) and d.org_id = ?` diff --git a/pkg/services/sqlstore/guardian_test.go b/pkg/services/sqlstore/guardian_test.go index dfdc51d675f..4c4d47a3e06 100644 --- a/pkg/services/sqlstore/guardian_test.go +++ b/pkg/services/sqlstore/guardian_test.go @@ -4,7 +4,6 @@ import ( "testing" m "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/setting" . "github.com/smartystreets/goconvey/convey" ) @@ -60,28 +59,3 @@ func TestGuardianDataAccess(t *testing.T) { }) }) } - -func createUser(name string, role string, isAdmin bool) m.User { - setting.AutoAssignOrg = true - setting.AutoAssignOrgRole = role - - currentUserCmd := m.CreateUserCommand{Login: name, Email: name + "@test.com", Name: "a " + name, IsAdmin: isAdmin} - err := CreateUser(¤tUserCmd) - So(err, ShouldBeNil) - - q1 := m.GetUserOrgListQuery{UserId: currentUserCmd.Result.Id} - GetUserOrgList(&q1) - So(q1.Result[0].Role, ShouldEqual, role) - - return currentUserCmd.Result -} - -func updateTestDashboardWithAcl(dashId int64, userId int64, permissionType m.PermissionType) { - err := AddOrUpdateDashboardPermission(&m.AddOrUpdateDashboardPermissionCommand{ - OrgId: 1, - UserId: userId, - DashboardId: dashId, - PermissionType: permissionType, - }) - So(err, ShouldBeNil) -} From d89c1260c6dd7fbb480ac139ccbdfd0435b04cc5 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Sat, 17 Jun 2017 03:00:13 +0200 Subject: [PATCH 3/5] WIP: refactor dash search and remove extra query --- pkg/services/sqlstore/dashboard.go | 38 +++++++++++------------------- 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 169edbf6214..73c1d7224e9 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -148,12 +148,14 @@ func GetDashboard(query *m.GetDashboardQuery) error { } type DashboardSearchProjection struct { - Id int64 - Title string - Slug string - Term string - IsFolder bool - ParentId int64 + Id int64 + Title string + Slug string + Term string + IsFolder bool + ParentId int64 + FolderSlug string + FolderTitle string } func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSearchProjection, error) { @@ -167,9 +169,10 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear dashboard_tag.term, dashboard.is_folder, dashboard.parent_id, - pd.title as folder_title + f.slug as folder_slug, + f.title as folder_title FROM dashboard - LEFT OUTER JOIN dashboard pd on pd.id = dashboard.parent_id + LEFT OUTER JOIN dashboard f on f.id = dashboard.parent_id LEFT OUTER JOIN dashboard_tag on dashboard_tag.dashboard_id = dashboard.id`) if query.IsStarred { @@ -268,26 +271,13 @@ func SearchDashboards(query *search.FindPersistedDashboardsQuery) error { // appends parent folders for any hits to the search result func appendDashboardFolders(res []DashboardSearchProjection) ([]DashboardSearchProjection, error) { - var dashboardFolderIds []int64 for _, item := range res { if item.ParentId > 0 { - dashboardFolderIds = append(dashboardFolderIds, item.ParentId) - } - } - - if len(dashboardFolderIds) > 0 { - folderQuery := &m.GetDashboardsQuery{DashboardIds: dashboardFolderIds} - err := GetDashboards(folderQuery) - if err != nil { - return nil, err - } - - for _, folder := range folderQuery.Result { res = append(res, DashboardSearchProjection{ - Id: folder.Id, + Id: item.ParentId, IsFolder: true, - Slug: folder.Slug, - Title: folder.Title, + Slug: item.FolderSlug, + Title: item.FolderTitle, }) } } From 5cf63e3726692544c36f668dbb39d4f869b0e2d7 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Sat, 17 Jun 2017 03:21:08 +0200 Subject: [PATCH 4/5] WIP: remove unused test file --- pkg/services/guardian/guardian_test.go | 70 -------------------------- 1 file changed, 70 deletions(-) delete mode 100644 pkg/services/guardian/guardian_test.go diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go deleted file mode 100644 index 4bec550f791..00000000000 --- a/pkg/services/guardian/guardian_test.go +++ /dev/null @@ -1,70 +0,0 @@ -package guardian - -import ( - "testing" - - "github.com/grafana/grafana/pkg/bus" - m "github.com/grafana/grafana/pkg/models" - . "github.com/smartystreets/goconvey/convey" -) - -func TestGuardian(t *testing.T) { - - Convey("Given a user with list of dashboards that they have access to", t, func() { - hitList := []int64{1, 2} - - var orgId int64 = 1 - var userId int64 = 1 - - Convey("And the user is a Grafana admin", func() { - bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { - query.Result = &m.SignedInUser{IsGrafanaAdmin: true} - return nil - }) - - filteredHitlist, err := FilterRestrictedDashboards(hitList, orgId, userId) - So(err, ShouldBeNil) - - Convey("should return all dashboards", func() { - So(len(filteredHitlist), ShouldEqual, 2) - So(filteredHitlist[0], ShouldEqual, 1) - So(filteredHitlist[1], ShouldEqual, 2) - }) - }) - - Convey("And the user is an org admin", func() { - bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { - query.Result = &m.SignedInUser{IsGrafanaAdmin: false, OrgRole: m.ROLE_ADMIN} - return nil - }) - - filteredHitlist, err := FilterRestrictedDashboards(hitList, orgId, userId) - So(err, ShouldBeNil) - - Convey("should return all dashboards", func() { - So(len(filteredHitlist), ShouldEqual, 2) - So(filteredHitlist[0], ShouldEqual, 1) - So(filteredHitlist[1], ShouldEqual, 2) - }) - }) - - Convey("And the user is an editor", func() { - bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { - query.Result = &m.SignedInUser{IsGrafanaAdmin: false, OrgRole: m.ROLE_EDITOR} - return nil - }) - bus.AddHandler("test2", func(query *m.GetAllowedDashboardsQuery) error { - query.Result = []int64{1} - return nil - }) - - filteredHitlist, err := FilterRestrictedDashboards(hitList, orgId, userId) - So(err, ShouldBeNil) - - Convey("should return dashboard that editor has access to", func() { - So(len(filteredHitlist), ShouldEqual, 1) - So(filteredHitlist[0], ShouldEqual, 1) - }) - }) - }) -} From d1e1c4be933c9c9abcfa4d95cdaa9accf62a0a56 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Sun, 18 Jun 2017 00:16:33 +0200 Subject: [PATCH 5/5] WIP: refactor user group modal --- public/app/features/org/all.js | 1 + .../features/org/create_user_group_modal.ts | 38 +++++++++++++++++++ .../org/partials/create_user_group.html | 27 +++++++------ public/app/features/org/user_groups_ctrl.ts | 20 ++-------- 4 files changed, 58 insertions(+), 28 deletions(-) create mode 100644 public/app/features/org/create_user_group_modal.ts diff --git a/public/app/features/org/all.js b/public/app/features/org/all.js index 0a81c375eb5..140f87fafce 100644 --- a/public/app/features/org/all.js +++ b/public/app/features/org/all.js @@ -10,4 +10,5 @@ define([ './prefs_control', './user_groups_ctrl', './user_group_details_ctrl', + './create_user_group_modal', ], function () {}); diff --git a/public/app/features/org/create_user_group_modal.ts b/public/app/features/org/create_user_group_modal.ts new file mode 100644 index 00000000000..5acc684041f --- /dev/null +++ b/public/app/features/org/create_user_group_modal.ts @@ -0,0 +1,38 @@ +/// + +import coreModule from 'app/core/core_module'; +import appEvents from 'app/core/app_events'; +import _ from 'lodash'; + +export class CreateUserGroupCtrl { + userGroupName = ''; + + /** @ngInject */ + constructor(private backendSrv, private $scope, $sce, private $location) { + } + + createUserGroup() { + this.backendSrv.post('/api/user-groups', {name: this.userGroupName}).then((result) => { + if (result.userGroupId) { + this.$location.path('/org/user-groups/edit/' + result.userGroupId); + } + this.dismiss(); + }); + } + + dismiss() { + appEvents.emit('hide-modal'); + } +} + +export function createUserGroupModal() { + return { + restrict: 'E', + templateUrl: 'public/app/features/org/partials/create_user_group.html', + controller: CreateUserGroupCtrl, + bindToController: true, + controllerAs: 'ctrl', + }; +} + +coreModule.directive('createUserGroupModal', createUserGroupModal); diff --git a/public/app/features/org/partials/create_user_group.html b/public/app/features/org/partials/create_user_group.html index bad08b206d9..1ef0ef2822e 100644 --- a/public/app/features/org/partials/create_user_group.html +++ b/public/app/features/org/partials/create_user_group.html @@ -1,23 +1,26 @@ -
+

- Create User Group + Create User Group

- + +
-
-
-
- -
-
- -
-
+ +
+
+
+ +
+
+ +
+
+
diff --git a/public/app/features/org/user_groups_ctrl.ts b/public/app/features/org/user_groups_ctrl.ts index 547d23d896f..da1f767acd5 100644 --- a/public/app/features/org/user_groups_ctrl.ts +++ b/public/app/features/org/user_groups_ctrl.ts @@ -1,6 +1,7 @@ /// import coreModule from 'app/core/core_module'; +import {appEvents} from 'app/core/core'; export class UserGroupsCtrl { userGroups: any; @@ -10,7 +11,6 @@ export class UserGroupsCtrl { totalPages: number; showPaging = false; query: any = ''; - userGroupName: any = ''; navModel: any; /** @ngInject */ @@ -40,14 +40,6 @@ export class UserGroupsCtrl { this.get(); } - createUserGroup() { - this.backendSrv.post('/api/user-groups', {name: this.userGroupName}).then((result) => { - if (result.userGroupId) { - this.$location.path('/org/user-groups/edit/' + result.userGroupId); - } - }); - } - deleteUserGroup(userGroup) { this.$scope.appEvent('confirm-modal', { title: 'Delete', @@ -66,13 +58,9 @@ export class UserGroupsCtrl { } openUserGroupModal() { - var modalScope = this.$scope.$new(); - modalScope.createUserGroup = this.createUserGroup.bind(this); - - this.$scope.appEvent('show-modal', { - src: 'public/app/features/org/partials/create_user_group.html', - modalClass: 'modal--narrow', - scope: modalScope + appEvents.emit('show-modal', { + templateHtml: '', + modalClass: 'modal--narrow' }); } }