Skip to content

Update help channel system to use forum channels#2318

Merged
ChrisLovering merged 10 commits into
mainfrom
help-channel-rewrite
Nov 25, 2022
Merged

Update help channel system to use forum channels#2318
ChrisLovering merged 10 commits into
mainfrom
help-channel-rewrite

Conversation

@ChrisLovering
Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering commented Nov 1, 2022

This leverages Discord's new forum channel feature, which removes the need for a lot of our custom logic, simplifying the help channel cog significantly.

Sadly this is a rewrite, so the diff isn't very useful. I'd recommend reviewing just the new code.

Marked as do not merge as there are fixup commits I'd like to squash before this is merged.

@ChrisLovering
Copy link
Copy Markdown
Member Author

Draft until forum channels are fully available.

@ChrisLovering ChrisLovering marked this pull request as draft November 1, 2022 18:55
@onerandomusername
Copy link
Copy Markdown
Contributor

Draft until forum channels are fully available.

forum channels are out to 90% of community guilds and pydis has early access, why are you waiting for something that has no eta?

@onerandomusername
Copy link
Copy Markdown
Contributor

An important thing to note is that in general, guilds without the community feature don't have the option to have forum channels. So if you're looking on a guild that doesn't have community, it won't have forum channels.

@ChrisLovering
Copy link
Copy Markdown
Member Author

Draft until forum channels are fully available.

forum channels are out to 90% of community guilds and pydis has early access, why are you waiting for something that has no eta?

Because our test server, which is marked as community is yet to have them.

@onerandomusername
Copy link
Copy Markdown
Contributor

Ah, so your test server is in the 10%. Gotcha.

@ChrisLovering ChrisLovering marked this pull request as ready for review November 11, 2022 13:10
Copy link
Copy Markdown
Contributor

@shtlrs shtlrs left a comment

Choose a reason for hiding this comment

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

That's a substantial reduction of the amount of code we had.
I unfortunately cannot test this since I don't have access to the forum feature.

I left some comments here & there, thanks Chris !

Comment thread bot/exts/help_channels/_stats.py Outdated
Comment thread bot/exts/help_channels/_stats.py Outdated
Comment thread bot/exts/help_channels/_channel.py Outdated
Comment thread bot/exts/help_channels/_channel.py Outdated
Comment thread bot/exts/help_channels/_channel.py Outdated
Comment thread bot/exts/help_channels/_channel.py Outdated
Comment thread bot/exts/help_channels/_cog.py
Comment thread bot/exts/help_channels/_cog.py Outdated
Comment thread bot/exts/help_channels/_cog.py
Comment thread bot/exts/help_channels/_cog.py
@ChrisLovering ChrisLovering added p: 1 - high High Priority a: help channels Related to the help channel system t: enhancement Changes or improvements to existing features s: needs review Author is waiting for someone to review and approve review: do not merge The PR can be reviewed but cannot be merged now labels Nov 13, 2022
@ChrisLovering
Copy link
Copy Markdown
Member Author

Marked as do not merge as there are fixup commits I'd like to squash before this is merged.

Copy link
Copy Markdown
Contributor

@HassanAbouelela HassanAbouelela left a comment

Choose a reason for hiding this comment

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

Looks and works really nicely :D

Just a few issues

Comment thread bot/exts/help_channels/_channel.py Outdated
Comment thread bot/exts/help_channels/_channel.py Outdated
Comment thread bot/exts/help_channels/_cog.py
Comment thread bot/exts/help_channels/_cog.py
Comment thread bot/exts/help_channels/_message.py Outdated
Copy link
Copy Markdown
Contributor

@shtlrs shtlrs left a comment

Choose a reason for hiding this comment

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

Beautiful work.
Unfortunately, I didn't test this since I don't have access to the forum feature.

Thanks Chris !

Copy link
Copy Markdown
Member

@mbaruh mbaruh left a comment

Choose a reason for hiding this comment

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

Consider changing CODEOWNERS

@ChrisLovering
Copy link
Copy Markdown
Member Author

ChrisLovering commented Nov 14, 2022

Example alert from cfec9d9 (after adding foo as a filter token)
image

@ChrisLovering ChrisLovering removed the review: do not merge The PR can be reviewed but cannot be merged now label Nov 25, 2022
This leverages Discord's new forum chanel feature, which removes the need for a lot of our custom logic, simplifying the help channel cog significantly.
This removes the need for the old helper entirely
This was due to the hlep channels causing many events top be pushed to modlog due to how the old system worked. Now that we use a forum chanel, this is no longer the case.
It is expected that this code will be delete whent he new fitler cog is added, and we start filtering on thread names genericly.
@ChrisLovering ChrisLovering merged commit 04bba4c into main Nov 25, 2022
@ChrisLovering ChrisLovering deleted the help-channel-rewrite branch November 25, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: help channels Related to the help channel system p: 1 - high High Priority s: needs review Author is waiting for someone to review and approve t: enhancement Changes or improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants