From 2bb8f341cd47ae85c2ee6be01284a0cc0c092143 Mon Sep 17 00:00:00 2001
From: juhopekkavuorela <juho-pekka.vuorela@paytrail.com>
Date: Wed, 19 Apr 2023 13:09:21 +0300
Subject: [PATCH] Improve refundRequest Validation

---
 src/Request/RefundRequest.php            |  13 +--
 tests/Request/EmailRefundRequestTest.php | 127 +++++------------------
 2 files changed, 31 insertions(+), 109 deletions(-)

diff --git a/src/Request/RefundRequest.php b/src/Request/RefundRequest.php
index 5ac8af9..25fc479 100644
--- a/src/Request/RefundRequest.php
+++ b/src/Request/RefundRequest.php
@@ -31,8 +31,8 @@ public function validate()
     {
         $props = get_object_vars($this);
 
-        if (! empty($this->items)) {
-            // Count the total amount of the payment.
+        if (!empty($this->items)) {
+            // Count the total amount of the refund.
             $items_total = array_reduce($this->items, function ($carry = 0, ?RefundItem $item = null) {
                 if ($item === null) {
                     return $carry;
@@ -49,12 +49,8 @@ public function validate()
             $items_total = $this->amount;
         }
 
-        if (empty($props['amount'])) {
-            throw new ValidationException('Amount can not be empty');
-        }
-
-        if (filter_var($props['amount'], FILTER_VALIDATE_INT) === false) {
-            throw new ValidationException('Amount is not an integer');
+        if (empty($props['amount']) && empty($this->items)) {
+            throw new ValidationException('Amount can not be empty when making refund without items');
         }
 
         if ($items_total !== $props['amount']) {
@@ -65,6 +61,7 @@ public function validate()
             throw new ValidationException('CallbackUrls are not set');
         }
 
+        // Validate callbackUrls
         $this->callbackUrls->validate();
 
         return true;
diff --git a/tests/Request/EmailRefundRequestTest.php b/tests/Request/EmailRefundRequestTest.php
index d72d8f7..79d6663 100644
--- a/tests/Request/EmailRefundRequestTest.php
+++ b/tests/Request/EmailRefundRequestTest.php
@@ -12,11 +12,11 @@
 
 class EmailRefundRequestTest extends TestCase
 {
-    public function testEmailRefundRequest()
+    public function testEmailRefundRequestIsValid()
     {
-        $er = new EmailRefundRequest();
-        $er->setAmount(20);
-        $er->setEmail('some@email.com');
+        $emailRequest = new EmailRefundRequest();
+        $emailRequest->setAmount(20);
+        $emailRequest->setEmail('some@email.com');
 
         $item = new RefundItem();
         $item->setAmount(10)
@@ -26,115 +26,40 @@ public function testEmailRefundRequest()
         $item2->setAmount(10)
             ->setStamp('anotherStamp');
 
-        $er->setItems([$item, $item2]);
+        $emailRequest->setItems([$item, $item2]);
 
         $cb = new CallbackUrl();
         $cb->setCancel('https://some.url.com/cancel')
             ->setSuccess('https://some.url.com/success');
 
-        $er->setCallbackUrls($cb);
+        $emailRequest->setCallbackUrls($cb);
 
-        $er->setRefundReference('ref-1234')
+        $emailRequest->setRefundReference('ref-1234')
             ->setRefundStamp('c7557cd5d5f548daa5332ccc4abb264f');
 
-        $this->assertEquals(true, $er->validate());
+        $this->assertTrue($emailRequest->validate());
     }
 
-    public function testExceptions()
+    public function testRefundRequestWithoutItemsAndAmountThrowsException()
     {
-        $er = new EmailRefundRequest();
-
-        try {
-            $er->validate();
-        } catch (ValidationException $e) {
-            $this->assertEquals("Amount can not be empty", $e->getMessage());
-        }
-
-        $er->setAmount(99999);
-
-        try {
-            $er->validate();
-        } catch (ValidationException $e) {
-            $this->assertEquals('CallbackUrls are not set', $e->getMessage());
-        }
-
-        $cb = new CallbackUrl();
-        $er->setCallbackUrls($cb);
-
-        try {
-            $er->validate();
-        } catch (ValidationException $e) {
-            $this->assertEquals('Success is empty', $e->getMessage());
-        }
-
-        $cb->setSuccess('someurl.somewhere.com/success');
-        $er->setCallbackUrls($cb);
-
-        try {
-            $er->validate();
-        } catch (ValidationException $e) {
-            $this->assertEquals('Cancel is empty', $e->getMessage());
-        }
-
-        $cb->setCancel('someurl.somewhere.com/cancel');
-        $er->setCallbackUrls($cb);
-
-        try {
-            $er->validate();
-        } catch (ValidationException $e) {
-            $this->assertEquals('Success is not a valid URL', $e->getMessage());
-        }
-
-        $cb->setSuccess('https://someurl.somewhere.com/success');
-        $er->setCallbackUrls($cb);
-
-        try {
-            $er->validate();
-        } catch (ValidationException $e) {
-            $this->assertEquals('Cancel is not a valid URL', $e->getMessage());
-        }
-
-        $cb->setCancel('https://someurl.somewhere.com/cancel');
-        $er->setCallbackUrls($cb);
-
-        try {
-            $er->validate();
-        } catch (ValidationException $e) {
-            $this->assertEquals('email can not be empty', $e->getMessage());
-        }
-
-        $er->setEmail('some@email.com');
-
-        // Items are not mandatory, so should pass from here
-        try {
-            $er->validate();
-        } catch (ValidationException $e) {
-            var_dump($e->getMessage());
-        }
-
-        $item = new RefundItem();
-        $item->setAmount(110)
-            ->setStamp('someStamp');
-
-        $item2 = new RefundItem();
-        $item2->setAmount(10)
-            ->setStamp('anotherStamp');
-
-        $er->setItems([$item, $item2]);
-
-        // Fails, as refund->total was set to 9999
-        try {
-            $er->validate();
-        } catch (ValidationException $e) {
-            $this->assertEquals('ItemsTotal does not match Amount', $e->getMessage());
-        }
+        $this->expectException(ValidationException::class);
+        (new EmailRefundRequest())->validate();
+    }
 
-        // Set correct amount
-        $er->setAmount(120);
+    public function testRefundWithItemsAndAmountMismatchThrowsException()
+    {
+        $this->expectException(ValidationException::class);
+        (new EmailRefundRequest())->setAmount(100)
+            ->setItems([
+                (new RefundItem())->setAmount(200)
+                    ->setStamp('foobar')
+            ])->validate();
+    }
 
-        try {
-            $this->assertEquals(true, $er->validate());
-        } catch (ValidationException $e) {
-        }
+    public function testRefundRequestWithoutCallbackUrlsThrowsError()
+    {
+        $this->expectException(ValidationException::class);
+        (new EmailRefundRequest())->setAmount(100)
+            ->validate();
     }
 }