Added /audit button to reports (moderation)#1499
Conversation
|
Please add screenshots of the feature and all edge cases in action, makes reviewing it muuuch easier...^^ |
Zabuzard
changed the title
|
Made some changes as you asked. |
|
(please dont resolve comments urself, let the author of the comment do it as confirmation that they consider it done, thanks) |
|
sorry I'm kinda new to this pr stuff |
here are the screenshots too |
|
|
So it shows the last page only now? not sure how i feel about that. the user could have 100 pages and then u show only a single item bc the last page happened to only include one item yet. it should at least show a full page or have buttons i guess. also, shouldnt it be page 1 and not the last? im pretty sure /audit sorts it with the most recent actions shown first, with a full page |
everything works and the pages turn too I forgot to take screenshot 😭 but tested it and it worked just as intended. |
surajkumar
left a comment
surajkumar
left a comment
There was a problem hiding this comment.
Small change but the rest of the PR at my glance looks good. Will be happy to provide the second tick on Zabu's approval
this time I didn't forget to take screenshots for page turning |
it works I didn't think it would need all the screenshots
|
Made the changes and also changed the name to .json.template this time sorry for the earlier mistake. |
|
and yeah its not the same screenshot every time I take a new one every time I change the code to show that it is tested and works |
Tested it with "1234567890L" id and it didn't crash just gave the message and also made it ephemeral because Christolis asked it to be ephemeral. |
|
and also if you like some name changes tell me, I'm not good at naming. |
|
Looked at it thoroughly and changed the names, there might be some that I missed so if any, tell me I'll change them. |
christolis
left a comment
christolis
left a comment
There was a problem hiding this comment.
While testing this I noticed that its behavior is a little different from the /audit command.
More specifically, assuming that a member has more than a page of actions taken against them, while both this new audit button and the original audit command sort the actions by recency, the new audit command inverts the pages if that makes sense.
I'll let the following screenshots tell the story:
Original /audit command
New Audit button
Notice how on the same user, in the new Audit button, the actions are stacked in the first page.
Another thing I have noticed is that when a user turns a page in an ephemeral message after clicking the Audit button, a new embed is send, but perhaps this is a limitation and I am fine with it if that's the case.
Pressing "Next" or "Previous" sends an entirely new message instead of editing that ephemeral message.
| import org.togetherjava.tjbot.features.utils.AmbientColors; | ||
| import org.togetherjava.tjbot.features.utils.MessageUtils; | ||
|
|
||
| import java.awt.*; |
| import java.awt.*; | ||
| import java.time.Instant; | ||
| import java.time.temporal.ChronoUnit; | ||
| import java.util.*; |
| import net.dv8tion.jda.api.entities.Message; | ||
| import net.dv8tion.jda.api.entities.MessageEmbed; | ||
| import net.dv8tion.jda.api.entities.Role; | ||
| import net.dv8tion.jda.api.entities.*; |
| private static List<List<ActionRecord>> groupActionsByPages(List<ActionRecord> actions) { | ||
| return ModerationUtils.groupActionsByPages(actions); | ||
| } |
There was a problem hiding this comment.
What is preventing us from entirely removing this function from this class and directly referencing ModerationUtils.groupActionsByPages() where it's needed? It has one usage anyways, specifically in AuditCommand.java in line 107:
List<List<ActionRecord>> groupedActions = groupActionsByPages(actions);| * Generates a structural text overview outlining the count total of each action type. | ||
| * | ||
| * @param moderationActions a collection of history records | ||
| * @return a formatted markdown description summary |
There was a problem hiding this comment.
NIT: The "markdown" word should be capitalized, considering you are referring to the formatting language.
| EmbedBuilder auditEmbed = | ||
| new EmbedBuilder().setTitle("Audit log of **%s**".formatted(user.getName())) | ||
| .setAuthor(user.getName(), null, user.getEffectiveAvatarUrl()) | ||
| .setColor(Color.BLACK) |
There was a problem hiding this comment.
Please make use of the recently introduced AmbientColors class to set the color of this embed.
…mports and some other changes
4 participants
Adds a button that lets the moderator that clicked it see the audit log of the reported user.
Resolves #1498