From cbcf251bb394262d553217490ca5df29f94d88c7 Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 31 Mar 2021 19:36:08 +0200 Subject: [PATCH] Fix #4570 --- .../Requests/Models/Budget/UpdateRequest.php | 14 ++- app/Http/Requests/BudgetFormUpdateRequest.php | 2 +- app/Repositories/Budget/BudgetRepository.php | 92 +++++++++++-------- .../AutoBudget/ValidatesAutoBudgetRequest.php | 15 +-- changelog.md | 1 + 5 files changed, 74 insertions(+), 50 deletions(-) diff --git a/app/Api/V1/Requests/Models/Budget/UpdateRequest.php b/app/Api/V1/Requests/Models/Budget/UpdateRequest.php index d8b149fcda..13fe5b0176 100644 --- a/app/Api/V1/Requests/Models/Budget/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Budget/UpdateRequest.php @@ -47,7 +47,7 @@ class UpdateRequest extends FormRequest public function getAll(): array { // this is the way: - $fields = [ + $fields = [ 'name' => ['name', 'string'], 'active' => ['active', 'boolean'], 'order' => ['order', 'integer'], @@ -57,8 +57,17 @@ class UpdateRequest extends FormRequest 'auto_budget_amount' => ['auto_budget_amount', 'string'], 'auto_budget_period' => ['auto_budget_period', 'string'], ]; + $allData = $this->getAllData($fields); + if (array_key_exists('auto_budget_type', $allData)) { + $types = [ + 'none' => 0, + 'reset' => 1, + 'rollover' => 2, + ]; + $allData['auto_budget_type'] = $types[$allData['auto_budget_type']] ?? 0; + } - return $this->getAllData($fields); + return $allData; } /** @@ -69,7 +78,6 @@ class UpdateRequest extends FormRequest public function rules(): array { $budget = $this->route()->parameter('budget'); - return [ 'name' => sprintf('between:1,100|uniqueObjectForUser:budgets,name,%d', $budget->id), 'active' => [new IsBoolean], diff --git a/app/Http/Requests/BudgetFormUpdateRequest.php b/app/Http/Requests/BudgetFormUpdateRequest.php index d7728fcd2a..a907df9db4 100644 --- a/app/Http/Requests/BudgetFormUpdateRequest.php +++ b/app/Http/Requests/BudgetFormUpdateRequest.php @@ -73,7 +73,7 @@ class BudgetFormUpdateRequest extends FormRequest return [ 'name' => $nameRule, 'active' => 'numeric|between:0,1', - 'auto_budget_option' => 'numeric|between:0,2', + 'auto_budget_type' => 'numeric|between:0,2', 'auto_budget_currency_id' => 'exists:transaction_currencies,id', 'auto_budget_amount' => 'min:0|max:1000000000', 'auto_budget_period' => 'in:daily,weekly,monthly,quarterly,half_year,yearly', diff --git a/app/Repositories/Budget/BudgetRepository.php b/app/Repositories/Budget/BudgetRepository.php index 6f2a8516e5..11520a7959 100644 --- a/app/Repositories/Budget/BudgetRepository.php +++ b/app/Repositories/Budget/BudgetRepository.php @@ -395,6 +395,7 @@ class BudgetRepository implements BudgetRepositoryInterface public function update(Budget $budget, array $data): Budget { Log::debug('Now in update()'); + // TODO update rules $oldName = $budget->name; if (array_key_exists('name', $data)) { @@ -408,8 +409,47 @@ class BudgetRepository implements BudgetRepositoryInterface // update or create auto-budget: $autoBudget = $this->getAutoBudget($budget); - // get currency: - $currency = null; + // first things first: delete when no longer required: + $autoBudgetType = array_key_exists('auto_budget_type', $data) ? $data['auto_budget_type'] : null; + + if (0 === $autoBudgetType && null !== $autoBudget) { + // delete! + $autoBudget->delete(); + + return $budget; + } + if (0 === $autoBudgetType && null === $autoBudget) { + return $budget; + } + if (null === $autoBudgetType && null === $autoBudget) { + return $budget; + } + $this->updateAutoBudget($budget, $data); + + return $budget; + } + + /** + * @param Budget $budget + * @param array $data + */ + private function updateAutoBudget(Budget $budget, array $data): void + { + // update or create auto-budget: + $autoBudget = $this->getAutoBudget($budget); + + // grab default currency: + $currency = app('amount')->getDefaultCurrencyByUser($this->user); + + if (null === $autoBudget) { + // at this point it's a blind assumption auto_budget_type is 1 or 2. + $autoBudget = new AutoBudget; + $autoBudget->auto_budget_type = $data['auto_budget_type']; + $autoBudget->budget_id = $budget->id; + $autoBudget->transaction_currency_id = $currency->id; + } + + // set or update the currency. if (array_key_exists('currency_id', $data) || array_key_exists('currency_code', $data)) { $repos = app(CurrencyRepositoryInterface::class); $currencyId = (int)($data['currency_id'] ?? 0); @@ -418,50 +458,24 @@ class BudgetRepository implements BudgetRepositoryInterface if (null === $currency) { $currency = $repos->findByCodeNull($currencyCode); } - } - if (null === $currency) { - $currency = app('amount')->getDefaultCurrencyByUser($this->user); - } - if (null === $autoBudget - && array_key_exists('auto_budget_type', $data) - && array_key_exists('auto_budget_amount', $data) - && 0 !== $data['auto_budget_type'] - && 'none' !== $data['auto_budget_type'] - ) { - // only create if all are here: - $autoBudget = new AutoBudget; - $autoBudget->budget_id = $budget->id; - $autoBudget->transaction_currency_id = $currency->id; - } - if (null !== $autoBudget && null !== $currency) { - $autoBudget->transaction_currency_id = $currency->id; + if (null !== $currency) { + $autoBudget->transaction_currency_id = $currency->id; + } } - // update existing type - if (array_key_exists('auto_budget_type', $data) && 0 !== $data['auto_budget_type']) { - $autoBudgetType = $data['auto_budget_type']; - if ('reset' === $autoBudgetType) { - $autoBudget->auto_budget_type = AutoBudget::AUTO_BUDGET_RESET; - } - if ('rollover' === $autoBudgetType) { - $autoBudget->auto_budget_type = AutoBudget::AUTO_BUDGET_ROLLOVER; - } - if ('none' === $autoBudgetType && null !== $autoBudget) { - $autoBudget->delete(); - - return $budget; - } + // change values if submitted or presented: + if (array_key_exists('auto_budget_type', $data)) { + $autoBudget->auto_budget_type = $data['auto_budget_type']; } - if (array_key_exists('auto_budget_amount', $data) && null !== $autoBudget) { + if (array_key_exists('auto_budget_amount', $data)) { $autoBudget->amount = $data['auto_budget_amount']; + } - if (array_key_exists('auto_budget_period', $data) && null !== $autoBudget) { + if (array_key_exists('auto_budget_period', $data)) { $autoBudget->period = $data['auto_budget_period']; } - if (null !== $autoBudget) { - $autoBudget->save(); - } - return $budget; + + $autoBudget->save(); } /** diff --git a/app/Validation/AutoBudget/ValidatesAutoBudgetRequest.php b/app/Validation/AutoBudget/ValidatesAutoBudgetRequest.php index 7d8909de01..a9c844a115 100644 --- a/app/Validation/AutoBudget/ValidatesAutoBudgetRequest.php +++ b/app/Validation/AutoBudget/ValidatesAutoBudgetRequest.php @@ -22,6 +22,7 @@ declare(strict_types=1); namespace FireflyIII\Validation\AutoBudget; + use Illuminate\Validation\Validator; /** @@ -36,27 +37,27 @@ trait ValidatesAutoBudgetRequest { $data = $validator->getData(); $type = $data['auto_budget_type'] ?? ''; - $amount = $data['auto_budget_amount'] ?? ''; - $period = (string)($data['auto_budget_period'] ?? ''); - $currencyId = $data['auto_budget_currency_id'] ?? ''; - $currencyCode = $data['auto_budget_currency_code'] ?? ''; + $amount = array_key_exists('auto_budget_amount', $data) ? $data['auto_budget_amount'] : null; + $period = array_key_exists('auto_budget_period', $data) ? $data['auto_budget_period'] : null; + $currencyId = array_key_exists('auto_budget_currency_id', $data) ? (int)$data['auto_budget_currency_id'] : null; + $currencyCode = array_key_exists('auto_budget_currency_code', $data) ? $data['auto_budget_currency_code'] : null; if (is_numeric($type)) { $type = (int)$type; } - if (0 === $type || 'none' === $type || '' === $type) { + if (0 === $type) { return; } // basic float check: if ('' === $amount) { $validator->errors()->add('auto_budget_amount', (string)trans('validation.amount_required_for_auto_budget')); } - if (1 !== bccomp((string)$amount, '0')) { + if (null !== $amount && 1 !== bccomp((string)$amount, '0')) { $validator->errors()->add('auto_budget_amount', (string)trans('validation.auto_budget_amount_positive')); } if ('' === $period) { $validator->errors()->add('auto_budget_period', (string)trans('validation.auto_budget_period_mandatory')); } - if ('' === $currencyCode && '' === $currencyId) { + if (null !== $amount && null !== $currencyId && null !== $currencyCode && '' === $currencyCode && '' === $currencyId) { $validator->errors()->add('auto_budget_amount', (string)trans('validation.require_currency_info')); } } diff --git a/changelog.md b/changelog.md index ce28df8b45..38043c3706 100644 --- a/changelog.md +++ b/changelog.md @@ -25,6 +25,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - [Issue 4560](https://github.com/firefly-iii/firefly-iii/issues/4560) The account number would be stored in the BIC field, if the BIC field was set. - [Issue 4562](https://github.com/firefly-iii/firefly-iii/issues/4562) Hidden budgets were visible in v2. - [Issue 4567](https://github.com/firefly-iii/firefly-iii/issues/4567) Missing translation marked as intentionally missing. +- [Issue 4570](https://github.com/firefly-iii/firefly-iii/issues/4570) It was impossible to set or change auto budgets. ### Security - Nothing (yet)