-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 functions.completeError and functions.completeSuccess #1301
base: master
Are you sure you want to change the base?
add functions.completeError and functions.completeSuccess #1301
Conversation
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.
Thanks for jumping on that issue so quickly @yogasw ! This looks like a great start, but I've left a few comments to double-check - I'm not super familiar with Slack functions so forgive me if I ask some extra questions to make sure I've understood things correctly. If you could also add some tests for both the inner_events
and function_event
files, I'd appreciate it!
Type string `json:"type"` | ||
Name string `json:"name"` | ||
Description string `json:"description"` | ||
Title string `json:"title"` |
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.
I believe there is a IsRequired
parameter in the OutputParameter returned in the Function Event based on the example event provided in the documentation.
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.
i will update it
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.
but why when I activate debug mode this value is not there?
https://gist.github.com/yogasw/65d68c081bfd5bea14520cedc907a979
} | ||
|
||
// Function is a struct for functions in FunctionExecuted events | ||
type Function struct { |
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.
Where did you find the documentation for FormEnabled
and DateReleased
? I don't see it on the event documentation page.
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.
I got it from the debug mode of the slack-go socket mode, because before that I had not found the official documentation
https://gist.github.com/yogasw/65d68c081bfd5bea14520cedc907a979
@@ -0,0 +1,83 @@ | |||
package slack |
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.
Please ensure that the WebAPI functions accept a context as the first function parameter and rename this FunctionCompleteErrorContext
. You can create a second function called FunctionCompleteError
that explicitly sets contact.Background()
if you'd like.
You can look at ListEventAuthorizationsContext and ListEventAuthorizations for an example of how to implement it.
} | ||
|
||
// FunctionCompleteSuccess indicates function is completed | ||
func (api *Client) FunctionCompleteSuccess(functionExecutionId string, options ...FunctionCompleteSuccessRequestOption) error { |
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.
I believe the API method returns a customer_id
, please add it as a function output to most closely mirror the published API specification.
this PR to handle event function_executed and response the function with functions.completeSuccess and functions.completeError