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

Remove automatically adding # symbol to channel names #5629

Merged

Conversation

wmcbroomd2d
Copy link
Contributor

@wmcbroomd2d wmcbroomd2d commented Nov 29, 2022

SUMMARY

For channels with IDs that starts with G4 a # symbol is being prepended causing it to fail.
Channel IDs can start with either C, G, or D but there is no documentation on the length of the ID so it isn't possible to be able to create logic to catch all the possible IDs without false positives from channel name values.
This change removes the logic to automatically prepend # to passed in channel value.

Fixes issue #2608

https://api.slack.com/docs/conversations-api#shared_channels

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

slack

ADDITIONAL INFORMATION

When trying to edit a slack message in a private channel by using the message_id argument I get an error that the channel was not found with the Channel ID G4xxxxxxx

  1. When build_payload_for_slack is called there is an if which prepends # if channel_id does not starts with a certain symbols or does not have one of the listed two character prefixes. I've stumbled upon a channel which ID starts with G4 but current implementation doesn't have this string in the list so I got #G4xxxxxx added to payload.
  2. It fails when message_id is supplied and since channel_id can not be the channel name and the ID is in the form of G4xxxxxxx which is returned by previous task run.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor notification plugins plugin (any type) small_patch Hopefully easy to review labels Nov 29, 2022
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Could you please add a changelog fragment? Thanks.

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I've added a first comment. Could you please add a changelog fragment? Thanks.

plugins/modules/slack.py Outdated Show resolved Hide resolved
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Nov 29, 2022
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed small_patch Hopefully easy to review labels Nov 29, 2022
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Nov 29, 2022
@wmcbroomd2d wmcbroomd2d changed the title Add regex to match all channel ids Remove automatically adding # symbol to channel names Nov 30, 2022
plugins/modules/slack.py Show resolved Hide resolved
plugins/modules/slack.py Outdated Show resolved Hide resolved
plugins/modules/slack.py Show resolved Hide resolved
plugins/modules/slack.py Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Nov 30, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

wmcbroomd2d and others added 4 commits November 30, 2022 16:06
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
…id.yml

Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
@felixfontein felixfontein merged commit 03039a5 into ansible-collections:main Dec 1, 2022
@patchback
Copy link

patchback bot commented Dec 1, 2022

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/03039a56c0a409b574f79f456df72950d95362c6/pr-5629

Backported as #5641

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Dec 1, 2022
* Add regex to match all channel ids

* Add changelog fragment

* Allow matching of channel ids with 9-11 characters

* Fix file name

* Update changelogs/fragments/5629-add-channel-prefix-regex.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Remove channel auto prepend #

* Update changelog fragment

* Add prepend_hash option

* Add version_added to prepend_hash doc string

Co-authored-by: Felix Fontein <felix@fontein.de>

* Add description of possible values for the prepend_hash option

Co-authored-by: Felix Fontein <felix@fontein.de>

* Remove old channel assign statement

* Update changelogs/fragments/5629-add-prepend-hash-option-for-channel-id.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelog fragment tag

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 03039a5)
@wmcbroomd2d wmcbroomd2d deleted the fix-slack-channel-id branch December 1, 2022 21:20
@felixfontein
Copy link
Collaborator

@wmcbroomd2d thanks for improving the module!

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Dec 1, 2022
felixfontein pushed a commit that referenced this pull request Dec 1, 2022
…symbol to channel names (#5641)

Remove automatically adding # symbol to channel names (#5629)

* Add regex to match all channel ids

* Add changelog fragment

* Allow matching of channel ids with 9-11 characters

* Fix file name

* Update changelogs/fragments/5629-add-channel-prefix-regex.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Remove channel auto prepend #

* Update changelog fragment

* Add prepend_hash option

* Add version_added to prepend_hash doc string

Co-authored-by: Felix Fontein <felix@fontein.de>

* Add description of possible values for the prepend_hash option

Co-authored-by: Felix Fontein <felix@fontein.de>

* Remove old channel assign statement

* Update changelogs/fragments/5629-add-prepend-hash-option-for-channel-id.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelog fragment tag

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 03039a5)

Co-authored-by: William McBroom <william.mcbroom@draft2digital.com>
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.

None yet

3 participants