Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PWA-138: Re-factor getUserDetails to use GraphQL (useAwaitQuery Edition) #2004

Merged
merged 7 commits into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions packages/peregrine/lib/hooks/useAwaitQuery.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { useCallback } from 'react';
import { useApolloClient } from '@apollo/react-hooks';

/**
* A hook that will return a invokable query that returns a Promise. Intended
* to be used in Redux thunks that shouldn't have knowledge of queries being ran
* but needed the ability to fetch data asyncronously inside of their actions.
*
* @param {DocumentNode} query - parsed GraphQL operation description
*
* @returns {Function} callback that runs the query and returns a Promise
*/
export const useAwaitQuery = query => {
tjwiebell marked this conversation as resolved.
Show resolved Hide resolved
const apolloClient = useApolloClient();

return useCallback(
options => {
return apolloClient.query({
tjwiebell marked this conversation as resolved.
Show resolved Hide resolved
...options,
query
});
},
[apolloClient, query]
);
};
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
import { Magento2 } from '../../../../RestApi';
import actions from '../actions';
import { getUserDetails, resetPassword } from '../asyncActions';

jest.mock('../../../../RestApi');
jest.mock('../../../../util/simplePersistence');

const { request } = Magento2;
const dispatch = jest.fn();
const getState = jest.fn(() => ({
user: { isSignedIn: false }
}));
const thunkArgs = [dispatch, getState];
const fetchUserDetails = jest
.fn()
.mockResolvedValue({ data: { customer: {} } });

describe('getUserDetails', () => {
test('it returns a thunk', () => {
expect(getUserDetails()).toBeInstanceOf(Function);
expect(getUserDetails({ fetchUserDetails })).toBeInstanceOf(Function);
});

test('its thunk returns undefined', async () => {
const result = await getUserDetails()(...thunkArgs);
const result = await getUserDetails({ fetchUserDetails })(...thunkArgs);

expect(result).toBeUndefined();
});
Expand All @@ -28,7 +29,7 @@ describe('getUserDetails', () => {
user: { isSignedIn: true }
}));

await getUserDetails()(...thunkArgs);
await getUserDetails({ fetchUserDetails })(...thunkArgs);

expect(dispatch).toHaveBeenCalledTimes(2);
expect(dispatch).toHaveBeenNthCalledWith(
Expand All @@ -37,7 +38,7 @@ describe('getUserDetails', () => {
);
expect(dispatch).toHaveBeenNthCalledWith(
2,
actions.getDetails.receive()
actions.getDetails.receive({})
);
});

Expand All @@ -46,9 +47,9 @@ describe('getUserDetails', () => {
user: { isSignedIn: true }
}));
const error = new Error('ERROR');
request.mockRejectedValueOnce(error);
fetchUserDetails.mockRejectedValueOnce(error);

await getUserDetails()(...thunkArgs);
await getUserDetails({ fetchUserDetails })(...thunkArgs);

expect(dispatch).toHaveBeenCalledTimes(2);
expect(dispatch).toHaveBeenNthCalledWith(
Expand All @@ -66,7 +67,7 @@ describe('getUserDetails', () => {
user: { isSignedIn: false }
}));

await getUserDetails()(...thunkArgs);
await getUserDetails({ fetchUserDetails })(...thunkArgs);

expect(dispatch).not.toHaveBeenCalled();
});
Expand Down
10 changes: 6 additions & 4 deletions packages/peregrine/lib/store/actions/user/asyncActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const signOut = ({ history }) => async dispatch => {
refresh({ history });
};

export const getUserDetails = () =>
export const getUserDetails = ({ fetchUserDetails }) =>
async function thunk(...args) {
const [dispatch, getState] = args;
const { user } = getState();
Expand All @@ -69,11 +69,13 @@ export const getUserDetails = () =>
dispatch(actions.getDetails.request());

try {
const userDetails = await request('/rest/V1/customers/me', {
method: 'GET'
const { data } = await fetchUserDetails({
// until we can investigate some odd behavior with apollo-cache-persist
// not busting the cache on sign out, avoid caching user details.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tjwiebell did we create a ticket for this?

fetchPolicy: 'network-only'
tjwiebell marked this conversation as resolved.
Show resolved Hide resolved
});

dispatch(actions.getDetails.receive(userDetails));
dispatch(actions.getDetails.receive(data.customer));
} catch (error) {
dispatch(actions.getDetails.receive(error));
}
Expand Down
12 changes: 6 additions & 6 deletions packages/peregrine/lib/talons/CreateAccount/useCreateAccount.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useCallback, useMemo, useState } from 'react';
import { useMutation } from '@apollo/react-hooks';
import { useUserContext } from '@magento/peregrine/lib/context/user';
import { useCartContext } from '@magento/peregrine/lib/context/cart';
import { useAwaitQuery } from '@magento/peregrine/lib/hooks/useAwaitQuery';

/**
* Returns props necessary to render CreateAccount component. In particular this
Expand All @@ -22,6 +23,7 @@ import { useCartContext } from '@magento/peregrine/lib/context/cart';
*/
export const useCreateAccount = props => {
const {
customerQuery,
initialValues = {},
onSubmit,
createAccountQuery,
Expand All @@ -38,6 +40,7 @@ export const useCreateAccount = props => {
createAccountQuery
);
const [signIn, { error: signInError }] = useMutation(signInQuery);
const fetchUserDetails = useAwaitQuery(customerQuery);

const errors = [];
if (createAccountError) {
Expand Down Expand Up @@ -72,12 +75,8 @@ export const useCreateAccount = props => {
const token =
response && response.data.generateCustomerToken.token;

setToken(token);

// Then get user details
await getUserDetails();

// Then reset the cart
await setToken(token);
await getUserDetails({ fetchUserDetails });
await removeCart();
await getCartDetails({ forceRefresh: true });

Expand All @@ -92,6 +91,7 @@ export const useCreateAccount = props => {
},
[
createAccount,
fetchUserDetails,
getCartDetails,
getUserDetails,
onSubmit,
Expand Down
9 changes: 6 additions & 3 deletions packages/peregrine/lib/talons/Navigation/useNavigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useCallback, useEffect, useState } from 'react';
import { useAppContext } from '@magento/peregrine/lib/context/app';
import { useCatalogContext } from '@magento/peregrine/lib/context/catalog';
import { useUserContext } from '@magento/peregrine/lib/context/user';
import { useAwaitQuery } from '@magento/peregrine/lib/hooks/useAwaitQuery';

const ancestors = {
CREATE_ACCOUNT: 'SIGN_IN',
Expand All @@ -11,16 +12,18 @@ const ancestors = {
MENU: null
};

export const useNavigation = () => {
export const useNavigation = props => {
const { customerQuery } = props;
// retrieve app state from context
const [appState, { closeDrawer }] = useAppContext();
const [catalogState, { actions: catalogActions }] = useCatalogContext();
const [, { getUserDetails }] = useUserContext();
const fetchUserDetails = useAwaitQuery(customerQuery);

// request data from server
useEffect(() => {
getUserDetails();
}, [getUserDetails]);
getUserDetails({ fetchUserDetails });
}, [fetchUserDetails, getUserDetails]);

// extract relevant data from app state
const { drawer } = appState;
Expand Down
25 changes: 16 additions & 9 deletions packages/peregrine/lib/talons/SignIn/useSignIn.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import { useCallback, useRef, useState } from 'react';
import { useUserContext } from '../../context/user';
import { useMutation } from '@apollo/react-hooks';
import { useCartContext } from '../../context/cart';
import { useAwaitQuery } from '../../hooks/useAwaitQuery';

export const useSignIn = props => {
const {
setDefaultUsername,
showCreateAccount,
showForgotPassword,
query
signInMutation,
customerQuery
} = props;

const [isSigningIn, setIsSigningIn] = useState(false);
Expand All @@ -19,7 +21,9 @@ export const useSignIn = props => {
{ getUserDetails, setToken }
] = useUserContext();

const [signIn, { error: signInError }] = useMutation(query);
const fetchUserDetails = useAwaitQuery(customerQuery);

const [signIn, { error: signInError }] = useMutation(signInMutation);

const errors = [];
if (signInError) {
Expand All @@ -43,12 +47,8 @@ export const useSignIn = props => {
const token =
response && response.data.generateCustomerToken.token;

setToken(token);

// Then get user details
await getUserDetails();

// Then reset the cart
await setToken(token);
await getUserDetails({ fetchUserDetails });
await removeCart();
await getCartDetails({ forceRefresh: true });
} catch (error) {
Expand All @@ -59,7 +59,14 @@ export const useSignIn = props => {
setIsSigningIn(false);
}
},
[getCartDetails, getUserDetails, removeCart, setToken, signIn]
[
fetchUserDetails,
getCartDetails,
getUserDetails,
removeCart,
setToken,
signIn
]
);

const handleForgotPassword = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ jest.mock('@magento/peregrine/lib/context/cart', () => {
return { useCartContext };
});

jest.mock('@magento/peregrine/lib/hooks/useAwaitQuery', () => {
const useAwaitQuery = jest
.fn()
.mockResolvedValue({ data: { customer: {} } });

return { useAwaitQuery };
});

const props = {
onSubmit: jest.fn()
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ import defaultClasses from './createAccount.css';
import { useCreateAccount } from '@magento/peregrine/lib/talons/CreateAccount/useCreateAccount';
import CREATE_ACCOUNT_MUTATION from '../../queries/createAccount.graphql';
import SIGN_IN_MUTATION from '../../queries/signIn.graphql';
import GET_CUSTOMER_QUERY from '../../queries/getCustomer.graphql';

const LEAD =
'Check out faster, use multiple addresses, track orders and more by creating an account!';

const CreateAccount = props => {
const talonProps = useCreateAccount({
customerQuery: GET_CUSTOMER_QUERY,
initialValues: props.initialValues,
createAccountQuery: CREATE_ACCOUNT_MUTATION,
signInQuery: SIGN_IN_MUTATION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ jest.mock('@magento/peregrine/lib/context/user', () => {
return { useUserContext };
});

jest.mock('@magento/peregrine/lib/hooks/useAwaitQuery', () => {
const useAwaitQuery = jest
.fn()
.mockResolvedValue({ data: { customer: {} } });

return { useAwaitQuery };
});

test('renders correctly when open', () => {
const instance = createTestInstance(<Navigation />);

Expand Down
6 changes: 4 additions & 2 deletions packages/venia-ui/lib/components/Navigation/navigation.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import React from 'react';
import { shape, string } from 'prop-types';

import { useNavigation } from '@magento/peregrine/lib/talons/Navigation/useNavigation';

import { mergeClasses } from '../../classify';
import AuthBar from '../AuthBar';
import AuthModal from '../AuthModal';
import CategoryTree from '../CategoryTree';
import NavHeader from './navHeader';
import defaultClasses from './navigation.css';
import { useNavigation } from '@magento/peregrine/lib/talons/Navigation/useNavigation';
import GET_CUSTOMER_QUERY from '../../queries/getCustomer.graphql';

const Navigation = props => {
const {
Expand All @@ -26,7 +28,7 @@ const Navigation = props => {
showMyAccount,
showSignIn,
view
} = useNavigation();
} = useNavigation({ customerQuery: GET_CUSTOMER_QUERY });

const classes = mergeClasses(defaultClasses, props.classes);
const rootClassName = isOpen ? classes.root_open : classes.root;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ jest.mock('@magento/peregrine/lib/context/user', () => {
return { useUserContext };
});

jest.mock('@magento/peregrine/lib/hooks/useAwaitQuery', () => {
const useAwaitQuery = jest
.fn()
.mockResolvedValue({ data: { customer: {} } });

return { useAwaitQuery };
});

const props = {
setDefaultUsername: jest.fn(),
showCreateAccount: jest.fn(),
Expand Down
4 changes: 3 additions & 1 deletion packages/venia-ui/lib/components/SignIn/signIn.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ import combine from '../../util/combineValidators';
import defaultClasses from './signIn.css';
import { useSignIn } from '@magento/peregrine/lib/talons/SignIn/useSignIn';
import SIGN_IN_MUTATION from '../../queries/signIn.graphql';
import GET_CUSTOMER_QUERY from '../../queries/getCustomer.graphql';

const SignIn = props => {
const classes = mergeClasses(defaultClasses, props.classes);
const { setDefaultUsername, showCreateAccount, showForgotPassword } = props;

const talonProps = useSignIn({
query: SIGN_IN_MUTATION,
customerQuery: GET_CUSTOMER_QUERY,
signInMutation: SIGN_IN_MUTATION,
setDefaultUsername,
showCreateAccount,
showForgotPassword
Expand Down
9 changes: 9 additions & 0 deletions packages/venia-ui/lib/queries/getCustomer.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# expects bearer header to be set via context to return data
query getCustomer {
tjwiebell marked this conversation as resolved.
Show resolved Hide resolved
customer {
id
email
firstname
lastname
}
}