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

fix: STRF-11899 Update cart when multiple coupons are removed #1197

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

huntario
Copy link
Contributor

@huntario huntario commented Apr 15, 2024

What?

When removing multiple coupons, bust the cache because the cart has updated. If we don't the request signature just above this point -

({ response2 } = cachedResponse2);
- in the code will be the same as the last and we erroneously return 'cachedResponse2' which is stale.

The request to update the cart is different because it contains the coupon ID, but since it uses a redirect back to the cart, that request is undifferentiated from the previous request to the cart.php

Tickets / Documentation

Add links to any relevant tickets and documentation.

Screenshots (if appropriate)

Before: First coupon removal works as expected, but subsequent removals don't appear to take effect until refresh.

Screen-2024-04-15-100043.mp4

After: additional coupons removed as expected

Screen-2024-04-15-100253.mp4

cc @bigcommerce/storefront-team

Copy link
Contributor

@jairo-bc jairo-bc left a comment

Choose a reason for hiding this comment

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

I'm ok with the change, we just need to double check on historical reason of having this and keep eyes on amount of traffic we potentially can have because of this change

@huntario
Copy link
Contributor Author

huntario commented Apr 15, 2024

...we just need to double check on historical reason of having this

Looks like it was added here - #124 - with the intention of speeding up development. Noticed this comment - #124 (comment) - which lead to clearing the cache if a post request is used. I've linked the ticket for that change to the current ticket.

...and keep eyes on amount of traffic we potentially can have because of this change

I've placed a link in the internal ticket that should allow us to track any change in usage.

@jairo-bc jairo-bc merged commit 13a31ba into bigcommerce:master Apr 16, 2024
6 checks passed
Copy link
Contributor

🎉 This PR is included in version 7.5.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants