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

[Cart] fix existing cart initalization on customer login #1779

Merged
merged 2 commits into from
Nov 15, 2021

Conversation

albertmueller
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no

specially on findLatestByStoreAndCustomer it can happen that more then one cart exists for the customer, when the user has logged in, he has been shown an empty cart. although several carts exist, the last cart should be loaded.

@solverat
Copy link
Contributor

also see #1699

@@ -73,6 +75,7 @@ public function findCartById($id): ?OrderInterface
{
$list = $this->getList();
$list->setCondition('o_id = ? AND order__id is null ', [$id]);
$list->setLimit(1);
Copy link
Member

Choose a reason for hiding this comment

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

that should always ever return one or null since we directly filter on o_id.

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, but as far as i know, the query is faster with a limit, even if you won't notice it in reality :)

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure, it feels wrong here, so please remove it

@@ -41,6 +41,7 @@ public function findNamedForCustomer(CustomerInterface $customer, $name): ?Order
{
$list = $this->getList();
$list->setCondition('customer__id = ? AND name = ? AND order__id is null', [$customer->getId(), $name]);
$list->setLimit(1);
Copy link
Member

Choose a reason for hiding this comment

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

is not right here, named can return multiple carts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm, but then is the following code below wrong:
if (count($objects) === 1 && $objects[0] instanceof OrderInterface) { return $objects[0]; }
because only one cart will returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this function somewhere in use @dpfaffenbauer? i can't find a usage in coreshop, if not then how about to return the found collection of carts?

Copy link
Member

Choose a reason for hiding this comment

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

true, it always ever return if it is one. We don't have any usage of it CoreShop. I don't think it even works... So we should remove it with 3.0

@@ -58,6 +59,7 @@ public function findLatestByStoreAndCustomer(StoreInterface $store, CustomerInte
$list->setCondition('customer__id = ? AND store = ? AND order__id is null ', [$customer->getId(), $store->getId()]);
$list->setOrderKey('o_creationDate');
$list->setOrder('DESC');
$list->setLimit(1);
Copy link
Member

Choose a reason for hiding this comment

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

that is right

@dpfaffenbauer dpfaffenbauer merged commit 2cf7908 into coreshop:master Nov 15, 2021
@dpfaffenbauer
Copy link
Member

thanks

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.

3 participants