Skip to content

Commit

Permalink
Merge pull request #2573 from dpfaffenbauer/issue/2572
Browse files Browse the repository at this point in the history
[Core] fix price calculation with immutables and adjustments
  • Loading branch information
dpfaffenbauer authored Mar 5, 2024
2 parents ef44b9c + 2b22cfd commit 6618d6c
Show file tree
Hide file tree
Showing 22 changed files with 203 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Feature: Adding a new cart item rule
And the cart item action has a condition amount with value "90" to "150"
And the cart item action has a action discount-percent with 10% discount
Then I refresh my cart
And the cart discount should be "-1000" excluding tax
And the cart item discount should be "-1000" excluding tax

Scenario: Add a new amount condition with is invalid cause of min value
Given adding a cart price rule named "amount"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ Feature: Adding a new cart rule
And the cart rule has a cart-item-action action
And the cart item action has a action discount with 20 in currency "EUR" off
And I apply the voucher code "asdf" to my cart
Then the cart discount should be "-2000" including tax
Then the cart item discount should be "-2000" including tax
Then the cart total should be "8000" including tax

Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ Feature: Adding a new cart rule
And the tax rule group has a tax rule for country "Austria" with tax rate "AT"
And the site has a product "Shoe" priced at 10000
And the product has the tax rule group "AT"
And I add the product "Shoe" to my cart
And the product is active and published and available for store "Austria"

Scenario: Add a new discount rule with 100 percent discount
Given adding a cart price rule named "discount-10"
And the cart rule is not a voucher rule
And the cart rule is active
And the cart rule has a action discount-percent with 100% discount
And I add the product "Shoe" to my cart
Given adding a cart price rule named "discount-100"
And the cart rule is a voucher rule with code "discount-100"
And the cart rule is active
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ Feature: Adding a new cart rule
And the cart rule has a cart-item-action action
And the cart item action has a action discount-percent with 10% discount
And I apply the voucher code "asdf" to my cart
Then the cart discount should be "-1000" including tax
Then the cart item discount should be "-1000" including tax
Then the cart total should be "9000" including tax
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ Feature: Adding a new cart rule
And the cart rule has a cart-item-action action
And the cart item action has a action discount-percent with 10% discount
And I apply the voucher code "asdf" to my cart
Then the cart discount should be "-1000" including tax
Then the cart item discount should be "-1000" including tax
Then the cart total should be "9000" including tax

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ Feature: Adding a new cart rule
And the cart rule has a cart-item-action action
And the cart item action has a action discount-percent with 10% discount
And I apply the voucher code "asdf" to my cart
Then the cart discount should be "-1200" including tax
Then the cart item discount should be "-1200" including tax
Then the cart total should be "10800" including tax

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
@ui @checkout
Feature: Ability to complete the checkout

Background:
Given the site operates on a store in "Austria"
And the store "Austria" is the default store
And the site has a product "TShirt" priced at 10000
And the product is active and published and available for store "Austria"
And I am a logged in customer
And the customer has an address with country "Austria", "4600", "Wels", "Freiung", "9-11/N3"
And the site has a carrier "Post" and ships for 10 in currency "EUR"
And the site has a payment provider "Bankwire" using factory "offline"
And adding a cart price rule named "Surcharge"
And the cart rule is active
And the cart rule is not a voucher rule
And the cart rule has a action surcharge with 100 in currency "EUR" off
Then I add this product to the cart

Scenario: I proceed to the checkout
Given I am at the address checkout step
Then I should be on the address checkout step
And I use the last address as invoice address
And I submit the address step
Then I should be on the shipping checkout step
When I submit the shipping step
Then I should be on the payment checkout step
And I select the payment provider "Bankwire"
When I submit the payment step
Then I should be on the summary checkout step
When I accept the checkout terms of service
And I submit the order
Then I should be on the thank you page
And The order total should be "€210.00"
47 changes: 47 additions & 0 deletions src/CoreShop/Behat/Context/Domain/CartContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use CoreShop\Component\Core\Model\OrderItemInterface;
use CoreShop\Component\Core\Model\ProductInterface;
use CoreShop\Component\Order\Context\CartContextInterface;
use CoreShop\Component\Order\Model\AdjustmentInterface;
use CoreShop\Component\Product\Model\ProductUnitInterface;
use CoreShop\Component\Taxation\Model\TaxItemInterface;
use Pimcore\Model\DataObject\Fieldcollection;
Expand Down Expand Up @@ -399,6 +400,29 @@ public function cartDiscountShouldBeIncludingTax($total): void
);
}

/**
* @Then /^the cart item discount should be "([^"]+)" including tax$/
*/
public function cartItemDiscountShouldBeIncludingTax($total): void
{
$cartItems = $this->cartContext->getCart()->getItems();
$discounts = 0;

foreach ($cartItems as $cartItem) {
$discounts += $cartItem->getAdjustmentsTotal(AdjustmentInterface::CART_PRICE_RULE, true);
}

Assert::eq(
$total,
$discounts,
sprintf(
'Cart discount is expected to be %s, but it is %s',
$total,
$discounts,
),
);
}

/**
* @Then /^the cart discount should be "([^"]+)" excluding tax$/
*/
Expand All @@ -415,6 +439,29 @@ public function cartDiscountShouldBeExcludingTax($total): void
);
}

/**
* @Then /^the cart item discount should be "([^"]+)" excluding tax$/
*/
public function cartItemDiscountShouldBeExcludingTax($total): void
{
$cartItems = $this->cartContext->getCart()->getItems();
$discounts = 0;

foreach ($cartItems as $cartItem) {
$discounts += $cartItem->getAdjustmentsTotal(AdjustmentInterface::CART_PRICE_RULE, false);
}

Assert::eq(
$total,
$discounts,
sprintf(
'Cart discount is expected to be %s, but it is %s',
$total,
$discounts,
),
);
}

/**
* @Then /^there should be no product in (my cart)$/
*/
Expand Down
1 change: 1 addition & 0 deletions src/CoreShop/Behat/Context/Setup/StoreContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ private function createStore(

$this->entityManager->flush();

$store->setIsDefault(true);
$store->setName($name);
$store->setCurrency($currency);
$store->setBaseCountry($country);
Expand Down
8 changes: 8 additions & 0 deletions src/CoreShop/Behat/Context/Ui/Frontend/CheckoutContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,14 @@ public function IShouldBeOnTheThankYouPage(): void
$this->thankYouPage->verify(['token' => $this->thankYouPage->getToken()]);
}

/**
* @When The order total should be :total
*/
public function theOrderTotalShouldBe(string $orderTotal): void
{
Assert::same($this->thankYouPage->getOrderTotal(), $orderTotal);
}

/**
* @When I re-capture payment for same order
*/
Expand Down
22 changes: 17 additions & 5 deletions src/CoreShop/Behat/Page/Frontend/Checkout/ThankYouPage.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,27 @@ public function getToken(): string
return $this->getElement('order_token')->getAttribute('data-test-order-token');
}

public function recapturePaymentForThisOrder(): void
{
$this->getSession()->visit($this->makePathAbsolute("en/shop/pay/{$this->getToken()}"));
}

public function getOrderTotal(): string
{
$orderTotalText = $this->getElement('order_total')->getText();

if (str_contains($orderTotalText, ',')) {
return strstr($orderTotalText, ',', true);
}

return trim($orderTotalText);
}

protected function getDefinedElements(): array
{
return array_merge(parent::getDefinedElements(), [
'order_token' => '[data-test-order-token]',
'order_total' => '[data-test-order-total]',
]);
}

public function recapturePaymentForThisOrder(): void
{
$this->getSession()->visit($this->makePathAbsolute("en/shop/pay/{$this->getToken()}"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,6 @@ interface ThankYouPageInterface extends FrontendPageInterface
{
public function getToken(): string;
public function recapturePaymentForThisOrder(): void;

public function getOrderTotal(): string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@

</td>
<td class="text-right">
-{{ currency.format(priceRule.getDiscount(true), order.currency.isoCode) }}
{{ currency.format(priceRule.getDiscount(true), order.currency.isoCode) }}
</td>
<td class="text-right">
-{{ currency.format(priceRule.getDiscount(false), order.currency.isoCode) }}
{{ currency.format(priceRule.getDiscount(false), order.currency.isoCode) }}
</td>
</tr>
{% endfor %}
Expand Down Expand Up @@ -226,7 +226,7 @@
<strong>{{ 'coreshop.ui.total'|trans }}:</strong>
</td>
<td colspan="1"
class="text-right cart-total-price">
class="text-right cart-total-price" {{ coreshop_test_html_attribute('order-total') }}>
{{ currency.format(order.convertedTotal, order.currency.isoCode) }}
</td>
</tr>
Expand Down
23 changes: 23 additions & 0 deletions src/CoreShop/Component/Core/Cart/Rule/Applier/CartRuleApplier.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ protected function apply(OrderInterface $cart, PriceRuleItemInterface $cartPrice
$totalAmount = [];
$totalDiscountPossible = 0;

$discountableItems = [];

foreach ($cart->getItems() as $item) {
if ($item->getTotal() <= 0) {
continue;
}

$discountableItems[] = $item;
}

if (count($discountableItems) === 0) {
return;
}

foreach ($cart->getItems() as $item) {
$totalAmount[] = $item->getTotal(false);
$totalDiscountPossible += $item->getTotal($withTax);
Expand Down Expand Up @@ -186,11 +200,20 @@ protected function apply(OrderInterface $cart, PriceRuleItemInterface $cartPrice
}
}

/*
* https://github.com/coreshop/CoreShop/issues/2572
*
* Since we are applying the discount to the cart,
* we add the adjustment to the cart-item as neutral.
*
* we need this adjustment so we know how much to refund or credit.
*/
$item->addAdjustment($this->adjustmentFactory->createWithData(
AdjustmentInterface::CART_PRICE_RULE,
$cartPriceRuleItem->getCartPriceRule()->getName(),
$positive ? $amountGross : (-1 * $amountGross),
$positive ? $amountNet : (-1 * $amountNet),
true
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,19 @@ protected function apply(

$discount = min($discount, $totalDiscountPossible);

/*
* Cart Item Rules are always applied to the cart-item, but as neutral adjustments
* So to know how much our "virtual" total is, we need to get these cart-price rules values
* otherwise, we might end up with a negative total
*/
if ($withTax) {
$discount = min($discount, $orderItem->getTotal());
$total = $orderItem->getTotal() + $orderItem->getNeutralAdjustmentsTotal(AdjustmentInterface::CART_PRICE_RULE);
} else {
$discount = min($discount, $orderItem->getTotal(false));
$total = $orderItem->getTotal(false) + $orderItem->getNeutralAdjustmentsTotal(AdjustmentInterface::CART_PRICE_RULE, false);
}

$discount = min($discount, $total);

if (0 === $discount) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ public function process(OrderInterface $cart): void
$itemDiscount,
$context
);

/*
* https://github.com/coreshop/CoreShop/issues/2572
*
* Since we are recalculating the items-total, we also need to respect the adjustments and
* re-add them to the item total as well.
*/
$item->recalculateAdjustmentsTotal();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,7 @@ final class CartSubtotalProcessor implements CartProcessorInterface
{
public function process(OrderInterface $cart): void
{
$subtotalGross = 0;
$subtotalNet = 0;

/**
* @var OrderItemInterface $item
*/
foreach ($cart->getItems() as $item) {
$subtotalGross += $item->getTotal(true);
$subtotalNet += $item->getTotal(false);
}

$cart->setSubtotal($subtotalGross, true);
$cart->setSubtotal($subtotalNet, false);

$cart->recalculateSubtotal();
$cart->recalculateAdjustmentsTotal();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public function applyRule(OrderInterface $cart, array $configuration, PriceRuleI
$cartPriceRuleItem->getCartPriceRule()->getName(),
$cartPriceRuleItem->getDiscount(true),
$cartPriceRuleItem->getDiscount(false),
true
),
);

Expand Down
2 changes: 2 additions & 0 deletions src/CoreShop/Component/Order/Model/AdjustableInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public function removeAdjustment(AdjustmentInterface $adjustment);

public function getAdjustmentsTotal(?string $type = null, bool $withTax = true): int;

public function getNeutralAdjustmentsTotal(?string $type = null, bool $withTax = true): int;

public function removeAdjustments(string $type = null);

public function removeAdjustmentsRecursively(string $type = null);
Expand Down
20 changes: 20 additions & 0 deletions src/CoreShop/Component/Order/Model/AdjustableTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,26 @@ public function getAdjustmentsTotal(?string $type = null, bool $withTax = true):
return $total;
}

public function getNeutralAdjustmentsTotal(?string $type = null, bool $withTax = true): int
{
if (null === $type) {
if ($withTax) {
return $this->getPimcoreAdjustmentTotalGross() ?: 0;
}

return $this->getPimcoreAdjustmentTotalNet() ?: 0;
}

$total = 0;
foreach ($this->getAdjustments($type) as $adjustment) {
if ($adjustment->getNeutral()) {
$total += $adjustment->getAmount($withTax);
}
}

return $total;
}

public function recalculateAdjustmentsTotal()
{
$adjustmentsTotalGross = 0;
Expand Down
Loading

0 comments on commit 6618d6c

Please sign in to comment.