From c19f156a96483ccc3d50d349e0e425ee6e7857ad Mon Sep 17 00:00:00 2001 From: Ezequiel Victorero Date: Thu, 9 Feb 2023 15:44:09 -0300 Subject: [PATCH] PublicDashboards: use share type (#63059) --- .../publicdashboards/database/database.go | 3 +- .../database/database_test.go | 8 ++++- .../publicdashboards/models/errors.go | 12 +++---- .../publicdashboards/models/models.go | 27 ++++++++------ .../publicdashboards/service/service.go | 30 ++++++++++------ .../publicdashboards/service/service_test.go | 35 +++++++++++++++++++ .../publicdashboards/validation/validation.go | 14 ++++++++ .../validation/validation_test.go | 30 +++++++++++++++- 8 files changed, 130 insertions(+), 29 deletions(-) diff --git a/pkg/services/publicdashboards/database/database.go b/pkg/services/publicdashboards/database/database.go index 681dbea3cc2..7087e8cd625 100644 --- a/pkg/services/publicdashboards/database/database.go +++ b/pkg/services/publicdashboards/database/database.go @@ -232,10 +232,11 @@ func (d *PublicDashboardStoreImpl) Update(ctx context.Context, cmd SavePublicDas return err } - sqlResult, err := sess.Exec("UPDATE dashboard_public SET is_enabled = ?, annotations_enabled = ?, time_selection_enabled = ?, time_settings = ?, updated_by = ?, updated_at = ? WHERE uid = ?", + sqlResult, err := sess.Exec("UPDATE dashboard_public SET is_enabled = ?, annotations_enabled = ?, time_selection_enabled = ?, share = ?, time_settings = ?, updated_by = ?, updated_at = ? WHERE uid = ?", cmd.PublicDashboard.IsEnabled, cmd.PublicDashboard.AnnotationsEnabled, cmd.PublicDashboard.TimeSelectionEnabled, + cmd.PublicDashboard.Share, string(timeSettingsJSON), cmd.PublicDashboard.UpdatedBy, cmd.PublicDashboard.UpdatedAt.UTC().Format("2006-01-02 15:04:05"), diff --git a/pkg/services/publicdashboards/database/database_test.go b/pkg/services/publicdashboards/database/database_test.go index d8da2b866f7..b5656f64f87 100644 --- a/pkg/services/publicdashboards/database/database_test.go +++ b/pkg/services/publicdashboards/database/database_test.go @@ -402,6 +402,7 @@ func TestIntegrationCreatePublicDashboard(t *testing.T) { IsEnabled: true, AnnotationsEnabled: true, TimeSelectionEnabled: true, + Share: PublicShareType, Uid: "pubdash-uid", DashboardUid: savedDashboard.UID, OrgId: savedDashboard.OrgID, @@ -417,10 +418,11 @@ func TestIntegrationCreatePublicDashboard(t *testing.T) { pubdash, err := publicdashboardStore.FindByDashboardUid(context.Background(), savedDashboard.OrgID, savedDashboard.UID) require.NoError(t, err) - assert.Equal(t, pubdash.AccessToken, "NOTAREALUUID") + assert.Equal(t, cmd.PublicDashboard.AccessToken, pubdash.AccessToken) assert.True(t, pubdash.IsEnabled) assert.True(t, pubdash.AnnotationsEnabled) assert.True(t, pubdash.TimeSelectionEnabled) + assert.Equal(t, cmd.PublicDashboard.Share, pubdash.Share) // verify we didn't update all dashboards pubdash2, err := publicdashboardStore.FindByDashboardUid(context.Background(), savedDashboard2.OrgID, savedDashboard2.UID) @@ -502,6 +504,7 @@ func TestIntegrationUpdatePublicDashboard(t *testing.T) { IsEnabled: true, AnnotationsEnabled: false, TimeSelectionEnabled: false, + Share: PublicShareType, CreatedAt: DefaultTime, CreatedBy: 7, AccessToken: "fakeaccesstoken", @@ -519,6 +522,7 @@ func TestIntegrationUpdatePublicDashboard(t *testing.T) { IsEnabled: false, AnnotationsEnabled: true, TimeSelectionEnabled: true, + Share: EmailShareType, TimeSettings: &TimeSettings{From: "now-8", To: "now"}, UpdatedAt: time.Now().UTC().Round(time.Second), UpdatedBy: 8, @@ -540,6 +544,7 @@ func TestIntegrationUpdatePublicDashboard(t *testing.T) { assert.Equal(t, updatedPublicDashboard.IsEnabled, pdRetrieved.IsEnabled) assert.Equal(t, updatedPublicDashboard.AnnotationsEnabled, pdRetrieved.AnnotationsEnabled) assert.Equal(t, updatedPublicDashboard.TimeSelectionEnabled, pdRetrieved.TimeSelectionEnabled) + assert.Equal(t, updatedPublicDashboard.Share, pdRetrieved.Share) // not updated dashboard shouldn't have changed pdNotUpdatedRetrieved, err := publicdashboardStore.FindByDashboardUid(context.Background(), anotherSavedDashboard.OrgID, anotherSavedDashboard.UID) @@ -547,6 +552,7 @@ func TestIntegrationUpdatePublicDashboard(t *testing.T) { assert.NotEqual(t, updatedPublicDashboard.UpdatedAt, pdNotUpdatedRetrieved.UpdatedAt) assert.NotEqual(t, updatedPublicDashboard.IsEnabled, pdNotUpdatedRetrieved.IsEnabled) assert.NotEqual(t, updatedPublicDashboard.AnnotationsEnabled, pdNotUpdatedRetrieved.AnnotationsEnabled) + assert.NotEqual(t, updatedPublicDashboard.Share, pdNotUpdatedRetrieved.Share) }) } diff --git a/pkg/services/publicdashboards/models/errors.go b/pkg/services/publicdashboards/models/errors.go index 0edaebb1610..780aacbe792 100644 --- a/pkg/services/publicdashboards/models/errors.go +++ b/pkg/services/publicdashboards/models/errors.go @@ -9,15 +9,15 @@ var ( ErrDashboardNotFound = errutil.NewBase(errutil.StatusNotFound, "publicdashboards.dashboardNotFound", errutil.WithPublicMessage("Dashboard not found")) ErrPanelNotFound = errutil.NewBase(errutil.StatusNotFound, "publicdashboards.panelNotFound", errutil.WithPublicMessage("Public dashboard panel not found")) - ErrBadRequest = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.badRequest") - ErrPanelQueriesNotFound = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.panelQueriesNotFound", errutil.WithPublicMessage("Failed to extract queries from panel")) - ErrInvalidAccessToken = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidAccessToken", errutil.WithPublicMessage("Invalid access token")) - ErrInvalidPanelId = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidPanelId", errutil.WithPublicMessage("Invalid panel id")) - ErrInvalidUid = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidUid", errutil.WithPublicMessage("Invalid Uid")) - + ErrBadRequest = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.badRequest") + ErrPanelQueriesNotFound = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.panelQueriesNotFound", errutil.WithPublicMessage("Failed to extract queries from panel")) + ErrInvalidAccessToken = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidAccessToken", errutil.WithPublicMessage("Invalid access token")) + ErrInvalidPanelId = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidPanelId", errutil.WithPublicMessage("Invalid panel id")) + ErrInvalidUid = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidUid", errutil.WithPublicMessage("Invalid Uid")) ErrPublicDashboardIdentifierNotSet = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.identifierNotSet", errutil.WithPublicMessage("No Uid for public dashboard specified")) ErrPublicDashboardHasTemplateVariables = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.hasTemplateVariables", errutil.WithPublicMessage("Public dashboard has template variables")) ErrInvalidInterval = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidInterval", errutil.WithPublicMessage("intervalMS should be greater than 0")) ErrInvalidMaxDataPoints = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.maxDataPoints", errutil.WithPublicMessage("maxDataPoints should be greater than 0")) ErrInvalidTimeRange = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidTimeRange", errutil.WithPublicMessage("Invalid time range")) + ErrInvalidShareType = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidShareType", errutil.WithPublicMessage("Invalid share type")) ) diff --git a/pkg/services/publicdashboards/models/models.go b/pkg/services/publicdashboards/models/models.go index b9b2c44892c..44d05ee9b3c 100644 --- a/pkg/services/publicdashboards/models/models.go +++ b/pkg/services/publicdashboards/models/models.go @@ -25,10 +25,19 @@ func (e PublicDashboardErr) Error() string { return "Dashboard Error" } -const QuerySuccess = "success" -const QueryFailure = "failure" +const ( + QuerySuccess = "success" + QueryFailure = "failure" + EmailShareType ShareType = "email" + PublicShareType ShareType = "public" +) -var QueryResultStatuses = []string{QuerySuccess, QueryFailure} +var ( + QueryResultStatuses = []string{QuerySuccess, QueryFailure} + ValidShareTypes = []ShareType{EmailShareType, PublicShareType} +) + +type ShareType string type PublicDashboard struct { Uid string `json:"uid" xorm:"pk uid"` @@ -39,13 +48,11 @@ type PublicDashboard struct { AccessToken string `json:"accessToken" xorm:"access_token"` AnnotationsEnabled bool `json:"annotationsEnabled" xorm:"annotations_enabled"` TimeSelectionEnabled bool `json:"timeSelectionEnabled" xorm:"time_selection_enabled"` - Share string `json:"share"` - - CreatedBy int64 `json:"createdBy" xorm:"created_by"` - UpdatedBy int64 `json:"updatedBy" xorm:"updated_by"` - - CreatedAt time.Time `json:"createdAt" xorm:"created_at"` - UpdatedAt time.Time `json:"updatedAt" xorm:"updated_at"` + Share ShareType `json:"share" xorm:"share"` + CreatedBy int64 `json:"createdBy" xorm:"created_by"` + UpdatedBy int64 `json:"updatedBy" xorm:"updated_by"` + CreatedAt time.Time `json:"createdAt" xorm:"created_at"` + UpdatedAt time.Time `json:"updatedAt" xorm:"updated_at"` } // Alias the generated type diff --git a/pkg/services/publicdashboards/service/service.go b/pkg/services/publicdashboards/service/service.go index 2d557248e4e..79bca16642c 100644 --- a/pkg/services/publicdashboards/service/service.go +++ b/pkg/services/publicdashboards/service/service.go @@ -142,11 +142,6 @@ func (pd *PublicDashboardServiceImpl) Create(ctx context.Context, u *user.Signed return nil, err } - // set default value for time settings - if dto.PublicDashboard.TimeSettings == nil { - dto.PublicDashboard.TimeSettings = &TimeSettings{} - } - // validate fields err = validation.ValidatePublicDashboard(dto, dashboard) if err != nil { @@ -162,6 +157,15 @@ func (pd *PublicDashboardServiceImpl) Create(ctx context.Context, u *user.Signed return nil, ErrBadRequest.Errorf("Create: public dashboard already exists: %s", dto.PublicDashboard.Uid) } + // set default value for time settings + if dto.PublicDashboard.TimeSettings == nil { + dto.PublicDashboard.TimeSettings = &TimeSettings{} + } + + if dto.PublicDashboard.Share == "" { + dto.PublicDashboard.Share = PublicShareType + } + uid, err := pd.NewPublicDashboardUid(ctx) if err != nil { return nil, err @@ -181,6 +185,7 @@ func (pd *PublicDashboardServiceImpl) Create(ctx context.Context, u *user.Signed AnnotationsEnabled: dto.PublicDashboard.AnnotationsEnabled, TimeSelectionEnabled: dto.PublicDashboard.TimeSelectionEnabled, TimeSettings: dto.PublicDashboard.TimeSettings, + Share: dto.PublicDashboard.Share, CreatedBy: dto.UserId, CreatedAt: time.Now(), AccessToken: accessToken, @@ -217,11 +222,6 @@ func (pd *PublicDashboardServiceImpl) Update(ctx context.Context, u *user.Signed return nil, ErrDashboardNotFound.Errorf("Update: dashboard not found by orgId: %d and dashboardUid: %s", u.OrgID, dto.DashboardUid) } - // set default value for time settings - if dto.PublicDashboard.TimeSettings == nil { - dto.PublicDashboard.TimeSettings = &TimeSettings{} - } - // get existing public dashboard if exists existingPubdash, err := pd.store.Find(ctx, dto.PublicDashboard.Uid) if err != nil { @@ -236,6 +236,15 @@ func (pd *PublicDashboardServiceImpl) Update(ctx context.Context, u *user.Signed return nil, err } + // set default value for time settings + if dto.PublicDashboard.TimeSettings == nil { + dto.PublicDashboard.TimeSettings = &TimeSettings{} + } + + if dto.PublicDashboard.Share == "" { + dto.PublicDashboard.Share = existingPubdash.Share + } + // set values to update cmd := SavePublicDashboardCommand{ PublicDashboard: PublicDashboard{ @@ -244,6 +253,7 @@ func (pd *PublicDashboardServiceImpl) Update(ctx context.Context, u *user.Signed AnnotationsEnabled: dto.PublicDashboard.AnnotationsEnabled, TimeSelectionEnabled: dto.PublicDashboard.TimeSelectionEnabled, TimeSettings: dto.PublicDashboard.TimeSettings, + Share: dto.PublicDashboard.Share, UpdatedBy: dto.UserId, UpdatedAt: time.Now(), }, diff --git a/pkg/services/publicdashboards/service/service_test.go b/pkg/services/publicdashboards/service/service_test.go index 8a1acd1389b..fd8e26971ae 100644 --- a/pkg/services/publicdashboards/service/service_test.go +++ b/pkg/services/publicdashboards/service/service_test.go @@ -145,6 +145,7 @@ func TestCreatePublicDashboard(t *testing.T) { IsEnabled: true, AnnotationsEnabled: false, TimeSelectionEnabled: true, + Share: EmailShareType, DashboardUid: "NOTTHESAME", OrgId: 9999999, TimeSettings: timeSettings, @@ -169,6 +170,7 @@ func TestCreatePublicDashboard(t *testing.T) { assert.NotEqual(t, &time.Time{}, pubdash.CreatedAt) // Time settings set by db assert.Equal(t, timeSettings, pubdash.TimeSettings) + assert.Equal(t, dto.PublicDashboard.Share, pubdash.Share) // accessToken is valid uuid _, err = uuid.Parse(pubdash.AccessToken) require.NoError(t, err, "expected a valid UUID, got %s", pubdash.AccessToken) @@ -321,6 +323,39 @@ func TestCreatePublicDashboard(t *testing.T) { require.Error(t, err) assert.True(t, ErrBadRequest.Is(err)) }) + + t.Run("Validate pubdash has default share value", func(t *testing.T) { + sqlStore := db.InitTestDB(t) + quotaService := quotatest.New(false, nil) + dashboardStore, err := dashboardsDB.ProvideDashboardStore(sqlStore, sqlStore.Cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, sqlStore.Cfg), quotaService) + require.NoError(t, err) + publicdashboardStore := database.ProvideStore(sqlStore) + dashboard := insertTestDashboard(t, dashboardStore, "testDashie", 1, 0, true, []map[string]interface{}{}, nil) + + service := &PublicDashboardServiceImpl{ + log: log.New("test.logger"), + store: publicdashboardStore, + } + + dto := &SavePublicDashboardDTO{ + DashboardUid: dashboard.UID, + OrgId: dashboard.OrgID, + UserId: 7, + PublicDashboard: &PublicDashboard{ + IsEnabled: true, + DashboardUid: "NOTTHESAME", + OrgId: 9999999, + }, + } + + _, err = service.Create(context.Background(), SignedInUser, dto) + require.NoError(t, err) + + pubdash, err := service.FindByDashboardUid(context.Background(), dashboard.OrgID, dashboard.UID) + require.NoError(t, err) + // if share type is empty should be populated with public by default + assert.Equal(t, PublicShareType, pubdash.Share) + }) } func TestUpdatePublicDashboard(t *testing.T) { diff --git a/pkg/services/publicdashboards/validation/validation.go b/pkg/services/publicdashboards/validation/validation.go index 2094cf3545d..51961343802 100644 --- a/pkg/services/publicdashboards/validation/validation.go +++ b/pkg/services/publicdashboards/validation/validation.go @@ -13,6 +13,11 @@ func ValidatePublicDashboard(dto *SavePublicDashboardDTO, dashboard *dashboards. return ErrPublicDashboardHasTemplateVariables.Errorf("ValidateSavePublicDashboard: public dashboard has template variables") } + // if it is empty we override it in the service with public for retro compatibility + if dto.PublicDashboard.Share != "" && !IsValidShareType(dto.PublicDashboard.Share) { + return ErrInvalidShareType.Errorf("ValidateSavePublicDashboard: invalid share type") + } + return nil } @@ -58,3 +63,12 @@ func IsValidAccessToken(token string) bool { func IsValidShortUID(uid string) bool { return uid != "" && util.IsValidShortUID(uid) } + +func IsValidShareType(shareType ShareType) bool { + for _, t := range ValidShareTypes { + if t == shareType { + return true + } + } + return false +} diff --git a/pkg/services/publicdashboards/validation/validation_test.go b/pkg/services/publicdashboards/validation/validation_test.go index 5ba9dcd8c30..d1944783fbd 100644 --- a/pkg/services/publicdashboards/validation/validation_test.go +++ b/pkg/services/publicdashboards/validation/validation_test.go @@ -37,11 +37,39 @@ func TestValidatePublicDashboard(t *testing.T) { }`) dashboardData, _ := simplejson.NewJson(templateVars) dashboard := dashboards.NewDashboardFromJson(dashboardData) - dto := &SavePublicDashboardDTO{DashboardUid: "abc123", OrgId: 1, UserId: 1, PublicDashboard: nil} + dto := &SavePublicDashboardDTO{DashboardUid: "abc123", OrgId: 1, UserId: 1, PublicDashboard: &PublicDashboard{}} err := ValidatePublicDashboard(dto, dashboard) require.NoError(t, err) }) + + t.Run("Returns no error when valid shareType value is received", func(t *testing.T) { + templateVars := []byte(`{ + "templating": { + "list": [] + } + }`) + dashboardData, _ := simplejson.NewJson(templateVars) + dashboard := dashboards.NewDashboardFromJson(dashboardData) + dto := &SavePublicDashboardDTO{DashboardUid: "abc123", OrgId: 1, UserId: 1, PublicDashboard: &PublicDashboard{Share: EmailShareType}} + + err := ValidatePublicDashboard(dto, dashboard) + require.NoError(t, err) + }) + + t.Run("Returns error when invalid shareType value", func(t *testing.T) { + templateVars := []byte(`{ + "templating": { + "list": [] + } + }`) + dashboardData, _ := simplejson.NewJson(templateVars) + dashboard := dashboards.NewDashboardFromJson(dashboardData) + dto := &SavePublicDashboardDTO{DashboardUid: "abc123", OrgId: 1, UserId: 1, PublicDashboard: &PublicDashboard{Share: "invalid"}} + + err := ValidatePublicDashboard(dto, dashboard) + require.Error(t, err) + }) } func TestValidateQueryPublicDashboardRequest(t *testing.T) {