Skip to content
This repository was archived by the owner on Mar 11, 2021. It is now read-only.
Open
Changes from 3 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
353edeb
feat(actions): Actions system infra.
michaelkleinhenz Aug 17, 2018
6ea5c68
fix(tests): Fixed the tests.
michaelkleinhenz Aug 17, 2018
be9dae7
fix(doc): Fixed documentation.
michaelkleinhenz Aug 17, 2018
529be05
Merge remote-tracking branch 'upstream/master' into actions_infra
michaelkleinhenz Aug 17, 2018
008f7ff
Merge branch 'master' into actions_infra
michaelkleinhenz Aug 17, 2018
72ad9ce
Merge branch 'master' into actions_infra
michaelkleinhenz Aug 17, 2018
ab89650
fix(doc): Added doc.
michaelkleinhenz Aug 17, 2018
b7fa5c0
fix(code): Moved Change.
michaelkleinhenz Aug 17, 2018
5589d68
fix(code): Refactoring Change.Set.
michaelkleinhenz Aug 17, 2018
f625367
fix(formatting): Format fixed.
michaelkleinhenz Aug 17, 2018
00305cf
fix(staterule): Added state rule.
michaelkleinhenz Aug 17, 2018
88fdfe9
fix(actions): Enabled state rule.
michaelkleinhenz Aug 17, 2018
a60efd3
Merge remote-tracking branch 'upstream/master' into actions_staterule
michaelkleinhenz Aug 20, 2018
532ca2f
Fixed formatting.
michaelkleinhenz Aug 20, 2018
fa75f51
Fix documentation.
michaelkleinhenz Aug 20, 2018
13d3f53
Merge branch 'master' into actions_staterule
michaelkleinhenz Aug 20, 2018
9aab756
Fixed WIT.
michaelkleinhenz Aug 20, 2018
94ca51e
Fixed golden files.
michaelkleinhenz Aug 20, 2018
0edeacc
Merge remote-tracking branch 'upstream/master' into actions_staterule
michaelkleinhenz Aug 24, 2018
e832f8b
Fixed PR comments.
michaelkleinhenz Aug 24, 2018
13640c8
Merge branch 'master' into actions_staterule
michaelkleinhenz Aug 24, 2018
1ce8bd8
Merge remote-tracking branch 'upstream/master' into actions_staterule
michaelkleinhenz Aug 27, 2018
5b4d19c
Fixed group loading.
michaelkleinhenz Aug 27, 2018
4764322
Used id.Slice.
michaelkleinhenz Aug 27, 2018
feecf1c
Fixed error reporting.
michaelkleinhenz Aug 27, 2018
4b6acc6
Added test for contains. Fixes memory leak.
michaelkleinhenz Aug 27, 2018
35e6034
Added test for removeElement.
michaelkleinhenz Aug 27, 2018
d867145
Added removeElement() test.
michaelkleinhenz Aug 27, 2018
dab1b62
Merge remote-tracking branch 'upstream/master' into actions_staterule
michaelkleinhenz Aug 27, 2018
f758549
Fixed slice creation.
michaelkleinhenz Aug 27, 2018
649ef2a
Removed debug code.
michaelkleinhenz Aug 27, 2018
58f5187
Fixed equals.
michaelkleinhenz Aug 27, 2018
973fe2d
Fixed isDirty structural inconsistency in onStateChange().
michaelkleinhenz Aug 28, 2018
69d1af4
Fixed err check.
michaelkleinhenz Aug 28, 2018
9a8d624
Fixed leftover bug.
michaelkleinhenz Aug 28, 2018
5001aa2
Changed return type on fuseChanges.
michaelkleinhenz Aug 28, 2018
7dafce4
Fixed tests, removed action instances.
michaelkleinhenz Aug 28, 2018
595316f
Merge branch 'master' into actions_staterule
michaelkleinhenz Aug 29, 2018
6f4d7fb
Fixed empty return bug.
Aug 31, 2018
3cb93a2
Removed redundant code.
Aug 31, 2018
f83ff16
Updated OnChange() signature to not work with cbr.
Aug 31, 2018
9f4de7d
Removed pointer args.
Aug 31, 2018
5ff5380
Merge remote-tracking branch 'upstream/master' into actions_staterule
michaelkleinhenz Sep 6, 2018
89c8cb4
Fixed some minor issues.
michaelkleinhenz Sep 6, 2018
e71b346
Merge remote-tracking branch 'upstream/master' into actions_staterule
kwk Sep 11, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions actions/rules/action_state_to_metastate.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (act ActionStateToMetaState) loadWorkItemByID(id uuid.UUID) (*workitem.Work
func (act ActionStateToMetaState) storeWorkItem(workitem *workitem.WorkItem) (*workitem.WorkItem, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think appl.WorkItems().Save is good enough. No need to have another function to wrap it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I still have to wrap it into a Transactional, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think the way it is is easier to comprehend later.

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")
}
Expand Down Expand Up @@ -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())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to stringify the error here. Please get rid of err.Error(). You've already wrapped this error.

Copy link
Collaborator Author

@michaelkleinhenz michaelkleinhenz Aug 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in feecf1c

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is fine. But maybe it makes sense to specify which space could not be loaded?

Copy link
Collaborator Author

@michaelkleinhenz michaelkleinhenz Aug 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is contained in the wrapped error. It contains the ID that is not found in the message. I don't see a reason for repeating this.

}
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())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, please get rid of err.Error().

Copy link
Collaborator Author

@michaelkleinhenz michaelkleinhenz Aug 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in feecf1c

}
var relevantBoards []*workitem.Board
for _, board := range boards {
Expand Down