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

Warn users and delete user accounts after period of inactivity #2088

Merged
merged 13 commits into from
Jan 27, 2025

Conversation

Anuj-Gupta4
Copy link
Collaborator

@Anuj-Gupta4 Anuj-Gupta4 commented Jan 15, 2025

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation
  • πŸ§‘β€πŸ’» Refactor
  • βœ… Test
  • πŸ€– Build or CI
  • ❓ Other (please specify)

Related Issue

Describe this PR

added a new field to track users last active date based on login in users table
added a task endpoint to warn users before deletion and delete users if the inactivity threshold has been met.

Checklist before requesting a review

[optional] What gif best describes this PR or how it makes you feel?

@github-actions github-actions bot added enhancement New feature or request backend Related to backend code migration Contains a DB migration labels Jan 15, 2025
@Anuj-Gupta4
Copy link
Collaborator Author

We will probably need a scheduler to be able to periodically warn and delete inactive users.

@Anuj-Gupta4 Anuj-Gupta4 marked this pull request as ready for review January 17, 2025 03:54
Copy link
Member

@spwoodcock spwoodcock left a comment

Choose a reason for hiding this comment

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

If we add a db connection on each login_required call, I think this means the update is done for every single endpoint called.

Instead, would it be better to add this logic to the /me endpoint only, when the login details are retreived (on first page load)?

Could you simply add to the select SQL .one logic something like UPDATE users SET last_login_at = NOW() WHERE id = %(user_id)s;

@spwoodcock
Copy link
Member

You could probably merge this into the existing logic, adding a RETURNING clause to get the user after update

@Sujanadh
Copy link
Collaborator

Sujanadh commented Jan 22, 2025

add migration in fmtm_base_schema as well

Copy link
Member

@spwoodcock spwoodcock left a comment

Choose a reason for hiding this comment

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

Excellent work! πŸŽ‰

Note to self: needs a cron to run this as a final step, but for now the admin calling the endpoint periodically also works

@spwoodcock spwoodcock changed the title Feat/gdpr compliance Warn users and delete user accounts after period of inactivity Jan 27, 2025
@spwoodcock spwoodcock merged commit bb2fa96 into development Jan 27, 2025
7 checks passed
@spwoodcock spwoodcock deleted the feat/GDPR-compliance branch January 27, 2025 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to backend code enhancement New feature or request migration Contains a DB migration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants