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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/admin_purge_comment_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/admin_purge_community_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ impl AdminPurgeCommunityView {
query = query.filter(admin_purge_community::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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/admin_purge_person_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ impl AdminPurgePersonView {
query = query.filter(admin_purge_person::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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/admin_purge_post_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ impl AdminPurgePostView {
query = query.filter(admin_purge_post::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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/mod_add_community_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ impl ModAddCommunityView {
query = query.filter(person_alias_1.field(person::id).eq(other_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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/mod_add_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ impl ModAddView {
query = query.filter(person_alias_1.field(person::id).eq(other_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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ impl ModBanFromCommunityView {
query = query.filter(mod_ban_from_community::other_person_id.eq(other_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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/mod_ban_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ impl ModBanView {
query = query.filter(person_alias_1.field(person::id).eq(other_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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/mod_feature_post_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ impl ModFeaturePostView {
query = query.filter(post::id.eq(post_id));
}

// If a comment ID is given, then don't find any results
if params.comment_id.is_some() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/mod_hide_community_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ impl ModHideCommunityView {
query = query.filter(mod_hide_community::mod_person_id.eq(admin_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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/mod_lock_post_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ impl ModLockPostView {
query = query.filter(post::id.eq(post_id));
}

// If a comment ID is given, then don't find any results
if params.comment_id.is_some() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/mod_remove_comment_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.

return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/mod_remove_community_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ impl ModRemoveCommunityView {
query = query.filter(mod_remove_community::mod_person_id.eq(mod_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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
5 changes: 5 additions & 0 deletions crates/db_views_moderator/src/mod_remove_post_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ impl ModRemovePostView {
query = query.filter(post::id.eq(post_id));
}

// If a comment ID is given, then don't find any results
if params.comment_id.is_some() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ impl ModTransferCommunityView {
query = query.filter(person_alias_1.field(person::id).eq(other_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() {
return Ok(vec![]);
}

let (limit, offset) = limit_and_offset(params.page, params.limit)?;

query
Expand Down