Expand forms and improve validation for multi-account piggy banks

This commit is contained in:
James Cole 2024-12-06 08:10:31 +01:00
parent 4819b5ac5d
commit ea4be9dd0c
No known key found for this signature in database
GPG Key ID: B49A324B7EAD6D80
16 changed files with 149 additions and 57 deletions

View File

@ -24,8 +24,11 @@ declare(strict_types=1);
namespace FireflyIII\Api\V1\Requests\Models\PiggyBank;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\TransactionCurrency;
use FireflyIII\Repositories\Account\AccountRepositoryInterface;
use FireflyIII\Rules\IsValidPositiveAmount;
use FireflyIII\Rules\IsValidZeroOrMoreAmount;
use FireflyIII\Support\Request\ChecksLogin;
use FireflyIII\Support\Request\ConvertsDataTypes;
use Illuminate\Foundation\Http\FormRequest;
@ -73,27 +76,30 @@ class StoreRequest extends FormRequest
'accounts' => 'required',
'accounts.*' => 'array|required',
'accounts.*.account_id' => 'required|numeric|belongsToUser:accounts,id',
'accounts.*.current_amount' => ['numeric', new IsValidPositiveAmount()],
'accounts.*.current_amount' => ['numeric', new IsValidZeroOrMoreAmount()],
'object_group_id' => 'numeric|belongsToUser:object_groups,id',
'object_group_title' => ['min:1', 'max:255'],
'target_amount' => ['required', new IsValidPositiveAmount()],
'target_amount' => ['required', new IsValidZeroOrMoreAmount()],
'start_date' => 'date|nullable',
'transaction_currency_id' => 'exists:transaction_currencies,id',
'transaction_currency_code' => 'exists:transaction_currencies,code',
'transaction_currency_id' => 'exists:transaction_currencies,id|required_without:transaction_currency_code',
'transaction_currency_code' => 'exists:transaction_currencies,code|required_without:transaction_currency_id',
'target_date' => 'date|nullable|after:start_date',
'notes' => 'max:65000',
];
}
/**
* Can only store money on liabilities and asset accouns.
* Can only store money on liabilities and asset accounts.
*/
public function withValidator(Validator $validator): void
{
$validator->after(
function (Validator $validator): void {
// validate start before end only if both are there.
$data = $validator->getData();
$data = $validator->getData();
$currency = $this->getCurrencyFromData($data);
$targetAmount = (string) ($data['target_amount'] ?? '0');
$currentAmount = '0';
if (array_key_exists('accounts', $data) && is_array($data['accounts'])) {
$repository = app(AccountRepositoryInterface::class);
$types = config('firefly.piggy_bank_account_types');
@ -101,6 +107,13 @@ class StoreRequest extends FormRequest
$accountId = (int) ($array['account_id'] ?? 0);
$account = $repository->find($accountId);
if (null !== $account) {
// check currency here.
$accountCurrency = $repository->getAccountCurrency($account);
$isMultiCurrency = $repository->getMetaValue($account, 'is_multi_currency');
$currentAmount = bcadd($currentAmount, (string)($array['current_amount'] ?? '0'));
if ($accountCurrency->id !== $currency->id && 'true' !== $isMultiCurrency) {
$validator->errors()->add(sprintf('accounts.%d', $index), trans('validation.invalid_account_currency'));
}
$type = $account->accountType->type;
if (!in_array($type, $types, true)) {
$validator->errors()->add(sprintf('accounts.%d', $index), trans('validation.invalid_account_type'));
@ -108,6 +121,9 @@ class StoreRequest extends FormRequest
}
}
}
if(bccomp($targetAmount, $currentAmount) === -1 && bccomp($targetAmount, '0') === 1) {
$validator->errors()->add('target_amount', trans('validation.current_amount_too_much'));
}
}
);
if ($validator->fails()) {
@ -126,10 +142,27 @@ class StoreRequest extends FormRequest
continue;
}
$return[] = [
'account_id' => $this->integerFromValue((string)($entry['account_id'] ?? '0')),
'current_amount' => $this->clearString($entry['current_amount'] ?? '0'),
'account_id' => $this->integerFromValue((string) ($entry['account_id'] ?? '0')),
'current_amount' => $this->clearString((string) ($entry['current_amount'] ?? '0')),
];
}
return $return;
}
private function getCurrencyFromData(array $data): TransactionCurrency
{
if (array_key_exists('transaction_currency_code', $data) && '' !== (string) $data['transaction_currency_code']) {
$currency = TransactionCurrency::whereCode($data['transaction_currency_code'])->first();
if (null !== $currency) {
return $currency;
}
}
if (array_key_exists('transaction_currency_id', $data) && '' !== (string) $data['transaction_currency_id']) {
$currency = TransactionCurrency::find((int) $data['transaction_currency_id']);
if (null !== $currency) {
return $currency;
}
}
throw new FireflyException('Unexpected empty currency.');
}
}

View File

@ -37,7 +37,11 @@ use Illuminate\Database\QueryException;
*/
class PiggyBankFactory
{
private User $user;
public User $user {
set(User $value) {
$this->user = $value;
}
}
private CurrencyRepositoryInterface $currencyRepository;
private AccountRepositoryInterface $accountRepository;
private PiggyBankRepositoryInterface $piggyBankRepository;
@ -138,11 +142,6 @@ class PiggyBankFactory
return $this->user->piggyBanks()->where('piggy_banks.name', $name)->first();
}
public function setUser(User $user): void
{
$this->user = $user;
}
private function getCurrency(array $data): TransactionCurrency {
// currency:
$defaultCurrency = app('amount')->getDefaultCurrency();
@ -197,7 +196,8 @@ class PiggyBankFactory
private function getMaxOrder(): int
{
return (int)$this->user->piggyBanks()->max('piggy_banks.order');
return (int) $this->piggyBankRepository->getPiggyBanks()->max('order');
}
private function linkToAccountIds(PiggyBank $piggyBank, array $accounts): void {
@ -207,7 +207,7 @@ class PiggyBankFactory
if(null === $account) {
continue;
}
$piggyBank->accounts()->syncWithoutDetaching([$account->id, ['current_amount' => $info['current_amount'] ?? '0']]);
$piggyBank->accounts()->syncWithoutDetaching([$account->id => ['current_amount' => $info['current_amount'] ?? '0']]);
}
}
}

View File

@ -92,10 +92,11 @@ class CreateController extends Controller
public function store(PiggyBankStoreRequest $request)
{
$data = $request->getPiggyBankData();
if (null === $data['startdate']) {
$data['startdate'] = today(config('app.timezone'));
if (null === $data['start_date']) {
$data['start_date'] = today(config('app.timezone'));
}
$piggyBank = $this->piggyRepos->store($data);
var_dump($data);exit;
session()->flash('success', (string)trans('firefly.stored_piggy_bank', ['name' => $piggyBank->name]));
app('preferences')->mark();

View File

@ -43,15 +43,21 @@ class PiggyBankStoreRequest extends FormRequest
*/
public function getPiggyBankData(): array
{
return [
$data = [
'name' => $this->convertString('name'),
'startdate' => $this->getCarbonDate('startdate'),
'account_id' => $this->convertInteger('account_id'),
'targetamount' => $this->convertString('targetamount'),
'targetdate' => $this->getCarbonDate('targetdate'),
'start_date' => $this->getCarbonDate('start_date'),
//'account_id' => $this->convertInteger('account_id'),
'accounts' => $this->get('accounts'),
'target_amount' => $this->convertString('target_amount'),
'target_date' => $this->getCarbonDate('target_date'),
'notes' => $this->stringWithNewlines('notes'),
'object_group_title' => $this->convertString('object_group'),
];
if(!is_array($data['accounts'])) {
$data['accounts'] = [];
}
return $data;
}
/**
@ -61,10 +67,11 @@ class PiggyBankStoreRequest extends FormRequest
{
return [
'name' => 'required|min:1|max:255|uniquePiggyBankForUser',
'account_id' => 'required|belongsToUser:accounts',
'targetamount' => ['nullable', new IsValidPositiveAmount()],
'startdate' => 'date',
'targetdate' => 'date|nullable',
'accounts' => 'required|array',
'accounts.*' => 'required|belongsToUser:accounts',
'target_amount' => ['nullable', new IsValidPositiveAmount()],
'start_date' => 'date',
'target_date' => 'date|nullable',
'order' => 'integer|min:1',
'object_group' => 'min:0|max:255',
'notes' => 'min:1|max:32768|nullable',
@ -73,6 +80,10 @@ class PiggyBankStoreRequest extends FormRequest
public function withValidator(Validator $validator): void
{
// need to have more than one account.
// accounts need to have the same currency or be multi-currency(?).
if ($validator->fails()) {
Log::channel('audit')->error(sprintf('Validation errors in %s', __CLASS__), $validator->errors()->toArray());
}

View File

@ -35,46 +35,46 @@ class AccountType extends Model
{
use ReturnsIntegerIdTrait;
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string ASSET = 'Asset account';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string BENEFICIARY = 'Beneficiary account';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string CASH = 'Cash account';
#[\Deprecated] /** @deprecated */
public const string CREDITCARD = 'Credit card';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string DEBT = 'Debt';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string DEFAULT = 'Default account';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string EXPENSE = 'Expense account';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string IMPORT = 'Import account';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string INITIAL_BALANCE = 'Initial balance account';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string LIABILITY_CREDIT = 'Liability credit account';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string LOAN = 'Loan';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string MORTGAGE = 'Mortgage';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string RECONCILIATION = 'Reconciliation account';
#[\Deprecated]
#[\Deprecated] /** @deprecated */
public const string REVENUE = 'Revenue account';
protected $casts

View File

@ -182,8 +182,8 @@ trait ModifiesPiggyBanks
*/
public function store(array $data): PiggyBank
{
$factory = new PiggyBankFactory();
$factory->setUser($this->user);
$factory = new PiggyBankFactory();
$factory->user = $this->user;
return $factory->store($data);
}

View File

@ -356,8 +356,8 @@ class PiggyBankRepository implements PiggyBankRepositoryInterface
#[\Override] public function resetOrder(): void
{
$factory = new PiggyBankFactory();
$factory->setUser($this->user);
$factory = new PiggyBankFactory();
$factory->user = $this->user;
$factory->resetOrder();
}
}

View File

@ -283,9 +283,9 @@ trait RecurringTransactionTrait
protected function updatePiggyBank(RecurrenceTransaction $transaction, int $piggyId): void
{
/** @var PiggyBankFactory $factory */
$factory = app(PiggyBankFactory::class);
$factory->setUser($transaction->recurrence->user);
$piggyBank = $factory->find($piggyId, null);
$factory = app(PiggyBankFactory::class);
$factory->user = $transaction->recurrence->user;
$piggyBank = $factory->find($piggyId, null);
if (null !== $piggyBank) {
/** @var null|RecurrenceMeta $entry */
$entry = $transaction->recurrenceTransactionMeta()->where('name', 'piggy_bank_id')->first();

View File

@ -24,6 +24,7 @@ declare(strict_types=1);
namespace FireflyIII\Support\Form;
use FireflyIII\Enums\AccountTypeEnum;
use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Models\Account;
use FireflyIII\Models\AccountType;
@ -141,12 +142,25 @@ class AccountForm
*/
public function assetAccountList(string $name, $value = null, ?array $options = null): string
{
$types = [AccountType::ASSET, AccountType::DEFAULT];
$types = [AccountTypeEnum::ASSET->value, AccountTypeEnum::DEFAULT->value];
$grouped = $this->getAccountsGrouped($types);
return $this->select($name, $grouped, $value, $options);
}
/**
* Basic list of asset accounts.
*
* @param mixed $value
*/
public function assetLiabilityMultiAccountList(string $name, $value = null, ?array $options = null): string
{
$types = [AccountTypeEnum::ASSET->value, AccountTypeEnum::DEFAULT->value, AccountTypeEnum::MORTGAGE->value, AccountTypeEnum::DEBT->value,AccountTypeEnum::LOAN->value];
$grouped = $this->getAccountsGrouped($types);
return $this->multiSelect($name, $grouped, $value, $options);
}
/**
* Same list but all liabilities as well.
*
@ -154,7 +168,7 @@ class AccountForm
*/
public function longAccountList(string $name, $value = null, ?array $options = null): string
{
$types = [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::CREDITCARD, AccountType::LOAN];
$types = [AccountType::ASSET, AccountType::DEFAULT, AccountType::MORTGAGE, AccountType::DEBT, AccountType::LOAN];
$grouped = $this->getAccountsGrouped($types);
return $this->select($name, $grouped, $value, $options);

View File

@ -56,6 +56,25 @@ trait FormSupport
return $html;
}
public function multiSelect(string $name, ?array $list = null, $selected = null, ?array $options = null): string
{
$list ??= [];
$label = $this->label($name, $options);
$options = $this->expandOptionArray($name, $label, $options);
$classes = $this->getHolderClasses($name);
$selected = $this->fillFieldValue($name, $selected);
unset($options['autocomplete'], $options['placeholder']);
try {
$html = view('form.multi-select', compact('classes', 'name', 'label', 'selected', 'options', 'list'))->render();
} catch (\Throwable $e) {
app('log')->debug(sprintf('Could not render multi-select(): %s', $e->getMessage()));
$html = 'Could not render multi-select.';
}
return $html;
}
protected function label(string $name, ?array $options = null): string
{
$options ??= [];

View File

@ -197,6 +197,7 @@ return [
'assetAccountCheckList',
'assetAccountList',
'longAccountList',
'assetLiabilityMultiAccountList',
],
],
'CurrencyForm' => [

View File

@ -2197,6 +2197,7 @@ return [
'amount' => 'Amount',
'overview' => 'Overview',
'saveOnAccount' => 'Save on account',
'saveOnAccounts' => 'Save on account(s)',
'unknown' => 'Unknown',
'monthly' => 'Monthly',
'profile' => 'Profile',

View File

@ -69,6 +69,7 @@ return [
// Ignore this comment
'targetamount' => 'Target amount',
'target_amount' => 'Target amount',
'account_role' => 'Account role',
'opening_balance_date' => 'Opening balance date',
'cc_type' => 'Credit card payment plan',
@ -106,7 +107,9 @@ return [
'deletePermanently' => 'Delete permanently',
'cancel' => 'Cancel',
'targetdate' => 'Target date',
'target_date' => 'Target date',
'startdate' => 'Start date',
'start_date' => 'Start date',
'tag' => 'Tag',
'under' => 'Under',
'symbol' => 'Symbol',
@ -116,7 +119,6 @@ return [
'creditCardNumber' => 'Credit card number',
'has_headers' => 'Headers',
'date_format' => 'Date format',
'specifix' => 'Bank- or file specific fixes',
'attachments[]' => 'Attachments',
'title' => 'Title',
'notes' => 'Notes',
@ -125,8 +127,7 @@ return [
'size' => 'Size',
'trigger' => 'Trigger',
'stop_processing' => 'Stop processing',
'start_date' => 'Start of range',
'end_date' => 'End of range',
'end_date' => 'End date',
'enddate' => 'End date',
'move_rules_before_delete' => 'Rule group',
'start' => 'Start of range',

View File

@ -26,6 +26,8 @@ declare(strict_types=1);
return [
'invalid_account_type' => 'A piggy bank can only be linked to asset accounts and liabilities',
'invalid_account_currency' => 'This account does not use the currency you have selected',
'current_amount_too_much' => 'The combined amount in "current_amount" cannot exceed the "target_amount".',
'filter_must_be_in' => 'Filter ":filter" must be one of: :values',
'filter_not_string' => 'Filter ":filter" is expected to be a string of text',
'bad_api_filter' => 'This API endpoint does not support ":filter" as a filter.',

View File

@ -0,0 +1,10 @@
<div class="{{ classes }}" id="{{ name }}_holder">
<label for="{{ options.id }}" class="col-sm-4 control-label">{{ label }}</label>
<div class="col-sm-8">
{{ Html.select(name~"[]", list, selected).id(options.id).class('form-control').attribute('multiple').attribute('autocomplete','off').attribute('spellcheck','false').attribute('placeholder', options.placeholder) }}
{% include 'form.help' %}
{% include 'form.feedback' %}
</div>
</div>

View File

@ -8,7 +8,6 @@
<form method="POST" action="{{ route('piggy-banks.store') }}" accept-charset="UTF-8" class="form-horizontal" id="store" enctype="multipart/form-data">
<input name="_token" type="hidden" value="{{ csrf_token() }}">
<input type="hidden" name="repeats" value="0"/>
<div class="row">
<div class="col-lg-6 col-md-6 col-sm-12 col-xs-12">
<div class="box box-primary">
@ -16,10 +15,10 @@
<h3 class="box-title">{{ 'mandatoryFields'|_ }}</h3>
</div>
<div class="box-body">
{{ ExpandedForm.text('name') }}
{{ AccountForm.assetAccountList('account_id', null, {label: 'saveOnAccount'|_ }) }}
{{ ExpandedForm.amountNoCurrency('targetamount') }}
{{ AccountForm.assetLiabilityMultiAccountList('accounts', null, {label: 'saveOnAccounts'|_ }) }}
{{ ExpandedForm.amountNoCurrency('target_amount') }}
</div>
</div>
@ -30,7 +29,7 @@
<h3 class="box-title">{{ 'optionalFields'|_ }}</h3>
</div>
<div class="box-body">
{{ ExpandedForm.date('targetdate') }}
{{ ExpandedForm.date('target_date') }}
{{ ExpandedForm.textarea('notes', null, {helpText: trans('firefly.field_supports_markdown')} ) }}
{{ ExpandedForm.file('attachments[]', {'multiple': 'multiple','helpText': trans('firefly.upload_max_file_size', {'size': uploadSize|filesize}) }) }}
{{ ExpandedForm.objectGroup() }}