-
-
Notifications
You must be signed in to change notification settings - Fork 508
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
feat: add delete account option (attempt 2) #980
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Thank you for your PR! Just a couple of comments
I was actually halfway through implementing this when you opened your PR, so also have a database migration that will make this really fast on the server. I can include my changes once this is in :D
Oh wow, that's such a quick response 🤩 I assume you still want to merge this, right? If you've already done a lot, I'm happy to close this.. Assuming you do, I rename it everywhere. Are we keeping the endpoint as it is? |
Hahaha, I was looking at this anyway so all the context is there :D Yup we can go with this! I'll probs just rebase what I have, as there are a couple of related changes I want to add. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple more things then I think we are good to go! 🚀
I think I've changed everything you wanted, though I might've missed something somewhere (please let me know if there is) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thank you for working on this 🙏
Seeing as this is your first time contributing, if you would like a holographic contributors-only Atuin sticker, then please fill out this form!
We do also have a Discord if you'd like to ask any questions, or just fancy hanging out!
Either way - thank you so much for the time and effort here ✨
Thanks so much, I really love the idea of the stickers 🤩 Have just filled the form. I still worry about accidental deletion though. Would it not make sense to put some sort of safety in? |
* Added DELETE register endpoint * Added remove function to database * Added unregister to client * Updated docs * Renamed functions * Reformatting * Used execute instead of fetch in delete_user
This is implementing an account deletion option, similar to #266. I've made the following changes
remove_user
function to the database that deletes the user fromusers
,sessions
, andhistory
/register
endpoint but could be moved. If this gets called by a logged in user, the account gets deleteunregister
command to the CLII'm wondering how we should prevent accidental account deletion. We could ask the user provide their password, email, or username
Any suggestions?
PS: Thanks for looking into this and thanks for building this, it's a really cool tool 😊