-
Notifications
You must be signed in to change notification settings - Fork 393
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
Add support for Workflow Steps from Apps #597
Add support for Workflow Steps from Apps #597
Conversation
fa67db1
to
dde364c
Compare
@@ -17,7 +17,7 @@ export type AnyMiddlewareArgs = | |||
| SlackViewMiddlewareArgs | |||
| SlackShortcutMiddlewareArgs; | |||
|
|||
interface AllMiddlewareArgs { | |||
export interface AllMiddlewareArgs { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
src/WorkflowStep.ts
Outdated
const token = selectToken(context); | ||
|
||
return (config: Parameters<StepCompleteFn>[0]) => { | ||
const { outputs = [] } = config; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outputs should default to an empty object, as it's not an array, but a key/value mapping in the step execute portion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going off of the other comments around config, since outputs
is optional per the API docs, I altered this to be the following:
return (config: Parameters<StepCompleteFn>[0] = {}) => {
return client.workflows.stepCompleted({
token,
workflow_step_execute_id,
...config,
});
};
Testing didn't reveal any issues in my case, but let me know if this doesn't suit for some reason!
package.json
Outdated
@@ -44,8 +44,8 @@ | |||
"dependencies": { | |||
"@slack/logger": "^2.0.0", | |||
"@slack/oauth": "^1.2.0", | |||
"@slack/types": "^1.6.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder :: @slack/types
and @slack/web-api
need to be updated to appropriate versions prior to merge
Codecov Report
@@ Coverage Diff @@
## feat-workflow-steps #597 +/- ##
=======================================================
- Coverage 83.27% 80.95% -2.33%
=======================================================
Files 7 8 +1
Lines 598 693 +95
Branches 193 206 +13
=======================================================
+ Hits 498 561 +63
- Misses 67 95 +28
- Partials 33 37 +4
Continue to review full report at Codecov.
|
47a53bf
to
f2c17a0
Compare
Summary
This pull request contains the proposed API changes outlined here to incorporate support for Workflow Steps from Apps.
Requirements