adhocracy-plus
adhocracy-plus copied to clipboard
liqd
WIP: [8104] apps/accounts: add option to delete account to user profile
First attempt at adding the account deletion submenu
page: http://127.0.0.1:8004/account/account_deletion/
Tasks
- [ ] PR name contains story or task reference
- [ ] Documentation (docs and inline)
- [ ] Tests (including n+1 and django_assert_num_queries where applicable)
- [ ] Changelog
Hey @goapunk @mcastro-lqd, I'm still pondering the best way to handle the delete account process.
Right now, the modal can pop up without any checks on the frontend, meaning someone could enter the wrong password or leave the input blank. Then, if they submit, any errors from the server validation just reload.
My idea is to ditch the modal altogether and instead have a checkbox below the password field saying something like:
- [ ] I understand that I won't be able to recover my account.
If any required field is empty the browser validation will also trigger on submit.
I reckon the password alone is sufficient security-wise, as it already adds a layer of complexity.
@hom3mad3
Right now, the modal can pop up without any checks on the frontend, meaning someone could enter the wrong password or leave the input blank. Then, if they submit, any errors from the server validation just reload.
I guess the modal step makes it a bit clunky but otherwise I'd say that's just how django forms work. One argument against the checkbox is that this would be a change in our design pattern. We use a modal to confirm deletion everywhere else on the page, so deviating from this seems not ideal to me. If this is not a viable way to do it we could of course write it all in react to have a more coherent flow or at least add some js which disables the button until you entered a password?
@hom3mad3 it sounds also fine for me, as I said I did it based on the story request, but I'm open for some changes in case of a high complexity.
@goapunk @hom3mad3 I think it is fine for now to make do with the django form errors, even if page needs to reload for user to get the error.
@m4ra @hom3mad3 what's the state / how do we proceed ? Does this need more work or is it ready for review / merging?
@m4ra @hom3mad3 what's the state / how do we proceed ? Does this need more work or is it ready for review / merging?
@goapunk i'm not sure, I believe a pm should decide on this matter. personally, I'm okay with keeping it as is, although I'm not entirely satisfied with the current status.
@hom3mad3 @m4ra talked to pm and they are ok with this solution, so I would suggest @m4ra can review it
the password doesn't autocomple now, and the related pictures of the user are deleted. I will merge this, and we will work on the email notification on a new PR @goapunk