fix: render spinner correctly

src/components/AbsoluteCenter/AbsoluteCenter.js

@@ -1,5 +1,5 @@
 import React from 'react'
-import { node } from 'prop-types'
+import { node, bool } from 'prop-types'
 import styles from './AbsoluteCenter.module.css'
 
 /**
@@ -8,12 +8,23 @@ import styles from './AbsoluteCenter.module.css'
  * rendered in a row by default.
  */
 
-const AbsoluteCenter = ({ children }) => (
-    <div className={styles.center}>{children}</div>
-)
+const AbsoluteCenter = ({ children, vertical }) => {
+    const classNames = [styles.center]
+
+    if (vertical) {
+        classNames.push(styles.vertical)
+    }
+
+    return <div className={classNames.join(' ')}>{children}</div>
+}
+
+AbsoluteCenter.defaultProps = {
+    vertical: false,
+}
 
 AbsoluteCenter.propTypes = {
     children: node.isRequired,
+    vertical: bool,
 }
 
 export default AbsoluteCenter

src/components/AbsoluteCenter/AbsoluteCenter.module.css

@@ -9,3 +9,7 @@
     right: 0;
     pointer-events: none;
 }
+
+.vertical {
+    flex-direction: column;
+}

src/components/AbsoluteCenter/AbsoluteCenter.test.js

@@ -8,4 +8,10 @@ describe('<AbsoluteCenter>', () => {
 
         expect(wrapper).toMatchSnapshot()
     })
+
+    it('responds to a vertical prop', () => {
+        const wrapper = shallow(<AbsoluteCenter vertical>Text</AbsoluteCenter>)
+
+        expect(wrapper).toMatchSnapshot()
+    })
 })

src/components/AbsoluteCenter/__snapshots__/AbsoluteCenter.test.js.snap

@@ -7,3 +7,11 @@ exports[`<AbsoluteCenter> renders correctly 1`] = `
   Text
 </div>
 `;
+
+exports[`<AbsoluteCenter> responds to a vertical prop 1`] = `
+<div
+  className="center vertical"
+>
+  Text
+</div>
+`;

src/components/Aligner/Aligner.test.js

@@ -10,7 +10,7 @@ describe('<Aligner>', () => {
     })
 
     it('responds to a vertical prop', () => {
-        const wrapper = shallow(<Aligner>Text</Aligner>)
+        const wrapper = shallow(<Aligner vertical>Text</Aligner>)
 
         expect(wrapper).toMatchSnapshot()
     })

src/components/Aligner/__snapshots__/Aligner.test.js.snap

@@ -10,7 +10,7 @@ exports[`<Aligner> renders correctly 1`] = `
 
 exports[`<Aligner> responds to a vertical prop 1`] = `
 <div
-  className="horizontal"
+  className="vertical"
 >
   Text
 </div>

src/components/AuthWall/AuthWall.js

@@ -8,7 +8,7 @@ import { AbsoluteCenter } from '../AbsoluteCenter'
 
 export const UnconnectedAuthWall = ({
     children,
-    isFetching,
+    didFetch,
     errorMessage,
     isAuthorized,
     fetchMeIfNeeded,
@@ -17,10 +17,11 @@ export const UnconnectedAuthWall = ({
         fetchMeIfNeeded()
     }, [fetchMeIfNeeded])
 
-    if (isFetching) {
+    if (!didFetch) {
         return (
-            <AbsoluteCenter>
+            <AbsoluteCenter vertical>
                 <CircularLoader />
+                Checking permissions
             </AbsoluteCenter>
         )
     }
@@ -38,7 +39,7 @@ export const UnconnectedAuthWall = ({
 
 UnconnectedAuthWall.propTypes = {
     children: node.isRequired,
-    isFetching: bool.isRequired,
+    didFetch: bool.isRequired,
     errorMessage: string.isRequired,
     isAuthorized: bool.isRequired,
     fetchMeIfNeeded: func.isRequired,
@@ -50,7 +51,7 @@ const mapStateToProps = state => {
 
     /* istanbul ignore next */
     return {
-        isFetching: selectors.getIsFetching(me),
+        didFetch: selectors.getDidFetch(me),
         errorMessage: selectors.getErrorMessage(me),
         isAuthorized: selectors.getIsAuthorized(me),
     }

src/components/AuthWall/AuthWall.test.js

@@ -4,36 +4,40 @@ import { UnconnectedAuthWall as AuthWall } from './AuthWall'
 
 const defaultProps = {
     children: 'Children',
-    isFetching: false,
+    didFetch: false,
     errorMessage: '',
     isAuthorized: false,
     fetchMeIfNeeded: () => {},
 }
 
 describe('<AuthWall>', () => {
     it('renders a spinner when loading', () => {
-        const props = { ...defaultProps, isFetching: true }
+        const props = { ...defaultProps, didFetch: false }
         const wrapper = shallow(<AuthWall {...props} />)
 
         expect(wrapper).toMatchSnapshot()
     })
 
     it('renders errors if they occur', () => {
-        const props = { ...defaultProps, errorMessage: 'Something went wrong' }
+        const props = {
+            ...defaultProps,
+            didFetch: true,
+            errorMessage: 'Something went wrong',
+        }
         const wrapper = shallow(<AuthWall {...props} />)
 
         expect(wrapper).toMatchSnapshot()
     })
 
     it('renders a message for unauthorized users', () => {
-        const props = { ...defaultProps, isAuthorized: false }
+        const props = { ...defaultProps, didFetch: true, isAuthorized: false }
         const wrapper = shallow(<AuthWall {...props} />)
 
         expect(wrapper).toMatchSnapshot()
     })
 
     it('renders the children for authorized users', () => {
-        const props = { ...defaultProps, isAuthorized: true }
+        const props = { ...defaultProps, didFetch: true, isAuthorized: true }
         const wrapper = shallow(<AuthWall {...props} />)
 
         expect(wrapper).toMatchSnapshot()

src/components/AuthWall/__snapshots__/AuthWall.test.js.snap

@@ -7,8 +7,11 @@ exports[`<AuthWall> renders a message for unauthorized users 1`] = `
 `;
 
 exports[`<AuthWall> renders a spinner when loading 1`] = `
-<AbsoluteCenter>
+<AbsoluteCenter
+  vertical={true}
+>
   <CircularLoader />
+  Checking permissions
 </AbsoluteCenter>
 `;
 

src/data/me/actions.js

@@ -6,11 +6,17 @@ import * as selectors from './reducer'
 
 export const fetchMeSuccess = payload => ({
     type: types.FETCH_ME_SUCCESS,
+    meta: {
+        receivedAt: Date.now(),
+    },
     payload,
 })
 
 export const fetchMeFail = error => ({
     type: types.FETCH_ME_FAIL,
+    meta: {
+        receivedAt: Date.now(),
+    },
     error,
 })
 

src/data/me/actions.test.js

@@ -18,6 +18,9 @@ const { origin, pathname } = new URL(endpoints.me)
  * Mocks
  */
 
+// Return consistent date for testing
+Date.now = jest.fn(() => 1)
+
 // Allow selectors to be mocked
 selectors.getShouldFetch = jest.fn() // eslint-disable-line import/namespace
 rootSelectors.getMe = jest.fn() // eslint-disable-line import/namespace
@@ -34,6 +37,9 @@ describe('fetchMeSuccess', () => {
         const actual = actions.fetchMeSuccess('payload')
         const expected = {
             type: types.FETCH_ME_SUCCESS,
+            meta: {
+                receivedAt: 1,
+            },
             payload: 'payload',
         }
 
@@ -46,6 +52,9 @@ describe('fetchMeFail', () => {
         const actual = actions.fetchMeFail('error')
         const expected = {
             type: types.FETCH_ME_FAIL,
+            meta: {
+                receivedAt: 1,
+            },
             error: 'error',
         }
 
@@ -74,6 +83,9 @@ describe('fetchMe', () => {
             { type: types.FETCH_ME },
             {
                 type: types.FETCH_ME_SUCCESS,
+                meta: {
+                    receivedAt: 1,
+                },
                 payload: mockResponse,
             },
         ]
@@ -94,6 +106,9 @@ describe('fetchMe', () => {
             { type: types.FETCH_ME },
             {
                 type: types.FETCH_ME_FAIL,
+                meta: {
+                    receivedAt: 1,
+                },
                 error: error,
             },
         ]
@@ -126,6 +141,9 @@ describe('fetchMeIfNeeded', () => {
             { type: types.FETCH_ME },
             {
                 type: types.FETCH_ME_SUCCESS,
+                meta: {
+                    receivedAt: 1,
+                },
                 payload: mockResponse,
             },
         ]

src/data/me/reducer.js

@@ -3,7 +3,7 @@ import * as types from './actionTypes'
 const fallbackMessage =
     'Something went wrong, but no errormessage was provided.'
 const initialState = {
-    didFetchSuccessfully: false,
+    lastUpdated: 0,
     errorMessage: '',
     isFetching: false,
     data: {},
@@ -15,14 +15,14 @@ const reducer = (state = initialState, action) => {
             return { ...state, isFetching: true }
         case types.FETCH_ME_SUCCESS:
             return {
-                didFetchSuccessfully: true,
+                lastUpdated: action.meta.receivedAt,
                 errorMessage: '',
                 isFetching: false,
                 data: action.payload,
             }
         case types.FETCH_ME_FAIL:
             return {
-                ...state,
+                lastUpdated: action.meta.receivedAt,
                 errorMessage: action.error.message || fallbackMessage,
                 isFetching: false,
                 data: {},
@@ -40,6 +40,7 @@ export default reducer
 
 export const getIsFetching = state => state.isFetching
 export const getErrorMessage = state => state.errorMessage
+export const getDidFetch = state => !!state.lastUpdated
 
 export const getIsAuthorized = state => {
     const { authorities } = state.data
@@ -57,18 +58,19 @@ export const getIsAuthorized = state => {
 
 export const getShouldFetch = state => {
     const {
+        lastUpdated,
         isFetching,
         errorMessage,
         data: { authorities },
-        didFetchSuccessfully,
     } = state
     const hasError = !!errorMessage
+    const hasFetched = !!lastUpdated
     const hasAuthorities = !!authorities
 
     if (isFetching || hasAuthorities) {
         return false
     }
 
-    // Fetch if there's an error, if it hasn't fetched yet or there are no authorities
-    return hasError || !didFetchSuccessfully || !hasAuthorities
+    // Fetch if there's an error or if it hasn't fetched yet
+    return hasError || !hasFetched
 }