Stars: Add dashboard_uid and org_id to table (#96408)

This commit is contained in:
Stephanie Hingtgen 2024-11-15 08:51:31 -07:00 committed by GitHub
parent c6a90ed3cd
commit b12c731d59
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 340 additions and 79 deletions

View File

@ -570,14 +570,14 @@ Content-Type: application/json
## Star a dashboard ## Star a dashboard
`POST /api/user/stars/dashboard/:dashboardId` `POST /api/user/stars/dashboard/uid/:uid`
Stars the given Dashboard for the actual user. Stars the given Dashboard for the actual user.
**Example Request**: **Example Request**:
```http ```http
POST /api/user/stars/dashboard/1 HTTP/1.1 POST /api/user/stars/dashboard/uid/BqokFhx7z HTTP/1.1
Accept: application/json Accept: application/json
Content-Type: application/json Content-Type: application/json
Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
@ -594,14 +594,14 @@ Content-Type: application/json
## Unstar a dashboard ## Unstar a dashboard
`DELETE /api/user/stars/dashboard/:dashboardId` `DELETE /api/user/stars/dashboard/uid/:uid`
Deletes the starring of the given Dashboard for the actual user. Deletes the starring of the given Dashboard for the actual user.
**Example Request**: **Example Request**:
```http ```http
DELETE /api/user/stars/dashboard/1 HTTP/1.1 DELETE /api/user/stars/dashboard/uid/BqokFhx7z HTTP/1.1
Accept: application/json Accept: application/json
Content-Type: application/json Content-Type: application/json
Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk

View File

@ -483,7 +483,7 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog
require.NoError(b, err) require.NoError(b, err)
starSvc := startest.NewStarServiceFake() starSvc := startest.NewStarServiceFake()
starSvc.ExpectedUserStars = &star.GetUserStarsResult{UserStars: make(map[int64]bool)} starSvc.ExpectedUserStars = &star.GetUserStarsResult{UserStars: make(map[string]bool)}
hs := &HTTPServer{ hs := &HTTPServer{
CacheService: localcache.New(5*time.Minute, 10*time.Minute), CacheService: localcache.New(5*time.Minute, 10*time.Minute),

View File

@ -322,11 +322,11 @@ func (s *ServiceImpl) buildStarredItemsNavLinks(c *contextmodel.ReqContext) ([]*
} }
if len(starredDashboardResult.UserStars) > 0 { if len(starredDashboardResult.UserStars) > 0 {
var ids []int64 var uids []string
for id := range starredDashboardResult.UserStars { for uid := range starredDashboardResult.UserStars {
ids = append(ids, id) uids = append(uids, uid)
} }
starredDashboards, err := s.dashboardService.GetDashboards(c.Req.Context(), &dashboards.GetDashboardsQuery{DashboardIDs: ids, OrgID: c.SignedInUser.GetOrgID()}) starredDashboards, err := s.dashboardService.GetDashboards(c.Req.Context(), &dashboards.GetDashboardsQuery{DashboardUIDs: uids, OrgID: c.SignedInUser.GetOrgID()})
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -220,7 +220,7 @@ func (ss *sqlStore) Delete(ctx context.Context, cmd *org.DeleteOrgCommand) error
} }
deletes := []string{ deletes := []string{
"DELETE FROM star WHERE EXISTS (SELECT 1 FROM dashboard WHERE org_id = ? AND star.dashboard_id = dashboard.id)", "DELETE FROM star WHERE EXISTS (SELECT 1 FROM dashboard WHERE org_id = ? AND star.dashboard_uid = dashboard.uid)",
"DELETE FROM dashboard_tag WHERE EXISTS (SELECT 1 FROM dashboard WHERE org_id = ? AND dashboard_tag.dashboard_id = dashboard.id)", "DELETE FROM dashboard_tag WHERE EXISTS (SELECT 1 FROM dashboard WHERE org_id = ? AND dashboard_tag.dashboard_id = dashboard.id)",
"DELETE FROM dashboard WHERE org_id = ?", "DELETE FROM dashboard WHERE org_id = ?",
"DELETE FROM api_key WHERE org_id = ?", "DELETE FROM api_key WHERE org_id = ?",

View File

@ -82,8 +82,8 @@ func (s *SearchService) SearchHandler(ctx context.Context, query *Query) (model.
// filter by starred dashboard IDs when starred dashboards are requested and no UID or ID filters are specified to improve query performance // filter by starred dashboard IDs when starred dashboards are requested and no UID or ID filters are specified to improve query performance
if query.IsStarred && len(query.DashboardIds) == 0 && len(query.DashboardUIDs) == 0 { if query.IsStarred && len(query.DashboardIds) == 0 && len(query.DashboardUIDs) == 0 {
for id := range staredDashIDs.UserStars { for uid := range staredDashIDs.UserStars {
query.DashboardIds = append(query.DashboardIds, id) query.DashboardUIDs = append(query.DashboardUIDs, uid)
} }
} }
@ -118,7 +118,7 @@ func (s *SearchService) SearchHandler(ctx context.Context, query *Query) (model.
// set starred dashboards // set starred dashboards
for _, dashboard := range hits { for _, dashboard := range hits {
if _, ok := staredDashIDs.UserStars[dashboard.ID]; ok { if _, ok := staredDashIDs.UserStars[dashboard.UID]; ok {
dashboard.IsStarred = true dashboard.IsStarred = true
} }
} }

View File

@ -23,14 +23,14 @@ func TestSearch_SortedResults(t *testing.T) {
us := usertest.NewUserServiceFake() us := usertest.NewUserServiceFake()
ds := dashboards.NewFakeDashboardService(t) ds := dashboards.NewFakeDashboardService(t)
ds.On("SearchDashboards", mock.Anything, mock.AnythingOfType("*dashboards.FindPersistedDashboardsQuery")).Return(model.HitList{ ds.On("SearchDashboards", mock.Anything, mock.AnythingOfType("*dashboards.FindPersistedDashboardsQuery")).Return(model.HitList{
&model.Hit{ID: 16, Title: "CCAA", Type: "dash-db", Tags: []string{"BB", "AA"}}, &model.Hit{UID: "test", Title: "CCAA", Type: "dash-db", Tags: []string{"BB", "AA"}},
&model.Hit{ID: 10, Title: "AABB", Type: "dash-db", Tags: []string{"CC", "AA"}}, &model.Hit{UID: "test2", Title: "AABB", Type: "dash-db", Tags: []string{"CC", "AA"}},
&model.Hit{ID: 15, Title: "BBAA", Type: "dash-db", Tags: []string{"EE", "AA", "BB"}}, &model.Hit{UID: "test3", Title: "BBAA", Type: "dash-db", Tags: []string{"EE", "AA", "BB"}},
&model.Hit{ID: 25, Title: "bbAAa", Type: "dash-db", Tags: []string{"EE", "AA", "BB"}}, &model.Hit{UID: "test4", Title: "bbAAa", Type: "dash-db", Tags: []string{"EE", "AA", "BB"}},
&model.Hit{ID: 17, Title: "FOLDER", Type: "dash-folder"}, &model.Hit{UID: "test5", Title: "FOLDER", Type: "dash-folder"},
}, nil) }, nil)
us.ExpectedSignedInUser = &user.SignedInUser{IsGrafanaAdmin: true} us.ExpectedSignedInUser = &user.SignedInUser{IsGrafanaAdmin: true}
ss.ExpectedUserStars = &star.GetUserStarsResult{UserStars: map[int64]bool{10: true, 12: true}} ss.ExpectedUserStars = &star.GetUserStarsResult{UserStars: map[string]bool{"test2": true, "test7": true}}
svc := &SearchService{ svc := &SearchService{
sqlstore: db, sqlstore: db,
starService: ss, starService: ss,
@ -66,12 +66,12 @@ func TestSearch_StarredResults(t *testing.T) {
us := usertest.NewUserServiceFake() us := usertest.NewUserServiceFake()
ds := dashboards.NewFakeDashboardService(t) ds := dashboards.NewFakeDashboardService(t)
ds.On("SearchDashboards", mock.Anything, mock.AnythingOfType("*dashboards.FindPersistedDashboardsQuery")).Return(model.HitList{ ds.On("SearchDashboards", mock.Anything, mock.AnythingOfType("*dashboards.FindPersistedDashboardsQuery")).Return(model.HitList{
&model.Hit{ID: 1, Title: "A", Type: "dash-db"}, &model.Hit{UID: "test", Title: "A", Type: "dash-db"},
&model.Hit{ID: 2, Title: "B", Type: "dash-db"}, &model.Hit{UID: "test2", Title: "B", Type: "dash-db"},
&model.Hit{ID: 3, Title: "C", Type: "dash-db"}, &model.Hit{UID: "test3", Title: "C", Type: "dash-db"},
}, nil) }, nil)
us.ExpectedSignedInUser = &user.SignedInUser{} us.ExpectedSignedInUser = &user.SignedInUser{}
ss.ExpectedUserStars = &star.GetUserStarsResult{UserStars: map[int64]bool{1: true, 3: true, 4: true}} ss.ExpectedUserStars = &star.GetUserStarsResult{UserStars: map[string]bool{"test": true, "test3": true, "test4": true}}
svc := &SearchService{ svc := &SearchService{
sqlstore: db, sqlstore: db,
starService: ss, starService: ss,

View File

@ -35,6 +35,7 @@ func (oss *OSSMigrations) AddMigration(mg *Migrator) {
addStarMigrations(mg) addStarMigrations(mg)
addOrgMigrations(mg) addOrgMigrations(mg)
addDashboardMigration(mg) // Do NOT add more migrations to this function. addDashboardMigration(mg) // Do NOT add more migrations to this function.
addDashboardUIDStarMigrations(mg)
addDataSourceMigration(mg) addDataSourceMigration(mg)
addApiKeyMigrations(mg) addApiKeyMigrations(mg)
addDashboardSnapshotMigrations(mg) addDashboardSnapshotMigrations(mg)
@ -141,20 +142,3 @@ func (oss *OSSMigrations) AddMigration(mg *Migrator) {
accesscontrol.AddReceiverCreateScopeMigration(mg) accesscontrol.AddReceiverCreateScopeMigration(mg)
} }
func addStarMigrations(mg *Migrator) {
starV1 := Table{
Name: "star",
Columns: []*Column{
{Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true},
{Name: "user_id", Type: DB_BigInt, Nullable: false},
{Name: "dashboard_id", Type: DB_BigInt, Nullable: false},
},
Indices: []*Index{
{Cols: []string{"user_id", "dashboard_id"}, Type: UniqueIndex},
},
}
mg.AddMigration("create star table", NewAddTableMigration(starV1))
mg.AddMigration("add unique index star.user_id_dashboard_id", NewAddIndexMigration(starV1, starV1.Indices[0]))
}

View File

@ -0,0 +1,91 @@
package migrations
import (
"fmt"
. "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"xorm.io/xorm"
)
// does not rely on dashboard table existing, can be run before dashboard migrations
func addStarMigrations(mg *Migrator) {
starV1 := Table{
Name: "star",
Columns: []*Column{
{Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true},
{Name: "user_id", Type: DB_BigInt, Nullable: false},
{Name: "dashboard_id", Type: DB_BigInt, Nullable: false},
},
Indices: []*Index{
{Cols: []string{"user_id", "dashboard_id"}, Type: UniqueIndex},
},
}
mg.AddMigration("create star table", NewAddTableMigration(starV1))
mg.AddMigration("add unique index star.user_id_dashboard_id", NewAddIndexMigration(starV1, starV1.Indices[0]))
mg.AddMigration("Add column dashboard_uid in star", NewAddColumnMigration(starV1, &Column{
Name: "dashboard_uid", Type: DB_NVarchar, Length: 40, Nullable: true,
}))
mg.AddMigration("Add column org_id in star", NewAddColumnMigration(starV1, &Column{
Name: "org_id", Type: DB_BigInt, Nullable: true, Default: "1",
}))
mg.AddMigration("Add column updated in star", NewAddColumnMigration(starV1, &Column{
Name: "updated", Type: DB_DateTime, Nullable: true,
}))
mg.AddMigration("add index in star table on dashboard_uid, org_id and user_id columns",
NewAddIndexMigration(starV1, &Index{
Cols: []string{"user_id", "dashboard_uid", "org_id"},
Type: UniqueIndex,
}))
}
// relies on the dashboard table existing & must be run after the dashboard migrations are run
func addDashboardUIDStarMigrations(mg *Migrator) {
mg.AddMigration("Add missing dashboard_uid and org_id to star", &FillDashbordUIDMigration{})
}
type FillDashbordUIDMigration struct {
MigrationBase
}
func (m *FillDashbordUIDMigration) SQL(dialect Dialect) string {
return "code migration"
}
func (m *FillDashbordUIDMigration) Exec(sess *xorm.Session, mg *Migrator) error {
return RunStarMigrations(sess, mg.Dialect.DriverName())
}
func RunStarMigrations(sess *xorm.Session, driverName string) error {
// sqlite
sql := `UPDATE star
SET
dashboard_uid = (SELECT uid FROM dashboard WHERE dashboard.id = star.dashboard_id),
org_id = (SELECT org_id FROM dashboard WHERE dashboard.id = star.dashboard_id),
updated = DATETIME('now')
WHERE
(dashboard_uid IS NULL OR org_id IS NULL)
AND EXISTS (SELECT 1 FROM dashboard WHERE dashboard.id = star.dashboard_id);`
if driverName == Postgres {
sql = `UPDATE star
SET dashboard_uid = dashboard.uid,
org_id = dashboard.org_id,
updated = NOW()
FROM dashboard
WHERE star.dashboard_id = dashboard.id
AND (star.dashboard_uid IS NULL OR star.org_id IS NULL);`
} else if driverName == MySQL {
sql = `UPDATE star
LEFT JOIN dashboard ON star.dashboard_id = dashboard.id
SET star.dashboard_uid = dashboard.uid,
star.org_id = dashboard.org_id,
star.updated = NOW()
WHERE star.dashboard_uid IS NULL OR star.org_id IS NULL;`
}
if _, err := sess.Exec(sql); err != nil {
return fmt.Errorf("failed to set dashboard_uid, org_id, and updated for star: %w", err)
}
return nil
}

View File

@ -4,9 +4,11 @@ import (
"context" "context"
"net/http" "net/http"
"strconv" "strconv"
"time"
"github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/infra/log"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/star" "github.com/grafana/grafana/pkg/services/star"
@ -16,15 +18,18 @@ import (
type API struct { type API struct {
starService star.Service starService star.Service
dashboardService dashboards.DashboardService dashboardService dashboards.DashboardService
logger log.Logger
} }
func ProvideApi( func ProvideApi(
starService star.Service, starService star.Service,
dashboardService dashboards.DashboardService, dashboardService dashboards.DashboardService,
) *API { ) *API {
starLogger := log.New("stars.api")
api := &API{ api := &API{
starService: starService, starService: starService,
dashboardService: dashboardService, dashboardService: dashboardService,
logger: starLogger,
} }
return api return api
} }
@ -63,11 +68,11 @@ func (api *API) GetStars(c *contextmodel.ReqContext) response.Response {
uids := []string{} uids := []string{}
if len(iuserstars.UserStars) > 0 { if len(iuserstars.UserStars) > 0 {
var ids []int64 var uids []string
for id := range iuserstars.UserStars { for uid := range iuserstars.UserStars {
ids = append(ids, id) uids = append(uids, uid)
} }
starredDashboards, err := api.dashboardService.GetDashboards(c.Req.Context(), &dashboards.GetDashboardsQuery{DashboardIDs: ids, OrgID: c.SignedInUser.GetOrgID()}) starredDashboards, err := api.dashboardService.GetDashboards(c.Req.Context(), &dashboards.GetDashboardsQuery{DashboardUIDs: uids, OrgID: c.SignedInUser.GetOrgID()})
if err != nil { if err != nil {
return response.ErrOrFallback(http.StatusInternalServerError, "Failed to fetch dashboards", err) return response.ErrOrFallback(http.StatusInternalServerError, "Failed to fetch dashboards", err)
} }
@ -106,7 +111,10 @@ func (api *API) StarDashboard(c *contextmodel.ReqContext) response.Response {
return response.Error(http.StatusBadRequest, "Invalid dashboard ID", nil) return response.Error(http.StatusBadRequest, "Invalid dashboard ID", nil)
} }
cmd := star.StarDashboardCommand{UserID: userID, DashboardID: id} api.logger.Warn("POST /user/stars/dashboard/{dashboard_id} is deprecated, please use POST /user/stars/dashboard/uid/{dashboard_uid} instead")
cmd := star.StarDashboardCommand{UserID: userID, DashboardID: id, Updated: time.Now()}
// nolint:staticcheck
if cmd.DashboardID <= 0 { if cmd.DashboardID <= 0 {
return response.Error(http.StatusBadRequest, "Missing dashboard id", nil) return response.Error(http.StatusBadRequest, "Missing dashboard id", nil)
} }
@ -146,7 +154,7 @@ func (api *API) StarDashboardByUID(c *contextmodel.ReqContext) response.Response
return rsp return rsp
} }
cmd := star.StarDashboardCommand{UserID: userID, DashboardID: dash.ID} cmd := star.StarDashboardCommand{UserID: userID, DashboardID: dash.ID, DashboardUID: uid, OrgID: c.SignedInUser.GetOrgID(), Updated: time.Now()}
if err := api.starService.Add(c.Req.Context(), &cmd); err != nil { if err := api.starService.Add(c.Req.Context(), &cmd); err != nil {
return response.Error(http.StatusInternalServerError, "Failed to star dashboard", err) return response.Error(http.StatusInternalServerError, "Failed to star dashboard", err)
@ -182,7 +190,10 @@ func (api *API) UnstarDashboard(c *contextmodel.ReqContext) response.Response {
return response.Error(http.StatusBadRequest, "Only users and service accounts can star dashboards", nil) return response.Error(http.StatusBadRequest, "Only users and service accounts can star dashboards", nil)
} }
api.logger.Warn("DELETE /user/stars/dashboard/{dashboard_id} is deprecated, please use DELETE /user/stars/dashboard/uid/{dashboard_uid} instead")
cmd := star.UnstarDashboardCommand{UserID: userID, DashboardID: id} cmd := star.UnstarDashboardCommand{UserID: userID, DashboardID: id}
// nolint:staticcheck
if cmd.DashboardID <= 0 { if cmd.DashboardID <= 0 {
return response.Error(http.StatusBadRequest, "Missing dashboard id", nil) return response.Error(http.StatusBadRequest, "Missing dashboard id", nil)
} }
@ -217,12 +228,7 @@ func (api *API) UnstarDashboardByUID(c *contextmodel.ReqContext) response.Respon
return response.Error(http.StatusBadRequest, "Only users and service accounts can star dashboards", nil) return response.Error(http.StatusBadRequest, "Only users and service accounts can star dashboards", nil)
} }
dash, rsp := api.getDashboardHelper(c.Req.Context(), c.SignedInUser.GetOrgID(), 0, uid) cmd := star.UnstarDashboardCommand{UserID: userID, DashboardUID: uid, OrgID: c.SignedInUser.GetOrgID()}
if rsp != nil {
return rsp
}
cmd := star.UnstarDashboardCommand{UserID: userID, DashboardID: dash.ID}
if err := api.starService.Delete(c.Req.Context(), &cmd); err != nil { if err := api.starService.Delete(c.Req.Context(), &cmd); err != nil {
return response.Error(http.StatusInternalServerError, "Failed to unstar dashboard", err) return response.Error(http.StatusInternalServerError, "Failed to unstar dashboard", err)

View File

@ -5,6 +5,7 @@ import (
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards"
@ -13,7 +14,7 @@ import (
"github.com/grafana/grafana/pkg/web" "github.com/grafana/grafana/pkg/web"
) )
func TestStarDashboard(t *testing.T) { func TestStarDashboardID(t *testing.T) {
api := ProvideApi(startest.NewStarServiceFake(), dashboards.NewFakeDashboardService(t)) api := ProvideApi(startest.NewStarServiceFake(), dashboards.NewFakeDashboardService(t))
testCases := []struct { testCases := []struct {
@ -82,3 +83,75 @@ func TestStarDashboard(t *testing.T) {
}) })
} }
} }
func TestStarDashboardUID(t *testing.T) {
svc := dashboards.NewFakeDashboardService(t)
svc.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{UID: "test", OrgID: 1}, nil)
api := ProvideApi(startest.NewStarServiceFake(), svc)
testCases := []struct {
name string
signedInUser *user.SignedInUser
expectedStatus int
params map[string]string
}{
{
name: "Star dashboard with user",
params: map[string]string{
":uid": "test",
},
signedInUser: &user.SignedInUser{
UserID: 1,
OrgID: 1,
IsAnonymous: false,
},
expectedStatus: 200,
},
{
name: "Star dashboard with anonymous user",
params: map[string]string{
":uid": "test",
},
signedInUser: &user.SignedInUser{
UserID: 0,
OrgID: 1,
IsAnonymous: true,
},
expectedStatus: 400,
},
{
name: "Star dashboard with API Key",
params: map[string]string{
":uid": "test",
},
signedInUser: &user.SignedInUser{
UserID: 0,
OrgID: 1,
ApiKeyID: 3,
IsAnonymous: false,
},
expectedStatus: 400,
},
{
name: "Star dashboard with Service Account",
params: map[string]string{
":uid": "test",
},
signedInUser: &user.SignedInUser{
UserID: 1,
OrgID: 3,
IsServiceAccount: true,
},
expectedStatus: 200,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
req := web.SetURLParams(&http.Request{}, tc.params)
c := &contextmodel.ReqContext{SignedInUser: tc.signedInUser, Context: &web.Context{Req: req}}
resp := api.StarDashboardByUID(c)
assert.Equal(t, tc.expectedStatus, resp.Status())
})
}
}

View File

@ -1,37 +1,52 @@
package star package star
import "errors" import (
"errors"
"time"
)
var ErrCommandValidationFailed = errors.New("command missing required fields") var ErrCommandValidationFailed = errors.New("command missing required fields")
type Star struct { type Star struct {
ID int64 `xorm:"pk autoincr 'id'" db:"id"` ID int64 `xorm:"pk autoincr 'id'" db:"id"`
UserID int64 `xorm:"user_id" db:"user_id"` UserID int64 `xorm:"user_id" db:"user_id"`
DashboardID int64 `xorm:"dashboard_id" db:"dashboard_id"` // Deprecated: use DashboardUID
DashboardID int64 `xorm:"dashboard_id" db:"dashboard_id"`
DashboardUID string `xorm:"dashboard_uid" db:"dashboard_uid"`
OrgID int64 `xorm:"org_id" db:"org_id"`
Updated time.Time `xorm:"updated" db:"updated"`
} }
// ---------------------- // ----------------------
// COMMANDS // COMMANDS
type StarDashboardCommand struct { type StarDashboardCommand struct {
UserID int64 `xorm:"user_id"` UserID int64 `xorm:"user_id"`
DashboardID int64 `xorm:"dashboard_id"` // Deprecated: use DashboardUID
DashboardID int64 `xorm:"dashboard_id"`
DashboardUID string `xorm:"dashboard_uid"`
OrgID int64 `xorm:"org_id"`
Updated time.Time `xorm:"updated"`
} }
func (cmd *StarDashboardCommand) Validate() error { func (cmd *StarDashboardCommand) Validate() error {
if cmd.DashboardID == 0 || cmd.UserID == 0 { // nolint:staticcheck
if (cmd.DashboardID == 0 && cmd.DashboardUID == "" && cmd.OrgID == 0) || cmd.UserID == 0 {
return ErrCommandValidationFailed return ErrCommandValidationFailed
} }
return nil return nil
} }
type UnstarDashboardCommand struct { type UnstarDashboardCommand struct {
UserID int64 `xorm:"user_id"` UserID int64 `xorm:"user_id"`
DashboardID int64 `xorm:"dashboard_id"` DashboardID int64 `xorm:"dashboard_id"`
DashboardUID string `xorm:"dashboard_uid"`
OrgID int64 `xorm:"org_id"`
} }
func (cmd *UnstarDashboardCommand) Validate() error { func (cmd *UnstarDashboardCommand) Validate() error {
if cmd.DashboardID == 0 || cmd.UserID == 0 { // nolint:staticcheck
if (cmd.DashboardID == 0 && cmd.DashboardUID == "" && cmd.OrgID == 0) || cmd.UserID == 0 {
return ErrCommandValidationFailed return ErrCommandValidationFailed
} }
return nil return nil
@ -45,10 +60,13 @@ type GetUserStarsQuery struct {
} }
type IsStarredByUserQuery struct { type IsStarredByUserQuery struct {
UserID int64 `xorm:"user_id"` UserID int64 `xorm:"user_id"`
DashboardID int64 `xorm:"dashboard_id"` DashboardID int64 `xorm:"dashboard_id"`
DashboardUID string `xorm:"dashboard_uid"`
OrgID int64 `xorm:"org_id"`
Updated time.Time `xorm:"updated"`
} }
type GetUserStarsResult struct { type GetUserStarsResult struct {
UserStars map[int64]bool UserStars map[string]bool
} }

View File

@ -4,18 +4,30 @@ import (
"context" "context"
"github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/sqlstore/migrations"
"github.com/grafana/grafana/pkg/services/star" "github.com/grafana/grafana/pkg/services/star"
) )
type Service struct { type Service struct {
store store store store
logger log.Logger
} }
func ProvideService(db db.DB) star.Service { func ProvideService(db db.DB) star.Service {
starLogger := log.New("stars")
// fill out dashboard_uid, org_id and updated columns for stars
// need to run this at startup in case any downgrade happened after
// the initial migration
err := migrations.RunStarMigrations(db.GetEngine().NewSession(), db.GetDialect().DriverName())
if err != nil {
starLogger.Error("Failed to run star migrations", "err", err)
}
return &Service{ return &Service{
store: &sqlStore{ store: &sqlStore{
db: db, db: db,
}, },
logger: starLogger,
} }
} }

View File

@ -3,6 +3,7 @@ package starimpl
import ( import (
"context" "context"
"testing" "testing"
"time"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -24,7 +25,7 @@ func testIntegrationUserStarsDataAccess(t *testing.T, fn getStore) {
ss := db.InitTestDB(t) ss := db.InitTestDB(t)
starStore := fn(ss) starStore := fn(ss)
t.Run("Given saved star", func(t *testing.T) { t.Run("Given saved star by dashboard id", func(t *testing.T) {
cmd := star.StarDashboardCommand{ cmd := star.StarDashboardCommand{
DashboardID: 10, DashboardID: 10,
UserID: 12, UserID: 12,
@ -64,22 +65,72 @@ func testIntegrationUserStarsDataAccess(t *testing.T, fn getStore) {
}) })
}) })
t.Run("Given saved star by dashboard UID", func(t *testing.T) {
cmd := star.StarDashboardCommand{
DashboardUID: "test",
OrgID: 1,
UserID: 12,
}
err := starStore.Insert(context.Background(), &cmd)
require.NoError(t, err)
t.Run("Get should return true when starred", func(t *testing.T) {
query := star.IsStarredByUserQuery{UserID: 12, DashboardUID: "test", OrgID: 1}
isStarred, err := starStore.Get(context.Background(), &query)
require.NoError(t, err)
require.True(t, isStarred)
})
t.Run("Get should return false when not starred", func(t *testing.T) {
query := star.IsStarredByUserQuery{UserID: 12, DashboardUID: "testing", OrgID: 1}
isStarred, err := starStore.Get(context.Background(), &query)
require.NoError(t, err)
require.False(t, isStarred)
})
t.Run("List should return a list of size 1", func(t *testing.T) {
query := star.GetUserStarsQuery{UserID: 12}
result, err := starStore.List(context.Background(), &query)
require.NoError(t, err)
require.Equal(t, 1, len(result.UserStars))
})
t.Run("Delete should remove the star", func(t *testing.T) {
deleteQuery := star.UnstarDashboardCommand{DashboardUID: "test", OrgID: 1, UserID: 12}
err := starStore.Delete(context.Background(), &deleteQuery)
require.NoError(t, err)
getQuery := star.IsStarredByUserQuery{UserID: 12, DashboardUID: "test", OrgID: 1}
isStarred, err := starStore.Get(context.Background(), &getQuery)
require.NoError(t, err)
require.False(t, isStarred)
})
})
t.Run("DeleteByUser should remove the star for user", func(t *testing.T) { t.Run("DeleteByUser should remove the star for user", func(t *testing.T) {
star1 := star.StarDashboardCommand{ star1 := star.StarDashboardCommand{
DashboardID: 10, DashboardUID: "test",
UserID: 12, OrgID: 1,
Updated: time.Now(),
DashboardID: 10,
UserID: 12,
} }
err := starStore.Insert(context.Background(), &star1) err := starStore.Insert(context.Background(), &star1)
require.NoError(t, err) require.NoError(t, err)
star2 := star.StarDashboardCommand{ star2 := star.StarDashboardCommand{
DashboardID: 11, DashboardUID: "test2",
UserID: 12, OrgID: 1,
Updated: time.Now(),
DashboardID: 11,
UserID: 12,
} }
err = starStore.Insert(context.Background(), &star2) err = starStore.Insert(context.Background(), &star2)
require.NoError(t, err) require.NoError(t, err)
star3 := star.StarDashboardCommand{ star3 := star.StarDashboardCommand{
DashboardID: 11, DashboardUID: "test2",
UserID: 11, OrgID: 1,
Updated: time.Now(),
DashboardID: 11,
UserID: 11,
} }
err = starStore.Insert(context.Background(), &star3) err = starStore.Insert(context.Background(), &star3)
require.NoError(t, err) require.NoError(t, err)

View File

@ -14,9 +14,22 @@ type sqlStore struct {
func (s *sqlStore) Get(ctx context.Context, query *star.IsStarredByUserQuery) (bool, error) { func (s *sqlStore) Get(ctx context.Context, query *star.IsStarredByUserQuery) (bool, error) {
var isStarred bool var isStarred bool
err := s.db.WithDbSession(ctx, func(sess *db.Session) error { err := s.db.WithDbSession(ctx, func(sess *db.Session) error {
if query.DashboardUID != "" && query.OrgID != 0 {
rawSQL := "SELECT 1 from star where user_id=? and dashboard_uid=? and org_id=?"
results, err := sess.Query(rawSQL, query.UserID, query.DashboardUID, query.OrgID)
if err != nil {
return err
}
isStarred = len(results) != 0
return nil
}
// TODO: Remove this block after all dashboards have a UID
// && the deprecated endpoints have been removed
rawSQL := "SELECT 1 from star where user_id=? and dashboard_id=?" rawSQL := "SELECT 1 from star where user_id=? and dashboard_id=?"
results, err := sess.Query(rawSQL, query.UserID, query.DashboardID) results, err := sess.Query(rawSQL, query.UserID, query.DashboardID)
if err != nil { if err != nil {
return err return err
} }
@ -30,8 +43,12 @@ func (s *sqlStore) Get(ctx context.Context, query *star.IsStarredByUserQuery) (b
func (s *sqlStore) Insert(ctx context.Context, cmd *star.StarDashboardCommand) error { func (s *sqlStore) Insert(ctx context.Context, cmd *star.StarDashboardCommand) error {
return s.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { return s.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
entity := star.Star{ entity := star.Star{
UserID: cmd.UserID, UserID: cmd.UserID,
DashboardID: cmd.DashboardID, // nolint:staticcheck
DashboardID: cmd.DashboardID,
DashboardUID: cmd.DashboardUID,
OrgID: cmd.OrgID,
Updated: cmd.Updated,
} }
_, err := sess.Insert(&entity) _, err := sess.Insert(&entity)
@ -45,7 +62,16 @@ func (s *sqlStore) Insert(ctx context.Context, cmd *star.StarDashboardCommand) e
func (s *sqlStore) Delete(ctx context.Context, cmd *star.UnstarDashboardCommand) error { func (s *sqlStore) Delete(ctx context.Context, cmd *star.UnstarDashboardCommand) error {
return s.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { return s.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
if cmd.DashboardUID != "" && cmd.OrgID != 0 {
var rawSQL = "DELETE FROM star WHERE user_id=? and dashboard_uid=? and org_id=?"
_, err := sess.Exec(rawSQL, cmd.UserID, cmd.DashboardUID, cmd.OrgID)
return err
}
// TODO: Remove this block after all dashboards have a UID
// && the deprecated endpoints have been removed
var rawSQL = "DELETE FROM star WHERE user_id=? and dashboard_id=?" var rawSQL = "DELETE FROM star WHERE user_id=? and dashboard_id=?"
// nolint:staticcheck
_, err := sess.Exec(rawSQL, cmd.UserID, cmd.DashboardID) _, err := sess.Exec(rawSQL, cmd.UserID, cmd.DashboardID)
return err return err
}) })
@ -60,12 +86,12 @@ func (s *sqlStore) DeleteByUser(ctx context.Context, userID int64) error {
} }
func (s *sqlStore) List(ctx context.Context, query *star.GetUserStarsQuery) (*star.GetUserStarsResult, error) { func (s *sqlStore) List(ctx context.Context, query *star.GetUserStarsQuery) (*star.GetUserStarsResult, error) {
userStars := make(map[int64]bool) userStars := make(map[string]bool)
err := s.db.WithDbSession(ctx, func(dbSession *db.Session) error { err := s.db.WithDbSession(ctx, func(dbSession *db.Session) error {
var stars = make([]star.Star, 0) var stars = make([]star.Star, 0)
err := dbSession.Where("user_id=?", query.UserID).Find(&stars) err := dbSession.Where("user_id=?", query.UserID).Find(&stars)
for _, star := range stars { for _, star := range stars {
userStars[star.DashboardID] = true userStars[star.DashboardUID] = true
} }
return err return err
}) })