From 353edebc79f6f6f45f4d9516600efaf24c60a1e1 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 17 Aug 2018 11:44:52 +0200 Subject: [PATCH 01/33] feat(actions): Actions system infra. --- actions/actions.go | 99 ++++++++++++++ actions/actions_whitebox_test.go | 180 +++++++++++++++++++++++++ actions/rules/action.go | 28 ++++ actions/rules/action_field_set.go | 104 ++++++++++++++ actions/rules/action_field_set_test.go | 135 +++++++++++++++++++ actions/rules/action_nil.go | 17 +++ convert/changeset.go | 16 +++ 7 files changed, 579 insertions(+) create mode 100644 actions/actions.go create mode 100644 actions/actions_whitebox_test.go create mode 100644 actions/rules/action.go create mode 100644 actions/rules/action_field_set.go create mode 100644 actions/rules/action_field_set_test.go create mode 100644 actions/rules/action_nil.go create mode 100644 convert/changeset.go diff --git a/actions/actions.go b/actions/actions.go new file mode 100644 index 0000000000..22332b7228 --- /dev/null +++ b/actions/actions.go @@ -0,0 +1,99 @@ +package actions + +/* + The actions system is a key component for process automation in WIT. It provides + a way of executing user-configurable, dynamic process steps depending on user + settings, schema settings and events in the WIT. + + The idea here is to provide a simple, yet powerful "publish-subscribe" system that + can connect any "event" in the system to any "action" with a clear decoupling + of events and actions with the goal of making the associations later dynamic and + configurable by the user ("user connects this event to this action"). Think + of a "IFTTT for WIT" (https://en.wikipedia.org/wiki/IFTTT). + + Actions are generic and atomic execution steps that do exactly one task and + are configurable. The actions system around the actions provide a key-based + execution of the actions. + + Some examples for an application of this system would be: + - closing all children of a parent WI that is being closed (the user connects the + "close" attribute change event of a WI to an action that closes all WIs of + a matching query). + - sending out notifications for mentions on markdown (the system executes an + action "send notification" for every mention found in markdown values). + - moving all WIs from one iteration to the next in the time sequence when + the original iteration is closed. + + For all these automations, the actions system provides a re-usable, flexible + and later user configurable way of doing that without creating lots of + custom code and/or custom process implementations that are hardcoded in the + WIT. +*/ + +import ( + "context" + + errs "github.com/pkg/errors" + uuid "github.com/satori/go.uuid" + + "github.com/fabric8-services/fabric8-wit/actions/rules" + "github.com/fabric8-services/fabric8-wit/application" + "github.com/fabric8-services/fabric8-wit/convert" +) + +// ExecuteActionsByOldNew executes all actions given in the actionConfigList +// using the mapped configuration strings and returns the new context entity. +// It takes the old version and the new version of the context entity, comparing them. +func ExecuteActionsByOldNew(ctx context.Context, db application.DB, userID uuid.UUID, oldContext convert.ChangeDetector, newContext convert.ChangeDetector, actionConfigList map[string]string) (convert.ChangeDetector, []convert.Change, error) { + if oldContext == nil || newContext == nil { + return nil, nil, errs.New("execute actions called with nil entities") + } + contextChanges, err := oldContext.ChangeSet(newContext) + if err != nil { + return nil, nil, err + } + return ExecuteActionsByChangeset(ctx, db, userID, newContext, contextChanges, actionConfigList) +} + +// ExecuteActionsByChangeset executes all actions given in the actionConfigs +// using the mapped configuration strings and returns the new context entity. +// It takes a []Change that describes the differences between the old and the new context. +func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uuid.UUID, newContext convert.ChangeDetector, contextChanges []convert.Change, actionConfigs map[string]string) (convert.ChangeDetector, []convert.Change, error) { + var actionChanges []convert.Change + for actionKey := range actionConfigs { + var err error + actionConfig := actionConfigs[actionKey] + switch actionKey { + case rules.ActionKeyNil: + newContext, actionChanges, err = executeAction(rules.ActionNil{}, actionConfig, newContext, contextChanges, &actionChanges) + case rules.ActionKeyFieldSet: + newContext, actionChanges, err = executeAction(rules.ActionFieldSet{ + Db: db, + Ctx: ctx, + UserID: &userID, + }, actionConfig, newContext, contextChanges, &actionChanges) + case rules.ActionKeyStateToMetastate: + newContext, actionChanges, err = executeAction(rules.ActionStateToMetaState{ + Db: db, + Ctx: ctx, + UserID: &userID, + }, actionConfig, newContext, contextChanges, &actionChanges) + default: + return nil, nil, errs.New("action key " + actionKey + " is unknown") + } + if err != nil { + return nil, nil, err + } + } + return newContext, actionChanges, nil +} + +// executeAction executes the action given. The actionChanges contain the changes made by +// prior action executions. The execution is expected to add/update their changes on this +// change set. +func executeAction(act rules.Action, configuration string, newContext convert.ChangeDetector, contextChanges []convert.Change, actionChanges *[]convert.Change) (convert.ChangeDetector, []convert.Change, error) { + if act == nil { + return nil, nil, errs.New("rule can not be nil") + } + return act.OnChange(newContext, contextChanges, configuration, actionChanges) +} diff --git a/actions/actions_whitebox_test.go b/actions/actions_whitebox_test.go new file mode 100644 index 0000000000..a7f3bfe637 --- /dev/null +++ b/actions/actions_whitebox_test.go @@ -0,0 +1,180 @@ +package actions + +import ( + "testing" + + "github.com/fabric8-services/fabric8-wit/gormtestsupport" + "github.com/fabric8-services/fabric8-wit/resource" + tf "github.com/fabric8-services/fabric8-wit/test/testfixture" + "github.com/fabric8-services/fabric8-wit/workitem" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +func TestSuiteAction(t *testing.T) { + resource.Require(t, resource.Database) + suite.Run(t, &ActionSuite{DBTestSuite: gormtestsupport.NewDBTestSuite()}) +} + +type ActionSuite struct { + gormtestsupport.DBTestSuite +} + +func createWICopy(wi workitem.WorkItem, state string, boardcolumns []interface{}) workitem.WorkItem { + var wiCopy workitem.WorkItem + wiCopy.ID = wi.ID + wiCopy.SpaceID = wi.SpaceID + wiCopy.Type = wi.Type + wiCopy.Number = wi.Number + wiCopy.Fields = map[string]interface{}{} + for k := range wi.Fields { + wiCopy.Fields[k] = wi.Fields[k] + } + wiCopy.Fields[workitem.SystemState] = state + wiCopy.Fields[workitem.SystemBoardcolumns] = boardcolumns + return wiCopy +} + +func (s *ActionSuite) TestChangeSet() { + fxt := tf.NewTestFixture(s.T(), s.DB, tf.CreateWorkItemEnvironment(), tf.WorkItems(2)) + + s.T().Run("different ID", func(t *testing.T) { + _, err := fxt.WorkItems[0].ChangeSet(*fxt.WorkItems[1]) + require.Error(t, err) + }) + + s.T().Run("same instance", func(t *testing.T) { + changes, err := fxt.WorkItems[0].ChangeSet(*fxt.WorkItems[0]) + require.NoError(t, err) + require.Empty(t, changes) + }) + + s.T().Run("no changes, same column order", func(t *testing.T) { + wiCopy := createWICopy(*fxt.WorkItems[0], workitem.SystemStateNew, []interface{}{"bcid0", "bcid1"}) + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + changes, err := fxt.WorkItems[0].ChangeSet(wiCopy) + require.NoError(t, err) + require.Empty(t, changes) + }) + + s.T().Run("no changes, mixed column order", func(t *testing.T) { + wiCopy := createWICopy(*fxt.WorkItems[0], workitem.SystemStateNew, []interface{}{"bcid1", "bcid0"}) + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + changes, err := fxt.WorkItems[0].ChangeSet(wiCopy) + require.NoError(t, err) + require.Empty(t, changes) + }) + + s.T().Run("state changes", func(t *testing.T) { + wiCopy := createWICopy(*fxt.WorkItems[0], workitem.SystemStateNew, []interface{}{"bcid0", "bcid1"}) + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateOpen + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + changes, err := fxt.WorkItems[0].ChangeSet(wiCopy) + require.NoError(t, err) + require.Len(t, changes, 1) + require.Equal(t, workitem.SystemState, changes[0].AttributeName) + require.Equal(t, workitem.SystemStateOpen, changes[0].NewValue) + require.Equal(t, workitem.SystemStateNew, changes[0].OldValue) + }) + + s.T().Run("column changes", func(t *testing.T) { + wiCopy := createWICopy(*fxt.WorkItems[0], workitem.SystemStateNew, []interface{}{"bcid0"}) + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + changes, err := fxt.WorkItems[0].ChangeSet(wiCopy) + require.NoError(t, err) + require.Len(t, changes, 1) + require.Equal(t, workitem.SystemBoardcolumns, changes[0].AttributeName) + require.Equal(t, wiCopy.Fields[workitem.SystemBoardcolumns], changes[0].OldValue) + require.Equal(t, fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns], changes[0].NewValue) + }) + + s.T().Run("multiple changes", func(t *testing.T) { + wiCopy := createWICopy(*fxt.WorkItems[0], workitem.SystemStateOpen, []interface{}{"bcid0"}) + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + changes, err := fxt.WorkItems[0].ChangeSet(wiCopy) + require.NoError(t, err) + require.Len(t, changes, 2) + // we intentionally test the order here as the code under test needs + // to be expanded later, supporting more changes and this is an + // integrity test on the current impl. + require.Equal(t, workitem.SystemState, changes[0].AttributeName) + require.Equal(t, workitem.SystemStateNew, changes[0].NewValue) + require.Equal(t, workitem.SystemStateOpen, changes[0].OldValue) + require.Equal(t, workitem.SystemBoardcolumns, changes[1].AttributeName) + require.Equal(t, fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns], changes[1].NewValue) + require.Equal(t, wiCopy.Fields[workitem.SystemBoardcolumns], changes[1].OldValue) + }) + + s.T().Run("new instance", func(t *testing.T) { + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{} + changes, err := fxt.WorkItems[0].ChangeSet(nil) + require.NoError(t, err) + require.Len(t, changes, 1) + require.Equal(t, workitem.SystemState, changes[0].AttributeName) + require.Equal(t, workitem.SystemStateNew, changes[0].NewValue) + require.Nil(t, changes[0].OldValue) + }) +} + +func (s *ActionSuite) TestActionExecution() { + fxt := tf.NewTestFixture(s.T(), s.DB, tf.CreateWorkItemEnvironment(), tf.WorkItems(2)) + userID := fxt.Identities[0].ID + + s.T().Run("by Old New", func(t *testing.T) { + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + newVersion := createWICopy(*fxt.WorkItems[0], workitem.SystemStateOpen, []interface{}{"bcid0", "bcid1"}) + _, changes, err := ExecuteActionsByOldNew(s.Ctx, s.GormDB, userID, fxt.WorkItems[0], newVersion, map[string]string{ + "Nil": "{ noConfig: 'none' }", + }) + require.NoError(t, err) + require.Len(t, changes, 0) + }) + + s.T().Run("by ChangeSet", func(t *testing.T) { + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + newVersion := createWICopy(*fxt.WorkItems[0], workitem.SystemStateOpen, []interface{}{"bcid0", "bcid1"}) + contextChanges, err := fxt.WorkItems[0].ChangeSet(newVersion) + require.NoError(t, err) + afterActionWI, changes, err := ExecuteActionsByChangeset(s.Ctx, s.GormDB, userID, newVersion, contextChanges, map[string]string{ + "Nil": "{ noConfig: 'none' }", + }) + require.NoError(t, err) + require.Len(t, changes, 0) + require.Equal(t, workitem.SystemStateOpen, afterActionWI.(workitem.WorkItem).Fields["system.state"]) + }) + + s.T().Run("unknown rule", func(t *testing.T) { + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + newVersion := createWICopy(*fxt.WorkItems[0], workitem.SystemStateOpen, []interface{}{"bcid0", "bcid1"}) + contextChanges, err := fxt.WorkItems[0].ChangeSet(newVersion) + require.NoError(t, err) + _, _, err = ExecuteActionsByChangeset(s.Ctx, s.GormDB, userID, newVersion, contextChanges, map[string]string{ + "unknownRule": "{ noConfig: 'none' }", + }) + require.NotNil(t, err) + }) + + s.T().Run("sideffects", func(t *testing.T) { + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + newVersion := createWICopy(*fxt.WorkItems[0], workitem.SystemStateOpen, []interface{}{"bcid0", "bcid1"}) + contextChanges, err := fxt.WorkItems[0].ChangeSet(newVersion) + require.NoError(t, err) + // Intentionally not using a constant here! + afterActionWI, changes, err := ExecuteActionsByChangeset(s.Ctx, s.GormDB, userID, newVersion, contextChanges, map[string]string{ + "FieldSet": "{ \"system.state\": \"resolved\" }", + }) + require.NoError(t, err) + require.Len(t, changes, 1) + require.Equal(t, workitem.SystemStateResolved, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) + }) +} diff --git a/actions/rules/action.go b/actions/rules/action.go new file mode 100644 index 0000000000..f0080755f7 --- /dev/null +++ b/actions/rules/action.go @@ -0,0 +1,28 @@ +package rules + +import "github.com/fabric8-services/fabric8-wit/convert" + +const ( + // ActionKeyNil is the key for the ActionKeyNil action rule. + ActionKeyNil = "Nil" + // ActionKeyFieldSet is the key for the ActionKeyFieldSet action rule. + ActionKeyFieldSet = "FieldSet" + // ActionKeyStateToMetastate is the key for the ActionKeyStateToMetastate action rule. + ActionKeyStateToMetastate = "BidirectionalStateToColumn" + + // ActionKeyStateToMetastateConfigMetastate is the key for the ActionKeyStateToMetastateConfigMetastate config parameter. + ActionKeyStateToMetastateConfigMetastate = "metaState" +) + +// Action defines an action on change of an entity. Executing an +// Action might have sideffects, but will always return the original +// given context with all changes of the Action to this context. Note +// that the execution may have sideeffects on other entities beyond the +// context. +type Action interface { + // OnChange executes this action by looking at a change set of + // updated attributes. It returns the new context. Note that this + // needs the new (after change) context and the old value(s) as + // part of the changeset. + OnChange(newContext convert.ChangeDetector, contextChanges []convert.Change, configuration string, actionChanges *[]convert.Change) (convert.ChangeDetector, []convert.Change, error) +} diff --git a/actions/rules/action_field_set.go b/actions/rules/action_field_set.go new file mode 100644 index 0000000000..5e4d21a509 --- /dev/null +++ b/actions/rules/action_field_set.go @@ -0,0 +1,104 @@ +package rules + +import ( + "context" + "encoding/json" + "reflect" + + errs "github.com/pkg/errors" + uuid "github.com/satori/go.uuid" + + "github.com/fabric8-services/fabric8-wit/application" + "github.com/fabric8-services/fabric8-wit/convert" + "github.com/fabric8-services/fabric8-wit/workitem" +) + +// ActionFieldSet takes a configuration JSON object that has field +// names as the keys and a value as the argument. It updates the +// given ChangeDetector and sets the Field[key] value to the +// values given. Note that this only works on WorkItems. +type ActionFieldSet struct { + Db application.DB + Ctx context.Context + UserID *uuid.UUID +} + +// make sure the rule is implementing the interface. +var _ Action = ActionFieldSet{} + +func (act ActionFieldSet) storeWorkItem(wi *workitem.WorkItem) (*workitem.WorkItem, error) { + if act.Ctx == nil { + return nil, errs.New("context is nil") + } + if act.Db == nil { + return nil, errs.New("database is nil") + } + if act.UserID == nil { + return nil, errs.New("userID is nil") + } + var storeResultWorkItem *workitem.WorkItem + err := application.Transactional(act.Db, func(appl application.Application) error { + var err error + storeResultWorkItem, err = appl.WorkItems().Save(act.Ctx, wi.SpaceID, *wi, *act.UserID) + if err != nil { + return errs.Wrap(err, "error updating work item") + } + return nil + }) + if err != nil { + return nil, err + } + return storeResultWorkItem, nil +} + +// OnChange executes the action rule. +func (act ActionFieldSet) OnChange(newContext convert.ChangeDetector, contextChanges []convert.Change, configuration string, actionChanges *[]convert.Change) (convert.ChangeDetector, []convert.Change, error) { + // check if the newContext is a WorkItem, fail otherwise. + wiContext, ok := newContext.(workitem.WorkItem) + if !ok { + return nil, nil, errs.New("given context is not a WorkItem: " + reflect.TypeOf(newContext).String()) + } + // deserialize the config JSON. + var rawType map[string]interface{} + err := json.Unmarshal([]byte(configuration), &rawType) + if err != nil { + return nil, nil, errs.Wrap(err, "failed to unmarshall from action configuration to a map: "+configuration) + } + // load WIT. + wit, err := act.Db.WorkItemTypes().Load(act.Ctx, wiContext.Type) + if err != nil { + return nil, nil, errs.Wrap(err, "error loading work item type: "+err.Error()) + } + // iterate over the fields. + for k, v := range rawType { + if wiContext.Fields[k] != v { + fieldType, ok := wit.Fields[k] + if !ok { + return nil, nil, errs.New("unknown field name: " + k) + } + *actionChanges = append(*actionChanges, convert.Change{ + AttributeName: k, + NewValue: v, + OldValue: wiContext.Fields[k], + }) + newValue, err := fieldType.Type.ConvertToModel(v) + if err != nil { + return nil, nil, errs.Wrap(err, "error converting new value to model") + } + wiContext.Fields[k] = newValue + } + } + // store the WorkItem. + actionResultContext, err := act.storeWorkItem(&wiContext) + if err != nil { + return nil, nil, err + } + // iterate over the resulting wi, see if all keys are there. + // if not, the key was an unknown key. + for k := range rawType { + if _, ok := actionResultContext.Fields[k]; !ok { + return nil, nil, errs.New("field attribute unknown: " + k) + } + } + return *actionResultContext, *actionChanges, nil +} diff --git a/actions/rules/action_field_set_test.go b/actions/rules/action_field_set_test.go new file mode 100644 index 0000000000..fee1847f57 --- /dev/null +++ b/actions/rules/action_field_set_test.go @@ -0,0 +1,135 @@ +package rules + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/fabric8-services/fabric8-wit/convert" + "github.com/fabric8-services/fabric8-wit/gormtestsupport" + "github.com/fabric8-services/fabric8-wit/resource" + tf "github.com/fabric8-services/fabric8-wit/test/testfixture" + "github.com/fabric8-services/fabric8-wit/workitem" +) + +func TestSuiteActionFieldSet(t *testing.T) { + resource.Require(t, resource.Database) + suite.Run(t, &ActionFieldSetSuite{DBTestSuite: gormtestsupport.NewDBTestSuite()}) +} + +type ActionFieldSetSuite struct { + gormtestsupport.DBTestSuite +} + +func createWICopy(wi workitem.WorkItem, state string, boardcolumns []interface{}) workitem.WorkItem { + var wiCopy workitem.WorkItem + wiCopy.ID = wi.ID + wiCopy.SpaceID = wi.SpaceID + wiCopy.Type = wi.Type + wiCopy.Number = wi.Number + wiCopy.Fields = map[string]interface{}{} + for k := range wi.Fields { + wiCopy.Fields[k] = wi.Fields[k] + } + wiCopy.Fields[workitem.SystemState] = state + wiCopy.Fields[workitem.SystemBoardcolumns] = boardcolumns + return wiCopy +} + +func (s *ActionFieldSetSuite) TestActionExecution() { + fxt := tf.NewTestFixture(s.T(), s.DB, tf.CreateWorkItemEnvironment(), tf.WorkItems(2)) + require.NotNil(s.T(), fxt) + require.Len(s.T(), fxt.WorkItems, 2) + + s.T().Run("sideffects", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment(), tf.WorkItems(2)) + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + newVersion := createWICopy(*fxt.WorkItems[0], workitem.SystemStateOpen, []interface{}{"bcid0", "bcid1"}) + contextChanges, err := fxt.WorkItems[0].ChangeSet(newVersion) + require.NoError(t, err) + action := ActionFieldSet{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges []convert.Change + // Not using constants here intentionally. + afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 1) + require.Equal(t, workitem.SystemState, convertChanges[0].AttributeName) + require.Equal(t, workitem.SystemStateOpen, convertChanges[0].OldValue) + require.Equal(t, workitem.SystemStateResolved, convertChanges[0].NewValue) + require.Equal(t, workitem.SystemStateResolved, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) + }) + + s.T().Run("stacking", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment(), tf.WorkItems(2)) + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + newVersion := createWICopy(*fxt.WorkItems[0], workitem.SystemStateOpen, []interface{}{"bcid0", "bcid1"}) + contextChanges, err := fxt.WorkItems[0].ChangeSet(newVersion) + require.NoError(t, err) + action := ActionFieldSet{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges []convert.Change + // Not using constants here intentionally. + afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 1) + require.Equal(t, workitem.SystemState, convertChanges[0].AttributeName) + require.Equal(t, workitem.SystemStateOpen, convertChanges[0].OldValue) + require.Equal(t, workitem.SystemStateResolved, convertChanges[0].NewValue) + require.Equal(t, workitem.SystemStateResolved, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) + // doing another change, the convertChange needs to stack. + afterActionWI, convertChanges, err = action.OnChange(afterActionWI, []convert.Change{}, "{ \"system.state\": \"new\" }", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 2) + require.Equal(t, workitem.SystemState, convertChanges[0].AttributeName) + require.Equal(t, workitem.SystemStateOpen, convertChanges[0].OldValue) + require.Equal(t, workitem.SystemStateResolved, convertChanges[0].NewValue) + require.Equal(t, workitem.SystemState, convertChanges[1].AttributeName) + require.Equal(t, workitem.SystemStateResolved, convertChanges[1].OldValue) + require.Equal(t, workitem.SystemStateNew, convertChanges[1].NewValue) + require.Equal(t, workitem.SystemStateNew, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) + }) + + s.T().Run("unknown field", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment(), tf.WorkItems(2)) + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + newVersion := createWICopy(*fxt.WorkItems[0], workitem.SystemStateOpen, []interface{}{"bcid0", "bcid1"}) + contextChanges, err := fxt.WorkItems[0].ChangeSet(newVersion) + require.NoError(t, err) + action := ActionFieldSet{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges []convert.Change + _, _, err = action.OnChange(newVersion, contextChanges, "{ \"system.notavailable\": \"updatedState\" }", &convertChanges) + require.NotNil(t, err) + }) + + s.T().Run("non-json configuration", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment(), tf.WorkItems(2)) + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{"bcid0", "bcid1"} + newVersion := createWICopy(*fxt.WorkItems[0], workitem.SystemStateOpen, []interface{}{"bcid0", "bcid1"}) + contextChanges, err := fxt.WorkItems[0].ChangeSet(newVersion) + require.NoError(t, err) + action := ActionFieldSet{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges []convert.Change + _, convertChanges, err = action.OnChange(newVersion, contextChanges, "someNonJSON", &convertChanges) + require.NotNil(t, err) + }) +} diff --git a/actions/rules/action_nil.go b/actions/rules/action_nil.go new file mode 100644 index 0000000000..2b99448e3f --- /dev/null +++ b/actions/rules/action_nil.go @@ -0,0 +1,17 @@ +package rules + +import ( + "github.com/fabric8-services/fabric8-wit/convert" +) + +// ActionNil is a dummy action rule that does nothing and has no sideffects. +type ActionNil struct { +} + +// make sure the rule is implementing the interface. +var _ Action = ActionNil{} + +// OnChange executes the action rule. +func (act ActionNil) OnChange(newContext convert.ChangeDetector, contextChanges []convert.Change, configuration string, actionChanges *[]convert.Change) (convert.ChangeDetector, []convert.Change, error) { + return newContext, nil, nil +} diff --git a/convert/changeset.go b/convert/changeset.go new file mode 100644 index 0000000000..70575c81cf --- /dev/null +++ b/convert/changeset.go @@ -0,0 +1,16 @@ +package convert + +// ChangeDetector defines funcs for getting a changeset from two +// instances of a class. This interface has to be implemented by +// all +type ChangeDetector interface { + ChangeSet(older ChangeDetector) ([]Change, error) +} + +// Change defines a set of changed values in an entity. It holds +// the attribute name as the key and old and new values. +type Change struct { + AttributeName string + NewValue interface{} + OldValue interface{} +} From 6ea5c68b9056d62de73d871142a7d1a7da79e01f Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 17 Aug 2018 12:36:52 +0200 Subject: [PATCH 02/33] fix(tests): Fixed the tests. --- actions/actions.go | 2 + workitem/workitem.go | 113 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/actions/actions.go b/actions/actions.go index 22332b7228..8e484a0c14 100644 --- a/actions/actions.go +++ b/actions/actions.go @@ -72,12 +72,14 @@ func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uu Ctx: ctx, UserID: &userID, }, actionConfig, newContext, contextChanges, &actionChanges) + /* commented out for now until this rule is added case rules.ActionKeyStateToMetastate: newContext, actionChanges, err = executeAction(rules.ActionStateToMetaState{ Db: db, Ctx: ctx, UserID: &userID, }, actionConfig, newContext, contextChanges, &actionChanges) + */ default: return nil, nil, errs.New("action key " + actionKey + " is unknown") } diff --git a/workitem/workitem.go b/workitem/workitem.go index 42a85da7cc..4a05743651 100644 --- a/workitem/workitem.go +++ b/workitem/workitem.go @@ -1,10 +1,14 @@ package workitem import ( + "sort" + "reflect" "time" "github.com/fabric8-services/fabric8-wit/log" + "github.com/fabric8-services/fabric8-wit/convert" + errs "github.com/pkg/errors" uuid "github.com/satori/go.uuid" ) @@ -53,3 +57,112 @@ func (wi WorkItem) GetLastModified() time.Time { log.Debug(nil, map[string]interface{}{"wi_id": wi.ID}, "Last modified value: %v", lastModified) return *lastModified } + +// ChangeSet derives a changeset between this workitem and a given workitem. +func (wi WorkItem) ChangeSet(older convert.ChangeDetector) ([]convert.Change, error) { + if older == nil { + // this is changeset for a new ChangeDetector, report all observed attributes to + // the change set. This needs extension once we support more attributes. + changeSet := []convert.Change{ + { + AttributeName: SystemState, + NewValue: wi.Fields[SystemState], + OldValue: nil, + }, + } + if wi.Fields[SystemBoardcolumns] != nil && len(wi.Fields[SystemBoardcolumns].([]interface{})) != 0 { + changeSet = append(changeSet, convert.Change{ + AttributeName: SystemBoardcolumns, + NewValue: wi.Fields[SystemBoardcolumns], + OldValue: nil, + }) + } + return changeSet, nil + } + olderWorkItem, ok := older.(WorkItem) + if !ok { + return nil, errs.New("Other entity is not a WorkItem: " + reflect.TypeOf(older).String()) + } + if wi.ID != olderWorkItem.ID { + return nil, errs.New("Other entity has not the same ID: " + olderWorkItem.ID.String()) + } + changes := []convert.Change{} + // CAUTION: we're only supporting changes to the system.state and to the + // board position relationship for now. If we need to support more + // attribute changes, this has to be added here. This will be likely + // necessary when adding new Actions. + // compare system.state + if wi.Fields[SystemState] != olderWorkItem.Fields[SystemState] { + changes = append(changes, convert.Change{ + AttributeName: SystemState, + NewValue: wi.Fields[SystemState], + OldValue: olderWorkItem.Fields[SystemState], + }) + } + // compare system.boardcolumns + // this field looks like this: + // system.boardcolumns": ["43f9e838-3b4b-45e8-85eb-dd402e8324b5", "69699af8-cb28-4b90-b829-24c1aad12797"] + if wi.Fields[SystemBoardcolumns] == nil && olderWorkItem.Fields[SystemBoardcolumns] == nil { + return changes, nil + } + if wi.Fields[SystemBoardcolumns] == nil || olderWorkItem.Fields[SystemBoardcolumns] == nil { + changes = append(changes, convert.Change{ + AttributeName: SystemBoardcolumns, + NewValue: wi.Fields[SystemBoardcolumns], + OldValue: olderWorkItem.Fields[SystemBoardcolumns], + }) + return changes, nil + } + if len(wi.Fields[SystemBoardcolumns].([]interface{})) == 0 || len(olderWorkItem.Fields[SystemBoardcolumns].([]interface{})) == 0 { + if len(wi.Fields[SystemBoardcolumns].([]interface{})) == 0 && len(olderWorkItem.Fields[SystemBoardcolumns].([]interface{})) == 0 { + // both lists are empty, return no change. + return changes, nil + } + // one of the lists is empty, do return a change. + changes = append(changes, convert.Change{ + AttributeName: SystemBoardcolumns, + NewValue: wi.Fields[SystemBoardcolumns], + OldValue: olderWorkItem.Fields[SystemBoardcolumns], + }) + return changes, nil + } + bcThis, ok1 := wi.Fields[SystemBoardcolumns].([]interface{}) + bcOlder, ok2 := olderWorkItem.Fields[SystemBoardcolumns].([]interface{}) + if !ok1 || !ok2 { + return nil, errs.New("Boardcolumn slice is not a interface{} slice") + } + if len(bcThis) != len(bcOlder) { + changes = append(changes, convert.Change{ + AttributeName: SystemBoardcolumns, + NewValue: wi.Fields[SystemBoardcolumns], + OldValue: olderWorkItem.Fields[SystemBoardcolumns], + }) + return changes, nil + } + // because of the handing of interface{}, we need to do manual conversion here. + thisCopyStr := make([]string, len(bcThis)) + for i := range bcThis { + thisCopyStr[i], ok = bcThis[i].(string) + if !ok { + return nil, errs.New("Boardcolumn slice values are not of type string") + } + } + olderCopyStr := make([]string, len(bcOlder)) + for i := range bcOlder { + olderCopyStr[i], ok = bcOlder[i].(string) + if !ok { + return nil, errs.New("Boardcolumn slice values are not of type string") + } + } + sort.Strings(thisCopyStr) + sort.Strings(olderCopyStr) + if !reflect.DeepEqual(thisCopyStr, olderCopyStr) { + changes = append(changes, convert.Change{ + AttributeName: SystemBoardcolumns, + NewValue: wi.Fields[SystemBoardcolumns], + OldValue: olderWorkItem.Fields[SystemBoardcolumns], + }) + return changes, nil + } + return changes, nil +} From be9dae7062a6619fb9f91316b44e3cb192b5693f Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 17 Aug 2018 12:51:19 +0200 Subject: [PATCH 03/33] fix(doc): Fixed documentation. --- convert/changeset.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/convert/changeset.go b/convert/changeset.go index 70575c81cf..28f4f1a726 100644 --- a/convert/changeset.go +++ b/convert/changeset.go @@ -2,7 +2,7 @@ package convert // ChangeDetector defines funcs for getting a changeset from two // instances of a class. This interface has to be implemented by -// all +// all entities that should trigger action rule runs. type ChangeDetector interface { ChangeSet(older ChangeDetector) ([]Change, error) } From ab89650fc58bdd55bf68cf1ecc83c9b98f6c5bf3 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 17 Aug 2018 13:21:41 +0200 Subject: [PATCH 04/33] fix(doc): Added doc. --- actions/doc.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 actions/doc.go diff --git a/actions/doc.go b/actions/doc.go new file mode 100644 index 0000000000..3c86c231bc --- /dev/null +++ b/actions/doc.go @@ -0,0 +1,40 @@ +/* +Package actions system is a key component for process automation in WIT. It provides a +way of executing user-configurable, dynamic process steps depending on user +settings, schema settings and events in the WIT. + +The idea here is to provide a simple, yet powerful "signal-slot" system that +can connect any "event" in the system to any "action" with a clear decoupling of +events and actions with the goal of making the associations later dynamic and +configurable by the user ("user connects this event to this action"). Think +of a "IFTTT for WIT". + +Actions are generic and atomic execution steps that do exactly one task and are +configurable. The actions system around the actions provide a key-based +execution of the actions. + +Some examples for an application of this system would be: + * closing all childs of a parent that is being closed (the user connects the + "close" attribute change event of a WI to an action that closes all + WIs of a matching query). + * sending out notifications for mentions on markdown (the system executes + an action "send notification" for every mention found in markdown values). + * moving all WIs from one iteration to the next in the time sequence when + the original iteration is closed. + +For all these automations, the actions system provides a re-usable, flexible and +later user configurable way of doing that without creating lots of custom code +and/or custom process implementations that are hardcoded in the WIT. + +The current PR provides the basic actions infrastructure and an implementation of +an example action rules for testing. + +This package provides two methods ExecuteActionsByOldNew() and ExecuteActionsByChangeset() +that can be called by a client (for example the controller on a request) with an entity +that is the context of the action run and a configuration for the context. The +configuration consists of a list of rule keys (identifying the rules that apply) and +respective configuration for the rules. The actions system will run the rules +sequentially and return the new context entity and a set of changes done while running +the rules. Note that executing actions may have sideffects on data beyond the context. +*/ +package actions From b7fa5c00aa00c30466e27a8d42f1fb5455b69546 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 17 Aug 2018 13:54:19 +0200 Subject: [PATCH 05/33] fix(code): Moved Change. --- actions/actions.go | 10 +++++----- {convert => actions/change}/changeset.go | 6 +++--- actions/rules/action.go | 4 ++-- actions/rules/action_field_set.go | 6 +++--- actions/rules/action_field_set_test.go | 12 ++++++------ actions/rules/action_nil.go | 4 ++-- workitem/workitem.go | 20 ++++++++++---------- 7 files changed, 31 insertions(+), 31 deletions(-) rename {convert => actions/change}/changeset.go (80%) diff --git a/actions/actions.go b/actions/actions.go index 8e484a0c14..2a03cb38e9 100644 --- a/actions/actions.go +++ b/actions/actions.go @@ -37,14 +37,14 @@ import ( uuid "github.com/satori/go.uuid" "github.com/fabric8-services/fabric8-wit/actions/rules" + "github.com/fabric8-services/fabric8-wit/actions/change" "github.com/fabric8-services/fabric8-wit/application" - "github.com/fabric8-services/fabric8-wit/convert" ) // ExecuteActionsByOldNew executes all actions given in the actionConfigList // using the mapped configuration strings and returns the new context entity. // It takes the old version and the new version of the context entity, comparing them. -func ExecuteActionsByOldNew(ctx context.Context, db application.DB, userID uuid.UUID, oldContext convert.ChangeDetector, newContext convert.ChangeDetector, actionConfigList map[string]string) (convert.ChangeDetector, []convert.Change, error) { +func ExecuteActionsByOldNew(ctx context.Context, db application.DB, userID uuid.UUID, oldContext change.Detector, newContext change.Detector, actionConfigList map[string]string) (change.Detector, []change.Change, error) { if oldContext == nil || newContext == nil { return nil, nil, errs.New("execute actions called with nil entities") } @@ -58,8 +58,8 @@ func ExecuteActionsByOldNew(ctx context.Context, db application.DB, userID uuid. // ExecuteActionsByChangeset executes all actions given in the actionConfigs // using the mapped configuration strings and returns the new context entity. // It takes a []Change that describes the differences between the old and the new context. -func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uuid.UUID, newContext convert.ChangeDetector, contextChanges []convert.Change, actionConfigs map[string]string) (convert.ChangeDetector, []convert.Change, error) { - var actionChanges []convert.Change +func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uuid.UUID, newContext change.Detector, contextChanges []change.Change, actionConfigs map[string]string) (change.Detector, []change.Change, error) { + var actionChanges []change.Change for actionKey := range actionConfigs { var err error actionConfig := actionConfigs[actionKey] @@ -93,7 +93,7 @@ func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uu // executeAction executes the action given. The actionChanges contain the changes made by // prior action executions. The execution is expected to add/update their changes on this // change set. -func executeAction(act rules.Action, configuration string, newContext convert.ChangeDetector, contextChanges []convert.Change, actionChanges *[]convert.Change) (convert.ChangeDetector, []convert.Change, error) { +func executeAction(act rules.Action, configuration string, newContext change.Detector, contextChanges []change.Change, actionChanges *[]change.Change) (change.Detector, []change.Change, error) { if act == nil { return nil, nil, errs.New("rule can not be nil") } diff --git a/convert/changeset.go b/actions/change/changeset.go similarity index 80% rename from convert/changeset.go rename to actions/change/changeset.go index 28f4f1a726..4eb44ce70d 100644 --- a/convert/changeset.go +++ b/actions/change/changeset.go @@ -1,10 +1,10 @@ -package convert +package change // ChangeDetector defines funcs for getting a changeset from two // instances of a class. This interface has to be implemented by // all entities that should trigger action rule runs. -type ChangeDetector interface { - ChangeSet(older ChangeDetector) ([]Change, error) +type Detector interface { + ChangeSet(older Detector) ([]Change, error) } // Change defines a set of changed values in an entity. It holds diff --git a/actions/rules/action.go b/actions/rules/action.go index f0080755f7..d4ae73dbe3 100644 --- a/actions/rules/action.go +++ b/actions/rules/action.go @@ -1,6 +1,6 @@ package rules -import "github.com/fabric8-services/fabric8-wit/convert" +import "github.com/fabric8-services/fabric8-wit/actions/change" const ( // ActionKeyNil is the key for the ActionKeyNil action rule. @@ -24,5 +24,5 @@ type Action interface { // updated attributes. It returns the new context. Note that this // needs the new (after change) context and the old value(s) as // part of the changeset. - OnChange(newContext convert.ChangeDetector, contextChanges []convert.Change, configuration string, actionChanges *[]convert.Change) (convert.ChangeDetector, []convert.Change, error) + OnChange(newContext change.Detector, contextChanges []change.Change, configuration string, actionChanges *[]change.Change) (change.Detector, []change.Change, error) } diff --git a/actions/rules/action_field_set.go b/actions/rules/action_field_set.go index 5e4d21a509..b4c3da636d 100644 --- a/actions/rules/action_field_set.go +++ b/actions/rules/action_field_set.go @@ -9,7 +9,7 @@ import ( uuid "github.com/satori/go.uuid" "github.com/fabric8-services/fabric8-wit/application" - "github.com/fabric8-services/fabric8-wit/convert" + "github.com/fabric8-services/fabric8-wit/actions/change" "github.com/fabric8-services/fabric8-wit/workitem" ) @@ -52,7 +52,7 @@ func (act ActionFieldSet) storeWorkItem(wi *workitem.WorkItem) (*workitem.WorkIt } // OnChange executes the action rule. -func (act ActionFieldSet) OnChange(newContext convert.ChangeDetector, contextChanges []convert.Change, configuration string, actionChanges *[]convert.Change) (convert.ChangeDetector, []convert.Change, error) { +func (act ActionFieldSet) OnChange(newContext change.Detector, contextChanges []change.Change, configuration string, actionChanges *[]change.Change) (change.Detector, []change.Change, error) { // check if the newContext is a WorkItem, fail otherwise. wiContext, ok := newContext.(workitem.WorkItem) if !ok { @@ -76,7 +76,7 @@ func (act ActionFieldSet) OnChange(newContext convert.ChangeDetector, contextCha if !ok { return nil, nil, errs.New("unknown field name: " + k) } - *actionChanges = append(*actionChanges, convert.Change{ + *actionChanges = append(*actionChanges, change.Change{ AttributeName: k, NewValue: v, OldValue: wiContext.Fields[k], diff --git a/actions/rules/action_field_set_test.go b/actions/rules/action_field_set_test.go index fee1847f57..7cca61007f 100644 --- a/actions/rules/action_field_set_test.go +++ b/actions/rules/action_field_set_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - "github.com/fabric8-services/fabric8-wit/convert" + "github.com/fabric8-services/fabric8-wit/actions/change" "github.com/fabric8-services/fabric8-wit/gormtestsupport" "github.com/fabric8-services/fabric8-wit/resource" tf "github.com/fabric8-services/fabric8-wit/test/testfixture" @@ -54,7 +54,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges []convert.Change + var convertChanges []change.Change // Not using constants here intentionally. afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", &convertChanges) require.NoError(t, err) @@ -77,7 +77,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges []convert.Change + var convertChanges []change.Change // Not using constants here intentionally. afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", &convertChanges) require.NoError(t, err) @@ -87,7 +87,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { require.Equal(t, workitem.SystemStateResolved, convertChanges[0].NewValue) require.Equal(t, workitem.SystemStateResolved, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) // doing another change, the convertChange needs to stack. - afterActionWI, convertChanges, err = action.OnChange(afterActionWI, []convert.Change{}, "{ \"system.state\": \"new\" }", &convertChanges) + afterActionWI, convertChanges, err = action.OnChange(afterActionWI, []change.Change{}, "{ \"system.state\": \"new\" }", &convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) require.Equal(t, workitem.SystemState, convertChanges[0].AttributeName) @@ -111,7 +111,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges []convert.Change + var convertChanges []change.Change _, _, err = action.OnChange(newVersion, contextChanges, "{ \"system.notavailable\": \"updatedState\" }", &convertChanges) require.NotNil(t, err) }) @@ -128,7 +128,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges []convert.Change + var convertChanges []change.Change _, convertChanges, err = action.OnChange(newVersion, contextChanges, "someNonJSON", &convertChanges) require.NotNil(t, err) }) diff --git a/actions/rules/action_nil.go b/actions/rules/action_nil.go index 2b99448e3f..eae3573fa0 100644 --- a/actions/rules/action_nil.go +++ b/actions/rules/action_nil.go @@ -1,7 +1,7 @@ package rules import ( - "github.com/fabric8-services/fabric8-wit/convert" + "github.com/fabric8-services/fabric8-wit/actions/change" ) // ActionNil is a dummy action rule that does nothing and has no sideffects. @@ -12,6 +12,6 @@ type ActionNil struct { var _ Action = ActionNil{} // OnChange executes the action rule. -func (act ActionNil) OnChange(newContext convert.ChangeDetector, contextChanges []convert.Change, configuration string, actionChanges *[]convert.Change) (convert.ChangeDetector, []convert.Change, error) { +func (act ActionNil) OnChange(newContext change.Detector, contextChanges []change.Change, configuration string, actionChanges *[]change.Change) (change.Detector, []change.Change, error) { return newContext, nil, nil } diff --git a/workitem/workitem.go b/workitem/workitem.go index 4a05743651..f530e0d7f0 100644 --- a/workitem/workitem.go +++ b/workitem/workitem.go @@ -6,7 +6,7 @@ import ( "time" "github.com/fabric8-services/fabric8-wit/log" - "github.com/fabric8-services/fabric8-wit/convert" + "github.com/fabric8-services/fabric8-wit/actions/change" errs "github.com/pkg/errors" uuid "github.com/satori/go.uuid" @@ -59,11 +59,11 @@ func (wi WorkItem) GetLastModified() time.Time { } // ChangeSet derives a changeset between this workitem and a given workitem. -func (wi WorkItem) ChangeSet(older convert.ChangeDetector) ([]convert.Change, error) { +func (wi WorkItem) ChangeSet(older change.Detector) ([]change.Change, error) { if older == nil { // this is changeset for a new ChangeDetector, report all observed attributes to // the change set. This needs extension once we support more attributes. - changeSet := []convert.Change{ + changeSet := []change.Change{ { AttributeName: SystemState, NewValue: wi.Fields[SystemState], @@ -71,7 +71,7 @@ func (wi WorkItem) ChangeSet(older convert.ChangeDetector) ([]convert.Change, er }, } if wi.Fields[SystemBoardcolumns] != nil && len(wi.Fields[SystemBoardcolumns].([]interface{})) != 0 { - changeSet = append(changeSet, convert.Change{ + changeSet = append(changeSet, change.Change{ AttributeName: SystemBoardcolumns, NewValue: wi.Fields[SystemBoardcolumns], OldValue: nil, @@ -86,14 +86,14 @@ func (wi WorkItem) ChangeSet(older convert.ChangeDetector) ([]convert.Change, er if wi.ID != olderWorkItem.ID { return nil, errs.New("Other entity has not the same ID: " + olderWorkItem.ID.String()) } - changes := []convert.Change{} + changes := []change.Change{} // CAUTION: we're only supporting changes to the system.state and to the // board position relationship for now. If we need to support more // attribute changes, this has to be added here. This will be likely // necessary when adding new Actions. // compare system.state if wi.Fields[SystemState] != olderWorkItem.Fields[SystemState] { - changes = append(changes, convert.Change{ + changes = append(changes, change.Change{ AttributeName: SystemState, NewValue: wi.Fields[SystemState], OldValue: olderWorkItem.Fields[SystemState], @@ -106,7 +106,7 @@ func (wi WorkItem) ChangeSet(older convert.ChangeDetector) ([]convert.Change, er return changes, nil } if wi.Fields[SystemBoardcolumns] == nil || olderWorkItem.Fields[SystemBoardcolumns] == nil { - changes = append(changes, convert.Change{ + changes = append(changes, change.Change{ AttributeName: SystemBoardcolumns, NewValue: wi.Fields[SystemBoardcolumns], OldValue: olderWorkItem.Fields[SystemBoardcolumns], @@ -119,7 +119,7 @@ func (wi WorkItem) ChangeSet(older convert.ChangeDetector) ([]convert.Change, er return changes, nil } // one of the lists is empty, do return a change. - changes = append(changes, convert.Change{ + changes = append(changes, change.Change{ AttributeName: SystemBoardcolumns, NewValue: wi.Fields[SystemBoardcolumns], OldValue: olderWorkItem.Fields[SystemBoardcolumns], @@ -132,7 +132,7 @@ func (wi WorkItem) ChangeSet(older convert.ChangeDetector) ([]convert.Change, er return nil, errs.New("Boardcolumn slice is not a interface{} slice") } if len(bcThis) != len(bcOlder) { - changes = append(changes, convert.Change{ + changes = append(changes, change.Change{ AttributeName: SystemBoardcolumns, NewValue: wi.Fields[SystemBoardcolumns], OldValue: olderWorkItem.Fields[SystemBoardcolumns], @@ -157,7 +157,7 @@ func (wi WorkItem) ChangeSet(older convert.ChangeDetector) ([]convert.Change, er sort.Strings(thisCopyStr) sort.Strings(olderCopyStr) if !reflect.DeepEqual(thisCopyStr, olderCopyStr) { - changes = append(changes, convert.Change{ + changes = append(changes, change.Change{ AttributeName: SystemBoardcolumns, NewValue: wi.Fields[SystemBoardcolumns], OldValue: olderWorkItem.Fields[SystemBoardcolumns], From 5589d6827176aa5b0e6c8bef485051cca883e9bc Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 17 Aug 2018 14:21:53 +0200 Subject: [PATCH 06/33] fix(code): Refactoring Change.Set. --- actions/actions.go | 8 ++++---- actions/change/changeset.go | 5 ++++- actions/rules/action.go | 2 +- actions/rules/action_field_set.go | 2 +- actions/rules/action_field_set_test.go | 10 +++++----- actions/rules/action_nil.go | 2 +- workitem/workitem.go | 4 ++-- 7 files changed, 18 insertions(+), 15 deletions(-) diff --git a/actions/actions.go b/actions/actions.go index 2a03cb38e9..942d515d03 100644 --- a/actions/actions.go +++ b/actions/actions.go @@ -44,7 +44,7 @@ import ( // ExecuteActionsByOldNew executes all actions given in the actionConfigList // using the mapped configuration strings and returns the new context entity. // It takes the old version and the new version of the context entity, comparing them. -func ExecuteActionsByOldNew(ctx context.Context, db application.DB, userID uuid.UUID, oldContext change.Detector, newContext change.Detector, actionConfigList map[string]string) (change.Detector, []change.Change, error) { +func ExecuteActionsByOldNew(ctx context.Context, db application.DB, userID uuid.UUID, oldContext change.Detector, newContext change.Detector, actionConfigList map[string]string) (change.Detector, change.Set, error) { if oldContext == nil || newContext == nil { return nil, nil, errs.New("execute actions called with nil entities") } @@ -58,8 +58,8 @@ func ExecuteActionsByOldNew(ctx context.Context, db application.DB, userID uuid. // ExecuteActionsByChangeset executes all actions given in the actionConfigs // using the mapped configuration strings and returns the new context entity. // It takes a []Change that describes the differences between the old and the new context. -func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uuid.UUID, newContext change.Detector, contextChanges []change.Change, actionConfigs map[string]string) (change.Detector, []change.Change, error) { - var actionChanges []change.Change +func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uuid.UUID, newContext change.Detector, contextChanges []change.Change, actionConfigs map[string]string) (change.Detector, change.Set, error) { + var actionChanges change.Set for actionKey := range actionConfigs { var err error actionConfig := actionConfigs[actionKey] @@ -93,7 +93,7 @@ func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uu // executeAction executes the action given. The actionChanges contain the changes made by // prior action executions. The execution is expected to add/update their changes on this // change set. -func executeAction(act rules.Action, configuration string, newContext change.Detector, contextChanges []change.Change, actionChanges *[]change.Change) (change.Detector, []change.Change, error) { +func executeAction(act rules.Action, configuration string, newContext change.Detector, contextChanges change.Set, actionChanges *change.Set) (change.Detector, change.Set, error) { if act == nil { return nil, nil, errs.New("rule can not be nil") } diff --git a/actions/change/changeset.go b/actions/change/changeset.go index 4eb44ce70d..3db3dc124f 100644 --- a/actions/change/changeset.go +++ b/actions/change/changeset.go @@ -1,10 +1,13 @@ package change +// Set is a set of changes to an entitiy. +type Set []Change + // ChangeDetector defines funcs for getting a changeset from two // instances of a class. This interface has to be implemented by // all entities that should trigger action rule runs. type Detector interface { - ChangeSet(older Detector) ([]Change, error) + ChangeSet(older Detector) (Set, error) } // Change defines a set of changed values in an entity. It holds diff --git a/actions/rules/action.go b/actions/rules/action.go index d4ae73dbe3..db7b7de4a6 100644 --- a/actions/rules/action.go +++ b/actions/rules/action.go @@ -24,5 +24,5 @@ type Action interface { // updated attributes. It returns the new context. Note that this // needs the new (after change) context and the old value(s) as // part of the changeset. - OnChange(newContext change.Detector, contextChanges []change.Change, configuration string, actionChanges *[]change.Change) (change.Detector, []change.Change, error) + OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) } diff --git a/actions/rules/action_field_set.go b/actions/rules/action_field_set.go index b4c3da636d..6e6c7378dc 100644 --- a/actions/rules/action_field_set.go +++ b/actions/rules/action_field_set.go @@ -52,7 +52,7 @@ func (act ActionFieldSet) storeWorkItem(wi *workitem.WorkItem) (*workitem.WorkIt } // OnChange executes the action rule. -func (act ActionFieldSet) OnChange(newContext change.Detector, contextChanges []change.Change, configuration string, actionChanges *[]change.Change) (change.Detector, []change.Change, error) { +func (act ActionFieldSet) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { // check if the newContext is a WorkItem, fail otherwise. wiContext, ok := newContext.(workitem.WorkItem) if !ok { diff --git a/actions/rules/action_field_set_test.go b/actions/rules/action_field_set_test.go index 7cca61007f..e1a26f443f 100644 --- a/actions/rules/action_field_set_test.go +++ b/actions/rules/action_field_set_test.go @@ -54,7 +54,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges []change.Change + var convertChanges change.Set // Not using constants here intentionally. afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", &convertChanges) require.NoError(t, err) @@ -77,7 +77,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges []change.Change + var convertChanges change.Set // Not using constants here intentionally. afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", &convertChanges) require.NoError(t, err) @@ -87,7 +87,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { require.Equal(t, workitem.SystemStateResolved, convertChanges[0].NewValue) require.Equal(t, workitem.SystemStateResolved, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) // doing another change, the convertChange needs to stack. - afterActionWI, convertChanges, err = action.OnChange(afterActionWI, []change.Change{}, "{ \"system.state\": \"new\" }", &convertChanges) + afterActionWI, convertChanges, err = action.OnChange(afterActionWI, change.Set{}, "{ \"system.state\": \"new\" }", &convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) require.Equal(t, workitem.SystemState, convertChanges[0].AttributeName) @@ -111,7 +111,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges []change.Change + var convertChanges change.Set _, _, err = action.OnChange(newVersion, contextChanges, "{ \"system.notavailable\": \"updatedState\" }", &convertChanges) require.NotNil(t, err) }) @@ -128,7 +128,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges []change.Change + var convertChanges change.Set _, convertChanges, err = action.OnChange(newVersion, contextChanges, "someNonJSON", &convertChanges) require.NotNil(t, err) }) diff --git a/actions/rules/action_nil.go b/actions/rules/action_nil.go index eae3573fa0..8a005da3ee 100644 --- a/actions/rules/action_nil.go +++ b/actions/rules/action_nil.go @@ -12,6 +12,6 @@ type ActionNil struct { var _ Action = ActionNil{} // OnChange executes the action rule. -func (act ActionNil) OnChange(newContext change.Detector, contextChanges []change.Change, configuration string, actionChanges *[]change.Change) (change.Detector, []change.Change, error) { +func (act ActionNil) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { return newContext, nil, nil } diff --git a/workitem/workitem.go b/workitem/workitem.go index f530e0d7f0..641b8e7923 100644 --- a/workitem/workitem.go +++ b/workitem/workitem.go @@ -59,11 +59,11 @@ func (wi WorkItem) GetLastModified() time.Time { } // ChangeSet derives a changeset between this workitem and a given workitem. -func (wi WorkItem) ChangeSet(older change.Detector) ([]change.Change, error) { +func (wi WorkItem) ChangeSet(older change.Detector) (change.Set, error) { if older == nil { // this is changeset for a new ChangeDetector, report all observed attributes to // the change set. This needs extension once we support more attributes. - changeSet := []change.Change{ + changeSet := change.Set{ { AttributeName: SystemState, NewValue: wi.Fields[SystemState], From f62536745eee297aa417fe870c761440cd29637d Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 17 Aug 2018 15:32:30 +0200 Subject: [PATCH 07/33] fix(formatting): Format fixed. --- actions/actions.go | 2 +- actions/doc.go | 34 +++++++++++++++---------------- actions/rules/action_field_set.go | 2 +- workitem/workitem.go | 4 ++-- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/actions/actions.go b/actions/actions.go index 942d515d03..a2db8f80e8 100644 --- a/actions/actions.go +++ b/actions/actions.go @@ -36,8 +36,8 @@ import ( errs "github.com/pkg/errors" uuid "github.com/satori/go.uuid" - "github.com/fabric8-services/fabric8-wit/actions/rules" "github.com/fabric8-services/fabric8-wit/actions/change" + "github.com/fabric8-services/fabric8-wit/actions/rules" "github.com/fabric8-services/fabric8-wit/application" ) diff --git a/actions/doc.go b/actions/doc.go index 3c86c231bc..3f062fc6c1 100644 --- a/actions/doc.go +++ b/actions/doc.go @@ -1,38 +1,38 @@ -/* -Package actions system is a key component for process automation in WIT. It provides a -way of executing user-configurable, dynamic process steps depending on user +/* +Package actions system is a key component for process automation in WIT. It provides a +way of executing user-configurable, dynamic process steps depending on user settings, schema settings and events in the WIT. -The idea here is to provide a simple, yet powerful "signal-slot" system that -can connect any "event" in the system to any "action" with a clear decoupling of -events and actions with the goal of making the associations later dynamic and -configurable by the user ("user connects this event to this action"). Think +The idea here is to provide a simple, yet powerful "signal-slot" system that +can connect any "event" in the system to any "action" with a clear decoupling of +events and actions with the goal of making the associations later dynamic and +configurable by the user ("user connects this event to this action"). Think of a "IFTTT for WIT". -Actions are generic and atomic execution steps that do exactly one task and are -configurable. The actions system around the actions provide a key-based +Actions are generic and atomic execution steps that do exactly one task and are +configurable. The actions system around the actions provide a key-based execution of the actions. Some examples for an application of this system would be: - * closing all childs of a parent that is being closed (the user connects the - "close" attribute change event of a WI to an action that closes all + * closing all childs of a parent that is being closed (the user connects the + "close" attribute change event of a WI to an action that closes all WIs of a matching query). - * sending out notifications for mentions on markdown (the system executes + * sending out notifications for mentions on markdown (the system executes an action "send notification" for every mention found in markdown values). - * moving all WIs from one iteration to the next in the time sequence when + * moving all WIs from one iteration to the next in the time sequence when the original iteration is closed. -For all these automations, the actions system provides a re-usable, flexible and -later user configurable way of doing that without creating lots of custom code +For all these automations, the actions system provides a re-usable, flexible and +later user configurable way of doing that without creating lots of custom code and/or custom process implementations that are hardcoded in the WIT. -The current PR provides the basic actions infrastructure and an implementation of +The current PR provides the basic actions infrastructure and an implementation of an example action rules for testing. This package provides two methods ExecuteActionsByOldNew() and ExecuteActionsByChangeset() that can be called by a client (for example the controller on a request) with an entity that is the context of the action run and a configuration for the context. The -configuration consists of a list of rule keys (identifying the rules that apply) and +configuration consists of a list of rule keys (identifying the rules that apply) and respective configuration for the rules. The actions system will run the rules sequentially and return the new context entity and a set of changes done while running the rules. Note that executing actions may have sideffects on data beyond the context. diff --git a/actions/rules/action_field_set.go b/actions/rules/action_field_set.go index 6e6c7378dc..6aae26d4ea 100644 --- a/actions/rules/action_field_set.go +++ b/actions/rules/action_field_set.go @@ -8,8 +8,8 @@ import ( errs "github.com/pkg/errors" uuid "github.com/satori/go.uuid" - "github.com/fabric8-services/fabric8-wit/application" "github.com/fabric8-services/fabric8-wit/actions/change" + "github.com/fabric8-services/fabric8-wit/application" "github.com/fabric8-services/fabric8-wit/workitem" ) diff --git a/workitem/workitem.go b/workitem/workitem.go index 641b8e7923..b44620b1f2 100644 --- a/workitem/workitem.go +++ b/workitem/workitem.go @@ -1,12 +1,12 @@ package workitem import ( - "sort" "reflect" + "sort" "time" - "github.com/fabric8-services/fabric8-wit/log" "github.com/fabric8-services/fabric8-wit/actions/change" + "github.com/fabric8-services/fabric8-wit/log" errs "github.com/pkg/errors" uuid "github.com/satori/go.uuid" From 00305cf3a62d4ddefaa5cd0b7bd03d444cbb1817 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 17 Aug 2018 16:37:35 +0200 Subject: [PATCH 08/33] fix(staterule): Added state rule. --- actions/rules/action_state_to_metastate.go | 514 ++++++++++++++++++ .../rules/action_state_to_metastate_test.go | 336 ++++++++++++ ...ll_show_after_update_work_item.golden.json | 2 +- ...assignee_null_update_work_item.golden.json | 2 +- ...ll_show_after_update_work_item.golden.json | 2 +- ...er_label_null_update_work_item.golden.json | 2 +- ...t_include_children.res.payload.golden.json | 4 +- .../include_children.res.payload.golden.json | 8 +- .../A_with_tree-view=false.res.golden.json | 2 +- .../A_with_tree-view=true.res.golden.json | 2 +- .../B,C_with_tree-view=false.res.golden.json | 4 +- .../B,C_with_tree-view=true.res.golden.json | 6 +- .../B_with_tree-view=false.res.golden.json | 2 +- .../B_with_tree-view=true.res.golden.json | 4 +- .../C_with_tree-view=false.res.golden.json | 2 +- .../C_with_tree-view=true.res.golden.json | 6 +- .../D_with_tree-view=false.res.golden.json | 2 +- .../D_with_tree-view=true.res.golden.json | 4 +- .../E_with_tree-view=false.res.golden.json | 2 +- .../E_with_tree-view=true.res.golden.json | 2 +- .../list_children/ok.res.payload.golden.json | 4 +- .../ok.has_children.res.payload.golden.json | 2 +- ...ok.has_no_children.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_2.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_2.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_2.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_2.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_2.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_2.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_2.res.payload.golden.json | 2 +- .../valid_sample_3.res.payload.golden.json | 2 +- .../valid_sample_1.res.payload.golden.json | 2 +- .../valid_sample_2.res.payload.golden.json | 2 +- .../create/ok.res.payload.golden.json | 4 +- .../list/ok.res.paylpad.golden.json | 4 +- .../show/ok.res.payload.golden.json | 4 +- spacetemplate/assets/base.yaml | 4 +- test/testfixture/make_functions.go | 16 +- workitem/workitemtype.go | 1 + 52 files changed, 923 insertions(+), 72 deletions(-) create mode 100644 actions/rules/action_state_to_metastate.go create mode 100644 actions/rules/action_state_to_metastate_test.go diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go new file mode 100644 index 0000000000..51431d7fe0 --- /dev/null +++ b/actions/rules/action_state_to_metastate.go @@ -0,0 +1,514 @@ +package rules + +import ( + "github.com/fabric8-services/fabric8-wit/actions/change" + "context" + "encoding/json" + + errs "github.com/pkg/errors" + uuid "github.com/satori/go.uuid" + + "github.com/fabric8-services/fabric8-wit/application" + "github.com/fabric8-services/fabric8-wit/workitem" +) + +// ActionStateToMetaState implements the bidirectional mapping between state and column. +type ActionStateToMetaState struct { + Db application.DB + Ctx context.Context + UserID *uuid.UUID +} + +// make sure the rule is implementing the interface. +var _ Action = ActionStateToMetaState{} + +func (act ActionStateToMetaState) containsUUID(s []uuid.UUID, e uuid.UUID) bool { + for _, a := range s { + if a == e { + return true + } + } + return false +} + +func (act ActionStateToMetaState) contains(s []interface{}, e interface{}) bool { + for _, a := range s { + if a == e { + return true + } + } + return false +} + +func (act ActionStateToMetaState) removeElement(s []interface{}, e interface{}) []interface{} { + for idx, a := range s { + if a == e { + s = append(s[:idx], s[idx+1:]...) + // we don't return here as there may be multiple copies of e in s. + } + } + return s +} + +// difference returns the differences between two slices. It returns two slices containing +// the added and removed elements compared from old and new arguments. +func (act ActionStateToMetaState) difference(old []interface{}, new []interface{}) ([]interface{}, []interface{}) { + var added []interface{} + var removed []interface{} + // added in slice2 + for _, elem := range new { + if !act.contains(old, elem) { + added = append(added, elem) + } + } + // removed in slice2 + for _, elem := range old { + if !act.contains(new, elem) { + removed = append(removed, elem) + } + } + return added, removed +} + +func (act ActionStateToMetaState) loadWorkItemBoardsBySpaceID(spaceID uuid.UUID) ([]*workitem.Board, error) { + if act.Ctx == nil { + return nil, errs.New("context is nil") + } + if act.Db == nil { + return nil, errs.New("database is nil") + } + space, err := act.Db.Spaces().Load(act.Ctx, spaceID) + if err != nil { + return nil, errs.Wrap(err, "error loading space: "+err.Error()) + } + boards, err := act.Db.Boards().List(act.Ctx, space.SpaceTemplateID) + if err != nil { + return nil, errs.Wrap(err, "error loading work item type: "+err.Error()) + } + return boards, nil +} + +func (act ActionStateToMetaState) loadWorkItemTypeGroupsBySpaceID(spaceID uuid.UUID) ([]*workitem.WorkItemTypeGroup, error) { + if act.Ctx == nil { + return nil, errs.New("context is nil") + } + if act.Db == nil { + return nil, errs.New("database is nil") + } + space, err := act.Db.Spaces().Load(act.Ctx, spaceID) + if err != nil { + return nil, errs.Wrap(err, "error loading space: "+err.Error()) + } + groups, err := act.Db.WorkItemTypeGroups().List(act.Ctx, space.SpaceTemplateID) + if err != nil { + return nil, errs.Wrap(err, "error loading work item type: "+err.Error()) + } + return groups, nil +} + +func (act ActionStateToMetaState) loadWorkItemTypeByID(id uuid.UUID) (*workitem.WorkItemType, error) { + if act.Ctx == nil { + return nil, errs.New("context is nil") + } + if act.Db == nil { + return nil, errs.New("database is nil") + } + wit, err := act.Db.WorkItemTypes().Load(act.Ctx, id) + if err != nil { + return nil, errs.Wrap(err, "error loading work item type: "+err.Error()) + } + return wit, nil +} + +func (act ActionStateToMetaState) loadWorkItemByID(id uuid.UUID) (*workitem.WorkItem, error) { + if act.Ctx == nil { + return nil, errs.New("context is nil") + } + if act.Db == nil { + return nil, errs.New("database is nil") + } + wi, err := act.Db.WorkItems().LoadByID(act.Ctx, id) + if err != nil { + return nil, errs.Wrap(err, "error loading work item: "+err.Error()) + } + return wi, nil +} + +func (act ActionStateToMetaState) storeWorkItem(workitem *workitem.WorkItem) (*workitem.WorkItem, error) { + if act.Ctx == nil { + return nil, errs.New("context is nil") + } + if act.Db == nil { + return nil, errs.New("database is nil") + } + if act.UserID == nil { + return nil, errs.New("userID is nil") + } + err := application.Transactional(act.Db, func(appl application.Application) error { + var err error + workitem, err = appl.WorkItems().Save(act.Ctx, workitem.SpaceID, *workitem, *act.UserID) + if err != nil { + return errs.Wrap(err, "error updating work item") + } + return nil + }) + if err != nil { + return nil, err + } + return workitem, nil +} + +func (act ActionStateToMetaState) getValueListFromFieldType(wit *workitem.WorkItemType, fieldName string) ([]interface{}, error) { + fieldType := wit.Fields[fieldName].Type + switch t := fieldType.(type) { + case workitem.EnumType: + return t.Values, nil + } + return nil, errs.New("given field on workitemtype " + wit.ID.String() + " is not an enum field: " + fieldName) +} + +func (act ActionStateToMetaState) getStateToMetastateMap(workitemTypeID uuid.UUID) (map[string]string, error) { + wit, err := act.loadWorkItemTypeByID(workitemTypeID) + if err != nil { + return nil, err + } + stateList, err := act.getValueListFromFieldType(wit, workitem.SystemState) + if err != nil { + return nil, err + } + metastateList, err := act.getValueListFromFieldType(wit, workitem.SystemMetaState) + if err != nil { + return nil, err + } + if len(stateList) != len(metastateList) { + return nil, errs.New("inconsistent number of states and metatstates in the current work item type (must be equal)") + } + stateToMetastateMap := make(map[string]string) + for idx := range stateList { + thisState, ok := stateList[idx].(string) + if !ok { + return nil, errs.New("state value in value list is not of type string") + } + thisMetastate, ok := metastateList[idx].(string) + if !ok { + return nil, errs.New("metastate value in value list is not of type string") + } + // this is important: we only add a new entry if there + // is not an entry already existing. Therefore, we're only + // using the first mapping, satisfying the req for getting the + // first matching metastate for a state. + // this is done here even if the usecase where we have + // multiple states with the same value (duplicate states) + // might be not that common. + if _, ok := stateToMetastateMap[thisState]; !ok { + stateToMetastateMap[thisState] = thisMetastate + } + } + return stateToMetastateMap, nil +} + +func (act ActionStateToMetaState) getMetastateToStateMap(workitemTypeID uuid.UUID) (map[string]string, error) { + stateToMetastate, err := act.getStateToMetastateMap(workitemTypeID) + if err != nil { + return nil, err + } + metastateToStateMap := make(map[string]string) + for state, metastate := range stateToMetastate { + // this is important: we only add a new entry if there + // is not an entry already existing. Therefore, we're only + // using the first mapping, satisfying the req for getting the + // first matching state for a metastate. + if _, ok := metastateToStateMap[metastate]; !ok { + metastateToStateMap[metastate] = state + } + } + return metastateToStateMap, nil +} + +func (act ActionStateToMetaState) addOrUpdateChange(changes change.Set, attributeName string, oldValue interface{}, newValue interface{}) change.Set { + for _, change := range changes { + if change.AttributeName == attributeName { + change.NewValue = newValue + return changes + } + } + newChanges := append(changes, change.Change{ + AttributeName: attributeName, + OldValue: oldValue, + NewValue: newValue, + }) + return newChanges +} + +func (act ActionStateToMetaState) fuseChanges(c1 change.Set, c2 change.Set) *change.Set { + for _, change := range c2 { + c1 = act.addOrUpdateChange(c1, change.AttributeName, change.OldValue, change.NewValue) + } + return &c1 +} + +// OnChange executes the action rule. +func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { + if actionChanges == nil { + return nil, nil, errs.New("given actionChanges is nil") + } + if len(contextChanges) == 0 { + // no changes, just return what we have. + return newContext, *actionChanges, nil + } + // if we have multiple changes, this iterates over them and cherrypicks, fusing the results together. + var err error + var executionChanges change.Set + for _, change := range contextChanges { + if change.AttributeName == workitem.SystemState { + newContext, executionChanges, err = act.OnStateChange(newContext, contextChanges, configuration, actionChanges) + } + if change.AttributeName == workitem.SystemBoardcolumns { + newContext, executionChanges, err = act.OnBoardColumnsChange(newContext, contextChanges, configuration, actionChanges) + } + if err != nil { + return nil, nil, err + } + actionChanges = act.fuseChanges(*actionChanges, executionChanges) + } + // return the result + return newContext, *actionChanges, nil +} + +// OnBoardColumnsChange is executed when the columns change. It eventually updates the metastate and the state. +func (act ActionStateToMetaState) OnBoardColumnsChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { + // we already assume that the rule applies, this needs to be checked in the controller. + // there is no additional check on the rule key. + wi, ok := newContext.(workitem.WorkItem) + if !ok { + return nil, nil, errs.New("given context is not a WorkItem instance") + } + // extract columns that changed from oldValue newValue + var columnsAdded []interface{} + for _, change := range contextChanges { + if change.AttributeName == workitem.SystemBoardcolumns { + var oldValue []interface{} + oldValue, ok := change.OldValue.([]interface{}) + if !ok && change.OldValue != nil { + // OldValue may be nil, so only throw error when non-nil and + // can not convert. + return nil, nil, errs.New("error converting oldValue set") + } + newValue, ok := change.NewValue.([]interface{}) + if !ok { + return nil, nil, errs.New("error converting newValue set") + } + columnsAdded, _ = act.difference(oldValue, newValue) + } + } + // from here on, we ignore the removed columns as a possible metastate transition is + // only happening on *adding* a new column. Removing column does not trigger a change. + if len(columnsAdded) == 0 { + // somehow, no actual changes on the columns. + return newContext, *actionChanges, nil + } + // get the mapping. + mapping, err := act.getMetastateToStateMap(wi.Type) + if err != nil { + return nil, nil, err + } + // create a dirty flag. Note that we can not use len(actionChanges) as this + // may contain previous changes from the action chain. + wiDirty := false + var changes change.Set + // go over all added columns. We support multiple added columns at once. + for _, columnID := range columnsAdded { + // get board and column by columnID. + var thisColumn workitem.BoardColumn + boards, err := act.loadWorkItemBoardsBySpaceID(wi.SpaceID) + if err != nil { + return nil, nil, err + } + for _, board := range boards { + for _, column := range board.Columns { + if columnID == column.ID.String() { + thisColumn = column + } + } + } + // at this point, we don't check if the board was + // relevant (matching the type group) as the move into + // the column has already happened. We just make sure + // this rule applies to the column. + if thisColumn.TransRuleKey != ActionKeyStateToMetastate { + // this is a column that does not apply to the rule, we don't apply here. + return newContext, *actionChanges, nil + } + // unmarshall the configuration. + config := map[string]string{} + err = json.Unmarshal([]byte(thisColumn.TransRuleArgument), &config) + if err != nil { + return nil, nil, err + } + // extract the column's metastate config. + if metaState, ok := config[ActionKeyStateToMetastateConfigMetastate]; ok { + if metaState == wi.Fields[workitem.SystemMetaState] { + // the WIs metastate is already the same as the columns + // metastate, so nothing to do. + return newContext, *actionChanges, nil + } + // the metatstate changes, so set it on the WI. + changes = act.addOrUpdateChange(changes, workitem.SystemMetaState, wi.Fields[workitem.SystemMetaState], metaState) + wi.Fields[workitem.SystemMetaState] = metaState + wiDirty = true + // next, check if the state needs to change as well from the metastate. + if wi.Fields[workitem.SystemState] != mapping[metaState] { + // yes, the state changes as well. + changes = act.addOrUpdateChange(changes, workitem.SystemState, wi.Fields[workitem.SystemState], mapping[metaState]) + wi.Fields[workitem.SystemState] = mapping[metaState] + } + } + } + // finally, store the new work item state if something changed. + var resultingWorkItem workitem.WorkItem + if wiDirty { + result, err := act.storeWorkItem(&wi) + resultingWorkItem = *result + if err != nil { + return nil, nil, err + } + } + // return to sender + return resultingWorkItem, changes, nil +} + +// OnStateChange is executed when the state changes. It eventually updates the metastate and the boardcolumns. +func (act ActionStateToMetaState) OnStateChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { + wi, ok := newContext.(workitem.WorkItem) + if !ok { + return nil, nil, errs.New("given context is not a WorkItem instance") + } + // get the mapping. + mapping, err := act.getStateToMetastateMap(wi.Type) + if err != nil { + return nil, nil, err + } + // create a dirty flag. Note that we can not use len(actionChanges) as this + // may contain previous changes from the action chain. + wiDirty := false + // update the workitem accordingly. + wiState := wi.Fields[workitem.SystemState].(string) + if wi.Fields[workitem.SystemMetaState] == mapping[wiState] { + // metastate remains stable, nothing to do. + return newContext, *actionChanges, nil + } + // otherwise, update the metastate from the state. + changes := act.addOrUpdateChange(*actionChanges, workitem.SystemMetaState, wi.Fields[workitem.SystemMetaState], mapping[wiState]) + wi.Fields[workitem.SystemMetaState] = mapping[wiState] + wiDirty = true + // next, get the columns of the workitem and see if these needs to be updated. + boards, err := act.loadWorkItemBoardsBySpaceID(wi.SpaceID) + if err != nil { + return nil, nil, err + } + // next, check which boards are relevant for this WI. + groups, err := act.loadWorkItemTypeGroupsBySpaceID(wi.SpaceID) + if err != nil { + return nil, nil, err + } + var relevantBoards []*workitem.Board + for _, board := range boards { + // this rule is only dealing with TypeLevelContext boards right now + // this may need to be extended when we allow other boards. + if board.ContextType == "TypeLevelContext" { + // now check if the type level in the Context includes the current WIs type + thisBoardContext, err := uuid.FromString(board.Context) + if err != nil { + return nil, nil, err + } + for _, group := range groups { + if group.ID == thisBoardContext && act.containsUUID(group.TypeList, wi.Type) { + // this board is relevant. + relevantBoards = append(relevantBoards, board) + } + } + } + } + // next, iterate over all relevant boards, checking their rule config + // and update the WI position accordingly. + var systemBoardColumns []interface{} + systemBoardColumns, ok = wi.Fields[workitem.SystemBoardcolumns].([]interface{}) + if !ok && wi.Fields[workitem.SystemBoardcolumns] != nil { + // wi.Fields[workitem.SystemBoardcolumns] may be empty, so we do a fallback. + return nil, nil, errs.New("type conversion failed for boardcolumns") + } + oldColumnsConfig := make([]interface{}, len(systemBoardColumns)) + copy(oldColumnsConfig, systemBoardColumns) + columnsChanged := false + for _, board := range relevantBoards { + // each WI can only be in exactly one columns on each board. + // we apply the first column that matches. If that happens, we + // flip alreadyPlacedInColumn and all subsequent matching columns + // are non-matching. + alreadyPlacedInColumn := false + for _, column := range board.Columns { + columnRuleKey := column.TransRuleKey + columnRuleConfig := column.TransRuleArgument + if columnRuleKey == ActionKeyStateToMetastate { + // unmarshall the configuration. + config := map[string]string{} + err := json.Unmarshal([]byte(columnRuleConfig), &config) + if err != nil { + return nil, nil, err + } + if metaState, ok := config[ActionKeyStateToMetastateConfigMetastate]; ok { + if metaState == wi.Fields[workitem.SystemMetaState] && !alreadyPlacedInColumn { + // the column config matches the *new* metastate, so the WI needs to + // appear in this column. + var currentSystemBoardColumn []interface{} + currentSystemBoardColumn, ok = wi.Fields[workitem.SystemBoardcolumns].([]interface{}) + // flip alreadyPlacedInColumn, so this will be the new column. Other + // matching columns are ignored on this board. + alreadyPlacedInColumn = true + if !ok && wi.Fields[workitem.SystemBoardcolumns] != nil { + // again, wi.Fields[workitem.SystemBoardcolumns] may be empty, so we + // only fail here when the conversion fails AND the slice is non nil. + return nil, nil, errs.New("error converting SystemBoardcolumns set") + } + if !act.contains(currentSystemBoardColumn, column.ID.String()) { + wi.Fields[workitem.SystemBoardcolumns] = append(currentSystemBoardColumn, column.ID.String()) + } + columnsChanged = true + } else { + // the column *does not* match the *new* metastate, so the column has to + // be removed from the WIs columns. Note that we can't just remove all + // entries in wi.Fields[workitem.SystemBoardcolumns] as there may be + // other columns from non-relevant boards in it that need to be left + // untouched. + var currentSystemBoardColumn []interface{} + currentSystemBoardColumn, ok = wi.Fields[workitem.SystemBoardcolumns].([]interface{}) + if !ok && wi.Fields[workitem.SystemBoardcolumns] != nil { + // again, wi.Fields[workitem.SystemBoardcolumns] may be empty, so we + // only fail here when the conversion fails AND the slice is non nil. + return nil, nil, errs.New("error converting SystemBoardcolumns set") + } + wi.Fields[workitem.SystemBoardcolumns] = act.removeElement(currentSystemBoardColumn, column.ID.String()) + columnsChanged = true + } + } else { + return nil, nil, errs.New("invalid configuration for transRuleKey '" + ActionKeyStateToMetastate + "': " + columnRuleConfig) + } + } + } + } + // if the column set has changed, create an entry for the change set. + if columnsChanged { + changes = act.addOrUpdateChange(changes, workitem.SystemBoardcolumns, oldColumnsConfig, wi.Fields[workitem.SystemBoardcolumns]) + } + // finally, store the new work item state if something changed. + var resultingWorkItem workitem.WorkItem + if wiDirty { + result, err := act.storeWorkItem(&wi) + resultingWorkItem = *result + if err != nil { + return nil, nil, err + } + } + // and return to sender. + return resultingWorkItem, changes, nil +} \ No newline at end of file diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go new file mode 100644 index 0000000000..718f1ee14f --- /dev/null +++ b/actions/rules/action_state_to_metastate_test.go @@ -0,0 +1,336 @@ +package rules + +import ( + "testing" + + "github.com/fabric8-services/fabric8-wit/actions/change" + "github.com/fabric8-services/fabric8-wit/resource" + "github.com/fabric8-services/fabric8-wit/workitem" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/fabric8-services/fabric8-wit/gormtestsupport" + tf "github.com/fabric8-services/fabric8-wit/test/testfixture" +) + +func TestSuiteActionStateToMetastate(t *testing.T) { + resource.Require(t, resource.Database) + suite.Run(t, &ActionStateToMetastateSuite{DBTestSuite: gormtestsupport.NewDBTestSuite()}) +} + +type ActionStateToMetastateSuite struct { + gormtestsupport.DBTestSuite +} + +func (s *ActionStateToMetastateSuite) TestActionExecution() { + // Note on the fixture: by default, the created board is attached to the type group + // with the same index. The columns on each board are as follows: + // 0 "New" "mNew" + // 1 "In Progress" "mInprogress" + // 2 "Resolved" "mResolved" + // 3 "Approved" "mResolved" + // All columns are set to the "BidirectionalStateToColumn" rule. The type has + // the following states/metastates: + // "new" "mNew" + // "open" "mOpen" + // "in progress" "mInprogress" + // "resolved" "mResolved" + // "closed" "mClosed" + fxtFn := func(t *testing.T) *tf.TestFixture { + // this sets up a work item in state "new" with matching metastate "mNew" and the corresponding column 0 ("New") + // this represents a single work item in an ordered "new" state. + return tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment(), tf.WorkItemBoards(1), tf.WorkItems(1, func(fxt *tf.TestFixture, idx int) error { + fxt.WorkItems[idx].Fields[workitem.SystemState] = workitem.SystemStateNew + fxt.WorkItems[idx].Fields[workitem.SystemMetaState] = "mNew" + fxt.WorkItems[idx].Fields[workitem.SystemBoardcolumns] = []string{ + fxt.WorkItemBoards[idx].Columns[0].ID.String(), + } + return nil + })) + } + // test the fixture creation. + fxt := fxtFn(s.T()) + require.NotNil(s.T(), fxt) + require.Len(s.T(), fxt.WorkItems, 1) + require.Equal(s.T(), "mNew", fxt.WorkItems[0].Fields[workitem.SystemMetaState]) + require.Equal(s.T(), workitem.SystemStateNew, fxt.WorkItems[0].Fields[workitem.SystemState]) + require.Equal(s.T(), fxt.WorkItemBoards[0].Columns[0].ID.String(), fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns].([]interface{})[0]) + + s.T().Run("updating the state for an existing work item", func(t *testing.T) { + fxt := fxtFn(t) + // set the state to "in progress" and create a changeset. + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateInProgress + contextChanges := change.Set{ + { + AttributeName: workitem.SystemState, + OldValue: workitem.SystemStateNew, + NewValue: workitem.SystemStateInProgress, + }, + } + // run the test. + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges change.Set + // note: the rule does not use the explicit configuration, but reads from the template. + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 2) + // check metastate validity. + require.Equal(t, workitem.SystemMetaState, convertChanges[0].AttributeName) + require.Equal(t, "mNew", convertChanges[0].OldValue) + require.Equal(t, "mInprogress", convertChanges[0].NewValue) + require.Equal(t, "mInprogress", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + // check column validity. + require.Equal(t, workitem.SystemBoardcolumns, convertChanges[1].AttributeName) + require.Len(t, convertChanges[1].OldValue, 1) + require.Equal(t, convertChanges[1].OldValue.([]interface{})[0], fxt.WorkItemBoards[0].Columns[0].ID.String()) + require.Equal(t, convertChanges[1].NewValue.([]interface{})[0], fxt.WorkItemBoards[0].Columns[1].ID.String()) + require.Equal(t, "mInprogress", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + require.Len(t, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{}), 1) + require.Equal(t, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{})[0], fxt.WorkItemBoards[0].Columns[1].ID.String()) + }) + + s.T().Run("updating the state for a vanilla work item", func(t *testing.T) { + fxt := fxtFn(t) + // this should be a vanilla work item, where metastate and boardcolumns is nil + delete(fxt.WorkItems[0].Fields, workitem.SystemMetaState) + delete(fxt.WorkItems[0].Fields, workitem.SystemBoardcolumns) + // set the state to "in progress" and create a changeset. + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateInProgress + contextChanges := change.Set{ + { + AttributeName: workitem.SystemState, + OldValue: nil, + NewValue: workitem.SystemStateInProgress, + }, + } + // run the test. + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges change.Set + // note: the rule does not use the explicit configuration, but reads from the template. + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 2) + // check metastate validity. + require.Equal(t, workitem.SystemMetaState, convertChanges[0].AttributeName) + require.Nil(t, convertChanges[0].OldValue) + require.Equal(t, "mInprogress", convertChanges[0].NewValue) + require.Equal(t, "mInprogress", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + // check column validity. + require.Equal(t, workitem.SystemBoardcolumns, convertChanges[1].AttributeName) + require.Empty(t, convertChanges[1].OldValue) + require.Equal(t, fxt.WorkItemBoards[0].Columns[1].ID.String(), convertChanges[1].NewValue.([]interface{})[0]) + require.Equal(t, "mInprogress", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + require.Len(t, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{}), 1) + require.Equal(t, fxt.WorkItemBoards[0].Columns[1].ID.String(), afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{})[0]) + }) + + s.T().Run("updating multiple attributes and state", func(t *testing.T) { + fxt := fxtFn(t) + // this should be a vanilla work item, where metastate and boardcolumns is nil + delete(fxt.WorkItems[0].Fields, workitem.SystemMetaState) + delete(fxt.WorkItems[0].Fields, workitem.SystemBoardcolumns) + // set the state to "in progress" and create a changeset. + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateInProgress + fxt.WorkItems[0].Fields[workitem.SystemTitle] = "Updated Title" + contextChanges := change.Set{ + { + AttributeName: workitem.SystemTitle, + OldValue: nil, + NewValue: "Updated Title", + }, + { + AttributeName: workitem.SystemState, + OldValue: nil, + NewValue: workitem.SystemStateInProgress, + }, + } + // run the test. + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges change.Set + // note: the rule does not use the explicit configuration, but reads from the template. + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 2) + // check metastate validity. + require.Equal(t, workitem.SystemMetaState, convertChanges[0].AttributeName) + require.Nil(t, convertChanges[0].OldValue) + require.Equal(t, "mInprogress", convertChanges[0].NewValue) + require.Equal(t, "mInprogress", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + // check column validity. + require.Equal(t, workitem.SystemBoardcolumns, convertChanges[1].AttributeName) + require.Empty(t, convertChanges[1].OldValue) + require.Equal(t, fxt.WorkItemBoards[0].Columns[1].ID.String(), convertChanges[1].NewValue.([]interface{})[0]) + require.Equal(t, "mInprogress", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + require.Len(t, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{}), 1) + require.Equal(t, fxt.WorkItemBoards[0].Columns[1].ID.String(), afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{})[0]) + }) + + s.T().Run("updating the state for a work item with multiple metastate mappings on columns", func(t *testing.T) { + fxt := fxtFn(t) + // this should be a vanilla work item, where metastate and boardcolumns is nil + delete(fxt.WorkItems[0].Fields, workitem.SystemMetaState) + delete(fxt.WorkItems[0].Fields, workitem.SystemBoardcolumns) + // set the state to "resolved" and create a changeset. + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateResolved + contextChanges := change.Set{ + { + AttributeName: workitem.SystemState, + OldValue: nil, + NewValue: workitem.SystemStateResolved, + }, + } + // run the test. + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges change.Set + // note: the rule does not use the explicit configuration, but reads from the template. + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 2) + // check metastate validity. + require.Equal(t, workitem.SystemMetaState, convertChanges[0].AttributeName) + require.Nil(t, convertChanges[0].OldValue) + require.Equal(t, "mResolved", convertChanges[0].NewValue) + require.Equal(t, "mResolved", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + // check column validity. For the resolved state, two columns are matching, + // but only the first one (column 2) should be used and available in the WI. + require.Equal(t, workitem.SystemBoardcolumns, convertChanges[1].AttributeName) + require.Empty(t, convertChanges[1].OldValue) + require.Equal(t, convertChanges[1].NewValue.([]interface{})[0], fxt.WorkItemBoards[0].Columns[2].ID.String()) + require.Equal(t, "mResolved", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + require.Len(t, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{}), 1) + require.Equal(t, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{})[0], fxt.WorkItemBoards[0].Columns[2].ID.String()) + }) + + s.T().Run("updating the columns for an existing work item", func(t *testing.T) { + fxt := fxtFn(t) + // set the column to the "in progress" column and create a changeset. + contextChanges := change.Set{ + { + AttributeName: workitem.SystemBoardcolumns, + OldValue: fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns], + NewValue: []interface{}{fxt.WorkItemBoards[0].Columns[1].ID.String()}, + }, + } + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{fxt.WorkItemBoards[0].Columns[1].ID.String()} + // run the test. + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges change.Set + // note: the rule does not use the explicit configuration, but reads from the template. + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 2) + // check metastate validity. + require.Equal(t, workitem.SystemMetaState, convertChanges[0].AttributeName) + require.Equal(t, "mNew", convertChanges[0].OldValue) + require.Equal(t, "mInprogress", convertChanges[0].NewValue) + require.Equal(t, "mInprogress", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + // check state validity. + require.Equal(t, workitem.SystemState, convertChanges[1].AttributeName) + require.Equal(t, workitem.SystemStateNew, convertChanges[1].OldValue) + require.Equal(t, workitem.SystemStateInProgress, convertChanges[1].NewValue) + require.Equal(t, workitem.SystemStateInProgress, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) + }) + + s.T().Run("updating the state and columns for an existing work item", func(t *testing.T) { + fxt := fxtFn(t) + contextChanges := change.Set{ + { + AttributeName: workitem.SystemState, + OldValue: fxt.WorkItems[0].Fields[workitem.SystemState], + NewValue: workitem.SystemStateResolved, + }, + { + AttributeName: workitem.SystemBoardcolumns, + OldValue: fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns], + NewValue: []interface{}{fxt.WorkItemBoards[0].Columns[2].ID.String()}, + }, + } + // set the state + fxt.WorkItems[0].Fields[workitem.SystemState] = workitem.SystemStateResolved + // set the column to the "resolved" column and create a changeset. + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{fxt.WorkItemBoards[0].Columns[2].ID.String()} + // run the test. + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges change.Set + // note: the rule does not use the explicit configuration, but reads from the template. + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 2) + // check metastate validity. + require.Equal(t, workitem.SystemMetaState, convertChanges[0].AttributeName) + require.Equal(t, "mNew", convertChanges[0].OldValue) + require.Equal(t, "mResolved", convertChanges[0].NewValue) + require.Equal(t, "mResolved", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + // check column validity. For the resolved state, two columns are matching, + // but only the first one (column 2) should be used and available in the WI. + require.Equal(t, workitem.SystemBoardcolumns, convertChanges[1].AttributeName) + require.Equal(t, []interface{}{fxt.WorkItemBoards[0].Columns[2].ID.String()}, convertChanges[1].OldValue) + require.Equal(t, convertChanges[1].NewValue.([]interface{})[0], fxt.WorkItemBoards[0].Columns[2].ID.String()) + require.Equal(t, "mResolved", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + require.Len(t, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{}), 1) + require.Equal(t, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemBoardcolumns].([]interface{})[0], fxt.WorkItemBoards[0].Columns[2].ID.String()) + // check state validity. + require.Equal(t, workitem.SystemStateResolved, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) + }) + + s.T().Run("updating the columns for a vanilla work item", func(t *testing.T) { + fxt := fxtFn(t) + // set the column to the "in progress" column and create a changeset. + contextChanges := change.Set{ + { + AttributeName: workitem.SystemBoardcolumns, + OldValue: nil, + NewValue: []interface{}{fxt.WorkItemBoards[0].Columns[1].ID.String()}, + }, + } + delete(fxt.WorkItems[0].Fields, workitem.SystemMetaState) + fxt.WorkItems[0].Fields[workitem.SystemBoardcolumns] = []interface{}{fxt.WorkItemBoards[0].Columns[1].ID.String()} + // run the test. + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + var convertChanges change.Set + // note: the rule does not use the explicit configuration, but reads from the template. + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + require.NoError(t, err) + require.Len(t, convertChanges, 2) + // check metastate validity. + require.Equal(t, workitem.SystemMetaState, convertChanges[0].AttributeName) + require.Nil(t, convertChanges[0].OldValue) + require.Equal(t, "mInprogress", convertChanges[0].NewValue) + require.Equal(t, "mInprogress", afterActionWI.(workitem.WorkItem).Fields[workitem.SystemMetaState]) + // check state validity. + require.Equal(t, workitem.SystemState, convertChanges[1].AttributeName) + require.Equal(t, workitem.SystemStateNew, convertChanges[1].OldValue) + require.Equal(t, workitem.SystemStateInProgress, convertChanges[1].NewValue) + require.Equal(t, workitem.SystemStateInProgress, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) + }) + +} \ No newline at end of file diff --git a/controller/test-files/search/show/filter_assignee_null_show_after_update_work_item.golden.json b/controller/test-files/search/show/filter_assignee_null_show_after_update_work_item.golden.json index e5604a8da1..fb75f8c98c 100755 --- a/controller/test-files/search/show/filter_assignee_null_show_after_update_work_item.golden.json +++ b/controller/test-files/search/show/filter_assignee_null_show_after_update_work_item.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/filter_assignee_null_update_work_item.golden.json b/controller/test-files/search/show/filter_assignee_null_update_work_item.golden.json index e5604a8da1..fb75f8c98c 100755 --- a/controller/test-files/search/show/filter_assignee_null_update_work_item.golden.json +++ b/controller/test-files/search/show/filter_assignee_null_update_work_item.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/filter_label_null_show_after_update_work_item.golden.json b/controller/test-files/search/show/filter_label_null_show_after_update_work_item.golden.json index e5604a8da1..fb75f8c98c 100755 --- a/controller/test-files/search/show/filter_label_null_show_after_update_work_item.golden.json +++ b/controller/test-files/search/show/filter_label_null_show_after_update_work_item.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/filter_label_null_update_work_item.golden.json b/controller/test-files/search/show/filter_label_null_update_work_item.golden.json index e5604a8da1..fb75f8c98c 100755 --- a/controller/test-files/search/show/filter_label_null_update_work_item.golden.json +++ b/controller/test-files/search/show/filter_label_null_update_work_item.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C-D_and_B-E_search_for_B_and_C/do_not_include_children.res.payload.golden.json b/controller/test-files/search/show/in_topology_A-B-C-D_and_B-E_search_for_B_and_C/do_not_include_children.res.payload.golden.json index 734aedc792..008142bc86 100755 --- a/controller/test-files/search/show/in_topology_A-B-C-D_and_B-E_search_for_B_and_C/do_not_include_children.res.payload.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C-D_and_B-E_search_for_B_and_C/do_not_include_children.res.payload.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, @@ -87,7 +87,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 3, "system.order": 3000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C-D_and_B-E_search_for_B_and_C/include_children.res.payload.golden.json b/controller/test-files/search/show/in_topology_A-B-C-D_and_B-E_search_for_B_and_C/include_children.res.payload.golden.json index 011ea55a6d..198592e2e1 100755 --- a/controller/test-files/search/show/in_topology_A-B-C-D_and_B-E_search_for_B_and_C/include_children.res.payload.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C-D_and_B-E_search_for_B_and_C/include_children.res.payload.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, @@ -92,7 +92,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 3, "system.order": 3000, "system.remote_item_id": null, @@ -183,7 +183,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, @@ -267,7 +267,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 5, "system.order": 5000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/A_with_tree-view=false.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/A_with_tree-view=false.res.golden.json index e0b331557b..8fe566357c 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/A_with_tree-view=false.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/A_with_tree-view=false.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/A_with_tree-view=true.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/A_with_tree-view=true.res.golden.json index d6b1410b16..290d20d9b6 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/A_with_tree-view=true.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/A_with_tree-view=true.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B,C_with_tree-view=false.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B,C_with_tree-view=false.res.golden.json index 27a63b3ed4..b109bb9868 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B,C_with_tree-view=false.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B,C_with_tree-view=false.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, @@ -87,7 +87,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 3, "system.order": 3000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B,C_with_tree-view=true.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B,C_with_tree-view=true.res.golden.json index 17e85a4f68..08be1f8209 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B,C_with_tree-view=true.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B,C_with_tree-view=true.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, @@ -92,7 +92,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 3, "system.order": 3000, "system.remote_item_id": null, @@ -183,7 +183,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B_with_tree-view=false.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B_with_tree-view=false.res.golden.json index fbd5ef5078..7236dd8fe2 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B_with_tree-view=false.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B_with_tree-view=false.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B_with_tree-view=true.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B_with_tree-view=true.res.golden.json index 2233bc212d..72a795d278 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B_with_tree-view=true.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/B_with_tree-view=true.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, @@ -94,7 +94,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/C_with_tree-view=false.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/C_with_tree-view=false.res.golden.json index 56c1d1ef7e..000b46bc63 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/C_with_tree-view=false.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/C_with_tree-view=false.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 3, "system.order": 3000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/C_with_tree-view=true.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/C_with_tree-view=true.res.golden.json index 7a79ecf888..68ae5cf9d2 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/C_with_tree-view=true.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/C_with_tree-view=true.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 3, "system.order": 3000, "system.remote_item_id": null, @@ -94,7 +94,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, @@ -178,7 +178,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/D_with_tree-view=false.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/D_with_tree-view=false.res.golden.json index 17a977f1db..09c6828f4b 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/D_with_tree-view=false.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/D_with_tree-view=false.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 4, "system.order": 4000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/D_with_tree-view=true.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/D_with_tree-view=true.res.golden.json index a0ad2636b7..df14302710 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/D_with_tree-view=true.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/D_with_tree-view=true.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 4, "system.order": 4000, "system.remote_item_id": null, @@ -94,7 +94,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/E_with_tree-view=false.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/E_with_tree-view=false.res.golden.json index 0b5f1e9a78..03eecab3a4 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/E_with_tree-view=false.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/E_with_tree-view=false.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 5, "system.order": 5000, "system.remote_item_id": null, diff --git a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/E_with_tree-view=true.res.golden.json b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/E_with_tree-view=true.res.golden.json index a4206f1b0a..01822afa04 100755 --- a/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/E_with_tree-view=true.res.golden.json +++ b/controller/test-files/search/show/in_topology_A-B-C_and_A-D_search_for/E_with_tree-view=true.res.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 5, "system.order": 5000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/list_children/ok.res.payload.golden.json b/controller/test-files/work_item/list_children/ok.res.payload.golden.json index 100a831b56..42236cf583 100755 --- a/controller/test-files/work_item/list_children/ok.res.payload.golden.json +++ b/controller/test-files/work_item/list_children/ok.res.payload.golden.json @@ -3,7 +3,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 3, "system.order": 3000, "system.remote_item_id": null, @@ -86,7 +86,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/show/ok.has_children.res.payload.golden.json b/controller/test-files/work_item/show/ok.has_children.res.payload.golden.json index 745cbc3033..60b55911ca 100755 --- a/controller/test-files/work_item/show/ok.has_children.res.payload.golden.json +++ b/controller/test-files/work_item/show/ok.has_children.res.payload.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/show/ok.has_no_children.res.payload.golden.json b/controller/test-files/work_item/show/ok.has_no_children.res.payload.golden.json index 5e1a4d7740..e8914714c9 100755 --- a/controller/test-files/work_item/show/ok.has_no_children.res.payload.golden.json +++ b/controller/test-files/work_item/show/ok.has_no_children.res.payload.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/area/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/area/valid_sample_1.res.payload.golden.json index 3af17f4afc..e02de52582 100755 --- a/controller/test-files/work_item/update/area/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/area/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "area_field": "00000000-0000-0000-0000-000000000001", "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/boardcolumn/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/boardcolumn/valid_sample_1.res.payload.golden.json index 6e7259eebe..1af9186cf3 100755 --- a/controller/test-files/work_item/update/boardcolumn/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/boardcolumn/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "boardcolumn_field": "00000000-0000-0000-0000-000000000001", "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/bool/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/bool/valid_sample_1.res.payload.golden.json index 3ae0717d1b..bfd377073d 100755 --- a/controller/test-files/work_item/update/bool/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/bool/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "bool_field": false, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 3, "system.order": 3000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/codebase/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/codebase/valid_sample_1.res.payload.golden.json index 433a216e8f..66dd2954bb 100755 --- a/controller/test-files/work_item/update/codebase/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/codebase/valid_sample_1.res.payload.golden.json @@ -9,7 +9,7 @@ "codebaseid": "dunno" }, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 4, "system.order": 4000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/duration/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/duration/valid_sample_1.res.payload.golden.json index d818e2d632..100f036563 100755 --- a/controller/test-files/work_item/update/duration/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/duration/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "duration_field": 300000000, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 5, "system.order": 5000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/duration/valid_sample_2.res.payload.golden.json b/controller/test-files/work_item/update/duration/valid_sample_2.res.payload.golden.json index 04b18ce4be..d31598d654 100755 --- a/controller/test-files/work_item/update/duration/valid_sample_2.res.payload.golden.json +++ b/controller/test-files/work_item/update/duration/valid_sample_2.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "duration_field": -5400000000000, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 5, "system.order": 5000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/float/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/float/valid_sample_1.res.payload.golden.json index 7eaff9e31f..f54b95595a 100755 --- a/controller/test-files/work_item/update/float/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/float/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "float_field": -1111.1, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 6, "system.order": 6000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/float/valid_sample_2.res.payload.golden.json b/controller/test-files/work_item/update/float/valid_sample_2.res.payload.golden.json index 3225056d12..cadc2e7a7e 100755 --- a/controller/test-files/work_item/update/float/valid_sample_2.res.payload.golden.json +++ b/controller/test-files/work_item/update/float/valid_sample_2.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "float_field": 555.2, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 6, "system.order": 6000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/instant/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/instant/valid_sample_1.res.payload.golden.json index 3dc9acba48..9bf43b268d 100755 --- a/controller/test-files/work_item/update/instant/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/instant/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "instant_field": "0001-01-01T00:00:00Z", "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 7, "system.order": 7000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/integer/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/integer/valid_sample_1.res.payload.golden.json index b751f0c603..6791aa423f 100755 --- a/controller/test-files/work_item/update/integer/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/integer/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "integer_field": 333, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 8, "system.order": 8000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/integer/valid_sample_2.res.payload.golden.json b/controller/test-files/work_item/update/integer/valid_sample_2.res.payload.golden.json index c70334c63e..58120a3138 100755 --- a/controller/test-files/work_item/update/integer/valid_sample_2.res.payload.golden.json +++ b/controller/test-files/work_item/update/integer/valid_sample_2.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "integer_field": -100, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 8, "system.order": 8000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/iteration/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/iteration/valid_sample_1.res.payload.golden.json index 2765e10eae..33fe81df34 100755 --- a/controller/test-files/work_item/update/iteration/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/iteration/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "iteration_field": "00000000-0000-0000-0000-000000000001", "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 9, "system.order": 9000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/iteration/valid_sample_2.res.payload.golden.json b/controller/test-files/work_item/update/iteration/valid_sample_2.res.payload.golden.json index 8d6e7f010b..d453f59071 100755 --- a/controller/test-files/work_item/update/iteration/valid_sample_2.res.payload.golden.json +++ b/controller/test-files/work_item/update/iteration/valid_sample_2.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "iteration_field": "", "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 9, "system.order": 9000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/label/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/label/valid_sample_1.res.payload.golden.json index ef6730fa3b..10e65686db 100755 --- a/controller/test-files/work_item/update/label/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/label/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "label_field": "00000000-0000-0000-0000-000000000001", "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 10, "system.order": 10000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/label/valid_sample_2.res.payload.golden.json b/controller/test-files/work_item/update/label/valid_sample_2.res.payload.golden.json index 6f3c4cb7b3..2b3e6b4bf0 100755 --- a/controller/test-files/work_item/update/label/valid_sample_2.res.payload.golden.json +++ b/controller/test-files/work_item/update/label/valid_sample_2.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "label_field": "", "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 10, "system.order": 10000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/markup/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/markup/valid_sample_1.res.payload.golden.json index a9e9d889af..80f1673a2a 100755 --- a/controller/test-files/work_item/update/markup/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/markup/valid_sample_1.res.payload.golden.json @@ -6,7 +6,7 @@ "markup": "PlainText" }, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 11, "system.order": 11000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/markup/valid_sample_2.res.payload.golden.json b/controller/test-files/work_item/update/markup/valid_sample_2.res.payload.golden.json index b96867d2b5..24eb97cad9 100755 --- a/controller/test-files/work_item/update/markup/valid_sample_2.res.payload.golden.json +++ b/controller/test-files/work_item/update/markup/valid_sample_2.res.payload.golden.json @@ -6,7 +6,7 @@ "markup": "Markdown" }, "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 11, "system.order": 11000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/string/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/string/valid_sample_1.res.payload.golden.json index 991b2b7535..c48debcc24 100755 --- a/controller/test-files/work_item/update/string/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/string/valid_sample_1.res.payload.golden.json @@ -3,7 +3,7 @@ "attributes": { "string_field": "bar", "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 12, "system.order": 12000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/url/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/url/valid_sample_1.res.payload.golden.json index ba9847b4f3..4c30dee002 100755 --- a/controller/test-files/work_item/update/url/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/url/valid_sample_1.res.payload.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 13, "system.order": 13000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/url/valid_sample_2.res.payload.golden.json b/controller/test-files/work_item/update/url/valid_sample_2.res.payload.golden.json index ec6956b7cf..51456459d5 100755 --- a/controller/test-files/work_item/update/url/valid_sample_2.res.payload.golden.json +++ b/controller/test-files/work_item/update/url/valid_sample_2.res.payload.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 13, "system.order": 13000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/url/valid_sample_3.res.payload.golden.json b/controller/test-files/work_item/update/url/valid_sample_3.res.payload.golden.json index 80d18b6bee..8aa5b96c76 100755 --- a/controller/test-files/work_item/update/url/valid_sample_3.res.payload.golden.json +++ b/controller/test-files/work_item/update/url/valid_sample_3.res.payload.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 13, "system.order": 13000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/user/valid_sample_1.res.payload.golden.json b/controller/test-files/work_item/update/user/valid_sample_1.res.payload.golden.json index 47beaf2887..3171783ba1 100755 --- a/controller/test-files/work_item/update/user/valid_sample_1.res.payload.golden.json +++ b/controller/test-files/work_item/update/user/valid_sample_1.res.payload.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 14, "system.order": 14000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item/update/user/valid_sample_2.res.payload.golden.json b/controller/test-files/work_item/update/user/valid_sample_2.res.payload.golden.json index 0757d8184a..113361c637 100755 --- a/controller/test-files/work_item/update/user/valid_sample_2.res.payload.golden.json +++ b/controller/test-files/work_item/update/user/valid_sample_2.res.payload.golden.json @@ -2,7 +2,7 @@ "data": { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 14, "system.order": 14000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item_link/create/ok.res.payload.golden.json b/controller/test-files/work_item_link/create/ok.res.payload.golden.json index 57f182dce3..6c4656484c 100755 --- a/controller/test-files/work_item_link/create/ok.res.payload.golden.json +++ b/controller/test-files/work_item_link/create/ok.res.payload.golden.json @@ -90,7 +90,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, @@ -170,7 +170,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/controller/test-files/work_item_link/list/ok.res.paylpad.golden.json b/controller/test-files/work_item_link/list/ok.res.paylpad.golden.json index 8a79b621b6..65e726bc93 100755 --- a/controller/test-files/work_item_link/list/ok.res.paylpad.golden.json +++ b/controller/test-files/work_item_link/list/ok.res.paylpad.golden.json @@ -92,7 +92,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1, "system.remote_item_id": null, @@ -172,7 +172,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2, "system.remote_item_id": null, diff --git a/controller/test-files/work_item_link/show/ok.res.payload.golden.json b/controller/test-files/work_item_link/show/ok.res.payload.golden.json index 57f182dce3..6c4656484c 100755 --- a/controller/test-files/work_item_link/show/ok.res.payload.golden.json +++ b/controller/test-files/work_item_link/show/ok.res.payload.golden.json @@ -90,7 +90,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, @@ -170,7 +170,7 @@ { "attributes": { "system.created_at": "0001-01-01T00:00:00Z", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 2, "system.order": 2000, "system.remote_item_id": null, diff --git a/spacetemplate/assets/base.yaml b/spacetemplate/assets/base.yaml index 303f6f45a2..3e718a641a 100644 --- a/spacetemplate/assets/base.yaml +++ b/spacetemplate/assets/base.yaml @@ -50,7 +50,7 @@ work_item_types: "system.metastate": label: Meta State description: The meta-state of the work item - read_only: yes + read_only: no required: no type: simple_type: @@ -58,7 +58,7 @@ work_item_types: base_type: kind: string # This will allow other WITs to overwrite the values of the state. - rewritable_values: no + rewritable_values: yes # the sequence of the values need to match the sequence of the # system.state attributes. This encapsulates the mapping. values: diff --git a/test/testfixture/make_functions.go b/test/testfixture/make_functions.go index a21c4b914d..514d523bc6 100644 --- a/test/testfixture/make_functions.go +++ b/test/testfixture/make_functions.go @@ -384,32 +384,32 @@ func makeWorkItemBoards(fxt *TestFixture) error { ID: uuid.NewV4(), Name: testsupport.CreateRandomValidTestName("New"), Order: 0, - TransRuleKey: "updateStateFromColumnMove", - TransRuleArgument: "{ 'metastate': 'mNew' }", + TransRuleKey: "BidirectionalStateToColumn", + TransRuleArgument: "{ \"metaState\": \"mNew\" }", BoardID: fxt.WorkItemBoards[i].ID, }, { ID: uuid.NewV4(), Name: testsupport.CreateRandomValidTestName("In Progress"), Order: 1, - TransRuleKey: "updateStateFromColumnMove", - TransRuleArgument: "{ 'metastate': 'mInprogress' }", + TransRuleKey: "BidirectionalStateToColumn", + TransRuleArgument: "{ \"metaState\": \"mInprogress\" }", BoardID: fxt.WorkItemBoards[i].ID, }, { ID: uuid.NewV4(), Name: testsupport.CreateRandomValidTestName("Resolved"), Order: 2, - TransRuleKey: "updateStateFromColumnMove", - TransRuleArgument: "{ 'metastate': 'mResolved' }", + TransRuleKey: "BidirectionalStateToColumn", + TransRuleArgument: "{ \"metaState\": \"mResolved\" }", BoardID: fxt.WorkItemBoards[i].ID, }, { ID: uuid.NewV4(), Name: testsupport.CreateRandomValidTestName("Approved"), Order: 3, - TransRuleKey: "updateStateFromColumnMove", - TransRuleArgument: "{ 'metastate': 'mResolved' }", + TransRuleKey: "BidirectionalStateToColumn", + TransRuleArgument: "{ \"metaState\": \"mResolved\" }", BoardID: fxt.WorkItemBoards[i].ID, }, } diff --git a/workitem/workitemtype.go b/workitem/workitemtype.go index 491d315da0..3d902dbf01 100644 --- a/workitem/workitemtype.go +++ b/workitem/workitemtype.go @@ -26,6 +26,7 @@ const ( SystemDescriptionMarkup = "system.description.markup" SystemDescriptionRendered = "system.description.rendered" SystemState = "system.state" + SystemMetaState = "system.metastate" SystemAssignees = "system.assignees" SystemCreator = "system.creator" SystemCreatedAt = "system.created_at" From 88fdfe9f25c8c64d81fecfd059bad43d34815f41 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 17 Aug 2018 16:40:45 +0200 Subject: [PATCH 09/33] fix(actions): Enabled state rule. --- actions/actions.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/actions/actions.go b/actions/actions.go index a2db8f80e8..c568074aaf 100644 --- a/actions/actions.go +++ b/actions/actions.go @@ -72,14 +72,12 @@ func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uu Ctx: ctx, UserID: &userID, }, actionConfig, newContext, contextChanges, &actionChanges) - /* commented out for now until this rule is added case rules.ActionKeyStateToMetastate: newContext, actionChanges, err = executeAction(rules.ActionStateToMetaState{ Db: db, Ctx: ctx, UserID: &userID, }, actionConfig, newContext, contextChanges, &actionChanges) - */ default: return nil, nil, errs.New("action key " + actionKey + " is unknown") } From 532ca2f8e107f5a0898e838d99182c132d7c71ee Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 20 Aug 2018 13:24:49 +0200 Subject: [PATCH 10/33] Fixed formatting. --- actions/rules/action_state_to_metastate.go | 4 ++-- actions/rules/action_state_to_metastate_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 51431d7fe0..47cbfe2139 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -1,9 +1,9 @@ package rules import ( - "github.com/fabric8-services/fabric8-wit/actions/change" "context" "encoding/json" + "github.com/fabric8-services/fabric8-wit/actions/change" errs "github.com/pkg/errors" uuid "github.com/satori/go.uuid" @@ -511,4 +511,4 @@ func (act ActionStateToMetaState) OnStateChange(newContext change.Detector, cont } // and return to sender. return resultingWorkItem, changes, nil -} \ No newline at end of file +} diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go index 718f1ee14f..28f6b027ae 100644 --- a/actions/rules/action_state_to_metastate_test.go +++ b/actions/rules/action_state_to_metastate_test.go @@ -333,4 +333,4 @@ func (s *ActionStateToMetastateSuite) TestActionExecution() { require.Equal(t, workitem.SystemStateInProgress, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) }) -} \ No newline at end of file +} From fa75f5109b0cc853f4780972ef41be33e10a3061 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 20 Aug 2018 14:21:20 +0200 Subject: [PATCH 11/33] Fix documentation. --- actions/doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/doc.go b/actions/doc.go index 3f062fc6c1..8b66759ee7 100644 --- a/actions/doc.go +++ b/actions/doc.go @@ -3,7 +3,7 @@ Package actions system is a key component for process automation in WIT. It prov way of executing user-configurable, dynamic process steps depending on user settings, schema settings and events in the WIT. -The idea here is to provide a simple, yet powerful "signal-slot" system that +The idea here is to provide a simple, yet powerful "publish-subscribe" system that can connect any "event" in the system to any "action" with a clear decoupling of events and actions with the goal of making the associations later dynamic and configurable by the user ("user connects this event to this action"). Think From 9aab75608dcdc2dfbecbeecb1cc978bb2b9c7d64 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 20 Aug 2018 16:36:03 +0200 Subject: [PATCH 12/33] Fixed WIT. --- workitem/workitemtype.go | 1 - 1 file changed, 1 deletion(-) diff --git a/workitem/workitemtype.go b/workitem/workitemtype.go index c4ca676428..3635b51669 100644 --- a/workitem/workitemtype.go +++ b/workitem/workitemtype.go @@ -26,7 +26,6 @@ const ( SystemDescriptionMarkup = "system.description.markup" SystemDescriptionRendered = "system.description.rendered" SystemState = "system.state" - SystemMetaState = "system.metastate" SystemAssignees = "system.assignees" SystemCreator = "system.creator" SystemCreatedAt = "system.created_at" From 94ca51e5b3d2629fb61703a15233a02ef389d52b Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 20 Aug 2018 20:32:46 +0200 Subject: [PATCH 13/33] Fixed golden files. --- .../work_item/update/workitem_type.res.payload.golden.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/test-files/work_item/update/workitem_type.res.payload.golden.json b/controller/test-files/work_item/update/workitem_type.res.payload.golden.json index 58462c84e9..f5c8afb263 100755 --- a/controller/test-files/work_item/update/workitem_type.res.payload.golden.json +++ b/controller/test-files/work_item/update/workitem_type.res.payload.golden.json @@ -9,7 +9,7 @@ "system.description": "```\nMissing fields in workitem type: Second WorkItem Type\n\nType1 Assigned To : First User (jon_doe), Second User (lorem_ipsum)\nType1 bar : hello\nType1 fooBar : open\nType1 integer-or-float-list : 101\nType1 reporter : First User (jon_doe)\n```\ndescription1\n", "system.description.markup": "Markdown", "system.description.rendered": "\u003cpre\u003e\u003ccode class=\"prettyprint\"\u003e\u003cspan class=\"typ\"\u003eMissing\u003c/span\u003e \u003cspan class=\"pln\"\u003efields\u003c/span\u003e \u003cspan class=\"kwd\"\u003ein\u003c/span\u003e \u003cspan class=\"pln\"\u003eworkitem\u003c/span\u003e \u003cspan class=\"kwd\"\u003etype\u003c/span\u003e\u003cspan class=\"pun\"\u003e:\u003c/span\u003e \u003cspan class=\"typ\"\u003eSecond\u003c/span\u003e \u003cspan class=\"typ\"\u003eWorkItem\u003c/span\u003e \u003cspan class=\"typ\"\u003eType\u003c/span\u003e\n\n\u003cspan class=\"typ\"\u003eType1\u003c/span\u003e \u003cspan class=\"typ\"\u003eAssigned\u003c/span\u003e \u003cspan class=\"typ\"\u003eTo\u003c/span\u003e \u003cspan class=\"pun\"\u003e:\u003c/span\u003e \u003cspan class=\"typ\"\u003eFirst\u003c/span\u003e \u003cspan class=\"typ\"\u003eUser\u003c/span\u003e \u003cspan class=\"pun\"\u003e(\u003c/span\u003e\u003cspan class=\"pln\"\u003ejon_doe\u003c/span\u003e\u003cspan class=\"pun\"\u003e)\u003c/span\u003e\u003cspan class=\"pun\"\u003e,\u003c/span\u003e \u003cspan class=\"typ\"\u003eSecond\u003c/span\u003e \u003cspan class=\"typ\"\u003eUser\u003c/span\u003e \u003cspan class=\"pun\"\u003e(\u003c/span\u003e\u003cspan class=\"pln\"\u003elorem_ipsum\u003c/span\u003e\u003cspan class=\"pun\"\u003e)\u003c/span\u003e\n\u003cspan class=\"typ\"\u003eType1\u003c/span\u003e \u003cspan class=\"pln\"\u003ebar\u003c/span\u003e \u003cspan class=\"pun\"\u003e:\u003c/span\u003e \u003cspan class=\"pln\"\u003ehello\u003c/span\u003e\n\u003cspan class=\"typ\"\u003eType1\u003c/span\u003e \u003cspan class=\"pln\"\u003efooBar\u003c/span\u003e \u003cspan class=\"pun\"\u003e:\u003c/span\u003e \u003cspan class=\"pln\"\u003eopen\u003c/span\u003e\n\u003cspan class=\"typ\"\u003eType1\u003c/span\u003e \u003cspan class=\"pln\"\u003einteger\u003c/span\u003e\u003cspan class=\"pun\"\u003e-\u003c/span\u003e\u003cspan class=\"kwd\"\u003eor\u003c/span\u003e\u003cspan class=\"pun\"\u003e-\u003c/span\u003e\u003cspan class=\"kwd\"\u003efloat\u003c/span\u003e\u003cspan class=\"pun\"\u003e-\u003c/span\u003e\u003cspan class=\"pln\"\u003elist\u003c/span\u003e \u003cspan class=\"pun\"\u003e:\u003c/span\u003e \u003cspan class=\"dec\"\u003e101\u003c/span\u003e\n\u003cspan class=\"typ\"\u003eType1\u003c/span\u003e \u003cspan class=\"pln\"\u003ereporter\u003c/span\u003e \u003cspan class=\"pun\"\u003e:\u003c/span\u003e \u003cspan class=\"typ\"\u003eFirst\u003c/span\u003e \u003cspan class=\"typ\"\u003eUser\u003c/span\u003e \u003cspan class=\"pun\"\u003e(\u003c/span\u003e\u003cspan class=\"pln\"\u003ejon_doe\u003c/span\u003e\u003cspan class=\"pun\"\u003e)\u003c/span\u003e\n\u003c/code\u003e\u003c/pre\u003e\n\n\u003cp\u003edescription1\u003c/p\u003e\n", - "system.metastate": null, + "system.metastate": "mNew", "system.number": 1, "system.order": 1000, "system.remote_item_id": null, From e832f8bd9574b4d055c36b2cf6a64122bf4bf414 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 24 Aug 2018 14:27:14 +0200 Subject: [PATCH 14/33] Fixed PR comments. --- actions/actions.go | 30 ------ actions/rules/action_state_to_metastate.go | 104 ++++++--------------- 2 files changed, 29 insertions(+), 105 deletions(-) diff --git a/actions/actions.go b/actions/actions.go index c568074aaf..f217f5235b 100644 --- a/actions/actions.go +++ b/actions/actions.go @@ -1,35 +1,5 @@ package actions -/* - The actions system is a key component for process automation in WIT. It provides - a way of executing user-configurable, dynamic process steps depending on user - settings, schema settings and events in the WIT. - - The idea here is to provide a simple, yet powerful "publish-subscribe" system that - can connect any "event" in the system to any "action" with a clear decoupling - of events and actions with the goal of making the associations later dynamic and - configurable by the user ("user connects this event to this action"). Think - of a "IFTTT for WIT" (https://en.wikipedia.org/wiki/IFTTT). - - Actions are generic and atomic execution steps that do exactly one task and - are configurable. The actions system around the actions provide a key-based - execution of the actions. - - Some examples for an application of this system would be: - - closing all children of a parent WI that is being closed (the user connects the - "close" attribute change event of a WI to an action that closes all WIs of - a matching query). - - sending out notifications for mentions on markdown (the system executes an - action "send notification" for every mention found in markdown values). - - moving all WIs from one iteration to the next in the time sequence when - the original iteration is closed. - - For all these automations, the actions system provides a re-usable, flexible - and later user configurable way of doing that without creating lots of - custom code and/or custom process implementations that are hardcoded in the - WIT. -*/ - import ( "context" diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 47cbfe2139..2b00350502 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -71,12 +71,6 @@ func (act ActionStateToMetaState) difference(old []interface{}, new []interface{ } func (act ActionStateToMetaState) loadWorkItemBoardsBySpaceID(spaceID uuid.UUID) ([]*workitem.Board, error) { - if act.Ctx == nil { - return nil, errs.New("context is nil") - } - if act.Db == nil { - return nil, errs.New("database is nil") - } space, err := act.Db.Spaces().Load(act.Ctx, spaceID) if err != nil { return nil, errs.Wrap(err, "error loading space: "+err.Error()) @@ -88,45 +82,7 @@ func (act ActionStateToMetaState) loadWorkItemBoardsBySpaceID(spaceID uuid.UUID) return boards, nil } -func (act ActionStateToMetaState) loadWorkItemTypeGroupsBySpaceID(spaceID uuid.UUID) ([]*workitem.WorkItemTypeGroup, error) { - if act.Ctx == nil { - return nil, errs.New("context is nil") - } - if act.Db == nil { - return nil, errs.New("database is nil") - } - space, err := act.Db.Spaces().Load(act.Ctx, spaceID) - if err != nil { - return nil, errs.Wrap(err, "error loading space: "+err.Error()) - } - groups, err := act.Db.WorkItemTypeGroups().List(act.Ctx, space.SpaceTemplateID) - if err != nil { - return nil, errs.Wrap(err, "error loading work item type: "+err.Error()) - } - return groups, nil -} - -func (act ActionStateToMetaState) loadWorkItemTypeByID(id uuid.UUID) (*workitem.WorkItemType, error) { - if act.Ctx == nil { - return nil, errs.New("context is nil") - } - if act.Db == nil { - return nil, errs.New("database is nil") - } - wit, err := act.Db.WorkItemTypes().Load(act.Ctx, id) - if err != nil { - return nil, errs.Wrap(err, "error loading work item type: "+err.Error()) - } - return wit, nil -} - func (act ActionStateToMetaState) loadWorkItemByID(id uuid.UUID) (*workitem.WorkItem, error) { - if act.Ctx == nil { - return nil, errs.New("context is nil") - } - if act.Db == nil { - return nil, errs.New("database is nil") - } wi, err := act.Db.WorkItems().LoadByID(act.Ctx, id) if err != nil { return nil, errs.Wrap(err, "error loading work item: "+err.Error()) @@ -135,15 +91,6 @@ func (act ActionStateToMetaState) loadWorkItemByID(id uuid.UUID) (*workitem.Work } func (act ActionStateToMetaState) storeWorkItem(workitem *workitem.WorkItem) (*workitem.WorkItem, error) { - if act.Ctx == nil { - return nil, errs.New("context is nil") - } - if act.Db == nil { - return nil, errs.New("database is nil") - } - if act.UserID == nil { - return nil, errs.New("userID is nil") - } err := application.Transactional(act.Db, func(appl application.Application) error { var err error workitem, err = appl.WorkItems().Save(act.Ctx, workitem.SpaceID, *workitem, *act.UserID) @@ -158,30 +105,31 @@ func (act ActionStateToMetaState) storeWorkItem(workitem *workitem.WorkItem) (*w return workitem, nil } -func (act ActionStateToMetaState) getValueListFromFieldType(wit *workitem.WorkItemType, fieldName string) ([]interface{}, error) { - fieldType := wit.Fields[fieldName].Type - switch t := fieldType.(type) { +func (act ActionStateToMetaState) getValueListFromEnumField(wit *workitem.WorkItemType, fieldName string) ([]interface{}, error) { + enumFieldType := wit.Fields[fieldName].Type + switch t := enumFieldType.(type) { case workitem.EnumType: return t.Values, nil } return nil, errs.New("given field on workitemtype " + wit.ID.String() + " is not an enum field: " + fieldName) } +// getStateToMetastateMap returns the mapping from state to metastate values read from the template. func (act ActionStateToMetaState) getStateToMetastateMap(workitemTypeID uuid.UUID) (map[string]string, error) { - wit, err := act.loadWorkItemTypeByID(workitemTypeID) + wit, err := act.Db.WorkItemTypes().Load(act.Ctx, workitemTypeID) if err != nil { return nil, err } - stateList, err := act.getValueListFromFieldType(wit, workitem.SystemState) + stateList, err := act.getValueListFromEnumField(wit, workitem.SystemState) if err != nil { return nil, err } - metastateList, err := act.getValueListFromFieldType(wit, workitem.SystemMetaState) + metastateList, err := act.getValueListFromEnumField(wit, workitem.SystemMetaState) if err != nil { return nil, err } if len(stateList) != len(metastateList) { - return nil, errs.New("inconsistent number of states and metatstates in the current work item type (must be equal)") + return nil, errs.Errorf("inconsistent number of states and metatstates in the current work item type (must be equal): %d states != %d metastates", len(stateList), len(metastateList)) } stateToMetastateMap := make(map[string]string) for idx := range stateList { @@ -193,13 +141,9 @@ func (act ActionStateToMetaState) getStateToMetastateMap(workitemTypeID uuid.UUI if !ok { return nil, errs.New("metastate value in value list is not of type string") } - // this is important: we only add a new entry if there - // is not an entry already existing. Therefore, we're only - // using the first mapping, satisfying the req for getting the - // first matching metastate for a state. - // this is done here even if the usecase where we have - // multiple states with the same value (duplicate states) - // might be not that common. + // only the first state<->metatstate mapping needs to be taken as + // per definition. So we're only adding a new entry here if there + // is not an entry already existing. if _, ok := stateToMetastateMap[thisState]; !ok { stateToMetastateMap[thisState] = thisMetastate } @@ -207,6 +151,7 @@ func (act ActionStateToMetaState) getStateToMetastateMap(workitemTypeID uuid.UUI return stateToMetastateMap, nil } +// getMetastateToStateMap returns the mapping from metastate to state values read from the template. func (act ActionStateToMetaState) getMetastateToStateMap(workitemTypeID uuid.UUID) (map[string]string, error) { stateToMetastate, err := act.getStateToMetastateMap(workitemTypeID) if err != nil { @@ -247,8 +192,17 @@ func (act ActionStateToMetaState) fuseChanges(c1 change.Set, c2 change.Set) *cha return &c1 } -// OnChange executes the action rule. +// OnChange executes the action rule. It implements rules.Action. func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { + if act.Ctx == nil { + return nil, nil, errs.New("context is nil") + } + if act.Db == nil { + return nil, nil, errs.New("database is nil") + } + if act.UserID == nil { + return nil, nil, errs.New("userID is nil") + } if actionChanges == nil { return nil, nil, errs.New("given actionChanges is nil") } @@ -261,10 +215,10 @@ func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextCh var executionChanges change.Set for _, change := range contextChanges { if change.AttributeName == workitem.SystemState { - newContext, executionChanges, err = act.OnStateChange(newContext, contextChanges, configuration, actionChanges) + newContext, executionChanges, err = act.onStateChange(newContext, contextChanges, configuration, actionChanges) } if change.AttributeName == workitem.SystemBoardcolumns { - newContext, executionChanges, err = act.OnBoardColumnsChange(newContext, contextChanges, configuration, actionChanges) + newContext, executionChanges, err = act.onBoardColumnsChange(newContext, contextChanges, configuration, actionChanges) } if err != nil { return nil, nil, err @@ -275,8 +229,8 @@ func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextCh return newContext, *actionChanges, nil } -// OnBoardColumnsChange is executed when the columns change. It eventually updates the metastate and the state. -func (act ActionStateToMetaState) OnBoardColumnsChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { +// onBoardColumnsChange is executed when the columns change. It eventually updates the metastate and the state. +func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { // we already assume that the rule applies, this needs to be checked in the controller. // there is no additional check on the rule key. wi, ok := newContext.(workitem.WorkItem) @@ -377,8 +331,8 @@ func (act ActionStateToMetaState) OnBoardColumnsChange(newContext change.Detecto return resultingWorkItem, changes, nil } -// OnStateChange is executed when the state changes. It eventually updates the metastate and the boardcolumns. -func (act ActionStateToMetaState) OnStateChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { +// onStateChange is executed when the state changes. It eventually updates the metastate and the boardcolumns. +func (act ActionStateToMetaState) onStateChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { wi, ok := newContext.(workitem.WorkItem) if !ok { return nil, nil, errs.New("given context is not a WorkItem instance") @@ -407,7 +361,7 @@ func (act ActionStateToMetaState) OnStateChange(newContext change.Detector, cont return nil, nil, err } // next, check which boards are relevant for this WI. - groups, err := act.loadWorkItemTypeGroupsBySpaceID(wi.SpaceID) + groups, err := act.Db.WorkItemTypeGroups().List(act.Ctx, wi.SpaceID) if err != nil { return nil, nil, err } From 5b4d19cf4bb52ae0161b49d944a3464001aa234c Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 27 Aug 2018 14:20:17 +0200 Subject: [PATCH 15/33] Fixed group loading. --- actions/rules/action_state_to_metastate.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 2b00350502..3ab5dd2cf0 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -93,7 +93,7 @@ func (act ActionStateToMetaState) loadWorkItemByID(id uuid.UUID) (*workitem.Work func (act ActionStateToMetaState) storeWorkItem(workitem *workitem.WorkItem) (*workitem.WorkItem, error) { err := application.Transactional(act.Db, func(appl application.Application) error { var err error - workitem, err = appl.WorkItems().Save(act.Ctx, workitem.SpaceID, *workitem, *act.UserID) + workitem, _, err = appl.WorkItems().Save(act.Ctx, workitem.SpaceID, *workitem, *act.UserID) if err != nil { return errs.Wrap(err, "error updating work item") } @@ -361,9 +361,13 @@ func (act ActionStateToMetaState) onStateChange(newContext change.Detector, cont return nil, nil, err } // next, check which boards are relevant for this WI. - groups, err := act.Db.WorkItemTypeGroups().List(act.Ctx, wi.SpaceID) + space, err := act.Db.Spaces().Load(act.Ctx, wi.SpaceID) if err != nil { - return nil, nil, err + return nil, nil, errs.Wrap(err, "error loading space: "+err.Error()) + } + groups, err := act.Db.WorkItemTypeGroups().List(act.Ctx, space.SpaceTemplateID) + if err != nil { + return nil, nil, errs.Wrap(err, "error loading type groups: "+err.Error()) } var relevantBoards []*workitem.Board for _, board := range boards { From 47643222e63e2c1c70ce1b57f610b2cd33c90172 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 27 Aug 2018 15:32:19 +0200 Subject: [PATCH 16/33] Used id.Slice. --- actions/rules/action_state_to_metastate.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 3ab5dd2cf0..52fc0d9c42 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "github.com/fabric8-services/fabric8-wit/actions/change" + "github.com/fabric8-services/fabric8-wit/id" errs "github.com/pkg/errors" uuid "github.com/satori/go.uuid" @@ -22,6 +23,7 @@ type ActionStateToMetaState struct { // make sure the rule is implementing the interface. var _ Action = ActionStateToMetaState{} +/* func (act ActionStateToMetaState) containsUUID(s []uuid.UUID, e uuid.UUID) bool { for _, a := range s { if a == e { @@ -30,6 +32,7 @@ func (act ActionStateToMetaState) containsUUID(s []uuid.UUID, e uuid.UUID) bool } return false } +*/ func (act ActionStateToMetaState) contains(s []interface{}, e interface{}) bool { for _, a := range s { @@ -380,7 +383,8 @@ func (act ActionStateToMetaState) onStateChange(newContext change.Detector, cont return nil, nil, err } for _, group := range groups { - if group.ID == thisBoardContext && act.containsUUID(group.TypeList, wi.Type) { + var typeListSlice id.Slice = group.TypeList + if group.ID == thisBoardContext && typeListSlice.Contains(wi.Type) { // this board is relevant. relevantBoards = append(relevantBoards, board) } From feecf1c6516ea161c05a91182563ba6d822033bd Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 27 Aug 2018 15:34:08 +0200 Subject: [PATCH 17/33] Fixed error reporting. --- actions/rules/action_state_to_metastate.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 52fc0d9c42..6fdfd4ca55 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -366,11 +366,11 @@ func (act ActionStateToMetaState) onStateChange(newContext change.Detector, cont // next, check which boards are relevant for this WI. space, err := act.Db.Spaces().Load(act.Ctx, wi.SpaceID) if err != nil { - return nil, nil, errs.Wrap(err, "error loading space: "+err.Error()) + return nil, nil, errs.Wrap(err, "error loading space") } groups, err := act.Db.WorkItemTypeGroups().List(act.Ctx, space.SpaceTemplateID) if err != nil { - return nil, nil, errs.Wrap(err, "error loading type groups: "+err.Error()) + return nil, nil, errs.Wrap(err, "error loading type groups") } var relevantBoards []*workitem.Board for _, board := range boards { From 4b6acc61dc01512fe29772db14607bcdd2913628 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 27 Aug 2018 16:04:13 +0200 Subject: [PATCH 18/33] Added test for contains. Fixes memory leak. --- actions/rules/action_state_to_metastate.go | 14 ++++++- .../rules/action_state_to_metastate_test.go | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 6fdfd4ca55..085dfe0c26 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -46,11 +46,23 @@ func (act ActionStateToMetaState) contains(s []interface{}, e interface{}) bool func (act ActionStateToMetaState) removeElement(s []interface{}, e interface{}) []interface{} { for idx, a := range s { if a == e { - s = append(s[:idx], s[idx+1:]...) + copy(s[idx:], s[idx+1:]) + s[len(s)-1] = nil + s = s[:len(s)-1] // we don't return here as there may be multiple copies of e in s. } } return s + + /* + for idx, a := range s { + if a == e { + s = append(s[:idx], s[idx+1:]...) + // we don't return here as there may be multiple copies of e in s. + } + } + return s + */ } // difference returns the differences between two slices. It returns two slices containing diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go index 28f6b027ae..ab12841428 100644 --- a/actions/rules/action_state_to_metastate_test.go +++ b/actions/rules/action_state_to_metastate_test.go @@ -23,6 +23,47 @@ type ActionStateToMetastateSuite struct { gormtestsupport.DBTestSuite } +func (s *ActionStateToMetastateSuite) TestContainsElement() { + s.T().Run("contains an element", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + // there is no other way of creating an []interface{}. + // but we have plenty of memory, so be it. + var a []interface{} + a = append(a, 0) + a = append(a, 1) + a = append(a, 2) + a = append(a, 3) + a = append(a, 2) + require.True(t, action.contains(a, 2)) + require.True(t, action.contains(a, 3)) + require.True(t, action.contains(a, 0)) + }) + s.T().Run("not contains an element", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + // there is no other way of creating an []interface{}. + // but we have plenty of memory, so be it. + var a []interface{} + a = append(a, 0) + a = append(a, 1) + a = append(a, 2) + a = append(a, 3) + a = append(a, 2) + require.False(t, action.contains(a, 4)) + require.False(t, action.contains(a, nil)) + require.False(t, action.contains(a, "foo")) + }) +} + func (s *ActionStateToMetastateSuite) TestActionExecution() { // Note on the fixture: by default, the created board is attached to the type group // with the same index. The columns on each board are as follows: From 35e603400b4769be888722b5a75f18c0c9aad457 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 27 Aug 2018 16:10:14 +0200 Subject: [PATCH 19/33] Added test for removeElement. --- actions/rules/action_state_to_metastate.go | 21 --------- .../rules/action_state_to_metastate_test.go | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 085dfe0c26..7464d4b0e4 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -23,17 +23,6 @@ type ActionStateToMetaState struct { // make sure the rule is implementing the interface. var _ Action = ActionStateToMetaState{} -/* -func (act ActionStateToMetaState) containsUUID(s []uuid.UUID, e uuid.UUID) bool { - for _, a := range s { - if a == e { - return true - } - } - return false -} -*/ - func (act ActionStateToMetaState) contains(s []interface{}, e interface{}) bool { for _, a := range s { if a == e { @@ -53,16 +42,6 @@ func (act ActionStateToMetaState) removeElement(s []interface{}, e interface{}) } } return s - - /* - for idx, a := range s { - if a == e { - s = append(s[:idx], s[idx+1:]...) - // we don't return here as there may be multiple copies of e in s. - } - } - return s - */ } // difference returns the differences between two slices. It returns two slices containing diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go index ab12841428..0f9ac3df90 100644 --- a/actions/rules/action_state_to_metastate_test.go +++ b/actions/rules/action_state_to_metastate_test.go @@ -64,6 +64,53 @@ func (s *ActionStateToMetastateSuite) TestContainsElement() { }) } +func (s *ActionStateToMetastateSuite) TestRemoveElement() { + s.T().Run("removing an existing element", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + // there is no other way of creating an []interface{}. + // but we have plenty of memory, so be it. + var a []interface{} + a = append(a, 0) + a = append(a, 1) + a = append(a, 2) + a = append(a, 3) + a = append(a, 2) + a = action.removeElement(a, 2) + require.Len(t, a, 3) + require.False(t, action.contains(a, 2)) + a = action.removeElement(a, 1) + require.Len(t, a, 2) + require.False(t, action.contains(a, 1)) + }) + s.T().Run("removing a non-existing element", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + // there is no other way of creating an []interface{}. + // but we have plenty of memory, so be it. + var a []interface{} + a = append(a, 0) + a = append(a, 1) + a = append(a, 2) + a = append(a, 3) + a = append(a, 2) + a = action.removeElement(a, 4) + require.Len(t, a, 5) + require.True(t, action.contains(a, 0)) + require.True(t, action.contains(a, 1)) + require.True(t, action.contains(a, 2)) + require.True(t, action.contains(a, 3)) + }) +} + func (s *ActionStateToMetastateSuite) TestActionExecution() { // Note on the fixture: by default, the created board is attached to the type group // with the same index. The columns on each board are as follows: From d867145d20d15824a896d20858306042e2865136 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 27 Aug 2018 16:35:24 +0200 Subject: [PATCH 20/33] Added removeElement() test. --- .../rules/action_state_to_metastate_test.go | 102 ++++++++++++++++-- 1 file changed, 93 insertions(+), 9 deletions(-) diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go index 0f9ac3df90..6650279a97 100644 --- a/actions/rules/action_state_to_metastate_test.go +++ b/actions/rules/action_state_to_metastate_test.go @@ -1,6 +1,7 @@ package rules import ( + "fmt" "testing" "github.com/fabric8-services/fabric8-wit/actions/change" @@ -23,6 +24,18 @@ type ActionStateToMetastateSuite struct { gormtestsupport.DBTestSuite } +func ArrayEquals(a []interface{}, b []interface{}) bool { + if len(a) != len(b) { + return false + } + for i, v := range a { + if v != b[i] { + return false + } + } + return true +} + func (s *ActionStateToMetastateSuite) TestContainsElement() { s.T().Run("contains an element", func(t *testing.T) { fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) @@ -80,12 +93,23 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { a = append(a, 2) a = append(a, 3) a = append(a, 2) - a = action.removeElement(a, 2) - require.Len(t, a, 3) - require.False(t, action.contains(a, 2)) a = action.removeElement(a, 1) - require.Len(t, a, 2) - require.False(t, action.contains(a, 1)) + var expected []interface{} + expected = append(expected, 0) + expected = append(expected, 2) + expected = append(expected, 3) + expected = append(expected, 2) + require.Len(t, a, 4) + require.True(t, ArrayEquals(expected, a)) + a = action.removeElement(a, 3) + expected = []interface{}{} + expected = append(expected, 0) + expected = append(expected, 2) + expected = append(expected, 2) + require.Len(t, a, 3) + fmt.Println(expected) + fmt.Println(a) + require.True(t, ArrayEquals(expected, a)) }) s.T().Run("removing a non-existing element", func(t *testing.T) { fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) @@ -104,10 +128,70 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { a = append(a, 2) a = action.removeElement(a, 4) require.Len(t, a, 5) - require.True(t, action.contains(a, 0)) - require.True(t, action.contains(a, 1)) - require.True(t, action.contains(a, 2)) - require.True(t, action.contains(a, 3)) + var expected []interface{} + expected = append(expected, 0) + expected = append(expected, 1) + expected = append(expected, 2) + expected = append(expected, 3) + expected = append(expected, 2) + require.True(t, ArrayEquals(expected, a)) + }) + s.T().Run("removing a duplicate element", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + // there is no other way of creating an []interface{}. + // but we have plenty of memory, so be it. + var a []interface{} + a = append(a, 0) + a = append(a, 1) + a = append(a, 2) + a = append(a, 3) + a = append(a, 2) + a = action.removeElement(a, 2) + var expected []interface{} + expected = append(expected, 0) + expected = append(expected, 1) + expected = append(expected, 3) + require.Len(t, a, 3) + require.True(t, ArrayEquals(expected, a)) + }) +} + +func (s *ActionStateToMetastateSuite) TestDifference() { + s.T().Run("finding differences", func(t *testing.T) { + fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) + action := ActionStateToMetaState{ + Db: s.GormDB, + Ctx: s.Ctx, + UserID: &fxt.Identities[0].ID, + } + // there is no other way of creating an []interface{}. + // but we have plenty of memory, so be it. + var a []interface{} + a = append(a, 0) + a = append(a, 1) + a = append(a, 2) + a = append(a, 3) + a = append(a, 2) + var b []interface{} + b = append(b, 2) + b = append(b, 3) + b = append(b, 5) + added, removed := action.difference(a, b) + require.Len(t, added, 1) + require.Len(t, removed, 2) + // wasting plenty more memory here + var expectedAdded []interface{} + expectedAdded = append(expectedAdded, 5) + var expectedRemoved []interface{} + expectedRemoved = append(expectedRemoved, 0) + expectedRemoved = append(expectedRemoved, 1) + require.True(t, ArrayEquals(added, expectedAdded)) + require.True(t, ArrayEquals(removed, expectedRemoved)) }) } From f7585496797b753df8365f60d5822728019dba6b Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 27 Aug 2018 17:02:58 +0200 Subject: [PATCH 21/33] Fixed slice creation. --- .../rules/action_state_to_metastate_test.go | 82 +++---------------- 1 file changed, 11 insertions(+), 71 deletions(-) diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go index 6650279a97..b14ad324ec 100644 --- a/actions/rules/action_state_to_metastate_test.go +++ b/actions/rules/action_state_to_metastate_test.go @@ -44,14 +44,7 @@ func (s *ActionStateToMetastateSuite) TestContainsElement() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - // there is no other way of creating an []interface{}. - // but we have plenty of memory, so be it. - var a []interface{} - a = append(a, 0) - a = append(a, 1) - a = append(a, 2) - a = append(a, 3) - a = append(a, 2) + a := []interface{}{0, 1, 2, 3, 2} require.True(t, action.contains(a, 2)) require.True(t, action.contains(a, 3)) require.True(t, action.contains(a, 0)) @@ -63,14 +56,7 @@ func (s *ActionStateToMetastateSuite) TestContainsElement() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - // there is no other way of creating an []interface{}. - // but we have plenty of memory, so be it. - var a []interface{} - a = append(a, 0) - a = append(a, 1) - a = append(a, 2) - a = append(a, 3) - a = append(a, 2) + a := []interface{}{0, 1, 2, 3, 2} require.False(t, action.contains(a, 4)) require.False(t, action.contains(a, nil)) require.False(t, action.contains(a, "foo")) @@ -85,27 +71,13 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - // there is no other way of creating an []interface{}. - // but we have plenty of memory, so be it. - var a []interface{} - a = append(a, 0) - a = append(a, 1) - a = append(a, 2) - a = append(a, 3) - a = append(a, 2) + a := []interface{}{0, 1, 2, 3, 2} a = action.removeElement(a, 1) - var expected []interface{} - expected = append(expected, 0) - expected = append(expected, 2) - expected = append(expected, 3) - expected = append(expected, 2) + expected := []interface{}{0, 2, 3, 2} require.Len(t, a, 4) require.True(t, ArrayEquals(expected, a)) a = action.removeElement(a, 3) - expected = []interface{}{} - expected = append(expected, 0) - expected = append(expected, 2) - expected = append(expected, 2) + expected = []interface{}{0, 2, 2} require.Len(t, a, 3) fmt.Println(expected) fmt.Println(a) @@ -118,22 +90,10 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - // there is no other way of creating an []interface{}. - // but we have plenty of memory, so be it. - var a []interface{} - a = append(a, 0) - a = append(a, 1) - a = append(a, 2) - a = append(a, 3) - a = append(a, 2) + a := []interface{}{0, 1, 2, 3, 2} a = action.removeElement(a, 4) require.Len(t, a, 5) - var expected []interface{} - expected = append(expected, 0) - expected = append(expected, 1) - expected = append(expected, 2) - expected = append(expected, 3) - expected = append(expected, 2) + expected := []interface{}{0, 1, 2, 3, 2} require.True(t, ArrayEquals(expected, a)) }) s.T().Run("removing a duplicate element", func(t *testing.T) { @@ -143,19 +103,9 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - // there is no other way of creating an []interface{}. - // but we have plenty of memory, so be it. - var a []interface{} - a = append(a, 0) - a = append(a, 1) - a = append(a, 2) - a = append(a, 3) - a = append(a, 2) + a := []interface{}{0, 1, 2, 3, 2} a = action.removeElement(a, 2) - var expected []interface{} - expected = append(expected, 0) - expected = append(expected, 1) - expected = append(expected, 3) + expected := []interface{}{0, 1, 3} require.Len(t, a, 3) require.True(t, ArrayEquals(expected, a)) }) @@ -169,18 +119,8 @@ func (s *ActionStateToMetastateSuite) TestDifference() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - // there is no other way of creating an []interface{}. - // but we have plenty of memory, so be it. - var a []interface{} - a = append(a, 0) - a = append(a, 1) - a = append(a, 2) - a = append(a, 3) - a = append(a, 2) - var b []interface{} - b = append(b, 2) - b = append(b, 3) - b = append(b, 5) + a := []interface{}{0, 1, 2, 3, 2} + b := []interface{}{2, 3, 5} added, removed := action.difference(a, b) require.Len(t, added, 1) require.Len(t, removed, 2) From 649ef2ac324232ac6bad955e887dfc62479dd10b Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 27 Aug 2018 17:06:41 +0200 Subject: [PATCH 22/33] Removed debug code. --- actions/rules/action_state_to_metastate_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go index b14ad324ec..12d57fe177 100644 --- a/actions/rules/action_state_to_metastate_test.go +++ b/actions/rules/action_state_to_metastate_test.go @@ -79,8 +79,6 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { a = action.removeElement(a, 3) expected = []interface{}{0, 2, 2} require.Len(t, a, 3) - fmt.Println(expected) - fmt.Println(a) require.True(t, ArrayEquals(expected, a)) }) s.T().Run("removing a non-existing element", func(t *testing.T) { From 58f5187d15626f33ee0ff26adc2cd7cc7298e089 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Mon, 27 Aug 2018 17:15:49 +0200 Subject: [PATCH 23/33] Fixed equals. --- .../rules/action_state_to_metastate_test.go | 25 +++++-------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go index 12d57fe177..221971b5b2 100644 --- a/actions/rules/action_state_to_metastate_test.go +++ b/actions/rules/action_state_to_metastate_test.go @@ -1,7 +1,6 @@ package rules import ( - "fmt" "testing" "github.com/fabric8-services/fabric8-wit/actions/change" @@ -24,18 +23,6 @@ type ActionStateToMetastateSuite struct { gormtestsupport.DBTestSuite } -func ArrayEquals(a []interface{}, b []interface{}) bool { - if len(a) != len(b) { - return false - } - for i, v := range a { - if v != b[i] { - return false - } - } - return true -} - func (s *ActionStateToMetastateSuite) TestContainsElement() { s.T().Run("contains an element", func(t *testing.T) { fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) @@ -75,11 +62,11 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { a = action.removeElement(a, 1) expected := []interface{}{0, 2, 3, 2} require.Len(t, a, 4) - require.True(t, ArrayEquals(expected, a)) + require.Equal(t, expected, a) a = action.removeElement(a, 3) expected = []interface{}{0, 2, 2} require.Len(t, a, 3) - require.True(t, ArrayEquals(expected, a)) + require.Equal(t, expected, a) }) s.T().Run("removing a non-existing element", func(t *testing.T) { fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) @@ -92,7 +79,7 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { a = action.removeElement(a, 4) require.Len(t, a, 5) expected := []interface{}{0, 1, 2, 3, 2} - require.True(t, ArrayEquals(expected, a)) + require.Equal(t, expected, a) }) s.T().Run("removing a duplicate element", func(t *testing.T) { fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) @@ -105,7 +92,7 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { a = action.removeElement(a, 2) expected := []interface{}{0, 1, 3} require.Len(t, a, 3) - require.True(t, ArrayEquals(expected, a)) + require.Equal(t, expected, a) }) } @@ -128,8 +115,8 @@ func (s *ActionStateToMetastateSuite) TestDifference() { var expectedRemoved []interface{} expectedRemoved = append(expectedRemoved, 0) expectedRemoved = append(expectedRemoved, 1) - require.True(t, ArrayEquals(added, expectedAdded)) - require.True(t, ArrayEquals(removed, expectedRemoved)) + require.Equal(t, added, expectedAdded) + require.Equal(t, removed, expectedRemoved) }) } From 973fe2d754e230dedb7d01f396f4b7abec17a260 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Tue, 28 Aug 2018 10:51:35 +0200 Subject: [PATCH 24/33] Fixed isDirty structural inconsistency in onStateChange(). --- actions/rules/action_state_to_metastate.go | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 7464d4b0e4..d3c7f81fe4 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -186,7 +186,7 @@ func (act ActionStateToMetaState) fuseChanges(c1 change.Set, c2 change.Set) *cha return &c1 } -// OnChange executes the action rule. It implements rules.Action. +// OnChange executes the action rule. It implements rules.Action. Returns the Work Item with eventual changes applied. func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { if act.Ctx == nil { return nil, nil, errs.New("context is nil") @@ -223,7 +223,7 @@ func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextCh return newContext, *actionChanges, nil } -// onBoardColumnsChange is executed when the columns change. It eventually updates the metastate and the state. +// onBoardColumnsChange is executed when the columns change. It eventually updates the metastate and the state. Returns the Work Item with eventual changes applied. func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { // we already assume that the rule applies, this needs to be checked in the controller. // there is no additional check on the rule key. @@ -325,7 +325,7 @@ func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detecto return resultingWorkItem, changes, nil } -// onStateChange is executed when the state changes. It eventually updates the metastate and the boardcolumns. +// onStateChange is executed when the state changes. It eventually updates the metastate and the boardcolumns. Returns the Work Item with eventual changes applied. func (act ActionStateToMetaState) onStateChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { wi, ok := newContext.(workitem.WorkItem) if !ok { @@ -336,9 +336,6 @@ func (act ActionStateToMetaState) onStateChange(newContext change.Detector, cont if err != nil { return nil, nil, err } - // create a dirty flag. Note that we can not use len(actionChanges) as this - // may contain previous changes from the action chain. - wiDirty := false // update the workitem accordingly. wiState := wi.Fields[workitem.SystemState].(string) if wi.Fields[workitem.SystemMetaState] == mapping[wiState] { @@ -348,7 +345,6 @@ func (act ActionStateToMetaState) onStateChange(newContext change.Detector, cont // otherwise, update the metastate from the state. changes := act.addOrUpdateChange(*actionChanges, workitem.SystemMetaState, wi.Fields[workitem.SystemMetaState], mapping[wiState]) wi.Fields[workitem.SystemMetaState] = mapping[wiState] - wiDirty = true // next, get the columns of the workitem and see if these needs to be updated. boards, err := act.loadWorkItemBoardsBySpaceID(wi.SpaceID) if err != nil { @@ -455,12 +451,10 @@ func (act ActionStateToMetaState) onStateChange(newContext change.Detector, cont } // finally, store the new work item state if something changed. var resultingWorkItem workitem.WorkItem - if wiDirty { - result, err := act.storeWorkItem(&wi) - resultingWorkItem = *result - if err != nil { - return nil, nil, err - } + result, err := act.storeWorkItem(&wi) + resultingWorkItem = *result + if err != nil { + return nil, nil, err } // and return to sender. return resultingWorkItem, changes, nil From 69d1af491dbdfc4a1c94e6f168120eb6e0a12a1e Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Tue, 28 Aug 2018 12:15:34 +0200 Subject: [PATCH 25/33] Fixed err check. --- actions/rules/action_state_to_metastate.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index d3c7f81fe4..b6834a9534 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -217,7 +217,7 @@ func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextCh if err != nil { return nil, nil, err } - actionChanges = act.fuseChanges(*actionChanges, executionChanges) + actionChanges = &act.fuseChanges(*actionChanges, executionChanges) } // return the result return newContext, *actionChanges, nil @@ -450,12 +450,10 @@ func (act ActionStateToMetaState) onStateChange(newContext change.Detector, cont changes = act.addOrUpdateChange(changes, workitem.SystemBoardcolumns, oldColumnsConfig, wi.Fields[workitem.SystemBoardcolumns]) } // finally, store the new work item state if something changed. - var resultingWorkItem workitem.WorkItem result, err := act.storeWorkItem(&wi) - resultingWorkItem = *result if err != nil { return nil, nil, err } // and return to sender. - return resultingWorkItem, changes, nil + return *result, changes, nil } From 9a8d624c41cc3e4e796de287f421ad0f55a40ca8 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Tue, 28 Aug 2018 12:17:07 +0200 Subject: [PATCH 26/33] Fixed leftover bug. --- actions/rules/action_state_to_metastate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index b6834a9534..d4bde780d9 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -217,7 +217,7 @@ func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextCh if err != nil { return nil, nil, err } - actionChanges = &act.fuseChanges(*actionChanges, executionChanges) + actionChanges = act.fuseChanges(*actionChanges, executionChanges) } // return the result return newContext, *actionChanges, nil From 5001aa26822bf53e6006bfc929dbd0bf61a3a512 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Tue, 28 Aug 2018 12:42:02 +0200 Subject: [PATCH 27/33] Changed return type on fuseChanges. --- actions/rules/action_state_to_metastate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index d4bde780d9..66c4db2cbd 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -179,11 +179,11 @@ func (act ActionStateToMetaState) addOrUpdateChange(changes change.Set, attribut return newChanges } -func (act ActionStateToMetaState) fuseChanges(c1 change.Set, c2 change.Set) *change.Set { +func (act ActionStateToMetaState) fuseChanges(c1 change.Set, c2 change.Set) change.Set { for _, change := range c2 { c1 = act.addOrUpdateChange(c1, change.AttributeName, change.OldValue, change.NewValue) } - return &c1 + return c1 } // OnChange executes the action rule. It implements rules.Action. Returns the Work Item with eventual changes applied. @@ -217,7 +217,7 @@ func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextCh if err != nil { return nil, nil, err } - actionChanges = act.fuseChanges(*actionChanges, executionChanges) + *actionChanges = act.fuseChanges(*actionChanges, executionChanges) } // return the result return newContext, *actionChanges, nil From 7dafce4c888749769f60e83ae43b750d5a746389 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Tue, 28 Aug 2018 15:45:22 +0200 Subject: [PATCH 28/33] Fixed tests, removed action instances. --- .../rules/action_state_to_metastate_test.go | 48 +++---------------- 1 file changed, 7 insertions(+), 41 deletions(-) diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go index 221971b5b2..a789c6efcc 100644 --- a/actions/rules/action_state_to_metastate_test.go +++ b/actions/rules/action_state_to_metastate_test.go @@ -25,24 +25,14 @@ type ActionStateToMetastateSuite struct { func (s *ActionStateToMetastateSuite) TestContainsElement() { s.T().Run("contains an element", func(t *testing.T) { - fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) - action := ActionStateToMetaState{ - Db: s.GormDB, - Ctx: s.Ctx, - UserID: &fxt.Identities[0].ID, - } + action := ActionStateToMetaState{} a := []interface{}{0, 1, 2, 3, 2} require.True(t, action.contains(a, 2)) require.True(t, action.contains(a, 3)) require.True(t, action.contains(a, 0)) }) s.T().Run("not contains an element", func(t *testing.T) { - fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) - action := ActionStateToMetaState{ - Db: s.GormDB, - Ctx: s.Ctx, - UserID: &fxt.Identities[0].ID, - } + action := ActionStateToMetaState{} a := []interface{}{0, 1, 2, 3, 2} require.False(t, action.contains(a, 4)) require.False(t, action.contains(a, nil)) @@ -52,44 +42,26 @@ func (s *ActionStateToMetastateSuite) TestContainsElement() { func (s *ActionStateToMetastateSuite) TestRemoveElement() { s.T().Run("removing an existing element", func(t *testing.T) { - fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) - action := ActionStateToMetaState{ - Db: s.GormDB, - Ctx: s.Ctx, - UserID: &fxt.Identities[0].ID, - } a := []interface{}{0, 1, 2, 3, 2} - a = action.removeElement(a, 1) + a = ActionStateToMetaState{}.removeElement(a, 1) expected := []interface{}{0, 2, 3, 2} require.Len(t, a, 4) require.Equal(t, expected, a) - a = action.removeElement(a, 3) + a = ActionStateToMetaState{}.removeElement(a, 3) expected = []interface{}{0, 2, 2} require.Len(t, a, 3) require.Equal(t, expected, a) }) s.T().Run("removing a non-existing element", func(t *testing.T) { - fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) - action := ActionStateToMetaState{ - Db: s.GormDB, - Ctx: s.Ctx, - UserID: &fxt.Identities[0].ID, - } a := []interface{}{0, 1, 2, 3, 2} - a = action.removeElement(a, 4) + a = ActionStateToMetaState{}.removeElement(a, 4) require.Len(t, a, 5) expected := []interface{}{0, 1, 2, 3, 2} require.Equal(t, expected, a) }) s.T().Run("removing a duplicate element", func(t *testing.T) { - fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) - action := ActionStateToMetaState{ - Db: s.GormDB, - Ctx: s.Ctx, - UserID: &fxt.Identities[0].ID, - } a := []interface{}{0, 1, 2, 3, 2} - a = action.removeElement(a, 2) + a = ActionStateToMetaState{}.removeElement(a, 2) expected := []interface{}{0, 1, 3} require.Len(t, a, 3) require.Equal(t, expected, a) @@ -98,15 +70,9 @@ func (s *ActionStateToMetastateSuite) TestRemoveElement() { func (s *ActionStateToMetastateSuite) TestDifference() { s.T().Run("finding differences", func(t *testing.T) { - fxt := tf.NewTestFixture(t, s.DB, tf.CreateWorkItemEnvironment()) - action := ActionStateToMetaState{ - Db: s.GormDB, - Ctx: s.Ctx, - UserID: &fxt.Identities[0].ID, - } a := []interface{}{0, 1, 2, 3, 2} b := []interface{}{2, 3, 5} - added, removed := action.difference(a, b) + added, removed := ActionStateToMetaState{}.difference(a, b) require.Len(t, added, 1) require.Len(t, removed, 2) // wasting plenty more memory here From 6f4d7fb4cb87a56977c4a88509cef74edaa56491 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 31 Aug 2018 13:47:08 +0200 Subject: [PATCH 29/33] Fixed empty return bug. --- actions/rules/action_state_to_metastate.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 66c4db2cbd..70c94d422b 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -320,9 +320,11 @@ func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detecto if err != nil { return nil, nil, err } + // return to sender + return resultingWorkItem, changes, nil } // return to sender - return resultingWorkItem, changes, nil + return wi, changes, nil } // onStateChange is executed when the state changes. It eventually updates the metastate and the boardcolumns. Returns the Work Item with eventual changes applied. From 3cb93a214ab771f38f78bd615afb70cbf14d4c20 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 31 Aug 2018 13:50:50 +0200 Subject: [PATCH 30/33] Removed redundant code. --- actions/rules/action_state_to_metastate.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 70c94d422b..b326d3486a 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -313,15 +313,13 @@ func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detecto } } // finally, store the new work item state if something changed. - var resultingWorkItem workitem.WorkItem if wiDirty { result, err := act.storeWorkItem(&wi) - resultingWorkItem = *result if err != nil { return nil, nil, err } // return to sender - return resultingWorkItem, changes, nil + return *result, changes, nil } // return to sender return wi, changes, nil From f83ff16a62d11032d57fdd3d8fb19e17c1d928e4 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 31 Aug 2018 17:42:18 +0200 Subject: [PATCH 31/33] Updated OnChange() signature to not work with cbr. --- actions/actions.go | 8 +++--- actions/rules/action.go | 2 +- actions/rules/action_field_set.go | 6 ++-- actions/rules/action_field_set_test.go | 10 +++---- actions/rules/action_nil.go | 2 +- actions/rules/action_state_to_metastate.go | 22 +++++++-------- .../rules/action_state_to_metastate_test.go | 28 +++++++++---------- 7 files changed, 39 insertions(+), 39 deletions(-) diff --git a/actions/actions.go b/actions/actions.go index f217f5235b..59255473c7 100644 --- a/actions/actions.go +++ b/actions/actions.go @@ -35,19 +35,19 @@ func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uu actionConfig := actionConfigs[actionKey] switch actionKey { case rules.ActionKeyNil: - newContext, actionChanges, err = executeAction(rules.ActionNil{}, actionConfig, newContext, contextChanges, &actionChanges) + newContext, actionChanges, err = executeAction(rules.ActionNil{}, actionConfig, newContext, contextChanges, actionChanges) case rules.ActionKeyFieldSet: newContext, actionChanges, err = executeAction(rules.ActionFieldSet{ Db: db, Ctx: ctx, UserID: &userID, - }, actionConfig, newContext, contextChanges, &actionChanges) + }, actionConfig, newContext, contextChanges, actionChanges) case rules.ActionKeyStateToMetastate: newContext, actionChanges, err = executeAction(rules.ActionStateToMetaState{ Db: db, Ctx: ctx, UserID: &userID, - }, actionConfig, newContext, contextChanges, &actionChanges) + }, actionConfig, newContext, contextChanges, actionChanges) default: return nil, nil, errs.New("action key " + actionKey + " is unknown") } @@ -61,7 +61,7 @@ func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uu // executeAction executes the action given. The actionChanges contain the changes made by // prior action executions. The execution is expected to add/update their changes on this // change set. -func executeAction(act rules.Action, configuration string, newContext change.Detector, contextChanges change.Set, actionChanges *change.Set) (change.Detector, change.Set, error) { +func executeAction(act rules.Action, configuration string, newContext change.Detector, contextChanges change.Set, actionChanges change.Set) (change.Detector, change.Set, error) { if act == nil { return nil, nil, errs.New("rule can not be nil") } diff --git a/actions/rules/action.go b/actions/rules/action.go index db7b7de4a6..d87bff003f 100644 --- a/actions/rules/action.go +++ b/actions/rules/action.go @@ -24,5 +24,5 @@ type Action interface { // updated attributes. It returns the new context. Note that this // needs the new (after change) context and the old value(s) as // part of the changeset. - OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) + OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges change.Set) (change.Detector, change.Set, error) } diff --git a/actions/rules/action_field_set.go b/actions/rules/action_field_set.go index c38b3f0a59..4411c228d1 100644 --- a/actions/rules/action_field_set.go +++ b/actions/rules/action_field_set.go @@ -52,7 +52,7 @@ func (act ActionFieldSet) storeWorkItem(wi *workitem.WorkItem) (*workitem.WorkIt } // OnChange executes the action rule. -func (act ActionFieldSet) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { +func (act ActionFieldSet) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges change.Set) (change.Detector, change.Set, error) { // check if the newContext is a WorkItem, fail otherwise. wiContext, ok := newContext.(workitem.WorkItem) if !ok { @@ -76,7 +76,7 @@ func (act ActionFieldSet) OnChange(newContext change.Detector, contextChanges ch if !ok { return nil, nil, errs.New("unknown field name: " + k) } - *actionChanges = append(*actionChanges, change.Change{ + actionChanges = append(actionChanges, change.Change{ AttributeName: k, NewValue: v, OldValue: wiContext.Fields[k], @@ -100,5 +100,5 @@ func (act ActionFieldSet) OnChange(newContext change.Detector, contextChanges ch return nil, nil, errs.New("field attribute unknown: " + k) } } - return *actionResultContext, *actionChanges, nil + return *actionResultContext, actionChanges, nil } diff --git a/actions/rules/action_field_set_test.go b/actions/rules/action_field_set_test.go index e1a26f443f..ad630402eb 100644 --- a/actions/rules/action_field_set_test.go +++ b/actions/rules/action_field_set_test.go @@ -56,7 +56,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { } var convertChanges change.Set // Not using constants here intentionally. - afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", &convertChanges) + afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 1) require.Equal(t, workitem.SystemState, convertChanges[0].AttributeName) @@ -79,7 +79,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { } var convertChanges change.Set // Not using constants here intentionally. - afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", &convertChanges) + afterActionWI, convertChanges, err := action.OnChange(newVersion, contextChanges, "{ \"system.state\": \"resolved\" }", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 1) require.Equal(t, workitem.SystemState, convertChanges[0].AttributeName) @@ -87,7 +87,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { require.Equal(t, workitem.SystemStateResolved, convertChanges[0].NewValue) require.Equal(t, workitem.SystemStateResolved, afterActionWI.(workitem.WorkItem).Fields[workitem.SystemState]) // doing another change, the convertChange needs to stack. - afterActionWI, convertChanges, err = action.OnChange(afterActionWI, change.Set{}, "{ \"system.state\": \"new\" }", &convertChanges) + afterActionWI, convertChanges, err = action.OnChange(afterActionWI, change.Set{}, "{ \"system.state\": \"new\" }", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) require.Equal(t, workitem.SystemState, convertChanges[0].AttributeName) @@ -112,7 +112,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { UserID: &fxt.Identities[0].ID, } var convertChanges change.Set - _, _, err = action.OnChange(newVersion, contextChanges, "{ \"system.notavailable\": \"updatedState\" }", &convertChanges) + _, _, err = action.OnChange(newVersion, contextChanges, "{ \"system.notavailable\": \"updatedState\" }", convertChanges) require.NotNil(t, err) }) @@ -129,7 +129,7 @@ func (s *ActionFieldSetSuite) TestActionExecution() { UserID: &fxt.Identities[0].ID, } var convertChanges change.Set - _, convertChanges, err = action.OnChange(newVersion, contextChanges, "someNonJSON", &convertChanges) + _, convertChanges, err = action.OnChange(newVersion, contextChanges, "someNonJSON", convertChanges) require.NotNil(t, err) }) } diff --git a/actions/rules/action_nil.go b/actions/rules/action_nil.go index 8a005da3ee..56ab95e784 100644 --- a/actions/rules/action_nil.go +++ b/actions/rules/action_nil.go @@ -12,6 +12,6 @@ type ActionNil struct { var _ Action = ActionNil{} // OnChange executes the action rule. -func (act ActionNil) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { +func (act ActionNil) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges change.Set) (change.Detector, change.Set, error) { return newContext, nil, nil } diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index b326d3486a..abb154b868 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -187,7 +187,7 @@ func (act ActionStateToMetaState) fuseChanges(c1 change.Set, c2 change.Set) chan } // OnChange executes the action rule. It implements rules.Action. Returns the Work Item with eventual changes applied. -func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { +func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges change.Set) (change.Detector, change.Set, error) { if act.Ctx == nil { return nil, nil, errs.New("context is nil") } @@ -202,7 +202,7 @@ func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextCh } if len(contextChanges) == 0 { // no changes, just return what we have. - return newContext, *actionChanges, nil + return newContext, actionChanges, nil } // if we have multiple changes, this iterates over them and cherrypicks, fusing the results together. var err error @@ -217,14 +217,14 @@ func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextCh if err != nil { return nil, nil, err } - *actionChanges = act.fuseChanges(*actionChanges, executionChanges) + actionChanges = act.fuseChanges(actionChanges, executionChanges) } // return the result - return newContext, *actionChanges, nil + return newContext, actionChanges, nil } // onBoardColumnsChange is executed when the columns change. It eventually updates the metastate and the state. Returns the Work Item with eventual changes applied. -func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { +func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges change.Set) (change.Detector, change.Set, error) { // we already assume that the rule applies, this needs to be checked in the controller. // there is no additional check on the rule key. wi, ok := newContext.(workitem.WorkItem) @@ -253,7 +253,7 @@ func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detecto // only happening on *adding* a new column. Removing column does not trigger a change. if len(columnsAdded) == 0 { // somehow, no actual changes on the columns. - return newContext, *actionChanges, nil + return newContext, actionChanges, nil } // get the mapping. mapping, err := act.getMetastateToStateMap(wi.Type) @@ -285,7 +285,7 @@ func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detecto // this rule applies to the column. if thisColumn.TransRuleKey != ActionKeyStateToMetastate { // this is a column that does not apply to the rule, we don't apply here. - return newContext, *actionChanges, nil + return newContext, actionChanges, nil } // unmarshall the configuration. config := map[string]string{} @@ -298,7 +298,7 @@ func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detecto if metaState == wi.Fields[workitem.SystemMetaState] { // the WIs metastate is already the same as the columns // metastate, so nothing to do. - return newContext, *actionChanges, nil + return newContext, actionChanges, nil } // the metatstate changes, so set it on the WI. changes = act.addOrUpdateChange(changes, workitem.SystemMetaState, wi.Fields[workitem.SystemMetaState], metaState) @@ -326,7 +326,7 @@ func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detecto } // onStateChange is executed when the state changes. It eventually updates the metastate and the boardcolumns. Returns the Work Item with eventual changes applied. -func (act ActionStateToMetaState) onStateChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) { +func (act ActionStateToMetaState) onStateChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges change.Set) (change.Detector, change.Set, error) { wi, ok := newContext.(workitem.WorkItem) if !ok { return nil, nil, errs.New("given context is not a WorkItem instance") @@ -340,10 +340,10 @@ func (act ActionStateToMetaState) onStateChange(newContext change.Detector, cont wiState := wi.Fields[workitem.SystemState].(string) if wi.Fields[workitem.SystemMetaState] == mapping[wiState] { // metastate remains stable, nothing to do. - return newContext, *actionChanges, nil + return newContext, actionChanges, nil } // otherwise, update the metastate from the state. - changes := act.addOrUpdateChange(*actionChanges, workitem.SystemMetaState, wi.Fields[workitem.SystemMetaState], mapping[wiState]) + changes := act.addOrUpdateChange(actionChanges, workitem.SystemMetaState, wi.Fields[workitem.SystemMetaState], mapping[wiState]) wi.Fields[workitem.SystemMetaState] = mapping[wiState] // next, get the columns of the workitem and see if these needs to be updated. boards, err := act.loadWorkItemBoardsBySpaceID(wi.SpaceID) diff --git a/actions/rules/action_state_to_metastate_test.go b/actions/rules/action_state_to_metastate_test.go index a789c6efcc..1f7bed8904 100644 --- a/actions/rules/action_state_to_metastate_test.go +++ b/actions/rules/action_state_to_metastate_test.go @@ -137,9 +137,9 @@ func (s *ActionStateToMetastateSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges change.Set + convertChanges := change.Set{} // note: the rule does not use the explicit configuration, but reads from the template. - afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) // check metastate validity. @@ -177,9 +177,9 @@ func (s *ActionStateToMetastateSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges change.Set + convertChanges := change.Set{} // note: the rule does not use the explicit configuration, but reads from the template. - afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) // check metastate validity. @@ -222,9 +222,9 @@ func (s *ActionStateToMetastateSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges change.Set + convertChanges := change.Set{} // note: the rule does not use the explicit configuration, but reads from the template. - afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) // check metastate validity. @@ -261,9 +261,9 @@ func (s *ActionStateToMetastateSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges change.Set + convertChanges := change.Set{} // note: the rule does not use the explicit configuration, but reads from the template. - afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) // check metastate validity. @@ -298,9 +298,9 @@ func (s *ActionStateToMetastateSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges change.Set + convertChanges := change.Set{} // note: the rule does not use the explicit configuration, but reads from the template. - afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) // check metastate validity. @@ -339,9 +339,9 @@ func (s *ActionStateToMetastateSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges change.Set + convertChanges := change.Set{} // note: the rule does not use the explicit configuration, but reads from the template. - afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) // check metastate validity. @@ -379,9 +379,9 @@ func (s *ActionStateToMetastateSuite) TestActionExecution() { Ctx: s.Ctx, UserID: &fxt.Identities[0].ID, } - var convertChanges change.Set + convertChanges := change.Set{} // note: the rule does not use the explicit configuration, but reads from the template. - afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", &convertChanges) + afterActionWI, convertChanges, err := action.OnChange(*fxt.WorkItems[0], contextChanges, "", convertChanges) require.NoError(t, err) require.Len(t, convertChanges, 2) // check metastate validity. From 9f4de7d1f6ac59951e90c760b33fec1fb183e356 Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Fri, 31 Aug 2018 17:46:02 +0200 Subject: [PATCH 32/33] Removed pointer args. --- actions/rules/action_state_to_metastate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index abb154b868..53dafe52cf 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -99,7 +99,7 @@ func (act ActionStateToMetaState) storeWorkItem(workitem *workitem.WorkItem) (*w return workitem, nil } -func (act ActionStateToMetaState) getValueListFromEnumField(wit *workitem.WorkItemType, fieldName string) ([]interface{}, error) { +func (act ActionStateToMetaState) getValueListFromEnumField(wit workitem.WorkItemType, fieldName string) ([]interface{}, error) { enumFieldType := wit.Fields[fieldName].Type switch t := enumFieldType.(type) { case workitem.EnumType: @@ -114,11 +114,11 @@ func (act ActionStateToMetaState) getStateToMetastateMap(workitemTypeID uuid.UUI if err != nil { return nil, err } - stateList, err := act.getValueListFromEnumField(wit, workitem.SystemState) + stateList, err := act.getValueListFromEnumField(*wit, workitem.SystemState) if err != nil { return nil, err } - metastateList, err := act.getValueListFromEnumField(wit, workitem.SystemMetaState) + metastateList, err := act.getValueListFromEnumField(*wit, workitem.SystemMetaState) if err != nil { return nil, err } From 89c8cb4e990f9cec263f12181247fb863054effd Mon Sep 17 00:00:00 2001 From: Michael Kleinhenz Date: Thu, 6 Sep 2018 14:31:52 +0200 Subject: [PATCH 33/33] Fixed some minor issues. --- actions/rules/action_state_to_metastate.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/actions/rules/action_state_to_metastate.go b/actions/rules/action_state_to_metastate.go index 53dafe52cf..636f7c3652 100644 --- a/actions/rules/action_state_to_metastate.go +++ b/actions/rules/action_state_to_metastate.go @@ -67,11 +67,11 @@ func (act ActionStateToMetaState) difference(old []interface{}, new []interface{ func (act ActionStateToMetaState) loadWorkItemBoardsBySpaceID(spaceID uuid.UUID) ([]*workitem.Board, error) { space, err := act.Db.Spaces().Load(act.Ctx, spaceID) if err != nil { - return nil, errs.Wrap(err, "error loading space: "+err.Error()) + return nil, errs.Wrap(err, "error loading space") } boards, err := act.Db.Boards().List(act.Ctx, space.SpaceTemplateID) if err != nil { - return nil, errs.Wrap(err, "error loading work item type: "+err.Error()) + return nil, errs.Wrap(err, "error loading work item type") } return boards, nil } @@ -79,7 +79,7 @@ func (act ActionStateToMetaState) loadWorkItemBoardsBySpaceID(spaceID uuid.UUID) func (act ActionStateToMetaState) loadWorkItemByID(id uuid.UUID) (*workitem.WorkItem, error) { wi, err := act.Db.WorkItems().LoadByID(act.Ctx, id) if err != nil { - return nil, errs.Wrap(err, "error loading work item: "+err.Error()) + return nil, errs.Wrap(err, "error loading work item") } return wi, nil } @@ -197,9 +197,6 @@ func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextCh if act.UserID == nil { return nil, nil, errs.New("userID is nil") } - if actionChanges == nil { - return nil, nil, errs.New("given actionChanges is nil") - } if len(contextChanges) == 0 { // no changes, just return what we have. return newContext, actionChanges, nil