Pretext per Workflow Case, Auto-Check for Private Channel, and Bug Fix #101
Conversation
…orkflow case * Automatically check to see if the channel is private (no # in channel name) and if so, don’t post to Slack with the username in the payload
|
@jordan8037310, thanks for your PR! By analyzing the blame information on this pull request, we identified @greg-1-anderson and @joshkoenig to be potential reviewers |
…ifications to work.
| 'channel' => $channel, | ||
| 'icon_emoji' => ':lightning_cloud:', | ||
| 'icon_emoji' => ':pantheon:', |
There was a problem hiding this comment.
I don't think this is (yet) a standard emoji. ;)
There was a problem hiding this comment.
Good point! We should probably convert this to a full url to a 128x128 Pantheon Emoji. Slack accepts full image urls for this field.
| @@ -13,7 +13,7 @@ | |||
|
|
|||
| // Load our hidden credentials. | |||
| // See the README.md for instructions on storing secrets. | |||
| $secrets = _get_secrets(array('slack_url'), $defaults); | |||
| $secrets = _get_secrets(array('slack_url','slack_channel'), $defaults); | |||
There was a problem hiding this comment.
Why store slack_channel in secrets?
|
@jordan8037310 thanks for the PR! I added a couple comments where I see potential issues. Respond when you have a chance. ;) |
|
@joshkoenig I didn't introduce that change, that's explicitly where the current script is looking. I'm just working with that information. Someone already made that decision to search for the slack channel in secrets.json and I am adding validation. |
| 'pretext' => 'Pantheon system update :rocket:' | ||
| ); | ||
| } | ||
|
|
||
| _slack_notification($secrets['slack_url'], $secrets['slack_channel'], $secrets['slack_username'], $text, $attachment, $secrets['always_show_text']); |
There was a problem hiding this comment.
@joshkoenig - You'll see here that someone has already configured the _slack_notification to look for slack_channel in the secrets. Perhaps you want to check git blame here?
There was a problem hiding this comment.
I was looking at:
We removed the channel from the code intentionally because if left blank the default channel the webhook is configured to post into should be used, which is what developers were expecting. Unless something's changed, you don't need to specify a channel in order for it to work.
However, we clearly missed taking it end-to-end, hence the confusion.
There was a problem hiding this comment.
As I mentioned elsewhere, I used secrets for all sorts of configuration early on. This is probably just one place that was missed when we took the non-secret configuration information out of secrets.json in all of the examples.
There was a problem hiding this comment.
Personally I think that even if it wasn't reading from the secrets file, I would probably just rework into a local config.json file that I would store parallel to the /private/scripts.
Much easier for us to keep scripts unified across all our clients with consistent inputs & outputs that way. This is especially attractive to us as we work with multiple agencies at times and will be doing some work to make slack notifications loop through multiple slack configs and channels.
Either way if you want to take the quicksilver-examples in a different direction that is cool, I just wanted to contribute where I saw this discrepancy!
…ult the webhook to the config set in Slack Admin
|
Updated per feedback! Let me know how this looks when you have some time 👍 |
|
Ah! Just did this myself. The pretext context of what is running is nice to have. I also have a 'before' and 'after' for 'clear_cache' so I know it's happening on LIVE (before) and when the flush is complete (after). |
Deploying :rocket: