[CreditMemo] Fixes for reworking credit memos after PR review

UPGRADE.md

@@ -1,12 +1,12 @@
 ### UPGRADE FROM 1.0.0-RC.1 TO 1.0.0-RC.2
 
-1. `CreditMemoUnit` changed to `LineItem` which is a resource entity now
+1. `Sylius\RefundPlugin\Entity\CreditMemoUnit` was changed to `Sylius\RefundPlugin\Entity\LineItem` which is a resource entity now.
 
-2. `CreditMemoUnitGeneratorInterface` changed to `LineItemsConverterInterface` and moved from `Generator` to `Converter` directory
+2. `Sylius\RefundPlugin\Generator\CreditMemoUnitGeneratorInterface` was changed to `Sylius\RefundPlugin\Converter\LineItemsConverterInterface`.
 
-3. `OrderItemUnitCreditMemoUnitGenerator` changed to `LineItemsConverter` and moved from `Generator` to `Converter` directory
+3. `Sylius\RefundPlugin\Generator\OrderItemUnitCreditMemoUnitGenerator` was changed to `Sylius\RefundPlugin\Converter\LineItemsConverter`.
 
-4. `ShipmentCreditMemoUnitGenerator` changed to `ShipmentLineItemsConverter` and moved from `Generator` to `Converter` directory
+4. `Sylius\RefundPlugin\Generator\ShipmentCreditMemoUnitGenerator` was changed to `Sylius\RefundPlugin\Converter\ShipmentLineItemsConverter`.
 
 There are no migrations that provide backward compatibility, save current credit memos before upgrading the version of plugin. 
 

features/refunding_part_of_order_unit.feature

@@ -28,23 +28,23 @@ Feature: Refunding a part of an order unit
 
     @ui @application
     Scenario: Refunding the whole order unit price after partial refund
-        Given the 1st "Mr. Meeseeks T-Shirt" product from order "#00000022" already has a refund of "$5.00" with "Space money" payment
+        Given the 1st "Mr. Meeseeks T-Shirt" product from order "#00000022" has a refund of "$5.00" with "Space money" payment
         When I want to refund some units of order "#00000022"
         And I decide to refund 1st "Mr. Meeseeks T-Shirt" product with "Space money" payment
         Then I should be notified that selected order units have been successfully refunded
         And this order refunded total should be "$12.30"
 
     @ui @javascript
     Scenario: Refunding the whole order price after partial refund
-        Given the 1st "Mr. Meeseeks T-Shirt" product from order "#00000022" already has a refund of "$5.00" with "Space money" payment
+        Given the 1st "Mr. Meeseeks T-Shirt" product from order "#00000022" has a refund of "$5.00" with "Space money" payment
         When I want to refund some units of order "#00000022"
         And I decide to refund all units of this order with "Space money" payment
         Then I should be notified that selected order units have been successfully refunded
         And this order refunded total should be "$44.60"
 
     @ui @application
     Scenario: Not being able to refund more money than order unit total
-        Given the 1st "Mr. Meeseeks T-Shirt" product from order "#00000022" already has a refund of "$5.00" with "Space money" payment
+        Given the 1st "Mr. Meeseeks T-Shirt" product from order "#00000022" has a refund of "$5.00" with "Space money" payment
         When I want to refund some units of order "#00000022"
         And I decide to refund "$10.00" from 1st "Mr. Meeseeks T-Shirt" product with "Space money" payment
         Then I should be notified that I cannot refund more money than the order unit total

features/viewing_details_of_credit_memo.feature

@@ -39,7 +39,7 @@ Feature: Viewing details of a credit memo
 
     @ui @application
     Scenario: Viewing details of a credit memo issued for a partial refund
-        Given the 1st "PHP T-Shirt" product from order "#00000022" already has a refund of "$5.50" with "Space money" payment
+        Given the 1st "PHP T-Shirt" product from order "#00000022" has a refund of "$5.50" with "Space money" payment
         When I browse the details of the only credit memo generated for order "#00000022"
         Then it should have sequential number generated from current date
         And it should be issued in "United States" channel

spec/Entity/LineItemSpec.php

@@ -7,6 +7,7 @@
 use PhpSpec\ObjectBehavior;
 use Sylius\Component\Resource\Model\ResourceInterface;
 use Sylius\RefundPlugin\Entity\LineItemInterface;
+use Sylius\RefundPlugin\Exception\LineItemsCannotBeMerged;
 
 final class LineItemSpec extends ObjectBehavior
 {
@@ -36,4 +37,49 @@ function it_has_proper_line_item_data(): void
         $this->taxAmount()->shouldReturn(200);
         $this->taxRate()->shouldReturn('10%');
     }
+
+    function it_merges_with_another_line_item(LineItemInterface $newLineItem): void
+    {
+        $newLineItem->name()->willReturn('Mjolnir');
+        $newLineItem->quantity()->willReturn(1);
+        $newLineItem->unitNetPrice()->willReturn(1000);
+        $newLineItem->unitGrossPrice()->willReturn(1100);
+        $newLineItem->netValue()->willReturn(1000);
+        $newLineItem->grossValue()->willReturn(1100);
+        $newLineItem->taxAmount()->willReturn(100);
+        $newLineItem->taxRate()->willReturn('10%');
+
+        $this->merge($newLineItem);
+
+        $this->quantity()->shouldReturn(3);
+        $this->netValue()->shouldReturn(3000);
+        $this->grossValue()->shouldReturn(3300);
+        $this->taxAmount()->shouldReturn(300);
+    }
+
+    function it_throws_an_exception_if_another_line_item_is_different_during_merging(LineItemInterface $newLineItem): void
+    {
+        $newLineItem->name()->willReturn('Stormbreaker');
+        $newLineItem->unitNetPrice()->willReturn(1000);
+        $newLineItem->unitGrossPrice()->willReturn(1100);
+        $newLineItem->taxRate()->willReturn('10%');
+
+        $this->shouldThrow(LineItemsCannotBeMerged::class)->during('merge', [$newLineItem]);
+    }
+
+    function it_compares_with_another_line_item(LineItemInterface $theSameLineItem, LineItemInterface $differentLineItem): void
+    {
+        $theSameLineItem->name()->willReturn('Mjolnir');
+        $theSameLineItem->unitNetPrice()->willReturn(1000);
+        $theSameLineItem->unitGrossPrice()->willReturn(1100);
+        $theSameLineItem->taxRate()->willReturn('10%');
+
+        $differentLineItem->name()->willReturn('Stormbreaker');
+        $differentLineItem->unitNetPrice()->willReturn(1000);
+        $differentLineItem->unitGrossPrice()->willReturn(1100);
+        $differentLineItem->taxRate()->willReturn('10%');
+
+        $this->compare($theSameLineItem)->shouldReturn(true);
+        $this->compare($differentLineItem)->shouldReturn(false);
+    }
 }

src/Converter/LineItemsConverter.php

@@ -31,7 +31,7 @@ public function convert(array $units): Collection
 
         /** @var UnitRefundInterface $unitRefund */
         foreach ($units as $unitRefund) {
-            /** @var OrderItemUnitInterface $orderItemUnit */
+            /** @var OrderItemUnitInterface|null $orderItemUnit */
             $orderItemUnit = $this->orderItemUnitRepository->find($unitRefund->id());
             Assert::notNull($orderItemUnit);
             Assert::lessThanEq($unitRefund->total(), $orderItemUnit->getTotal());
@@ -70,12 +70,7 @@ private function addLineItem(LineItemInterface $newLineItem, Collection $lineIte
     {
         /** @var LineItemInterface $lineItem */
         foreach ($lineItems as $lineItem) {
-            if (
-                $lineItem->name() === $newLineItem->name() &&
-                $lineItem->unitNetPrice() === $newLineItem->unitNetPrice() &&
-                $lineItem->unitGrossPrice() === $newLineItem->unitGrossPrice() &&
-                $lineItem->taxRate() === $newLineItem->taxRate()
-            ) {
+            if ($lineItem->compare($newLineItem)) {
                 $lineItem->merge($newLineItem);
 
                 return $lineItems;
@@ -98,7 +93,7 @@ private function getTaxRate(Collection $taxAdjustments): ?string
 
         $label = $taxAdjustments->first()->getLabel();
 
-        if (preg_match('#\((.*?)\)#', $label, $matches)) {
+        if (preg_match('/\((.*?)\)/', $label, $matches)) {
             return end($matches);
         }
 

src/Converter/LineItemsConverterInterface.php

@@ -11,7 +11,7 @@
 interface LineItemsConverterInterface
 {
     /**
-     * @param array<UnitRefundInterface> $units
+     * @param UnitRefundInterface[] $units
      *
      * @return Collection|LineItemInterface[]
      */

src/Converter/ShipmentLineItemsConverter.php

@@ -28,7 +28,7 @@ public function convert(array $units): Collection
 
         /** @var UnitRefundInterface $unitRefund */
         foreach ($units as $unitRefund) {
-            /** @var AdjustmentInterface $shippingAdjustment */
+            /** @var AdjustmentInterface|null $shippingAdjustment */
             $shippingAdjustment = $this
                 ->adjustmentRepository
                 ->findOneBy(['id' => $unitRefund->id(), 'type' => AdjustmentInterface::SHIPPING_ADJUSTMENT])

src/Entity/LineItem.php

@@ -4,6 +4,8 @@
 
 namespace Sylius\RefundPlugin\Entity;
 
+use Sylius\RefundPlugin\Exception\LineItemsCannotBeMerged;
+
 /** @final */
 class LineItem implements LineItemInterface
 {
@@ -106,9 +108,27 @@ public function taxRate(): ?string
 
     public function merge(LineItemInterface $newLineItem): void
     {
+        if (!$this->compare($newLineItem)) {
+            throw LineItemsCannotBeMerged::occur();
+        }
+
         $this->quantity += $newLineItem->quantity();
         $this->netValue += $newLineItem->netValue();
         $this->grossValue += $newLineItem->grossValue();
         $this->taxAmount += $newLineItem->taxAmount();
     }
+
+    public function compare(LineItemInterface $lineItem): bool
+    {
+        if (
+            $this->name() === $lineItem->name() &&
+            $this->unitNetPrice() === $lineItem->unitNetPrice() &&
+            $this->unitGrossPrice() === $lineItem->unitGrossPrice() &&
+            $this->taxRate() === $lineItem->taxRate()
+        ) {
+            return true;
+        }
+
+        return false;
+    }
 }

src/Entity/LineItemInterface.php

@@ -27,4 +27,6 @@ public function taxAmount(): int;
     public function taxRate(): ?string;
 
     public function merge(self $newLineItem): void;
+
+    public function compare(self $lineItem): bool;
 }

src/Exception/LineItemsCannotBeMerged.php

@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sylius\RefundPlugin\Exception;
+
+final class LineItemsCannotBeMerged extends \InvalidArgumentException
+{
+    public static function occur(): self
+    {
+        return new self('Line items cannot be merged');
+    }
+}

src/Resources/config/doctrine/CreditMemo.orm.xml

@@ -27,7 +27,7 @@
 
         <many-to-many field="lineItems" target-entity="Sylius\RefundPlugin\Entity\LineItemInterface">
             <cascade>
-                <cascade-persist />
+                <cascade-all />
             </cascade>
             <join-table name="sylius_refund_credit_memo_line_items">
                 <join-columns>

src/Resources/views/Download/creditMemo.html.twig

@@ -17,6 +17,7 @@
         .credit-memo table tr.item td{ border-bottom: 1px solid #eee; }
         .credit-memo table tr.item.last td { border-bottom: none; }
         .credit-memo table tr.total td { border-top: 2px solid #eee; font-weight: bold; }
+        .credit-memo table tr.number td { text-align: center; font-size: 20px; font-weight: bold; padding-bottom: 40px; }
     </style>
 </head>
 
@@ -32,8 +33,7 @@
                         </td>
 
                         <td>
-                            Credit memo #{{ creditMemo.number }}<br>
-                            Issued at: {{ creditMemo.issuedAt|date('Y-m-d H:i:s') }}<br>
+                            {{ 'sylius_refund.ui.issued_at'|trans }}: {{ creditMemo.issuedAt|date('Y-m-d H:i:s') }}<br>
                         </td>
                     </tr>
                 </table>
@@ -72,6 +72,10 @@
         </tr>
         {% endif %}
 
+        <tr class="number">
+            <td colspan="9">{{ 'sylius_refund.ui.credit_memo'|trans }} #{{ creditMemo.number }}</td>
+        </tr>
+
         <tr class="heading">
             <td>{{ 'sylius_refund.ui.no'|trans }}</td>
             <td>{{ 'sylius.ui.name'|trans }}</td>
@@ -89,19 +93,19 @@
                 <td>{{ loop.index }}</td>
                 <td>{{ item.name }}</td>
                 <td>{{ item.quantity }}</td>
-                <td>{{ (item.unitNetPrice/100)|number_format(2) }}</td>
-                <td>{{ (item.netValue/100)|number_format(2) }}</td>
+                <td>{{ '%0.2f'|format(item.unitNetPrice/100) }}</td>
+                <td>{{ '%0.2f'|format(item.netValue/100) }}</td>
                 <td>{{ item.taxRate }}</td>
-                <td>{{ (item.taxAmount/100)|number_format(2) }}</td>
-                <td>{{ (item.grossValue/100)|number_format(2) }}</td>
+                <td>{{ '%0.2f'|format(item.taxAmount/100) }}</td>
+                <td>{{ '%0.2f'|format(item.grossValue/100) }}</td>
                 <td>{{ creditMemo.currencyCode }}</td>
             </tr>
         {% endfor %}
 
         <tr class="total">
             <td colspan="6"></td>
             <td>{{ 'sylius.ui.total'|trans }}:</td>
-            <td>{{ (creditMemo.total/100)|number_format(2) }}</td>
+            <td>{{ '%0.2f'|format(creditMemo.total/100) }}</td>
             <td>{{ creditMemo.currencyCode }}</td>
         </tr>
 
@@ -117,7 +121,7 @@
                 <tr class="tax-item">
                     <td colspan="6"></td>
                     <td>{{ item.label }}</td>
-                    <td>{{ (item.amount/100)|number_format(2) }}</td>
+                    <td>{{ '%0.2f'|format(item.amount/100) }}</td>
                     <td>{{ creditMemo.currencyCode }}</td>
                 </tr>
             {% endfor %}

src/Resources/views/Order/Admin/CreditMemo/details.html.twig

@@ -92,19 +92,19 @@
                             {{ item.quantity }}
                         </td>
                         <td class="right aligned line-item-unit-net-price">
-                            {{ (item.unitNetPrice/100)|number_format(2) }}
+                            {{ '%0.2f'|format(item.unitNetPrice/100) }}
                         </td>
                         <td class="right aligned line-item-net-value">
-                            {{ (item.netValue/100)|number_format(2) }}
+                            {{ '%0.2f'|format(item.netValue/100) }}
                         </td>
                         <td class="single line">
                             {{ item.taxRate }}
                         </td>
                         <td class="right aligned line-item-tax-amount">
-                            {{ (item.taxAmount/100)|number_format(2) }}
+                            {{ '%0.2f'|format(item.taxAmount/100) }}
                         </td>
                         <td class="right aligned line-item-gross-value">
-                            {{ (item.grossValue/100)|number_format(2) }}
+                            {{ '%0.2f'|format(item.grossValue/100) }}
                         </td>
                         <td class="single line line-item-currency-code">
                             {{ credit_memo.currencyCode }}
@@ -118,7 +118,7 @@
                             <strong>{{ 'sylius.ui.total'|trans }}</strong>:
                         </th>
                         <th id="credit-memo-total" class="right aligned">
-                            {{ (credit_memo.total/100)|number_format(2) }}
+                            {{ '%0.2f'|format(credit_memo.total/100) }}
                         </th>
                         <th id="credit-memo-total-currency-code" class="single line">
                             {{ credit_memo.currencyCode }}
@@ -130,7 +130,7 @@
                                 {{ item.label }}:
                             </th>
                             <th class="right aligned tax-item-amount">
-                                {{ (item.amount/100)|number_format(2) }}
+                                {{ '%0.2f'|format(item.amount/100) }}
                             </th>
                             <th class="single line tax-item-currency-code">
                                 {{ credit_memo.currencyCode }}

tests/Behat/Context/Setup/RefundingContext.php

@@ -58,9 +58,9 @@ public function productFromOrderHasAlreadyBeenRefunded(
     }
 
     /**
-     * @Given /^the (\d)(?:|st|nd|rd) "([^"]+)" product from order "#([^"]+)" already has a refund of ("[^"]+") with ("[^"]+" payment)$/
+     * @Given /^the (\d)(?:|st|nd|rd) "([^"]+)" product from order "#([^"]+)" has a refund of ("[^"]+") with ("[^"]+" payment)$/
      */
-    public function partOfProductFromOrderHasAlreadyBeenRefunded(
+    public function theProductFromOrderHasARefundOfWith(
         int $unitNumber,
         string $productName,
         string $orderNumber,

tests/Behat/Context/Transform/PriceContext.php

@@ -9,12 +9,10 @@
 final class PriceContext implements Context
 {
     /**
-     * @Transform /^"(\-)?((?:\d+\.)?\d+)"$/
+     * @Transform /^"(\-)?(\d+(?:\.\d{1,2})?)"$/
      */
     public function getPriceFromString(string $sign, string $price): int
     {
-        $this->validatePriceString($price);
-
         $price = (int) round((float) $price * 100, 2);
 
         if ('-' === $sign) {
@@ -23,12 +21,4 @@ public function getPriceFromString(string $sign, string $price): int
 
         return $price;
     }
-
-    /** @throws \InvalidArgumentException */
-    private function validatePriceString(string $price): void
-    {
-        if (!(bool) preg_match('/^\d+(?:\.\d{1,2})?$/', $price)) {
-            throw new \InvalidArgumentException('Price string should not have more than 2 decimal digits.');
-        }
-    }
 }