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

Common useWishlist helper hook. #3141

Merged
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
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { gql } from '@apollo/client';

import { WishlistFragment } from './wishlistFragment';

export const GET_CONFIGURABLE_THUMBNAIL_SOURCE = gql`
query getConfigurableThumbnailSource {
storeConfig {
Expand All @@ -11,38 +9,6 @@ export const GET_CONFIGURABLE_THUMBNAIL_SOURCE = gql`
}
`;

export const GET_MULTIPLE_WISHLISTS_ENABLED = gql`
query getMultipleWishlistsEnabled {
storeConfig {
id
enable_multiple_wishlists
}
}
`;

export const ADD_TO_WISHLIST = gql`
mutation addProductToWishlist(
$wishlistId: ID!
$itemOptions: WishlistItemInput!
) {
addProductsToWishlist(
wishlistId: $wishlistId
wishlistItems: [$itemOptions]
) {
user_errors {
code
message
}
wishlist {
...WishlistFragment
}
}
}
${WishlistFragment}
`;

export default {
addProductToWishlistMutation: ADD_TO_WISHLIST,
getMultipleWishlistsEnabledQuery: GET_MULTIPLE_WISHLISTS_ENABLED,
getConfigurableThumbnailSource: GET_CONFIGURABLE_THUMBNAIL_SOURCE
};
51 changes: 36 additions & 15 deletions packages/peregrine/lib/talons/CartPage/ProductListing/useProduct.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useCartContext } from '@magento/peregrine/lib/context/cart';
import { useUserContext } from '@magento/peregrine/lib/context/user';
import configuredVariant from '@magento/peregrine/lib/util/configuredVariant';

import { useWishlist } from './useWishlist';
import { useWishlist } from '../../Wishlist/Wishlist/useWishlist';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a talon or a hook? Does it need to have the Wishlist/Wishlist nesting? I don't really know where to put this file anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you are right, this isn't ideal but I dint delete or move code in this case. It was just for presentational purposes. I will move this hook to a different location once I get approval on the code.

import { deriveErrorMessage } from '../../../util/deriveErrorMessage';
import mergeOperations from '../../../util/shallowMerge';

Expand Down Expand Up @@ -36,7 +36,7 @@ import DEFAULT_OPERATIONS from './product.gql';
export const useProduct = props => {
const {
item,
onAddToWishlistSuccess,
updateWishlistToastProps,
setActiveEditItem,
setIsCartUpdating
} = props;
Expand Down Expand Up @@ -109,12 +109,34 @@ export const useProduct = props => {
return null;
}, [formatMessage, showLoginToast]);

const onWishlistUpdate = useCallback(async () => {
await removeItemFromCart({
variables: {
cartId,
itemId: item.id
}
});
}, [removeItemFromCart, item, cartId]);

const wishlistItemOptions = {
...item,
sku: item.product.sku,
selected_options: item.configurable_options.map(
option => option.configurable_product_option_value_uid
)
};

const onWishlistUpdateError = useCallback(() => {
setDisplayError(true);
}, []);

const wishlistTalonProps = useWishlist({
removeItemFromCart,
cartId,
item,
setDisplayError,
onAddToWishlistSuccess,
item: wishlistItemOptions,
onWishlistUpdateError,
updateWishlistToastProps,
onWishlistUpdate,
operations: props.operations
});
const {
Expand Down Expand Up @@ -143,16 +165,15 @@ export const useProduct = props => {
updateItemLoading
]);

const handleSaveForLater = useCallback(
async (...args) => {
if (!isSignedIn) {
setShowLoginToast(current => ++current);
} else {
await handleAddToWishlist(args);
}
},
[isSignedIn, handleAddToWishlist]
);
const handleSaveForLater = useCallback(async () => {
if (!isSignedIn) {
setShowLoginToast(current => ++current);
} else {
// should update this to the wishlist selected by the user in
// https://jira.corp.magento.com/browse/PWA-1599
await handleAddToWishlist('0');
}
}, [isSignedIn, handleAddToWishlist]);

const derivedErrorMessage = useMemo(() => {
return (
Expand Down

This file was deleted.

4 changes: 2 additions & 2 deletions packages/peregrine/lib/talons/CartPage/useCartPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ export const useCartPage = props => {
return (data && data.cart.items) || [];
}, [data]);

const onAddToWishlistSuccess = useCallback(successToastProps => {
const updateWishlistToastProps = useCallback(successToastProps => {
setWishlistSuccessProps(successToastProps);
}, []);

return {
cartItems,
hasItems,
isCartUpdating,
onAddToWishlistSuccess,
updateWishlistToastProps,
setIsCartUpdating,
shouldShowLoadingIndicator,
wishlistSuccessProps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,10 @@ export const useProductFullDetail = props => {

// Normalization object for product details we need for rendering.
const productDetails = {
description: product.description,
name: product.name,
...product,
quantity: 1,
price: productPrice,
sku: product.sku
selected_options: selectedOptionsArray
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment for this object implies it was specifically used as a container for display details. I'd prefer to separate the concerns here, and keep the wishlistItemOptions separate from the data used for rendering the product page.

It would also help to include jsdoc in the useWishlist hook so we know what composes an item.

If we decide to keep your modifications to it we should at least remove the unused wishlistItemOptions prop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, all that is gonna change. This will be a helper hook and the consumer of this hook is required to send the data it needs to work in this case wishlistItemOptions.

};

const derivedErrorMessage = useMemo(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`should return correct shape 1`] = `
Object {
"called": true,
"error": null,
"handleAddToWishlist": [Function],
"isDisabled": true,
"isItemAdded": false,
"loading": true,
}
`;

exports[`testing handleAddToWishlist should add the product to list 1`] = `
Array [
Object {
"variables": Object {
"itemOptions": Object {
"quantity": "1",
"selected_options": "selected options",
"sku": "sku",
},
"wishlistId": "0",
},
},
]
`;

exports[`testing handleAddToWishlist should call onWishlistUpdateError if there was an error calling the mutation 1`] = `
Array [
"Something went wrong",
]
`;

exports[`testing handleAddToWishlist should call updateWishlistToastProps 1`] = `
Array [
Object {
"message": "Item successfully added to your favorites list.",
"timeout": 5000,
"type": "info",
},
]
`;

exports[`testing handleAddToWishlist should use necessary translations 1`] = `
Array [
Array [
Object {
"defaultMessage": "Item successfully added to your favorites list.",
"id": "cartPage.wishlist.ce.successMessage",
},
],
]
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`should return correct shape 1`] = `
Object {
"called": true,
"error": null,
"handleAddToWishlist": [Function],
"handleModalClose": [Function],
"handleModalOpen": [Function],
"isDisabled": true,
"isItemAdded": false,
"isModalOpen": false,
"loading": true,
}
`;

exports[`testing handleAddToWishlist should add the product to list 1`] = `
Array [
Object {
"variables": Object {
"itemOptions": Object {
"quantity": "1",
"selected_options": "selected options",
"sku": "sku",
},
"wishlistId": "0",
},
},
]
`;

exports[`testing handleAddToWishlist should call onWishlistUpdateError if there was an error calling the mutation 1`] = `
Array [
"Something went wrong",
]
`;

exports[`testing handleAddToWishlist should call updateWishlistToastProps 1`] = `
Array [
Object {
"message": "Item successfully added to Bday List.",
"timeout": 5000,
"type": "info",
},
]
`;

exports[`testing handleAddToWishlist should use necessary translations 1`] = `
Array [
Array [
Object {
"defaultMessage": "Item successfully added to Bday List.",
"id": "cartPage.wishlist.ee.successMessage",
},
Object {
"wishlistName": "Bday List",
},
],
]
`;
Loading