Feature: #1 Other types of graphs #27
Conversation
Updated Market model to include buttons in formatted Slack object. Added condition to Market model to render stock quote's correct graph.
…cture, which now includes interactive buttons and a callback id. Refactored Market model per RuboCop.
| attachments: [slack_attachment] | ||
| } | ||
| else | ||
| fail 'Message token is not actually coming from Slack.' |
There was a problem hiding this comment.
This should be a 401 via error! ..., see https://github.com/ruby-grape/grape#raising-exceptions. With fail it's a 500, the server has failed, but in this case the problem is the client's data, so we want a 401.
There was a problem hiding this comment.
Add a test for this API endpoint for some basics, like parsing a good payload, and a non-matching SLACK_VERIFICATION_TOKEN.
There was a problem hiding this comment.
And this should be a one-liner as a guard:
error! ... unless token == ...
|
This is awesome. Made some small-ish comments, I'd like the API fix at the least here before merging, please. I'd like to make this a premium feature, so buttons would render, but clicking on them would not work or buttons don't render. You don't have to do it in this PR, I can take care of it later - the number of teams is growing fast and I am not having much success breaking even on hosting :( You win at a lifetime premium subscription on market.playplay.io obviously, let me know the team name! |
| @@ -37,6 +37,9 @@ Announce that you sold a symbol. | |||
|
|
|||
| Display current positions. Optionally specify a user to display someone else's current positions. | |||
|
|
|||
| ####Interactive Chart Buttons | |||
There was a problem hiding this comment.
Add a space after #### and an empty line below to make nice.
…n-matching verification tokens. Updated README with animated GIF and styling. Added parameter validation to slack_endpoint and renamed class to GraphEndpoint.
|
Hey @dblock, I made the fixes to the API, added the appropriate tests and altered the market model to take in optional arguments. And you have your animated Gif :) Thanks, David |
| @@ -1,4 +1,5 @@ | |||
| require 'slack-market/api/endpoints/teams_endpoint' | |||
| require 'slack-market/api/endpoints/subscriptions_endpoint' | |||
| require 'slack-market/api/endpoints/status_endpoint' | |||
| require 'slack-market/api/endpoints/slack_endpoint' | |||
There was a problem hiding this comment.
Should be called graph_endpoint.rb.
I think you had a good point wrt making it "slack" vs. "graph", but there're two things that are difficult in computer science, caching and naming :) I prefer domain-specific names, in my experience they just always last longer.
|
Great! It's almost there. I would have merged without that rogue |
|
@dblock, Thanks for your quick feedback. I updated the file names and removed that rogue |
|
Merged this, closes #1, thanks so much! Will get it deployed soon, I'm wrapping up making this a cheap, but paid service. What's your team id? Want to give you a free subscription. |
* Created /api/graph endpoint for Interactive Slack Messages. Updated Market model to include buttons in formatted Slack object. Added condition to Market model to render stock quote's correct graph. * Updated model and endpoint spec to account for new slack message structure, which now includes interactive buttons and a callback id. Refactored Market model per RuboCop. * downgraded firefox to version 46.01 for selenium-webdriver tests * created spec to slack_endpoint test for parsing a good payload and non-matching verification tokens. Updated README with animated GIF and styling. Added parameter validation to slack_endpoint and renamed class to GraphEndpoint. * refactored slack_endpoint_spec * Changed slack_endpoint to be called graph_endpoint
|
Having configured this on market.playplay.io I see your point about the graph endpoint - slack only supports a single action URL. Probably Feel free to PR an update, otherwise I'll get to it at some point. |
|
Hey @dblock, I'll make the PR. I find it interesting that they only support a single URL. Do you have any idea as to why that's the case? This was a lot of fun to code. I'd like to continue adding features and fixing bugs if possible. Are there any other issues I can work on? The team name is doodlebots and the id is T1L7R335Z |
|
I've upgraded the team. I've added you as collaborator of the repo. You should continue making pull requests, this is just in case a piano falls on my head and to help out with future committers/PRs! Spend as much or as little time as you can/want, no obligations. There're a bunch of neat feature requests in issues. I like #20, #14 and #16 for example. |
Hey @dblock ,
To add enhanced graphs in a slack message interactive buttons needed to be included in the market model. Slack's formatting for interactive buttons can be found here.
Clicking on each button will render the appropriate chart for that stock. If charts are set to off, then no chart or buttons will be rendered.
Slack handles a button click in a message by making a POST request to a publicly accessible endpoint, which I created in slack_endpoint.rb.
The following configurations are necessary for this work:
/api/graph. For example my URL for testing purposes was the followinghttps://sleepy-eyrie-81347.herokuapp.com/api/graph
SLACK_VERIFICATION_TOKENand set it equal to the verification token found in the App Credentials. This token is being used to verify that the message is coming directly from a Slack channel and its usage is highly suggested by Slack for security purposes.Also please note the changes to both quote_spec.rb and documentation_spec.rb. All tests should complete without errors.
Please let me know if you run into any issues. This was fun!
Thanks,
David