From a3bf2f1cf12c30a6df299105735ff3ebf90b4f5b Mon Sep 17 00:00:00 2001 From: Dessalines Date: Thu, 15 Feb 2024 08:52:04 -0500 Subject: [PATCH] Auto resolve reports on removing a comment or post. Fixes #4390 (#4402) * Automatically resolve report when post/comment is removed (#3850) * Automatically resolve report when post/comment is removed * also handle apub removes * Removing auto-resolve report triggers. * Dont allow creating reports for deleted / removed items. * Running pgformat. * Fixing test. * Addressing PR comments. * Forgot comment report. --------- Co-authored-by: Nutomic --- crates/api/src/comment_report/create.rs | 9 +++- crates/api/src/post_report/create.rs | 8 +++- crates/api_common/src/utils.rs | 9 ++++ crates/api_crud/src/comment/remove.rs | 6 ++- crates/api_crud/src/post/remove.rs | 6 ++- .../apub/src/activities/community/report.rs | 9 +++- crates/apub/src/activities/deletion/delete.rs | 6 ++- crates/db_schema/src/impls/comment_report.rs | 24 +++++++++- crates/db_schema/src/impls/post_report.rs | 42 ++++++++++++++-- .../src/impls/private_message_report.rs | 12 ++++- crates/db_schema/src/traits.rs | 8 ++++ crates/db_views/src/post_report_view.rs | 14 ++---- .../down.sql | 48 +++++++++++++++++++ .../up.sql | 8 ++++ 14 files changed, 188 insertions(+), 21 deletions(-) create mode 100644 migrations/2024-01-25-151400_remove_auto_resolve_report_trigger/down.sql create mode 100644 migrations/2024-01-25-151400_remove_auto_resolve_report_trigger/up.sql diff --git a/crates/api/src/comment_report/create.rs b/crates/api/src/comment_report/create.rs index 51f972b57..f8075460f 100644 --- a/crates/api/src/comment_report/create.rs +++ b/crates/api/src/comment_report/create.rs @@ -5,7 +5,11 @@ use lemmy_api_common::{ comment::{CommentReportResponse, CreateCommentReport}, context::LemmyContext, send_activity::{ActivityChannel, SendActivityData}, - utils::{check_community_user_action, send_new_report_email_to_admins}, + utils::{ + check_comment_deleted_or_removed, + check_community_user_action, + send_new_report_email_to_admins, + }, }; use lemmy_db_schema::{ source::{ @@ -40,6 +44,9 @@ pub async fn create_comment_report( ) .await?; + // Don't allow creating reports for removed / deleted comments + check_comment_deleted_or_removed(&comment_view.comment)?; + let report_form = CommentReportForm { creator_id: person_id, comment_id, diff --git a/crates/api/src/post_report/create.rs b/crates/api/src/post_report/create.rs index 0840cfee4..1327d7bb9 100644 --- a/crates/api/src/post_report/create.rs +++ b/crates/api/src/post_report/create.rs @@ -5,7 +5,11 @@ use lemmy_api_common::{ context::LemmyContext, post::{CreatePostReport, PostReportResponse}, send_activity::{ActivityChannel, SendActivityData}, - utils::{check_community_user_action, send_new_report_email_to_admins}, + utils::{ + check_community_user_action, + check_post_deleted_or_removed, + send_new_report_email_to_admins, + }, }; use lemmy_db_schema::{ source::{ @@ -40,6 +44,8 @@ pub async fn create_post_report( ) .await?; + check_post_deleted_or_removed(&post_view.post)?; + let report_form = PostReportForm { creator_id: person_id, post_id, diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 55df7a6ec..138364c84 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -225,6 +225,7 @@ pub async fn check_community_mod_action( Ok(()) } +/// Don't allow creating reports for removed / deleted posts pub fn check_post_deleted_or_removed(post: &Post) -> Result<(), LemmyError> { if post.deleted || post.removed { Err(LemmyErrorType::Deleted)? @@ -233,6 +234,14 @@ pub fn check_post_deleted_or_removed(post: &Post) -> Result<(), LemmyError> { } } +pub fn check_comment_deleted_or_removed(comment: &Comment) -> Result<(), LemmyError> { + if comment.deleted || comment.removed { + Err(LemmyErrorType::Deleted)? + } else { + Ok(()) + } +} + /// Throws an error if a recipient has blocked a person. #[tracing::instrument(skip_all)] pub async fn check_person_block( diff --git a/crates/api_crud/src/comment/remove.rs b/crates/api_crud/src/comment/remove.rs index 4642078dd..5bb6f55b1 100644 --- a/crates/api_crud/src/comment/remove.rs +++ b/crates/api_crud/src/comment/remove.rs @@ -10,10 +10,11 @@ use lemmy_api_common::{ use lemmy_db_schema::{ source::{ comment::{Comment, CommentUpdateForm}, + comment_report::CommentReport, moderator::{ModRemoveComment, ModRemoveCommentForm}, post::Post, }, - traits::Crud, + traits::{Crud, Reportable}, }; use lemmy_db_views::structs::{CommentView, LocalUserView}; use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; @@ -48,6 +49,9 @@ pub async fn remove_comment( .await .with_lemmy_type(LemmyErrorType::CouldntUpdateComment)?; + CommentReport::resolve_all_for_object(&mut context.pool(), comment_id, local_user_view.person.id) + .await?; + // Mod tables let form = ModRemoveCommentForm { mod_person_id: local_user_view.person.id, diff --git a/crates/api_crud/src/post/remove.rs b/crates/api_crud/src/post/remove.rs index 5b3b33a2e..cbcf069b6 100644 --- a/crates/api_crud/src/post/remove.rs +++ b/crates/api_crud/src/post/remove.rs @@ -11,8 +11,9 @@ use lemmy_db_schema::{ source::{ moderator::{ModRemovePost, ModRemovePostForm}, post::{Post, PostUpdateForm}, + post_report::PostReport, }, - traits::Crud, + traits::{Crud, Reportable}, }; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyError; @@ -47,6 +48,9 @@ pub async fn remove_post( ) .await?; + PostReport::resolve_all_for_object(&mut context.pool(), post_id, local_user_view.person.id) + .await?; + // Mod tables let form = ModRemovePostForm { mod_person_id: local_user_view.person.id, diff --git a/crates/apub/src/activities/community/report.rs b/crates/apub/src/activities/community/report.rs index 11941675a..6b1fce066 100644 --- a/crates/apub/src/activities/community/report.rs +++ b/crates/apub/src/activities/community/report.rs @@ -14,7 +14,10 @@ use activitypub_federation::{ kinds::activity::FlagType, traits::{ActivityHandler, Actor}, }; -use lemmy_api_common::context::LemmyContext; +use lemmy_api_common::{ + context::LemmyContext, + utils::{check_comment_deleted_or_removed, check_post_deleted_or_removed}, +}; use lemmy_db_schema::{ source::{ activity::ActivitySendTargets, @@ -104,6 +107,8 @@ impl ActivityHandler for Report { let reason = self.reason()?; match self.object.dereference(context).await? { PostOrComment::Post(post) => { + check_post_deleted_or_removed(&post)?; + let report_form = PostReportForm { creator_id: actor.id, post_id: post.id, @@ -115,6 +120,8 @@ impl ActivityHandler for Report { PostReport::report(&mut context.pool(), &report_form).await?; } PostOrComment::Comment(comment) => { + check_comment_deleted_or_removed(&comment)?; + let report_form = CommentReportForm { creator_id: actor.id, comment_id: comment.id, diff --git a/crates/apub/src/activities/deletion/delete.rs b/crates/apub/src/activities/deletion/delete.rs index 875dc9e89..18f8cf6fb 100644 --- a/crates/apub/src/activities/deletion/delete.rs +++ b/crates/apub/src/activities/deletion/delete.rs @@ -12,6 +12,7 @@ use lemmy_api_common::context::LemmyContext; use lemmy_db_schema::{ source::{ comment::{Comment, CommentUpdateForm}, + comment_report::CommentReport, community::{Community, CommunityUpdateForm}, moderator::{ ModRemoveComment, @@ -22,8 +23,9 @@ use lemmy_db_schema::{ ModRemovePostForm, }, post::{Post, PostUpdateForm}, + post_report::PostReport, }, - traits::Crud, + traits::{Crud, Reportable}, }; use lemmy_utils::error::{LemmyError, LemmyErrorType}; use url::Url; @@ -136,6 +138,7 @@ pub(in crate::activities) async fn receive_remove_action( .await?; } DeletableObjects::Post(post) => { + PostReport::resolve_all_for_object(&mut context.pool(), post.id, actor.id).await?; let form = ModRemovePostForm { mod_person_id: actor.id, post_id: post.id, @@ -154,6 +157,7 @@ pub(in crate::activities) async fn receive_remove_action( .await?; } DeletableObjects::Comment(comment) => { + CommentReport::resolve_all_for_object(&mut context.pool(), comment.id, actor.id).await?; let form = ModRemoveCommentForm { mod_person_id: actor.id, comment_id: comment.id, diff --git a/crates/db_schema/src/impls/comment_report.rs b/crates/db_schema/src/impls/comment_report.rs index ff93915e1..19c12876f 100644 --- a/crates/db_schema/src/impls/comment_report.rs +++ b/crates/db_schema/src/impls/comment_report.rs @@ -1,6 +1,9 @@ use crate::{ - newtypes::{CommentReportId, PersonId}, - schema::comment_report::dsl::{comment_report, resolved, resolver_id, updated}, + newtypes::{CommentId, CommentReportId, PersonId}, + schema::comment_report::{ + comment_id, + dsl::{comment_report, resolved, resolver_id, updated}, + }, source::comment_report::{CommentReport, CommentReportForm}, traits::Reportable, utils::{get_conn, naive_now, DbPool}, @@ -17,6 +20,7 @@ use diesel_async::RunQueryDsl; impl Reportable for CommentReport { type Form = CommentReportForm; type IdType = CommentReportId; + type ObjectIdType = CommentId; /// creates a comment report and returns it /// /// * `conn` - the postgres connection @@ -53,6 +57,22 @@ impl Reportable for CommentReport { .await } + async fn resolve_all_for_object( + pool: &mut DbPool<'_>, + comment_id_: CommentId, + by_resolver_id: PersonId, + ) -> Result { + let conn = &mut get_conn(pool).await?; + update(comment_report.filter(comment_id.eq(comment_id_))) + .set(( + resolved.eq(true), + resolver_id.eq(by_resolver_id), + updated.eq(naive_now()), + )) + .execute(conn) + .await + } + /// unresolve a comment report /// /// * `conn` - the postgres connection diff --git a/crates/db_schema/src/impls/post_report.rs b/crates/db_schema/src/impls/post_report.rs index b0071f965..face766db 100644 --- a/crates/db_schema/src/impls/post_report.rs +++ b/crates/db_schema/src/impls/post_report.rs @@ -1,6 +1,9 @@ use crate::{ - newtypes::{PersonId, PostReportId}, - schema::post_report::dsl::{post_report, resolved, resolver_id, updated}, + newtypes::{PersonId, PostId, PostReportId}, + schema::post_report::{ + dsl::{post_report, resolved, resolver_id, updated}, + post_id, + }, source::post_report::{PostReport, PostReportForm}, traits::Reportable, utils::{get_conn, naive_now, DbPool}, @@ -17,6 +20,7 @@ use diesel_async::RunQueryDsl; impl Reportable for PostReport { type Form = PostReportForm; type IdType = PostReportId; + type ObjectIdType = PostId; async fn report(pool: &mut DbPool<'_>, post_report_form: &PostReportForm) -> Result { let conn = &mut get_conn(pool).await?; @@ -42,6 +46,22 @@ impl Reportable for PostReport { .await } + async fn resolve_all_for_object( + pool: &mut DbPool<'_>, + post_id_: PostId, + by_resolver_id: PersonId, + ) -> Result { + let conn = &mut get_conn(pool).await?; + update(post_report.filter(post_id.eq(post_id_))) + .set(( + resolved.eq(true), + resolver_id.eq(by_resolver_id), + updated.eq(naive_now()), + )) + .execute(conn) + .await + } + async fn unresolve( pool: &mut DbPool<'_>, report_id: Self::IdType, @@ -75,7 +95,6 @@ mod tests { traits::Crud, utils::build_db_pool_for_tests, }; - use pretty_assertions::assert_eq; use serial_test::serial; async fn init(pool: &mut DbPool<'_>) -> (Person, PostReport) { @@ -135,4 +154,21 @@ mod tests { Person::delete(pool, person.id).await.unwrap(); Post::delete(pool, report.post_id).await.unwrap(); } + + #[tokio::test] + #[serial] + async fn test_resolve_all_post_reports() { + let pool = &build_db_pool_for_tests().await; + let pool = &mut pool.into(); + + let (person, report) = init(pool).await; + + let resolved_count = PostReport::resolve_all_for_object(pool, report.post_id, person.id) + .await + .unwrap(); + assert_eq!(resolved_count, 1); + + Person::delete(pool, person.id).await.unwrap(); + Post::delete(pool, report.post_id).await.unwrap(); + } } diff --git a/crates/db_schema/src/impls/private_message_report.rs b/crates/db_schema/src/impls/private_message_report.rs index ca2187960..c20783db0 100644 --- a/crates/db_schema/src/impls/private_message_report.rs +++ b/crates/db_schema/src/impls/private_message_report.rs @@ -1,5 +1,5 @@ use crate::{ - newtypes::{PersonId, PrivateMessageReportId}, + newtypes::{PersonId, PrivateMessageId, PrivateMessageReportId}, schema::private_message_report::dsl::{private_message_report, resolved, resolver_id, updated}, source::private_message_report::{PrivateMessageReport, PrivateMessageReportForm}, traits::Reportable, @@ -17,6 +17,7 @@ use diesel_async::RunQueryDsl; impl Reportable for PrivateMessageReport { type Form = PrivateMessageReportForm; type IdType = PrivateMessageReportId; + type ObjectIdType = PrivateMessageId; async fn report( pool: &mut DbPool<'_>, @@ -45,6 +46,15 @@ impl Reportable for PrivateMessageReport { .await } + // TODO: this is unused because private message doesnt have remove handler + async fn resolve_all_for_object( + _pool: &mut DbPool<'_>, + _pm_id_: PrivateMessageId, + _by_resolver_id: PersonId, + ) -> Result { + unimplemented!() + } + async fn unresolve( pool: &mut DbPool<'_>, report_id: Self::IdType, diff --git a/crates/db_schema/src/traits.rs b/crates/db_schema/src/traits.rs index e58319c0b..b0434a65c 100644 --- a/crates/db_schema/src/traits.rs +++ b/crates/db_schema/src/traits.rs @@ -144,6 +144,7 @@ pub trait Blockable { pub trait Reportable { type Form; type IdType; + type ObjectIdType; async fn report(pool: &mut DbPool<'_>, form: &Self::Form) -> Result where Self: Sized; @@ -152,6 +153,13 @@ pub trait Reportable { report_id: Self::IdType, resolver_id: PersonId, ) -> Result + where + Self: Sized; + async fn resolve_all_for_object( + pool: &mut DbPool<'_>, + comment_id_: Self::ObjectIdType, + by_resolver_id: PersonId, + ) -> Result where Self: Sized; async fn unresolve( diff --git a/crates/db_views/src/post_report_view.rs b/crates/db_views/src/post_report_view.rs index c503ae81a..b7c5df6a9 100644 --- a/crates/db_views/src/post_report_view.rs +++ b/crates/db_views/src/post_report_view.rs @@ -201,7 +201,6 @@ mod tests { community::{Community, CommunityInsertForm, CommunityModerator, CommunityModeratorForm}, instance::Instance, local_user::{LocalUser, LocalUserInsertForm}, - moderator::{ModRemovePost, ModRemovePostForm}, person::{Person, PersonInsertForm}, post::{Post, PostInsertForm}, post_report::{PostReport, PostReportForm}, @@ -350,14 +349,11 @@ mod tests { .unwrap(); assert_eq!(2, report_count); - // Writing post removal to mod log should automatically resolve reports - let remove_form = ModRemovePostForm { - mod_person_id: inserted_timmy.id, - post_id: inserted_jessica_report.post_id, - reason: None, - removed: Some(true), - }; - ModRemovePost::create(pool, &remove_form).await.unwrap(); + // Pretend the post was removed, and resolve all reports for that object. + // This is called manually in the API for post removals + PostReport::resolve_all_for_object(pool, inserted_jessica_report.post_id, inserted_timmy.id) + .await + .unwrap(); let read_jessica_report_view_after_resolve = PostReportView::read(pool, inserted_jessica_report.id, inserted_timmy.id) diff --git a/migrations/2024-01-25-151400_remove_auto_resolve_report_trigger/down.sql b/migrations/2024-01-25-151400_remove_auto_resolve_report_trigger/down.sql new file mode 100644 index 000000000..101fe441c --- /dev/null +++ b/migrations/2024-01-25-151400_remove_auto_resolve_report_trigger/down.sql @@ -0,0 +1,48 @@ +-- Automatically resolve all reports for a given post once it is marked as removed +CREATE OR REPLACE FUNCTION post_removed_resolve_reports () + RETURNS TRIGGER + LANGUAGE plpgsql + AS $$ +BEGIN + UPDATE + post_report + SET + resolved = TRUE, + resolver_id = NEW.mod_person_id, + updated = now() + WHERE + post_report.post_id = NEW.post_id; + RETURN NULL; +END +$$; + +CREATE OR REPLACE TRIGGER post_removed_resolve_reports + AFTER INSERT ON mod_remove_post + FOR EACH ROW + WHEN (NEW.removed) + EXECUTE PROCEDURE post_removed_resolve_reports (); + +-- Same when comment is marked as removed +CREATE OR REPLACE FUNCTION comment_removed_resolve_reports () + RETURNS TRIGGER + LANGUAGE plpgsql + AS $$ +BEGIN + UPDATE + comment_report + SET + resolved = TRUE, + resolver_id = NEW.mod_person_id, + updated = now() + WHERE + comment_report.comment_id = NEW.comment_id; + RETURN NULL; +END +$$; + +CREATE OR REPLACE TRIGGER comment_removed_resolve_reports + AFTER INSERT ON mod_remove_comment + FOR EACH ROW + WHEN (NEW.removed) + EXECUTE PROCEDURE comment_removed_resolve_reports (); + diff --git a/migrations/2024-01-25-151400_remove_auto_resolve_report_trigger/up.sql b/migrations/2024-01-25-151400_remove_auto_resolve_report_trigger/up.sql new file mode 100644 index 000000000..0ea58fc3c --- /dev/null +++ b/migrations/2024-01-25-151400_remove_auto_resolve_report_trigger/up.sql @@ -0,0 +1,8 @@ +DROP TRIGGER IF EXISTS post_removed_resolve_reports ON mod_remove_post; + +DROP FUNCTION IF EXISTS post_removed_resolve_reports; + +DROP TRIGGER IF EXISTS comment_removed_resolve_reports ON mod_remove_comment; + +DROP FUNCTION IF EXISTS comment_removed_resolve_reports; +