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

Provide default value when actions are missing in payload #9

Merged
merged 2 commits into from
Feb 3, 2021

Conversation

nijave
Copy link
Contributor

@nijave nijave commented Jan 22, 2021

See #8

Copy link
Collaborator

@crazyoptimist crazyoptimist left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@crazyoptimist
Copy link
Collaborator

Sorry for late reply.
Could you figure out the CHANGELOG according to the existing style like this:

#### 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)

Copy link
Collaborator

@crazyoptimist crazyoptimist left a 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. 🙏

@dblock
Copy link
Contributor

dblock commented Jan 29, 2021

This needs a test, please.

@crazyoptimist
Copy link
Collaborator

Could you add a test @nijave ?
Or may I do another PR adding a test and close this one?

@nijave nijave changed the title Provide default value when actions is missing in payload Provide default value when actions are missing in payload Feb 1, 2021
CHANGELOG.md Show resolved Hide resolved
@nijave nijave force-pushed the nv-actions-default-value branch from fbe9634 to 97ba16c Compare February 2, 2021 00:01
@dblock
Copy link
Contributor

dblock commented Feb 2, 2021

@nijave I won't merge without a spec, please add.

@nijave nijave force-pushed the nv-actions-default-value branch from 97ba16c to dff37ba Compare February 3, 2021 02:27
Copy link
Contributor

@dblock dblock left a 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)
Copy link
Contributor

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')
Copy link
Contributor

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.

@nijave nijave force-pushed the nv-actions-default-value branch from dff37ba to 2ebb6e0 Compare February 3, 2021 04:17
@dblock dblock merged commit 8574bed into slack-ruby:master Feb 3, 2021
@nijave nijave deleted the nv-actions-default-value branch February 5, 2021 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants