🪟 🎉 Add username editing #14242

YatsukBogdan1 · 2022-06-29T10:05:17Z

What

Closes #1268

How

I refactored User Settings Account section, so Name, Email and Password sections follow the same structure

Loom

https://www.loom.com/share/e1f141488f454dc78dc1d1e35a496b92

edmundito · 2022-06-29T13:05:58Z

airbyte-webapp/src/packages/cloud/services/auth/AuthService.tsx

@@ -112,6 +114,11 @@ export const AuthenticationProvider: React.FC = ({ children }) => {
        queryClient.removeQueries();
        loggedOut();
      },
+      async updateName(name: string): Promise<void> {
+        await userService.changeName(state.currentUser?.authUserId || "", state.currentUser?.userId || "", name);


The "" should never happen. One way you can prevent this is to check if the current user exits, for example:

async updateName(name: string): Promise<void> { if (!state.currentUser) return; // Now you can use `state.currentUser.authUserId` without the `?` }

edmundito · 2022-06-29T13:07:42Z

...ebapp/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/typings.d.ts

@@ -0,0 +1,3 @@
+export interface FormValues {


The convention for the types filename is types.ts

edmundito · 2022-06-29T13:08:35Z

...pp/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/NameSection.tsx

+import { useCurrentUser } from "packages/cloud/services/auth/AuthService";
+import FeedbackBlock from "pages/SettingsPage/components/FeedbackBlock";
+
+const NameSection: React.FC = () => {


We're moving away from exporting defaults, so this can be:

Suggested change

const NameSection: React.FC = () => {

export const NameSection: React.FC = () => {

The same suggestion applies for other areas of the code, then you can just export it directly on the index.ts file.

edmundito · 2022-06-29T13:09:40Z

airbyte-webapp/src/packages/cloud/views/users/AccountSettingsView/AccountSettingsView.tsx

-          </Formik>
-        </Content>
-      </SettingsCard>
+      <NameSection />


This makes it so much cleaner, thanks for extracting 🎉

edmundito · 2022-06-29T13:13:03Z

airbyte-webapp/src/packages/cloud/services/auth/AuthService.tsx

+        await userService.changeName(state.currentUser?.authUserId || "", state.currentUser?.userId || "", name);
+        updateUserName({ value: name });
+        return authService.updateProfile(name);


Add safety checks here to ensure both firebase and our user service get updated, and if one of them fails, rollback and return. the userName state should be updated last when it's guaranteed that both the user service and firebse got the update.

edmundito · 2022-06-29T13:14:21Z

...p/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/hooks/useName.ts

+  changeName: (values: FormValues, { setSubmitting, setFieldValue }: FormikHelpers<FormValues>) => void;
+};
+
+const useName: UseNameHook = () => {


This hook doesn't return the name, should probably be useChangeName

edmundito · 2022-06-29T13:15:42Z

airbyte-webapp/src/packages/cloud/services/auth/reducer.ts

+    if (!state.currentUser) {
+      return state;
+    }


As mentioned in a separate comment, this should never happen if we check that the current user exists first.

I had this error, so that was only workaround here

TS2322: Type '{ name: string; email?: string | undefined; authUserId?: string | undefined; userId?: string | undefined; status?: UserStatus | undefined; intercomHash?: string | undefined; }' is not assignable to type 'User'. Types of property 'email' are incompatible. Type 'string | undefined' is not assignable to type 'string'. Type 'undefined' is not assignable to type 'string'.

edmundito · 2022-06-29T13:17:24Z

...pp/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/NameSection.tsx

+              <LoadingButton type="submit" isLoading={isSubmitting}>
+                <FormattedMessage id="settings.accountSettings.updateName" />
+              </LoadingButton>
+              <FeedbackBlock errorMessage={errorMessage} successMessage={successMessage} isLoading={isSubmitting} />


Suggested change

<FeedbackBlock errorMessage={errorMessage} successMessage={successMessage} isLoading={isSubmitting} />

<FeedbackBlock errorMessage={errorMessage} successMessage={successMessage} />

The LoadingButton already gives us a loading indicator.

Suggested change

<FeedbackBlock errorMessage={errorMessage} successMessage={successMessage} isLoading={isSubmitting} />

<FeedbackBlock errorMessage={errorMessage} successMessage={successMessage} isLoading={isSubmitting} />

edmundito

Looking a lot better but needs more validation and some minor things. I tested locally and looks great. Also update with the latest master branch.

edmundito · 2022-07-19T14:15:44Z

...p/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/hooks/useName.ts

+      switch (err.code) {
+        default:


This switch does not seem necessary

edmundito · 2022-07-19T14:18:16Z

...pp/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/NameSection.tsx

+export const NameSection: React.FC = () => {
+  const { formatMessage } = useIntl();
+  const user = useCurrentUser();
+  const { changeName, successMessage, errorMessage } = useChangeName();


Great use of a hook to keep the component clean.

...pp/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/NameSection.tsx

edmundito · 2022-07-19T14:30:04Z

...pp/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/NameSection.tsx

+                  )}
+                </Field>
+              </RowFieldItem>
+              <LoadingButton type="submit" isLoading={isSubmitting}>


Should be disabled if the form is not valid

edmundito · 2022-07-19T14:31:52Z

...p/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/hooks/useName.ts

+          setErrorMessage(
+            formatMessage({
+              id: "settings.accountSettings.updateNameError",
+            }) + JSON.stringify(err)


This error may be confusing to users. It may be better to console.error this error instead of stringifying it. @timroes is there a way we can send this error to Sentry?

this logic is copied from PasswordSection. not sure, how the error should be approached here

...pp/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/NameSection.tsx

# Conflicts: # airbyte-webapp/src/packages/cloud/views/users/AccountSettingsView/AccountSettingsView.tsx

…t valid

edmundito · 2022-07-20T20:21:21Z

...pp/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/NameSection.tsx

+            name: user.name,
+          }}
+          onSubmit={changeName}
+          validate={validate}


We've been using yup to validate the schema. Search the code for an example.

…n schema

edmundito · 2022-07-21T22:51:35Z

...pp/src/packages/cloud/views/users/AccountSettingsView/components/NameSection/NameSection.tsx

+                  )}
+                </Field>
+              </RowFieldItem>
+              <LoadingButton disabled={!isValid} type="submit" isLoading={isSubmitting}>


Just tested and probably we should also check if the form is dirty before we enable the button. Otherwise, the user can press the button to save the same name over and over.

YatsukBogdan1 added 2 commits June 29, 2022 12:58

Adds editing for user name is settings tab

90c7ead

Merge branch 'master' into byatsuk/feature-add-username-editing

34fd3cb

YatsukBogdan1 requested a review from a team as a code owner June 29, 2022 10:05

github-actions bot added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Jun 29, 2022

edmundito suggested changes Jun 29, 2022

View reviewed changes

edmundito changed the title ~~Add username editing~~ 🪟 🎉 Add username editing Jun 30, 2022

Fixes eslint issues; Fixes comments from PR

6227682

edmundito requested a review from timroes July 19, 2022 14:18

edmundito suggested changes Jul 19, 2022

View reviewed changes

YatsukBogdan1 added 2 commits July 20, 2022 11:32

Merge branch 'master' into byatsuk/feature-add-username-editing

680daaa

# Conflicts: # airbyte-webapp/src/packages/cloud/views/users/AccountSettingsView/AccountSettingsView.tsx

Adds validation for NameSection; Disables update button if form is no…

86afdc7

…t valid

YatsukBogdan1 requested a review from edmundito July 20, 2022 08:57

edmundito suggested changes Jul 20, 2022

View reviewed changes

YatsukBogdan1 added 2 commits July 21, 2022 12:52

Merge branch 'master' into byatsuk/feature-add-username-editing

f0eedf2

Refactors formik validation logic from validate function to validatio…

0f0558d

…n schema

YatsukBogdan1 requested a review from edmundito July 21, 2022 09:57

edmundito suggested changes Jul 21, 2022

View reviewed changes

Updates submit button to be disabled while form is dirty

a25799d

edmundito approved these changes Aug 3, 2022

View reviewed changes

YatsukBogdan1 merged commit 8282a45 into master Aug 8, 2022

YatsukBogdan1 deleted the byatsuk/feature-add-username-editing branch August 8, 2022 08:48

octavia-squidington-iii mentioned this pull request Aug 10, 2022

Bump Airbyte version from 0.39.42-alpha to 0.40.0-alpha #15493

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🪟 🎉 Add username editing #14242

🪟 🎉 Add username editing #14242

YatsukBogdan1 commented Jun 29, 2022

edmundito Jun 29, 2022

edmundito Jun 29, 2022

edmundito Jun 29, 2022

edmundito Jun 29, 2022

edmundito Jun 29, 2022

edmundito Jun 29, 2022

edmundito Jun 29, 2022

YatsukBogdan1 Jul 4, 2022

edmundito Jun 29, 2022

edmundito left a comment

edmundito Jul 19, 2022

edmundito Jul 19, 2022

edmundito Jul 19, 2022

edmundito Jul 19, 2022

YatsukBogdan1 Jul 20, 2022

edmundito Jul 20, 2022

YatsukBogdan1 Jul 21, 2022

edmundito Jul 21, 2022

	const NameSection: React.FC = () => {
	export const NameSection: React.FC = () => {

	<FeedbackBlock errorMessage={errorMessage} successMessage={successMessage} isLoading={isSubmitting} />
	<FeedbackBlock errorMessage={errorMessage} successMessage={successMessage} />

🪟 🎉 Add username editing #14242

🪟 🎉 Add username editing #14242

Conversation

YatsukBogdan1 commented Jun 29, 2022

What

How

Loom

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edmundito left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment