Refactor registrations#destroy to validate password if required #23636
Conversation
| }, status: :bad_request | ||
| return | ||
| end | ||
| current_user.destroy |
There was a problem hiding this comment.
I'm not calling super here because Devise's registrations#destroy uses Rails routing and we want to control the response from the front-end (navigate to /home on success, handle errors in the deletion modal on failure).
| @@ -51,6 +51,26 @@ def create | |||
| end | |||
| end | |||
|
|
|||
| def destroy | |||
| # TODO: (madelynkasula) Remove this flag when the new account | |||
| # deletion UI is available for all users. | |||
There was a problem hiding this comment.
It might be helpful for this comment to mention the ACCOUNT_DELETION_NEW_FLOW experiment flag by name, so when we are tearing out that flag we also find this comment.
There was a problem hiding this comment.
good call. adding that now
dashboard/config/locales/en.yml
Outdated
| @@ -199,6 +199,7 @@ en: | |||
| pair_programming: 'I have a partner at my computer' | |||
| student_privacy: "Learn more about why you're not seeing your full name <a href='%{student_privacy_blog}' target='_blank'>here</a>." | |||
| password: | |||
| invalid_password_entered: 'The password you entered is invalid.' | |||
There was a problem hiding this comment.
This must already exist somewhere. Can we find whatever existing 'bad password' message we have and reuse that?
There was a problem hiding this comment.
On login, we use a "Invalid email or password" message, which I think would be the closest existing string in dashboard. I could add the error to current_user and return that?
if password_required && invalid_password
current_user.errors.add :current_password
render json: {
errors: current_user.errors.as_json(full_messages: true)
}, status: :bad_request
return
end
There was a problem hiding this comment.
I might prefer that just to be uniform and reuse existing translations. I'm okay if you feel that's more complex than necessary though.
There was a problem hiding this comment.
no, i don't think that's too complex. it might actually make processing the error easier on the front-end. i'll refactor to that
|
@islemaster good call. i added that to our backlog and will make a follow-up PR |
For the new account deletion flow (spec), account deletion will be controlled by a React component using an AJAX request rather than a Rails form. This PR refactors the
registrations#destroymethod to allow for this new configuration while still maintaining old behavior by honoring anew_destroy_flowflag inparams.For the new flow, we want to require the user to input their password (if they have one) before deleting their account.