Fix budget limit tests

app/Api/V1/Controllers/Models/BudgetLimit/ShowController.php

@@ -104,7 +104,7 @@ class ShowController extends Controller
      */
     public function show(Request $request, Budget $budget, BudgetLimit $budgetLimit): JsonResponse
     {
-        if ($budget->id !== $budgetLimit->budget_id) {
+        if ((int)$budget->id !== (int)$budgetLimit->budget_id) {
             throw new FireflyException('20028: The budget limit does not belong to the budget.');
         }
         // continue!

app/Api/V1/Controllers/Models/BudgetLimit/UpdateController.php

@@ -74,7 +74,7 @@ class UpdateController extends Controller
     public function update(UpdateRequest $request, Budget $budget, BudgetLimit $budgetLimit): JsonResponse
     {
 
-        if ($budget->id !== $budgetLimit->budget_id) {
+        if ((int)$budget->id !== (int)$budgetLimit->budget_id) {
             throw new FireflyException('20028: The budget limit does not belong to the budget.');
         }
         $data              = $request->getAll();

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

@@ -23,9 +23,11 @@ declare(strict_types=1);
 
 namespace FireflyIII\Api\V1\Requests\Models\BudgetLimit;
 
+use Carbon\Carbon;
 use FireflyIII\Support\Request\ChecksLogin;
 use FireflyIII\Support\Request\ConvertsDataTypes;
 use Illuminate\Foundation\Http\FormRequest;
+use Illuminate\Validation\Validator;
 
 /**
  * Class UpdateRequest
@@ -43,13 +45,15 @@ class UpdateRequest extends FormRequest
      */
     public function getAll(): array
     {
-return [
+        $fields = [
-            'start'         => $this->date('start'),
+            'start'         => ['start', 'date'],
-            'end'           => $this->date('end'),
+            'end'           => ['end', 'date'],
-            'amount'        => $this->string('amount'),
+            'amount'        => ['amount', 'string'],
-            'currency_id'   => $this->integer('currency_id'),
+            'currency_id'   => ['currency_id', 'integer'],
-            'currency_code' => $this->string('currency_code'),
+            'currency_code' => ['currency_code', 'string'],
         ];
+
+        return $this->getAllData($fields);
     }
 
     /**
@@ -60,12 +64,37 @@ return [
     public function rules(): array
     {
         return [
-            'start'         => 'before:end|date',
+            'start'         => 'date',
-            'end'           => 'after:start|date',
+            'end'           => 'date',
             'amount'        => 'gt:0',
             'currency_id'   => 'numeric|exists:transaction_currencies,id',
             'currency_code' => 'min:3|max:3|exists:transaction_currencies,code',
         ];
     }
 
+    /**
+     * Configure the validator instance with special rules for after the basic validation rules.
+     *
+     * @param Validator $validator
+     *                            TODO duplicate code.
+     *
+     * @return void
+     */
+    public function withValidator(Validator $validator): void
+    {
+        $validator->after(
+            function (Validator $validator) {
+                // validate start before end only if both are there.
+                $data = $validator->getData();
+                if (array_key_exists('start', $data) && array_key_exists('end', $data)) {
+                    $start = new Carbon($data['start']);
+                    $end   = new Carbon($data['end']);
+                    if ($end->isBefore($start)) {
+                        $validator->errors()->add('end', (string)trans('validation.date_after'));
+                    }
+                }
+            }
+        );
+    }
+
 }

app/Repositories/Budget/BudgetLimitRepository.php

@@ -359,8 +359,8 @@ class BudgetLimitRepository implements BudgetLimitRepositoryInterface
     {
         $budgetLimit->amount     = array_key_exists('amount', $data) ? $data['amount'] : $budgetLimit->amount;
         $budgetLimit->budget_id  = array_key_exists('budget_id', $data) ? $data['budget_id'] : $budgetLimit->budget_id;
-        $budgetLimit->start_date = array_key_exists('start_date', $data) ? $data['start_date']->format('Y-m-d 00:00:00') : $budgetLimit->start_date;
+        $budgetLimit->start_date = array_key_exists('start', $data) ? $data['start']->format('Y-m-d 00:00:00') : $budgetLimit->start_date;
-        $budgetLimit->end_date   = array_key_exists('end_date', $data) ? $data['end_date']->format('Y-m-d 23:59:59') : $budgetLimit->end_date;
+        $budgetLimit->end_date   = array_key_exists('end', $data) ? $data['end']->format('Y-m-d 23:59:59') : $budgetLimit->end_date;
 
         // if no currency has been provided, use the user's default currency:
         $currency = null;
@@ -381,8 +381,6 @@ class BudgetLimitRepository implements BudgetLimitRepositoryInterface
         $budgetLimit->transaction_currency_id = $currency->id;
         $budgetLimit->save();
 
-        Log::debug(sprintf('Updated budget limit with ID #%d and amount %s', $budgetLimit->id, $data['amount']));
-
         return $budgetLimit;
     }
 

app/Services/Internal/Destroy/BillDestroyService.php

@@ -33,16 +33,6 @@ use Log;
  */
 class BillDestroyService
 {
-    /**
-     * Constructor.
-     */
-    public function __construct()
-    {
-        if ('testing' === config('app.env')) {
-            Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this)));
-        }
-    }
-
     /**
      * @param Bill $bill
      */

app/Services/Internal/Destroy/BudgetDestroyService.php

@@ -34,16 +34,6 @@ use Log;
  */
 class BudgetDestroyService
 {
-    /**
-     * Constructor.
-     */
-    public function __construct()
-    {
-        if ('testing' === config('app.env')) {
-            Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this)));
-        }
-    }
-
     /**
      * @param Budget $budget
      */

app/Services/Internal/Destroy/CategoryDestroyService.php

@@ -34,16 +34,6 @@ use Log;
  */
 class CategoryDestroyService
 {
-    /**
-     * Constructor.
-     */
-    public function __construct()
-    {
-        if ('testing' === config('app.env')) {
-            Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this)));
-        }
-    }
-
     /**
      * @param Category $category
      */

app/Validation/AccountValidator.php

@@ -67,10 +67,6 @@ class AccountValidator
 
         /** @var AccountRepositoryInterface accountRepository */
         $this->accountRepository = app(AccountRepositoryInterface::class);
-
-        if ('testing' === config('app.env')) {
-            Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this)));
-        }
     }
 
     /**

tests/Api/Models/Account/StoreControllerTest.php

@@ -49,10 +49,8 @@ class StoreControllerTest extends TestCase
 
     /**
      * @param array $submission
-     *
+     * emptyDataProvider / storeDataProvider
-     * @dataProvider storeDataProvider
+     * @dataProvider emptyDataProvider
-     *
-     * @ data Provider emptyDataProvider
      */
     public function testStore(array $submission): void
     {

tests/Api/Models/Attachment/StoreControllerTest.php

@@ -51,7 +51,7 @@ class StoreControllerTest extends TestCase
     /**
      * @param array $submission
      *
-     * @ data Provider storeDataProvider
+     * emptyDataProvider / storeDataProvider
      * @dataProvider emptyDataProvider
      */
     public function testStore(array $submission): void

tests/Api/Models/AvailableBudget/StoreControllerTest.php

@@ -51,8 +51,8 @@ class StoreControllerTest extends TestCase
     /**
      * @param array $submission
      *
-     * @dataProvider storeDataProvider
+     * emptyDataProvider / storeDataProvider
-     * @ data Provider emptyDataProvider
+     * @dataProvider emptyDataProvider
      */
     public function testStore(array $submission): void
     {

tests/Api/Models/Bill/StoreControllerTest.php

@@ -51,8 +51,8 @@ class StoreControllerTest extends TestCase
     /**
      * @param array $submission
      *
-     * @dataProvider storeDataProvider
+     * emptyDataProvider / storeDataProvider
-     * @ data Provider emptyDataProvider
+     * @dataProvider emptyDataProvider
      */
     public function testStore(array $submission): void
     {

tests/Api/Models/Budget/StoreControllerTest.php

@@ -51,8 +51,8 @@ class StoreControllerTest extends TestCase
     /**
      * @param array $submission
      *
-     * @dataProvider storeDataProvider
+     * emptyDataProvider / storeDataProvider
-     * @ data Provider emptyDataProvider
+     * @dataProvider emptyDataProvider
      */
     public function testStore(array $submission): void
     {

tests/Api/Models/BudgetLimit/StoreControllerTest.php

@@ -51,8 +51,8 @@ class StoreControllerTest extends TestCase
     /**
      * @param array $submission
      *
-     * @dataProvider storeDataProvider
+     * emptyDataProvider / storeDataProvider
-     * @ data Provider emptyDataProvider
+     * @dataProvider emptyDataProvider
      */
     public function testStore(array $submission): void
     {

tests/Api/Models/BudgetLimit/UpdateControllerTest.php

@@ -58,7 +58,6 @@ class UpdateControllerTest extends TestCase
             'updated_at',
         ];
         $route  = route('api.v1.budgets.limits.update', [$submission['id'], $submission['bl_id']]);
-
         $this->updateAndCompare($route, $submission, $ignore);
     }
 
@@ -100,11 +99,43 @@ class UpdateControllerTest extends TestCase
         $autoBudgetType  = $autoBudgetTypes[rand(0, count($autoBudgetTypes) - 1)];
 
         $set = [
-            'name' => [
+            'currency_id'   => [
                 'id'           => 1,
                 'bl_id'        => 1,
                 'fields'       => [
-                    'amount' => ['test_value' => number_format($faker->randomFloat(2,10,100), 2)],
+                    'currency_id' => ['test_value' => (string)$rand],
+                ],
+                'extra_ignore' => ['currency_code','currency_name','currency_symbol'],
+            ],
+            'currency_code' => [
+                'id'           => 1,
+                'bl_id'        => 1,
+                'fields'       => [
+                    'currency_code' => ['test_value' => $currencies[$rand]],
+                ],
+                'extra_ignore' => ['currency_id','currency_name','currency_symbol'],
+            ],
+            'start'         => [
+                'id'           => 1,
+                'bl_id'        => 1,
+                'fields'       => [
+                    'start' => ['test_value' => $faker->dateTimeBetween('-2 year', '-1 year')->format('Y-m-d')],
+                ],
+                'extra_ignore' => [],
+            ],
+            'end'           => [
+                'id'           => 1,
+                'bl_id'        => 1,
+                'fields'       => [
+                    'end' => ['test_value' => $faker->dateTimeBetween('-1 year', 'now')->format('Y-m-d')],
+                ],
+                'extra_ignore' => [],
+            ],
+            'amount'        => [
+                'id'           => 1,
+                'bl_id'        => 1,
+                'fields'       => [
+                    'amount' => ['test_value' => number_format($faker->randomFloat(2, 10, 100), 2)],
                 ],
                 'extra_ignore' => [],
             ],