From 1bd87357b66296318842dce78c4c13475e2ff91f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toma=C5=BE=20Jerman?= Date: Thu, 17 Oct 2019 12:17:48 +0200 Subject: [PATCH] Allow module field name/kind change if no records --- compose/repository/module.go | 20 +++++------ compose/service/module.go | 30 +++++++++++++++-- tests/compose/module_test.go | 64 ++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 14 deletions(-) diff --git a/compose/repository/module.go b/compose/repository/module.go index efdb0f1ed..2df5bbce8 100644 --- a/compose/repository/module.go +++ b/compose/repository/module.go @@ -25,6 +25,7 @@ type ( FindFields(moduleIDs ...uint64) (ff types.ModuleFieldSet, err error) Create(mod *types.Module) (*types.Module, error) Update(mod *types.Module) (*types.Module, error) + UpdateFields(moduleID uint64, ff types.ModuleFieldSet, hasRecords bool) (err error) DeleteByID(namespaceID, moduleID uint64) error } @@ -148,10 +149,6 @@ func (r module) Create(mod *types.Module) (*types.Module, error) { return nil, err } - if err = r.updateFields(mod.ID, mod.Fields); err != nil { - return nil, err - } - return mod, nil } @@ -159,14 +156,10 @@ func (r module) Update(mod *types.Module) (*types.Module, error) { now := time.Now().Truncate(time.Second) mod.UpdatedAt = &now - if err := r.updateFields(mod.ID, mod.Fields); err != nil { - return nil, err - } - return mod, r.db().Update(r.table(), mod, "id") } -func (r module) updateFields(moduleID uint64, ff types.ModuleFieldSet) error { +func (r module) UpdateFields(moduleID uint64, ff types.ModuleFieldSet, hasRecords bool) error { if existing, err := r.FindFields(moduleID); err != nil { return err } else { @@ -187,13 +180,16 @@ func (r module) updateFields(moduleID uint64, ff types.ModuleFieldSet) error { for idx, f := range ff { if e := existing.FindByID(f.ID); e != nil { f.CreatedAt = e.CreatedAt - f.UpdatedAt = &now // We do not have any other code in place that would handle changes of field name and kind, so we need // to reset any changes made to the field. // @todo remove when we are able to handle field rename & type change - f.Name = e.Name - f.Kind = e.Kind + if hasRecords { + f.Name = e.Name + f.Kind = e.Kind + } else { + f.UpdatedAt = &now + } } else { f.ID = 0 } diff --git a/compose/service/module.go b/compose/service/module.go index 5ac20bfff..34c572eb9 100644 --- a/compose/service/module.go +++ b/compose/service/module.go @@ -23,6 +23,7 @@ type ( ac moduleAccessController moduleRepo repository.ModuleRepository + recordRepo repository.RecordRepository pageRepo repository.PageRepository nsRepo repository.NamespaceRepository } @@ -68,6 +69,7 @@ func (svc module) With(ctx context.Context) ModuleService { ac: svc.ac, moduleRepo: repository.Module(ctx, db), + recordRepo: repository.Record(ctx, db), pageRepo: repository.Page(ctx, db), nsRepo: repository.Namespace(ctx, db), } @@ -166,7 +168,17 @@ func (svc module) Create(mod *types.Module) (*types.Module, error) { return nil, ErrNoCreatePermissions.withStack() } - return svc.moduleRepo.Create(mod) + mod, err := svc.moduleRepo.Create(mod) + if err != nil { + return nil, err + } + + err = svc.moduleRepo.UpdateFields(mod.ID, mod.Fields, false) + if err != nil { + return nil, err + } + + return mod, nil } func (svc module) Update(mod *types.Module) (m *types.Module, err error) { @@ -199,7 +211,21 @@ func (svc module) Update(mod *types.Module) (m *types.Module, err error) { m.Meta = mod.Meta m.Fields = mod.Fields - return svc.moduleRepo.Update(m) + m, err = svc.moduleRepo.Update(m) + if err != nil { + return nil, err + } + + _, ff, err := svc.recordRepo.Find(m, types.RecordFilter{}) + if err != nil { + return nil, err + } + err = svc.moduleRepo.UpdateFields(m.ID, m.Fields, ff.Count > 0) + if err != nil { + return nil, err + } + + return mod, err } func (svc module) DeleteByID(namespaceID, moduleID uint64) error { diff --git a/tests/compose/module_test.go b/tests/compose/module_test.go index ea79bc789..06ef20594 100644 --- a/tests/compose/module_test.go +++ b/tests/compose/module_test.go @@ -24,6 +24,9 @@ func (h helper) repoMakeModule(ns *types.Namespace, name string, ff ...*types.Mo Create(&types.Module{Name: name, NamespaceID: ns.ID, Fields: ff}) h.a.NoError(err) + err = h.repoModule().UpdateFields(m.ID, m.Fields, false) + h.a.NoError(err) + return m } @@ -144,6 +147,67 @@ func TestModuleUpdate(t *testing.T) { h.a.Equal("changed-name", m.Name) } +func TestModuleFieldsUpdate(t *testing.T) { + h := newHelper(t) + h.allow(types.NamespacePermissionResource.AppendWildcard(), "read") + ns := h.repoMakeNamespace("some-namespace") + m := h.repoMakeModule(ns, "some-module", &types.ModuleField{Kind: "String", Name: "existing"}) + h.allow(types.ModulePermissionResource.AppendWildcard(), "update") + + f := m.Fields[0] + fjs := fmt.Sprintf(`{ "name": "%s", "fields": [{ "fieldID": "%d", "name": "existing_edited", "kind": "Number" }, { "name": "new", "kind": "DateTime" }] }`, m.Name, f.ID) + h.apiInit(). + Post(fmt.Sprintf("/namespace/%d/module/%d", ns.ID, m.ID)). + JSON(fjs). + Expect(t). + Status(http.StatusOK). + Assert(helpers.AssertNoErrors). + End() + + ff, err := h.repoModule().FindFields(m.ID) + h.a.NoError(err) + h.a.NotNil(ff) + h.a.Len(ff, 2) + + h.a.NotNil(ff[0].UpdatedAt) + h.a.Equal(ff[0].Name, "existing_edited") + h.a.Equal(ff[0].Kind, "Number") + h.a.Nil(ff[1].UpdatedAt) + h.a.Equal(ff[1].Name, "new") + h.a.Equal(ff[1].Kind, "DateTime") +} + +func TestModuleFieldsPreventUpdate_ifRecordExists(t *testing.T) { + h := newHelper(t) + h.allow(types.NamespacePermissionResource.AppendWildcard(), "read") + ns := h.repoMakeNamespace("some-namespace") + m := h.repoMakeModule(ns, "some-module", &types.ModuleField{Kind: "String", Name: "existing"}) + h.repoMakeRecord(m, &types.RecordValue{Name: "existing", Value: "value"}) + h.allow(types.ModulePermissionResource.AppendWildcard(), "update") + + f := m.Fields[0] + fjs := fmt.Sprintf(`{ "name": "%s", "fields": [{ "fieldID": "%d", "name": "existing_edited", "kind": "Number" }, { "name": "new", "kind": "DateTime" }] }`, m.Name, f.ID) + h.apiInit(). + Post(fmt.Sprintf("/namespace/%d/module/%d", ns.ID, m.ID)). + JSON(fjs). + Expect(t). + Status(http.StatusOK). + Assert(helpers.AssertNoErrors). + End() + + ff, err := h.repoModule().FindFields(m.ID) + h.a.NoError(err) + h.a.NotNil(ff) + h.a.Len(ff, 2) + + h.a.Nil(ff[0].UpdatedAt) + h.a.Equal(ff[0].Name, "existing") + h.a.Equal(ff[0].Kind, "String") + h.a.Nil(ff[1].UpdatedAt) + h.a.Equal(ff[1].Name, "new") + h.a.Equal(ff[1].Kind, "DateTime") +} + func TestModuleDeleteForbidden(t *testing.T) { h := newHelper(t)