From 8a8bfc5176dcf2fef2a46af94bf87f552f02302e Mon Sep 17 00:00:00 2001 From: mabashian Date: Wed, 18 Nov 2020 09:25:24 -0500 Subject: [PATCH 1/2] Convert Login.jsx to functional component in preparation for social auth integration --- awx/ui_next/src/App.test.jsx | 9 +- awx/ui_next/src/screens/Login/Login.jsx | 251 +++++++-------- awx/ui_next/src/screens/Login/Login.test.jsx | 319 +++++++++---------- 3 files changed, 271 insertions(+), 308 deletions(-) diff --git a/awx/ui_next/src/App.test.jsx b/awx/ui_next/src/App.test.jsx index c2d667df3b..a6fe414b39 100644 --- a/awx/ui_next/src/App.test.jsx +++ b/awx/ui_next/src/App.test.jsx @@ -1,5 +1,5 @@ import React from 'react'; - +import { act } from 'react-dom/test-utils'; import { mountWithContexts } from '../testUtils/enzymeHelpers'; import App from './App'; @@ -7,8 +7,11 @@ import App from './App'; jest.mock('./api'); describe('', () => { - test('renders ok', () => { - const wrapper = mountWithContexts(); + test('renders ok', async () => { + let wrapper; + await act(async () => { + wrapper = mountWithContexts(); + }); expect(wrapper.length).toBe(1); }); }); diff --git a/awx/ui_next/src/screens/Login/Login.jsx b/awx/ui_next/src/screens/Login/Login.jsx index f08c8e68a7..d4c56c26df 100644 --- a/awx/ui_next/src/screens/Login/Login.jsx +++ b/awx/ui_next/src/screens/Login/Login.jsx @@ -1,12 +1,17 @@ -import React, { Component } from 'react'; +import React, { useCallback, useEffect } from 'react'; import { Redirect, withRouter } from 'react-router-dom'; import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; +import { Formik } from 'formik'; import styled from 'styled-components'; import { LoginForm, LoginPage as PFLoginPage } from '@patternfly/react-core'; +import useRequest, { useDismissableError } from '../../util/useRequest'; import { RootAPI } from '../../api'; +import { BrandName } from '../../variables'; +import AlertModal from '../../components/AlertModal'; +import ErrorDetail from '../../components/ErrorDetail'; -const loginLogoSrc = '/static/media/logo-login.svg'; +import brandLogo from './brand-logo.svg'; const LoginPage = styled(PFLoginPage)` & .pf-c-brand { @@ -14,149 +19,131 @@ const LoginPage = styled(PFLoginPage)` } `; -class AWXLogin extends Component { - constructor(props) { - super(props); - - this.state = { - username: '', - password: '', - hasAuthError: false, - hasValidationError: false, - isAuthenticating: false, - isLoading: true, - logo: null, - loginInfo: null, - brandName: null, - }; - - this.handleChangeUsername = this.handleChangeUsername.bind(this); - this.handleChangePassword = this.handleChangePassword.bind(this); - this.handleLoginButtonClick = this.handleLoginButtonClick.bind(this); - this.loadCustomLoginInfo = this.loadCustomLoginInfo.bind(this); - } - - async componentDidMount() { - await this.loadCustomLoginInfo(); - } - - async loadCustomLoginInfo() { - this.setState({ isLoading: true }); - try { - const [ - { - data: { custom_logo, custom_login_info }, - }, - { - data: { BRAND_NAME }, - }, - ] = await Promise.all([RootAPI.read(), RootAPI.readAssetVariables()]); - const logo = custom_logo +function AWXLogin({ alt, i18n, isAuthenticated }) { + const { + isLoading: isCustomLoginInfoLoading, + error: customLoginInfoError, + request: fetchCustomLoginInfo, + result: { logo, loginInfo }, + } = useRequest( + useCallback(async () => { + const { + data: { custom_logo, custom_login_info }, + } = await RootAPI.read(); + const logoSrc = custom_logo ? `data:image/jpeg;${custom_logo}` - : loginLogoSrc; - this.setState({ - brandName: BRAND_NAME, - logo, + : brandLogo; + return { + logo: logoSrc, loginInfo: custom_login_info, - }); - } catch (err) { - this.setState({ brandName: 'AWX', logo: loginLogoSrc }); - } finally { - this.setState({ isLoading: false }); - } - } + }; + }, []), + { logo: brandLogo, loginInfo: null } + ); - async handleLoginButtonClick(event) { - const { username, password, isAuthenticating } = this.state; + const { + error: loginInfoError, + dismissError: dismissLoginInfoError, + } = useDismissableError(customLoginInfoError); - event.preventDefault(); + useEffect(() => { + fetchCustomLoginInfo(); + }, [fetchCustomLoginInfo]); - if (isAuthenticating) { - return; - } - - this.setState({ hasAuthError: false, isAuthenticating: true }); - try { - // note: if authentication is successful, the appropriate cookie will be set automatically - // and isAuthenticated() (the source of truth) will start returning true. + const { + isLoading: isAuthenticating, + error: authenticationError, + request: authenticate, + } = useRequest( + useCallback(async ({ username, password }) => { await RootAPI.login(username, password); - } catch (err) { - if (err && err.response && err.response.status === 401) { - this.setState({ hasValidationError: true }); - } else { - this.setState({ hasAuthError: true }); - } - } finally { - this.setState({ isAuthenticating: false }); - } + }, []) + ); + + const { + error: authError, + dismissError: dismissAuthError, + } = useDismissableError(authenticationError); + + const handleSubmit = async values => { + dismissAuthError(); + await authenticate(values); + }; + + const brandName = BrandName; + + if (isCustomLoginInfoLoading) { + return null; } - handleChangeUsername(value) { - this.setState({ username: value, hasValidationError: false }); + if (isAuthenticated(document.cookie)) { + return ; } - handleChangePassword(value) { - this.setState({ password: value, hasValidationError: false }); + let helperText; + if (authError?.response?.status === 401) { + helperText = i18n._(t`Invalid username or password. Please try again.`); + } else { + helperText = i18n._(t`There was a problem signing in. Please try again.`); } - render() { - const { - brandName, - hasAuthError, - hasValidationError, - username, - password, - isLoading, - logo, - loginInfo, - } = this.state; - const { alt, i18n, isAuthenticated } = this.props; - - if (isLoading) { - return null; - } - - if (isAuthenticated(document.cookie)) { - return ; - } - - let helperText; - if (hasValidationError) { - helperText = i18n._(t`Invalid username or password. Please try again.`); - } else { - helperText = i18n._(t`There was a problem signing in. Please try again.`); - } - - return ( - + - - - ); - } + {formik => ( + <> + { + formik.setFieldValue('password', val); + dismissAuthError(); + }} + onChangeUsername={val => { + formik.setFieldValue('username', val); + dismissAuthError(); + }} + onLoginButtonClick={formik.handleSubmit} + passwordLabel={i18n._(t`Password`)} + passwordValue={formik.values.password} + showHelperText={authError} + usernameLabel={i18n._(t`Username`)} + usernameValue={formik.values.username} + /> + + )} + + {loginInfoError && ( + + {i18n._( + t`Failed to fetch custom login configuration settings. System defaults will be shown instead.` + )} + + + )} + + ); } -export { AWXLogin as _AWXLogin }; export default withI18n()(withRouter(AWXLogin)); +export { AWXLogin as _AWXLogin }; diff --git a/awx/ui_next/src/screens/Login/Login.test.jsx b/awx/ui_next/src/screens/Login/Login.test.jsx index 62948ffc1d..014c015da8 100644 --- a/awx/ui_next/src/screens/Login/Login.test.jsx +++ b/awx/ui_next/src/screens/Login/Login.test.jsx @@ -1,5 +1,5 @@ import React from 'react'; - +import { act } from 'react-dom/test-utils'; import { RootAPI } from '../../api'; import { mountWithContexts, @@ -55,11 +55,6 @@ describe('', () => { custom_logo: 'images/foo.jpg', }, }); - RootAPI.readAssetVariables.mockResolvedValue({ - data: { - BRAND_NAME: 'AWX', - }, - }); }); afterEach(() => { @@ -67,27 +62,28 @@ describe('', () => { }); test('initially renders without crashing', async done => { - const loginWrapper = mountWithContexts( - false} /> + let wrapper; + await act(async () => { + wrapper = mountWithContexts( false} />); + }); + const { usernameInput, passwordInput, submitButton } = await findChildren( + wrapper ); - const { - awxLogin, - usernameInput, - passwordInput, - submitButton, - } = await findChildren(loginWrapper); expect(usernameInput.props().value).toBe(''); expect(passwordInput.props().value).toBe(''); - expect(awxLogin.state('hasValidationError')).toBe(false); expect(submitButton.props().isDisabled).toBe(false); + expect(wrapper.find('AlertModal').length).toBe(0); done(); }); test('custom logo renders Brand component with correct src and alt', async done => { - const loginWrapper = mountWithContexts( - false} /> - ); - const { loginHeaderLogo } = await findChildren(loginWrapper); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( + false} /> + ); + }); + const { loginHeaderLogo } = await findChildren(wrapper); const { alt, src } = loginHeaderLogo.props(); expect([alt, src]).toEqual([ 'Foo Application', @@ -98,195 +94,172 @@ describe('', () => { test('default logo renders Brand component with correct src and alt', async done => { RootAPI.read.mockResolvedValue({ data: {} }); - const loginWrapper = mountWithContexts( - false} /> - ); - const { loginHeaderLogo } = await findChildren(loginWrapper); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( false} />); + }); + const { loginHeaderLogo } = await findChildren(wrapper); const { alt, src } = loginHeaderLogo.props(); - expect(alt).toEqual('AWX'); - expect(src).toContain('logo-login.svg'); + expect([alt, src]).toEqual(['AWX', 'brand-logo.svg']); done(); }); - test('default logo renders on data initialization error', async done => { - RootAPI.read.mockRejectedValueOnce({ response: { status: 500 } }); - const loginWrapper = mountWithContexts( - false} /> + test('data initialization error is properly handled', async done => { + RootAPI.read.mockRejectedValueOnce( + new Error({ + response: { + config: { + method: 'get', + url: '/api/v2', + }, + data: 'An error occurred', + status: 500, + }, + }) ); - const { loginHeaderLogo } = await findChildren(loginWrapper); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( false} />); + }); + const { loginHeaderLogo } = await findChildren(wrapper); const { alt, src } = loginHeaderLogo.props(); - expect(alt).toEqual('AWX'); - expect(src).toContain('logo-login.svg'); + expect([alt, src]).toEqual(['AWX', 'brand-logo.svg']); + expect(wrapper.find('AlertModal').length).toBe(1); done(); }); test('state maps to un/pw input value props', async done => { - const loginWrapper = mountWithContexts( - false} /> - ); - const { usernameInput, passwordInput } = await findChildren(loginWrapper); - usernameInput.props().onChange({ currentTarget: { value: 'un' } }); - passwordInput.props().onChange({ currentTarget: { value: 'pw' } }); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('username') === 'un' - ); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('password') === 'pw' - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( false} />); + }); + await waitForElement(wrapper, 'LoginForm', el => el.length === 1); + await act(async () => { + wrapper.find('TextInputBase#pf-login-username-id').prop('onChange')('un'); + wrapper.find('TextInputBase#pf-login-password-id').prop('onChange')('pw'); + }); + wrapper.update(); + expect( + wrapper.find('TextInputBase#pf-login-username-id').prop('value') + ).toEqual('un'); + expect( + wrapper.find('TextInputBase#pf-login-password-id').prop('value') + ).toEqual('pw'); done(); }); test('handles input validation errors and clears on input value change', async done => { - const formError = '.pf-c-form__helper-text.pf-m-error'; - const loginWrapper = mountWithContexts( - false} /> - ); - const { usernameInput, passwordInput, submitButton } = await findChildren( - loginWrapper + RootAPI.login.mockRejectedValueOnce( + new Error({ + response: { + config: { + method: 'post', + url: '/api/login/', + }, + data: 'An error occurred', + status: 401, + }, + }) ); - RootAPI.login.mockRejectedValueOnce({ response: { status: 401 } }); - usernameInput.props().onChange({ currentTarget: { value: 'invalid' } }); - passwordInput.props().onChange({ currentTarget: { value: 'invalid' } }); - submitButton.simulate('click'); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('username') === 'invalid' - ); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('password') === 'invalid' - ); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('hasValidationError') === true - ); - await waitForElement(loginWrapper, formError, el => el.length === 1); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( false} />); + }); + await waitForElement(wrapper, 'LoginForm', el => el.length === 1); - usernameInput.props().onChange({ currentTarget: { value: 'dsarif' } }); - passwordInput.props().onChange({ currentTarget: { value: 'freneticpny' } }); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('username') === 'dsarif' - ); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('password') === 'freneticpny' - ); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('hasValidationError') === false - ); - await waitForElement(loginWrapper, formError, el => el.length === 0); + expect( + wrapper.find('TextInputBase#pf-login-username-id').prop('value') + ).toEqual(''); + expect( + wrapper.find('TextInputBase#pf-login-password-id').prop('value') + ).toEqual(''); + expect(wrapper.find('FormHelperText').prop('isHidden')).toEqual(true); - done(); - }); + await act(async () => { + wrapper.find('TextInputBase#pf-login-username-id').prop('onChange')('un'); + wrapper.find('TextInputBase#pf-login-password-id').prop('onChange')('pw'); + }); + wrapper.update(); - test('handles other errors and clears on resubmit', async done => { - const loginWrapper = mountWithContexts( - false} /> - ); - const { usernameInput, passwordInput, submitButton } = await findChildren( - loginWrapper - ); + expect( + wrapper.find('TextInputBase#pf-login-username-id').prop('value') + ).toEqual('un'); + expect( + wrapper.find('TextInputBase#pf-login-password-id').prop('value') + ).toEqual('pw'); - RootAPI.login.mockRejectedValueOnce({ response: { status: 500 } }); - submitButton.simulate('click'); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('hasAuthError') === true - ); + await act(async () => { + wrapper.find('Button[type="submit"]').invoke('onClick')(); + }); + wrapper.update(); - usernameInput.props().onChange({ currentTarget: { value: 'sgrimes' } }); - passwordInput.props().onChange({ currentTarget: { value: 'ovid' } }); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('username') === 'sgrimes' - ); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('password') === 'ovid' - ); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('hasAuthError') === true - ); + expect(wrapper.find('FormHelperText').prop('isHidden')).toEqual(false); + expect( + wrapper.find('TextInput#pf-login-username-id').prop('validated') + ).toEqual('error'); + expect( + wrapper.find('TextInput#pf-login-password-id').prop('validated') + ).toEqual('error'); - submitButton.simulate('click'); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('hasAuthError') === false - ); - done(); - }); + await act(async () => { + wrapper.find('TextInputBase#pf-login-username-id').prop('onChange')( + 'foo' + ); + wrapper.find('TextInputBase#pf-login-password-id').prop('onChange')( + 'bar' + ); + }); + wrapper.update(); - test('no login requests are made when already authenticating', async done => { - const loginWrapper = mountWithContexts( - false} /> - ); - const { awxLogin, submitButton } = await findChildren(loginWrapper); - - awxLogin.setState({ isAuthenticating: true }); - submitButton.simulate('click'); - submitButton.simulate('click'); - expect(RootAPI.login).toHaveBeenCalledTimes(0); - - awxLogin.setState({ isAuthenticating: false }); - submitButton.simulate('click'); - submitButton.simulate('click'); - expect(RootAPI.login).toHaveBeenCalledTimes(1); + expect( + wrapper.find('TextInputBase#pf-login-username-id').prop('value') + ).toEqual('foo'); + expect( + wrapper.find('TextInputBase#pf-login-password-id').prop('value') + ).toEqual('bar'); + expect(wrapper.find('FormHelperText').prop('isHidden')).toEqual(true); + expect( + wrapper.find('TextInput#pf-login-username-id').prop('validated') + ).toEqual('default'); + expect( + wrapper.find('TextInput#pf-login-password-id').prop('validated') + ).toEqual('default'); done(); }); test('submit calls api.login successfully', async done => { - const loginWrapper = mountWithContexts( - false} /> - ); - const { usernameInput, passwordInput, submitButton } = await findChildren( - loginWrapper - ); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( false} />); + }); + await waitForElement(wrapper, 'LoginForm', el => el.length === 1); + + await act(async () => { + wrapper.find('TextInputBase#pf-login-username-id').prop('onChange')('un'); + wrapper.find('TextInputBase#pf-login-password-id').prop('onChange')('pw'); + }); + wrapper.update(); + + await act(async () => { + wrapper.find('Button[type="submit"]').invoke('onClick')(); + }); + wrapper.update(); - usernameInput.props().onChange({ currentTarget: { value: 'gthorpe' } }); - passwordInput.props().onChange({ currentTarget: { value: 'hydro' } }); - submitButton.simulate('click'); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('isAuthenticating') === true - ); - await waitForElement( - loginWrapper, - 'AWXLogin', - el => el.state('isAuthenticating') === false - ); expect(RootAPI.login).toHaveBeenCalledTimes(1); - expect(RootAPI.login).toHaveBeenCalledWith('gthorpe', 'hydro'); + expect(RootAPI.login).toHaveBeenCalledWith('un', 'pw'); done(); }); test('render Redirect to / when already authenticated', async done => { - const loginWrapper = mountWithContexts( - true} /> - ); - await waitForElement(loginWrapper, 'Redirect', el => el.length === 1); - await waitForElement(loginWrapper, 'Redirect', el => el.props().to === '/'); + let wrapper; + await act(async () => { + wrapper = mountWithContexts( true} />); + }); + await waitForElement(wrapper, 'Redirect', el => el.length === 1); + await waitForElement(wrapper, 'Redirect', el => el.props().to === '/'); done(); }); }); From 7ff82db691ae191e3d7938f85ae861bc29a6be19 Mon Sep 17 00:00:00 2001 From: mabashian Date: Mon, 30 Nov 2020 10:38:41 -0500 Subject: [PATCH 2/2] Fix login after source variables change --- awx/ui_next/src/screens/Login/Login.jsx | 29 ++++++++++++-------- awx/ui_next/src/screens/Login/Login.test.jsx | 10 +++++-- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/awx/ui_next/src/screens/Login/Login.jsx b/awx/ui_next/src/screens/Login/Login.jsx index d4c56c26df..ca358a0d74 100644 --- a/awx/ui_next/src/screens/Login/Login.jsx +++ b/awx/ui_next/src/screens/Login/Login.jsx @@ -7,11 +7,10 @@ import styled from 'styled-components'; import { LoginForm, LoginPage as PFLoginPage } from '@patternfly/react-core'; import useRequest, { useDismissableError } from '../../util/useRequest'; import { RootAPI } from '../../api'; -import { BrandName } from '../../variables'; import AlertModal from '../../components/AlertModal'; import ErrorDetail from '../../components/ErrorDetail'; -import brandLogo from './brand-logo.svg'; +const loginLogoSrc = '/static/media/logo-login.svg'; const LoginPage = styled(PFLoginPage)` & .pf-c-brand { @@ -24,21 +23,27 @@ function AWXLogin({ alt, i18n, isAuthenticated }) { isLoading: isCustomLoginInfoLoading, error: customLoginInfoError, request: fetchCustomLoginInfo, - result: { logo, loginInfo }, + result: { brandName, logo, loginInfo }, } = useRequest( useCallback(async () => { - const { - data: { custom_logo, custom_login_info }, - } = await RootAPI.read(); + const [ + { + data: { custom_logo, custom_login_info }, + }, + { + data: { BRAND_NAME }, + }, + ] = await Promise.all([RootAPI.read(), RootAPI.readAssetVariables()]); const logoSrc = custom_logo ? `data:image/jpeg;${custom_logo}` - : brandLogo; + : loginLogoSrc; return { + brandName: BRAND_NAME, logo: logoSrc, loginInfo: custom_login_info, }; }, []), - { logo: brandLogo, loginInfo: null } + { brandName: null, logo: loginLogoSrc, loginInfo: null } ); const { @@ -70,8 +75,6 @@ function AWXLogin({ alt, i18n, isAuthenticated }) { await authenticate(values); }; - const brandName = BrandName; - if (isCustomLoginInfoLoading) { return null; } @@ -91,7 +94,11 @@ function AWXLogin({ alt, i18n, isAuthenticated }) { ', () => { async function findChildren(wrapper) { const [ @@ -100,7 +106,7 @@ describe('', () => { }); const { loginHeaderLogo } = await findChildren(wrapper); const { alt, src } = loginHeaderLogo.props(); - expect([alt, src]).toEqual(['AWX', 'brand-logo.svg']); + expect([alt, src]).toEqual(['AWX', '/static/media/logo-login.svg']); done(); }); @@ -123,7 +129,7 @@ describe('', () => { }); const { loginHeaderLogo } = await findChildren(wrapper); const { alt, src } = loginHeaderLogo.props(); - expect([alt, src]).toEqual(['AWX', 'brand-logo.svg']); + expect([alt, src]).toEqual([null, '/static/media/logo-login.svg']); expect(wrapper.find('AlertModal').length).toBe(1); done(); });