Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JBPM-5370 - Administration service for processes and tasks - services… #642

Merged
merged 1 commit into from Oct 26, 2016

Conversation

mswiderski
Copy link
Contributor

… api and kie server

depends on kiegroup/droolsjbpm-knowledge#177

@mswiderski
Copy link
Contributor Author

jenkins retest this

1 similar comment
@mswiderski
Copy link
Contributor Author

jenkins retest this

@mswiderski
Copy link
Contributor Author

anyone?

@mswiderski
Copy link
Contributor Author

@krisv any comments or good to be merged? starting to get stale a bit here...

Copy link
Member

@krisv krisv left a comment

Choose a reason for hiding this comment

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

Overall +1 (sorry for the delay on the review ;))

@@ -393,7 +393,7 @@ public void triggerNode(long nodeId) {
org.jbpm.workflow.instance.NodeInstance nodeInstance = (org.jbpm.workflow.instance.NodeInstance)
((org.jbpm.workflow.instance.NodeInstanceContainer) getNodeInstanceContainer())
.getNodeInstance(getNode().getNodeContainer().getNode(nodeId));
triggerNodeInstance(nodeInstance, null);
triggerNodeInstance(nodeInstance, org.jbpm.workflow.core.Node.CONNECTION_DEFAULT_TYPE);
Copy link
Member

Choose a reason for hiding this comment

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

@mswiderski what is the reason for changing this? I would expect the argument to indicate why the node was triggered, in this case the node is triggered explicitly by the user, not through an incoming connection, so wouldn't null be more adequate (to distinguish from incoming connection)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@krisv the reason is that some of the NodeInstances (e.g. SubProcessNodeInstance or TimerNodeInstance) throw exception if the connection is different than the default. So instead of fixing all the node instance to check if it's null I thought it's better to simply provide the default connection when retriggering.

@mswiderski mswiderski merged commit 0fe889e into kiegroup:master Oct 26, 2016
@mswiderski mswiderski deleted the JBPM-5370 branch October 26, 2016 12:10
hxf0801 pushed a commit to hxf0801/jbpm that referenced this pull request Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants