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

Dont blank out post or community info. Fixes #1813 #1841

Merged
merged 9 commits into from
Oct 27, 2021
Merged

Conversation

dessalines
Copy link
Member

Going on record saying I really don't like this, since the title of the post and community would still be in the modlog, and would also show the reason for it being removed. This PR will let you go to removed posts and communities and see the content.

@Nutomic
Copy link
Member

Nutomic commented Oct 16, 2021

Maybe we should discuss this on Lemmy, and get opinions from more people before merging.

@dessalines
Copy link
Member Author

dessalines commented Oct 20, 2021

I thought about moving the blanking into the view reads ... but it seemed too dangerous a thing. Its better to separate query logic from post-query manipulation.

.filter(|p| p.post.deleted || p.post.removed)
{
pv.post = pv.to_owned().post.blank_out_deleted_or_removed_info();
}
Copy link
Member

Choose a reason for hiding this comment

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

Postview also contains PersonSafe and CommunitySafe, i believe they also need to be blanked out here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Those don't need to be blanked, as they weren't the removed items. Its the same in community/read.rs, which also has a PersonSafe.

Copy link
Member

Choose a reason for hiding this comment

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

How do you know? Its possible that someone calls this endpoint with a post where the author or community was deleted. That would allow circumventing the blanking.

@dessalines
Copy link
Member Author

@Nutomic This is actually passing: https://cloud.drone.io/LemmyNet/lemmy/1686

@Nutomic Nutomic merged commit d9ecabe into main Oct 27, 2021
@Nutomic
Copy link
Member

Nutomic commented Oct 27, 2021

Seems like something is broken with the ARM builder (or with our tests).

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