On http changes for workflow progress#1
Conversation
| return self.postNodeNotification(message); | ||
| } else if (_.has(message, 'taskId') && _.has(message, 'progress')) { | ||
| // This will be progress update notification if taskId is specified | ||
| return self.postProgressEvent(message); |
There was a problem hiding this comment.
I would suggest to refactor this to following so other develops can choose to develop other task notifications other than progress notification.
} else if (_.has(message, 'taskId')) {
if (_.has(message, 'progress'))
{
// This will be progress update notification if taskId is specified
return self.postProgressEvent(message);
}
if (_.has(message, 'SOMETHING_ELSE'))
{
// others can choose to post other kind of task notification without need to modify your code.
}
| }; | ||
|
|
||
| return new notificationApiService(); | ||
| NotificationApiService.prototype.postProgressEvent = function(data) { |
There was a problem hiding this comment.
I would suggest to change function name to postProgressNotification to be better aligned with previous function. This is not a big deal anyway.
| taskName: graphObject[0].tasks[data.taskId].friendlyName, | ||
| progress: data.progress | ||
| } | ||
| }; |
There was a problem hiding this comment.
suggest to return something in this .then block. One option would be combine following then block with this one.
| return new notificationApiService(); | ||
| NotificationApiService.prototype.postProgressEvent = function(data) { | ||
| var progressData; | ||
| return waterline.taskdependencies.find({taskId: data.taskId}) |
There was a problem hiding this comment.
Copy @yyscamper 's comments at on-taskgraph:
use 'findOne instead offind, so that you can use graphObject rather
than graphObject[0]` in following code.
cgx027
left a comment
There was a problem hiding this comment.
Great overall. 👍 after review comments.
add callback handler for wsman microservices
Add SEL/LC logging via inventory microservice. Expose through northb…
Code for review.