diff --git a/pkg/api/api.go b/pkg/api/api.go index 1561c28ea21..5248740b5ae 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -277,13 +277,6 @@ func (hs *HTTPServer) registerRoutes() { userRoute.Get("/teams", routing.Wrap(hs.GetSignedInUserTeamList)) userRoute.Get("/stars", routing.Wrap(hs.starApi.GetStars)) - // Deprecated: use /stars/dashboard/uid/:uid API instead. - // nolint:staticcheck - userRoute.Post("/stars/dashboard/:id", routing.Wrap(hs.starApi.StarDashboard)) - // Deprecated: use /stars/dashboard/uid/:uid API instead. - // nolint:staticcheck - userRoute.Delete("/stars/dashboard/:id", routing.Wrap(hs.starApi.UnstarDashboard)) - userRoute.Post("/stars/dashboard/uid/:uid", routing.Wrap(hs.starApi.StarDashboardByUID)) userRoute.Delete("/stars/dashboard/uid/:uid", routing.Wrap(hs.starApi.UnstarDashboardByUID)) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index c38c72b9333..5b02aa33dd6 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -44,7 +44,7 @@ const ( anonString = "Anonymous" ) -func (hs *HTTPServer) isDashboardStarredByUser(c *contextmodel.ReqContext, dashID int64) (bool, error) { +func (hs *HTTPServer) isDashboardStarredByUser(c *contextmodel.ReqContext, dashUID string) (bool, error) { ctx, span := tracer.Start(c.Req.Context(), "api.isDashboardStarredByUser") defer span.End() c.Req = c.Req.WithContext(ctx) @@ -62,7 +62,7 @@ func (hs *HTTPServer) isDashboardStarredByUser(c *contextmodel.ReqContext, dashI return false, err } - query := star.IsStarredByUserQuery{UserID: userID, DashboardID: dashID} + query := star.IsStarredByUserQuery{UserID: userID, DashboardUID: dashUID} return hs.starService.IsStarredByUser(c.Req.Context(), &query) } @@ -158,7 +158,7 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response adminEvaluator := accesscontrol.EvalPermission(dashboards.ActionDashboardsPermissionsWrite, dashScope) canAdmin, _ := hs.AccessControl.Evaluate(ctx, c.SignedInUser, adminEvaluator) - isStarred, err := hs.isDashboardStarredByUser(c, dash.ID) + isStarred, err := hs.isDashboardStarredByUser(c, dash.UID) if err != nil { return response.Error(http.StatusInternalServerError, "Error while checking if dashboard was starred by user", err) } diff --git a/pkg/services/sqlstore/migrations/star_mig.go b/pkg/services/sqlstore/migrations/star_mig.go index 4521acd7f2b..59ea8ed09cb 100644 --- a/pkg/services/sqlstore/migrations/star_mig.go +++ b/pkg/services/sqlstore/migrations/star_mig.go @@ -37,6 +37,9 @@ func addStarMigrations(mg *Migrator) { Cols: []string{"user_id", "dashboard_uid", "org_id"}, Type: UniqueIndex, })) + + // NOTE: in Grafana 12.2 the dashboard_id is no longer used + // However, we will keep the column + index so that rollback is still possible } // relies on the dashboard table existing & must be run after the dashboard migrations are run diff --git a/pkg/services/star/api/api.go b/pkg/services/star/api/api.go index b77abf72f51..42110ba2c09 100644 --- a/pkg/services/star/api/api.go +++ b/pkg/services/star/api/api.go @@ -3,7 +3,6 @@ package api import ( "context" "net/http" - "strconv" "time" "github.com/grafana/grafana/pkg/api/response" @@ -69,46 +68,6 @@ func (api *API) GetStars(c *contextmodel.ReqContext) response.Response { return response.JSON(http.StatusOK, uids) } -// swagger:route POST /user/stars/dashboard/{dashboard_id} signed_in_user starDashboard -// -// Star a dashboard. -// -// Stars the given Dashboard for the actual user. -// -// Deprecated: true -// -// Responses: -// 200: okResponse -// 400: badRequestError -// 401: unauthorisedError -// 403: forbiddenError -// 500: internalServerError -func (api *API) StarDashboard(c *contextmodel.ReqContext) response.Response { - userID, err := identity.UserIdentifier(c.GetID()) - if err != nil { - return response.Error(http.StatusBadRequest, "Only users and service accounts can star dashboards", nil) - } - - id, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64) - if err != nil { - return response.Error(http.StatusBadRequest, "Invalid dashboard ID", nil) - } - - 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 { - return response.Error(http.StatusBadRequest, "Missing dashboard id", nil) - } - - if err := api.starService.Add(c.Req.Context(), &cmd); err != nil { - return response.Error(http.StatusInternalServerError, "Failed to star dashboard", err) - } - - return response.Success("Dashboard starred!") -} - // swagger:route POST /user/stars/dashboard/uid/{dashboard_uid} signed_in_user starDashboardByUID // // Star a dashboard. @@ -146,48 +105,6 @@ func (api *API) StarDashboardByUID(c *contextmodel.ReqContext) response.Response return response.Success("Dashboard starred!") } -// swagger:route DELETE /user/stars/dashboard/{dashboard_id} signed_in_user unstarDashboard -// -// Unstar a dashboard. -// -// Deletes the starring of the given Dashboard for the actual user. -// -// Deprecated: true -// -// Please refer to the [new](#/signed_in_user/unstarDashboardByUID) API instead -// -// Responses: -// 200: okResponse -// 400: badRequestError -// 401: unauthorisedError -// 403: forbiddenError -// 500: internalServerError -func (api *API) UnstarDashboard(c *contextmodel.ReqContext) response.Response { - id, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64) - if err != nil { - return response.Error(http.StatusBadRequest, "Invalid dashboard ID", nil) - } - - userID, err := identity.UserIdentifier(c.GetID()) - if err != 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} - // nolint:staticcheck - if cmd.DashboardID <= 0 { - return response.Error(http.StatusBadRequest, "Missing dashboard id", nil) - } - - if err := api.starService.Delete(c.Req.Context(), &cmd); err != nil { - return response.Error(http.StatusInternalServerError, "Failed to unstar dashboard", err) - } - - return response.Success("Dashboard unstarred") -} - // swagger:route DELETE /user/stars/dashboard/uid/{dashboard_uid} signed_in_user unstarDashboardByUID // // Unstar a dashboard. diff --git a/pkg/services/star/api/api_test.go b/pkg/services/star/api/api_test.go index 40638d1f10e..1469ef46c9a 100644 --- a/pkg/services/star/api/api_test.go +++ b/pkg/services/star/api/api_test.go @@ -14,76 +14,6 @@ import ( "github.com/grafana/grafana/pkg/web" ) -func TestStarDashboardID(t *testing.T) { - api := ProvideApi(startest.NewStarServiceFake(), dashboards.NewFakeDashboardService(t)) - - testCases := []struct { - name string - signedInUser *user.SignedInUser - expectedStatus int - params map[string]string - }{ - { - name: "Star dashboard with user", - params: map[string]string{ - ":id": "1", - }, - signedInUser: &user.SignedInUser{ - UserID: 1, - OrgID: 1, - IsAnonymous: false, - }, - expectedStatus: 200, - }, - { - name: "Star dashboard with anonymous user", - params: map[string]string{ - ":id": "1", - }, - signedInUser: &user.SignedInUser{ - UserID: 0, - OrgID: 1, - IsAnonymous: true, - }, - expectedStatus: 400, - }, - { - name: "Star dashboard with API Key", - params: map[string]string{ - ":id": "1", - }, - signedInUser: &user.SignedInUser{ - UserID: 0, - OrgID: 1, - ApiKeyID: 3, - IsAnonymous: false, - }, - expectedStatus: 400, - }, - { - name: "Star dashboard with Service Account", - params: map[string]string{ - ":id": "1", - }, - 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.StarDashboard(c) - assert.Equal(t, tc.expectedStatus, resp.Status()) - }) - } -} - func TestStarDashboardUID(t *testing.T) { svc := dashboards.NewFakeDashboardService(t) svc.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{UID: "test", OrgID: 1}, nil) diff --git a/pkg/services/star/model.go b/pkg/services/star/model.go index 2f4694c5c8c..c52196928aa 100644 --- a/pkg/services/star/model.go +++ b/pkg/services/star/model.go @@ -10,7 +10,7 @@ var ErrCommandValidationFailed = errors.New("command missing required fields") type Star struct { ID int64 `xorm:"pk autoincr 'id'" db:"id"` UserID int64 `xorm:"user_id" db:"user_id"` - // Deprecated: use DashboardUID + // Deprecated: use DashboardUID (since 12.2 this value may not match the dashboards table) DashboardID int64 `xorm:"dashboard_id" db:"dashboard_id"` DashboardUID string `xorm:"dashboard_uid" db:"dashboard_uid"` OrgID int64 `xorm:"org_id" db:"org_id"` @@ -30,8 +30,7 @@ type StarDashboardCommand struct { } func (cmd *StarDashboardCommand) Validate() error { - // nolint:staticcheck - if (cmd.DashboardID == 0 && cmd.DashboardUID == "" && cmd.OrgID == 0) || cmd.UserID == 0 { + if (cmd.DashboardUID == "" && cmd.OrgID == 0) || cmd.UserID == 0 { return ErrCommandValidationFailed } return nil @@ -39,14 +38,13 @@ func (cmd *StarDashboardCommand) Validate() error { type UnstarDashboardCommand struct { UserID int64 `xorm:"user_id"` - DashboardID int64 `xorm:"dashboard_id"` DashboardUID string `xorm:"dashboard_uid"` OrgID int64 `xorm:"org_id"` } func (cmd *UnstarDashboardCommand) Validate() error { // nolint:staticcheck - if (cmd.DashboardID == 0 && cmd.DashboardUID == "" && cmd.OrgID == 0) || cmd.UserID == 0 { + if (cmd.DashboardUID == "" && cmd.OrgID == 0) || cmd.UserID == 0 { return ErrCommandValidationFailed } return nil @@ -61,7 +59,6 @@ type GetUserStarsQuery struct { type IsStarredByUserQuery struct { UserID int64 `xorm:"user_id"` - DashboardID int64 `xorm:"dashboard_id"` DashboardUID string `xorm:"dashboard_uid"` OrgID int64 `xorm:"org_id"` Updated time.Time `xorm:"updated"` diff --git a/pkg/services/star/starimpl/store_test.go b/pkg/services/star/starimpl/store_test.go index ad65ac82a5e..14d88249d5b 100644 --- a/pkg/services/star/starimpl/store_test.go +++ b/pkg/services/star/starimpl/store_test.go @@ -27,46 +27,6 @@ func testIntegrationUserStarsDataAccess(t *testing.T, fn getStore) { ss := db.InitTestDB(t) starStore := fn(ss) - t.Run("Given saved star by dashboard id", func(t *testing.T) { - cmd := star.StarDashboardCommand{ - DashboardID: 10, - 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, DashboardID: 10} - 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, DashboardID: 12} - 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{DashboardID: 10, UserID: 12} - err := starStore.Delete(context.Background(), &deleteQuery) - require.NoError(t, err) - getQuery := star.IsStarredByUserQuery{UserID: 12, DashboardID: 10} - isStarred, err := starStore.Get(context.Background(), &getQuery) - require.NoError(t, err) - require.False(t, isStarred) - }) - }) - t.Run("Given saved star by dashboard UID", func(t *testing.T) { cmd := star.StarDashboardCommand{ DashboardUID: "test", @@ -113,7 +73,6 @@ func testIntegrationUserStarsDataAccess(t *testing.T, fn getStore) { DashboardUID: "test", OrgID: 1, Updated: time.Now(), - DashboardID: 10, UserID: 12, } err := starStore.Insert(context.Background(), &star1) @@ -122,7 +81,6 @@ func testIntegrationUserStarsDataAccess(t *testing.T, fn getStore) { DashboardUID: "test2", OrgID: 1, Updated: time.Now(), - DashboardID: 11, UserID: 12, } err = starStore.Insert(context.Background(), &star2) @@ -131,7 +89,6 @@ func testIntegrationUserStarsDataAccess(t *testing.T, fn getStore) { DashboardUID: "test2", OrgID: 1, Updated: time.Now(), - DashboardID: 11, UserID: 11, } err = starStore.Insert(context.Background(), &star3) diff --git a/pkg/services/star/starimpl/xorm_store.go b/pkg/services/star/starimpl/xorm_store.go index a8d4a367189..e833ba6d6d1 100644 --- a/pkg/services/star/starimpl/xorm_store.go +++ b/pkg/services/star/starimpl/xorm_store.go @@ -2,6 +2,8 @@ package starimpl import ( "context" + "math/rand" + "time" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/star" @@ -14,22 +16,9 @@ type sqlStore struct { func (s *sqlStore) Get(ctx context.Context, query *star.IsStarredByUserQuery) (bool, error) { var isStarred bool 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) + 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=?" - results, err := sess.Query(rawSQL, query.UserID, query.DashboardID) if err != nil { return err } @@ -42,6 +31,11 @@ func (s *sqlStore) Get(ctx context.Context, query *star.IsStarredByUserQuery) (b func (s *sqlStore) Insert(ctx context.Context, cmd *star.StarDashboardCommand) error { return s.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + // nolint:staticcheck + if cmd.DashboardID == 0 { + cmd.DashboardID = time.Now().UnixMicro() + rand.Int63n(5000) // random unique value + } + entity := star.Star{ UserID: cmd.UserID, // nolint:staticcheck @@ -62,17 +56,8 @@ func (s *sqlStore) Insert(ctx context.Context, cmd *star.StarDashboardCommand) e func (s *sqlStore) Delete(ctx context.Context, cmd *star.UnstarDashboardCommand) 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=?" - // nolint:staticcheck - _, err := sess.Exec(rawSQL, cmd.UserID, cmd.DashboardID) + 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 }) } diff --git a/public/api-merged.json b/public/api-merged.json index b7cc9cbda1d..01c013b02d6 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -10978,76 +10978,6 @@ } } }, - "/user/stars/dashboard/{dashboard_id}": { - "post": { - "description": "Stars the given Dashboard for the actual user.", - "tags": [ - "signed_in_user" - ], - "summary": "Star a dashboard.", - "operationId": "starDashboard", - "deprecated": true, - "parameters": [ - { - "type": "string", - "name": "dashboard_id", - "in": "path", - "required": true - } - ], - "responses": { - "200": { - "$ref": "#/responses/okResponse" - }, - "400": { - "$ref": "#/responses/badRequestError" - }, - "401": { - "$ref": "#/responses/unauthorisedError" - }, - "403": { - "$ref": "#/responses/forbiddenError" - }, - "500": { - "$ref": "#/responses/internalServerError" - } - } - }, - "delete": { - "description": "Deletes the starring of the given Dashboard for the actual user.", - "tags": [ - "signed_in_user" - ], - "summary": "Unstar a dashboard.", - "operationId": "unstarDashboard", - "deprecated": true, - "parameters": [ - { - "type": "string", - "name": "dashboard_id", - "in": "path", - "required": true - } - ], - "responses": { - "200": { - "$ref": "#/responses/okResponse" - }, - "400": { - "$ref": "#/responses/badRequestError" - }, - "401": { - "$ref": "#/responses/unauthorisedError" - }, - "403": { - "$ref": "#/responses/forbiddenError" - }, - "500": { - "$ref": "#/responses/internalServerError" - } - } - } - }, "/user/teams": { "get": { "description": "Return a list of all teams that the current user is member of.", diff --git a/public/openapi3.json b/public/openapi3.json index 313d7f86aff..34efbbffb3f 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -25634,80 +25634,6 @@ ] } }, - "/user/stars/dashboard/{dashboard_id}": { - "delete": { - "deprecated": true, - "description": "Deletes the starring of the given Dashboard for the actual user.", - "operationId": "unstarDashboard", - "parameters": [ - { - "in": "path", - "name": "dashboard_id", - "required": true, - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "$ref": "#/components/responses/okResponse" - }, - "400": { - "$ref": "#/components/responses/badRequestError" - }, - "401": { - "$ref": "#/components/responses/unauthorisedError" - }, - "403": { - "$ref": "#/components/responses/forbiddenError" - }, - "500": { - "$ref": "#/components/responses/internalServerError" - } - }, - "summary": "Unstar a dashboard.", - "tags": [ - "signed_in_user" - ] - }, - "post": { - "deprecated": true, - "description": "Stars the given Dashboard for the actual user.", - "operationId": "starDashboard", - "parameters": [ - { - "in": "path", - "name": "dashboard_id", - "required": true, - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "$ref": "#/components/responses/okResponse" - }, - "400": { - "$ref": "#/components/responses/badRequestError" - }, - "401": { - "$ref": "#/components/responses/unauthorisedError" - }, - "403": { - "$ref": "#/components/responses/forbiddenError" - }, - "500": { - "$ref": "#/components/responses/internalServerError" - } - }, - "summary": "Star a dashboard.", - "tags": [ - "signed_in_user" - ] - } - }, "/user/teams": { "get": { "description": "Return a list of all teams that the current user is member of.",