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

Add support for icon_emoji #477

Closed
mike-pt opened this issue Jan 16, 2019 · 18 comments
Closed

Add support for icon_emoji #477

mike-pt opened this issue Jan 16, 2019 · 18 comments

Comments

@mike-pt
Copy link

mike-pt commented Jan 16, 2019

Currently we can not set icon_emoji via slackSend, which means all notifications come with the Jenkins icon.
It would be nice to be able to change this, particularly cause we could base it on the state of the Job and even use different emoji based on that (like with colors)

@Akayeshmantha
Copy link

Hey mike am a newbie to jenkinsci.Can I have a brief description about this task

@mike-pt
Copy link
Author

mike-pt commented Jan 24, 2019

Well I think this is a simple thing really, I would try it my self but I don't have a self-hosted jenkins install where I could test the changes, however this should be very similar to what we do for botUser/channel

I'd say that to start with we need to add "set/get Icon_Emoji" like we do for channel and botUser:

public void setBotUser(boolean botUser) {
this.botUser = botUser;
}
public String getChannel() {
return channel;
}
@DataBoundSetter
public void setChannel(String channel) {
this.channel = Util.fixEmpty(channel);
}

then we can have that in StandardSlackService Notifier and probably every other place where we handle botUser or channel

@mike-pt
Copy link
Author

mike-pt commented Jan 24, 2019

Oh one thing to note is the icon_emoji is optional, not mandatory

@Akayeshmantha
Copy link

Akayeshmantha commented Jan 24, 2019

Thanks @mike-pt .So it's just adding setter and getter for imoji.If its so if you dont mind shall I try this task

@mike-pt
Copy link
Author

mike-pt commented Jan 24, 2019

I don't mind at all I can only thank you for taking the time!

@mike-pt
Copy link
Author

mike-pt commented Mar 13, 2019

@Akayeshmantha curious if you were able to make some progress on this

@sheeeng
Copy link

sheeeng commented Apr 25, 2019

Is anyone currently plan to work on this issue? It is not assigned to anyone.

@jetersen
Copy link
Member

Feel free to contribute 😓

@jep
Copy link

jep commented May 17, 2019

Just wanted to mention that until this is implemented, the default icon can be changed by going to your Slack settings -> App Directory and editing the configuration for your specific JenkinsCI integration.

jenkins-icon-setting

@EricBartusch
Copy link
Contributor

I've been messing with this a bit, but can't get it to work. I've added a parameter for icon_emoji, but it's disappearing on the request.

https://github.com/EricBartusch/slack-plugin/blob/28fd2bd25710a2e630d41b70f3d07f16eb0413a9/src/main/java/jenkins/plugins/slack/StandardSlackService.java#L172-L174

It gets put into the json just like all of the other parameters, but is ignored. My current guess is that the colon's aren't getting encoded properly. The documentation also says that if the as_user parameter is set, that this will get ignored, but I don't see that happening in the POST being made here. It's set when using a bot, but I'm not in that code block during my testing.

My only other guess is that as_user gets added automatically because the Jenkins CI app has an integration setting called "Customize name" and it can't be null. Where does the code for the Slack Jenkins CI app live? I'm pretty unfamiliar how Slack apps actually work.

@jetersen
Copy link
Member

@EricBartusch This depends whether you're using a bot user or a webhook.

@jetersen
Copy link
Member

jetersen commented May 30, 2019

You must use a bot user to use the icon_emoji.

Custom webhooks is a legacy feature: https://api.slack.com/custom-integrations/incoming-webhooks which mentions it does support icon_emoji.

I doubt https://my.slack.com/services/new/jenkins-ci supports icon_emoji

@EricBartusch
Copy link
Contributor

Thanks @Casz, I had spent way to long trying to make that work. I did manage to get it working with a bot user, but I had to remove the line hardcoding as_user to true when the iconEmoji is set:

url = "https://slack.com/api/chat.postMessage?token=" + populatedToken +
"&channel=" + roomId.replace("#", "") +
"&link_names=1" +
"&as_user=true";
if (threadTs.length() > 1) {

The documentation says when setting it to false, that the username should also be set. In my testing, I haven't done so, and the messages that get sent default to the bot's name, so I don't know how important it is. Also, is there a reason why it was being hardcoded to true in the first place?

@jetersen
Copy link
Member

@EricBartusch no reason looking at the PR that added it #258

also link_names should be optional and I don't think it currently works since it expects "true" or "false" though I might be wrong.

@EricBartusch
Copy link
Contributor

The link_names documetation is inconsistent. The documentation here makes it sound like it should be "true" or "false", but documentation here shows it as 1. I can confim setting it to true/false doesn't change anything and allows links, but 1/0 actually affects the ability to post links. I tested this both on the plugin and in the Slack API tester. I can make this a parameter that gets passed in my branch or make a new one for it, whatever makes more sense.

Looks like as_user works with either 0/1 or true/false. Setting it to true has my bot posting to the channel as me, and setting it to false has it posting as the bot name by default. Should I add a username parameter as well? If either username or icon_emoji is set, turn as_user off and use those values if not null.

@jetersen
Copy link
Member

adding both username and icon_emoji make sense 👍

@timja
Copy link
Member

timja commented Aug 10, 2019

This was done awhile ago and has been released

@timja timja closed this as completed Aug 10, 2019
@mike-pt
Copy link
Author

mike-pt commented Aug 11, 2019

Thanks, linking the PR (#578) here for future reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants