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

slack: add CP channel prefix #5249

Merged
merged 10 commits into from
Sep 12, 2022
Merged

Conversation

sprnza
Copy link
Contributor

@sprnza sprnza commented Sep 6, 2022

SUMMARY
  1. For channels wich ID starts with CP # symbol is being prepended
  2. docs improved for cases when message_id is supplied
SUMMARY
  1. CP string was added to the list which describes channel_id prefixed for which # symbol will be added in front of channel_id
  2. Added notes regarding channel_id has to be literal Channel ID returned by previous task run.
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

slack

ADDITIONAL INFORMATION
  1. When build_payload_for_slack is called there is an if which prepends # if channel_id starts with a certain symbols. I've stumbled upon a channel which ID starts with CP but current implementation doesn't have this string in the list so I got #CPxxxxxx added to payload.
  2. It fails when message_id is supplied and channel_id is either "mychannel_nameor#mychannel_nameit only works. Whenchannel_idis in form ofC0xxxxxxx` which is returned by previous task run.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added module module new_contributor Help guide this first time contributor notification plugins plugin (any type) small_patch Hopefully easy to review labels Sep 6, 2022
@sprnza
Copy link
Contributor Author

sprnza commented Sep 6, 2022

Unsolved root cause of issues is field channel doesn't accept channel name when message_id is provided.

@felixfontein
Copy link
Collaborator

Thanks for your contribution. Could you please add a changelog fragment? Also please fill out the PR form the next time instead of removing all the fields you find irrelevant. Thanks.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Sep 6, 2022
@ansibullbot ansibullbot added bug This issue/PR relates to a bug and removed small_patch Hopefully easy to review labels Sep 7, 2022
@sprnza
Copy link
Contributor Author

sprnza commented Sep 7, 2022

@felixfontein Done! Sorry about not following the rules. I've also added a docs change for issue I've also stumbled upon and it wasn't described in the docs.

@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 7, 2022
@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Sep 7, 2022
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Sep 7, 2022
plugins/modules/notification/slack.py Outdated Show resolved Hide resolved
plugins/modules/notification/slack.py Outdated Show resolved Hide resolved
plugins/modules/notification/slack.py Outdated Show resolved Hide resolved
plugins/modules/notification/slack.py Outdated Show resolved Hide resolved
changelogs/fragments/5249-add-new-channel-prefix.yml Outdated Show resolved Hide resolved
@felixfontein felixfontein changed the title add CP channel prefix slack: add CP channel prefix Sep 8, 2022
Co-authored-by: Felix Fontein <felix@fontein.de>
@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI and removed ci_verified Push fixes to PR branch to re-run CI labels Sep 8, 2022
Co-authored-by: Felix Fontein <felix@fontein.de>
This was referenced Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor notification plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants