Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show deleted and removed posts for profile views. Fixes #2624 #2729

Merged
merged 12 commits into from
Mar 1, 2023

Conversation

dessalines
Copy link
Member

No description provided.

@dessalines dessalines requested a review from Nutomic as a code owner February 13, 2023 17:22
@Nutomic
Copy link
Member

Nutomic commented Feb 14, 2023

If I understand the code right, any logged in user would be able to see all deleted/remove posts and comments on any user profile. That doesnt seem right, as even posts that were removed by mod action would still be visible. And deleted posts shouldnt be visibile to anyone else.

I would change this so deleted/removed posts and comments are only visible to the author.

@dessalines
Copy link
Member Author

Correct... I'll take another look at it and see if there's some way I can do it. My sql skills aren't too great, really wish we had a DB person.

@dessalines dessalines marked this pull request as draft February 14, 2023 20:38
@dessalines dessalines marked this pull request as ready for review February 14, 2023 21:36
@dessalines
Copy link
Member Author

K, got this working. Not sure how efficient it is.

@dessalines dessalines marked this pull request as draft February 15, 2023 14:48
@dessalines
Copy link
Member Author

An additional issue with this, is that it doesn't allow mods or admins to see deleted or removed content. Not a fun issue to try to solve with SQL.

@Nutomic
Copy link
Member

Nutomic commented Feb 18, 2023

An additional issue with this, is that it doesn't allow mods or admins to see deleted or removed content. Not a fun issue to try to solve with SQL.

That can be solved later in a separete PR. Or it could be done in Rust if we are reading those items from DB anyway.

@dessalines
Copy link
Member Author

I don't know if this is should be merged without that... as the PR currently stands, no mods or admins can see deleted or removed posts. Only the post creator can.

@Nutomic
Copy link
Member

Nutomic commented Feb 23, 2023

Thats strictly an improvement to the current behaviour, no?

@dessalines
Copy link
Member Author

For viewing individual posts, it scares me a bit, because what I've added means that mods and admins will no longer be able to see deleted or removed posts at post/X .

One hack I could do, is pass admin / mod status to the query builder, and only do this filtering if you're not a mod or admin. That's the only way I can think to do this easily.

@dessalines dessalines marked this pull request as ready for review February 23, 2023 19:35
@dessalines
Copy link
Member Author

Okay, I tested this, and it works. I got rid of the "blanking" functions as well.

A note tho, that this still isn't ideal because it isn't in SQL, and I'm not doing a join to the community_moderator table, and local_user table. A few cases I can think of:

  • A call to a front page GetPosts , from a regular user who moderates some communities, will not show deleted / removed posts for the communities they mod. However, they can go either to the community page (IE /c/their_comm), or to the posts directly.
  • A call to GetPosts from a user profile page, from a community moderator, will also not show deleted / removed.

@Nutomic Nutomic requested a review from makotech222 February 24, 2023 03:52

// Hide deleted and removed for non-admins or mods
if !is_mod_or_admin.unwrap_or(false) {
// If you are not the creator, then remove the other fields.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment seems wrong, its excluding these posts from results not removing any fields.

@@ -303,6 +342,23 @@ impl<'a> PostQuery<'a> {
))
.into_boxed();

// Hide deleted and removed for non-admins or mods
if !self.is_mod_or_admin.unwrap_or(false) {
// If you are not the creator, then remove the other fields.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

// TODO these should be removed, there should be another way to do this
pub trait DeleteableOrRemoveable {
fn blank_out_deleted_or_removed_info(self) -> Self;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the reason this isnt needed anymore is because normal users dont see removed content at all now, right? I think this will require a change in lemmy-ui so that removed content isnt rendered unless you click a button. Otherwise if there are offensive images or text posted, it would be difficult to hide.

Screenshot_20230224_181239

Something like this in phpBB, you have to click a button to see the content of a deleted post.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct for posts, but they shouldn't come back at all, for anyone but moderators / admins and creators. The front ends could still blur / hide content similar to what's above tho (at least for the mods and admins).

For comments, all results do need to come back, because otherwise the comment tree could be missing some branches. The front ends use the deleted and removed fields to hide results anyway, so this blanking was pretty pointless and needlessly complicated.

@Nutomic
Copy link
Member

Nutomic commented Feb 24, 2023

Regardless of the limitations I think this is still an improvement and its okay to merge. However I would only release it as part of 0.18, to avoid major changes in behaviour in a patch release.

Btw federation tests are failing.

@dessalines dessalines marked this pull request as draft February 28, 2023 17:28
@dessalines dessalines marked this pull request as ready for review March 1, 2023 03:45
@dessalines dessalines merged commit 48f1871 into main Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants