The Sign In Page and Input Consistency

packages/venia-concept/src/actions/user/__tests__/actions.spec.js

@@ -30,37 +30,6 @@ test('signIn.receive() returns a proper action object', () => {
     });
 });
 
-test('signInError.request.toString() returns the proper action type', () => {
-    expect(actions.signInError.request.toString()).toBe(
-        'USER/SIGN_IN_ERROR/REQUEST'
-    );
-});
-
-test('signInError.request() returns a proper action object', () => {
-    expect(actions.signInError.request(payload)).toEqual({
-        type: 'USER/SIGN_IN_ERROR/REQUEST',
-        payload
-    });
-});
-
-test('signInError.receive.toString() returns the proper action type', () => {
-    expect(actions.signInError.receive.toString()).toBe(
-        'USER/SIGN_IN_ERROR/RECEIVE'
-    );
-});
-
-test('signInError.receive() returns a proper action object', () => {
-    expect(actions.signInError.receive(payload)).toEqual({
-        type: 'USER/SIGN_IN_ERROR/RECEIVE',
-        payload
-    });
-    expect(actions.signInError.receive(error)).toEqual({
-        type: 'USER/SIGN_IN_ERROR/RECEIVE',
-        payload: error,
-        error: true
-    });
-});
-
 test('createAccountError.request.toString() returns the proper action type', () => {
     expect(actions.createAccountError.request.toString()).toBe(
         'USER/CREATE_ACCOUNT_ERROR/REQUEST'
@@ -123,37 +92,6 @@ test('resetCreateAccountError.receive() returns a proper action object', () => {
     });
 });
 
-test('resetSignInError.request.toString() returns the proper action type', () => {
-    expect(actions.resetSignInError.request.toString()).toBe(
-        'USER/RESET_SIGN_IN_ERROR/REQUEST'
-    );
-});
-
-test('resetSignInError.request() returns a proper action object', () => {
-    expect(actions.resetSignInError.request(payload)).toEqual({
-        type: 'USER/RESET_SIGN_IN_ERROR/REQUEST',
-        payload
-    });
-});
-
-test('resetSignInError.receive.toString() returns the proper action type', () => {
-    expect(actions.resetSignInError.receive.toString()).toBe(
-        'USER/RESET_SIGN_IN_ERROR/RECEIVE'
-    );
-});
-
-test('resetSignInError.receive() returns a proper action object', () => {
-    expect(actions.resetSignInError.receive(payload)).toEqual({
-        type: 'USER/RESET_SIGN_IN_ERROR/RECEIVE',
-        payload
-    });
-    expect(actions.resetSignInError.receive(error)).toEqual({
-        type: 'USER/RESET_SIGN_IN_ERROR/RECEIVE',
-        payload: error,
-        error: true
-    });
-});
-
 describe('resetPassword', () => {
     test('resetPassword.request.toString() returns the proper action type', () => {
         expect(actions.resetPassword.request.toString()).toBe(

packages/venia-concept/src/actions/user/__tests__/asyncActions.spec.js

@@ -53,108 +53,67 @@ afterAll(() => {
     getState.mockRestore();
 });
 
-test('getUserDetails() returns a thunk', () => {
-    expect(getUserDetails()).toBeInstanceOf(Function);
-});
-
 test('createAccount() returns a thunk', () => {
     expect(createAccount()).toBeInstanceOf(Function);
 });
 
-test('createNewUserRequest() returns a thunk', () => {
-    expect(createNewUserRequest()).toBeInstanceOf(Function);
-});
-
-test('getUserDetails thunk makes request to get customer details if user is signed in', async () => {
-    getState.mockImplementationOnce(() => ({
-        user: { isSignedIn: false }
-    }));
-
-    await getUserDetails()(...thunkArgs);
-
-    expect(request.mock.calls).toHaveLength(0);
-
-    getState.mockImplementationOnce(() => ({
-        user: { isSignedIn: true }
-    }));
-
-    await getUserDetails()(...thunkArgs);
-
-    const firstRequest = request.mock.calls[0];
-
-    expect(firstRequest[0]).toBe('/rest/V1/customers/me');
-    expect(firstRequest[1]).toHaveProperty('method', 'GET');
-});
-
-test('getUserDetails thunk dispatches resetSignInError', async () => {
-    getState.mockImplementationOnce(() => ({
-        user: { isSignedIn: true }
-    }));
-
-    await getUserDetails()(...thunkArgs);
-
-    expect(dispatch).toHaveBeenCalledWith(actions.resetSignInError.request());
-});
-
-test('getUserDetails thunk dispatches signInError on failed request', async () => {
-    const error = new Error('ERROR');
-    getState.mockImplementationOnce(() => ({
-        user: { isSignedIn: true }
-    }));
-    request.mockRejectedValueOnce(error);
-    await getUserDetails()(...thunkArgs);
-    expect(dispatch).toHaveBeenNthCalledWith(
-        2,
-        actions.signInError.receive(error)
-    );
-});
+describe('createNewUserRequest', () => {
+    test('it returns a thunk', () => {
+        expect(createNewUserRequest()).toBeInstanceOf(Function);
+    });
 
-test('createNewUserRequest thunk dispatches resetCreateAccountError', async () => {
-    await createNewUserRequest(accountInfo)(...thunkArgs);
+    test('its thunk dispatches resetCreateAccountError', async () => {
+        await createNewUserRequest(accountInfo)(...thunkArgs);
 
-    expect(dispatch).toHaveBeenCalledWith(
-        actions.resetCreateAccountError.request()
-    );
-});
+        expect(dispatch).toHaveBeenCalledWith(
+            actions.resetCreateAccountError.request()
+        );
+    });
 
-test('createNewUserRequest thunk dispatches signIn', async () => {
-    await createNewUserRequest(accountInfo)(...thunkArgs);
+    test('its thunk dispatches signIn', async () => {
+        await createNewUserRequest(accountInfo)(...thunkArgs);
 
-    expect(dispatch).toHaveBeenNthCalledWith(2, expect.any(Function));
-});
+        expect(dispatch).toHaveBeenNthCalledWith(2, expect.any(Function));
+    });
 
-test('createNewUserRequest thunk dispatches createAccountError on invalid account info', async () => {
-    const error = new TypeError('ERROR');
-    request.mockRejectedValueOnce(error);
+    test('its thunk dispatches createAccountError on invalid account info', async () => {
+        const error = new TypeError('ERROR');
+        request.mockRejectedValueOnce(error);
 
-    try {
-        await createNewUserRequest({})(...thunkArgs);
-    } catch (e) {}
+        try {
+            await createNewUserRequest({})(...thunkArgs);
+        } catch (e) {}
 
-    expect(dispatch).toHaveBeenNthCalledWith(
-        2,
-        actions.createAccountError.receive(error)
-    );
+        expect(dispatch).toHaveBeenNthCalledWith(
+            2,
+            actions.createAccountError.receive(error)
+        );
+    });
 });
 
 describe('signIn', () => {
     test('it returns a thunk', () => {
         expect(signIn()).toBeInstanceOf(Function);
     });
 
+    test('its thunk returns undefined', async () => {
+        const result = await signIn(credentials)(...thunkArgs);
+
+        expect(result).toBeUndefined();
+    });
+
     test('its thunk dispatches actions on success', async () => {
         await signIn(credentials)(...thunkArgs);
 
-        expect(dispatch).toHaveBeenCalledTimes(4);
-        expect(dispatch).toHaveBeenNthCalledWith(
-            1,
-            actions.resetSignInError.request()
-        );
+        expect(dispatch).toHaveBeenCalledTimes(5);
+        expect(dispatch).toHaveBeenNthCalledWith(1, actions.signIn.request());
         expect(dispatch).toHaveBeenNthCalledWith(2, actions.signIn.receive());
-        // removeCart
+        // getUserDetails
         expect(dispatch).toHaveBeenNthCalledWith(3, expect.any(Function));
-        // getCartDetails
+        // removeCart
         expect(dispatch).toHaveBeenNthCalledWith(4, expect.any(Function));
+        // getCartDetails
+        expect(dispatch).toHaveBeenNthCalledWith(5, expect.any(Function));
     });
 
     test('its thunk dispatches actions on error', async () => {
@@ -167,20 +126,17 @@ describe('signIn', () => {
 
         // Make assertions.
         expect(dispatch).toHaveBeenCalledTimes(2);
-        expect(dispatch).toHaveBeenNthCalledWith(
-            1,
-            actions.resetSignInError.request()
-        );
+        expect(dispatch).toHaveBeenNthCalledWith(1, actions.signIn.request());
         expect(dispatch).toHaveBeenNthCalledWith(
             2,
-            actions.signInError.receive(error)
+            actions.signIn.receive(error)
         );
     });
 
     test('its thunk makes requests on success', async () => {
         await signIn(credentials)(...thunkArgs);
 
-        expect(request).toHaveBeenCalledTimes(2);
+        expect(request).toHaveBeenCalledTimes(1);
 
         const tokenRequest = request.mock.calls[0];
         const tokenEndpoint = tokenRequest[0];
@@ -195,13 +151,6 @@ describe('signIn', () => {
             username: `${credentials.username}`,
             password: `${credentials.password}`
         });
-
-        const detailsRequest = request.mock.calls[1];
-        const detailsEndpoint = detailsRequest[0];
-        const detailsParams = detailsRequest[1];
-
-        expect(detailsEndpoint).toBe('/rest/V1/customers/me');
-        expect(detailsParams).toHaveProperty('method', 'GET');
     });
 });
 
@@ -228,6 +177,66 @@ describe('signOut', () => {
     });
 });
 
+describe('getUserDetails', () => {
+    test('it returns a thunk', () => {
+        expect(getUserDetails()).toBeInstanceOf(Function);
+    });
+
+    test('its thunk returns undefined', async () => {
+        const result = await getUserDetails()(...thunkArgs);
+
+        expect(result).toBeUndefined();
+    });
+
+    test('its thunk dispatches actions on success', async () => {
+        getState.mockImplementationOnce(() => ({
+            user: { isSignedIn: true }
+        }));
+
+        await getUserDetails()(...thunkArgs);
+
+        expect(dispatch).toHaveBeenCalledTimes(2);
+        expect(dispatch).toHaveBeenNthCalledWith(
+            1,
+            actions.getDetails.request()
+        );
+        expect(dispatch).toHaveBeenNthCalledWith(
+            2,
+            actions.getDetails.receive()
+        );
+    });
+
+    test('its thunk dispatches actions on error', async () => {
+        getState.mockImplementationOnce(() => ({
+            user: { isSignedIn: true }
+        }));
+        const error = new Error('ERROR');
+        request.mockRejectedValueOnce(error);
+
+        await getUserDetails()(...thunkArgs);
+
+        expect(dispatch).toHaveBeenCalledTimes(2);
+        expect(dispatch).toHaveBeenNthCalledWith(
+            1,
+            actions.getDetails.request()
+        );
+        expect(dispatch).toHaveBeenNthCalledWith(
+            2,
+            actions.getDetails.receive(error)
+        );
+    });
+
+    test('its thunk doesnt dispatch actions if the user is not signed in', async () => {
+        getState.mockImplementationOnce(() => ({
+            user: { isSignedIn: false }
+        }));
+
+        await getUserDetails()(...thunkArgs);
+
+        expect(dispatch).not.toHaveBeenCalled();
+    });
+});
+
 describe('resetPassword', () => {
     const email = '[email protected]';
 

packages/venia-concept/src/actions/user/actions.js

@@ -9,11 +9,7 @@ const actionMap = {
         RECEIVE: null,
         RESET: null
     },
-    RESET_SIGN_IN_ERROR: {
-        REQUEST: null,
-        RECEIVE: null
-    },
-    SIGN_IN_ERROR: {
+    GET_DETAILS: {
         REQUEST: null,
         RECEIVE: null
     },

packages/venia-concept/src/actions/user/asyncActions.js

@@ -13,7 +13,7 @@ export const signIn = credentials =>
     async function thunk(...args) {
         const [dispatch] = args;
 
-        dispatch(actions.resetSignInError.request());
+        dispatch(actions.signIn.request());
 
         try {
             const body = {
@@ -31,18 +31,17 @@ export const signIn = credentials =>
 
             setToken(response);
 
-            const userDetails = await request('/rest/V1/customers/me', {
-                method: 'GET'
-            });
+            await dispatch(actions.signIn.receive(response));
 
-            await dispatch(actions.signIn.receive(userDetails));
+            // Now that we're signed in, get this user's details.
+            dispatch(getUserDetails());
 
             // Now that we're signed in, forget the old (guest) cart
             // and fetch this customer's cart.
             await dispatch(removeCart());
             dispatch(getCartDetails({ forceRefresh: true }));
         } catch (error) {
-            dispatch(actions.signInError.receive(error));
+            dispatch(actions.signIn.receive(error));
         }
     };
 
@@ -66,15 +65,16 @@ export const getUserDetails = () =>
         const { user } = getState();
 
         if (user.isSignedIn) {
-            dispatch(actions.resetSignInError.request());
+            dispatch(actions.getDetails.request());
 
             try {
                 const userDetails = await request('/rest/V1/customers/me', {
                     method: 'GET'
                 });
-                dispatch(actions.signIn.receive(userDetails));
+
+                dispatch(actions.getDetails.receive(userDetails));
             } catch (error) {
-                dispatch(actions.signInError.receive(error));
+                dispatch(actions.getDetails.receive(error));
             }
         }
     };