Skip to content

Conversation

@kyb3r
Copy link
Collaborator

@kyb3r kyb3r commented Jan 24, 2019

Adds an anonymous command and now the bot logs all messages sent in thread channels. This closes #107 and closes #147

@kyb3r kyb3r requested review from Taaku18 and fourjr January 24, 2019 16:55
Copy link
Collaborator

@Taaku18 Taaku18 left a comment

Choose a reason for hiding this comment

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

Everything else on the client side looks good.

await self.process_commands(message)
if thread is not None:
await self.modmail_api.append_log(message, type='internal')

Copy link
Collaborator

Choose a reason for hiding this comment

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

Even though there isn't a command being invoked here, the message should still propagate through discord. Or else it might mask issues in the future.

Suggested change
await self.process_commands(message)

Copy link
Collaborator Author

@kyb3r kyb3r Jan 24, 2019

Choose a reason for hiding this comment

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

The documentation says that its the equivalent of those two calls, plus we need to know whether or not a command exists before we log an internal message.

Copy link
Collaborator

@Taaku18 Taaku18 Jan 24, 2019

Choose a reason for hiding this comment

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

@kyb3r I mean add it at the end of on_message, that "suggested change" section might be a bit misleading. So...

...
thread = await self.threads.find(channel=ctx.channel)
if thread is not None:
    await self.modmail_api.append_log(message, type='internal')
return await self.process_commands(message)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But we are not logging command messages

Copy link
Collaborator

@Taaku18 Taaku18 Jan 25, 2019

Choose a reason for hiding this comment

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

Yeah I know we're not logging commands, but we should still include return await self.invoke(ctx) at the end. This way "command_error" event will still be dispatched when an invalid command was received.

It doesn't really matter right now, but it may cause problems later as this suppresses the CommandNotFound from on_command_error.

Btw, also change:

if ctx.command:

to:

if ctx.command is not None:

@kyb3r kyb3r merged commit f7f5a74 into master Jan 24, 2019
@kyb3r kyb3r deleted the anon-cmd branch January 24, 2019 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

auto config option for note commands The ability to be anonymous in support tickets

4 participants