Fix so checkAction will be able to validate if the next action should be skipped or not #121
Conversation
|
Please if in future while merging also merge the https://github.com/Shuffle/shaffuru/pull/390 as it breaks one of the condition which could create a problem. |
| skippedParents := 0 | ||
| for _, parent := range parents[actionId] { | ||
| _, result := GetActionResult(ctx, *workflowExecution, parent) | ||
| if result.Status == "SKIPPED" { |
There was a problem hiding this comment.
Have you verified this works, both with a few skipped and a lot of them, along with some success, some failure etc?
This feels wrong, but I'm not sure why.
shared.go
Outdated
| log.Printf("ERROR %s", err) | ||
| } | ||
| copy(nextActions[index:], nextActions[index+1:]) | ||
| nextActions[len(nextActions)-1] = "" |
There was a problem hiding this comment.
This section is overcomplicated, and I'm worried about indexing issues. Did you test it well, both with a lot of items and 0? This seems like it can crash if there is only 1 item.
I typically make a new array entirely for stuff like this, as index modification is awful. Such as: Why would it be necessary to set a value to "" if you are just about to delete it? That doesn't seem necessary.
| CompletedAt: 0, | ||
| Status: "SKIPPED", | ||
| } | ||
| resultData, err := json.Marshal(newResult) |
There was a problem hiding this comment.
Add some space between each section. This is hard to read.
The simplest "rule" for easy readability and space is to have at least one newline at the end of a section, such as:
`if x == y {
}
if x2 == y2 {
}
|
@yashsinghcodes tell me when it's done please :) |
|
@frikky resolved the conflicts. Ready for merge. |
The approach is pretty simple we skip the actions if all the parents are skipped.
Now 3621997 fixes the issue where it was not related to child node skip but where the decide execution was not able to get the next action. Adding a validation with
CheckNextActionat the mainDecideExecutionshould already fix the problem as I found out during my tests but by makingCheckNextActionitself skips the action saves a lot of time in my opinion.@frikky let me know what you think if there are any mistake I'll try to resolve and retest ASAP. Thanks for all the help on this really learnt a lot 🙏