Completed bill tests.

app/Api/V1/Requests/Models/Bill/UpdateRequest.php

@@ -46,24 +46,23 @@ class UpdateRequest extends FormRequest
      */
     public function getAll(): array
     {
-        $active = true;
+        $fields = [
-        if (null !== $this->get('active')) {
+            'name'               => ['name', 'string'],
-            $active = $this->boolean('active');
+            'amount_min'         => ['amount_min', 'string'],
-        }
+            'amount_max'         => ['amount_max', 'string'],
-
+            'currency_id'        => ['currency_id', 'integer'],
-        return [
+            'currency_code'      => ['currency_code', 'string'],
-            'name'          => $this->string('name'),
+            'date'               => ['date', 'date'],
-            'amount_min'    => $this->string('amount_min'),
+            'repeat_freq'        => ['repeat_freq', 'string'],
-            'amount_max'    => $this->string('amount_max'),
+            'skip'               => ['skip', 'integer'],
-            'currency_id'   => $this->integer('currency_id'),
+            'active'             => ['active', 'boolean'],
-            'currency_code' => $this->string('currency_code'),
+            'order'              => ['order', 'integer'],
-            'date'          => $this->date('date'),
+            'notes'              => ['notes', 'nlString'],
-            'repeat_freq'   => $this->string('repeat_freq'),
+            'object_group_id'    => ['object_group_id', 'integer'],
-            'skip'          => $this->integer('skip'),
+            'object_group_title' => ['object_group_title', 'string'],
-            'active'        => $active,
-            'order'         => $this->integer('order'),
-            'notes'         => $this->nlString('notes'),
         ];
+
+        return $this->getAllData($fields);
     }
 
     /**
@@ -74,6 +73,7 @@ class UpdateRequest extends FormRequest
     public function rules(): array
     {
         $bill = $this->route()->parameter('bill');
+
         return [
             'name'          => sprintf('between:1,255|uniqueObjectForUser:bills,name,%d', $bill->id),
             'amount_min'    => 'numeric|gt:0',
@@ -100,12 +100,14 @@ class UpdateRequest extends FormRequest
         $validator->after(
             static function (Validator $validator) {
                 $data = $validator->getData();
+                if (array_key_exists('amount_min', $data) && array_key_exists('amount_max', $data)) {
                     $min = (float)($data['amount_min'] ?? 0);
                     $max = (float)($data['amount_max'] ?? 0);
                     if ($min > $max) {
                         $validator->errors()->add('amount_min', (string)trans('validation.amount_min_over_max'));
                     }
                 }
+            }
         );
     }
 }

app/Services/Internal/Update/BillUpdateService.php

@@ -23,12 +23,10 @@ declare(strict_types=1);
 
 namespace FireflyIII\Services\Internal\Update;
 
-use DB;
 use FireflyIII\Factory\TransactionCurrencyFactory;
 use FireflyIII\Models\Bill;
 use FireflyIII\Models\Rule;
 use FireflyIII\Models\RuleTrigger;
-use FireflyIII\Models\TransactionCurrency;
 use FireflyIII\Repositories\Bill\BillRepositoryInterface;
 use FireflyIII\Repositories\ObjectGroup\CreatesObjectGroups;
 use FireflyIII\Services\Internal\Support\BillServiceTrait;
@@ -55,25 +53,21 @@ class BillUpdateService
     public function update(Bill $bill, array $data): Bill
     {
         $this->user = $bill->user;
-        /** @var TransactionCurrencyFactory $factory */
-        $factory = app(TransactionCurrencyFactory::class);
-        /** @var TransactionCurrency $currency */
-        $currency = $factory->find($data['currency_id'] ?? null, $data['currency_code'] ?? null);
 
-        if (null === $currency) {
+        if (array_key_exists('currency_id', $data) || array_key_exists('currency_code', $data)) {
-            // use default currency:
+            $factory  = app(TransactionCurrencyFactory::class);
-            $currency = app('amount')->getDefaultCurrencyByUser($bill->user);
+            $currency = $factory->find($data['currency_id'] ?? null, $data['currency_code'] ?? null) ?? app('amount')->getDefaultCurrencyByUser($bill->user);
-        }
 
             // enable the currency if it isn't.
             $currency->enabled = true;
             $currency->save();
-
-        // new values
-        $data['transaction_currency_name'] = $currency->name;
-        $bill                              = $this->updateBillProperties($bill, $data);
             $bill->transaction_currency_id = $currency->id;
             $bill->save();
+        }
+        // update bill properties:
+        $bill = $this->updateBillProperties($bill, $data);
+        $bill->save();
+        $bill->refresh();
         // old values
         $oldData = [
             'name'                      => $bill->name,
@@ -84,24 +78,29 @@ class BillUpdateService
 
 
         // update note:
-        if (isset($data['notes'])) {
+        if (array_key_exists('notes', $data)) {
             $this->updateNote($bill, (string)$data['notes']);
         }
 
         // update order.
+        if (array_key_exists('order', $data)) {
             // update the order of the piggy bank:
             $oldOrder = (int)$bill->order;
             $newOrder = (int)($data['order'] ?? $oldOrder);
             if ($oldOrder !== $newOrder) {
                 $this->updateOrder($bill, $oldOrder, $newOrder);
             }
+        }
 
         // update rule actions.
+        if (array_key_exists('name', $data)) {
             $this->updateBillActions($bill, $oldData['name'], $data['name']);
             $this->updateBillTriggers($bill, $oldData, $data);
+        }
 
         // update using name:
-        $objectGroupTitle = $data['object_group'] ?? '';
+        if (array_key_exists('object_group_title', $data)) {
+            $objectGroupTitle = $data['object_group_title'] ?? '';
             if ('' !== $objectGroupTitle) {
                 $objectGroup = $this->findOrCreateObjectGroup($objectGroupTitle);
                 if (null !== $objectGroup) {
@@ -116,7 +115,8 @@ class BillUpdateService
                 $bill->objectGroups()->sync([]);
                 $bill->save();
             }
-
+        }
+        if (array_key_exists('object_group_id', $data)) {
             // try also with ID:
             $objectGroupId = (int)($data['object_group_id'] ?? 0);
             if (0 !== $objectGroupId) {
@@ -132,6 +132,7 @@ class BillUpdateService
                 $bill->objectGroups()->sync([]);
                 $bill->save();
             }
+        }
 
         return $bill;
     }
@@ -144,7 +145,6 @@ class BillUpdateService
      */
     private function updateBillProperties(Bill $bill, array $data): Bill
     {
-
         if (isset($data['name']) && '' !== (string)$data['name']) {
             $bill->name = $data['name'];
         }
@@ -161,10 +161,10 @@ class BillUpdateService
         if (isset($data['repeat_freq']) && '' !== (string)$data['repeat_freq']) {
             $bill->repeat_freq = $data['repeat_freq'];
         }
-        if (isset($data['skip']) && '' !== (string)$data['skip']) {
+        if (array_key_exists('skip', $data)) {
             $bill->skip = $data['skip'];
         }
-        if (isset($data['active']) && is_bool($data['active'])) {
+        if (array_key_exists('active', $data)) {
             $bill->active = $data['active'];
         }
 

tests/Api/Models/Bill/UpdateControllerTest.php

@@ -0,0 +1,192 @@
+<?php
+/*
+ * UpdateControllerTest.php
+ * Copyright (c) 2021 james@firefly-iii.org
+ *
+ * This file is part of Firefly III (https://github.com/firefly-iii).
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+namespace Tests\Api\Models\Bill;
+
+
+use Faker\Factory;
+use Laravel\Passport\Passport;
+use Log;
+use Tests\TestCase;
+use Tests\Traits\CollectsValues;
+use Tests\Traits\RandomValues;
+use Tests\Traits\TestHelpers;
+
+/**
+ * Class UpdateControllerTest
+ */
+class UpdateControllerTest extends TestCase
+{
+    use RandomValues, TestHelpers, CollectsValues;
+
+    /**
+     *
+     */
+    public function setUp(): void
+    {
+        parent::setUp();
+        Passport::actingAs($this->user());
+        Log::info(sprintf('Now in %s.', get_class($this)));
+    }
+
+
+    /**
+     * @dataProvider updateDataProvider
+     */
+    public function testUpdate(array $submission): void
+    {
+        $ignore = [
+            'created_at',
+            'updated_at',
+        ];
+        $route  = route('api.v1.bills.update', [$submission['id']]);
+
+        $this->updateAndCompare($route, $submission, $ignore);
+    }
+
+
+    /**
+     * @return array
+     */
+    public function updateDataProvider(): array
+    {
+        $submissions = [];
+        $all         = $this->updateDataSet();
+        foreach ($all as $name => $data) {
+            $submissions[] = [$data];
+        }
+
+        return $submissions;
+    }
+
+
+    /**
+     * @return array
+     */
+    public function updateDataSet(): array
+    {
+        $faker           = Factory::create();
+        $currencies      = [
+            1 => 'EUR',
+            2 => 'HUF',
+            3 => 'GBP',
+            4 => 'UAH',
+        ];
+        $repeatFreqs     = ['yearly', 'weekly', 'monthly'];
+        $repeatFreq      = $repeatFreqs[rand(0, count($repeatFreqs) - 1)];
+        $objectGroupId   = $faker->numberBetween(1, 2);
+        $objectGroupName = sprintf('Object group %d', $objectGroupId);
+        $rand            = rand(1, 4);
+        $set             = [
+            'name'       => [
+                'id'           => 1,
+                'fields'       => [
+                    'name' => ['test_value' => join(' ', $faker->words(4))],
+                ],
+                'extra_ignore' => [],
+            ],
+            'amount_min' => [
+                'id'           => 1,
+                'fields'       => [
+                    'amount_min' => ['test_value' => number_format($faker->randomFloat(2, 10, 50), 2)],
+                ],
+                'extra_ignore' => [],
+            ],
+            'amount_max' => [
+                'id'           => 1,
+                'fields'       => [
+                    'amount_max' => ['test_value' => number_format($faker->randomFloat(2, 60, 90), 2)],
+                ],
+                'extra_ignore' => [],
+            ],
+            'date'       => [
+                'id'           => 1,
+                'fields'       => [
+                    'date' => ['test_value' => $faker->dateTimeBetween('-1 year', 'now')->format('Y-m-d')],
+                ],
+                'extra_ignore' => [],
+            ],
+
+            'repeat_freq' => [
+                'id'           => 1,
+                'fields'       => [
+                    'repeat_freq' => ['test_value' => $repeatFreq],
+                ],
+                'extra_ignore' => [],
+            ],
+            'skip'        => [
+                'id'           => 1,
+                'fields'       => [
+                    'skip' => ['test_value' => $faker->numberBetween(1, 10)],
+                ],
+                'extra_ignore' => [],
+            ],
+
+            'active'            => [
+                'id'           => 1,
+                'fields'       => [
+                    'active' => ['test_value' => $faker->boolean],
+                ],
+                'extra_ignore' => [],
+            ],
+            'notes'             => [
+                'id'           => 1,
+                'fields'       => [
+                    'notes' => ['test_value' => join(' ', $faker->words(5))],
+                ],
+                'extra_ignore' => [],
+            ],
+            'object_group_id'   => [
+                'id'           => 1,
+                'fields'       => [
+                    'object_group_id' => ['test_value' => (string)$objectGroupId],
+                ],
+                'extra_ignore' => ['object_group_order', 'object_group_title'],
+            ],
+            'object_group_title' => [
+                'id'           => 1,
+                'fields'       => [
+                    'object_group_title' => ['test_value' => $objectGroupName],
+                ],
+                'extra_ignore' => ['object_group_order', 'object_group_id'],
+            ],
+            'currency_id'       => [
+                'id'           => 1,
+                'fields'       => [
+                    'currency_id' => ['test_value' => (string)$rand],
+                ],
+                'extra_ignore' => ['currency_code', 'currency_symbol'],
+            ],
+            'currency_code'     => [
+                'id'           => 1,
+                'fields'       => [
+                    'currency_code' => ['test_value' => $currencies[$rand]],
+                ],
+                'extra_ignore' => ['currency_id', 'currency_symbol'],
+            ],
+
+        ];
+
+        return $set;
+    }
+
+
+}