diff --git a/app/Api/V1/Controllers/Models/Account/ShowController.php b/app/Api/V1/Controllers/Models/Account/ShowController.php index 7fc3cda55e..aa5dd3ccdc 100644 --- a/app/Api/V1/Controllers/Models/Account/ShowController.php +++ b/app/Api/V1/Controllers/Models/Account/ShowController.php @@ -82,7 +82,7 @@ class ShowController extends Controller $pageSize = (int)app('preferences')->getForUser(auth()->user(), 'listPageSize', 50)->data; // get list of accounts. Count it and split it. - $this->repository->sortAccounts(); + $this->repository->resetAccountOrder(); $collection = $this->repository->getAccountsByType($types); $count = $collection->count(); $accounts = $collection->slice(($this->parameters->get('page') - 1) * $pageSize, $pageSize); @@ -111,7 +111,7 @@ class ShowController extends Controller public function show(Account $account): JsonResponse { // get list of accounts. Count it and split it. - $this->repository->sortAccounts(); + $this->repository->resetAccountOrder(); $account->refresh(); $manager = $this->getManager(); diff --git a/app/Api/V1/Controllers/Models/ObjectGroup/ShowController.php b/app/Api/V1/Controllers/Models/ObjectGroup/ShowController.php index 9048a712f5..267c18d8fc 100644 --- a/app/Api/V1/Controllers/Models/ObjectGroup/ShowController.php +++ b/app/Api/V1/Controllers/Models/ObjectGroup/ShowController.php @@ -78,7 +78,7 @@ class ShowController extends Controller // types to get, page size: $pageSize = (int)app('preferences')->getForUser(auth()->user(), 'listPageSize', 50)->data; - // get list of accounts. Count it and split it. + $this->repository->resetOrder(); $collection = $this->repository->get(); $count = $collection->count(); $objectGroups = $collection->slice(($this->parameters->get('page') - 1) * $pageSize, $pageSize); @@ -108,6 +108,8 @@ class ShowController extends Controller public function show(ObjectGroup $objectGroup): JsonResponse { $manager = $this->getManager(); + $this->repository->resetOrder(); + $objectGroup->refresh(); /** @var ObjectGroupTransformer $transformer */ $transformer = app(ObjectGroupTransformer::class); diff --git a/app/Api/V1/Controllers/Models/ObjectGroup/UpdateController.php b/app/Api/V1/Controllers/Models/ObjectGroup/UpdateController.php index 4e8ace2b9a..87842525d8 100644 --- a/app/Api/V1/Controllers/Models/ObjectGroup/UpdateController.php +++ b/app/Api/V1/Controllers/Models/ObjectGroup/UpdateController.php @@ -70,7 +70,7 @@ class UpdateController extends Controller { $data = $request->getUpdateData(); $this->repository->update($objectGroup, $data); - $this->repository->sort(); + $this->repository->resetOrder(); $manager = $this->getManager(); /** @var ObjectGroupTransformer $transformer */ diff --git a/app/Api/V1/Requests/Models/Category/UpdateRequest.php b/app/Api/V1/Requests/Models/Category/UpdateRequest.php index 4137eb2c5f..befa561963 100644 --- a/app/Api/V1/Requests/Models/Category/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Category/UpdateRequest.php @@ -43,16 +43,11 @@ class UpdateRequest extends FormRequest */ public function getAll(): array { - $notes = null; - $all = $this->all(); - if (array_key_exists('notes', $all)) { - $notes = $this->nlString('notes'); - } - - return [ - 'name' => $this->string('name'), - 'notes' => $notes, + $fields = [ + 'name' => ['name', 'string'], + 'notes' => ['notes', 'nlString'] ]; + return $this->getAllData($fields); } /** @@ -65,7 +60,7 @@ class UpdateRequest extends FormRequest $category = $this->route()->parameter('category'); return [ - 'name' => sprintf('required|between:1,100|uniqueObjectForUser:categories,name,%d', $category->id), + 'name' => sprintf('between:1,100|uniqueObjectForUser:categories,name,%d', $category->id), ]; } } diff --git a/app/Api/V1/Requests/Models/ObjectGroup/UpdateRequest.php b/app/Api/V1/Requests/Models/ObjectGroup/UpdateRequest.php index 21dc7e0a6a..e0e8c282df 100644 --- a/app/Api/V1/Requests/Models/ObjectGroup/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/ObjectGroup/UpdateRequest.php @@ -42,10 +42,11 @@ class UpdateRequest extends FormRequest */ public function getUpdateData(): array { - return [ - 'title' => $this->string('title'), - 'order' => $this->integer('order'), + $fields = [ + 'title' => ['title', 'string'], + 'order' =>['order', 'integer'] ]; + return $this->getAllData($fields); } /** diff --git a/app/Api/V1/Requests/Models/PiggyBank/StoreRequest.php b/app/Api/V1/Requests/Models/PiggyBank/StoreRequest.php index cb287f9313..7a3e264d49 100644 --- a/app/Api/V1/Requests/Models/PiggyBank/StoreRequest.php +++ b/app/Api/V1/Requests/Models/PiggyBank/StoreRequest.php @@ -43,17 +43,24 @@ class StoreRequest extends FormRequest */ public function getAll(): array { - return [ - 'name' => $this->string('name'), - 'account_id' => $this->integer('account_id'), - 'targetamount' => $this->string('target_amount'), - 'current_amount' => $this->string('current_amount'), - 'startdate' => $this->date('start_date'), - 'targetdate' => $this->date('target_date'), - 'notes' => $this->nlString('notes'), - 'object_group_id' => $this->integer('object_group_id'), - 'object_group_title' => $this->string('object_group_title'), + $fields = [ + 'order' => ['order', 'integer'], ]; + $data = $this->getAllData($fields); + + + $data['name'] = $this->string('name'); + $data['account_id'] = $this->integer('account_id'); + $data['targetamount'] = $this->string('target_amount'); + $data['current_amount'] = $this->string('current_amount'); + $data['startdate'] = $this->date('start_date'); + $data['targetdate'] = $this->date('target_date'); + $data['notes'] = $this->nlString('notes'); + $data['object_group_id'] = $this->integer('object_group_id'); + $data['object_group_title'] = $this->string('object_group_title'); + + return $data; + } /** diff --git a/app/Api/V1/Requests/Models/PiggyBank/UpdateRequest.php b/app/Api/V1/Requests/Models/PiggyBank/UpdateRequest.php index dd2cc4f341..e5816dc018 100644 --- a/app/Api/V1/Requests/Models/PiggyBank/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/PiggyBank/UpdateRequest.php @@ -45,24 +45,19 @@ class UpdateRequest extends FormRequest */ public function getAll(): array { - // if the value isn't present, dont return it at all. - // TODO this should be the way to collect fields for all API things. - // TODO make sure piggy bank uses 'start_date' etc. until right up to DB update. - // TODO can we configure this and return it from config? - - // TODO this is the way. $fields = [ - 'name' => ['name', 'string'], - 'account_id' => ['account_id', 'integer'], - 'targetamount' => ['target_amount', 'string'], - 'current_amount' => ['current_amount', 'string'], - 'startdate' => ['start_date', 'date'], - 'targetdate' => ['target_date', 'string'], - 'notes' => ['notes', 'nlString'], - 'order' => ['order', 'integer'], - 'object_group' => ['object_group', 'string'], - 'object_group_id' => ['object_group_id', 'integer'], + 'name' => ['name', 'string'], + 'account_id' => ['account_id', 'integer'], + 'targetamount' => ['target_amount', 'string'], + 'current_amount' => ['current_amount', 'string'], + 'startdate' => ['start_date', 'date'], + 'targetdate' => ['target_date', 'string'], + 'notes' => ['notes', 'nlString'], + 'order' => ['order', 'integer'], + 'object_group_title' => ['object_group_title', 'string'], + 'object_group_id' => ['object_group_id', 'integer'], ]; + return $this->getAllData($fields); } diff --git a/app/Http/Controllers/ObjectGroup/IndexController.php b/app/Http/Controllers/ObjectGroup/IndexController.php index 11fc6a7019..1c8b5e7362 100644 --- a/app/Http/Controllers/ObjectGroup/IndexController.php +++ b/app/Http/Controllers/ObjectGroup/IndexController.php @@ -65,7 +65,7 @@ class IndexController extends Controller public function index() { $this->repository->deleteEmpty(); - $this->repository->sort(); + $this->repository->resetOrder(); $subTitle = (string) trans('firefly.object_groups_index'); $objectGroups = $this->repository->get(); diff --git a/app/Http/Controllers/PiggyBank/IndexController.php b/app/Http/Controllers/PiggyBank/IndexController.php index 1f329f69bf..56df847f53 100644 --- a/app/Http/Controllers/PiggyBank/IndexController.php +++ b/app/Http/Controllers/PiggyBank/IndexController.php @@ -78,7 +78,7 @@ class IndexController extends Controller public function index(Request $request) { $this->cleanupObjectGroups(); - $this->piggyRepos->correctOrder(); + $this->piggyRepos->resetOrder(); $collection = $this->piggyRepos->getPiggyBanks(); $accounts = []; /** @var Carbon $end */ diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index e691133828..e3e7741cc4 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -245,7 +245,10 @@ class AccountRepository implements AccountRepositoryInterface if (!empty($types)) { $query->accountTypeIn($types); } - $query->orderBy('accounts.order', 'ASC'); + $res = array_intersect([AccountType::ASSET, AccountType::MORTGAGE, AccountType::LOAN, AccountType::DEBT], $types); + if (0 !== count($res)) { + $query->orderBy('accounts.order', 'ASC'); + } $query->orderBy('accounts.active', 'DESC'); $query->orderBy('accounts.name', 'ASC'); @@ -590,10 +593,10 @@ class AccountRepository implements AccountRepositoryInterface { $sets = [ [AccountType::DEFAULT, AccountType::ASSET], - [AccountType::EXPENSE, AccountType::BENEFICIARY], - [AccountType::REVENUE], + //[AccountType::EXPENSE, AccountType::BENEFICIARY], + //[AccountType::REVENUE], [AccountType::LOAN, AccountType::DEBT, AccountType::CREDITCARD, AccountType::MORTGAGE], - [AccountType::CASH, AccountType::INITIAL_BALANCE, AccountType::IMPORT, AccountType::RECONCILIATION], + //[AccountType::CASH, AccountType::INITIAL_BALANCE, AccountType::IMPORT, AccountType::RECONCILIATION], ]; foreach ($sets as $set) { Log::debug('Now in resetAccountOrder', $set); @@ -688,53 +691,6 @@ class AccountRepository implements AccountRepositoryInterface $this->user = $user; } - /** - * @inheritDoc - */ - public function sortAccounts(): void - { - // sort assets - $list = $this->user->accounts() - ->leftJoin('account_types', 'accounts.account_type_id', 'account_types.id') - ->where('account_types.type', AccountType::ASSET) - ->orderBy('accounts.order', 'ASC') - ->orderBy('accounts.name', 'ASC') - ->orderBy('accounts.created_at', 'ASC')->get(['accounts.id', 'accounts.order']); - $index = 1; - /** @var Account $account */ - foreach ($list as $account) { - if ($account->order !== $index) { - $account->order = $index; - $account->save(); - } - $index++; - } - - // sort liabilities - $list = $this->user->accounts() - ->leftJoin('account_types', 'accounts.account_type_id', 'account_types.id') - ->whereIn('account_types.type', [AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]) - ->orderBy('accounts.order', 'ASC') - ->orderBy('accounts.name', 'ASC') - ->orderBy('accounts.created_at', 'ASC')->get(['accounts.id', 'accounts.order']); - $index = 1; - /** @var Account $account */ - foreach ($list as $account) { - if ($account->order !== $index) { - $account->order = $index; - $account->save(); - } - $index++; - } - - // set the rest to zero: - $this->user->accounts() - ->leftJoin('account_types', 'accounts.account_type_id', 'account_types.id') - ->whereNotIn('account_types.type', [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]) - ->update(['order' => '0']); - - } - /** * @param array $data * diff --git a/app/Repositories/Account/AccountRepositoryInterface.php b/app/Repositories/Account/AccountRepositoryInterface.php index 7f2ba5b4e5..a26e9a1afe 100644 --- a/app/Repositories/Account/AccountRepositoryInterface.php +++ b/app/Repositories/Account/AccountRepositoryInterface.php @@ -289,11 +289,6 @@ interface AccountRepositoryInterface */ public function setUser(User $user); - /** - * Sort accounts (and fix the sort if necessary). - */ - public function sortAccounts(): void; - /** * @param array $data * diff --git a/app/Repositories/ObjectGroup/ObjectGroupRepository.php b/app/Repositories/ObjectGroup/ObjectGroupRepository.php index 67d402e7e5..ef16cff48e 100644 --- a/app/Repositories/ObjectGroup/ObjectGroupRepository.php +++ b/app/Repositories/ObjectGroup/ObjectGroupRepository.php @@ -36,8 +36,7 @@ use Log; */ class ObjectGroupRepository implements ObjectGroupRepositoryInterface { - /** @var User */ - private $user; + private User $user; /** * @inheritDoc @@ -89,7 +88,8 @@ class ObjectGroupRepository implements ObjectGroupRepositoryInterface { return $this->user->objectGroups() ->with(['piggyBanks', 'bills']) - ->orderBy('order', 'ASC')->orderBy('title', 'ASC')->get(); + ->orderBy('order', 'ASC') + ->orderBy('title', 'ASC')->get(); } /** @@ -133,43 +133,43 @@ class ObjectGroupRepository implements ObjectGroupRepositoryInterface /** * @inheritDoc */ - public function setOrder(ObjectGroup $objectGroup, int $order): ObjectGroup + public function setOrder(ObjectGroup $objectGroup, int $newOrder): ObjectGroup { - $order = 0 === $order ? 1 : $order; - $objectGroup->order = $order; - $objectGroup->save(); + $oldOrder = (int)$objectGroup->order; - Log::debug(sprintf('Objectgroup #%d order is now %d', $objectGroup->id, $order)); + if ($newOrder > $oldOrder) { + $this->user->objectGroups()->where('object_groups.order', '<=', $newOrder)->where('object_groups.order', '>', $oldOrder) + ->where('object_groups.id', '!=', $objectGroup->id) + ->decrement('object_groups.order', 1); + + $objectGroup->order = $newOrder; + $objectGroup->save(); + } + if ($newOrder < $oldOrder) { + $this->user->objectGroups()->where('object_groups.order', '>=', $newOrder)->where('object_groups.order', '<', $oldOrder) + ->where('object_groups.id', '!=', $objectGroup->id) + ->increment('object_groups.order', 1); + + $objectGroup->order = $newOrder; + $objectGroup->save(); + } + + Log::debug(sprintf('Objectgroup #%d order is now %d', $objectGroup->id, $newOrder)); return $objectGroup; } - /** - * @inheritDoc - */ - public function sort(): void - { - $all = $this->get(); - /** - * @var int $index - * @var ObjectGroup $group - */ - foreach ($all as $index => $group) { - $group->order = $index + 1; - $group->save(); - } - } - /** * @inheritDoc */ public function update(ObjectGroup $objectGroup, array $data): ObjectGroup { - $objectGroup->title = $data['title']; + if(array_key_exists('title', $data)) { + $objectGroup->title = $data['title']; + } - if (isset($data['order'])) { - $order = 0 === $data['order'] ? 1 : $data['order']; - $objectGroup->order = $order; + if(array_key_exists('order', $data)) { + $this->setOrder($objectGroup, (int)$data['order']); } $objectGroup->save(); @@ -184,4 +184,25 @@ class ObjectGroupRepository implements ObjectGroupRepositoryInterface { $this->user = $user; } + + /** + * @inheritDoc + */ + public function resetOrder(): void + { + Log::debug('Now in resetOrder'); + $list = $this->get(); + $index = 1; + /** @var ObjectGroup $objectGroup */ + foreach ($list as $objectGroup) { + if ($index !== (int)$objectGroup->order) { + Log::debug( + sprintf('objectGroup #%d ("%s"): order should %d be but is %d.', $objectGroup->id, $objectGroup->title, $index, $objectGroup->order) + ); + $objectGroup->order = $index; + $objectGroup->save(); + } + $index++; + } + } } diff --git a/app/Repositories/ObjectGroup/ObjectGroupRepositoryInterface.php b/app/Repositories/ObjectGroup/ObjectGroupRepositoryInterface.php index 9f377cd821..be3c9a84a4 100644 --- a/app/Repositories/ObjectGroup/ObjectGroupRepositoryInterface.php +++ b/app/Repositories/ObjectGroup/ObjectGroupRepositoryInterface.php @@ -37,6 +37,11 @@ interface ObjectGroupRepositoryInterface */ public function deleteAll(): void; + /** + * Delete all. + */ + public function resetOrder(): void; + /** * Delete empty ones. */ @@ -76,16 +81,11 @@ interface ObjectGroupRepositoryInterface /** * @param ObjectGroup $objectGroup - * @param int $index + * @param int $newOrder * * @return ObjectGroup */ - public function setOrder(ObjectGroup $objectGroup, int $index): ObjectGroup; - - /** - * Sort - */ - public function sort(): void; + public function setOrder(ObjectGroup $objectGroup, int $newOrder): ObjectGroup; /** * @param ObjectGroup $objectGroup diff --git a/app/Repositories/ObjectGroup/OrganisesObjectGroups.php b/app/Repositories/ObjectGroup/OrganisesObjectGroups.php index 62e75ee93b..1b1e53aaa7 100644 --- a/app/Repositories/ObjectGroup/OrganisesObjectGroups.php +++ b/app/Repositories/ObjectGroup/OrganisesObjectGroups.php @@ -53,6 +53,6 @@ trait OrganisesObjectGroups private function sortObjectGroups(): void { $repository = app(ObjectGroupRepositoryInterface::class); - $repository->sort(); + $repository->resetOrder(); } } diff --git a/app/Repositories/PiggyBank/ModifiesPiggyBanks.php b/app/Repositories/PiggyBank/ModifiesPiggyBanks.php index fd3411542b..5aad8ddb3f 100644 --- a/app/Repositories/PiggyBank/ModifiesPiggyBanks.php +++ b/app/Repositories/PiggyBank/ModifiesPiggyBanks.php @@ -26,7 +26,6 @@ namespace FireflyIII\Repositories\PiggyBank; use Carbon\Carbon; -use DB; use Exception; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\Note; @@ -35,7 +34,6 @@ use FireflyIII\Models\PiggyBankEvent; use FireflyIII\Models\PiggyBankRepetition; use FireflyIII\Models\TransactionJournal; use FireflyIII\Repositories\ObjectGroup\CreatesObjectGroups; -use FireflyIII\User; use Illuminate\Database\QueryException; use Log; @@ -127,12 +125,13 @@ trait ModifiesPiggyBanks /** * Correct order of piggies in case of issues. */ - public function correctOrder(): void + public function resetOrder(): void { - $set = $this->user->piggyBanks()->orderBy('order', 'ASC')->get(); + $set = $this->user->piggyBanks()->orderBy('piggy_banks.order', 'ASC')->get(['piggy_banks.*']); $current = 1; foreach ($set as $piggyBank) { if ((int)$piggyBank->order !== $current) { + Log::debug(sprintf('Piggy bank #%d ("%s") was at place %d but should be on %d', $piggyBank->id, $piggyBank->name, $piggyBank->order, $current)); $piggyBank->order = $current; $piggyBank->save(); } @@ -259,16 +258,28 @@ trait ModifiesPiggyBanks } /** - * set id of piggy bank. - * - * @param PiggyBank $piggyBank - * @param int $order - * - * @return bool + * @inheritDoc */ - public function setOrder(PiggyBank $piggyBank, int $order): bool + public function setOrder(PiggyBank $piggyBank, int $newOrder): bool { - $piggyBank->order = $order; + $oldOrder = (int)$piggyBank->order; + Log::debug(sprintf('Will move piggy bank #%d ("%s") from %d to %d', $piggyBank->id, $piggyBank->name, $oldOrder, $newOrder)); + if ($newOrder > $oldOrder) { + $this->user->piggyBanks()->where('piggy_banks.order', '<=', $newOrder)->where('piggy_banks.order', '>', $oldOrder) + ->where('piggy_banks.id', '!=', $piggyBank->id) + ->decrement('piggy_banks.order', 1); + $piggyBank->order = $newOrder; + Log::debug(sprintf('Order of piggy #%d ("%s") is now %d', $piggyBank->id, $piggyBank->name, $newOrder)); + $piggyBank->save(); + + return true; + } + + $this->user->piggyBanks()->where('piggy_banks.order', '>=', $newOrder)->where('piggy_banks.order', '<', $oldOrder) + ->where('piggy_banks.id', '!=', $piggyBank->id) + ->increment('piggy_banks.order', 1); + $piggyBank->order = $newOrder; + Log::debug(sprintf('Order of piggy #%d ("%s") is now %d', $piggyBank->id, $piggyBank->name, $newOrder)); $piggyBank->save(); return true; @@ -282,7 +293,11 @@ trait ModifiesPiggyBanks */ public function store(array $data): PiggyBank { - $data['order'] = $this->getMaxOrder() + 1; + $order = $this->getMaxOrder() + 1; + if (array_key_exists('order', $data)) { + $order = $data['order']; + } + $data['order'] = 31337; // very high when creating. $piggyData = $data; // unset fields just in case. unset($piggyData['object_group_title'], $piggyData['object_group_id'], $piggyData['notes'], $piggyData['current_amount']); @@ -294,6 +309,10 @@ trait ModifiesPiggyBanks throw new FireflyException('400005: Could not store new piggy bank.'); } + // reset order then set order: + $this->resetOrder(); + $this->setOrder($piggyBank, $order); + $this->updateNote($piggyBank, $data['notes']); // repetition is auto created. @@ -334,13 +353,15 @@ trait ModifiesPiggyBanks public function update(PiggyBank $piggyBank, array $data): PiggyBank { $piggyBank = $this->updateProperties($piggyBank, $data); - $this->updateNote($piggyBank, $data['notes'] ?? ''); + if (array_key_exists('notes', $data)) { + $this->updateNote($piggyBank, (string)$data['notes']); + } // update the order of the piggy bank: $oldOrder = (int)$piggyBank->order; $newOrder = (int)($data['order'] ?? $oldOrder); if ($oldOrder !== $newOrder) { - $this->updateOrder($piggyBank, $oldOrder, $newOrder); + $this->setOrder($piggyBank, $newOrder); } // if the piggy bank is now smaller than the current relevant rep, @@ -355,8 +376,8 @@ trait ModifiesPiggyBanks } // update using name: - if (array_key_exists('object_group', $data)) { - $objectGroupTitle = (string)$data['object_group']; + if (array_key_exists('object_group_title', $data)) { + $objectGroupTitle = (string)$data['object_group_title']; if ('' !== $objectGroupTitle) { $objectGroup = $this->findOrCreateObjectGroup($objectGroupTitle); if (null !== $objectGroup) { @@ -441,46 +462,12 @@ trait ModifiesPiggyBanks if (array_key_exists('targetdate', $data) && '' !== $data['targetdate']) { $piggyBank->targetdate = $data['targetdate']; } - $piggyBank->startdate = $data['startdate'] ?? $piggyBank->startdate; + if (array_key_exists('startdate', $data)) { + $piggyBank->startdate = $data['startdate']; + + } $piggyBank->save(); return $piggyBank; } - - /** - * @param PiggyBank $piggyBank - * @param int $oldOrder - * @param int $newOrder - */ - private function updateOrder(PiggyBank $piggyBank, int $oldOrder, int $newOrder): void - { - if ($newOrder > $oldOrder) { - // Iedereen [7 en lager] [hoger dan 3] behalve piggy zelf, puntje er af: - //piggy zelf naar 7 - /** @var User $user */ - $user = $this->user; - $user->piggyBanks()->where('piggy_banks.order', '<=', $newOrder)->where('piggy_banks.order', '>', $oldOrder) - ->where('piggy_banks.id', '!=', $piggyBank->id) - ->decrement('piggybanks.order',1); - - $piggyBank->order = $newOrder; - $piggyBank->save(); - } - if ($newOrder < $oldOrder) { - // - //Van 8 naar 2 - // iedereen [2 of hoger] en [kleiner dan 8] puntje er bij. - // 8 naar 2 - /** @var User $user */ - $user = $this->user; - $user->piggyBanks()->where('piggy_banks.order', '>=', $newOrder)->where('piggy_banks.order', '<', $oldOrder) - ->where('piggy_banks.id', '!=', $piggyBank->id) - ->increment('piggybanks.order',1); - - $piggyBank->order = $newOrder; - $piggyBank->save(); - } - - } - } diff --git a/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php b/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php index 59f392d173..3f891f2c94 100644 --- a/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php +++ b/app/Repositories/PiggyBank/PiggyBankRepositoryInterface.php @@ -71,7 +71,7 @@ interface PiggyBankRepositoryInterface /** * Correct order of piggies in case of issues. */ - public function correctOrder(): void; + public function resetOrder(): void; /** * Create a new event. @@ -267,11 +267,11 @@ interface PiggyBankRepositoryInterface * Set specific piggy bank to specific order. * * @param PiggyBank $piggyBank - * @param int $order + * @param int $newOrder * * @return bool */ - public function setOrder(PiggyBank $piggyBank, int $order): bool; + public function setOrder(PiggyBank $piggyBank, int $newOrder): bool; /** * @param User $user diff --git a/app/Services/Internal/Destroy/CurrencyDestroyService.php b/app/Services/Internal/Destroy/CurrencyDestroyService.php index e82fe525f8..bdec425c5d 100644 --- a/app/Services/Internal/Destroy/CurrencyDestroyService.php +++ b/app/Services/Internal/Destroy/CurrencyDestroyService.php @@ -33,16 +33,6 @@ use Log; */ class CurrencyDestroyService { - /** - * 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 TransactionCurrency $currency */ diff --git a/app/Services/Internal/Destroy/JournalDestroyService.php b/app/Services/Internal/Destroy/JournalDestroyService.php index 2c9a9867b1..321c3be0ae 100644 --- a/app/Services/Internal/Destroy/JournalDestroyService.php +++ b/app/Services/Internal/Destroy/JournalDestroyService.php @@ -38,16 +38,6 @@ use Log; */ class JournalDestroyService { - /** - * 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 TransactionJournal $journal */ diff --git a/app/Services/Internal/Destroy/RecurrenceDestroyService.php b/app/Services/Internal/Destroy/RecurrenceDestroyService.php index c1f7bebb49..6e704bbb2b 100644 --- a/app/Services/Internal/Destroy/RecurrenceDestroyService.php +++ b/app/Services/Internal/Destroy/RecurrenceDestroyService.php @@ -34,16 +34,6 @@ use Log; */ class RecurrenceDestroyService { - /** - * Constructor. - */ - public function __construct() - { - if ('testing' === config('app.env')) { - Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this))); - } - } - /** * Delete recurrence. * diff --git a/app/Services/Internal/Update/CategoryUpdateService.php b/app/Services/Internal/Update/CategoryUpdateService.php index 5a00fd9da5..819db8b38d 100644 --- a/app/Services/Internal/Update/CategoryUpdateService.php +++ b/app/Services/Internal/Update/CategoryUpdateService.php @@ -46,9 +46,6 @@ class CategoryUpdateService */ public function __construct() { - if ('testing' === config('app.env')) { - Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this))); - } if (auth()->check()) { $this->user = auth()->user(); } @@ -71,13 +68,15 @@ class CategoryUpdateService public function update(Category $category, array $data): Category { $oldName = $category->name; - $category->name = $data['name']; - $category->save(); + if(array_key_exists('name', $data)) { + $category->name = $data['name']; + $category->save(); + // update triggers and actions + $this->updateRuleTriggers($oldName, $data['name']); + $this->updateRuleActions($oldName, $data['name']); + $this->updateRecurrences($oldName, $data['name']); + } - // update triggers and actions - $this->updateRuleTriggers($oldName, $data['name']); - $this->updateRuleActions($oldName, $data['name']); - $this->updateRecurrences($oldName, $data['name']); $this->updateNotes($category, $data); return $category; diff --git a/app/Services/Internal/Update/CurrencyUpdateService.php b/app/Services/Internal/Update/CurrencyUpdateService.php index cb29b3c917..b1d99f0e05 100644 --- a/app/Services/Internal/Update/CurrencyUpdateService.php +++ b/app/Services/Internal/Update/CurrencyUpdateService.php @@ -33,16 +33,6 @@ use Log; */ class CurrencyUpdateService { - /** - * 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 TransactionCurrency $currency * @param array $data diff --git a/app/Services/Password/PwndVerifierV2.php b/app/Services/Password/PwndVerifierV2.php index a5588f0b5b..5afdb5e27c 100644 --- a/app/Services/Password/PwndVerifierV2.php +++ b/app/Services/Password/PwndVerifierV2.php @@ -34,16 +34,6 @@ use RuntimeException; */ class PwndVerifierV2 implements Verifier { - /** - * Constructor. - */ - public function __construct() - { - if ('testing' === config('app.env')) { - Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this))); - } - } - /** * Verify the given password against (some) service. * diff --git a/app/Support/Chart/Category/WholePeriodChartGenerator.php b/app/Support/Chart/Category/WholePeriodChartGenerator.php index c6c7c45bd4..e957d83384 100644 --- a/app/Support/Chart/Category/WholePeriodChartGenerator.php +++ b/app/Support/Chart/Category/WholePeriodChartGenerator.php @@ -37,16 +37,6 @@ use Log; */ class WholePeriodChartGenerator { - /** - * 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 * @param Carbon $start diff --git a/app/Transformers/AccountTransformer.php b/app/Transformers/AccountTransformer.php index dfc8a909f0..711f134c9d 100644 --- a/app/Transformers/AccountTransformer.php +++ b/app/Transformers/AccountTransformer.php @@ -27,15 +27,13 @@ namespace FireflyIII\Transformers; use Carbon\Carbon; use FireflyIII\Models\Account; use FireflyIII\Repositories\Account\AccountRepositoryInterface; -use Log; /** * Class AccountTransformer */ class AccountTransformer extends AbstractTransformer { - /** @var AccountRepositoryInterface */ - protected $repository; + protected AccountRepositoryInterface $repository; /** * @@ -61,8 +59,8 @@ class AccountTransformer extends AbstractTransformer // get account type: $fullType = $account->accountType->type; - $accountType = (string) config(sprintf('firefly.shortNamesByFullName.%s', $fullType)); - $liabilityType = (string) config(sprintf('firefly.shortLiabilityNameByFullName.%s', $fullType)); + $accountType = (string)config(sprintf('firefly.shortNamesByFullName.%s', $fullType)); + $liabilityType = (string)config(sprintf('firefly.shortLiabilityNameByFullName.%s', $fullType)); $liabilityType = '' === $liabilityType ? null : strtolower($liabilityType); // get account role (will only work if the type is asset. @@ -75,7 +73,7 @@ class AccountTransformer extends AbstractTransformer [$openingBalance, $openingBalanceDate] = $this->getOpeningBalance($account, $accountType); [$interest, $interestPeriod] = $this->getInterest($account, $accountType); - $openingBalance = number_format((float) $openingBalance, $decimalPlaces, '.', ''); + $openingBalance = number_format((float)$openingBalance, $decimalPlaces, '.', ''); $includeNetWorth = '0' !== $this->repository->getMetaValue($account, 'include_net_worth'); $longitude = null; $latitude = null; @@ -84,14 +82,21 @@ class AccountTransformer extends AbstractTransformer if (null !== $location) { $longitude = $location->longitude; $latitude = $location->latitude; - $zoomLevel = (int) $location->zoom_level; + $zoomLevel = (int)$location->zoom_level; } + + // no order for some accounts: + $order = (int)$account->order; + if (!in_array(strtolower($accountType), ['liability', 'liabilities', 'asset'])) { + $order = null; + } + return [ - 'id' => (string) $account->id, + 'id' => (string)$account->id, 'created_at' => $account->created_at->toAtomString(), 'updated_at' => $account->updated_at->toAtomString(), 'active' => $account->active, - 'order' => (int) $account->order, + 'order' => $order, 'name' => $account->name, 'type' => strtolower($accountType), 'account_role' => $accountRole, @@ -99,7 +104,7 @@ class AccountTransformer extends AbstractTransformer 'currency_code' => $currencyCode, 'currency_symbol' => $currencySymbol, 'currency_decimal_places' => $decimalPlaces, - 'current_balance' => number_format((float) app('steam')->balance($account, $date), $decimalPlaces, '.', ''), + 'current_balance' => number_format((float)app('steam')->balance($account, $date), $decimalPlaces, '.', ''), 'current_balance_date' => $date->format('Y-m-d'), 'notes' => $this->repository->getNoteText($account), 'monthly_payment_date' => $monthlyPaymentDate, @@ -107,11 +112,11 @@ class AccountTransformer extends AbstractTransformer 'account_number' => $this->repository->getMetaValue($account, 'account_number'), 'iban' => '' === $account->iban ? null : $account->iban, 'bic' => $this->repository->getMetaValue($account, 'BIC'), - 'virtual_balance' => number_format((float) $account->virtual_balance, $decimalPlaces, '.', ''), + 'virtual_balance' => number_format((float)$account->virtual_balance, $decimalPlaces, '.', ''), 'opening_balance' => $openingBalance, 'opening_balance_date' => $openingBalanceDate, 'liability_type' => $liabilityType, - 'interest' => (float) $interest, + 'interest' => (float)$interest, 'interest_period' => $interestPeriod, 'include_net_worth' => $includeNetWorth, 'longitude' => $longitude, @@ -136,7 +141,7 @@ class AccountTransformer extends AbstractTransformer private function getAccountRole(Account $account, string $accountType): ?string { $accountRole = $this->repository->getMetaValue($account, 'account_role'); - if ('asset' !== $accountType || '' === (string) $accountRole) { + if ('asset' !== $accountType || '' === (string)$accountRole) { $accountRole = null; } @@ -175,7 +180,7 @@ class AccountTransformer extends AbstractTransformer if (null === $currency) { $currency = app('amount')->getDefaultCurrencyByUser($account->user); } - $currencyId = (string) $currency->id; + $currencyId = (string)$currency->id; $currencyCode = $currency->code; $decimalPlaces = $currency->decimal_places; $currencySymbol = $currency->symbol; diff --git a/app/Transformers/ObjectGroupTransformer.php b/app/Transformers/ObjectGroupTransformer.php index 517b607303..7641fb341a 100644 --- a/app/Transformers/ObjectGroupTransformer.php +++ b/app/Transformers/ObjectGroupTransformer.php @@ -43,10 +43,6 @@ class ObjectGroupTransformer extends AbstractTransformer */ public function __construct() { - if ('testing' === config('app.env')) { - Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this))); - } - $this->repository = app(ObjectGroupRepositoryInterface::class); } @@ -62,15 +58,15 @@ class ObjectGroupTransformer extends AbstractTransformer $this->repository->setUser($objectGroup->user); return [ - 'id' => (int) $objectGroup->id, + 'id' => (string) $objectGroup->id, 'created_at' => $objectGroup->created_at->toAtomString(), 'updated_at' => $objectGroup->updated_at->toAtomString(), 'title' => $objectGroup->title, - 'order' => $objectGroup->order, + 'order' => (int) $objectGroup->order, 'links' => [ [ 'rel' => 'self', - 'uri' => '/groups/' . $objectGroup->id, + 'uri' => '/object_groups/' . $objectGroup->id, ], ], ]; diff --git a/phpunit.xml b/phpunit.xml index 09f8267509..999d2411bd 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -36,7 +36,7 @@ - ./tests/Api/Models + ./tests/Api/Models/PiggyBank diff --git a/tests/Api/Models/Account/StoreControllerTest.php b/tests/Api/Models/Account/StoreControllerTest.php index fe72c6a2a6..735d04a221 100644 --- a/tests/Api/Models/Account/StoreControllerTest.php +++ b/tests/Api/Models/Account/StoreControllerTest.php @@ -50,6 +50,7 @@ class StoreControllerTest extends TestCase /** * @param array $submission * emptyDataProvider / storeDataProvider + * * @dataProvider storeDataProvider */ public function testStore(array $submission): void @@ -82,7 +83,7 @@ class StoreControllerTest extends TestCase 'name' => function () { $faker = Factory::create(); - return $faker->name; + return $faker->uuid; }, 'iban' => function () { $faker = Factory::create(); @@ -104,15 +105,14 @@ class StoreControllerTest extends TestCase */ private function optionalSets(): array { - $faker = Factory::create(); - $currencies = [ + $faker = Factory::create(); + $currencies = [ 1 => 'EUR', 2 => 'HUF', 3 => 'GBP', 4 => 'UAH', ]; - $rand = rand(1, 4); - // rand + $rand = rand(1, 4); return [ @@ -192,7 +192,7 @@ class StoreControllerTest extends TestCase return [ 'asset' => [ 'parameters' => [], - 'fields' => [ + 'fields' => [ 'name' => $faker->uuid, 'type' => 'asset', 'account_role' => $this->randomAccountRole(), @@ -200,14 +200,14 @@ class StoreControllerTest extends TestCase ], 'expense' => [ 'parameters' => [], - 'fields' => [ + 'fields' => [ 'name' => $faker->uuid, 'type' => 'expense', ], ], 'liability' => [ 'parameters' => [], - 'fields' => [ + 'fields' => [ 'name' => $faker->uuid, 'type' => 'liabilities', 'liability_type' => $this->randomLiabilityType(), @@ -216,6 +216,9 @@ class StoreControllerTest extends TestCase 'interest' => $this->getRandomPercentage(), 'interest_period' => $this->getRandomInterestPeriod(), ], + 'ignore' => [ + 'opening_balance', 'opening_balance_date', + ], ], 'cc' => [ 'fields' => [ diff --git a/tests/Api/Models/Category/StoreControllerTest.php b/tests/Api/Models/Category/StoreControllerTest.php new file mode 100644 index 0000000000..c421293473 --- /dev/null +++ b/tests/Api/Models/Category/StoreControllerTest.php @@ -0,0 +1,128 @@ +. + */ + +namespace Tests\Api\Models\Category; + + +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 StoreControllerTest + */ +class StoreControllerTest 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))); + } + + + /** + * @param array $submission + * + * emptyDataProvider / storeDataProvider + * + * @dataProvider storeDataProvider + */ + public function testStore(array $submission): void + { + if ([] === $submission) { + $this->markTestSkipped('Empty data provider'); + } + $route = 'api.v1.categories.store'; + $this->storeAndCompare($route, $submission); + } + + /** + * @return array + */ + public function emptyDataProvider(): array + { + return [[[]]]; + + } + + /** + * @return array + */ + public function storeDataProvider(): array + { + $minimalSets = $this->minimalSets(); + $optionalSets = $this->optionalSets(); + $regenConfig = [ + 'name' => function () { + $faker = Factory::create(); + + return $faker->uuid; + }, + ]; + + return $this->genericDataProvider($minimalSets, $optionalSets, $regenConfig); + } + + /** + * @return array + */ + private function minimalSets(): array + { + $faker = Factory::create(); + + return [ + 'default_cat' => [ + 'parameters' => [1], + 'fields' => [ + 'name' => $faker->uuid, + ], + ], + ]; + } + + + /** + * @return \array[][] + */ + private function optionalSets(): array + { + $faker = Factory::create(); + + return [ + 'notes' => [ + 'fields' => [ + 'notes' => join(' ', $faker->words(5)), + ], + ], + ]; + } + +} \ No newline at end of file diff --git a/tests/Api/Models/Category/UpdateControllerTest.php b/tests/Api/Models/Category/UpdateControllerTest.php new file mode 100644 index 0000000000..d8ce962166 --- /dev/null +++ b/tests/Api/Models/Category/UpdateControllerTest.php @@ -0,0 +1,108 @@ +. + */ + +namespace Tests\Api\Models\Category; + + +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.categories.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(); + $set = [ + 'name' => [ + 'id' => 1, + 'fields' => [ + 'name' => ['test_value' => $faker->uuid], + ], + 'extra_ignore' => [], + ], + 'notes' => [ + 'id' => 1, + 'fields' => [ + 'notes' => ['test_value' => join(' ',$faker->words(5))], + ], + 'extra_ignore' => [], + ], + ]; + + return $set; + } + + +} \ No newline at end of file diff --git a/tests/Api/Models/ObjectGroup/UpdateControllerTest.php b/tests/Api/Models/ObjectGroup/UpdateControllerTest.php new file mode 100644 index 0000000000..270821f233 --- /dev/null +++ b/tests/Api/Models/ObjectGroup/UpdateControllerTest.php @@ -0,0 +1,108 @@ +. + */ + +namespace Tests\Api\Models\ObjectGroup; + + +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.object-groups.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(); + $set = [ + 'title' => [ + 'id' => 1, + 'fields' => [ + 'title' => ['test_value' => $faker->uuid], + ], + 'extra_ignore' => [], + ], + 'order' => [ + 'id' => 1, + 'fields' => [ + 'order' => ['test_value' => $faker->numberBetween(1, 2)], + ], + 'extra_ignore' => [], + ], + ]; + + return $set; + } + + +} \ No newline at end of file diff --git a/tests/Api/Models/PiggyBank/StoreControllerTest.php b/tests/Api/Models/PiggyBank/StoreControllerTest.php new file mode 100644 index 0000000000..d9ce390712 --- /dev/null +++ b/tests/Api/Models/PiggyBank/StoreControllerTest.php @@ -0,0 +1,163 @@ +. + */ + +namespace Tests\Api\Models\PiggyBank; + + +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 StoreControllerTest + */ +class StoreControllerTest 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))); + } + + + /** + * @param array $submission + * + * emptyDataProvider / storeDataProvider + * + * @dataProvider storeDataProvider + */ + public function testStore(array $submission): void + { + if ([] === $submission) { + $this->markTestSkipped('Empty data provider'); + } + $route = 'api.v1.piggy_banks.store'; + $this->storeAndCompare($route, $submission); + } + + /** + * @return array + */ + public function emptyDataProvider(): array + { + return [[[]]]; + + } + + /** + * @return array + */ + public function storeDataProvider(): array + { + $minimalSets = $this->minimalSets(); + $optionalSets = $this->optionalSets(); + $regenConfig = [ + 'name' => function () { + $faker = Factory::create(); + + return $faker->uuid; + }, + ]; + + return $this->genericDataProvider($minimalSets, $optionalSets, $regenConfig); + } + + /** + * @return array + */ + private function minimalSets(): array + { + $faker = Factory::create(); + + return [ + 'default_piggy' => [ + 'parameters' => [], + 'fields' => [ + 'name' => $faker->uuid, + 'account_id' => $faker->numberBetween(1, 3), + 'target_amount' => number_format($faker->randomFloat(2, 50, 100), 2), + ], + ], + ]; + } + + + /** + * @return \array[][] + */ + private function optionalSets(): array + { + $faker = Factory::create(); + + $objectGroupId = $faker->numberBetween(1, 2); + $objectGroupName = sprintf('Object group %d', $objectGroupId); + + return [ + 'current_amount' => [ + 'fields' => [ + 'current_amount' => number_format($faker->randomFloat(2, 10, 50), 2), + ], + ], + 'start_date' => [ + 'fields' => [ + 'start_date' => $faker->dateTimeBetween('-2 year', '-1 year')->format('Y-m-d'), + ], + ], + 'target_date' => [ + 'fields' => [ + 'target_date' => $faker->dateTimeBetween('+1 year', '+2 year')->format('Y-m-d'), + ], + ], + 'order' => [ + 'fields' => [ + 'order' => $faker->numberBetween(1, 5), + ], + ], + 'object_group_id' => [ + 'fields' => [ + 'object_group_id' => $objectGroupId, + ], + ], + 'object_group_title' => [ + 'fields' => [ + 'object_group_title' => $objectGroupName, + ], + ], + 'notes' => [ + 'fields' => [ + 'notes' => join(' ', $faker->words(5)), + ], + ], + ]; + } + +} \ No newline at end of file diff --git a/tests/Api/Models/PiggyBank/UpdateControllerTest.php b/tests/Api/Models/PiggyBank/UpdateControllerTest.php new file mode 100644 index 0000000000..e4a9fbb38d --- /dev/null +++ b/tests/Api/Models/PiggyBank/UpdateControllerTest.php @@ -0,0 +1,166 @@ +. + */ + +namespace Tests\Api\Models\PiggyBank; + + +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.piggy_banks.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(); + $objectGroupId = $faker->numberBetween(1, 2); + $objectGroupName = sprintf('Object group %d', $objectGroupId); + $set = [ + 'name' => [ + 'id' => 1, + 'fields' => [ + 'name' => ['test_value' => $faker->uuid], + ], + 'extra_ignore' => [], + ], + 'account_id' => [ + 'id' => 1, + 'fields' => [ + 'account_id' => ['test_value' => (string)$faker->numberBetween(1, 3)], + ], + 'extra_ignore' => ['account_name'], + ], + 'target_amount' => [ + 'id' => 1, + 'fields' => [ + 'target_amount' => ['test_value' => number_format($faker->randomFloat(2, 50, 100), 2)], + ], + 'extra_ignore' => ['percentage', 'current_amount', 'left_to_save'], + ], + 'current_amount' => [ + 'id' => 1, + 'fields' => [ + 'current_amount' => ['test_value' => number_format($faker->randomFloat(2, 5, 10), 2)], + ], + 'extra_ignore' => ['percentage', 'left_to_save'], + ], + 'start_date' => [ + 'id' => 1, + 'fields' => [ + 'start_date' => ['test_value' => $faker->dateTimeBetween('-2 year', '-1 year')->format('Y-m-d')], + ], + 'extra_ignore' => [], + ], + 'target_date' => [ + 'id' => 1, + 'fields' => [ + 'target_date' => ['test_value' => $faker->dateTimeBetween('+1 year', '+2 year')->format('Y-m-d')], + ], + 'extra_ignore' => ['save_per_month'], + ], + 'order' => [ + 'id' => 1, + 'fields' => [ + 'order' => ['test_value' => $faker->numberBetween(1, 5)], + ], + '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'], + ], + ]; + + return $set; + } + + +} \ No newline at end of file diff --git a/tests/Traits/TestHelpers.php b/tests/Traits/TestHelpers.php index da4e2709cc..6a20ec6bef 100644 --- a/tests/Traits/TestHelpers.php +++ b/tests/Traits/TestHelpers.php @@ -42,13 +42,21 @@ trait TestHelpers protected function genericDataProvider(array $minimalSets, array $startOptionalSets, array $regenConfig): array { $submissions = []; - foreach ($minimalSets as $set) { + /** + * @var string $name + * @var array $set + */ + foreach ($minimalSets as $name => $set) { $body = []; foreach ($set['fields'] as $field => $value) { $body[$field] = $value; } // minimal set is part of all submissions: - $submissions[] = [['fields' => $body, 'parameters' => $set['parameters'] ?? []]]; + $submissions[] = [[ + 'fields' => $body, + 'parameters' => $set['parameters'] ?? [], + 'ignore' => $set['ignore'] ?? [], + ]]; // then loop and add fields: $optionalSets = $startOptionalSets; @@ -59,6 +67,7 @@ trait TestHelpers // expand body with N extra fields: foreach ($combinations as $extraFields) { $second = $body; + $ignore = $set['ignore'] ?? []; // unused atm. foreach ($extraFields as $extraField) { // now loop optional sets on $extraField and add whatever the config is: foreach ($optionalSets[$extraField]['fields'] as $newField => $newValue) { @@ -67,7 +76,11 @@ trait TestHelpers } $second = $this->regenerateValues($second, $regenConfig); - $submissions[] = [['fields' => $second, 'parameters' => $set['parameters'] ?? []]]; + $submissions[] = [[ + 'fields' => $second, + 'parameters' => $set['parameters'] ?? [], + 'ignore' => $ignore, + ]]; } } unset($second); @@ -202,11 +215,11 @@ trait TestHelpers * @param string $route * @param array $content */ - protected function storeAndCompare(string $route, array $content, ?array $ignore = null): void + protected function storeAndCompare(string $route, array $content): void { - $ignore = $ignore ?? []; $submission = $content['fields']; $parameters = $content['parameters']; + $ignore = $content['ignore']; // submit! $response = $this->post(route($route, $parameters), $submission, ['Accept' => 'application/json']); $responseBody = $response->content(); @@ -218,13 +231,7 @@ trait TestHelpers // compare results: foreach ($responseJson['data']['attributes'] as $returnName => $returnValue) { - if (in_array($returnName, $ignore)) { - Log::debug(sprintf('Ignore value of "%s".', $returnName)); - continue; - } - - - if (array_key_exists($returnName, $submission)) { + if (array_key_exists($returnName, $submission) && !in_array($returnName, $ignore, true)) { // TODO still based on account routine: if ($this->ignoreCombination($route, $submission['type'] ?? 'blank', $returnName)) { continue; @@ -251,9 +258,9 @@ trait TestHelpers */ protected function ignoreCombination(string $area, string $left, string $right): bool { - if ('api.v1.attachments.store' === $area) { + if ('api.v1.accounts.store' === $area) { if ('expense' === $left - && in_array($right, ['virtual_balance', 'opening_balance', 'opening_balance_date'])) { + && in_array($right, ['order', 'virtual_balance', 'opening_balance', 'opening_balance_date'])) { return true; } }