Skip to content

[Notifier] Update information for slack on actual implementation #14647

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

Merged
merged 1 commit into from
Dec 4, 2020
Merged

[Notifier] Update information for slack on actual implementation #14647

merged 1 commit into from
Dec 4, 2020

Conversation

malteschlueter
Copy link
Contributor

The documentation wasn't updated after doing BC in the notifier component in slack.
See https://github.com/symfony/symfony/blob/5.2/src/Symfony/Component/Notifier/Bridge/Slack/CHANGELOG.md

Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Thank you! I completely missed this revert. This doc was extremely difficult to follow due to the new Slack integration I believe, so I hope this revert will help here.

cc @Nyholm @OskarStark fyi, as I know you struggled to get the 5.1 integration working.

@wouterj wouterj modified the milestones: 5.1, 5.2 Dec 3, 2020
@malteschlueter
Copy link
Contributor Author

malteschlueter commented Dec 3, 2020

@wouterj In general, what do you think about a new page under Learn more like "How to configure slack and send messages" including a little explanation how to get the token and the correct authorization in the Slack API? Or would that going too far for the symfony documentation?

For example it was a little bit difficult to find the correct documentation of the current Slack implementation which is symfony using.

@OskarStark
Copy link
Contributor

In general, what do you think about a new page under Learn more like "How to configure slack and send messages" including a little explanation how to get the token and the correct authorization in the Slack API? Or would that going too far for the symfony documentation?

I would love to have sth like this and as it is about Slack which is a big player I think we can and should provide it to make the experience as smooth as possible 👍

WDYT @Nyholm @javiereguiluz

SLACK_DSN=slack://default/ID
# If your slack webhook looks like "https://hooks.slack.com/services/XXXXXXXXX/XXXXXXXXX/XXXXXXXXXXXXXXXXXXXXXXXX" then use:
SLACK_DSN=slack://default/XXXXXXXXX/XXXXXXXXX/XXXXXXXXXXXXXXXXXXXXXXXX
SLACK_DSN=slack://TOKEN@default?channel=CHANNEL
Copy link
Contributor

Choose a reason for hiding this comment

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

is this true? 🤔

I currently use Symfony 5.2 and the following DSN:

SLACK_DSN=slack://token_to_be_changed@default/T......../B......./q...........

I mean is the channel required? IMO a webhook can only be configured for a dedicated channel which is "included" in the: /XXXXXXXXX/XXXXXXXXX/XXXXXXXXXXXXXXXXXXXXXXXX

Copy link
Contributor Author

@malteschlueter malteschlueter Dec 3, 2020

Choose a reason for hiding this comment

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

Yes with 5.2 :)
The slack implementation is now using the Slack API again, see https://api.slack.com/methods/chat.postMessage. Because of that you have to set the target channel and the token of your Slack app.
I used the token under "OAuth & Permissions" in the Slack app configuration interface and add the scope "chat:write". At the end i also had to add the bot by hand to the target channel in Slack.

@javiereguiluz
Copy link
Member

I'm usually against having lots of small pages ... for our maintenance is not great, but for users is also bad in many situations (they end jumping from one small page to another, opening 10 browser tabs ... instead of having a single page with all the info).

In these past years we've moved from "small pages" to "big reference pages". See for example the Routing page, it's large, but it covers 90% of the routing features, leaving the very specific and advanced features for other separate pages.

If possible, I'd like to do the same here. If the resulting page is impossibly long, this might be a caution warning about the component itself, which is too verbose or complex to setup.

@OskarStark
Copy link
Contributor

Makes sense, but you are not against a section (not an extra page) with the content, right?

@wouterj
Copy link
Member

wouterj commented Dec 3, 2020

Let's merge this PR as-is 👍

The current Notifier doc structure was build on 5.0. The component changed a lot since then and we've only "patched" the docs. I think it no longer fits the current docs ( e.g. Slack and another chatter also have their own page, etc.). Let's discuss how to better structure this part of the documentation in a new issue.

@OskarStark
Copy link
Contributor

Thank you Malte.

@OskarStark OskarStark merged commit aece272 into symfony:5.2 Dec 4, 2020
@malteschlueter malteschlueter deleted the patch-slack-information branch December 4, 2020 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants