fix: edit password action

[dnsos]

This PR makes fixes to the action of editing a password.

• we now display all error messages
• we also display a success message in a separate step/view
• we also check that the new password is different from the old one

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
giessdenkiez-de	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 14, 2023 9:54am

[dnsos]

Hm, even with this workaround we don't see the success notification because the new session causes the PasswordEditForm to always unmount. But we want to see a success message. I think we really have to reconsider creating a new session for checking the old password. FYI @ff6347

[ff6347]

Maybe the following works. When creating a new supbaseClient with import {createClient} from @supabase/supabase-js there is an option to persist the session into storage. See #562 Maybe we can create a client instance just for testing if the password works?

Like:

src/utils/validatePassword.ts

import {createClient} from @supabase/supabase-js
export const supabaseValidationClient = createClient<Database>(
	process.env.NEXT_PUBLIC_SUPABASE_URL,
 	process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY,
{
persistSession: false,
autoRefreshToken: false
})

Then in src/components/Forms/PasswordEditForm/index.tsx use it as

import {supabaseValidationClient} from "utils..."
supabaseValidationClient.auth.signInWithPassword({email, password})

[dnsos]

@ff6347, yes that approach works - but not the success notification for a successful password change. Even if we would use the non-persisting client for updateUser as well, a successful password update also results in the form component to unmount. This means with this workaround we get all the error messages, but not the success message. Perhaps that's acceptable, I'm not sure.

Another path we could take: We are in a migration of auth providers. This password change form should be considered a new feature. For the current GdK we don't have this form, so perhaps we could just not offer this functionality. We still have the password reset when logging in and additionally we could display something like this in the profile UI:

"Du möchtest dein Passwort ändern - wir schicken dir eine Mail dafür" (or similar). This would send the user the password reset email.

[ff6347]

This means with this workaround we get all the error messages, but not the success message. Perhaps that's acceptable, I'm not sure.

IMO This is better then not giving the possibility to change the password

[dnsos]

Okay @ff6347, I've made some more changes. We now continue giving the option of changing the password. For showing a success message I've had to make quite some refactoring.

Perhaps we could use the same flow for editing the account details. What do you think?

[ff6347]

Works like a charm. I ❤️ it. You removed a lot of my messy 🍝. Thanks for that. The only thing I would have done different is the updatePassword function. Since it returns only a string we have to review the content of the string to see if the action was successful.

Yes I know currently it throws an Error or returns a string. I still would have gone for some updatePassword(props): Promise<boolean> or updatePassword(props): Promise<{success: boolean; message: string}> but this is up to you. Not a blocker just personal style.

[ff6347]

Perhaps we could use the same flow for editing the account details. What do you think?

What is "the flow"?

[dnsos]

What is "the flow"?

The user flow. So making the account modal behave the same way as the password modal: first giving the form and when successful a success screen and the button to close the modal.

The difference is that when editing the account details we don't really need the success message because we can see the success when the new account details are displayed in the sidebar. This is not the case for the password change. So I don't see it as super necessary to go for the same user flow. But we could. What do you say?

[dnsos]

Yes I know currently it throws an Error or returns a string. I still would have gone for some updatePassword(props): Promise or updatePassword(props): Promise<{success: boolean; message: string}> but this is up to you. Not a blocker just personal style.

Hmm, I see your point. Still, returning only the success message seems to be the most straightforward option. Returning a boolean doesn't make too much sense because it would always return true, never false. If the function doesn't pass the validations, it throws.

Plus, having the success message side-by-side with the error messages feels pretty concise. So all in all, I would suggest we keep the current version.

[ff6347]

What is "the flow"?

The user flow. So making the account modal behave the same way as the password modal: first giving the form and when successful a success screen and the button to close the modal.

The difference is that when editing the account details we don't really need the success message because we can see the success when the new account details are displayed in the sidebar. This is not the case for the password change. So I don't see it as super necessary to go for the same user flow. But we could. What do you say?

Yes. Would be nice to confirm and not await that the user sees it.