Improve IBAN uniqueness.

This commit is contained in:
James Cole
2018-03-19 08:16:54 +01:00
parent 3fdb782321
commit 6ab03bb228
2 changed files with 64 additions and 13 deletions

View File

@@ -75,7 +75,7 @@ class AccountFormRequest extends Request
'name' => 'required|min:1|uniqueAccountForUser', 'name' => 'required|min:1|uniqueAccountForUser',
'openingBalance' => 'numeric|required_with:openingBalanceDate|nullable', 'openingBalance' => 'numeric|required_with:openingBalanceDate|nullable',
'openingBalanceDate' => 'date|required_with:openingBalance|nullable', 'openingBalanceDate' => 'date|required_with:openingBalance|nullable',
'iban' => ['iban', 'nullable', new UniqueIban(null)], 'iban' => ['iban', 'nullable', new UniqueIban(null, $this->string('what'))],
'BIC' => 'bic|nullable', 'BIC' => 'bic|nullable',
'virtualBalance' => 'numeric|nullable', 'virtualBalance' => 'numeric|nullable',
'currency_id' => 'exists:transaction_currencies,id', 'currency_id' => 'exists:transaction_currencies,id',
@@ -95,7 +95,7 @@ class AccountFormRequest extends Request
// add rules: // add rules:
$rules['id'] = 'belongsToUser:accounts'; $rules['id'] = 'belongsToUser:accounts';
$rules['name'] = 'required|min:1|uniqueAccountForUser:' . intval($this->get('id')); $rules['name'] = 'required|min:1|uniqueAccountForUser:' . intval($this->get('id'));
$rules['iban'] = ['iban', 'nullable', new UniqueIban($account)]; $rules['iban'] = ['iban', 'nullable', new UniqueIban($account, $account->accountType->type)];
} }
return $rules; return $rules;

View File

@@ -23,9 +23,11 @@ declare(strict_types=1);
namespace FireflyIII\Rules; namespace FireflyIII\Rules;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Account; use FireflyIII\Models\Account;
use FireflyIII\Models\AccountType;
use Illuminate\Contracts\Validation\Rule; use Illuminate\Contracts\Validation\Rule;
use Illuminate\Support\Collection; use Log;
/** /**
* Class UniqueIban * Class UniqueIban
@@ -35,14 +37,19 @@ class UniqueIban implements Rule
/** @var Account */ /** @var Account */
private $account; private $account;
/** @var string */
private $expectedType;
/** /**
* Create a new rule instance. * Create a new rule instance.
* *
* @param Account|null $account * @param Account|null $account
* @param string|null $expectedType
*/ */
public function __construct(?Account $account) public function __construct(?Account $account, ?string $expectedType)
{ {
$this->account = $account; $this->account = $account;
$this->expectedType = $expectedType;
} }
/** /**
@@ -62,24 +69,68 @@ class UniqueIban implements Rule
* @param mixed $value * @param mixed $value
* *
* @return bool * @return bool
* @throws FireflyException
*/ */
public function passes($attribute, $value) public function passes($attribute, $value)
{ {
if (!auth()->check()) { if (!auth()->check()) {
return true; // @codeCoverageIgnore return true; // @codeCoverageIgnore
} }
if (is_null($this->expectedType)) {
return true;
}
$maxCounts = [
AccountType::ASSET => 0,
AccountType::EXPENSE => 0,
AccountType::REVENUE => 0,
];
switch ($this->expectedType) {
case 'asset':
case AccountType::ASSET:
// iban should be unique amongst asset accounts
// should not be in use with expense or revenue accounts.
// ie: must be totally unique.
break;
case 'expense':
case AccountType::EXPENSE:
// should be unique amongst expense and asset accounts.
// may appear once in revenue accounts
$maxCounts[AccountType::REVENUE] = 1;
break;
case 'revenue':
case AccountType::REVENUE:
// should be unique amongst revenue and asset accounts.
// may appear once in expense accounts
$maxCounts[AccountType::EXPENSE] = 1;
break;
default:
$query = auth()->user()->accounts(); throw new FireflyException(sprintf('UniqueIban cannot handle type "%s"', $this->expectedType));
if (!is_null($this->account)) {
$query->where('accounts.id', '!=', $this->account->id);
} }
/** @var Collection $accounts */ foreach ($maxCounts as $type => $max) {
$accounts = $query->get(); $count = 0;
$query = auth()->user()
->accounts()
->leftJoin('account_types', 'account_types.id', '=', 'accounts.account_type_id')
->where('account_types.type', $type);
if (!is_null($this->account)) {
$query->where('accounts.id', '!=', $this->account->id);
}
$result = $query->get(['accounts.*']);
foreach ($result as $account) {
if ($account->iban === $value) {
$count++;
}
}
if ($count > $max) {
Log::debug(
sprintf(
'IBAN "%s" is in use with %d account(s) of type "%s", which is too much for expected type "%s"',
$value, $count, $type, $this->expectedType
)
);
/** @var Account $account */
foreach ($accounts as $account) {
if ($account->iban === $value) {
return false; return false;
} }
} }