-
Notifications
You must be signed in to change notification settings - Fork 11
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
Provide default value when actions are missing in payload #9
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.
LGTM 👍
Sorry for late reply. #### 0.3.1 (Next)
* [#9](https://github.com/slack-ruby/slack-ruby-bot-server-events/pull/9): Provide default value when actions key is missing in payload - [@nijave](https://github.com/nijave).
* Your contribution here.
#### 0.3.0 (12/30/2020)
|
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.
CHANGELOG need to be edited. 🙏
This needs a test, please. |
Could you add a test @nijave ? |
fbe9634
to
97ba16c
Compare
@nijave I won't merge without a spec, please add. |
97ba16c
to
dff37ba
Compare
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.
Specs aren't passing.
@@ -48,6 +48,19 @@ | |||
end | |||
end | |||
|
|||
context 'without actions in the payload' do | |||
before do | |||
payload.delete(:actions) |
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.
Take actions
out of the payload above, and merge(actions: ...)
in the existing spec, then you don't have to delete it here, which seems backwards.
post '/api/slack/action', payload: payload.to_json | ||
expect(last_response.status).to eq 201 | ||
response = JSON.parse(last_response.body) | ||
expect(response).to eq('text' => 'message_action: type match') |
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.
You need a spec that does an action, otherwise this is failing predictably to return any data.
dff37ba
to
2ebb6e0
Compare
See #8