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

[TITAN-298] - Update Cart by Removing and Changing Quantity #65

Merged
merged 11 commits into from
May 19, 2023

Conversation

RossoMaguire
Copy link
Contributor

Description

This PR implements the CartLinesUpdate mutation from Shopify API when clicking the increase and decrease quantity buttons on the cart page.

We have handled a few edge cases such as:

  • If the maximum amount of an item's total quantity available has already been added to the cart show a notification and don't increase the quantity
  • If the quantity is then decreased when this message is shown, hide this message ( adding a closed property to the ProductNotification component )
  • If the items quantity is gone to zero then remove it from the cart

Testing

  • Add a couple of items to the cart

  • Test increasing and decreasing the quantities on the cart page

  • Test decreasing to zero, it should remove that line item from cart and show a message

  • Test increasing the item quantity until the max available has been reached, it should show a message

  • The prices in both the cart table and the cart quick view component should update based on the change of quantity

  • Tests are added for the scenarios mentioned above

Screenshot 2023-05-17 at 17 09 02

Copy link
Member

@alvarocavalcanti alvarocavalcanti left a comment

Choose a reason for hiding this comment

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

Only one minor point, LGTM otherwise.

@@ -4,6 +4,10 @@ import styles from './ProductNotification.module.scss';
const cx = classNames.bind(styles);

const ProductNotification = ({ productNotification, cartPage }) => {
if (productNotification.close) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a scenario for this condition?

Also, can we refactor this early return into a ternary operator in the return below?

return productNotification.close ? null : (

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We talked about testing this in the component itself but there is no assertion, nothing is rendered, so what do we test?

We could test it in Cart.test.js by making sure that the notice is shown when the max quantity is reached and then disappears when its decreased again - that's the real world scenario and the reason for introducing this prop. See line 75 of CartTable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alvarocavalcanti I added that test in the latest commit. Let me know what you think, probably a more accurate test of this scenario.

components/Cart/__tests__/Cart.test.js Outdated Show resolved Hide resolved
Co-authored-by: Alvaro Cavalcanti <alvaro.cavalcanti@wpengine.com>
Copy link
Member

@alvarocavalcanti alvarocavalcanti left a comment

Choose a reason for hiding this comment

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

Thumbs Up, by Kevin Arnold

@RossoMaguire RossoMaguire merged commit 91ec025 into main May 19, 2023
@RossoMaguire RossoMaguire deleted the RossoMaguire/TITAN-298 branch May 19, 2023 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants