WordPress.org

Making WordPress.org

Opened 13 months ago

Closed 12 months ago

Last modified 12 months ago

#5402 closed defect (fixed)

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

Reported by: garrett-eclipse Owned by: 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 13 months 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 12 months ago.
Actual success test

Download all attachments as: .zip

Change History (10)

@garrett-eclipse
13 months ago

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

#1 @dd32
13 months 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
13 months 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
13 months 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
12 months 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
12 months 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 12 months ago by dd32 (previous) (diff)

#6 @garrett-eclipse
12 months ago

Thanks @dd32 seems to work nicely.

#7 @ocean90
12 months 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
12 months ago

In 10278:

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

Props ocean90.
See #5402.

Note: See TracTickets for help on using tickets.