Log tag changes

[marko-radosavljevic]

Is your feature request related to a problem? Please describe.

Managing tags (creating, editing, deleting) is done on the discord. It's not under version control, and we do not know who issued tag managing command.

And since a lot of people have permission to manage tags, it would be nice to be able to audit the changes, and revert in case tag gets accidentally deleted or similar.

Describe the solution you'd like

Every /tag-manage to be logged in #mod_audit_log, so we can know who edited the tag, when, and what are the changes being made.

[interacsion]

I'll start working on that

[Zabuzard]

I'll start working on that

Awesome, enjoy! If you have any questions, feel free to ask either here or in the Discord server 😃

[Zabuzard]

Design-wise, I would maybe create a separate class that offers a method to write arbitrary messages into the log channel. And then let the tag manage command class take an instance of that class and use the method there, whenever the tags are changed.

Not sure whether the existing mod routine class would be much of a help here honestly, it has a completely different purpose and flow. But I still feel like all mod-log actions should be near to it, at least in the same package. Hence why I would just create some ModAuditLogWriter class or similar there and give it a method log public void logMessage(String message) which others can then use.

[interacsion]

Should the bot log the action instantly, or wait for the routine?

[interacsion]

do we need to log it when someone uses /tag-manage raw?

[interacsion]

also, does it need to show the content of the tag when using create etc' and/or the old content when using edit etc'?

[Zabuzard]

Should the bot log the action instantly, or wait for the routine?

Yeah, just dont connect this code to the routine at all. Imo they dont have anything to do with each other.

do we need to log it when someone uses /tag-manage raw?

No. Just the once that actually change something: create, edit, delete, ...

also, does it need to show the content of the tag when using create etc' and/or the old content when using edit etc'?

Yeah, this is actually crucial to be able to "revert" accidental changes. It has to log the state before and after the change. I would suggest to just attach them as "file" to the embed, like a before.md and after.md file or similar.

[interacsion]

I would suggest to just attach them as "file" to the embed, like a before.md and after.md file or similar.

The thing is, when I attach the file to the embed it puts the file before the embed. I think that sending the files in different messages is the way to go.

[Zabuzard]

The thing is, when I attach the file to the embed it puts the file before the embed. I think that sending the files in different messages is the way to go.

If you say so - your feature, your design 👍 If you need feedback, show some screenshots 👌

[java-coding-prodigy]

#281 (comment)
you could have 2 other embeds, one for before and one for after. Embeds are always in order luckily.

[interacsion]

Good to know, but I can't see how is that better than sending different messages.

[interacsion]

What do you think?

[Zabuzard]

As long as it clears that it belongs together, I do not care that much.

Be aware though that ideally you create a solution that cant be "interrupted" by other concurrent logs. So if you write independent messages, another log could be written in between, concurrently. Which would be super confusing, I guess.

So if there is a solution that has the stuff grouped together ("status message", content before, content after) and does not look shit, I would prefer that.

[interacsion]

So I've researched a bit more about putting the attachments after the embed and there's no proper way to do it. the only way I can think of to make sure nothing interrupts it between the messages is to use some kind of lock, but then (obviously) we have to instruct developers to always use it when sending messages in the #mod_audit_log channel.

[marko-radosavljevic]

Would ask @Tais993, our JDA expert, for opinion and ideas on this. ^^

[Tais993]

I saw him asking for help in the JDA Discord :p, and he's unfortunatelly correct.

There's no way of putting the attachment before the embed, so an unfortunate situation.

I'd honestly make a second channel for logging tag changes, easiest solution?

Otherwise you'll have to lock or accept that a message comes in between, unfortunately.

[interacsion]

I'd honestly make a second channel for logging tag changes, easiest solution?

@Zabuzard, @marko-radosavljevic, what's your opinion?

[Zabuzard]

Could you show how it looks like if you put the attachments together with the message? I am not sure I understand why you think it looks "bad".

However, I definitely want to avoid introducing locks or similar, such a setup would be far too fragile in my opinion. In such a case I would probably rather accept the risk of the message being interrupted, but change the title of the attachments so that its clear to which event they belong to.

That said, you can introduce synchronization on that particular class if you want. But dont lock the channel itself and also dont have the lock on the audit routine schedule. But only on this new feature that you are adding here, by simply synchronize-ing the method, if you want. Its not a big deal though.

[interacsion]

I would obviously make it synchronized if we would use the multiple message approach.
this is what it looks like on a single message, I assume you can see whats the problem with it.

(there're two actions in the screenshot)
as you can see, the files are appearing above the embed.

[Zabuzard]

In my opinion that is okay, really. It is just a log after all and also doesnt happen often. The channel is also restricted to a small audience.

Discord also cuts down the size of those files if they are really large, right? Is it possible to disable the text preview maybe? (also make sure u name those "files" with .md to get the syntax highlighting).

So I am voting for having a single embed with the 3 things together.

[interacsion]

Discord also cuts down the size of those files if they are really large, right?

yes, it cuts the vertical size of the file preview, but the horizontal size expands as much as it can.

Is it possible to disable the text preview maybe?

I am pretty sure that's impossible. we can make them show as a spoiler but it won't change the size of the preview.

I think I prefer it to send as different messages, maybe with a spoiler

[Zabuzard]

I think I prefer it to send as different messages, maybe with a spoiler

@marko-radosavljevic your opinion on this? (as we will be the primary user of this mod-only feature)

I think the spoilers are actually worse.

I am voting for same message, no spoilers - I do not care if its above or below or whatever, as long as its clear that its one message containing of 3 things that belong together.

[marko-radosavljevic]

Yup, same with me, it's not something we would use extensively, doesn't need to have a perfect UX.

Like Zabu said, above or below, doesn't matter. It's important that it is consistent, so we can always read it the same way, without getting confused and having to track down before and after. And it should be accurate, so we can depend on it. Everything else is just cosmetics. So yeah, I'm also voting for same message, no spoilers. ☺️

Thanks ❤️

[interacsion]

no problem, as you wish :)

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[Zabuzard]

Development on this is still ongoing and we want it (@MaiTheLord )

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.