Skip to content

Conversation

@Jyoti-Prakash01
Copy link

No description provided.

<properties>
<property name="webhook" required="true">
<property-ui description="The Incoming WebHook URL that will accept external messages into Slack." label="Incoming WebHook" type="textBox"/>
<property name="oauthAccessToken" required="true">

Choose a reason for hiding this comment

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

So we don't want to get rid of the webhook capabilities here. We still want that ability.

Please create a new plugin step that be called "Post Notification to Slack (With App Token)" or something like that. Then it would resemble what we have in the internal Slack repository.

We have to be careful in every change that we make that the existing functionality still works for people.

return responseJson
}

def private executeIncomingWebhookPostMethod = {webhookURL, channel, message ->

Choose a reason for hiding this comment

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

Is this method used anywhere?

Choose a reason for hiding this comment

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

I think it would replace much of the logic here?
https://github.com/UrbanCode/Slack-UCD/blob/master/src/main/zip/postToSlack.groovy

Options:
A) I'm okay with including this code, but you should likely comment it out or label its purpose so we know.
B) Update the code where you expect it to be used. (But likely should be done in a separate PR).

Copy link
Author

Choose a reason for hiding this comment

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

New step that we have added, we are using only executePostMethod from SlackRestHelper.
Which i picked from slack ucd plugin (hcl github). We can remove it. Right now we are not using that method.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants