Skip to content
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

Implement bot-user support #258

Merged
merged 4 commits into from
Dec 6, 2016
Merged

Implement bot-user support #258

merged 4 commits into from
Dec 6, 2016

Conversation

mtyurt
Copy link
Contributor

@mtyurt mtyurt commented Oct 18, 2016

I have added a checkbox which indicates given token belongs to a bot user or not.

But why?

Recently, I have figured out that if I put @username to channel parameter, the plugin sends a direct message via @slackbot. But I didn't want @slackbot to send me a dm, a bot (preferably named jenkins-bot) should send me a dm. So here it is.

  • In global config and advanced job config, there is a checkbox named "Is Bot User?" (false by default).
  • If this checkbox is checked, the plugin will send a post request to https://slack.com/api/chat.postMessage .
  • Also, I added all parameters to request URI if "bot-user" is true, leaving the original case as it is, because sending parameters in JSON body didn't work for chat.postMessage endpoint, I assume there is a special case for "/services/hooks/jenkins-ci".
  • Lastly, there are a few string equals improvements that I couldn't resist.

Copy link

@rborer rborer left a comment

Choose a reason for hiding this comment

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

Nice addition, I was wondering how to achieve bot messaging not coming from slackbot, so +1 for me!

public String getBuildServerUrl() {
if(buildServerUrl == null || buildServerUrl == "") {
if(buildServerUrl == null || "".equals(buildServerUrl)) {
Copy link

Choose a reason for hiding this comment

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

@kmadel
Copy link
Contributor

kmadel commented Nov 19, 2016

Any updates to my comments? Also there are a number of conflicts.

@mtyurt
Copy link
Contributor Author

mtyurt commented Nov 19, 2016

@kmadel merge conflicts are now resolved. I'm confused about the comments, am I the target of these comments? Because I did not get any notification about it.

@kmadel
Copy link
Contributor

kmadel commented Nov 21, 2016

@mtyurt Not sure why you didn't get notified. But yes, those comments were directed at you. Need 'Is Bot' specific help file. Also, please update the README.md to document this feature. My plan it move all documentation into README and direct users from the Jenkins Wiki to the README. And Thanks!

@mtyurt
Copy link
Contributor Author

mtyurt commented Nov 22, 2016

@kmadel from your comment I understand all documentation should be in README file, however there aren't any documentation regarding configurations in README so far. I'll change the screenshot including bot user option, if we want to add bot-user explanation to README, I can add it after Security section as Bot User Support with a brief explanation.

Also, the screenshots in current README file is not up-to-date. Adding new credentials screen has a different order of fields. I'm uploading the screenshot I took while testing with hpi. The credentials has some text prefix, I have no idea why.
screen shot 2016-11-22 at 11 08 44

@kmadel
Copy link
Contributor

kmadel commented Nov 30, 2016

@mtyurt Yes, please add Bot User Support section to README as you mentioned - I know the README is still very much lacking, but we will get it where it needs to be one merge at a time.
Thanks!

Also, would be nice to get a few more thumbs up from a code review perspective.

@mtyurt
Copy link
Contributor Author

mtyurt commented Dec 2, 2016

@kmadel it's done, can you guys have a look please?

@kmadel kmadel merged commit 8b96c38 into jenkinsci:master Dec 6, 2016
@kmadel kmadel added this to the slack-2.2 milestone Dec 6, 2016
@mtyurt mtyurt deleted the bot-user branch December 6, 2016 14:49
@samrocketman
Copy link
Member

Feel free to subscribe to #296 for updates related to Slack Plugin 2.2 release.

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.

4 participants