Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Logging the /tag-manage command #296

Merged
merged 77 commits into from
Mar 14, 2022
Merged

Logging the /tag-manage command #296

merged 77 commits into from
Mar 14, 2022

Conversation

interacsion
Copy link
Contributor

@interacsion interacsion commented Dec 3, 2021 •
edited
Loading

What's the change?

/tag-manage actions are now logged on #mod_audit_log.

Why is it necessary?

It's enhancing the command /tag-manage. solving issue #281

What exactly did I add?

When executing the command successfully, the bot sends an embed on #mod_audit_log that's showing info about the executed command and the user who used it.

This is how it looks like:

image

@interacsion interacsion requested review from a team as code owners December 3, 2021 00:46
@CLAassistant
Copy link

CLAassistant commented Dec 3, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@Zabuzard Zabuzard added enhance command Modify or improve an existing command or group of commands of the bot priority: normal labels Dec 3, 2021
@Zabuzard Zabuzard linked an issue Dec 3, 2021 that may be closed by this pull request
Log tag changes #281
Closed
@Zabuzard Zabuzard added this to the Improvement phase 1 milestone Dec 3, 2021
@Zabuzard
Copy link
Member

Zabuzard commented Dec 3, 2021

For better UX, I added some details to the issue, see #281 (comment)

Kindly implement them as well, thanks 👍

They address in particular the massive cluttering and "spam" you would get with very long tag contents (for example /tag bfs) by simply having the content attached as a "file" instead of text. Also, addresses the issue that its then easier to copy paste to "undo" changes, as already suggessted for #273 .

RealYusufIsmail
RealYusufIsmail reviewed Dec 3, 2021
View reviewed changes
Copy link
Contributor

@RealYusufIsmail RealYusufIsmail left a comment

Choose a reason for hiding this comment

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

If you want adding a space and removing a space would make it look better

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/utils/ModAuditLogWriter.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/utils/ModAuditLogWriter.java Outdated Show resolved Hide resolved
RealYusufIsmail
RealYusufIsmail previously approved these changes Dec 4, 2021
View reviewed changes
Copy link
Contributor

@RealYusufIsmail RealYusufIsmail left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I approve since it look all good.

@Zabuzard
Copy link
Member

Zabuzard commented Dec 4, 2021 •
edited
Loading

Draft until the design is clear (see discussion in linked issue). (to prevent accidental merging)

@Zabuzard Zabuzard marked this pull request as draft December 4, 2021 11:05
@interacsion interacsion marked this pull request as ready for review December 4, 2021 17:36
Zabuzard
Zabuzard requested changes Dec 4, 2021
View reviewed changes
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/utils/VirtualFile.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/utils/VirtualFile.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/utils/VirtualFile.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/utils/VirtualFile.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/utils/VirtualFile.java Outdated Show resolved Hide resolved
@interacsion
Copy link
Contributor Author

interacsion commented Dec 5, 2021 •
edited
Loading

@Zabuzard, could you review my changes?

@RealYusufIsmail
Copy link
Contributor

@Zabuzard, could you review my changes?

No need to ping.

@Tais993
Copy link
Member

Tais993 commented Dec 5, 2021

image

Instead, re-request a review.
This shows up as a notification, so easier for Zabuzard.

@Tais993 Tais993 requested a review from Zabuzard December 5, 2021 19:25
@Zabuzard
Copy link
Member

Zabuzard commented Dec 6, 2021

Could you update the example pictures in the PR description?

Zabuzard
Zabuzard requested changes Dec 6, 2021
View reviewed changes
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
@Zabuzard
Copy link
Member

Zabuzard commented Dec 7, 2021

Tell me once the package situation is resolved and you think its "ready now". Then I will review again and mark everything resolved you already fixed 👍

Otherwise its kinda wasting time if I check again while "the design" is still changing around all the time 😄

@interacsion
Copy link
Contributor Author

interacsion commented Dec 7, 2021 •
edited
Loading

I have a few more things to fix and I am waiting for your opinion regarding this convo and this one.

I'll let you know once I fix everything.

@interacsion
Copy link
Contributor Author

I'll appreciate your opinion regarding this, this and this.

@interacsion
Copy link
Contributor Author

It should be ready for review now.

Zabuzard
Zabuzard reviewed Mar 12, 2022
View reviewed changes
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/moderation/ModAuditLogWriter.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/moderation/ModAuditLogWriter.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/routines/ModAuditLogRoutine.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/tags/TagManageCommand.java Outdated Show resolved Hide resolved
Zabuzard
Zabuzard previously approved these changes Mar 13, 2022
View reviewed changes
@Zabuzard Zabuzard merged commit dfa3696 into Together-Java:develop Mar 14, 2022
This was referenced Apr 6, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zabuzard Zabuzard Zabuzard approved these changes

@Tais993 Tais993 Tais993 approved these changes

@RealYusufIsmail RealYusufIsmail RealYusufIsmail left review comments

@illuminator3 illuminator3 Awaiting requested review from illuminator3

@Heatmanofurioso Heatmanofurioso Awaiting requested review from Heatmanofurioso

@java-coding-prodigy java-coding-prodigy Awaiting requested review from java-coding-prodigy

@marko-radosavljevic marko-radosavljevic Awaiting requested review from marko-radosavljevic

Assignees

@interacsion interacsion

Labels
enhance command Modify or improve an existing command or group of commands of the bot priority: major
Projects
None yet
Milestone
Improvement phase 1
Development

Successfully merging this pull request may close these issues.

Log tag changes
5 participants
@interacsion @CLAassistant @Zabuzard @RealYusufIsmail @Tais993