Fixes signup workflow and UI (#26263)

* fixes signup flow

* Apply suggestions from code review

Co-authored-by: Hugo Häggmark <hugo.haggmark@gmail.com>

* Update ForgottenPassword.tsx

* fixes build failure

* fixes build failure

Co-authored-by: Hugo Häggmark <hugo.haggmark@gmail.com>

pkg/api/api.go

@@ -88,6 +88,7 @@ func (hs *HTTPServer) registerRoutes() {
 	r.Get("/alerting/*", reqEditorRole, hs.Index)
 
 	// sign up
+	r.Get("/verify", hs.Index)
 	r.Get("/signup", hs.Index)
 	r.Get("/api/user/signup/options", Wrap(GetSignUpOptions))
 	r.Post("/api/user/signup", quota("user"), bind(dtos.SignUpForm{}), Wrap(SignUp))

public/app/core/components/ForgottenPassword/ForgottenPassword.tsx

@@ -3,7 +3,6 @@ import { Form, Field, Input, Button, Legend, Container, useStyles, HorizontalGro
 import { getBackendSrv } from '@grafana/runtime';
 import { css } from 'emotion';
 import { GrafanaTheme } from '@grafana/data';
-
 import config from 'app/core/config';
 
 interface EmailDTO {
@@ -21,6 +20,7 @@ const paragraphStyles = (theme: GrafanaTheme) => css`
 export const ForgottenPassword: FC = () => {
   const [emailSent, setEmailSent] = useState(false);
   const styles = useStyles(paragraphStyles);
+  const loginHref = `${config.appSubUrl}/login`;
 
   const sendEmail = async (formModel: EmailDTO) => {
     const res = await getBackendSrv().post('/api/user/password/send-reset-email', formModel);
@@ -34,7 +34,7 @@ export const ForgottenPassword: FC = () => {
       <div>
         <p>An email with a reset link has been sent to the email address. You should receive it shortly.</p>
         <Container margin="md" />
-        <LinkButton variant="primary" href="/login">
+        <LinkButton variant="primary" href={loginHref}>
           Back to login
         </LinkButton>
       </div>
@@ -55,7 +55,7 @@ export const ForgottenPassword: FC = () => {
           </Field>
           <HorizontalGroup>
             <Button>Send reset email</Button>
-            <LinkButton variant="link" href={`${config.appSubUrl}/login`}>
+            <LinkButton variant="link" href={loginHref}>
               Back to login
             </LinkButton>
           </HorizontalGroup>

public/app/core/components/Login/LoginPage.tsx

@@ -11,7 +11,6 @@ import { ChangePassword } from '../ForgottenPassword/ChangePassword';
 import { Branding } from 'app/core/components/Branding/Branding';
 import { HorizontalGroup, LinkButton } from '@grafana/ui';
 import { LoginLayout, InnerBox } from './LoginLayout';
-
 import config from 'app/core/config';
 
 const forgottenPasswordStyles = css`

public/app/core/components/Login/UserSignup.tsx

 import React, { FC } from 'react';
 import { LinkButton, VerticalGroup } from '@grafana/ui';
 import { css } from 'emotion';
+import { getConfig } from 'app/core/config';
 
 export const UserSignup: FC<{}> = () => {
+  const href = getConfig().verifyEmailEnabled ? `${getConfig().appSubUrl}/verify` : `${getConfig().appSubUrl}/signup`;
   return (
     <VerticalGroup
       className={css`
@@ -15,7 +17,7 @@ export const UserSignup: FC<{}> = () => {
           width: 100%;
           justify-content: center;
         `}
-        href="signup"
+        href={href}
         variant="secondary"
       >
         Sign Up

public/app/features/profile/SignupForm.tsx → public/app/core/components/Signup/Signup.tsx

 import React, { FC } from 'react';
-import { Button, LinkButton, Input, Form, Field } from '@grafana/ui';
-import { css } from 'emotion';
-
+import { connect, MapStateToProps } from 'react-redux';
+import { StoreState } from 'app/types';
+import { Form, Field, Input, Button, HorizontalGroup, LinkButton } from '@grafana/ui';
 import { getConfig } from 'app/core/config';
 import { getBackendSrv } from '@grafana/runtime';
+import appEvents from 'app/core/app_events';
+import { AppEvents } from '@grafana/data';
 
-interface SignupFormModel {
+interface SignupDTO {
+  name: string;
   email: string;
-  username?: string;
+  username: string;
+  orgName?: string;
   password: string;
-  orgName: string;
-  code?: string;
-  name?: string;
+  code: string;
+  confirm: string;
 }
-interface Props {
+
+interface ConnectedProps {
   email?: string;
-  orgName?: string;
-  username?: string;
   code?: string;
-  name?: string;
-  verifyEmailEnabled?: boolean;
-  autoAssignOrg?: boolean;
 }
 
-const buttonSpacing = css`
-  margin-left: 15px;
-`;
-
-export const SignupForm: FC<Props> = props => {
-  const verifyEmailEnabled = props.verifyEmailEnabled;
-  const autoAssignOrg = props.autoAssignOrg;
-
-  const onSubmit = async (formData: SignupFormModel) => {
+const SignupUnconnected: FC<ConnectedProps> = props => {
+  const onSubmit = async (formData: SignupDTO) => {
     if (formData.name === '') {
       delete formData.name;
     }
+    delete formData.confirm;
 
-    const response = await getBackendSrv().post('/api/user/signup/step2', {
-      email: formData.email,
-      code: formData.code,
-      username: formData.email,
-      orgName: formData.orgName,
-      password: formData.password,
-      name: formData.name,
-    });
+    const response = await getBackendSrv()
+      .post('/api/user/signup/step2', {
+        email: formData.email,
+        code: formData.code,
+        username: formData.email,
+        orgName: formData.orgName,
+        password: formData.password,
+        name: formData.name,
+      })
+      .catch(err => {
+        const msg = err.data?.message || err;
+        appEvents.emit(AppEvents.alertWarning, [msg]);
+      });
 
     if (response.code === 'redirect-to-select-org') {
       window.location.href = getConfig().appSubUrl + '/profile/select-org?signup=1';
@@ -52,63 +50,77 @@ export const SignupForm: FC<Props> = props => {
   };
 
   const defaultValues = {
-    orgName: props.orgName,
     email: props.email,
-    username: props.email,
     code: props.code,
-    name: props.name,
   };
 
   return (
     <Form defaultValues={defaultValues} onSubmit={onSubmit}>
-      {({ register, errors }) => {
-        return (
-          <>
-            {verifyEmailEnabled && (
-              <Field label="Email verification code (sent to your email)">
-                <Input name="code" ref={register} placeholder="Code" />
-              </Field>
-            )}
-            {!autoAssignOrg && (
-              <Field label="Org. name">
-                <Input name="orgName" placeholder="Org. name" ref={register} />
-              </Field>
-            )}
-            <Field label="Your name">
-              <Input name="name" placeholder="(optional)" ref={register} />
-            </Field>
-            <Field label="Email" invalid={!!errors.email} error={errors.email?.message}>
-              <Input
-                name="email"
-                type="email"
-                placeholder="Email"
-                ref={register({
-                  required: 'Email is required',
-                  pattern: {
-                    value: /^\S+@\S+$/,
-                    message: 'Email is invalid',
-                  },
-                })}
-              />
+      {({ errors, register, getValues }) => (
+        <>
+          <Field label="Your name">
+            <Input name="name" placeholder="(optional)" ref={register} />
+          </Field>
+          <Field label="Email" invalid={!!errors.email} error={errors.email?.message}>
+            <Input
+              name="email"
+              type="email"
+              placeholder="Email"
+              ref={register({
+                required: 'Email is required',
+                pattern: {
+                  value: /^\S+@\S+$/,
+                  message: 'Email is invalid',
+                },
+              })}
+            />
+          </Field>
+          {!getConfig().autoAssignOrg && (
+            <Field label="Org. name">
+              <Input name="orgName" placeholder="Org. name" ref={register} />
             </Field>
-            <Field label="Password" invalid={!!errors.password} error={errors.password?.message}>
-              <Input
-                name="password"
-                type="password"
-                placeholder="Password"
-                ref={register({ required: 'Password is required' })}
-              />
+          )}
+          {getConfig().verifyEmailEnabled && (
+            <Field label="Email verification code (sent to your email)">
+              <Input name="code" ref={register} placeholder="Code" />
             </Field>
+          )}
+          <Field label="Password" invalid={!!errors.password} error={errors?.password?.message}>
+            <Input
+              autoFocus
+              type="password"
+              name="password"
+              ref={register({
+                required: 'Password is required',
+              })}
+            />
+          </Field>
+          <Field label="Confirm password" invalid={!!errors.confirm} error={errors?.confirm?.message}>
+            <Input
+              type="password"
+              name="confirm"
+              ref={register({
+                required: 'Confirmed password is required',
+                validate: v => v === getValues().password || 'Passwords must match!',
+              })}
+            />
+          </Field>
 
+          <HorizontalGroup>
             <Button type="submit">Submit</Button>
-            <span className={buttonSpacing}>
-              <LinkButton href={getConfig().appSubUrl + '/login'} variant="secondary">
-                Back
-              </LinkButton>
-            </span>
-          </>
-        );
-      }}
+            <LinkButton variant="link" href={getConfig().appSubUrl + '/login'}>
+              Back to login
+            </LinkButton>
+          </HorizontalGroup>
+        </>
+      )}
     </Form>
   );
 };
+
+const mapStateToProps: MapStateToProps<ConnectedProps, {}, StoreState> = (state: StoreState) => ({
+  email: state.location.routeParams.email?.toString(),
+  code: state.location.routeParams.code?.toString(),
+});
+
+export const Signup = connect(mapStateToProps)(SignupUnconnected);

public/app/core/components/Signup/SignupPage.tsx

+import React, { FC } from 'react';
+import { LoginLayout, InnerBox } from '../Login/LoginLayout';
+import { Signup } from './Signup';
+
+export const SignupPage: FC = () => {
+  return (
+    <LoginLayout>
+      <InnerBox>
+        <Signup />
+      </InnerBox>
+    </LoginLayout>
+  );
+};
+
+export default SignupPage;

public/app/core/components/Signup/VerifyEmail.tsx

+import React, { FC, useState } from 'react';
+import { Form, Field, Input, Button, Legend, Container, HorizontalGroup, LinkButton } from '@grafana/ui';
+import { getConfig } from 'app/core/config';
+import { getBackendSrv } from '@grafana/runtime';
+import appEvents from 'app/core/app_events';
+import { AppEvents } from '@grafana/data';
+
+interface EmailDTO {
+  email: string;
+}
+
+export const VerifyEmail: FC = () => {
+  const [emailSent, setEmailSent] = useState(false);
+
+  const onSubmit = (formModel: EmailDTO) => {
+    getBackendSrv()
+      .post('/api/user/signup', formModel)
+      .then(() => {
+        setEmailSent(true);
+      })
+      .catch(err => {
+        const msg = err.data?.message || err;
+        appEvents.emit(AppEvents.alertWarning, [msg]);
+      });
+  };
+
+  if (emailSent) {
+    return (
+      <div>
+        <p>An email with a verification link has been sent to the email address. You should receive it shortly.</p>
+        <Container margin="md" />
+        <LinkButton variant="primary" href={getConfig().appSubUrl + '/signup'}>
+          Complete Signup
+        </LinkButton>
+      </div>
+    );
+  }
+
+  return (
+    <Form onSubmit={onSubmit}>
+      {({ register, errors }) => (
+        <>
+          <Legend>Verify Email</Legend>
+          <Field
+            label="Email"
+            description="Enter your email address to get a verification link sent to you"
+            invalid={!!(errors as any).email}
+            error={(errors as any).email?.message}
+          >
+            <Input placeholder="Email" name="email" ref={register({ required: true })} />
+          </Field>
+          <HorizontalGroup>
+            <Button>Send verification email</Button>
+            <LinkButton variant="link" href={getConfig().appSubUrl + '/login'}>
+              Back to login
+            </LinkButton>
+          </HorizontalGroup>
+        </>
+      )}
+    </Form>
+  );
+};

public/app/core/components/Signup/VerifyEmailPage.tsx

+import React, { FC } from 'react';
+
+import { LoginLayout, InnerBox } from '../Login/LoginLayout';
+import { VerifyEmail } from './VerifyEmail';
+import { getConfig } from 'app/core/config';
+
+export const VerifyEmailPage: FC = () => {
+  if (!getConfig().verifyEmailEnabled) {
+    window.location.href = getConfig().appSubUrl + '/signup';
+    return <></>;
+  }
+
+  return (
+    <LoginLayout>
+      <InnerBox>
+        <VerifyEmail />
+      </InnerBox>
+    </LoginLayout>
+  );
+};
+
+export default VerifyEmailPage;

public/app/features/profile/SignupForm.test.tsx

-import React from 'react';
-import { shallow } from 'enzyme';
-import { SignupForm } from './SignupForm';
-
-describe('SignupForm', () => {
-  describe('With different values for verifyEmail and autoAssignOrg', () => {
-    it('should render input fields', () => {
-      const wrapper = shallow(<SignupForm verifyEmailEnabled={true} autoAssignOrg={false} />);
-      expect(wrapper.exists('Forms.Input[name="orgName"]'));
-      expect(wrapper.exists('Forms.Input[name="code"]'));
-    });
-    it('should not render input fields', () => {
-      const wrapper = shallow(<SignupForm verifyEmailEnabled={false} autoAssignOrg={true} />);
-      expect(wrapper.exists('Forms.Input[name="orgName"]')).toBeFalsy();
-      expect(wrapper.exists('Forms.Input[name="code"]')).toBeFalsy();
-    });
-  });
-});

public/app/features/profile/SignupPage.tsx

-import React, { FC } from 'react';
-import { SignupForm } from './SignupForm';
-import Page from 'app/core/components/Page/Page';
-import { getConfig } from 'app/core/config';
-import { connect } from 'react-redux';
-import { hot } from 'react-hot-loader';
-import { StoreState } from 'app/types';
-
-const navModel = {
-  main: {
-    icon: 'grafana',
-    text: 'Sign Up',
-    subTitle: 'Register your Grafana account',
-    breadcrumbs: [{ title: 'Login', url: 'login' }],
-  },
-  node: {
-    text: '',
-  },
-};
-
-interface Props {
-  email?: string;
-  orgName?: string;
-  username?: string;
-  code?: string;
-  name?: string;
-}
-export const SignupPage: FC<Props> = props => {
-  return (
-    <Page navModel={navModel}>
-      <Page.Contents>
-        <h3 className="p-b-1">You're almost there.</h3>
-        <div className="p-b-1">
-          We just need a couple of more bits of
-          <br /> information to finish creating your account.
-        </div>
-        <SignupForm
-          {...props}
-          verifyEmailEnabled={getConfig().verifyEmailEnabled}
-          autoAssignOrg={getConfig().autoAssignOrg}
-        />
-      </Page.Contents>
-    </Page>
-  );
-};
-
-const mapStateToProps = (state: StoreState) => ({
-  ...state.location.routeParams,
-});
-
-export default hot(module)(connect(mapStateToProps)(SignupPage));

public/app/routes/routes.ts

@@ -4,7 +4,6 @@ import { applyRouteRegistrationHandlers } from './registry';
 // Pages
 import LdapPage from 'app/features/admin/ldap/LdapPage';
 import UserAdminPage from 'app/features/admin/UserAdminPage';
-import SignupPage from 'app/features/profile/SignupPage';
 import { LoginPage } from 'app/core/components/Login/LoginPage';
 
 import config from 'app/core/config';
@@ -436,13 +435,25 @@ export function setupAngularRoutes($routeProvider: route.IRouteProvider, $locati
           SafeDynamicImport(import(/* webpackChunkName: "SignupInvited" */ 'app/features/users/SignupInvited')),
       },
     })
+    .when('/verify', {
+      template: '<react-container />',
+      resolve: {
+        component: () =>
+          SafeDynamicImport(
+            import(/* webpackChunkName: "VerifyEmailPage" */ 'app/core/components/Signup/VerifyEmailPage')
+          ),
+      },
+      // @ts-ignore
+      pageClass: 'login-page sidemenu-hidden',
+    })
     .when('/signup', {
-      template: '<react-container/>',
-      //@ts-ignore
-      pageClass: 'sidemenu-hidden',
+      template: '<react-container />',
       resolve: {
-        component: () => SignupPage,
+        component: () =>
+          SafeDynamicImport(import(/* webpackChunkName: "SignupPage" */ 'app/core/components/Signup/SignupPage')),
       },
+      // @ts-ignore
+      pageClass: 'login-page sidemenu-hidden',
     })
     .when('/user/password/send-reset-email', {
       template: '<react-container />',