diff --git a/app/Api/V1/Controllers/Models/BudgetLimit/ShowController.php b/app/Api/V1/Controllers/Models/BudgetLimit/ShowController.php index 9050781296..4e45182cfc 100644 --- a/app/Api/V1/Controllers/Models/BudgetLimit/ShowController.php +++ b/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! diff --git a/app/Api/V1/Controllers/Models/BudgetLimit/UpdateController.php b/app/Api/V1/Controllers/Models/BudgetLimit/UpdateController.php index 56e2da1058..bad2d4418b 100644 --- a/app/Api/V1/Controllers/Models/BudgetLimit/UpdateController.php +++ b/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(); diff --git a/app/Api/V1/Requests/Models/BudgetLimit/UpdateRequest.php b/app/Api/V1/Requests/Models/BudgetLimit/UpdateRequest.php index aa4b406440..019a849e80 100644 --- a/app/Api/V1/Requests/Models/BudgetLimit/UpdateRequest.php +++ b/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 [ - 'start' => $this->date('start'), - 'end' => $this->date('end'), - 'amount' => $this->string('amount'), - 'currency_id' => $this->integer('currency_id'), - 'currency_code' => $this->string('currency_code'), + $fields = [ + 'start' => ['start', 'date'], + 'end' => ['end', 'date'], + 'amount' => ['amount', 'string'], + 'currency_id' => ['currency_id', 'integer'], + 'currency_code' => ['currency_code', 'string'], ]; + + return $this->getAllData($fields); } /** @@ -60,12 +64,37 @@ return [ public function rules(): array { return [ - 'start' => 'before:end|date', - 'end' => 'after:start|date', + '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')); + } + } + } + ); + } + } diff --git a/app/Repositories/Budget/BudgetLimitRepository.php b/app/Repositories/Budget/BudgetLimitRepository.php index 51074065a8..81cf81138b 100644 --- a/app/Repositories/Budget/BudgetLimitRepository.php +++ b/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->end_date = array_key_exists('end_date', $data) ? $data['end_date']->format('Y-m-d 23:59:59') : $budgetLimit->end_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', $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; } diff --git a/app/Services/Internal/Destroy/BillDestroyService.php b/app/Services/Internal/Destroy/BillDestroyService.php index 34d43a1fd4..735f282ba1 100644 --- a/app/Services/Internal/Destroy/BillDestroyService.php +++ b/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 */ diff --git a/app/Services/Internal/Destroy/BudgetDestroyService.php b/app/Services/Internal/Destroy/BudgetDestroyService.php index 745c4886fd..41280b83fb 100644 --- a/app/Services/Internal/Destroy/BudgetDestroyService.php +++ b/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 */ diff --git a/app/Services/Internal/Destroy/CategoryDestroyService.php b/app/Services/Internal/Destroy/CategoryDestroyService.php index e39ab91700..75ae8e5010 100644 --- a/app/Services/Internal/Destroy/CategoryDestroyService.php +++ b/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 */ diff --git a/app/Validation/AccountValidator.php b/app/Validation/AccountValidator.php index 7cbce0a91a..c544da6e8e 100644 --- a/app/Validation/AccountValidator.php +++ b/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))); - } } /** diff --git a/tests/Api/Models/Account/StoreControllerTest.php b/tests/Api/Models/Account/StoreControllerTest.php index f38e21b42c..6d40968105 100644 --- a/tests/Api/Models/Account/StoreControllerTest.php +++ b/tests/Api/Models/Account/StoreControllerTest.php @@ -49,10 +49,8 @@ class StoreControllerTest extends TestCase /** * @param array $submission - * - * @dataProvider storeDataProvider - * - * @ data Provider emptyDataProvider + * emptyDataProvider / storeDataProvider + * @dataProvider emptyDataProvider */ public function testStore(array $submission): void { diff --git a/tests/Api/Models/Attachment/StoreControllerTest.php b/tests/Api/Models/Attachment/StoreControllerTest.php index 332a36a382..ef83887c76 100644 --- a/tests/Api/Models/Attachment/StoreControllerTest.php +++ b/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 diff --git a/tests/Api/Models/AvailableBudget/StoreControllerTest.php b/tests/Api/Models/AvailableBudget/StoreControllerTest.php index 66c4032bc1..e5d543e7fd 100644 --- a/tests/Api/Models/AvailableBudget/StoreControllerTest.php +++ b/tests/Api/Models/AvailableBudget/StoreControllerTest.php @@ -51,8 +51,8 @@ class StoreControllerTest extends TestCase /** * @param array $submission * - * @dataProvider storeDataProvider - * @ data Provider emptyDataProvider + * emptyDataProvider / storeDataProvider + * @dataProvider emptyDataProvider */ public function testStore(array $submission): void { diff --git a/tests/Api/Models/Bill/StoreControllerTest.php b/tests/Api/Models/Bill/StoreControllerTest.php index 92da32a0d3..c55efb3211 100644 --- a/tests/Api/Models/Bill/StoreControllerTest.php +++ b/tests/Api/Models/Bill/StoreControllerTest.php @@ -51,8 +51,8 @@ class StoreControllerTest extends TestCase /** * @param array $submission * - * @dataProvider storeDataProvider - * @ data Provider emptyDataProvider + * emptyDataProvider / storeDataProvider + * @dataProvider emptyDataProvider */ public function testStore(array $submission): void { diff --git a/tests/Api/Models/Budget/StoreControllerTest.php b/tests/Api/Models/Budget/StoreControllerTest.php index 5705a01008..a37ed941fc 100644 --- a/tests/Api/Models/Budget/StoreControllerTest.php +++ b/tests/Api/Models/Budget/StoreControllerTest.php @@ -51,8 +51,8 @@ class StoreControllerTest extends TestCase /** * @param array $submission * - * @dataProvider storeDataProvider - * @ data Provider emptyDataProvider + * emptyDataProvider / storeDataProvider + * @dataProvider emptyDataProvider */ public function testStore(array $submission): void { diff --git a/tests/Api/Models/BudgetLimit/StoreControllerTest.php b/tests/Api/Models/BudgetLimit/StoreControllerTest.php index 23160ee61e..c3624a3751 100644 --- a/tests/Api/Models/BudgetLimit/StoreControllerTest.php +++ b/tests/Api/Models/BudgetLimit/StoreControllerTest.php @@ -51,8 +51,8 @@ class StoreControllerTest extends TestCase /** * @param array $submission * - * @dataProvider storeDataProvider - * @ data Provider emptyDataProvider + * emptyDataProvider / storeDataProvider + * @dataProvider emptyDataProvider */ public function testStore(array $submission): void { diff --git a/tests/Api/Models/BudgetLimit/UpdateControllerTest.php b/tests/Api/Models/BudgetLimit/UpdateControllerTest.php index 1872483008..546352ef73 100644 --- a/tests/Api/Models/BudgetLimit/UpdateControllerTest.php +++ b/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' => [], ],