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

[4.x]: forgetCart() not working as expected #3353

Closed
samhibberd opened this issue Dec 18, 2023 · 13 comments · Fixed by #3359
Closed

[4.x]: forgetCart() not working as expected #3353

samhibberd opened this issue Dec 18, 2023 · 13 comments · Fixed by #3359
Labels
bug commerce4 Issues related to Commerce v4

Comments

@samhibberd
Copy link

samhibberd commented Dec 18, 2023

What happened?

It looks like the changes to the Carts service getSessionCartNumber() method have introduced some unexpected behavior with the forgetCart() logic, as far as I can tell, it's not working for our use case:

We have some quick checkout logic that operates separate to the session cart and we used to be able to use the following to create a new cart separate from the session (before reloading in the original cart back into the session once we have finished):

$cartsService = Commerce::getInstance()->getCarts();
$cartsService->forgetCart();
$cart = $cartsService->getCart(forceSave: true);
...

But whatever we do, in 4.3.3, the getCart() logic in the example above returns the cart currently stored in the session (if there is one). Reverting to 4.3.2 reverts to the expected behavior.

It looks to be related to the request cookies I think??

Craft CMS version

4.5.12

Craft Commerce version

4.3.3

PHP version

8.2

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

@samhibberd samhibberd added commerce4 Issues related to Commerce v4 bug labels Dec 18, 2023
@samhibberd
Copy link
Author

Any thoughts on this would be much appreciated. Comes with the knock on impact of being unable to update beyond version 4.0.1.1 of the Stripe Commerce plugin as 4.1.0 now requires 4.3.3.

@nfourtythree
Copy link
Contributor

Hi @samhibberd

Thank you for your message, due to the nature of the getting the cart (now it uses cookies) you need to make sure you are omitting the cookies in your separate request.

Below is a little AJAX example you can drop into you templates to test this theory.

// Twig
<div>Current Cart: {{ craft.commerce.carts.cart.number }}</div>

<div>Separate cart: <span data-separate-cart></span></div>

// JS
var myHeaders = new Headers();
myHeaders.append("Accept", "application/json");

fetch("{{ actionUrl('commerce/cart/get-cart') }}", {
    method: 'GET',
    headers: myHeaders,
    redirect: 'follow',
    credentials: 'omit',
})
.then(response => response.json())
.then(result => {
    document.querySelector('[data-separate-cart]').innerHTML = result.cart.number;
})
.catch(error => console.log('error', error));

Hope this helps, thanks!

@samhibberd
Copy link
Author

Hi @nfourtythree, thanks for the example but it's not quite representative of our use case. In 4.3.2 (and previous versions) we needed to use the forgetCart() method to allow us to call the native getCart() method to create a new cart, rather than returning the cart in the current session (which getCart() always does if it exists), we then load the session cart back in after we have retrieved are new cart.

We use this for a quick checkout independent of any cart / basket that currently exists.

The process goes:

  1. Single request submits all the order info required for a single purchasable (custom/controller/quick-checkout)
  2. Store any cart in session and then forget
  3. Create new cart and populate
  4. Restore cart from session

The reason we need to do this is to take advantage of all the create cart logic which sets attributes, addresses etc etc:

https://github.com/craftcms/commerce/blob/c88038ed5958cc50087c959ae2d48764303fe33b/src/services/Carts.php#L120C9-L120C10

If this could be abstracted out of the getCart() method and called directly (or have a $forceNew argument?) then we could skip all of this, just create a new empty cart and that would be problem solved!!

@nfourtythree
Copy link
Contributor

nfourtythree commented Jan 4, 2024

Hi @samhibberd

Thank you for your reply. Below is a rough outline of some PHP code that I think could point you in the right direction for what you are trying to achieve.

<?php

namespace modules\controllers;

use Craft;
use craft\commerce\Plugin;
use craft\web\Controller;
use yii\web\Response;

class CartController extends Controller
{
    public function actionQuick(): Response
    {
        $request = Craft::$app->getRequest();
        $cookies = $request->getCookies();

        // Get current cookie info
        $cookieName = Plugin::getInstance()->getCarts()->cartCookie['name'];
        $cookie = $cookies->get($cookieName);
        $number = $cookie->value;

        // Set value
        $cookie->value = null;

        $newCart = Plugin::getInstance()->getCarts()->getCart();

        // Set new cart data
        // $newCart->setLineItems([
        //     // ...
        // ]);

        // Save new cart
        Craft::$app->getElements()->saveElement($newCart);

        // Forget cart
        Plugin::getInstance()->getCarts()->forgetCart();

        // Restore original cookie
        $cookie->value = $number;

        // This will be the original cart form the session
        $currentCart = Plugin::getInstance()->getCarts()->getCart();

        return $this->asSuccess('Success', [], '/shop');
    }
}

Hope this helps!

@samhibberd
Copy link
Author

Thanks @nfourtythree, will have a play now.

@nfourtythree
Copy link
Contributor

Hi @samhibberd

Just to keep you updated, I took a further look into this issue and have now pushed up a fix which we will look to get into a release of Commerce soon.

This fix updates the forgetCart() functionality to make it work inline with how you would expect. This means your code from your initial post will begin working.

Thanks!

@samhibberd
Copy link
Author

Thanks @nfourtythree, nice update.

lukeholder added a commit that referenced this issue Jan 10, 2024
…working-as-expected

Fixed #3353 `forgetCart()` not completely forgetting the cart
@nfourtythree
Copy link
Contributor

Commerce 4.4.0 has been released which includes this fix.

Thanks!

@samhibberd
Copy link
Author

Hi @nfourtythree, we finally had some time to update our craft install and test this fix properly, unfortunately it's still not working for us, unless we are doing something wrong?! In the example below it return the cart from the session and not the a newly created one as expected:

// No cart passed let's create and save a new one
// - If there is a cart in the current session we need to forget it whilst we get / create the quick checkout cart
$sessionCartNumber = $cartsService->getHasSessionCartNumber() ? $cartsService->getSessionCartNumber() : false;
if($sessionCartNumber) {
    $cartsService->forgetCart();
}
// - Create and save a new cart
$cart = $cartsService->getCart(forceSave: true);
// - Now forget the current session cart number again to remove the newly created cart from the session
$cartsService->forgetCart();
// - If there is one add the orginal cart back into the session
if ($sessionCartNumber) {
    $cartsService->setSessionCartNumber($sessionCartNumber);
}

Any thoughts??

@samhibberd
Copy link
Author

@nfourtythree any thought on this? Or do you think this could be a different issue?

@nfourtythree
Copy link
Contributor

nfourtythree commented Feb 27, 2024

Hi @samhibberd

Thank you for your reply. We have pushed another update to the carts service that should fix this issue. getHasSessionCartNumber() was only checking the private variables so the name of the method was slightly misleading. We have now updated it to make sure it checks if there is a cart cookie set with a value.

You will need to update your code as it looks like you were trying to call a protected method on the carts service. Here is my advised update:

$cartNumber = null;
if ($cartsService->getHasSessionCartNumber()) {
    $cartNumber = $cartsService->getCart()?->number;
    $cartsService->forgetCart();
}

// - Create and save a new cart
$cart = $cartsService->getCart(forceSave: true);

// - Now forget the current session cart number again to remove the newly created cart from the session
$cartsService->forgetCart();
// - If there is one add the orginal cart back into the session
if ($cartNumber) {
    $cartsService->setSessionCartNumber($cartNumber);
}

This fix will be included in the next release of Commerce.

To get this early, change your craftcms/commerce requirement in your project's composer.json to:

"require": {
  "craftcms/commerce": "dev-develop#04bbd83f8588a41295430a06e79d7a1765eb0bfe as 4.5.0",
  "...": "..."
}

Then run composer update.

Thanks!

@samhibberd
Copy link
Author

samhibberd commented Feb 27, 2024

Hey @nfourtythree great and thanks for fixing. Will test our side.

Thanks for reviewing our code, that was actually an untested example of what we are doing. We actually use custom behaviors in this case to get a session number only if it exists, which allows us to access the protected properties.

@lukeholder
Copy link
Member

4.5.1 is now out with a fix for this. Happy leap day!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug commerce4 Issues related to Commerce v4
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants