Skip to content

Allow role mentions in specific areas#1039

Merged
Den4200 merged 5 commits into
masterfrom
1038_allow_role_mentions_in_specific_areas
Jul 13, 2020
Merged

Allow role mentions in specific areas#1039
Den4200 merged 5 commits into
masterfrom
1038_allow_role_mentions_in_specific_areas

Conversation

@lemonsaurus
Copy link
Copy Markdown
Contributor

@lemonsaurus lemonsaurus commented Jul 12, 2020

This PR makes two changes:

  • Make the bot ping @Moderators instead of @everyone in #mod-alerts and #mod-logs
  • Allow role pings in the Syncers and in help_channels.py by adding the roles to allowed_mentions.

This is necessary for pings to come through after we changed to Discord.py 1.4.0a

Closes #1038.

Leon Sandøy added 2 commits July 12, 2020 13:23
Ping @Moderators in ModLog
210c0a0 
Instead of pinging @everyone, let's just ping the people who actually
need to see the mod alerts or the modlogs, which would be the mods.

`@everyone` is currently not permitted by our allowed_mentions setting,
so this also restores pings to those channels.

GitHub #1038
#1038
Allow role pings in Syncers and help_channels.py
57e210c 
Now that we're running Discord 1.4.0a, we need to explicitely allow all
the role mentions for sends that don't use ping one of the globally
whitelisted role pings, which are Moderators, Admins and Owners.

We were pinging roles other than Mods+ in exactly two cases:
- Inside the Syncers, whenever we ask for sync confirmation (if the
  number of roles or users to sync is unusually high)
- In the help_channels.py system, whenever we max out help channels and
  are unable to create more.

This commit addresses both of these.

GitHub #1038
#1038
@lemonsaurus lemonsaurus requested a review from a team as a code owner July 12, 2020 11:59
@lemonsaurus lemonsaurus requested review from GhostofGoes and ikuyarihS and removed request for a team July 12, 2020 11:59
@ghost ghost added the needs 2 approvals label Jul 12, 2020
SebastiaanZ
SebastiaanZ suggested changes Jul 12, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@SebastiaanZ SebastiaanZ left a comment

Choose a reason for hiding this comment

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

One tiny thing.

Comment thread config-default.yml Outdated
@ghost ghost added s: waiting for author Waiting for author to address a review or respond to a comment and removed needs 2 approvals labels Jul 12, 2020
vivax3794
vivax3794 reviewed Jul 12, 2020
View reviewed changes
Comment thread config-default.yml Outdated
@lemonsaurus @SebastiaanZ
Remove pointless comment
3fd89d5 
This comment violates the DRY principle.

Co-authored-by: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com>
@ghost ghost added needs 2 approvals and removed s: waiting for author Waiting for author to address a review or respond to a comment labels Jul 12, 2020
@lemonsaurus
Copy link
Copy Markdown
Contributor Author

lemonsaurus commented Jul 12, 2020

@vivax3794 @SebastiaanZ Good catch. I've removed that comment since it violates the DRY principle.

@ghost ghost removed the needs 1 approval label Jul 12, 2020
@lemonsaurus
Copy link
Copy Markdown
Contributor Author

lemonsaurus commented Jul 12, 2020

Consensus among core devs appears to be that we should continue to use @everyone pings.

The reason for this is that it allows the #mod-alerts channel, which typically has a low signal-to-noise ratio, to be muted. The trick to it is to surpress @here and @everyone for the entire server and then mute the channel. Changing it to a @Moderators ping makes this trick impossible.

Ideally, I think Discord should address this in the UI, but I suppose I don't really care whether we use one or the other, so I'll change it back and add the @everyone to the allowed_mentions for those sends. Don't merge this yet.

@jb3 jb3 added the s: WIP Work In Progress label Jul 12, 2020
Revert "Ping @Moderators in ModLog"
4527c03 
Let's continue to use "@everyone" for now, and add an explicit allow for
it so that it successfully pings people.

There's a full justification for this in the pull request.

#1038
@lemonsaurus lemonsaurus added a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) p: 0 - critical Needs to be addressed ASAP t: bug Something isn't working and removed s: WIP Work In Progress labels Jul 12, 2020
@lemonsaurus
Copy link
Copy Markdown
Contributor Author

lemonsaurus commented Jul 12, 2020

This should be good now, I think.

Den4200
Den4200 approved these changes Jul 13, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@Den4200 Den4200 left a comment

Choose a reason for hiding this comment

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

Looks good.

@Den4200 Den4200 merged commit ebf0ec8 into master Jul 13, 2020
@Den4200 Den4200 deleted the 1038_allow_role_mentions_in_specific_areas branch July 13, 2020 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vivax3794 vivax3794 vivax3794 left review comments

@Den4200 Den4200 Den4200 approved these changes

@GhostofGoes GhostofGoes Awaiting requested review from GhostofGoes

@ikuyarihS ikuyarihS Awaiting requested review from ikuyarihS

+2 more reviewers

@SebastiaanZ SebastiaanZ SebastiaanZ approved these changes

@ks129 ks129 ks129 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) p: 0 - critical Needs to be addressed ASAP t: bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow role mentions in specific areas

6 participants

@lemonsaurus @SebastiaanZ @ks129 @vivax3794 @Den4200 @jb3