Making WordPress.org

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#5402 closed defect (bug) (fixed)

Can @group be sanitized on the Trac>Slack integration to avoid the flag

Reported by: garrett-eclipse's profile garrett-eclipse Owned by: dd32's profile dd32
Milestone: Priority: normal
Component: Trac Keywords:
Cc:

Description

Hello,

Can the content posted from core Trac to Slack sanitize the @group as it currently will flag in the #core-firehose for example;
https://core.trac.wordpress.org/ticket/51184#comment:15
https://core.trac.wordpress.org/ticket/50910#comment:20
https://core.trac.wordpress.org/ticket/46431#comment:4

Screenshot coming.

Thanks

Attachments (2)

Screen Shot 2020-08-29 at 11.03.36 PM.png (734.9 KB) - added by garrett-eclipse 4 years ago.
@group instances of late that have flagged and can be seen highlighted in search.
Screen Shot 2020-09-15 at 9.30.22 PM.png (99.7 KB) - added by garrett-eclipse 4 years ago.
Actual success test

Download all attachments as: .zip

Change History (10)

@garrett-eclipse
4 years ago

@group instances of late that have flagged and can be seen highlighted in search.

#1 @dd32
4 years ago

Last time I looked into this, I was told that while it looks like it's a ping, it shouldn't actually be a ping.. I don't believe that's quite true though..

In testing, putting it inside a code tag should prevent it being a ping, but it looks like in these cases that's not helping.

Any suggestions on how we can escape it/etc that makes sense?

For what it's worth, we use Incoming webhooks for sending messages to Slack: https://meta.trac.wordpress.org/browser/sites/trunk/common/includes/slack/send.php

#2 @dd32
4 years ago

Ah, found it. Slack Formatting specifies that bots can't use @(channel|here|group) directly and should use <!(channel|here|group)> instead.. however.. Slack also has some auto-parsing which will convert it on the fly - I don't think that was always there. Since we set the link_names parameter to true, that gets triggered.

#3 @garrett-eclipse
4 years ago

Thanks for looking into this @dd32 it sounds like you've found the culprit. Reading into it sounds like we can omit the link_names param and disable the auto parsing as it only works on conversations and user groups which I don't think are desired;

We've already deprecated this functionality for user mentions, so always parsing manually will keep you prepared in case automatic parsing is deprecated for other entities like conversations or user groups in the future.

Deprecated link - https://api.slack.com/changelog/2017-09-the-one-about-usernames

They also mention manual parsing is preferred if they decide to deprecate other autoparsing features.

#4 @dd32
4 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 10264:

Slack: Disable link_names by default.

This probably isn't as useful today as it once was, as Slack usernames and WordPress.org usernames have diverged.

This should also prevent at-group from pinging the workspace.

Fixes #5402

#5 @dd32
4 years ago

Let's see if that worked... @group this is a test message :)

Looks good to me: https://wordpress.slack.com/archives/C03BD0WG1/p1600230488023600

Last edited 4 years ago by dd32 (previous) (diff)

#6 @garrett-eclipse
4 years ago

Thanks @dd32 seems to work nicely.

#7 @ocean90
4 years ago

@dd32 I think we should keep this enabled for the /here broadcasts because now the channel name is no longer linked.

Example: https://wordpress.slack.com/archives/C02RQBWTW/p1600279125290500

#8 @dd32
4 years ago

In 10278:

Slack: Link channel names in /here and /channel broadcast.

Props ocean90.
See #5402.

Note: See TracTickets for help on using tickets.