From 5cb3146e853d4c3c9cf87789d4e6be875c2fae0e Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Mon, 6 Mar 2017 13:20:54 +0200 Subject: [PATCH 1/4] Remove default messages from translations --- src/components/HeroSection/HeroSection.js | 7 ++----- src/containers/HeroSearchForm/HeroSearchForm.js | 7 ++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/components/HeroSection/HeroSection.js b/src/components/HeroSection/HeroSection.js index e830d48a..e2170a78 100644 --- a/src/components/HeroSection/HeroSection.js +++ b/src/components/HeroSection/HeroSection.js @@ -7,13 +7,10 @@ const HeroSection = props => (
- +
- +
diff --git a/src/containers/HeroSearchForm/HeroSearchForm.js b/src/containers/HeroSearchForm/HeroSearchForm.js index 4e960656..282099a7 100644 --- a/src/containers/HeroSearchForm/HeroSearchForm.js +++ b/src/containers/HeroSearchForm/HeroSearchForm.js @@ -6,10 +6,7 @@ import css from './HeroSearchForm.css'; const HeroSearchForm = props => { const { className, intl, handleSubmit, pristine, submitting } = props; const addClassName = className ? { className } : {}; - const placeholderMsg = { - id: 'HeroSearchForm.placeholder', - defaultMessage: 'Location search (soon)', - }; + const placeholderMsg = { id: 'HeroSearchForm.placeholder' }; return (
@@ -21,7 +18,7 @@ const HeroSearchForm = props => { placeholder={intl.formatMessage(placeholderMsg)} />
); From 305ccc3b82f9ed40d23353be448b21f8e3da8689 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Mon, 6 Mar 2017 13:26:32 +0200 Subject: [PATCH 2/4] Add proper i18n setup for the tests --- src/util/test-helpers.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/util/test-helpers.js b/src/util/test-helpers.js index ef40478e..615cc840 100644 --- a/src/util/test-helpers.js +++ b/src/util/test-helpers.js @@ -2,17 +2,20 @@ import React from 'react'; import renderer from 'react-test-renderer'; import { shallow } from 'enzyme'; import toJson from 'enzyme-to-json'; -import { IntlProvider } from 'react-intl'; +import { IntlProvider, addLocaleData } from 'react-intl'; +import en from 'react-intl/locale-data/en'; import { BrowserRouter } from 'react-router-dom'; import { Provider } from 'react-redux'; import configureStore from '../store'; +import localeData from '../translations/en.json'; // Provide all the context for components that connect to the Redux // store, i18n, router, etc. export const TestProvider = props => { const store = configureStore(); + addLocaleData([...en]); return ( - + {props.children} From 072a852c86b80cfab54aa25ee27947694a8d5770 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Tue, 7 Mar 2017 10:27:58 +0200 Subject: [PATCH 3/4] Separate different auth errors --- src/ducks/Auth.duck.js | 20 +++++++++++++------- src/ducks/Auth.test.js | 18 ++++++++++-------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/ducks/Auth.duck.js b/src/ducks/Auth.duck.js index e429b8bd..85c841be 100644 --- a/src/ducks/Auth.duck.js +++ b/src/ducks/Auth.duck.js @@ -22,31 +22,37 @@ export const LOGOUT_ERROR = 'app/Auth/LOGOUT_ERROR'; // ================ Reducer ================ // -const initialState = { authInfoLoaded: false, isAuthenticated: false, error: null }; +const initialState = { + authInfoLoaded: false, + isAuthenticated: false, + authInfoError: null, + loginError: null, + logoutError: null, +}; export default function reducer(state = initialState, action = {}) { const { type, payload } = action; switch (type) { case AUTH_INFO_REQUEST: - return { ...state, error: null }; + return { ...state, authInfoError: null }; case AUTH_INFO_SUCCESS: return { ...state, authInfoLoaded: true, isAuthenticated: authenticated(payload) }; case AUTH_INFO_ERROR: - return { ...state, error: payload }; + return { ...state, authInfoError: payload }; case LOGIN_REQUEST: - return { ...state, error: null }; + return { ...state, loginError: null, logoutError: null }; case LOGIN_SUCCESS: return { ...state, isAuthenticated: true }; case LOGIN_ERROR: - return { ...state, error: payload }; + return { ...state, loginError: payload }; case LOGOUT_REQUEST: - return { ...state, error: null }; + return { ...state, loginError: null, logoutError: null }; case LOGOUT_SUCCESS: return { ...state, isAuthenticated: false }; case LOGOUT_ERROR: - return { ...state, error: payload }; + return { ...state, logoutError: payload }; default: return state; } diff --git a/src/ducks/Auth.test.js b/src/ducks/Auth.test.js index 0fd66c51..942e3cae 100644 --- a/src/ducks/Auth.test.js +++ b/src/ducks/Auth.test.js @@ -26,7 +26,9 @@ describe('Auth duck', () => { const state = reducer(); expect(state.authInfoLoaded).toEqual(false); expect(state.isAuthenticated).toEqual(false); - expect(state.error).toBeNull(); + expect(state.authInfoError).toBeNull(); + expect(state.loginError).toBeNull(); + expect(state.logoutError).toBeNull(); }); it('should login successfully', () => { @@ -36,23 +38,23 @@ describe('Auth duck', () => { let state = reducer(); state = reducer(state, login(username, password)); expect(state.isAuthenticated).toEqual(false); - expect(state.error).toBeNull(); + expect(state.loginError).toBeNull(); state = reducer(state, loginSuccess()); expect(state.isAuthenticated).toEqual(true); - expect(state.error).toBeNull(); + expect(state.loginError).toBeNull(); }); it('should handle failed login', () => { let state = reducer(); state = reducer(state, login('username', 'pass')); expect(state.isAuthenticated).toEqual(false); - expect(state.error).toBeNull(); + expect(state.loginError).toBeNull(); const error = new Error('test error'); state = reducer(state, loginError(error)); expect(state.isAuthenticated).toEqual(false); - expect(state.error).toEqual(error); + expect(state.loginError).toEqual(error); }); it('should set initial state for unauthenticated users', () => { @@ -62,7 +64,7 @@ describe('Auth duck', () => { const state = reducer(initialState, authInfoSuccess(authInfoLoggedOut)); expect(state.authInfoLoaded).toEqual(true); expect(state.isAuthenticated).toEqual(false); - expect(state.error).toBeNull(); + expect(state.authInfoError).toBeNull(); }); it('should set initial state for anonymous users', () => { @@ -72,7 +74,7 @@ describe('Auth duck', () => { const state = reducer(initialState, authInfoSuccess(authInfoAnonymous)); expect(state.authInfoLoaded).toEqual(true); expect(state.isAuthenticated).toEqual(false); - expect(state.error).toBeNull(); + expect(state.authInfoError).toBeNull(); }); it('should set initial state for unauthenticated users', () => { @@ -82,7 +84,7 @@ describe('Auth duck', () => { const state = reducer(initialState, authInfoSuccess(authInfoLoggedIn)); expect(state.authInfoLoaded).toEqual(true); expect(state.isAuthenticated).toEqual(true); - expect(state.error).toBeNull(); + expect(state.authInfoError).toBeNull(); }); }); From b70bda169e4a2cdc54586ed881238e95becb0a63 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Tue, 7 Mar 2017 11:03:45 +0200 Subject: [PATCH 4/4] Show separate auth error messages --- src/components/PageLayout/PageLayout.js | 37 ++++++++++++++--- .../AuthenticationPage/AuthenticationPage.js | 40 ++++++++++++++++--- .../AuthenticationPage.test.js.snap | 4 +- src/translations/en.json | 9 +++-- 4 files changed, 75 insertions(+), 15 deletions(-) diff --git a/src/components/PageLayout/PageLayout.js b/src/components/PageLayout/PageLayout.js index b7adc73d..feb13a11 100644 --- a/src/components/PageLayout/PageLayout.js +++ b/src/components/PageLayout/PageLayout.js @@ -2,6 +2,7 @@ import React, { Component, PropTypes } from 'react'; import { connect } from 'react-redux'; import Helmet from 'react-helmet'; import { withRouter } from 'react-router-dom'; +import { FormattedMessage } from 'react-intl'; import { Topbar } from '../../containers'; const scrollToTop = () => { @@ -21,12 +22,32 @@ class PageLayout extends Component { } render() { - const { className, title, children, authError } = this.props; - // TODO: use FlashMessages for authError + const { className, title, children, authInfoError, logoutError } = this.props; + + // TODO: use FlashMessages for auth errors + + /* eslint-disable no-console */ + if (authInfoError && console && console.error) { + console.error(authInfoError); + } + if (logoutError && console && console.error) { + console.error(logoutError); + } + /* eslint-enable no-console */ + return (
- {authError ?

Error in auth: {authError.message}

: null} + {authInfoError + ?
+ +
+ : null} + {logoutError + ?
+ +
+ : null}

{title}

{children} @@ -37,17 +58,21 @@ class PageLayout extends Component { const { any, string, instanceOf, func } = PropTypes; -PageLayout.defaultProps = { className: '', children: null, authError: null }; +PageLayout.defaultProps = { className: '', children: null, authInfoError: null, logoutError: null }; PageLayout.propTypes = { className: string, title: string.isRequired, children: any, - authError: instanceOf(Error), + authInfoError: instanceOf(Error), + logoutError: instanceOf(Error), // history.listen function from withRouter listen: func.isRequired, }; -const mapStateToProps = state => ({ authError: state.Auth.error }); +const mapStateToProps = state => ({ + authInfoError: state.Auth.authInfoError, + logoutError: state.Auth.logoutError, +}); export default connect(mapStateToProps)(withRouter(PageLayout)); diff --git a/src/containers/AuthenticationPage/AuthenticationPage.js b/src/containers/AuthenticationPage/AuthenticationPage.js index 356d7f35..8c836f8f 100644 --- a/src/containers/AuthenticationPage/AuthenticationPage.js +++ b/src/containers/AuthenticationPage/AuthenticationPage.js @@ -1,23 +1,44 @@ import React, { PropTypes } from 'react'; import { connect } from 'react-redux'; import { Redirect } from 'react-router-dom'; +import { FormattedMessage } from 'react-intl'; import { PageLayout, NamedLink, NamedRedirect } from '../../components'; import { LoginForm, SignUpForm } from '../../containers'; import { login } from '../../ducks/Auth.duck'; export const AuthenticationPageComponent = props => { - const { location, tab, isAuthenticated, onLoginSubmit, onSignUpSubmit } = props; + const { + location, + tab, + isAuthenticated, + loginError, + onLoginSubmit, + onSignUpSubmit, + } = props; const isLogin = tab === 'login'; const from = location.state && location.state.from ? location.state.from : null; const authRedirect = from ? : ; + // TODO: use FlashMessages for auth errors + + /* eslint-disable no-console */ + if (loginError && console && console.error) { + console.error(loginError); + } + /* eslint-enable no-console */ + return ( {isAuthenticated ? authRedirect : null} + {loginError + ?
+ +
+ : null} {from ?

- You must log in to view the page at + {from}

: null} @@ -29,19 +50,28 @@ export const AuthenticationPageComponent = props => { ); }; -AuthenticationPageComponent.defaultProps = { tab: 'signup' }; +AuthenticationPageComponent.defaultProps = { + tab: 'signup', + authInfoError: null, + loginError: null, + logoutError: null, +}; -const { object, oneOf, shape, bool, func } = PropTypes; +const { object, oneOf, shape, bool, func, instanceOf } = PropTypes; AuthenticationPageComponent.propTypes = { location: shape({ state: object }).isRequired, tab: oneOf(['login', 'signup']), isAuthenticated: bool.isRequired, + loginError: instanceOf(Error), onLoginSubmit: func.isRequired, onSignUpSubmit: func.isRequired, }; -const mapStateToProps = state => ({ isAuthenticated: state.Auth.isAuthenticated }); +const mapStateToProps = state => ({ + isAuthenticated: state.Auth.isAuthenticated, + loginError: state.Auth.loginError, +}); const mapDispatchToProps = dispatch => ({ onLoginSubmit: ({ email, password }) => dispatch(login(email, password)), diff --git a/src/containers/AuthenticationPage/__snapshots__/AuthenticationPage.test.js.snap b/src/containers/AuthenticationPage/__snapshots__/AuthenticationPage.test.js.snap index 1b2ad3b3..4b349936 100644 --- a/src/containers/AuthenticationPage/__snapshots__/AuthenticationPage.test.js.snap +++ b/src/containers/AuthenticationPage/__snapshots__/AuthenticationPage.test.js.snap @@ -2,7 +2,9 @@ exports[`AuthenticationPageComponent matches snapshot 1`] = `

- You must log in to view the page at + /protected diff --git a/src/translations/en.json b/src/translations/en.json index 858a8517..c055e900 100644 --- a/src/translations/en.json +++ b/src/translations/en.json @@ -1,8 +1,11 @@ { - "landingpage.examplelink": "Show nice studios! (from json)", + "AuthenticationPage.loginFailed": "The email and password you entered did not match our records. Please double-check and try again.", + "AuthenticationPage.loginRequiredFor": "You must log in to view the page at", "HeroSearchForm.placeholder": "Location search (soon)", "HeroSearchForm.search": "Search", - "HeroSection.title": "Book Studiotime anywhere", "HeroSection.subTitle": "The largest online community to rent music studios", - "ListingPage.loadingListingData": "Loading listing data" + "HeroSection.title": "Book Studiotime anywhere", + "ListingPage.loadingListingData": "Loading listing data", + "PageLayout.authInfoFailed": "Could not get authentication information.", + "PageLayout.logoutFailed": "Logout failed. Please try again." }