diff --git a/src/Message/AbstractRequest.php b/src/Message/AbstractRequest.php index c73c286..192cbb2 100644 --- a/src/Message/AbstractRequest.php +++ b/src/Message/AbstractRequest.php @@ -144,6 +144,46 @@ protected function createResponse($data) return $this->response = new Response($this, $data); } + /** + * Filters out any characters that SagePay does not support from the item name. + * + * Believe it or not, SagePay actually have separate rules for allowed characters + * for item names and discount names, hence the need for two separate methods. + * + * @param string $name + * + * @return string + */ + protected function filterItemName($name) + { + $standardChars = "0-9a-zA-Z"; + $allowedSpecialChars = " +'/\\&:,.-{}"; + $pattern = '`[^'.$standardChars.preg_quote($allowedSpecialChars, '/').']`'; + $name = trim(substr(preg_replace($pattern, '', $name), 0, 100)); + + return $name; + } + + /** + * Filters out any characters that SagePay does not support from the discount name. + * + * Believe it or not, SagePay actually have separate rules for allowed characters + * for item names and discount names, hence the need for two separate methods. + * + * @param string $name + * + * @return string + */ + protected function filterDiscountName($name) + { + $standardChars = "0-9a-zA-Z"; + $allowedSpecialChars = " +'/\\:,.-{};_@()^\"~[]$=!#?|"; + $pattern = '`[^'.$standardChars.preg_quote($allowedSpecialChars, '/').']`'; + $name = trim(substr(preg_replace($pattern, '', $name), 0, 100)); + + return $name; + } + /** * Get an XML representation of the current cart items * @@ -160,17 +200,15 @@ protected function getItemData() } $xml = new \SimpleXMLElement(''); + $cartHasDiscounts = false; foreach ($items as $basketItem) { if ($basketItem->getPrice() < 0) { - $discounts = $xml->addChild('discounts'); - $discount = $discounts->addChild('discount'); - $discount->addChild('fixed', $basketItem->getPrice() * -1); - $discount->addChild('description', $basketItem->getName()); + $cartHasDiscounts = true; } else { $total = ($basketItem->getQuantity() * $basketItem->getPrice()); $item = $xml->addChild('item'); - $item->addChild('description', $basketItem->getName()); + $item->description = $this->filterItemName($basketItem->getName()); $item->addChild('quantity', $basketItem->getQuantity()); $item->addChild('unitNetAmount', $basketItem->getPrice()); $item->addChild('unitTaxAmount', '0.00'); @@ -178,9 +216,17 @@ protected function getItemData() $item->addChild('totalGrossAmount', $total); } } - + if ($cartHasDiscounts) { + $discounts = $xml->addChild('discounts'); + foreach ($items as $discountItem) { + if ($discountItem->getPrice() < 0) { + $discount = $discounts->addChild('discount'); + $discount->addChild('fixed', ($discountItem->getPrice() * $discountItem->getQuantity()) * -1); + $discount->description = $this->filterDiscountName($discountItem->getName()); + } + } + } $xmlString = $xml->asXML(); - if ($xmlString) { $result = $xmlString; } diff --git a/tests/Message/DirectAuthorizeRequestTest.php b/tests/Message/DirectAuthorizeRequestTest.php index 2fc08b5..3198a86 100644 --- a/tests/Message/DirectAuthorizeRequestTest.php +++ b/tests/Message/DirectAuthorizeRequestTest.php @@ -6,6 +6,11 @@ class DirectAuthorizeRequestTest extends TestCase { + /** + * @var \Omnipay\Common\Message\AbstractRequest $request + */ + protected $request; + public function setUp() { parent::setUp(); @@ -60,6 +65,7 @@ public function testNoBasket() // Then with a basket containing no items. $items = new \Omnipay\Common\ItemBag(array()); + $this->request->setItems($items); $data = $this->request->getData(); $this->assertArrayNotHasKey('BasketXML', $data); } @@ -182,4 +188,64 @@ public function testGetDataNullBillingAddress2() // converted to an empty string. I'm not clear how that would be // tested. } + + public function testBasketWithNoDiscount() + { + $items = new \Omnipay\Common\ItemBag(array( + new \Omnipay\Common\Item(array( + 'name' => 'Name', + 'description' => 'Description', + 'quantity' => 1, + 'price' => 1.23, + )) + )); + + $this->request->setItems($items); + $data = $this->request->getData(); + // The element does exist, and must contain the basket XML, with optional XML header and + // trailing newlines. + $this->assertArrayHasKey('BasketXML', $data); + $this->assertNotContains('', $data['BasketXML']); + } + + public function testMixedBasketWithSpecialChars() + { + $items = new \Omnipay\Common\ItemBag(array( + new \Omnipay\Common\Item(array( + 'name' => "Denisé's Odd & Wierd £name? #12345678901234567890123456789012345678901234567890123456789012345678901234567890", + 'description' => 'Description', + 'quantity' => 2, + 'price' => 4.23, + )), + array( + 'name' => "Denisé's \"Odd\" & Wierd £discount? #", + 'description' => 'My Offer', + 'quantity' => 2, + 'price' => -0.10, + ), + array( + // 101 character name + 'name' => '12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901', + 'description' => 'My 2nd Offer', + 'quantity' => 1, + 'price' => -1.60, + ) + )); + + // Names/descriptions should be max 100 characters in length, once invalid characters have been removed. + $expected = '' + . 'Denis\'s Odd & Wierd name 1234567890123456789012345678901234567890123456789012345678901234567890123452' + . '4.230.00' + . '4.238.46' + . '' + . '0.2Denis\'s "Odd" Wierd discount? #' + . '1.61234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890' + . ''; + + $this->request->setItems($items); + $data = $this->request->getData(); + + $this->assertArrayHasKey('BasketXML', $data); + $this->assertContains($expected, $data['BasketXML'], 'Basket XML does not match the expected output'); + } }