Skip to content

Commit

Permalink
Adding ability to restore content on user unban. (#4845)
Browse files Browse the repository at this point in the history
* Adding ability to restore content on user unban.

- Fixes #4721

* Fixing api tests.

* Fix package.json

* Fixing lemmy-js-client dep.

* Adding API test for restoring content.
  • Loading branch information
dessalines authored and Nutomic committed Oct 29, 2024
1 parent a280d63 commit 373ae26
Show file tree
Hide file tree
Showing 17 changed files with 239 additions and 168 deletions.
2 changes: 1 addition & 1 deletion api_tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"eslint": "^9.8.0",
"eslint-plugin-prettier": "^5.1.3",
"jest": "^29.5.0",
"lemmy-js-client": "0.19.5-alpha.1",
"lemmy-js-client": "0.20.0-alpha.4",
"prettier": "^3.2.5",
"ts-jest": "^29.1.0",
"typescript": "^5.5.4",
Expand Down
263 changes: 132 additions & 131 deletions api_tests/pnpm-lock.yaml

Large diffs are not rendered by default.

9 changes: 8 additions & 1 deletion api_tests/src/post.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,10 +502,17 @@ test("Enforce site ban federation for local user", async () => {
alpha,
alphaPerson.person.id,
false,
false,
true,
);
expect(unBanAlpha.banned).toBe(false);

// existing alpha post should be restored on beta
betaBanRes = await waitUntil(
() => getPost(beta, searchBeta1.post.id),
s => !s.post_view.post.removed,
);
expect(betaBanRes.post_view.post.removed).toBe(false);

// Login gets invalidated by ban, need to login again
if (!alphaUserPerson) {
throw "Missing alpha person";
Expand Down
8 changes: 4 additions & 4 deletions api_tests/src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,13 +419,13 @@ export async function banPersonFromSite(
api: LemmyHttp,
person_id: number,
ban: boolean,
remove_data: boolean,
remove_or_restore_data: boolean,
): Promise<BanPersonResponse> {
// Make sure lemmy-beta/c/main is cached on lemmy_alpha
let form: BanPerson = {
person_id,
ban,
remove_data,
remove_or_restore_data,
};
return api.banPerson(form);
}
Expand All @@ -434,13 +434,13 @@ export async function banPersonFromCommunity(
api: LemmyHttp,
person_id: number,
community_id: number,
remove_data: boolean,
remove_or_restore_data: boolean,
ban: boolean,
): Promise<BanFromCommunityResponse> {
let form: BanFromCommunity = {
person_id,
community_id,
remove_data: remove_data,
remove_or_restore_data,
ban,
};
return api.banFromCommunity(form);
Expand Down
20 changes: 15 additions & 5 deletions crates/api/src/community/ban.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ use lemmy_api_common::{
community::{BanFromCommunity, BanFromCommunityResponse},
context::LemmyContext,
send_activity::{ActivityChannel, SendActivityData},
utils::{check_community_mod_action, check_expire_time, remove_user_data_in_community},
utils::{
check_community_mod_action,
check_expire_time,
remove_or_restore_user_data_in_community,
},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -33,7 +37,6 @@ pub async fn ban_from_community(
local_user_view: LocalUserView,
) -> LemmyResult<Json<BanFromCommunityResponse>> {
let banned_person_id = data.person_id;
let remove_data = data.remove_data.unwrap_or(false);
let expires = check_expire_time(data.expires)?;

// Verify that only mods or admins can ban
Expand Down Expand Up @@ -85,9 +88,16 @@ pub async fn ban_from_community(
}

// Remove/Restore their data if that's desired
if remove_data {
remove_user_data_in_community(data.community_id, banned_person_id, &mut context.pool()).await?;
}
if data.remove_or_restore_data.unwrap_or(false) {
let remove_data = data.ban;
remove_or_restore_user_data_in_community(
data.community_id,
banned_person_id,
remove_data,
&mut context.pool(),
)
.await?;
};

// Mod tables
let form = ModBanFromCommunityForm {
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ pub(crate) async fn ban_nonlocal_user_from_local_communities(
target: &Person,
ban: bool,
reason: &Option<String>,
remove_data: &Option<bool>,
remove_or_restore_data: &Option<bool>,
expires: &Option<i64>,
context: &Data<LemmyContext>,
) -> LemmyResult<()> {
Expand Down Expand Up @@ -230,7 +230,7 @@ pub(crate) async fn ban_nonlocal_user_from_local_communities(
person_id: target.id,
ban,
reason: reason.clone(),
remove_data: *remove_data,
remove_or_restore_data: *remove_or_restore_data,
expires: *expires,
};

Expand Down
17 changes: 10 additions & 7 deletions crates/api/src/local_user/ban_person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
utils::{check_expire_time, is_admin, remove_user_data, restore_user_data},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -65,10 +65,13 @@ pub async fn ban_from_site(
}

// Remove their data if that's desired
let remove_data = data.remove_data.unwrap_or(false);
if remove_data {
remove_user_data(person.id, &context).await?;
}
if data.remove_or_restore_data.unwrap_or(false) {
if data.ban {
remove_user_data(person.id, &context).await?;
} else {
restore_user_data(person.id, &context).await?;
}
};

// Mod tables
let form = ModBanForm {
Expand All @@ -90,7 +93,7 @@ pub async fn ban_from_site(
&person,
data.ban,
&data.reason,
&data.remove_data,
&data.remove_or_restore_data,
&data.expires,
&context,
)
Expand All @@ -101,7 +104,7 @@ pub async fn ban_from_site(
moderator: local_user_view.person,
banned_user: person_view.person.clone(),
reason: data.reason.clone(),
remove_data: data.remove_data,
remove_or_restore_data: data.remove_or_restore_data,
ban: data.ban,
expires: data.expires,
},
Expand Down
2 changes: 1 addition & 1 deletion crates/api/src/site/purge/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub async fn purge_person(
moderator: local_user_view.person,
banned_user: person,
reason: data.reason.clone(),
remove_data: Some(true),
remove_or_restore_data: Some(true),
ban: true,
expires: None,
},
Expand Down
4 changes: 3 additions & 1 deletion crates/api_common/src/community.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ pub struct BanFromCommunity {
pub community_id: CommunityId,
pub person_id: PersonId,
pub ban: bool,
pub remove_data: Option<bool>,
/// Optionally remove or restore all their data. Useful for new troll accounts.
/// If ban is true, then this means remove. If ban is false, it means restore.
pub remove_or_restore_data: Option<bool>,
pub reason: Option<String>,
/// A time that the ban will expire, in unix epoch seconds.
///
Expand Down
5 changes: 3 additions & 2 deletions crates/api_common/src/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,9 @@ pub struct AddAdminResponse {
pub struct BanPerson {
pub person_id: PersonId,
pub ban: bool,
/// Optionally remove all their data. Useful for new troll accounts.
pub remove_data: Option<bool>,
/// Optionally remove or restore all their data. Useful for new troll accounts.
/// If ban is true, then this means remove. If ban is false, it means restore.
pub remove_or_restore_data: Option<bool>,
pub reason: Option<String>,
/// A time that the ban will expire, in unix epoch seconds.
///
Expand Down
2 changes: 1 addition & 1 deletion crates/api_common/src/send_activity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub enum SendActivityData {
moderator: Person,
banned_user: Person,
reason: Option<String>,
remove_data: Option<bool>,
remove_or_restore_data: Option<bool>,
ban: bool,
expires: Option<i64>,
},
Expand Down
23 changes: 20 additions & 3 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -791,13 +791,30 @@ pub async fn remove_user_data(
Ok(())
}

pub async fn remove_user_data_in_community(
/// We can't restore their images, but we can unremove their posts and comments
pub async fn restore_user_data(
banned_person_id: PersonId,
context: &LemmyContext,
) -> LemmyResult<()> {
let pool = &mut context.pool();

// Posts
Post::update_removed_for_creator(pool, banned_person_id, None, false).await?;

// Comments
Comment::update_removed_for_creator(pool, banned_person_id, false).await?;

Ok(())
}

pub async fn remove_or_restore_user_data_in_community(
community_id: CommunityId,
banned_person_id: PersonId,
remove: bool,
pool: &mut DbPool<'_>,
) -> LemmyResult<()> {
// Posts
Post::update_removed_for_creator(pool, banned_person_id, Some(community_id), true).await?;
Post::update_removed_for_creator(pool, banned_person_id, Some(community_id), remove).await?;

// Comments
// TODO Diesel doesn't allow updates with joins, so this has to be a loop
Expand All @@ -815,7 +832,7 @@ pub async fn remove_user_data_in_community(
pool,
comment_id,
&CommentUpdateForm {
removed: Some(true),
removed: Some(remove),
..Default::default()
},
)
Expand Down
11 changes: 8 additions & 3 deletions crates/apub/src/activities/block/block_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use anyhow::anyhow;
use chrono::{DateTime, Utc};
use lemmy_api_common::{
context::LemmyContext,
utils::{remove_user_data, remove_user_data_in_community},
utils::{remove_or_restore_user_data_in_community, remove_user_data},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -206,8 +206,13 @@ impl ActivityHandler for BlockUser {
.ok();

if self.remove_data.unwrap_or(false) {
remove_user_data_in_community(community.id, blocked_person.id, &mut context.pool())
.await?;
remove_or_restore_user_data_in_community(
community.id,
blocked_person.id,
true,
&mut context.pool(),
)
.await?;
}

// write to mod log
Expand Down
8 changes: 5 additions & 3 deletions crates/apub/src/activities/block/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ pub(crate) async fn send_ban_from_site(
moderator: Person,
banned_user: Person,
reason: Option<String>,
remove_data: Option<bool>,
remove_or_restore_data: Option<bool>,
ban: bool,
expires: Option<i64>,
context: Data<LemmyContext>,
Expand All @@ -158,7 +158,7 @@ pub(crate) async fn send_ban_from_site(
&site,
&banned_user.into(),
&moderator.into(),
remove_data.unwrap_or(false),
remove_or_restore_data.unwrap_or(false),
reason.clone(),
expires,
&context,
Expand All @@ -169,6 +169,7 @@ pub(crate) async fn send_ban_from_site(
&site,
&banned_user.into(),
&moderator.into(),
remove_or_restore_data.unwrap_or(false),
reason.clone(),
&context,
)
Expand Down Expand Up @@ -197,7 +198,7 @@ pub(crate) async fn send_ban_from_community(
&SiteOrCommunity::Community(community),
&banned_person.into(),
&mod_.into(),
data.remove_data.unwrap_or(false),
data.remove_or_restore_data.unwrap_or(false),
data.reason.clone(),
expires,
&context,
Expand All @@ -208,6 +209,7 @@ pub(crate) async fn send_ban_from_community(
&SiteOrCommunity::Community(community),
&banned_person.into(),
&mod_.into(),
data.remove_or_restore_data.unwrap_or(false),
data.reason.clone(),
&context,
)
Expand Down
21 changes: 20 additions & 1 deletion crates/apub/src/activities/block/undo_block_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ use activitypub_federation::{
protocol::verification::verify_domains_match,
traits::{ActivityHandler, Actor},
};
use lemmy_api_common::context::LemmyContext;
use lemmy_api_common::{
context::LemmyContext,
utils::{remove_or_restore_user_data_in_community, restore_user_data},
};
use lemmy_db_schema::{
source::{
activity::ActivitySendTargets,
Expand All @@ -36,6 +39,7 @@ impl UndoBlockUser {
target: &SiteOrCommunity,
user: &ApubPerson,
mod_: &ApubPerson,
restore_data: bool,
reason: Option<String>,
context: &Data<LemmyContext>,
) -> LemmyResult<()> {
Expand All @@ -58,6 +62,7 @@ impl UndoBlockUser {
kind: UndoType::Undo,
id: id.clone(),
audience,
restore_data: Some(restore_data),
};

let mut inboxes = ActivitySendTargets::to_inbox(user.shared_inbox_or_inbox());
Expand Down Expand Up @@ -114,6 +119,10 @@ impl ActivityHandler for UndoBlockUser {
)
.await?;

if self.restore_data.unwrap_or(false) {
restore_user_data(blocked_person.id, context).await?;
}

// write mod log
let form = ModBanForm {
mod_person_id: mod_person.id,
Expand All @@ -132,6 +141,16 @@ impl ActivityHandler for UndoBlockUser {
};
CommunityPersonBan::unban(&mut context.pool(), &community_user_ban_form).await?;

if self.restore_data.unwrap_or(false) {
remove_or_restore_user_data_in_community(
community.id,
blocked_person.id,
false,
&mut context.pool(),
)
.await?;
}

// write to mod log
let form = ModBanFromCommunityForm {
mod_person_id: mod_person.id,
Expand Down
4 changes: 2 additions & 2 deletions crates/apub/src/activities/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,15 +352,15 @@ pub async fn match_outgoing_activities(
moderator,
banned_user,
reason,
remove_data,
remove_or_restore_data,
ban,
expires,
} => {
send_ban_from_site(
moderator,
banned_user,
reason,
remove_data,
remove_or_restore_data,
ban,
expires,
context,
Expand Down
4 changes: 4 additions & 0 deletions crates/apub/src/protocol/activities/block/undo_block_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ pub struct UndoBlockUser {
pub(crate) kind: UndoType,
pub(crate) id: Url,
pub(crate) audience: Option<ObjectId<ApubCommunity>>,

/// Quick and dirty solution.
/// TODO: send a separate Delete activity instead
pub(crate) restore_data: Option<bool>,
}

#[async_trait::async_trait]
Expand Down

0 comments on commit 373ae26

Please sign in to comment.