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

after 30 days, replace comment.content and post.body with 'Deleted' #3208

Merged
merged 3 commits into from
Jun 26, 2023

Conversation

vijaykramesh
Copy link
Contributor

fixes #2977

on server startup, and then with a daily scheduler thread -

check for all posts & comments where deleted = true && updated < 30 days ago && body/content != "Deleted" and sets it to "Deleted"

this way deletes for comments & posts get federated after 30 days by default.

@vijaykramesh vijaykramesh force-pushed the clear_deleted_posts_comments branch 2 times, most recently from e8e2455 to 0c15d10 Compare June 20, 2023 06:33
.filter(post::updated.lt(now - 30.days()))
.filter(post::body.ne("Deleted")),
)
.set(post::body.eq("Deleted"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not harmonize this with the text used for example in https://github.com/LemmyNet/lemmy/blob/main/crates/db_schema/src/impls/comment.rs#L34 (and fixing the typo)?

Copy link
Member

Choose a reason for hiding this comment

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

Would also be good to localize it, see how its done for email

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't tackle localization yet (in part because it saves the actual string to the database, so I'm not sure if we should localize based on the author, the instance, or save the token and then localize it on the way back out for display purposes?

but I did tackle making this standard with what is already there in comment & post, namely having it set the text to

    let perma_deleted = "*Permanently Deleted*";

info!("Overwriting deleted posts...");
match diesel::update(
post::table
.filter(post::deleted.eq(true))
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't there also the removed column we could check?

Copy link
Member

Choose a reason for hiding this comment

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

removed is in case of mod action. I think those should be kept for transparency.

@Nutomic
Copy link
Member

Nutomic commented Jun 20, 2023

You have some conflicts, need to rebase with latest main branch.

@vijaykramesh vijaykramesh force-pushed the clear_deleted_posts_comments branch 6 times, most recently from af3f921 to 5e23bd8 Compare June 20, 2023 23:52
/// overwrite posts and comments 30d after deletion
fn overwrite_deleted_posts_and_comments(conn: &mut PgConnection) {
// TODO: localize this at display time via lemmy-translations
let perma_deleted = "*Permanently Deleted*";
Copy link
Member

Choose a reason for hiding this comment

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

You can move this to a constant so it can be reused across the different functions. No need for the todo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@vijaykramesh vijaykramesh force-pushed the clear_deleted_posts_comments branch 2 times, most recently from ad08b10 to a0e4be1 Compare June 21, 2023 15:44
@@ -111,14 +111,13 @@ impl Post {
) -> Result<Vec<Self>, Error> {
let conn = &mut get_conn(pool).await?;

let perma_deleted = "*Permananently Deleted*";
let perma_deleted_url = "https://deleted.com";
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels off. If there must be a URL I'd prefer it being something along the lines of the instance's homepage rather than a hardcoded domain controlled by no-idea-who

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@vijaykramesh vijaykramesh force-pushed the clear_deleted_posts_comments branch 2 times, most recently from 64098d9 to 76e3d94 Compare June 21, 2023 19:45
@Nutomic Nutomic merged commit 418bca7 into LemmyNet:main Jun 26, 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.

Clear text of deleted posts/comments after 30 days
4 participants