Skip to content

Commit

Permalink
Handle displaying of deleted and removed posts/comments (fixes #2624) (
Browse files Browse the repository at this point in the history
#3286)

* Handle displaying of deleted and removed posts/comments (fixes #2624)

* remove duplicate test

* fix tests

* no show_removed/show_deleted

* merge

* partially fix tests

* fix tests

* clippy

* fix tests

* get rid of build_post_response_deleted_allowed
  • Loading branch information
Nutomic committed Jul 20, 2023
1 parent d7051c4 commit 047db9a
Show file tree
Hide file tree
Showing 13 changed files with 273 additions and 178 deletions.
6 changes: 4 additions & 2 deletions api_tests/src/comment.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
getComments,
getCommentParentId,
resolveCommunity,
getPersonDetails,
} from "./shared";
import { CommentView } from "lemmy-js-client/dist/types/CommentView";

Expand Down Expand Up @@ -160,10 +161,11 @@ test("Remove a comment from admin and community on the same instance", async ()
expect(removeCommentRes.comment_view.comment.removed).toBe(true);

// Make sure that comment is removed on alpha (it gets pushed since an admin from beta removed it)
let refetchedPostComments = await getComments(
let refetchedPostComments = await getPersonDetails(
alpha,
postRes.post_view.post.id,
commentRes.comment_view.comment.creator_id,
);
console.log(refetchedPostComments.comments[0].comment);
expect(refetchedPostComments.comments[0].comment.removed).toBe(true);

let unremoveCommentRes = await removeComment(beta, false, betaCommentId);
Expand Down
8 changes: 4 additions & 4 deletions api_tests/src/post.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,8 @@ test("Enforce site ban for federated user", async () => {
expect(alphaUserOnBeta1.person?.person.banned).toBe(true);

// existing alpha post should be removed on beta
let searchBeta2 = await searchPostLocal(beta, postRes1.post_view.post);
expect(searchBeta2.posts[0].post.removed).toBe(true);
let searchBeta2 = await getPost(beta, searchBeta1.posts[0].post.id);
expect(searchBeta2.post_view.post.removed).toBe(true);

// Unban alpha
let unBanAlpha = await banPersonFromSite(
Expand Down Expand Up @@ -436,8 +436,8 @@ test("Enforce community ban for federated user", async () => {
expect(banAlpha.banned).toBe(true);

// ensure that the post by alpha got removed
let searchAlpha1 = await searchPostLocal(alpha, postRes1.post_view.post);
expect(searchAlpha1.posts[0].post.removed).toBe(true);
let searchAlpha1 = await getPost(alpha, searchBeta1.posts[0].post.id);
expect(searchAlpha1.post_view.post.removed).toBe(true);

// Alpha tries to make post on beta, but it fails because of ban
let postRes2 = await createPost(alpha, betaCommunity.community.id);
Expand Down
12 changes: 12 additions & 0 deletions api_tests/src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ import { CommentReportResponse } from "lemmy-js-client/dist/types/CommentReportR
import { CreateCommentReport } from "lemmy-js-client/dist/types/CreateCommentReport";
import { ListCommentReportsResponse } from "lemmy-js-client/dist/types/ListCommentReportsResponse";
import { ListCommentReports } from "lemmy-js-client/dist/types/ListCommentReports";
import { GetPersonDetailsResponse } from "lemmy-js-client/dist/types/GetPersonDetailsResponse";
import { GetPersonDetails } from "lemmy-js-client/dist/types/GetPersonDetails";

export interface API {
client: LemmyHttp;
Expand Down Expand Up @@ -646,6 +648,16 @@ export async function saveUserSettings(
): Promise<LoginResponse> {
return api.client.saveUserSettings(form);
}
export async function getPersonDetails(
api: API,
person_id: number,
): Promise<GetPersonDetailsResponse> {
let form: GetPersonDetails = {
auth: api.auth,
person_id: person_id,
};
return api.client.getPersonDetails(form);
}

export async function deleteUser(api: API): Promise<DeleteAccountResponse> {
let form: DeleteAccount = {
Expand Down
13 changes: 0 additions & 13 deletions crates/api_common/src/build_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,19 +82,6 @@ pub async fn build_post_response(
Ok(PostResponse { post_view })
}

// this is a variation of build_post_response that returns post even if end-user-delted or mod-removed.
// Assumption is that before this function is ever called, the user is already confirmed to be authorized to view post.
// See GitHub Issue #3588
pub async fn build_post_response_deleted_allowed(
context: &Data<LemmyContext>,
_community_id: CommunityId,
person_id: PersonId,
post_id: PostId,
) -> Result<PostResponse, LemmyError> {
let post_view = PostView::read(&mut context.pool(), post_id, Some(person_id), Some(true)).await?;
Ok(PostResponse { post_view })
}

// TODO: this function is a mess and should be split up to handle email seperately
#[tracing::instrument(skip_all)]
pub async fn send_local_notifs(
Expand Down
4 changes: 2 additions & 2 deletions crates/api_crud/src/post/delete.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::PerformCrud;
use actix_web::web::Data;
use lemmy_api_common::{
build_response::build_post_response_deleted_allowed,
build_response::build_post_response,
context::LemmyContext,
post::{DeletePost, PostResponse},
utils::{check_community_ban, check_community_deleted_or_removed, local_user_view_from_jwt},
Expand Down Expand Up @@ -52,7 +52,7 @@ impl PerformCrud for DeletePost {
)
.await?;

build_post_response_deleted_allowed(
build_post_response(
context,
orig_post.community_id,
local_user_view.person.id,
Expand Down
4 changes: 2 additions & 2 deletions crates/api_crud/src/post/remove.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::PerformCrud;
use actix_web::web::Data;
use lemmy_api_common::{
build_response::build_post_response_deleted_allowed,
build_response::build_post_response,
context::LemmyContext,
post::{PostResponse, RemovePost},
utils::{check_community_ban, is_mod_or_admin, local_user_view_from_jwt},
Expand Down Expand Up @@ -61,7 +61,7 @@ impl PerformCrud for RemovePost {
};
ModRemovePost::create(&mut context.pool(), &form).await?;

build_post_response_deleted_allowed(
build_post_response(
context,
orig_post.community_id,
local_user_view.person.id,
Expand Down
3 changes: 1 addition & 2 deletions crates/apub/src/api/list_comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ pub async fn list_comments(

let parent_path_cloned = parent_path.clone();
let post_id = data.post_id;
let local_user = local_user_view.map(|l| l.local_user);
let comments = CommentQuery {
listing_type,
sort,
Expand All @@ -63,7 +62,7 @@ pub async fn list_comments(
community_id,
parent_path: parent_path_cloned,
post_id,
local_user: local_user.as_ref(),
local_user: local_user_view.as_ref(),
page,
limit,
..Default::default()
Expand Down
11 changes: 2 additions & 9 deletions crates/apub/src/api/list_posts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use actix_web::web::{Json, Query};
use lemmy_api_common::{
context::LemmyContext,
post::{GetPosts, GetPostsResponse},
utils::{check_private_instance, is_mod_or_admin_opt, local_user_view_from_jwt_opt},
utils::{check_private_instance, local_user_view_from_jwt_opt},
};
use lemmy_db_schema::source::{community::Community, local_site::LocalSite};
use lemmy_db_views::post_view::PostQuery;
Expand Down Expand Up @@ -42,21 +42,14 @@ pub async fn list_posts(
community_id,
)?);

let is_mod_or_admin = Some(
is_mod_or_admin_opt(&mut context.pool(), local_user_view.as_ref(), community_id)
.await
.is_ok(),
);

let posts = PostQuery {
local_user: local_user_view.map(|l| l.local_user).as_ref(),
local_user: local_user_view.as_ref(),
listing_type,
sort,
community_id,
saved_only,
page,
limit,
is_mod_or_admin,
..Default::default()
}
.list(&mut context.pool())
Expand Down
39 changes: 14 additions & 25 deletions crates/apub/src/api/read_person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use actix_web::web::{Json, Query};
use lemmy_api_common::{
context::LemmyContext,
person::{GetPersonDetails, GetPersonDetailsResponse},
utils::{check_private_instance, is_admin, local_user_view_from_jwt_opt},
utils::{check_private_instance, local_user_view_from_jwt_opt},
};
use lemmy_db_schema::{
source::{local_site::LocalSite, person::Person},
Expand All @@ -26,7 +26,6 @@ pub async fn read_person(

let local_user_view = local_user_view_from_jwt_opt(data.auth.as_ref(), &context).await;
let local_site = LocalSite::read(&mut context.pool()).await?;
let is_admin = local_user_view.as_ref().map(|luv| is_admin(luv).is_ok());

check_private_instance(&local_user_view, &local_site)?;

Expand All @@ -53,48 +52,38 @@ pub async fn read_person(
let limit = data.limit;
let saved_only = data.saved_only;
let community_id = data.community_id;
let local_user = local_user_view.map(|l| l.local_user);
let local_user_clone = local_user.clone();
// If its saved only, you don't care what creator it was
// Or, if its not saved, then you only want it for that specific creator
let creator_id = if !saved_only.unwrap_or(false) {
Some(person_details_id)
} else {
None
};

let posts = PostQuery {
sort,
saved_only,
local_user:local_user.as_ref(),
local_user: local_user_view.as_ref(),
community_id,
is_mod_or_admin: is_admin,
is_profile_view: Some(true),
page,
limit,
creator_id:
// If its saved only, you don't care what creator it was
// Or, if its not saved, then you only want it for that specific creator
if !saved_only.unwrap_or(false) {
Some(person_details_id)
} else {
None
}
,
creator_id,
..Default::default()
}
.list(&mut context.pool())
.await?;

let comments = CommentQuery {
local_user: (local_user_clone.as_ref()),
local_user: (local_user_view.as_ref()),
sort: (sort.map(post_to_comment_sort_type)),
saved_only: (saved_only),
show_deleted_and_removed: (Some(false)),
community_id: (community_id),
is_profile_view: Some(true),
page: (page),
limit: (limit),
creator_id: (
// If its saved only, you don't care what creator it was
// Or, if its not saved, then you only want it for that specific creator
if !saved_only.unwrap_or(false) {
Some(person_details_id)
} else {
None
}
),
creator_id,
..Default::default()
}
.list(&mut context.pool())
Expand Down
17 changes: 7 additions & 10 deletions crates/apub/src/api/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,16 @@ pub async fn search(
data.community_id
};
let creator_id = data.creator_id;
let local_user = local_user_view.map(|l| l.local_user);
let local_user = local_user_view.as_ref().map(|l| l.local_user.clone());
match search_type {
SearchType::Posts => {
posts = PostQuery {
sort: (sort),
listing_type: (listing_type),
community_id: (community_id),
creator_id: (creator_id),
local_user: (local_user.as_ref()),
local_user: (local_user_view.as_ref()),
search_term: (Some(q)),
is_mod_or_admin: (is_admin),
page: (page),
limit: (limit),
..Default::default()
Expand All @@ -75,7 +74,7 @@ pub async fn search(
search_term: (Some(q)),
community_id: (community_id),
creator_id: (creator_id),
local_user: (local_user.as_ref()),
local_user: (local_user_view.as_ref()),
page: (page),
limit: (limit),
..Default::default()
Expand Down Expand Up @@ -112,15 +111,15 @@ pub async fn search(
let community_or_creator_included =
data.community_id.is_some() || data.community_name.is_some() || data.creator_id.is_some();

let local_user_ = local_user.clone();
let q = data.q.clone();

posts = PostQuery {
sort: (sort),
listing_type: (listing_type),
community_id: (community_id),
creator_id: (creator_id),
local_user: (local_user_.as_ref()),
local_user: (local_user_view.as_ref()),
search_term: (Some(q)),
is_mod_or_admin: (is_admin),
page: (page),
limit: (limit),
..Default::default()
Expand All @@ -130,14 +129,13 @@ pub async fn search(

let q = data.q.clone();

let local_user_ = local_user.clone();
comments = CommentQuery {
sort: (sort.map(post_to_comment_sort_type)),
listing_type: (listing_type),
search_term: (Some(q)),
community_id: (community_id),
creator_id: (creator_id),
local_user: (local_user_.as_ref()),
local_user: (local_user_view.as_ref()),
page: (page),
limit: (limit),
..Default::default()
Expand Down Expand Up @@ -186,7 +184,6 @@ pub async fn search(
community_id: (community_id),
creator_id: (creator_id),
url_search: (Some(q)),
is_mod_or_admin: (is_admin),
page: (page),
limit: (limit),
..Default::default()
Expand Down

0 comments on commit 047db9a

Please sign in to comment.