Fixes #17 Card number should be numeric. Do strip non numeric charact…

…ers from card number, allow only spaces
laravel-validation-rules · Dec 20, 2018 · 365ea79 · 365ea79
1 parent ed9a3b8
commit 365ea79
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 25 deletions.
diff --git a/src/Cards/Card.php b/src/Cards/Card.php
@@ -9,6 +9,7 @@
 use LVR\CreditCard\Exceptions\CreditCardLengthException;
 use LVR\CreditCard\Exceptions\CreditCardPatternException;
 use LVR\CreditCard\Exceptions\CreditCardChecksumException;
+use LVR\CreditCard\Exceptions\CreditCardCharactersException;
 
 abstract class Card
 {
@@ -90,9 +91,9 @@ public function __construct(string $card_number = '')
      */
     public function setCardNumber(string $card_number)
     {
-        $card_number = preg_replace('/[^0-9]/', '', $card_number);
+        $this->card_number = preg_replace('/\s+/', '', $card_number);
 
-        $this->card_number = $card_number;
+        $this->isValidCardNumber();
 
         if (! $this->validPattern()) {
             throw new CreditCardPatternException(
@@ -106,6 +107,7 @@ public function setCardNumber(string $card_number)
     /**
      * @return bool
      * @throws \LVR\CreditCard\Exceptions\CreditCardChecksumException
+     * @throws \LVR\CreditCard\Exceptions\CreditCardCharactersException
      * @throws \LVR\CreditCard\Exceptions\CreditCardException
      * @throws \LVR\CreditCard\Exceptions\CreditCardLengthException
      */
@@ -115,6 +117,12 @@ public function isValidCardNumber()
             throw new CreditCardException('Card number is not set');
         }
 
+        if (! is_numeric(preg_replace('/\s+/', '', $this->card_number))) {
+            throw new CreditCardCharactersException(
+                sprintf('Card number "%s" contains invalid characters', $this->card_number)
+            );
+        }
+
         if (! $this->validLength()) {
             throw new CreditCardLengthException(
                 sprintf('Incorrect "%s" card length', $this->card_number)

diff --git a/src/Exceptions/CreditCardCharactersException.php b/src/Exceptions/CreditCardCharactersException.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace LVR\CreditCard\Exceptions;
+
+class CreditCardCharactersException extends CreditCardException
+{
+}
diff --git a/src/ExpirationDateValidator.php b/src/ExpirationDateValidator.php
@@ -28,24 +28,6 @@ public function __construct(string $year, string $month)
         $this->month = trim($month);
     }
 
-    /**
-     * @param $year
-     *
-     * @return $this
-     */
-    protected function setYear($year)
-    {
-        $year = trim($year);
-
-        if (strlen($year) == 2) {
-            $year = substr(date('Y'), 0, 2).$year;
-        }
-
-        $this->year = $year;
-
-        return $this;
-    }
-
     /**
      * @param string $year
      * @param string $month
@@ -67,6 +49,24 @@ public function isValid()
             && $this->isFeatureDate();
     }
 
+    /**
+     * @param $year
+     *
+     * @return $this
+     */
+    protected function setYear($year)
+    {
+        $year = trim($year);
+
+        if (strlen($year) == 2) {
+            $year = substr(date('Y'), 0, 2).$year;
+        }
+
+        $this->year = $year;
+
+        return $this;
+    }
+
     /**
      * @return string
      */

diff --git a/tests/Unit/CardExpirationTest.php b/tests/Unit/CardExpirationTest.php
@@ -164,7 +164,7 @@ protected function yearValidator(string $year)
      */
     protected function monthValidator(string $month, ?int $year = null)
     {
-        $year = $year ?? Carbon::now()->addYear()->year;
+        $year = $year ? $year : Carbon::now()->addYear()->year;
 
         return Validator::make(
             [

diff --git a/tests/Unit/CardTest.php b/tests/Unit/CardTest.php
@@ -0,0 +1,50 @@
+<?php
+
+namespace LVR\CreditCard\Tests\Unit;
+
+use LVR\CreditCard\Factory;
+use LVR\CreditCard\Cards\Card;
+use LVR\CreditCard\Tests\TestCase;
+use LVR\CreditCard\Exceptions\CreditCardException;
+use LVR\CreditCard\Exceptions\CreditCardCharactersException;
+
+class CardTest extends TestCase
+{
+    /** @test **/
+    public function it_expects_card_number()
+    {
+        $this->expectException(CreditCardException::class);
+
+        Factory::makeFromNumber('');
+    }
+
+    /** @test **/
+    public function it_allows_spaces_in_card_numbers()
+    {
+        $this->assertInstanceOf(Card::class, Factory::makeFromNumber('4111 1111 1111 1111'));
+    }
+
+    /** @test **/
+    public function it_checks_if_all_card_number_characters_are_numeric()
+    {
+        $this->expectException(CreditCardCharactersException::class);
+
+        Factory::makeFromNumber('4111111x111111sss111');
+    }
+
+    /** @test **/
+    public function it_returns_card_name()
+    {
+        $card = Factory::makeFromNumber('4111 1111 1111 1111');
+
+        $this->assertEquals('visa', $card->name());
+    }
+
+    /** @test **/
+    public function it_returns_card_brand()
+    {
+        $card = Factory::makeFromNumber('4111 1111 1111 1111');
+
+        $this->assertEquals('Visa', $card->brand());
+    }
+}
diff --git a/tests/Unit/Cards/BaseCardTests.php b/tests/Unit/Cards/BaseCardTests.php
@@ -8,6 +8,10 @@
 use Illuminate\Support\Facades\Validator;
 use LVR\CreditCard\Exceptions\CreditCardException;
 use Illuminate\Foundation\Http\Middleware\TrimStrings;
+use LVR\CreditCard\Exceptions\CreditCardLengthException;
+use LVR\CreditCard\Exceptions\CreditCardPatternException;
+use LVR\CreditCard\Exceptions\CreditCardChecksumException;
+use LVR\CreditCard\Exceptions\CreditCardCharactersException;
 use Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull;
 
 abstract class BaseCardTests extends TestCase
@@ -76,7 +80,7 @@ public function it_recognises_valid_card_numbers()
                     ['card_number' => $number],
                     ['card_number' => new CardNumber]
                 )->passes(),
-                sprintf('The number: "%s" is recognised as invalid but should be valid', $number)
+                sprintf('The number: "%s" is recognized as invalid but should be valid', $number)
             );
         });
     }
@@ -95,7 +99,7 @@ public function it_recognises_invalid_card_numbers()
                     ['card_number' => $number],
                     ['card_number' => [new CardNumber]]
                 )->fails(),
-                sprintf('The number: "%s" is recognised as valid but should be invalid', $number)
+                sprintf('The number: "%s" is recognized as valid but should be invalid', $number)
             );
         });
     }
@@ -114,7 +118,7 @@ public function it_checks_number_length()
             $this->assertEquals(
                 CardNumber::MSG_CARD_LENGTH_INVALID,
                 $validator->messages()->first(),
-                sprintf('The number: "%s" is not recognised as invalid length', $number)
+                sprintf('The number: "%s" is not recognized as invalid length', $number)
             );
         });
     }
@@ -157,8 +161,26 @@ public function it_should_not_match_other_cards()
                                 false,
                                 sprintf('%s cards ("%s") pattern matches %s card.', $card, $number, $this->instance)
                             );
+                        } catch (CreditCardPatternException $ex) {
+                            $this->assertTrue(
+                                $ex->getMessage() === sprintf('Wrong "%s" card pattern', $number)
+                            );
+                        } catch (CreditCardLengthException $ex) {
+                            $this->assertTrue(
+                                $ex->getMessage() === sprintf('Incorrect "%s" card length', $number)
+                            );
+                        } catch (CreditCardChecksumException $ex) {
+                            $this->assertTrue(
+                                $ex->getMessage() === sprintf('Invalid card number: "%s". Checksum is wrong', $number)
+                            );
+                        } catch (CreditCardCharactersException $ex) {
+                            $this->assertTrue(
+                                $ex->getMessage() === sprintf('Card number "%s" contains invalid characters', $number)
+                            );
                         } catch (CreditCardException $ex) {
-                            $this->assertTrue($ex->getMessage() === sprintf('Wrong "%s" card pattern', $number));
+                            $this->assertTrue(
+                                $ex->getMessage() === 'Card number is not set'
+                            );
                         }
                     }
                 });