Red-discordbot: Unload modlog should disable modlog

[Bakersbakebread]

What were you trying to do?

Unload to disable modlog

What were you expecting to happen?

Unloading modlog disabled modlog

What actually happened?

Unloading modlog does not disable modlog

How can we reproduce this issue?

Unload modlog with an active channel set.

[Drapersniper]

Modlog cog and modlog api are two different things. The former is an interface for configuration that anyone could make a different looking one.

If you are referring to the API. Then perhaps a setting to disable the api. But that can get messy as cogs atm May depend on it. And assume it's always avaliable, so such a change would end up being breaking, unless the silence calls to the api when it's disable. But that's obfuscating the behaviour

[Dav-Git]

I don't see this being beneficial. Properly made cogs will provide an option to disable modlog entries or you can do so by disabling the casetype via the modlog cog.

[jack1142]

This has never gone anywhere but I want to bring up again moving commands for changing API settings out of the cogs and putting them directly in core commands. To be specific: moving [p]modlogset from modlog cog to core commands (possibly to [p]set modlog), moving [p]bankset out of bank cog (possibly to [p]set bank), moving [p]economyset registeramount (and probably also [p]bank reset and [p]bank prune) to [p]bankset/[p]set bank. IMO, they shouldn't be part of any cog since they allow to change settings of a core API, same as [p]autoimmune, [p]whitelist, or [p]embedset that also allow users to change API settings.

[Dav-Git]

This has never gone anywhere but I want to bring up again moving commands for changing API settings out of the cogs and putting them directly in core commands. To be specific: moving [p]modlogset from modlog cog to core commands (possibly to [p]set modlog), moving [p]bankset out of bank cog (possibly to [p]set bank), moving [p]economyset registeramount (and probably also [p]bank reset and [p]bank prune) to [p]bankset/[p]set bank. IMO, they shouldn't be part of any cog since they allow to change settings of a core API, same as [p]autoimmune, [p]whitelist, or [p]embedset that also allow users to change API settings.

I like that, however that would leave some cogs as relatively useless. No? E.g. for modlog there's not much more than the settings and the viewing of individual cases which at least makes me think it could be moved to core entirely. If that happens though, core ends up being massive again, so I can understand why the categorization and split into multiple cogs was made.

[jack1142]

I like that, however that would leave some cogs as relatively useless.

ModLog would still have [p]casesfor, [p]case, and [p]reason commands, Bank cog would be removed since there would be nothing in it, and Economy cog still has plenty to offer.

[jack1142]

I'm specifically only talking about command that allow to change API's settings, nothing else.

[mikeshardmind]

This isn't a bug, and I don't think it's something that should be changed either. As a documented and supported API that cogs use, it isn't intended to be replaced, and the output of cases to a channel can be controlled.

[jack1142]

Right, I forgot about the clue of the issue... I don't think the API should be disableable either, I just wanted
to use this as an occasion to propose (again) something that would make more sense for what ModLog really is - a core API.

[mikeshardmind]

+1 from me on moving the API management commands to core_commands, but I think it needs to happen on a breaking release since this is going to break people's usage of permissions, also makes it clearer that modlog isn't something that can be disabled.

[Bakersbakebread]

+1 from me on moving the API management commands to core_commands, but I think it needs to happen on a breaking release since this is going to break people's usage of permissions, also makes it clearer that modlog isn't something that can be disabled.

That's the crux of this issue, I expected the unload to disable it as this is common behaviour with most cogs.

It being hidden inside the cog and unloading the cog hides the options to disable it.

As in, clearing the modlog channel to globally disable.

[jack1142]

I made #4128 to express the idea of moving commands for API settings out of cogs. Does anyone involved in this issue has anything against closing this issue in favor of #4128?

[Bakersbakebread]

Nope, lgtm

[jack1142]

Closing in favour of #4128.