Stars: Remove deprecated internal ID apis (#110499)

This commit is contained in:
Ryan McKinley
2025-09-04 14:45:01 -05:00
committed by GitHub
parent 3d6d632686
commit 1dadf2cad9
10 changed files with 20 additions and 382 deletions
-7
View File
@@ -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))
+3 -3
View File
@@ -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)
}
@@ -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
-83
View File
@@ -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.
-70
View File
@@ -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)
+3 -6
View File
@@ -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"`
-43
View File
@@ -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)
+11 -26
View File
@@ -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
})
}
-70
View File
@@ -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.",
-74
View File
@@ -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.",