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.

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

Fixing extra modlog entries when post_id or comment_id is given. #4664

Merged
merged 2 commits into from
Apr 25, 2024

Conversation

dessalines
Copy link
Member

  • Previously when given a post_id, it didn't filter out any other modlog entries, such as community removals. This fixes that problem.
  • Context: Mod action history lemmy-ui#2437

- Previously when given a post_id, it didn't filter out any other
  modlog entries, such as community removals. This fixes that problem.
- Context: LemmyNet/lemmy-ui#2437
@@ -40,6 +40,11 @@ impl AdminPurgeCommentView {
query = query.filter(admin_purge_comment::admin_person_id.eq(admin_person_id));
};

// If a post or comment ID is given, then don't find any results
if params.post_id.is_some() || params.comment_id.is_some() {
Copy link
Member Author

Choose a reason for hiding this comment

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

I could've externalized this, but since the two other cases (only checking for one of them) also exist, it doesn't seem worth it.

Copy link
Member

Choose a reason for hiding this comment

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

Its good enough, I hope we can do a cleaner solution for this together with #2444

@@ -58,6 +58,11 @@ impl ModRemoveCommentView {
query = query.filter(comment::id.eq(comment_id));
}

// If a post ID is given, then don't find any results
if params.post_id.is_some() {
Copy link
Member Author

Choose a reason for hiding this comment

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

This one's the only weird case. We could show comment removals (since this has to join to the post table anyway, and filter by post_id) .

I tried it out in the front end, and although it works, it seems like if you provide a post id, you only want to see mod actions done on the post, not comments within the post.

I could be persuaded otherwise tho, as it might be useful to see actions taken on comments when filtering by a post id.

Copy link
Member

Choose a reason for hiding this comment

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

I think the approach you already made is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Both options should be possible, so there should be a "show_children" option that includes comments when given post_id, child comments when given comment_id, and maybe posts and comments when given community_id

Copy link
Member Author

Choose a reason for hiding this comment

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

Not a bad idea. Could you open an issue for that one? Lets leave that for a later time.

Copy link
Member

Choose a reason for hiding this comment

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

I think its fine like this.

@Nutomic Nutomic merged commit 8e3ff04 into main Apr 25, 2024
2 checks passed
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.

4 participants