Skip to content
This repository was archived by the owner on May 16, 2023. It is now read-only.

Fix SlackAction: change SlackAction "value" to "url" - #43

Open
dvehar wants to merge 5 commits into
gpedro:masterfrom
dvehar:fix-slack-action
Open

Fix SlackAction: change SlackAction "value" to "url"#43
dvehar wants to merge 5 commits into
gpedro:masterfrom
dvehar:fix-slack-action

Conversation

@dvehar

@dvehar dvehar commented Jun 13, 2018

Copy link
Copy Markdown

Using "value" does not work (maybe it used to). Example

Using "url" does work. Example

dvehar added 2 commits June 13, 2018 16:24
@dvehar
Remove dead code
b56be76
@dvehar
Change SlackAction "value" to "url"
dcc34fe 
Using "value" does not work (maybe it used to). [Example](https://api.slack.com/docs/messages/builder?msg=%7B%22text%22%3A%22%3C%40W1A2BC3DD%3E%20approved%20your%20travel%20request.%20Book%20any%20airline%20you%20like%20by%20continuing%20below.%22%2C%22channel%22%3A%22C061EG9SL%22%2C%22attachments%22%3A%5B%7B%22fallback%22%3A%22Book%20your%20flights%20at%20https%3A%2F%2Fflights.example.com%2Fbook%2Fr123456%22%2C%22actions%22%3A%5B%7B%22type%22%3A%22button%22%2C%22text%22%3A%22Book%20flights%20%F0%9F%9B%AB%22%2C%22value%22%3A%22https%3A%2F%2Fflights.example.com%2Fbook%2Fr123456%22%7D%5D%7D%5D%7D)

Using "url" does work. [Example](https://api.slack.com/docs/messages/builder?msg=%7B%22text%22%3A%22%3C%40W1A2BC3DD%3E%20approved%20your%20travel%20request.%20Book%20any%20airline%20you%20like%20by%20continuing%20below.%22%2C%22channel%22%3A%22C061EG9SL%22%2C%22attachments%22%3A%5B%7B%22fallback%22%3A%22Book%20your%20flights%20at%20https%3A%2F%2Fflights.example.com%2Fbook%2Fr123456%22%2C%22actions%22%3A%5B%7B%22type%22%3A%22button%22%2C%22text%22%3A%22Book%20flights%20%F0%9F%9B%AB%22%2C%22url%22%3A%22https%3A%2F%2Fflights.example.com%2Fbook%2Fr123456%22%7D%5D%7D%5D%7D)
@dvehar

dvehar commented Jun 13, 2018

Copy link
Copy Markdown
Author

@gpedro @galimru please review

@rgalimov

rgalimov commented Jun 19, 2018 •
edited
Loading

Copy link
Copy Markdown

Hi @dvehar, the value is needed for interactive message buttons as response value to help determine which button was pressed. When you use url field it will create link button like described here https://api.slack.com/docs/message-attachments#link_buttons
I think it would be better to add url field and leave other fields as is.

@dvehar
Refactor SlackAction to use an object oriented approach for each type…
cc66b37 
… of action

Creates the concrete SlackActions:
- BasicButtonSlackAction
- InteractiveButtonSlackAction
- InteractiveSelectSlackAction

Adds tests
@dvehar

dvehar commented Jun 22, 2018

Copy link
Copy Markdown
Author

@rgalimov I pushed some changes. I broke out the SlackAction functionality into specific subclasses (BasicButtonSlackAction, InteractiveButtonSlackAction, InteractiveSelectSlackAction) and added tests.

@dvehar

dvehar commented Jul 13, 2018

Copy link
Copy Markdown
Author

@rgalimov can you please review?

@galimru

galimru commented Jul 13, 2018

Copy link
Copy Markdown
Contributor

@dvehar this looks fine

@dvehar

dvehar commented Jul 13, 2018

Copy link
Copy Markdown
Author

@gpedro please review / merge.

@dvehar

dvehar commented Jul 21, 2018

Copy link
Copy Markdown
Author

@gpedro just following up. Can this change be merged?

@deepakputhraya

deepakputhraya commented Aug 28, 2018

Copy link
Copy Markdown

@gpedro @galimru Can these changes be merged?

@dvehar

dvehar commented Feb 4, 2019

Copy link
Copy Markdown
Author

@gpedro @jaypatel512 @rgalimov are you able to merge?

@rgalimov

rgalimov commented Feb 4, 2019

Copy link
Copy Markdown

@dvehar Sorry, I don't have access to merge PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers
@jaypatel512 jaypatel512 jaypatel512 approved these changes
@agentgonzo agentgonzo agentgonzo approved these changes
@rgalimov rgalimov rgalimov approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@dvehar @rgalimov @galimru @deepakputhraya @jaypatel512 @agentgonzo