Skip to content

Add grace period for account deletions#320

Merged
jonbarrow merged 4 commits intodevfrom
feat/grace-period
Jan 28, 2026
Merged

Add grace period for account deletions#320
jonbarrow merged 4 commits intodevfrom
feat/grace-period

Conversation

@jonbarrow
Copy link
Member

Resolves #303

Changes:

Adds new marked_for_deletion and hard_delete_time fields to the PNID model, and replaces all instances of pnid.scrub() with pnid.markForDeletion(). pnid.markForDeletion() sets marked_for_deletion to true and hard_delete_time to 7 days in the future. A new cron job runs every day at 2am (same as the BOSS server cron job) to run through all the pending deletions ready to be deleted, and then processes them. Also checks at server boot to catch any that happened during downtime, though once the cron job starts it shouldn't matter anyway

This lets us "rollback" any accidental deletions that may occur. There isn't a way for us to undo a pending deletion at the moment outside of direct DB access, but that should be fine for now since I don't think we'd be processing too many of those kinds of requests

Should be merged before #313, since these changes can be deployed at any time

@jonbarrow jonbarrow changed the title feat: add grace period for account deletions Add grace period for account deletions Jan 21, 2026
Copy link
Member

@DaniElectra DaniElectra left a comment

Choose a reason for hiding this comment

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

While we're here I think we should also update how the scrubbing works. Right now it scrubs the access level and sets it to 0, but this is kinda problematic especially for moderation, such as in the case where a PNID is banned and they delete their account.

I'd say to reset the level to 0 if it's above 0, but keep it as-is if it's lower than 0

@jonbarrow
Copy link
Member Author

While we're here I think we should also update how the scrubbing works. Right now it scrubs the access level and sets it to 0, but this is kinda problematic especially for moderation, such as in the case where a PNID is banned and they delete their account.

I'd say to reset the level to 0 if it's above 0, but keep it as-is if it's lower than 0

I forgot about this issue, sounds good to me

@jonbarrow jonbarrow requested a review from DaniElectra January 21, 2026 22:22
Copy link
Member

@DaniElectra DaniElectra left a comment

Choose a reason for hiding this comment

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

LGTM

@jonbarrow
Copy link
Member Author

@mrjvs does this look good to you? Wanted to make sure the cron job and such won't mess with any deployments since it's the first time I've use a cron job since the new infra came out

@mrjvs
Copy link
Contributor

mrjvs commented Jan 28, 2026

There's no problems with using nodejs cronjobs in the current infra setup.

If there is ever a need to scale the account server up to multiple instances, we may need a distributed locking system on top of cronjobs (so that we dont scrub the same account twice in the same millisecond)

@jonbarrow
Copy link
Member Author

There's no problems with using nodejs cronjobs in the current infra setup.

Gotcha

If there is ever a need to scale the account server up to multiple instances, we may need a distributed locking system on top of cronjobs (so that we dont scrub the same account twice in the same millisecond)

Is this a concern for now or do you think it can wait until later?

@mrjvs
Copy link
Contributor

mrjvs commented Jan 28, 2026

It can wait, I don't think account currently can be horizontally scaled anyway. There's more work to be done before that's an option.

@jonbarrow
Copy link
Member Author

Sounds good then, thanks 👍

@jonbarrow jonbarrow merged commit fde09d8 into dev Jan 28, 2026
7 checks passed
@jonbarrow jonbarrow deleted the feat/grace-period branch January 28, 2026 17:05
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.

[Enhancement]: Add a grace period for the account deletion process to allow people to recover their deleted account

3 participants