Folders: Fix creating/updating a folder whose title has leading and trailing spaces (#80909)

* Add tests

* Folders: Fix creating folder whose title has leading and trailing spaces

* Fix folder update

* Remove redundant argument

* Fix test
This commit is contained in:
Sofia Papagiannaki 2024-01-22 18:03:30 +02:00 committed by GitHub
parent e0402115ea
commit 4243079cb5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 97 additions and 29 deletions

View File

@ -56,7 +56,7 @@ func ProvideService(
features featuremgmt.FeatureToggles,
r prometheus.Registerer,
) folder.Service {
store := ProvideStore(db, cfg, features)
store := ProvideStore(db, cfg)
srv := &Service{
cfg: cfg,
log: log.New("folder-service"),
@ -457,7 +457,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
// well, but for now we take the UID from the newly created folder.
UID: dash.UID,
OrgID: cmd.OrgID,
Title: cmd.Title,
Title: dashFolder.Title,
Description: cmd.Description,
ParentUID: cmd.ParentUID,
}
@ -498,7 +498,7 @@ func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (
if foldr, err = s.store.Update(ctx, folder.UpdateFolderCommand{
UID: cmd.UID,
OrgID: cmd.OrgID,
NewTitle: cmd.NewTitle,
NewTitle: &dashFolder.Title,
NewDescription: cmd.NewDescription,
SignedInUser: user,
}); err != nil {

View File

@ -69,7 +69,7 @@ func TestIntegrationFolderService(t *testing.T) {
t.Run("Folder service tests", func(t *testing.T) {
dashStore := &dashboards.FakeDashboardStore{}
db := sqlstore.InitTestDB(t)
nestedFolderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures([]interface{}{"nestedFolders"}))
nestedFolderStore := ProvideStore(db, db.Cfg)
folderStore := foldertest.NewFakeFolderStore(t)
@ -219,23 +219,30 @@ func TestIntegrationFolderService(t *testing.T) {
dashboardFolder.ID = rand.Int63()
dashboardFolder.UID = util.GenerateShortUID()
dashboardFolder.OrgID = orgID
f := dashboards.FromDashboard(dashboardFolder)
_, err := service.store.Create(context.Background(), folder.CreateFolderCommand{
f, err := service.store.Create(context.Background(), folder.CreateFolderCommand{
OrgID: orgID,
Title: dashboardFolder.Title,
UID: dashboardFolder.UID,
SignedInUser: usr,
})
require.NoError(t, err)
assert.Equal(t, "Folder", f.Title)
dashStore.On("ValidateDashboardBeforeSave", mock.Anything, mock.AnythingOfType("*dashboards.Dashboard"), mock.AnythingOfType("bool")).Return(true, nil)
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(dashboardFolder, nil)
dashStore.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(dashboardFolder, nil)
folderStore.On("GetFolderByID", mock.Anything, orgID, dashboardFolder.ID).Return(f, nil)
title := "TEST-Folder"
updatedDashboardFolder := *dashboardFolder
updatedDashboardFolder.Title = title
dashStore.On("SaveDashboard", mock.Anything, mock.AnythingOfType("dashboards.SaveDashboardCommand")).Return(&updatedDashboardFolder, nil)
dashStore.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(&updatedDashboardFolder, nil)
folderStore.On("GetFolderByID", mock.Anything, orgID, dashboardFolder.ID).Return(&folder.Folder{
OrgID: orgID,
ID: dashboardFolder.ID,
UID: dashboardFolder.UID,
Title: title,
}, nil)
req := &folder.UpdateFolderCommand{
UID: dashboardFolder.UID,
OrgID: orgID,
@ -245,7 +252,7 @@ func TestIntegrationFolderService(t *testing.T) {
reqResult, err := service.Update(context.Background(), req)
require.NoError(t, err)
require.Equal(t, f, reqResult)
assert.Equal(t, title, reqResult.Title)
})
t.Run("When deleting folder by uid should not return access denied error", func(t *testing.T) {
@ -343,7 +350,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
featuresFlagOn := featuremgmt.WithFeatures("nestedFolders")
dashStore, err := database.ProvideDashboardStore(db, db.Cfg, featuresFlagOn, tagimpl.ProvideService(db), quotaService)
require.NoError(t, err)
nestedFolderStore := ProvideStore(db, db.Cfg, featuresFlagOn)
nestedFolderStore := ProvideStore(db, db.Cfg)
b := bus.ProvideBus(tracing.InitializeTracerForTest())
ac := acimpl.ProvideAccessControl(cfg)
@ -459,7 +466,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
featuresFlagOff := featuremgmt.WithFeatures()
dashStore, err := database.ProvideDashboardStore(db, db.Cfg, featuresFlagOff, tagimpl.ProvideService(db), quotaService)
require.NoError(t, err)
nestedFolderStore := ProvideStore(db, db.Cfg, featuresFlagOff)
nestedFolderStore := ProvideStore(db, db.Cfg)
serviceWithFlagOff := &Service{
cfg: cfg,
@ -622,7 +629,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
dashStore, err := database.ProvideDashboardStore(db, db.Cfg, tc.featuresFlag, tagimpl.ProvideService(db), quotaService)
require.NoError(t, err)
nestedFolderStore := ProvideStore(db, db.Cfg, tc.featuresFlag)
nestedFolderStore := ProvideStore(db, db.Cfg)
tc.service.dashboardStore = dashStore
tc.service.store = nestedFolderStore
@ -732,6 +739,70 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) {
})
}
func TestFolderServiceDualWrite(t *testing.T) {
g := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanSaveValue: true})
t.Cleanup(func() {
guardian.New = g
})
db := sqlstore.InitTestDB(t)
cfg := setting.NewCfg()
features := featuremgmt.WithFeatures()
nestedFolderStore := ProvideStore(db, cfg)
dashStore, err := database.ProvideDashboardStore(db, cfg, features, tagimpl.ProvideService(db), &quotatest.FakeQuotaService{})
require.NoError(t, err)
dashboardFolderStore := ProvideDashboardFolderStore(db)
folderService := &Service{
cfg: setting.NewCfg(),
store: nestedFolderStore,
db: sqlstore.InitTestDB(t),
dashboardStore: dashStore,
dashboardFolderStore: dashboardFolderStore,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
log: log.New("test-folder-service"),
accessControl: acimpl.ProvideAccessControl(cfg),
metrics: newFoldersMetrics(nil),
bus: bus.ProvideBus(tracing.InitializeTracerForTest()),
}
t.Run("When creating a folder it should trim leading and trailing spaces in both dashboard and folder tables", func(t *testing.T) {
f, err := folderService.Create(context.Background(), &folder.CreateFolderCommand{SignedInUser: usr, OrgID: orgID, Title: " my folder "})
require.NoError(t, err)
assert.Equal(t, "my folder", f.Title)
dashFolder, err := dashboardFolderStore.GetFolderByUID(context.Background(), orgID, f.UID)
require.NoError(t, err)
nestedFolder, err := nestedFolderStore.Get(context.Background(), folder.GetFolderQuery{UID: &f.UID, OrgID: orgID})
require.NoError(t, err)
assert.Equal(t, dashFolder.Title, nestedFolder.Title)
})
t.Run("When updating a folder it should trim leading and trailing spaces in both dashboard and folder tables", func(t *testing.T) {
f, err := folderService.Create(context.Background(), &folder.CreateFolderCommand{SignedInUser: usr, OrgID: orgID, Title: "my folder 2"})
require.NoError(t, err)
f, err = folderService.Update(context.Background(), &folder.UpdateFolderCommand{SignedInUser: usr, OrgID: orgID, UID: f.UID, NewTitle: util.Pointer(" my updated folder 2 "), Version: f.Version})
require.NoError(t, err)
assert.Equal(t, "my updated folder 2", f.Title)
dashFolder, err := dashboardFolderStore.GetFolderByUID(context.Background(), orgID, f.UID)
require.NoError(t, err)
nestedFolder, err := nestedFolderStore.Get(context.Background(), folder.GetFolderQuery{UID: &f.UID, OrgID: orgID})
require.NoError(t, err)
assert.Equal(t, dashFolder.Title, nestedFolder.Title)
})
}
func TestNestedFolderService(t *testing.T) {
t.Run("with feature flag unset", func(t *testing.T) {
t.Run("Should create a folder in both dashboard and folders tables", func(t *testing.T) {
@ -1196,7 +1267,7 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
featuresFlagOn := featuremgmt.WithFeatures("nestedFolders")
dashStore, err := database.ProvideDashboardStore(db, db.Cfg, featuresFlagOn, tagimpl.ProvideService(db), quotaService)
require.NoError(t, err)
nestedFolderStore := ProvideStore(db, db.Cfg, featuresFlagOn)
nestedFolderStore := ProvideStore(db, db.Cfg)
b := bus.ProvideBus(tracing.InitializeTracerForTest())
ac := acimpl.ProvideAccessControl(cfg)

View File

@ -11,7 +11,6 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
@ -21,14 +20,13 @@ type sqlStore struct {
db db.DB
log log.Logger
cfg *setting.Cfg
fm featuremgmt.FeatureToggles
}
// sqlStore implements the store interface.
var _ store = (*sqlStore)(nil)
func ProvideStore(db db.DB, cfg *setting.Cfg, features featuremgmt.FeatureToggles) *sqlStore {
return &sqlStore{db: db, log: log.New("folder-store"), cfg: cfg, fm: features}
func ProvideStore(db db.DB, cfg *setting.Cfg) *sqlStore {
return &sqlStore{db: db, log: log.New("folder-store"), cfg: cfg}
}
func (ss *sqlStore) Create(ctx context.Context, cmd folder.CreateFolderCommand) (*folder.Folder, error) {

View File

@ -11,7 +11,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
@ -29,7 +28,7 @@ func TestIntegrationCreate(t *testing.T) {
}
db := sqlstore.InitTestDB(t)
folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders))
folderStore := ProvideStore(db, db.Cfg)
orgID := CreateOrg(t, db)
@ -149,7 +148,7 @@ func TestIntegrationDelete(t *testing.T) {
}
db := sqlstore.InitTestDB(t)
folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders))
folderStore := ProvideStore(db, db.Cfg)
orgID := CreateOrg(t, db)
@ -196,7 +195,7 @@ func TestIntegrationUpdate(t *testing.T) {
}
db := sqlstore.InitTestDB(t)
folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders))
folderStore := ProvideStore(db, db.Cfg)
orgID := CreateOrg(t, db)
@ -371,7 +370,7 @@ func TestIntegrationGet(t *testing.T) {
}
db := sqlstore.InitTestDB(t)
folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders))
folderStore := ProvideStore(db, db.Cfg)
orgID := CreateOrg(t, db)
@ -450,7 +449,7 @@ func TestIntegrationGetParents(t *testing.T) {
}
db := sqlstore.InitTestDB(t)
folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders))
folderStore := ProvideStore(db, db.Cfg)
orgID := CreateOrg(t, db)
@ -518,7 +517,7 @@ func TestIntegrationGetChildren(t *testing.T) {
}
db := sqlstore.InitTestDB(t)
folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders))
folderStore := ProvideStore(db, db.Cfg)
orgID := CreateOrg(t, db)
@ -698,7 +697,7 @@ func TestIntegrationGetHeight(t *testing.T) {
}
db := sqlstore.InitTestDB(t)
folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders))
folderStore := ProvideStore(db, db.Cfg)
orgID := CreateOrg(t, db)
@ -731,7 +730,7 @@ func TestIntegrationGetFolders(t *testing.T) {
foldersNum := 10
db := sqlstore.InitTestDB(t)
folderStore := ProvideStore(db, db.Cfg, featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders))
folderStore := ProvideStore(db, db.Cfg)
orgID := CreateOrg(t, db)