From c1785d446ccb1b4becd5d2c243d98921407011ef Mon Sep 17 00:00:00 2001 From: Dessalines Date: Tue, 24 Sep 2024 13:09:35 -0400 Subject: [PATCH 1/5] Add modlog entries for bulk removals. - Added unit tests for removal / restore to api_common/utils. - Fixes #4699 --- crates/api/src/community/ban.rs | 2 + crates/api/src/local_user/ban_person.rs | 4 +- crates/api_common/src/utils.rs | 320 ++++++++++++++++-- .../apub/src/activities/block/block_user.rs | 9 +- .../src/activities/block/undo_block_user.rs | 4 +- crates/db_schema/src/impls/moderator.rs | 128 +++---- 6 files changed, 370 insertions(+), 97 deletions(-) diff --git a/crates/api/src/community/ban.rs b/crates/api/src/community/ban.rs index f7da2154f..64b1c7196 100644 --- a/crates/api/src/community/ban.rs +++ b/crates/api/src/community/ban.rs @@ -92,8 +92,10 @@ pub async fn ban_from_community( let remove_data = data.ban; remove_or_restore_user_data_in_community( data.community_id, + local_user_view.person.id, banned_person_id, remove_data, + &data.reason, &mut context.pool(), ) .await?; diff --git a/crates/api/src/local_user/ban_person.rs b/crates/api/src/local_user/ban_person.rs index 7ab8ad72a..cf660a432 100644 --- a/crates/api/src/local_user/ban_person.rs +++ b/crates/api/src/local_user/ban_person.rs @@ -67,9 +67,9 @@ pub async fn ban_from_site( // Remove their data if that's desired if data.remove_or_restore_data.unwrap_or(false) { if data.ban { - remove_user_data(person.id, &context).await?; + remove_user_data(local_user_view.person.id, person.id, &data.reason, &context).await?; } else { - restore_user_data(person.id, &context).await?; + restore_user_data(local_user_view.person.id, person.id, &data.reason, &context).await?; } }; diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 06c37ad16..eaf81649f 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -11,7 +11,7 @@ use chrono::{DateTime, Days, Local, TimeZone, Utc}; use enum_map::{enum_map, EnumMap}; use lemmy_db_schema::{ aggregates::structs::{PersonPostAggregates, PersonPostAggregatesForm}, - newtypes::{CommunityId, DbUrl, InstanceId, PersonId, PostId}, + newtypes::{CommentId, CommunityId, DbUrl, InstanceId, PersonId, PostId}, source::{ comment::{Comment, CommentUpdateForm}, community::{Community, CommunityModerator, CommunityUpdateForm}, @@ -23,6 +23,7 @@ use lemmy_db_schema::{ local_site::LocalSite, local_site_rate_limit::LocalSiteRateLimit, local_site_url_blocklist::LocalSiteUrlBlocklist, + moderator::{ModRemoveComment, ModRemoveCommentForm, ModRemovePost, ModRemovePostForm}, oauth_account::OAuthAccount, password_reset_request::PasswordResetRequest, person::{Person, PersonUpdateForm}, @@ -668,10 +669,14 @@ pub async fn purge_image_posts_for_community( } pub async fn remove_user_data( + mod_person_id: PersonId, banned_person_id: PersonId, + reason: &Option, context: &LemmyContext, ) -> LemmyResult<()> { let pool = &mut context.pool(); + let removed = true; + // Purge user images let person = Person::read(pool, banned_person_id).await?; if let Some(avatar) = person.avatar { @@ -695,11 +700,32 @@ pub async fn remove_user_data( .await?; // Posts - Post::update_removed_for_creator(pool, banned_person_id, None, true).await?; + let removed_posts = + Post::update_removed_for_creator(pool, banned_person_id, None, removed).await?; + create_modlog_entries_for_removed_or_restored_posts( + pool, + mod_person_id, + removed_posts.iter().map(|r| r.id).collect(), + removed, + reason, + ) + .await?; // Purge image posts purge_image_posts_for_person(banned_person_id, context).await?; + // Comments + let removed_comments = + Comment::update_removed_for_creator(pool, banned_person_id, removed).await?; + create_modlog_entries_for_removed_or_restored_comments( + pool, + mod_person_id, + removed_comments.iter().map(|r| r.id).collect(), + removed, + reason, + ) + .await?; + // Communities // Remove all communities where they're the top mod // for now, remove the communities manually @@ -717,7 +743,7 @@ pub async fn remove_user_data( pool, community_id, &CommunityUpdateForm { - removed: Some(true), + removed: Some(removed), ..Default::default() }, ) @@ -743,36 +769,111 @@ pub async fn remove_user_data( .await?; } - // Comments - Comment::update_removed_for_creator(pool, banned_person_id, true).await?; - Ok(()) } /// We can't restore their images, but we can unremove their posts and comments pub async fn restore_user_data( + mod_person_id: PersonId, banned_person_id: PersonId, + reason: &Option, context: &LemmyContext, ) -> LemmyResult<()> { let pool = &mut context.pool(); + let removed = false; // Posts - Post::update_removed_for_creator(pool, banned_person_id, None, false).await?; + let restored_posts = + Post::update_removed_for_creator(pool, banned_person_id, None, removed).await?; + create_modlog_entries_for_removed_or_restored_posts( + pool, + mod_person_id, + restored_posts.iter().map(|r| r.id).collect(), + removed, + reason, + ) + .await?; // Comments - Comment::update_removed_for_creator(pool, banned_person_id, false).await?; + let restored_comments = + Comment::update_removed_for_creator(pool, banned_person_id, removed).await?; + create_modlog_entries_for_removed_or_restored_comments( + pool, + mod_person_id, + restored_comments.iter().map(|r| r.id).collect(), + removed, + reason, + ) + .await?; + + Ok(()) +} + +async fn create_modlog_entries_for_removed_or_restored_posts( + pool: &mut DbPool<'_>, + mod_person_id: PersonId, + post_ids: Vec, + new_removed: bool, + reason: &Option, +) -> LemmyResult<()> { + // Build the forms + let forms = post_ids + .iter() + .map(|&post_id| ModRemovePostForm { + mod_person_id, + post_id, + removed: Some(new_removed), + reason: reason.clone(), + }) + .collect(); + + ModRemovePost::create_multiple(pool, &forms).await?; + + Ok(()) +} + +async fn create_modlog_entries_for_removed_or_restored_comments( + pool: &mut DbPool<'_>, + mod_person_id: PersonId, + comment_ids: Vec, + new_removed: bool, + reason: &Option, +) -> LemmyResult<()> { + // Build the forms + let forms = comment_ids + .iter() + .map(|&comment_id| ModRemoveCommentForm { + mod_person_id, + comment_id, + removed: Some(new_removed), + reason: reason.clone(), + }) + .collect(); + + ModRemoveComment::create_multiple(pool, &forms).await?; Ok(()) } pub async fn remove_or_restore_user_data_in_community( community_id: CommunityId, + mod_person_id: PersonId, banned_person_id: PersonId, remove: bool, + reason: &Option, pool: &mut DbPool<'_>, ) -> LemmyResult<()> { // Posts - Post::update_removed_for_creator(pool, banned_person_id, Some(community_id), remove).await?; + let posts = + Post::update_removed_for_creator(pool, banned_person_id, Some(community_id), remove).await?; + create_modlog_entries_for_removed_or_restored_posts( + pool, + mod_person_id, + posts.iter().map(|r| r.id).collect(), + remove, + reason, + ) + .await?; // Comments // TODO Diesel doesn't allow updates with joins, so this has to be a loop @@ -798,6 +899,15 @@ pub async fn remove_or_restore_user_data_in_community( .await?; } + create_modlog_entries_for_removed_or_restored_comments( + pool, + mod_person_id, + comments.iter().map(|r| r.comment.id).collect(), + remove, + reason, + ) + .await?; + Ok(()) } @@ -1067,11 +1177,21 @@ fn build_proxied_image_url( } #[cfg(test)] -#[allow(clippy::unwrap_used)] #[allow(clippy::indexing_slicing)] mod tests { use super::*; + use lemmy_db_schema::source::{ + comment::CommentInsertForm, + community::CommunityInsertForm, + person::PersonInsertForm, + post::PostInsertForm, + }; + use lemmy_db_views_moderator::structs::{ + ModRemoveCommentView, + ModRemovePostView, + ModlogListParams, + }; use pretty_assertions::assert_eq; use serial_test::serial; @@ -1093,48 +1213,42 @@ mod tests { } #[test] - fn test_limit_ban_term() { + fn test_limit_ban_term() -> LemmyResult<()> { // Ban expires in past, should throw error assert!(limit_expire_time(Utc::now() - Days::new(5)).is_err()); // Legitimate ban term, return same value let fourteen_days = Utc::now() + Days::new(14); - assert_eq!( - limit_expire_time(fourteen_days).unwrap(), - Some(fourteen_days) - ); + assert_eq!(limit_expire_time(fourteen_days)?, Some(fourteen_days)); let nine_years = Utc::now() + Days::new(365 * 9); - assert_eq!(limit_expire_time(nine_years).unwrap(), Some(nine_years)); + assert_eq!(limit_expire_time(nine_years)?, Some(nine_years)); // Too long ban term, changes to None (permanent ban) - assert_eq!( - limit_expire_time(Utc::now() + Days::new(365 * 11)).unwrap(), - None - ); + assert_eq!(limit_expire_time(Utc::now() + Days::new(365 * 11))?, None); + + Ok(()) } #[tokio::test] #[serial] - async fn test_proxy_image_link() { + async fn test_proxy_image_link() -> LemmyResult<()> { let context = LemmyContext::init_test_context().await; // image from local domain is unchanged - let local_url = Url::parse("http://lemmy-alpha/image.png").unwrap(); + let local_url = Url::parse("http://lemmy-alpha/image.png")?; let proxied = proxy_image_link_internal(local_url.clone(), PictrsImageMode::ProxyAllImages, &context) - .await - .unwrap(); + .await?; assert_eq!(&local_url, proxied.inner()); // image from remote domain is proxied - let remote_image = Url::parse("http://lemmy-beta/image.png").unwrap(); + let remote_image = Url::parse("http://lemmy-beta/image.png")?; let proxied = proxy_image_link_internal( remote_image.clone(), PictrsImageMode::ProxyAllImages, &context, ) - .await - .unwrap(); + .await?; assert_eq!( "https://lemmy-alpha/api/v3/image_proxy?url=http%3A%2F%2Flemmy-beta%2Fimage.png", proxied.as_str() @@ -1147,5 +1261,157 @@ mod tests { .await .is_ok() ); + + Ok(()) + } + + #[tokio::test] + #[serial] + async fn test_mod_remove_or_restore_data() -> LemmyResult<()> { + let context = LemmyContext::init_test_context().await; + let pool = &mut context.pool(); + + let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string()).await?; + + let new_mod = PersonInsertForm::test_form(inserted_instance.id, "modder"); + let inserted_mod = Person::create(pool, &new_mod).await?; + + let new_person = PersonInsertForm::test_form(inserted_instance.id, "chrimbus"); + let inserted_person = Person::create(pool, &new_person).await?; + + let new_community = CommunityInsertForm::new( + inserted_instance.id, + "mod_community crepes".to_string(), + "nada".to_owned(), + "pubkey".to_string(), + ); + let inserted_community = Community::create(pool, &new_community).await?; + + let post_form_1 = PostInsertForm::new( + "A test post tubular".into(), + inserted_person.id, + inserted_community.id, + ); + let inserted_post_1 = Post::create(pool, &post_form_1).await?; + + let post_form_2 = PostInsertForm::new( + "A test post radical".into(), + inserted_person.id, + inserted_community.id, + ); + let inserted_post_2 = Post::create(pool, &post_form_2).await?; + + let comment_form_1 = CommentInsertForm::new( + inserted_person.id, + inserted_post_1.id, + "A test comment tubular".into(), + ); + let _inserted_comment_1 = Comment::create(pool, &comment_form_1, None).await?; + + let comment_form_2 = CommentInsertForm::new( + inserted_person.id, + inserted_post_2.id, + "A test comment radical".into(), + ); + let _inserted_comment_2 = Comment::create(pool, &comment_form_2, None).await?; + + // Remove the user data + remove_user_data( + inserted_mod.id, + inserted_person.id, + &Some("a remove reason".to_string()), + &context, + ) + .await?; + + // Verify that their posts and comments are removed. + let params = ModlogListParams { + community_id: None, + mod_person_id: None, + other_person_id: None, + post_id: None, + comment_id: None, + page: None, + limit: None, + hide_modlog_names: false, + }; + + // Posts + let post_modlog = ModRemovePostView::list(pool, params).await?; + assert_eq!(2, post_modlog.len()); + + let mod_removed_posts = post_modlog + .iter() + .map(|p| p.mod_remove_post.removed) + .collect::>(); + assert_eq!(vec![true, true], mod_removed_posts); + + let removed_posts = post_modlog + .iter() + .map(|p| p.post.removed) + .collect::>(); + assert_eq!(vec![true, true], removed_posts); + + // Comments + let comment_modlog = ModRemoveCommentView::list(pool, params).await?; + assert_eq!(2, comment_modlog.len()); + + let mod_removed_comments = comment_modlog + .iter() + .map(|p| p.mod_remove_comment.removed) + .collect::>(); + assert_eq!(vec![true, true], mod_removed_comments); + + let removed_comments = comment_modlog + .iter() + .map(|p| p.comment.removed) + .collect::>(); + assert_eq!(vec![true, true], removed_comments); + + // Now restore the content, and make sure it got appended + restore_user_data( + inserted_mod.id, + inserted_person.id, + &Some("a restore reason".to_string()), + &context, + ) + .await?; + + // Posts + let post_modlog = ModRemovePostView::list(pool, params).await?; + assert_eq!(4, post_modlog.len()); + + let mod_restored_posts = post_modlog + .iter() + .map(|p| p.mod_remove_post.removed) + .collect::>(); + assert_eq!(vec![false, false, true, true], mod_restored_posts); + + let restored_posts = post_modlog + .iter() + .map(|p| p.post.removed) + .collect::>(); + // All of these will be false, cause its the current state of the post + assert_eq!(vec![false, false, false, false], restored_posts); + + // Comments + let comment_modlog = ModRemoveCommentView::list(pool, params).await?; + assert_eq!(4, comment_modlog.len()); + + let mod_restored_comments = comment_modlog + .iter() + .map(|p| p.mod_remove_comment.removed) + .collect::>(); + assert_eq!(vec![false, false, true, true], mod_restored_comments); + + let restored_comments = comment_modlog + .iter() + .map(|p| p.comment.removed) + .collect::>(); + assert_eq!(vec![false, false, false, false], restored_comments); + + Instance::delete(pool, inserted_instance.id).await?; + + Ok(()) } } diff --git a/crates/apub/src/activities/block/block_user.rs b/crates/apub/src/activities/block/block_user.rs index 29412cdfe..6ae566f37 100644 --- a/crates/apub/src/activities/block/block_user.rs +++ b/crates/apub/src/activities/block/block_user.rs @@ -160,6 +160,7 @@ impl ActivityHandler for BlockUser { let mod_person = self.actor.dereference(context).await?; let blocked_person = self.object.dereference(context).await?; let target = self.target.dereference(context).await?; + let reason = self.summary; match target { SiteOrCommunity::Site(_site) => { let blocked_person = Person::update( @@ -173,14 +174,14 @@ impl ActivityHandler for BlockUser { ) .await?; if self.remove_data.unwrap_or(false) { - remove_user_data(blocked_person.id, context).await?; + remove_user_data(mod_person.id, blocked_person.id, &reason, context).await?; } // write mod log let form = ModBanForm { mod_person_id: mod_person.id, other_person_id: blocked_person.id, - reason: self.summary, + reason, banned: Some(true), expires, }; @@ -207,8 +208,10 @@ impl ActivityHandler for BlockUser { if self.remove_data.unwrap_or(false) { remove_or_restore_user_data_in_community( community.id, + mod_person.id, blocked_person.id, true, + &reason, &mut context.pool(), ) .await?; @@ -219,7 +222,7 @@ impl ActivityHandler for BlockUser { mod_person_id: mod_person.id, other_person_id: blocked_person.id, community_id: community.id, - reason: self.summary, + reason, banned: Some(true), expires, }; diff --git a/crates/apub/src/activities/block/undo_block_user.rs b/crates/apub/src/activities/block/undo_block_user.rs index 416770348..d1eb8b3b7 100644 --- a/crates/apub/src/activities/block/undo_block_user.rs +++ b/crates/apub/src/activities/block/undo_block_user.rs @@ -120,7 +120,7 @@ impl ActivityHandler for UndoBlockUser { .await?; if self.restore_data.unwrap_or(false) { - restore_user_data(blocked_person.id, context).await?; + restore_user_data(mod_person.id, blocked_person.id, &None, context).await?; } // write mod log @@ -144,8 +144,10 @@ impl ActivityHandler for UndoBlockUser { if self.restore_data.unwrap_or(false) { remove_or_restore_user_data_in_community( community.id, + mod_person.id, blocked_person.id, false, + &None, &mut context.pool(), ) .await?; diff --git a/crates/db_schema/src/impls/moderator.rs b/crates/db_schema/src/impls/moderator.rs index 14964aa00..3be09c792 100644 --- a/crates/db_schema/src/impls/moderator.rs +++ b/crates/db_schema/src/impls/moderator.rs @@ -66,6 +66,20 @@ impl Crud for ModRemovePost { } } +impl ModRemovePost { + pub async fn create_multiple( + pool: &mut DbPool<'_>, + forms: &Vec, + ) -> Result { + use crate::schema::mod_remove_post::dsl::mod_remove_post; + let conn = &mut get_conn(pool).await?; + insert_into(mod_remove_post) + .values(forms) + .execute(conn) + .await + } +} + #[async_trait] impl Crud for ModLockPost { type InsertForm = ModLockPostForm; @@ -153,6 +167,20 @@ impl Crud for ModRemoveComment { } } +impl ModRemoveComment { + pub async fn create_multiple( + pool: &mut DbPool<'_>, + forms: &Vec, + ) -> Result { + use crate::schema::mod_remove_comment::dsl::mod_remove_comment; + let conn = &mut get_conn(pool).await?; + insert_into(mod_remove_comment) + .values(forms) + .execute(conn) + .await + } +} + #[async_trait] impl Crud for ModRemoveCommunity { type InsertForm = ModRemoveCommunityForm; @@ -465,7 +493,6 @@ impl Crud for AdminPurgeComment { } #[cfg(test)] -#[allow(clippy::unwrap_used)] #[allow(clippy::indexing_slicing)] mod tests { @@ -500,26 +527,25 @@ mod tests { traits::Crud, utils::build_db_pool_for_tests, }; + use diesel::result::Error; use pretty_assertions::assert_eq; use serial_test::serial; #[tokio::test] #[serial] - async fn test_crud() { + async fn test_crud() -> Result<(), Error> { let pool = &build_db_pool_for_tests().await; let pool = &mut pool.into(); - let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string()) - .await - .unwrap(); + let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string()).await?; let new_mod = PersonInsertForm::test_form(inserted_instance.id, "the mod"); - let inserted_mod = Person::create(pool, &new_mod).await.unwrap(); + let inserted_mod = Person::create(pool, &new_mod).await?; let new_person = PersonInsertForm::test_form(inserted_instance.id, "jim2"); - let inserted_person = Person::create(pool, &new_person).await.unwrap(); + let inserted_person = Person::create(pool, &new_person).await?; let new_community = CommunityInsertForm::new( inserted_instance.id, @@ -528,21 +554,21 @@ mod tests { "pubkey".to_string(), ); - let inserted_community = Community::create(pool, &new_community).await.unwrap(); + let inserted_community = Community::create(pool, &new_community).await?; let new_post = PostInsertForm::new( "A test post thweep".into(), inserted_person.id, inserted_community.id, ); - let inserted_post = Post::create(pool, &new_post).await.unwrap(); + let inserted_post = Post::create(pool, &new_post).await?; let comment_form = CommentInsertForm::new( inserted_person.id, inserted_post.id, "A test comment".into(), ); - let inserted_comment = Comment::create(pool, &comment_form, None).await.unwrap(); + let inserted_comment = Comment::create(pool, &comment_form, None).await?; // Now the actual tests @@ -553,12 +579,8 @@ mod tests { reason: None, removed: None, }; - let inserted_mod_remove_post = ModRemovePost::create(pool, &mod_remove_post_form) - .await - .unwrap(); - let read_mod_remove_post = ModRemovePost::read(pool, inserted_mod_remove_post.id) - .await - .unwrap(); + let inserted_mod_remove_post = ModRemovePost::create(pool, &mod_remove_post_form).await?; + let read_mod_remove_post = ModRemovePost::read(pool, inserted_mod_remove_post.id).await?; let expected_mod_remove_post = ModRemovePost { id: inserted_mod_remove_post.id, post_id: inserted_post.id, @@ -575,12 +597,8 @@ mod tests { post_id: inserted_post.id, locked: None, }; - let inserted_mod_lock_post = ModLockPost::create(pool, &mod_lock_post_form) - .await - .unwrap(); - let read_mod_lock_post = ModLockPost::read(pool, inserted_mod_lock_post.id) - .await - .unwrap(); + let inserted_mod_lock_post = ModLockPost::create(pool, &mod_lock_post_form).await?; + let read_mod_lock_post = ModLockPost::read(pool, inserted_mod_lock_post.id).await?; let expected_mod_lock_post = ModLockPost { id: inserted_mod_lock_post.id, post_id: inserted_post.id, @@ -597,12 +615,8 @@ mod tests { featured: false, is_featured_community: true, }; - let inserted_mod_feature_post = ModFeaturePost::create(pool, &mod_feature_post_form) - .await - .unwrap(); - let read_mod_feature_post = ModFeaturePost::read(pool, inserted_mod_feature_post.id) - .await - .unwrap(); + let inserted_mod_feature_post = ModFeaturePost::create(pool, &mod_feature_post_form).await?; + let read_mod_feature_post = ModFeaturePost::read(pool, inserted_mod_feature_post.id).await?; let expected_mod_feature_post = ModFeaturePost { id: inserted_mod_feature_post.id, post_id: inserted_post.id, @@ -620,12 +634,10 @@ mod tests { reason: None, removed: None, }; - let inserted_mod_remove_comment = ModRemoveComment::create(pool, &mod_remove_comment_form) - .await - .unwrap(); - let read_mod_remove_comment = ModRemoveComment::read(pool, inserted_mod_remove_comment.id) - .await - .unwrap(); + let inserted_mod_remove_comment = + ModRemoveComment::create(pool, &mod_remove_comment_form).await?; + let read_mod_remove_comment = + ModRemoveComment::read(pool, inserted_mod_remove_comment.id).await?; let expected_mod_remove_comment = ModRemoveComment { id: inserted_mod_remove_comment.id, comment_id: inserted_comment.id, @@ -644,13 +656,9 @@ mod tests { removed: None, }; let inserted_mod_remove_community = - ModRemoveCommunity::create(pool, &mod_remove_community_form) - .await - .unwrap(); + ModRemoveCommunity::create(pool, &mod_remove_community_form).await?; let read_mod_remove_community = - ModRemoveCommunity::read(pool, inserted_mod_remove_community.id) - .await - .unwrap(); + ModRemoveCommunity::read(pool, inserted_mod_remove_community.id).await?; let expected_mod_remove_community = ModRemoveCommunity { id: inserted_mod_remove_community.id, community_id: inserted_community.id, @@ -671,13 +679,9 @@ mod tests { expires: None, }; let inserted_mod_ban_from_community = - ModBanFromCommunity::create(pool, &mod_ban_from_community_form) - .await - .unwrap(); + ModBanFromCommunity::create(pool, &mod_ban_from_community_form).await?; let read_mod_ban_from_community = - ModBanFromCommunity::read(pool, inserted_mod_ban_from_community.id) - .await - .unwrap(); + ModBanFromCommunity::read(pool, inserted_mod_ban_from_community.id).await?; let expected_mod_ban_from_community = ModBanFromCommunity { id: inserted_mod_ban_from_community.id, community_id: inserted_community.id, @@ -698,8 +702,8 @@ mod tests { banned: None, expires: None, }; - let inserted_mod_ban = ModBan::create(pool, &mod_ban_form).await.unwrap(); - let read_mod_ban = ModBan::read(pool, inserted_mod_ban.id).await.unwrap(); + let inserted_mod_ban = ModBan::create(pool, &mod_ban_form).await?; + let read_mod_ban = ModBan::read(pool, inserted_mod_ban.id).await?; let expected_mod_ban = ModBan { id: inserted_mod_ban.id, mod_person_id: inserted_mod.id, @@ -718,12 +722,8 @@ mod tests { community_id: inserted_community.id, removed: None, }; - let inserted_mod_add_community = ModAddCommunity::create(pool, &mod_add_community_form) - .await - .unwrap(); - let read_mod_add_community = ModAddCommunity::read(pool, inserted_mod_add_community.id) - .await - .unwrap(); + let inserted_mod_add_community = ModAddCommunity::create(pool, &mod_add_community_form).await?; + let read_mod_add_community = ModAddCommunity::read(pool, inserted_mod_add_community.id).await?; let expected_mod_add_community = ModAddCommunity { id: inserted_mod_add_community.id, community_id: inserted_community.id, @@ -740,8 +740,8 @@ mod tests { other_person_id: inserted_person.id, removed: None, }; - let inserted_mod_add = ModAdd::create(pool, &mod_add_form).await.unwrap(); - let read_mod_add = ModAdd::read(pool, inserted_mod_add.id).await.unwrap(); + let inserted_mod_add = ModAdd::create(pool, &mod_add_form).await?; + let read_mod_add = ModAdd::read(pool, inserted_mod_add.id).await?; let expected_mod_add = ModAdd { id: inserted_mod_add.id, mod_person_id: inserted_mod.id, @@ -750,14 +750,12 @@ mod tests { when_: inserted_mod_add.when_, }; - Comment::delete(pool, inserted_comment.id).await.unwrap(); - Post::delete(pool, inserted_post.id).await.unwrap(); - Community::delete(pool, inserted_community.id) - .await - .unwrap(); - Person::delete(pool, inserted_person.id).await.unwrap(); - Person::delete(pool, inserted_mod.id).await.unwrap(); - Instance::delete(pool, inserted_instance.id).await.unwrap(); + Comment::delete(pool, inserted_comment.id).await?; + Post::delete(pool, inserted_post.id).await?; + Community::delete(pool, inserted_community.id).await?; + Person::delete(pool, inserted_person.id).await?; + Person::delete(pool, inserted_mod.id).await?; + Instance::delete(pool, inserted_instance.id).await?; assert_eq!(expected_mod_remove_post, read_mod_remove_post); assert_eq!(expected_mod_lock_post, read_mod_lock_post); @@ -768,5 +766,7 @@ mod tests { assert_eq!(expected_mod_ban, read_mod_ban); assert_eq!(expected_mod_add_community, read_mod_add_community); assert_eq!(expected_mod_add, read_mod_add); + + Ok(()) } } From 3a4f67fa609771c23c2228dccbd2174649b5b528 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Fri, 27 Sep 2024 09:02:28 -0400 Subject: [PATCH 2/5] Address PR comments. --- crates/db_schema/src/impls/moderator.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/db_schema/src/impls/moderator.rs b/crates/db_schema/src/impls/moderator.rs index 3be09c792..b2ef26e69 100644 --- a/crates/db_schema/src/impls/moderator.rs +++ b/crates/db_schema/src/impls/moderator.rs @@ -493,7 +493,6 @@ impl Crud for AdminPurgeComment { } #[cfg(test)] -#[allow(clippy::indexing_slicing)] mod tests { use crate::{ From 7c43f4e86f756f78be98a897c82fadc64585d67f Mon Sep 17 00:00:00 2001 From: Dessalines Date: Mon, 30 Sep 2024 08:41:11 -0400 Subject: [PATCH 3/5] Combining remove and restore functions. --- crates/api/src/local_user/ban_person.rs | 16 +- crates/api_common/src/utils.rs | 179 ++++++++---------- .../apub/src/activities/block/block_user.rs | 5 +- .../src/activities/block/undo_block_user.rs | 5 +- crates/db_schema/src/impls/comment.rs | 4 +- crates/db_schema/src/impls/post.rs | 4 +- 6 files changed, 94 insertions(+), 119 deletions(-) diff --git a/crates/api/src/local_user/ban_person.rs b/crates/api/src/local_user/ban_person.rs index cf660a432..2ace7f031 100644 --- a/crates/api/src/local_user/ban_person.rs +++ b/crates/api/src/local_user/ban_person.rs @@ -5,7 +5,7 @@ use lemmy_api_common::{ context::LemmyContext, person::{BanPerson, BanPersonResponse}, send_activity::{ActivityChannel, SendActivityData}, - utils::{check_expire_time, is_admin, remove_user_data, restore_user_data}, + utils::{check_expire_time, is_admin, remove_or_restore_user_data}, }; use lemmy_db_schema::{ source::{ @@ -66,11 +66,15 @@ pub async fn ban_from_site( // Remove their data if that's desired if data.remove_or_restore_data.unwrap_or(false) { - if data.ban { - remove_user_data(local_user_view.person.id, person.id, &data.reason, &context).await?; - } else { - restore_user_data(local_user_view.person.id, person.id, &data.reason, &context).await?; - } + let removed = data.ban; + remove_or_restore_user_data( + local_user_view.person.id, + person.id, + removed, + &data.reason, + &context, + ) + .await?; }; // Mod tables diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index eaf81649f..07ed51b9c 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -668,139 +668,106 @@ pub async fn purge_image_posts_for_community( Ok(()) } -pub async fn remove_user_data( +/// Removes or restores user data. +pub async fn remove_or_restore_user_data( mod_person_id: PersonId, banned_person_id: PersonId, + removed: bool, reason: &Option, context: &LemmyContext, ) -> LemmyResult<()> { let pool = &mut context.pool(); - let removed = true; - // Purge user images - let person = Person::read(pool, banned_person_id).await?; - if let Some(avatar) = person.avatar { - purge_image_from_pictrs(&avatar, context).await.ok(); - } - if let Some(banner) = person.banner { - purge_image_from_pictrs(&banner, context).await.ok(); - } - - // Update the fields to None - Person::update( - pool, - banned_person_id, - &PersonUpdateForm { - avatar: Some(None), - banner: Some(None), - bio: Some(None), - ..Default::default() - }, - ) - .await?; - - // Posts - let removed_posts = - Post::update_removed_for_creator(pool, banned_person_id, None, removed).await?; - create_modlog_entries_for_removed_or_restored_posts( - pool, - mod_person_id, - removed_posts.iter().map(|r| r.id).collect(), - removed, - reason, - ) - .await?; - - // Purge image posts - purge_image_posts_for_person(banned_person_id, context).await?; - - // Comments - let removed_comments = - Comment::update_removed_for_creator(pool, banned_person_id, removed).await?; - create_modlog_entries_for_removed_or_restored_comments( - pool, - mod_person_id, - removed_comments.iter().map(|r| r.id).collect(), - removed, - reason, - ) - .await?; - - // Communities - // Remove all communities where they're the top mod - // for now, remove the communities manually - let first_mod_communities = CommunityModeratorView::get_community_first_mods(pool).await?; - - // Filter to only this banned users top communities - let banned_user_first_communities: Vec = first_mod_communities - .into_iter() - .filter(|fmc| fmc.moderator.id == banned_person_id) - .collect(); - - for first_mod_community in banned_user_first_communities { - let community_id = first_mod_community.community.id; - Community::update( - pool, - community_id, - &CommunityUpdateForm { - removed: Some(removed), - ..Default::default() - }, - ) - .await?; - - // Delete the community images - if let Some(icon) = first_mod_community.community.icon { - purge_image_from_pictrs(&icon, context).await.ok(); + // Only these actions are possible when removing, not restoring + if removed { + // Purge user images + let person = Person::read(pool, banned_person_id).await?; + if let Some(avatar) = person.avatar { + purge_image_from_pictrs(&avatar, context).await.ok(); } - if let Some(banner) = first_mod_community.community.banner { + if let Some(banner) = person.banner { purge_image_from_pictrs(&banner, context).await.ok(); } + // Update the fields to None - Community::update( + Person::update( pool, - community_id, - &CommunityUpdateForm { - icon: Some(None), + banned_person_id, + &PersonUpdateForm { + avatar: Some(None), banner: Some(None), + bio: Some(None), ..Default::default() }, ) .await?; + + // Purge image posts + purge_image_posts_for_person(banned_person_id, context).await?; + + // Communities + // Remove all communities where they're the top mod + // for now, remove the communities manually + let first_mod_communities = CommunityModeratorView::get_community_first_mods(pool).await?; + + // Filter to only this banned users top communities + let banned_user_first_communities: Vec = first_mod_communities + .into_iter() + .filter(|fmc| fmc.moderator.id == banned_person_id) + .collect(); + + for first_mod_community in banned_user_first_communities { + let community_id = first_mod_community.community.id; + Community::update( + pool, + community_id, + &CommunityUpdateForm { + removed: Some(removed), + ..Default::default() + }, + ) + .await?; + + // Delete the community images + if let Some(icon) = first_mod_community.community.icon { + purge_image_from_pictrs(&icon, context).await.ok(); + } + if let Some(banner) = first_mod_community.community.banner { + purge_image_from_pictrs(&banner, context).await.ok(); + } + // Update the fields to None + Community::update( + pool, + community_id, + &CommunityUpdateForm { + icon: Some(None), + banner: Some(None), + ..Default::default() + }, + ) + .await?; + } } - Ok(()) -} - -/// We can't restore their images, but we can unremove their posts and comments -pub async fn restore_user_data( - mod_person_id: PersonId, - banned_person_id: PersonId, - reason: &Option, - context: &LemmyContext, -) -> LemmyResult<()> { - let pool = &mut context.pool(); - let removed = false; - // Posts - let restored_posts = + let removed_or_restored_posts = Post::update_removed_for_creator(pool, banned_person_id, None, removed).await?; create_modlog_entries_for_removed_or_restored_posts( pool, mod_person_id, - restored_posts.iter().map(|r| r.id).collect(), + removed_or_restored_posts.iter().map(|r| r.id).collect(), removed, reason, ) .await?; // Comments - let restored_comments = + let removed_or_restored_comments = Comment::update_removed_for_creator(pool, banned_person_id, removed).await?; create_modlog_entries_for_removed_or_restored_comments( pool, mod_person_id, - restored_comments.iter().map(|r| r.id).collect(), + removed_or_restored_comments.iter().map(|r| r.id).collect(), removed, reason, ) @@ -813,7 +780,7 @@ async fn create_modlog_entries_for_removed_or_restored_posts( pool: &mut DbPool<'_>, mod_person_id: PersonId, post_ids: Vec, - new_removed: bool, + removed: bool, reason: &Option, ) -> LemmyResult<()> { // Build the forms @@ -822,7 +789,7 @@ async fn create_modlog_entries_for_removed_or_restored_posts( .map(|&post_id| ModRemovePostForm { mod_person_id, post_id, - removed: Some(new_removed), + removed: Some(removed), reason: reason.clone(), }) .collect(); @@ -836,7 +803,7 @@ async fn create_modlog_entries_for_removed_or_restored_comments( pool: &mut DbPool<'_>, mod_person_id: PersonId, comment_ids: Vec, - new_removed: bool, + removed: bool, reason: &Option, ) -> LemmyResult<()> { // Build the forms @@ -845,7 +812,7 @@ async fn create_modlog_entries_for_removed_or_restored_comments( .map(|&comment_id| ModRemoveCommentForm { mod_person_id, comment_id, - removed: Some(new_removed), + removed: Some(removed), reason: reason.clone(), }) .collect(); @@ -1316,9 +1283,10 @@ mod tests { let _inserted_comment_2 = Comment::create(pool, &comment_form_2, None).await?; // Remove the user data - remove_user_data( + remove_or_restore_user_data( inserted_mod.id, inserted_person.id, + true, &Some("a remove reason".to_string()), &context, ) @@ -1369,9 +1337,10 @@ mod tests { assert_eq!(vec![true, true], removed_comments); // Now restore the content, and make sure it got appended - restore_user_data( + remove_or_restore_user_data( inserted_mod.id, inserted_person.id, + false, &Some("a restore reason".to_string()), &context, ) diff --git a/crates/apub/src/activities/block/block_user.rs b/crates/apub/src/activities/block/block_user.rs index 6ae566f37..e291cc4a4 100644 --- a/crates/apub/src/activities/block/block_user.rs +++ b/crates/apub/src/activities/block/block_user.rs @@ -23,7 +23,7 @@ use anyhow::anyhow; use chrono::{DateTime, Utc}; use lemmy_api_common::{ context::LemmyContext, - utils::{remove_or_restore_user_data_in_community, remove_user_data}, + utils::{remove_or_restore_user_data, remove_or_restore_user_data_in_community}, }; use lemmy_db_schema::{ source::{ @@ -174,7 +174,8 @@ impl ActivityHandler for BlockUser { ) .await?; if self.remove_data.unwrap_or(false) { - remove_user_data(mod_person.id, blocked_person.id, &reason, context).await?; + remove_or_restore_user_data(mod_person.id, blocked_person.id, true, &reason, context) + .await?; } // write mod log diff --git a/crates/apub/src/activities/block/undo_block_user.rs b/crates/apub/src/activities/block/undo_block_user.rs index d1eb8b3b7..f9f6890b6 100644 --- a/crates/apub/src/activities/block/undo_block_user.rs +++ b/crates/apub/src/activities/block/undo_block_user.rs @@ -19,7 +19,7 @@ use activitypub_federation::{ }; use lemmy_api_common::{ context::LemmyContext, - utils::{remove_or_restore_user_data_in_community, restore_user_data}, + utils::{remove_or_restore_user_data, remove_or_restore_user_data_in_community}, }; use lemmy_db_schema::{ source::{ @@ -120,7 +120,8 @@ impl ActivityHandler for UndoBlockUser { .await?; if self.restore_data.unwrap_or(false) { - restore_user_data(mod_person.id, blocked_person.id, &None, context).await?; + remove_or_restore_user_data(mod_person.id, blocked_person.id, false, &None, context) + .await?; } // write mod log diff --git a/crates/db_schema/src/impls/comment.rs b/crates/db_schema/src/impls/comment.rs index 3a787a5c9..1f905a4df 100644 --- a/crates/db_schema/src/impls/comment.rs +++ b/crates/db_schema/src/impls/comment.rs @@ -40,12 +40,12 @@ impl Comment { pub async fn update_removed_for_creator( pool: &mut DbPool<'_>, for_creator_id: PersonId, - new_removed: bool, + removed: bool, ) -> Result, Error> { let conn = &mut get_conn(pool).await?; diesel::update(comment::table.filter(comment::creator_id.eq(for_creator_id))) .set(( - comment::removed.eq(new_removed), + comment::removed.eq(removed), comment::updated.eq(naive_now()), )) .get_results::(conn) diff --git a/crates/db_schema/src/impls/post.rs b/crates/db_schema/src/impls/post.rs index aa5568b0e..0c045cf24 100644 --- a/crates/db_schema/src/impls/post.rs +++ b/crates/db_schema/src/impls/post.rs @@ -145,7 +145,7 @@ impl Post { pool: &mut DbPool<'_>, for_creator_id: PersonId, for_community_id: Option, - new_removed: bool, + removed: bool, ) -> Result, Error> { let conn = &mut get_conn(pool).await?; @@ -157,7 +157,7 @@ impl Post { } update - .set((post::removed.eq(new_removed), post::updated.eq(naive_now()))) + .set((post::removed.eq(removed), post::updated.eq(naive_now()))) .get_results::(conn) .await } From 0b5259b3192e27ed10dcb222d26b12b7c304481b Mon Sep 17 00:00:00 2001 From: Dessalines Date: Mon, 30 Sep 2024 09:20:18 -0400 Subject: [PATCH 4/5] Trigger build. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 6c8398cd9..437d02eb2 100644 --- a/README.md +++ b/README.md @@ -162,4 +162,4 @@ When working on an issue or pull request, you can comment with any questions you ## Credits -Logo made by Andy Cuccaro (@andycuccaro) under the CC-BY-SA 4.0 license. +Logo made by Andy Cuccaro (@andycuccaro) unde the CC-BY-SA 4.0 license. From 846a6c56cd5ebc5f438056d076014c71da6fcea0 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Mon, 30 Sep 2024 09:20:32 -0400 Subject: [PATCH 5/5] Trigger build 2. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 437d02eb2..6c8398cd9 100644 --- a/README.md +++ b/README.md @@ -162,4 +162,4 @@ When working on an issue or pull request, you can comment with any questions you ## Credits -Logo made by Andy Cuccaro (@andycuccaro) unde the CC-BY-SA 4.0 license. +Logo made by Andy Cuccaro (@andycuccaro) under the CC-BY-SA 4.0 license.