-
Notifications
You must be signed in to change notification settings - Fork 9
Post major events within a group to slack #130
base: master
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,50 @@ | |||
Webhooks::Slack::Base = Struct.new(:event) do | |||
|
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.
This sure looks familiar :P
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.
Totally, I spun this off from loomio into metamaps land first, and then over to here. Absolutely worth noting that @gdpelican wrote a lot of this originally and should get a lot of the credit :)
(((o(゚▽゚)o))) thanks @Connoropolous @gdpelican, this is great. 👍 |
omg thank yall so much ! <3 <3 <3 i'm in the depths of a fellowship application so won't have the chance to review until the end of sunday |
Connor this is so amazing. I will be getting on a plane just now, but will Best, Derek On Fri, Mar 18, 2016 at 11:51 AM, Connor Turland [email protected]
|
I've also just share our activity email content with you! It lists major On Sat, Mar 19, 2016 at 2:25 PM, Derek Razo [email protected] wrote:
|
app/models/event.rb
Outdated
validates_inclusion_of :kind, :in => KINDS | ||
validates_presence_of :eventable | ||
|
||
acts_as_sequenced scope: :group_id, column: :sequence_id, skip: lambda {|e| e.group.nil? || e.group_id.nil? } |
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.
future self: look
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.
connor and eugene want to know what is this? and do we need 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.
Er, we use this to say events have a sequence_id, which is a better way to order things (in a discussion say) than by ID or created_at. You shouldn't need it for webhook stuff.
end | ||
|
||
def url_root | ||
ENV["ROOT_URL"] + "/#/" |
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.
we will need to set this on the heroku app where the job worker will be running. should look like
ROOT_URL=http://beta.cobudget.co
@data-doge @derekrazo this is ready for review again now :) |
Thanks x2000 for this Connor - you are amazing! @eugene will ping you with On Sun, Mar 27, 2016 at 10:10 PM, Connor Turland [email protected]
|
All good Derek :) fun and meaningful to contribute
Look forward to seeing it in place...
Hopefully it's useful enough to support designing a UI for it :)
|
Hi all, this looks like it was a great PR! Is there a reason it didn't get merged? @Connoropolous what configuration would be needed (on an app-wide or per-group level) to make this functional? |
@jessykate :) it was, I think, just a simple lack of time and will at the time. it could be set up to work as is, with some configuration on the app level, but I believe its the case that there's no UI for it, so adding particular configurations for particular groups has to be done through the interactive ruby console, which is how I was doing it. this PR could be expanded/combined with another one, without too much difficulty, that made it possible to configure things on a group basis from the UI |
for the app, it needs to be configured with two environment variables this should be set to point to a 48x48 px image |
nice!! i'd say let's split the UI into another ticket and focus on getting this merged, since as you say it's functional as-is. are you keen/have bandwidth to pull in the latest code we've been working on and fix the merge conflicts? (no worries if not, just LMK so i know if we should put our attention to it!) you mentioned you were doing this manually yourself-- are you running your own instance of cobudget? 😍 |
Just resolved the merge conflicts here on github, didn't seem to be anything that would break, but I think from here I'd say take it over if you can, and give it a test before deployment. Here's what to do:
or whatever your local development frontend instance is running at, for ROOT_URL then grab a webhook from a slack team you're part of.
You'll have to be running the |
@jessykate no, I'm not running my own instance of cobudget atm, I just meant while I was doing development :) |
Thanks @Connoropolous! I will put this on my list for next week. |
My pleasure. I take it cobudget is still in active use by you and some projects you work on :) |
@Connoropolous I believe I've set everything up... The rake task is running and I can see that it's receiving and processing the tasks, but nothing hits the slack channel. Any thoughts? (posted this on slack but am not sure if you are receiving notifications there :)). here's the webhook object: and here's the rake task output i see when an event gets triggered: thanks! |
@jessykate I just remembered your comment! sorry it took me a while, too many balls in the air. what comes to mind for debugging this scenario, is inspecting the result of the http post request. It may show an error. This could be related to slack updating the structure of the data since I wrote this? |
I can confirm that slack hasn't updated their API, as we're still using this code in prod for our slack webhooks in Loomio. That model LGTM, JessyKate... so I think you're interested in digging into |
Notes: