Optimized a validator. [skip ci]

app/Repositories/Account/AccountRepository.php

@@ -65,7 +65,7 @@ class AccountRepository implements AccountRepositoryInterface
     public function getAccounts(array $types)
     {
         $result = Auth::user()->accounts()->with(
-            ['accountmeta' => function(HasMany $query) {
+            ['accountmeta' => function (HasMany $query) {
                 $query->where('name', 'accountRole');
             }]
         )->accountTypeIn($types)->orderBy('accounts.name', 'ASC')->get(['accounts.*'])->sortBy('name');
@@ -242,7 +242,7 @@ class AccountRepository implements AccountRepositoryInterface
         }
 
         $accounts->each(
-            function(Account $account) use ($start, $end) {
+            function (Account $account) use ($start, $end) {
                 $account->startBalance = Steam::balance($account, $start, true);
                 $account->endBalance   = Steam::balance($account, $end, true);
                 $account->piggyBalance = 0;
@@ -282,7 +282,7 @@ class AccountRepository implements AccountRepositoryInterface
         $end      = clone Session::get('end', new Carbon);
 
         $accounts->each(
-            function(Account $account) use ($start, $end) {
+            function (Account $account) use ($start, $end) {
                 $account->startBalance = Steam::balance($account, $start);
                 $account->endBalance   = Steam::balance($account, $end);
 
@@ -321,7 +321,7 @@ class AccountRepository implements AccountRepositoryInterface
     public function getTransfersInRange(Account $account, Carbon $start, Carbon $end)
     {
         $set      = TransactionJournal::whereIn(
-            'id', function(Builder $q) use ($account, $start, $end) {
+            'id', function (Builder $q) use ($account, $start, $end) {
             $q->select('transaction_journals.id')
               ->from('transactions')
               ->leftJoin('transaction_journals', 'transaction_journals.id', '=', 'transactions.transaction_journal_id')
@@ -335,7 +335,7 @@ class AccountRepository implements AccountRepositoryInterface
         }
         )->get();
         $filtered = $set->filter(
-            function(TransactionJournal $journal) use ($account) {
+            function (TransactionJournal $journal) use ($account) {
                 if ($journal->destination_account->id == $account->id) {
                     return $journal;
                 }

app/Validation/FireflyValidator.php

@@ -9,9 +9,8 @@ use Crypt;
 use DB;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
-use Illuminate\Contracts\Encryption\DecryptException;
+use FireflyIII\User;
 use Illuminate\Validation\Validator;
-use Log;
 use Navigation;
 use Symfony\Component\Translation\TranslatorInterface;
 
@@ -93,54 +92,129 @@ class FireflyValidator extends Validator
      */
     public function validateUniqueAccountForUser($attribute, $value, $parameters)
     {
-        $type = null;
+        // because a user does not have to be logged in (tests and what-not).
-
+        if (!Auth::check()) {
-        /**
+            return $this->validateAccountAnonymously();
-         * Switch on different cases on which this method can respond:
-         */
-        $hasWhat          = isset($this->data['what']);
-        $hasAccountTypeId = isset($this->data['account_type_id']) && isset($this->data['name']);
-        $hasAccountId     = isset($this->data['id']);
-        $ignoreId         = 0;
-
-
-        if ($hasWhat) {
-            $search = Config::get('firefly.accountTypeByIdentifier.' . $this->data['what']);
-            $type   = AccountType::whereType($search)->first();
-            // this field can be used to find the exact type, and continue.
         }
 
-        if ($hasAccountTypeId) {
+        if (isset($this->data['what'])) {
+            return $this->validateByAccountTypeString($value, $parameters);
+        }
+
+        if (isset($this->data['account_type_id'])) {
+            return $this->validateByAccountTypeId($value, $parameters);
+        }
+
+
+        var_dump($attribute);
+        var_dump($value);
+        var_dump($parameters);
+        var_dump($this->data);
+
+        exit;
+
+
+        // try to determin type of account:
+        if (!empty($this->data['what'])) {
+            $search = Config::get('firefly.accountTypeByIdentifier.' . $this->data['what']);
+            $type   = AccountType::whereType($search)->first();
+        } else {
             $type = AccountType::find($this->data['account_type_id']);
         }
 
-        if ($hasAccountId) {
+        // ignore itself, if parameter is given:
-            /** @var Account $account */
+        if (isset($parameters[0])) {
-            $account  = Account::find($this->data['id']);
+            $ignoreId = $parameters[0];
+        } else {
+            $ignoreId = 0;
+        }
+
+        // reset to basic check, see what happens:
+        $userId   = Auth::user()->id;
         $ignoreId = intval($this->data['id']);
-            $type     = AccountType::find($account->account_type_id);
+
-            unset($account);
+        $set = Account::where('account_type_id', $type->id)->where('id', '!=', $ignoreId)->where('user_id', $userId)->get();
+        /** @var Account $entry */
+        foreach ($set as $entry) {
+            if ($entry->name == $value) {
+                return false;
+            }
+        }
+
+        return true;
+
     }
 
     /**
-         * Try to decrypt data just in case:
+     * @return bool
      */
-        try {
+    protected function validateAccountAnonymously()
-            $value = Crypt::decrypt($value);
+    {
-        } catch (DecryptException $e) {
+        if (!isset($this->data['user_id'])) {
-            // if it fails, probably not encrypted.
-        }
-
-
-        if (is_null($type)) {
-            Log::error('Could not determine type of account to validate.');
-
             return false;
         }
 
-        // get all accounts with this type, and find the name.
+        $user  = User::find($this->data['user_id']);
-        $userId = Auth::check() ? Auth::user()->id : 0;
+        $type  = AccountType::find($this->data['account_type_id'])->first();
-        $set    = Account::where('account_type_id', $type->id)->where('id', '!=', $ignoreId)->where('user_id', $userId)->get();
+        $value = $this->data['name'];
+
+        // decrypt if necessary
+        if (intval($this->data['encrypted']) === 1) {
+            $value = Crypt::decrypt($this->data['name']);
+        }
+
+
+        $set = $user->accounts()->where('account_type_id', $type->id)->get();
+        /** @var Account $entry */
+        foreach ($set as $entry) {
+            if ($entry->name == $value) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    /**
+     * @param $value
+     * @param $parameters
+     *
+     * @return bool
+     */
+    protected function validateByAccountTypeString($value, $parameters)
+    {
+        $search = Config::get('firefly.accountTypeByIdentifier.' . $this->data['what']);
+        $type   = AccountType::whereType($search)->first();
+        $ignore = isset($parameters[0]) ? intval($parameters[0]) : 0;
+
+        $set = Auth::user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)->get();
+        /** @var Account $entry */
+        foreach ($set as $entry) {
+            if ($entry->name == $value) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    /**
+     * @param $value
+     * @param $parameters
+     *
+     * @return bool
+     */
+    protected function validateByAccountTypeId($value, $parameters)
+    {
+        $type   = AccountType::find($this->data['account_type_id'])->first();
+        $ignore = isset($parameters[0]) ? intval($parameters[0]) : 0;
+
+        // if is encrypted, decrypt:
+        if (intval($this->data['encrypted']) === 1) {
+            $value = Crypt::decrypt($value);
+        }
+
+        $set = Auth::user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)->get();
         /** @var Account $entry */
         foreach ($set as $entry) {
             if ($entry->name == $value) {

tests/controllers/AccountControllerTest.php

@@ -21,6 +21,7 @@ class AccountControllerTest extends TestCase
     public function setUp()
     {
         parent::setUp();
+
         $this->createAccount();
     }
 
@@ -49,8 +50,12 @@ class AccountControllerTest extends TestCase
      */
     public function createAccount()
     {
+        $user = FactoryMuffin::create('FireflyIII\User');
+        $this->be($user);
         if (is_null($this->account)) {
             $this->account          = FactoryMuffin::create('FireflyIII\Models\Account');
+            $this->account->user_id = $user->id;
+            $this->account->save();
         }
     }
 

tests/repositories/AccountRepositoryTest.php

@@ -603,6 +603,8 @@ class AccountRepositoryTest extends TestCase
     }
 
     /**
+     * This function should give a big fat error, but it doesnt.
+     *
      * @covers FireflyIII\Repositories\Account\AccountRepository::store
      * @covers FireflyIII\Repositories\Account\AccountRepository::storeAccount
      * @covers FireflyIII\Repositories\Account\AccountRepository::storeMetadata
@@ -610,12 +612,12 @@ class AccountRepositoryTest extends TestCase
      */
     public function testStoreWithExistingAccount()
     {
-        $account = FactoryMuffin::create('FireflyIII\Models\Account');
+        $account = FactoryMuffin::create('FireflyIII\Models\Account'); // expense
-        FactoryMuffin::create('FireflyIII\Models\AccountType');
+        FactoryMuffin::create('FireflyIII\Models\AccountType'); // revenue
-        FactoryMuffin::create('FireflyIII\Models\AccountType');
+        FactoryMuffin::create('FireflyIII\Models\AccountType'); // asset
-        FactoryMuffin::create('FireflyIII\Models\TransactionType');
+        FactoryMuffin::create('FireflyIII\Models\TransactionType'); // withdrawal
-        FactoryMuffin::create('FireflyIII\Models\TransactionType');
+        FactoryMuffin::create('FireflyIII\Models\TransactionType'); // deposit
-        FactoryMuffin::create('FireflyIII\Models\TransactionType');
+        FactoryMuffin::create('FireflyIII\Models\TransactionType'); // transfer
         $currency = FactoryMuffin::create('FireflyIII\Models\TransactionCurrency');
         $this->be($account->user);
 
@@ -632,7 +634,6 @@ class AccountRepositoryTest extends TestCase
             'openingBalanceDate'     => '2015-01-01',
         ];
 
-
         $newAccount = $this->object->store($data);
 
         $this->assertEquals($account->name, $newAccount->name);