Skip to content

Watcher: Make message.text in slack messages optional #30071

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

Closed
elasticmachine opened this issue Feb 22, 2018 · 9 comments
Closed

Watcher: Make message.text in slack messages optional #30071

elasticmachine opened this issue Feb 22, 2018 · 9 comments

Comments

@elasticmachine
Copy link
Collaborator

Original comment by @spinscale:

From: https://discuss.elastic.co/t/make-message-text-not-required-for-slack-action/120939

The Slack action currently requires message.text to be present, though slack will gladly require a message without it, as a message can have an attachment which contains the text to be displayed. The current requirement for message.text means a message with attachment needs both message.text and message.attachments.text, causing the message displayed in Slack to look slightly disjointed. Would be excellent to remove this requirement.

@albendz
Copy link
Contributor

albendz commented May 10, 2018

Let me know if anyone is working on this and if not, I can grab it.

I'm new to this project so it will take me a little extra time to get to PR.

@spinscale
Copy link
Contributor

there is currently noone working on this, working on a PR is much appreciated!

@albendz
Copy link
Contributor

albendz commented May 13, 2018

Simplistic scan over the code solution:

  • remove final modifier on message and attachments, allowing them to be null
  • rearrange code to provide multiple constructors to allow for variable number of parameters
  • Support setters for message and attachments in case someone changes their mind later (Actual reason: provide setters for mutable fields that are optional within the constructor)

Based on the SlackAccount.java code, a null message and/or null Attachements is tolerated via null checks.

@albendz
Copy link
Contributor

albendz commented May 30, 2018

I am working on testing the solution above but I'm hitting so many dependency issues building the project in IntelliJ that I'm not making a lot of progress. I couldn't find a help page for this. Any pointers?

@tvernum
Copy link
Contributor

tvernum commented May 30, 2018

@albendz You should be able to use the gradle integration in IntelliJ to get everything working.

Run ./gradlew idea from the root of the elasticsearch/ project, and then import it into IntelliJ as a gradle project, using gradle wrapper.

IntelliJ can take some time to builds its caches and refresh indices, etc but that should give you a working project. If you run into specific errors, please let us know.

@albendz
Copy link
Contributor

albendz commented May 30, 2018

@tvernum Extremely obvious in retrospect. Thanks for the reminder.

@albendz
Copy link
Contributor

albendz commented Jun 4, 2018

Is there a better place to ask setup questions than on this issue? I'm now working on installing Java 9 alongside Java 10 against Oracle recommendation due to the build errors I'm getting.

@tvernum
Copy link
Contributor

tvernum commented Jun 5, 2018

We have a forum where you can ask questions.
Feel free to ping me @TimV there if you want to draw attention to your post (on this topic, not just any old post 😃).

@albendz
Copy link
Contributor

albendz commented Jun 9, 2018

Thanks Tim, I've created this thread: https://discuss.elastic.co/t/java-9-dependency/135214

Also, if you could check my earlier comments on this regarding my proposed solution, I would appreciate the verification of my approach.

hub-cap pushed a commit that referenced this issue Jul 11, 2018
Slack accepts an empty text or attachments, but not both. This commit
ensures that both are not empty when creating a watch.

Closes #30071

Replacing old pull request: #31288
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

4 participants