Skip to content

Dynamic available help channels message#1396

Merged
Xithrius merged 14 commits into
masterfrom
dynamic-available-help-channels
Feb 7, 2021
Merged

Dynamic available help channels message#1396
Xithrius merged 14 commits into
masterfrom
dynamic-available-help-channels

Conversation

@Xithrius
Copy link
Copy Markdown
Contributor

@Xithrius Xithrius commented Feb 5, 2021

Closes #1320
Rewrite of #1325

Users sometimes get confused on which channels are available for them to claim.

With this PR, a message in the #how-to-get-help channel will be edited to reflect which channels are currently available.

If the bot cannot edit the most recent message in the #how-to-get-help channel, then it knows it's trying to edit the webhook message that contains the guide for claiming a channel. It will send the dynamic message if this occurs.

If we do decide to remove or add more channels in the help: available category, then the bot will be able to pick up on them.

image

@Xithrius Xithrius changed the title Dynamic available help channels Dynamic available help channels message Feb 5, 2021
@Xithrius Xithrius added a: help channels Related to the help channel system p: 2 - normal Normal Priority t: feature New feature or request labels Feb 5, 2021
@coveralls
Copy link
Copy Markdown

coveralls commented Feb 5, 2021

Coverage Status

Coverage decreased (-0.1%) to 56.58% when pulling f089a08 on dynamic-available-help-channels into 6c2bd3c on master.

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.

Everything is looking solid so far. I have a few functionality concerns, but everything works as expected in general.

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
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.

Clean, and works as expected. LGTM

Copy link
Copy Markdown
Contributor

@Shivansh-007 Shivansh-007 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!

Comment thread bot/exts/help_channels/_cog.py Outdated
@Xithrius
Copy link
Copy Markdown
Contributor Author

Xithrius commented Feb 5, 2021

Here's what the dynamic message now looks like.
image

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

@Shivansh-007 Shivansh-007 left a comment

Choose a reason for hiding this comment

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

Thanks! Looks Good To Me!

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.

Anyone using the server template will need to update their config to move the how-to-get-help channel out of the help available category, but it shouldn't be a big deal. This is not important for the test server, as it uses the same channel for cooldown and how-to-get-help.

Copy link
Copy Markdown
Contributor

@gustavwilliam gustavwilliam left a comment

Choose a reason for hiding this comment

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

Thanks! Works like a charm — unless we go into some edge cases.

When lowering the maximim amount of help channels (restarting bot in-between), the higher number of help channels will be listed in the available help channels message. Let me show you an example, where we go from 3 to 2 channels. After restarting the bot with the new config, the following will happen:

  • The bot keeps updating the message with 3 help channels. One of them will be there statically (and incorrectly), while the other two are correctly and continuously updated
  • When deleting the original "these channels are available" message, the bot will resend the same three-channel message

One workaround for this is deleting the message and then restarting the bot.

The other edge case comes into play when we increase the number of help channels that should be available, like going from 2 to 3. This simply sends a second message with two channels and keeps updating that. Just delete the first message manually and things will work just fine.

However, both of these scenarios are uncommon enough that I'll still approve this PR. If you want to fix the mentioned bugs, that would be fantastic, but I really don't mind leaving it at this point. It works flawlessly, unless we get into some uncommon and strange scenarios.

@Xithrius
Copy link
Copy Markdown
Contributor Author

Xithrius commented Feb 7, 2021

Thank you for your approval!

I think that any of these edge cases will have to include a bot redeploy, as rules would probably have to be changed in order for them to take effect. If we encounter these edge cases, I'll be sure to put it into high/critical priority.

@Xithrius Xithrius merged commit 97969ee into master Feb 7, 2021
@Xithrius Xithrius deleted the dynamic-available-help-channels branch February 7, 2021 12:07
@Xithrius Xithrius restored the dynamic-available-help-channels branch February 7, 2021 12:45
@Xithrius
Copy link
Copy Markdown
Contributor Author

Xithrius commented Feb 7, 2021

Bot can fetch a message that was previously deleted, which causes a discord.NotFound error since it cannot find the message to edit.

On the actual server, no message has been deleted. The bot this time finds the webhook message, then tries to edit it.

I will make a change to check if the last message was sent by the bot. If not, it will send a dynamic message (or something similar to this).

image

@jb3
Copy link
Copy Markdown
Member

jb3 commented Feb 10, 2021

Where are we at on this? Should we open a new PR?

@Xithrius
Copy link
Copy Markdown
Contributor Author

I will be opening a new PR for this, I'm still working out the logic.

@Xithrius Xithrius deleted the dynamic-available-help-channels branch February 11, 2021 09:55
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: 2 - normal Normal Priority t: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a dynamic message at the bottom of #how-to-get-help

10 participants