From a0ea8dbc0000dc3f9a5e9b2abd1a8ac083bfe27f Mon Sep 17 00:00:00 2001 From: Nutomic Date: Tue, 5 Sep 2023 11:35:10 +0200 Subject: [PATCH] Auto resolve reports trigger (#3871) * Revert "Automatically resolve report when post/comment is removed (#3850)" This reverts commit f7f6766650b9b573a72075e564aed353c0131cd7. * Automatically resolve reports using db trigger * lint * use mod log tables * fix migration * fix ci * fix clippy --- crates/api_crud/src/comment/remove.rs | 6 +- crates/api_crud/src/post/remove.rs | 6 +- 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 | 41 +--- .../src/impls/private_message_report.rs | 12 +- crates/db_schema/src/traits.rs | 8 - crates/db_views/src/post_report_view.rs | 224 ++++-------------- .../down.sql | 8 + .../up.sql | 48 ++++ 10 files changed, 108 insertions(+), 275 deletions(-) create mode 100644 migrations/2023-09-01-112158_auto_resolve_report/down.sql create mode 100644 migrations/2023-09-01-112158_auto_resolve_report/up.sql diff --git a/crates/api_crud/src/comment/remove.rs b/crates/api_crud/src/comment/remove.rs index c18a8c5b7..2bb5c75f2 100644 --- a/crates/api_crud/src/comment/remove.rs +++ b/crates/api_crud/src/comment/remove.rs @@ -10,11 +10,10 @@ use lemmy_api_common::{ use lemmy_db_schema::{ source::{ comment::{Comment, CommentUpdateForm}, - comment_report::CommentReport, moderator::{ModRemoveComment, ModRemoveCommentForm}, post::Post, }, - traits::{Crud, Reportable}, + traits::Crud, }; use lemmy_db_views::structs::CommentView; use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType}; @@ -57,9 +56,6 @@ 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 2fbd6ccff..ee100cfdc 100644 --- a/crates/api_crud/src/post/remove.rs +++ b/crates/api_crud/src/post/remove.rs @@ -11,9 +11,8 @@ use lemmy_db_schema::{ source::{ moderator::{ModRemovePost, ModRemovePostForm}, post::{Post, PostUpdateForm}, - post_report::PostReport, }, - traits::{Crud, Reportable}, + traits::Crud, }; use lemmy_utils::error::LemmyError; @@ -55,9 +54,6 @@ 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/deletion/delete.rs b/crates/apub/src/activities/deletion/delete.rs index 537173b5f..af289640b 100644 --- a/crates/apub/src/activities/deletion/delete.rs +++ b/crates/apub/src/activities/deletion/delete.rs @@ -12,7 +12,6 @@ use lemmy_api_common::{context::LemmyContext, utils::sanitize_html_opt}; use lemmy_db_schema::{ source::{ comment::{Comment, CommentUpdateForm}, - comment_report::CommentReport, community::{Community, CommunityUpdateForm}, moderator::{ ModRemoveComment, @@ -23,9 +22,8 @@ use lemmy_db_schema::{ ModRemovePostForm, }, post::{Post, PostUpdateForm}, - post_report::PostReport, }, - traits::{Crud, Reportable}, + traits::Crud, }; use lemmy_utils::error::{LemmyError, LemmyErrorType}; use url::Url; @@ -133,7 +131,6 @@ 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, @@ -152,7 +149,6 @@ 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 19c12876f..ff93915e1 100644 --- a/crates/db_schema/src/impls/comment_report.rs +++ b/crates/db_schema/src/impls/comment_report.rs @@ -1,9 +1,6 @@ use crate::{ - newtypes::{CommentId, CommentReportId, PersonId}, - schema::comment_report::{ - comment_id, - dsl::{comment_report, resolved, resolver_id, updated}, - }, + newtypes::{CommentReportId, PersonId}, + schema::comment_report::dsl::{comment_report, resolved, resolver_id, updated}, source::comment_report::{CommentReport, CommentReportForm}, traits::Reportable, utils::{get_conn, naive_now, DbPool}, @@ -20,7 +17,6 @@ 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 @@ -57,22 +53,6 @@ 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 face766db..b4078d950 100644 --- a/crates/db_schema/src/impls/post_report.rs +++ b/crates/db_schema/src/impls/post_report.rs @@ -1,9 +1,6 @@ use crate::{ - newtypes::{PersonId, PostId, PostReportId}, - schema::post_report::{ - dsl::{post_report, resolved, resolver_id, updated}, - post_id, - }, + newtypes::{PersonId, PostReportId}, + schema::post_report::dsl::{post_report, resolved, resolver_id, updated}, source::post_report::{PostReport, PostReportForm}, traits::Reportable, utils::{get_conn, naive_now, DbPool}, @@ -20,7 +17,6 @@ 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?; @@ -46,22 +42,6 @@ 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, @@ -154,21 +134,4 @@ 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 c20783db0..ca2187960 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, PrivateMessageId, PrivateMessageReportId}, + newtypes::{PersonId, PrivateMessageReportId}, schema::private_message_report::dsl::{private_message_report, resolved, resolver_id, updated}, source::private_message_report::{PrivateMessageReport, PrivateMessageReportForm}, traits::Reportable, @@ -17,7 +17,6 @@ use diesel_async::RunQueryDsl; impl Reportable for PrivateMessageReport { type Form = PrivateMessageReportForm; type IdType = PrivateMessageReportId; - type ObjectIdType = PrivateMessageId; async fn report( pool: &mut DbPool<'_>, @@ -46,15 +45,6 @@ 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 4441923c1..9fa5a598c 100644 --- a/crates/db_schema/src/traits.rs +++ b/crates/db_schema/src/traits.rs @@ -155,7 +155,6 @@ pub trait Readable { pub trait Reportable { type Form; type IdType; - type ObjectIdType; async fn report(pool: &mut DbPool<'_>, form: &Self::Form) -> Result where Self: Sized; @@ -164,13 +163,6 @@ 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 1ab436935..324023ecf 100644 --- a/crates/db_views/src/post_report_view.rs +++ b/crates/db_views/src/post_report_view.rs @@ -194,11 +194,11 @@ mod tests { structs::LocalUserView, }; use lemmy_db_schema::{ - aggregates::structs::PostAggregates, source::{ community::{Community, CommunityInsertForm, CommunityModerator, CommunityModeratorForm}, instance::Instance, local_user::{LocalUser, LocalUserInsertForm}, + moderator::{ModRemovePost, ModRemovePostForm}, person::{Person, PersonInsertForm}, post::{Post, PostInsertForm}, post_report::{PostReport, PostReportForm}, @@ -291,12 +291,20 @@ mod tests { reason: "from sara".into(), }; - let inserted_sara_report = PostReport::report(pool, &sara_report_form).await.unwrap(); + PostReport::report(pool, &sara_report_form).await.unwrap(); + + let new_post_2 = PostInsertForm::builder() + .name("A test post crv 2".into()) + .creator_id(inserted_timmy.id) + .community_id(inserted_community.id) + .build(); + + let inserted_post_2 = Post::create(pool, &new_post_2).await.unwrap(); // jessica reports let jessica_report_form = PostReportForm { creator_id: inserted_jessica.id, - post_id: inserted_post.id, + post_id: inserted_post_2.id, original_post_name: "Orig post".into(), original_post_url: None, original_post_body: None, @@ -307,138 +315,21 @@ mod tests { .await .unwrap(); - let agg = PostAggregates::read(pool, inserted_post.id).await.unwrap(); - let read_jessica_report_view = PostReportView::read(pool, inserted_jessica_report.id, inserted_timmy.id) .await .unwrap(); - let expected_jessica_report_view = PostReportView { - post_report: inserted_jessica_report.clone(), - post: inserted_post.clone(), - community: Community { - id: inserted_community.id, - name: inserted_community.name, - icon: None, - removed: false, - deleted: false, - nsfw: false, - actor_id: inserted_community.actor_id.clone(), - local: true, - title: inserted_community.title, - description: None, - updated: None, - banner: None, - hidden: false, - posting_restricted_to_mods: false, - published: inserted_community.published, - instance_id: inserted_instance.id, - private_key: inserted_community.private_key.clone(), - public_key: inserted_community.public_key.clone(), - last_refreshed_at: inserted_community.last_refreshed_at, - followers_url: inserted_community.followers_url.clone(), - inbox_url: inserted_community.inbox_url.clone(), - shared_inbox_url: inserted_community.shared_inbox_url.clone(), - moderators_url: inserted_community.moderators_url.clone(), - featured_url: inserted_community.featured_url.clone(), - }, - creator: Person { - id: inserted_jessica.id, - name: inserted_jessica.name, - display_name: None, - published: inserted_jessica.published, - avatar: None, - actor_id: inserted_jessica.actor_id.clone(), - local: true, - banned: false, - deleted: false, - bot_account: false, - bio: None, - banner: None, - updated: None, - inbox_url: inserted_jessica.inbox_url.clone(), - shared_inbox_url: None, - matrix_user_id: None, - ban_expires: None, - instance_id: inserted_instance.id, - private_key: inserted_jessica.private_key, - public_key: inserted_jessica.public_key, - last_refreshed_at: inserted_jessica.last_refreshed_at, - }, - post_creator: Person { - id: inserted_timmy.id, - name: inserted_timmy.name.clone(), - display_name: None, - published: inserted_timmy.published, - avatar: None, - actor_id: inserted_timmy.actor_id.clone(), - local: true, - banned: false, - deleted: false, - bot_account: false, - bio: None, - banner: None, - updated: None, - inbox_url: inserted_timmy.inbox_url.clone(), - shared_inbox_url: None, - matrix_user_id: None, - ban_expires: None, - instance_id: inserted_instance.id, - private_key: inserted_timmy.private_key.clone(), - public_key: inserted_timmy.public_key.clone(), - last_refreshed_at: inserted_timmy.last_refreshed_at, - }, - creator_banned_from_community: false, - my_vote: None, - counts: PostAggregates { - id: agg.id, - post_id: inserted_post.id, - comments: 0, - score: 0, - upvotes: 0, - downvotes: 0, - published: agg.published, - newest_comment_time_necro: inserted_post.published, - newest_comment_time: inserted_post.published, - featured_community: false, - featured_local: false, - hot_rank: 1728, - hot_rank_active: 1728, - controversy_rank: 0.0, - community_id: inserted_post.community_id, - creator_id: inserted_post.creator_id, - }, - resolver: None, - }; - assert_eq!(read_jessica_report_view, expected_jessica_report_view); - - let mut expected_sara_report_view = expected_jessica_report_view.clone(); - expected_sara_report_view.post_report = inserted_sara_report; - expected_sara_report_view.my_vote = None; - expected_sara_report_view.creator = Person { - id: inserted_sara.id, - name: inserted_sara.name, - display_name: None, - published: inserted_sara.published, - avatar: None, - actor_id: inserted_sara.actor_id.clone(), - local: true, - banned: false, - deleted: false, - bot_account: false, - bio: None, - banner: None, - updated: None, - inbox_url: inserted_sara.inbox_url.clone(), - shared_inbox_url: None, - matrix_user_id: None, - ban_expires: None, - instance_id: inserted_instance.id, - private_key: inserted_sara.private_key, - public_key: inserted_sara.public_key, - last_refreshed_at: inserted_sara.last_refreshed_at, - }; + assert_eq!( + read_jessica_report_view.post_report, + inserted_jessica_report + ); + assert_eq!(read_jessica_report_view.post, inserted_post_2); + assert_eq!(read_jessica_report_view.community.id, inserted_community.id); + assert_eq!(read_jessica_report_view.creator.id, inserted_jessica.id); + assert_eq!(read_jessica_report_view.post_creator.id, inserted_timmy.id); + assert_eq!(read_jessica_report_view.my_vote, None); + assert_eq!(read_jessica_report_view.resolver, None); // Do a batch read of timmys reports let reports = PostReportQuery::default() @@ -446,13 +337,8 @@ mod tests { .await .unwrap(); - assert_eq!( - reports, - [ - expected_jessica_report_view.clone(), - expected_sara_report_view.clone() - ] - ); + assert_eq!(reports[0].creator.id, inserted_jessica.id); + assert_eq!(reports[1].creator.id, inserted_sara.id); // Make sure the counts are correct let report_count = PostReportView::get_report_count(pool, inserted_timmy.id, false, None) @@ -460,64 +346,42 @@ mod tests { .unwrap(); assert_eq!(2, report_count); - // Try to resolve the report - PostReport::resolve(pool, inserted_jessica_report.id, inserted_timmy.id) - .await - .unwrap(); + // 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(); + let read_jessica_report_view_after_resolve = PostReportView::read(pool, inserted_jessica_report.id, inserted_timmy.id) .await .unwrap(); - - let mut expected_jessica_report_view_after_resolve = expected_jessica_report_view; - expected_jessica_report_view_after_resolve - .post_report - .resolved = true; - expected_jessica_report_view_after_resolve - .post_report - .resolver_id = Some(inserted_timmy.id); - expected_jessica_report_view_after_resolve - .post_report - .updated = read_jessica_report_view_after_resolve.post_report.updated; - expected_jessica_report_view_after_resolve.resolver = Some(Person { - id: inserted_timmy.id, - name: inserted_timmy.name.clone(), - display_name: None, - published: inserted_timmy.published, - avatar: None, - actor_id: inserted_timmy.actor_id.clone(), - local: true, - banned: false, - deleted: false, - bot_account: false, - bio: None, - banner: None, - updated: None, - inbox_url: inserted_timmy.inbox_url.clone(), - shared_inbox_url: None, - matrix_user_id: None, - ban_expires: None, - instance_id: inserted_instance.id, - private_key: inserted_timmy.private_key.clone(), - public_key: inserted_timmy.public_key.clone(), - last_refreshed_at: inserted_timmy.last_refreshed_at, - }); - + assert!(read_jessica_report_view_after_resolve.post_report.resolved); + assert_eq!( + read_jessica_report_view_after_resolve + .post_report + .resolver_id, + Some(inserted_timmy.id) + ); assert_eq!( - read_jessica_report_view_after_resolve, - expected_jessica_report_view_after_resolve + read_jessica_report_view_after_resolve.resolver.unwrap().id, + inserted_timmy.id ); // Do a batch read of timmys reports // It should only show saras, which is unresolved let reports_after_resolve = PostReportQuery { - unresolved_only: (true), + unresolved_only: true, ..Default::default() } .list(pool, &timmy_view) .await .unwrap(); - assert_eq!(reports_after_resolve[0], expected_sara_report_view); + assert_eq!(reports_after_resolve.len(), 1); + assert_eq!(reports_after_resolve[0].creator.id, inserted_sara.id); // Make sure the counts are correct let report_count_after_resolved = diff --git a/migrations/2023-09-01-112158_auto_resolve_report/down.sql b/migrations/2023-09-01-112158_auto_resolve_report/down.sql new file mode 100644 index 000000000..0ea58fc3c --- /dev/null +++ b/migrations/2023-09-01-112158_auto_resolve_report/down.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; + diff --git a/migrations/2023-09-01-112158_auto_resolve_report/up.sql b/migrations/2023-09-01-112158_auto_resolve_report/up.sql new file mode 100644 index 000000000..101fe441c --- /dev/null +++ b/migrations/2023-09-01-112158_auto_resolve_report/up.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 (); +