diff --git a/pkg/services/publicdashboards/service/service.go b/pkg/services/publicdashboards/service/service.go index 6cca9ab0ab0..601d33f9efc 100644 --- a/pkg/services/publicdashboards/service/service.go +++ b/pkg/services/publicdashboards/service/service.go @@ -140,14 +140,14 @@ func (pd *PublicDashboardServiceImpl) FindPublicDashboardAndDashboardByAccessTok // Creates and validates the public dashboard and saves it to the database func (pd *PublicDashboardServiceImpl) Create(ctx context.Context, u *user.SignedInUser, dto *SavePublicDashboardDTO) (*PublicDashboard, error) { - // ensure dashboard exists - dashboard, err := pd.FindDashboard(ctx, u.OrgID, dto.DashboardUid) + // validate fields + err := validation.ValidatePublicDashboard(dto) if err != nil { return nil, err } - // validate fields - err = validation.ValidatePublicDashboard(dto, dashboard) + // ensure dashboard exists + _, err = pd.FindDashboard(ctx, u.OrgID, dto.DashboardUid) if err != nil { return nil, err } @@ -216,6 +216,12 @@ func (pd *PublicDashboardServiceImpl) Create(ctx context.Context, u *user.Signed // Update: updates an existing public dashboard based on publicdashboard.Uid func (pd *PublicDashboardServiceImpl) Update(ctx context.Context, u *user.SignedInUser, dto *SavePublicDashboardDTO) (*PublicDashboard, error) { + // validate fields + err := validation.ValidatePublicDashboard(dto) + if err != nil { + return nil, err + } + // validate if the dashboard exists dashboard, err := pd.FindDashboard(ctx, u.OrgID, dto.DashboardUid) if err != nil { @@ -234,12 +240,6 @@ func (pd *PublicDashboardServiceImpl) Update(ctx context.Context, u *user.Signed return nil, ErrPublicDashboardNotFound.Errorf("Update: public dashboard not found by uid: %s", dto.PublicDashboard.Uid) } - // validate dashboard - err = validation.ValidatePublicDashboard(dto, dashboard) - if err != nil { - return nil, err - } - // set default value for time settings if dto.PublicDashboard.TimeSettings == nil { dto.PublicDashboard.TimeSettings = &TimeSettings{} diff --git a/pkg/services/publicdashboards/service/service_test.go b/pkg/services/publicdashboards/service/service_test.go index 15fe107312c..2c55926288d 100644 --- a/pkg/services/publicdashboards/service/service_test.go +++ b/pkg/services/publicdashboards/service/service_test.go @@ -278,7 +278,7 @@ func TestCreatePublicDashboard(t *testing.T) { assert.Equal(t, defaultPubdashTimeSettings, pubdash.TimeSettings) }) - t.Run("Validate pubdash whose dashboard has template variables returns error", func(t *testing.T) { + t.Run("Creates pubdash whose dashboard has template variables successfully", 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) @@ -286,10 +286,12 @@ func TestCreatePublicDashboard(t *testing.T) { publicdashboardStore := database.ProvideStore(sqlStore) templateVars := make([]map[string]interface{}, 1) dashboard := insertTestDashboard(t, dashboardStore, "testDashie", 1, 0, true, templateVars, nil) + serviceWrapper := ProvideServiceWrapper(publicdashboardStore) service := &PublicDashboardServiceImpl{ - log: log.New("test.logger"), - store: publicdashboardStore, + log: log.New("test.logger"), + store: publicdashboardStore, + serviceWrapper: serviceWrapper, } dto := &SavePublicDashboardDTO{ @@ -304,7 +306,13 @@ func TestCreatePublicDashboard(t *testing.T) { } _, err = service.Create(context.Background(), SignedInUser, dto) - require.Error(t, err) + require.NoError(t, err) + + pubdash, err := service.FindByDashboardUid(context.Background(), dashboard.OrgID, dashboard.UID) + require.NoError(t, err) + + assert.Equal(t, dashboard.UID, pubdash.DashboardUid) + assert.Equal(t, dashboard.OrgID, pubdash.OrgId) }) t.Run("Throws an error when pubdash with generated access token already exists", func(t *testing.T) { diff --git a/pkg/services/publicdashboards/validation/validation.go b/pkg/services/publicdashboards/validation/validation.go index 51961343802..b48c6628b31 100644 --- a/pkg/services/publicdashboards/validation/validation.go +++ b/pkg/services/publicdashboards/validation/validation.go @@ -2,17 +2,12 @@ package validation import ( "github.com/google/uuid" - "github.com/grafana/grafana/pkg/services/dashboards" . "github.com/grafana/grafana/pkg/services/publicdashboards/models" "github.com/grafana/grafana/pkg/tsdb/legacydata" "github.com/grafana/grafana/pkg/util" ) -func ValidatePublicDashboard(dto *SavePublicDashboardDTO, dashboard *dashboards.Dashboard) error { - if hasTemplateVariables(dashboard) { - return ErrPublicDashboardHasTemplateVariables.Errorf("ValidateSavePublicDashboard: public dashboard has template variables") - } - +func ValidatePublicDashboard(dto *SavePublicDashboardDTO) error { // 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") @@ -21,12 +16,6 @@ func ValidatePublicDashboard(dto *SavePublicDashboardDTO, dashboard *dashboards. return nil } -func hasTemplateVariables(dashboard *dashboards.Dashboard) bool { - templateVariables := dashboard.Data.Get("templating").Get("list").MustArray() - - return len(templateVariables) > 0 -} - func ValidateQueryPublicDashboardRequest(req PublicDashboardQueryDTO, pd *PublicDashboard) error { if req.IntervalMs < 0 { return ErrInvalidInterval.Errorf("ValidateQueryPublicDashboardRequest: intervalMS should be greater than 0") diff --git a/pkg/services/publicdashboards/validation/validation_test.go b/pkg/services/publicdashboards/validation/validation_test.go index d1944783fbd..0d317547171 100644 --- a/pkg/services/publicdashboards/validation/validation_test.go +++ b/pkg/services/publicdashboards/validation/validation_test.go @@ -3,71 +3,23 @@ package validation import ( "testing" - "github.com/grafana/grafana/pkg/components/simplejson" - "github.com/grafana/grafana/pkg/services/dashboards" . "github.com/grafana/grafana/pkg/services/publicdashboards/models" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestValidatePublicDashboard(t *testing.T) { - t.Run("Returns validation error when dashboard has template variables", func(t *testing.T) { - templateVars := []byte(`{ - "templating": { - "list": [ - { - "name": "templateVariableName" - } - ] - } - }`) - dashboardData, _ := simplejson.NewJson(templateVars) - dashboard := dashboards.NewDashboardFromJson(dashboardData) - dto := &SavePublicDashboardDTO{DashboardUid: "abc123", OrgId: 1, UserId: 1, PublicDashboard: nil} - - err := ValidatePublicDashboard(dto, dashboard) - require.ErrorContains(t, err, ErrPublicDashboardHasTemplateVariables.Error()) - }) - - t.Run("Returns no validation error when dashboard has no template variables", 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{}} - - 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) + err := ValidatePublicDashboard(dto) 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) + err := ValidatePublicDashboard(dto) require.Error(t, err) }) }