Fix #4570

firefly-iii · Mar 31, 2021 · cbcf251 · cbcf251
1 parent 0756054
commit cbcf251
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 50 deletions.
diff --git 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 @@ public function getAll(): array
             '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 @@ public function getAll(): array
     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
@@ -73,7 +73,7 @@ public function rules(): array
         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
@@ -395,6 +395,7 @@ public function store(array $data): Budget
     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,60 +409,73 @@ public function update(Budget $budget, array $data): Budget
         // update or create auto-budget:
         $autoBudget = $this->getAutoBudget($budget);
 
-        // get currency:
-        $currency = null;
-        if (array_key_exists('currency_id', $data) || array_key_exists('currency_code', $data)) {
-            $repos        = app(CurrencyRepositoryInterface::class);
-            $currencyId   = (int)($data['currency_id'] ?? 0);
-            $currencyCode = (string)($data['currency_code'] ?? '');
-            $currency     = $repos->findNull($currencyId);
-            if (null === $currency) {
-                $currency = $repos->findByCodeNull($currencyCode);
-            }
+        // 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 (null === $currency) {
-            $currency = app('amount')->getDefaultCurrencyByUser($this->user);
+        if (0 === $autoBudgetType && null === $autoBudget) {
+            return $budget;
         }
-        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:
+        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;
         }
-        if (null !== $autoBudget && 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;
+        // 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);
+            $currencyCode = (string)($data['currency_code'] ?? '');
+            $currency     = $repos->findNull($currencyId);
+            if (null === $currency) {
+                $currency = $repos->findByCodeNull($currencyCode);
             }
-            if ('rollover' === $autoBudgetType) {
-                $autoBudget->auto_budget_type = AutoBudget::AUTO_BUDGET_ROLLOVER;
+            if (null !== $currency) {
+                $autoBudget->transaction_currency_id = $currency->id;
             }
-            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
@@ -22,6 +22,7 @@
 declare(strict_types=1);
 
 namespace FireflyIII\Validation\AutoBudget;
+
 use Illuminate\Validation\Validator;
 
 /**
@@ -36,27 +37,27 @@ protected function validateAutoBudgetAmount(Validator $validator): void
     {
         $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
@@ -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)