diff --git a/app/Api/V1/Controllers/Models/Account/ShowController.php b/app/Api/V1/Controllers/Models/Account/ShowController.php index 68a7353789..7fc3cda55e 100644 --- a/app/Api/V1/Controllers/Models/Account/ShowController.php +++ b/app/Api/V1/Controllers/Models/Account/ShowController.php @@ -110,6 +110,9 @@ class ShowController extends Controller */ public function show(Account $account): JsonResponse { + // get list of accounts. Count it and split it. + $this->repository->sortAccounts(); + $account->refresh(); $manager = $this->getManager(); /** @var AccountTransformer $transformer */ diff --git a/app/Api/V1/Controllers/Models/Account/UpdateController.php b/app/Api/V1/Controllers/Models/Account/UpdateController.php index 55abbf968c..665c10e7ca 100644 --- a/app/Api/V1/Controllers/Models/Account/UpdateController.php +++ b/app/Api/V1/Controllers/Models/Account/UpdateController.php @@ -30,6 +30,7 @@ use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Transformers\AccountTransformer; use Illuminate\Http\JsonResponse; use League\Fractal\Resource\Item; +use Log; /** * Class UpdateController @@ -69,6 +70,7 @@ class UpdateController extends Controller */ public function update(UpdateRequest $request, Account $account): JsonResponse { + Log::debug(sprintf('Now in %s', __METHOD__)); $data = $request->getUpdateData(); $data['type'] = config('firefly.shortNamesByFullName.' . $account->accountType->type); $account = $this->repository->update($account, $data); diff --git a/app/Api/V1/Controllers/Models/Attachment/StoreController.php b/app/Api/V1/Controllers/Models/Attachment/StoreController.php index 20cc6b3c5f..63218934cb 100644 --- a/app/Api/V1/Controllers/Models/Attachment/StoreController.php +++ b/app/Api/V1/Controllers/Models/Attachment/StoreController.php @@ -34,6 +34,7 @@ use FireflyIII\User; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use League\Fractal\Resource\Item; +use Log; /** * Class StoreController @@ -77,6 +78,7 @@ class StoreController extends Controller */ public function store(StoreRequest $request): JsonResponse { + Log::debug(sprintf('Now in %s', __METHOD__)); $data = $request->getAll(); $attachment = $this->repository->store($data); $manager = $this->getManager(); diff --git a/app/Api/V1/Requests/Models/Attachment/StoreRequest.php b/app/Api/V1/Requests/Models/Attachment/StoreRequest.php index 67ae3c7724..97c43cd933 100644 --- a/app/Api/V1/Requests/Models/Attachment/StoreRequest.php +++ b/app/Api/V1/Requests/Models/Attachment/StoreRequest.php @@ -45,11 +45,11 @@ class StoreRequest extends FormRequest public function getAll(): array { return [ - 'filename' => $this->string('filename'), - 'title' => $this->string('title'), - 'notes' => $this->nlString('notes'), - 'model' => $this->string('attachable_type'), - 'model_id' => $this->integer('attachable_id'), + 'filename' => $this->string('filename'), + 'title' => $this->string('title'), + 'notes' => $this->nlString('notes'), + 'attachable_type' => $this->string('attachable_type'), + 'attachable_id' => $this->integer('attachable_id'), ]; } diff --git a/app/Api/V1/Requests/Models/Attachment/UpdateRequest.php b/app/Api/V1/Requests/Models/Attachment/UpdateRequest.php index 1c498b3b68..288fd27354 100644 --- a/app/Api/V1/Requests/Models/Attachment/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Attachment/UpdateRequest.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Requests\Models\Attachment; +use FireflyIII\Rules\IsValidAttachmentModel; use FireflyIII\Support\Request\ChecksLogin; use FireflyIII\Support\Request\ConvertsDataTypes; use Illuminate\Foundation\Http\FormRequest; @@ -43,13 +44,15 @@ class UpdateRequest extends FormRequest */ public function getAll(): array { - return [ - 'filename' => $this->string('filename'), - 'title' => $this->string('title'), - 'notes' => $this->nlString('notes'), - 'model' => $this->string('attachable_type'), - 'model_id' => $this->integer('attachable_id'), + $fields = [ + 'filename' => ['filename', 'string'], + 'title' => ['title', 'string'], + 'notes' => ['notes', 'nlString'], + 'attachable_type' => ['attachable_type', 'string'], + 'attachable_id' => ['attachable_id', 'integer'], ]; + + return $this->getAllData($fields); } /** @@ -59,10 +62,23 @@ class UpdateRequest extends FormRequest */ public function rules(): array { + $models = config('firefly.valid_attachment_models'); + $models = array_map( + + static function (string $className) { + return str_replace('FireflyIII\\Models\\', '', $className); + }, $models + ); + $models = implode(',', $models); + $model = $this->string('attachable_type'); + + return [ - 'filename' => 'between:1,255', - 'title' => 'between:1,255', - 'notes' => 'between:1,65000', + 'filename' => 'between:1,255', + 'title' => 'between:1,255', + 'notes' => 'between:1,65000', + 'attachable_type' => sprintf('in:%s', $models), + 'attachable_id' => ['numeric', new IsValidAttachmentModel($model)], ]; } } diff --git a/app/Factory/AccountFactory.php b/app/Factory/AccountFactory.php index be1164bba8..833cfe9658 100644 --- a/app/Factory/AccountFactory.php +++ b/app/Factory/AccountFactory.php @@ -92,7 +92,8 @@ class AccountFactory // create it: $databaseData = ['user_id' => $this->user->id, 'account_type_id' => $type->id, - 'name' => $data['name'], 'order' => 0, + 'name' => $data['name'], + 'order' => 25000, 'virtual_balance' => $data['virtual_balance'] ?? null, 'active' => true === $data['active'], 'iban' => $data['iban'],]; $currency = $this->getCurrency((int)($data['currency_id'] ?? null), (string)($data['currency_code'] ?? null)); @@ -126,14 +127,24 @@ class AccountFactory // store location $this->storeNewLocation($return, $data); + // update order to be correct: + // set new order: - if (array_key_exists('order', $data)) { - $maxOrder = $this->accountRepository->maxOrder([$type->type]); - $order = $data['order'] > $maxOrder ? $maxOrder+1 : $data['order']; - $update = new AccountUpdateService; - $update->setUser($return->user); - $return = $update->updateAccountOrder($return,['order' => $order]); + $maxOrder = $this->accountRepository->maxOrder($type->type); + $order = null; + if (!array_key_exists('order', $data)) { + // take maxOrder + 1 + $order = $maxOrder + 1; } + if (array_key_exists('order', $data)) { + // limit order + $order = (int)($data['order'] > $maxOrder ? $maxOrder + 1 : $data['order']); + $order = 0 === $order ? $maxOrder + 1 : $order; + } + $updateService = app(AccountUpdateService::class); + $updateService->setUser($return->user); + Log::debug(sprintf('Will set order to %d', $order)); + $return = $updateService->update($return, ['order' => $order]); } return $return; diff --git a/app/Factory/AttachmentFactory.php b/app/Factory/AttachmentFactory.php index 14b7bd7ea0..98d87967de 100644 --- a/app/Factory/AttachmentFactory.php +++ b/app/Factory/AttachmentFactory.php @@ -46,16 +46,16 @@ class AttachmentFactory public function create(array $data): ?Attachment { // append if necessary. - $model = false === strpos($data['model'], 'FireflyIII') ? sprintf('FireflyIII\\Models\\%s', $data['model']) : $data['model']; + $model = false === strpos($data['attachable_type'], 'FireflyIII') ? sprintf('FireflyIII\\Models\\%s', $data['attachable_type']) : $data['attachable_type']; // get journal instead of transaction. if (Transaction::class === $model) { /** @var Transaction $transaction */ - $transaction = $this->user->transactions()->find((int) $data['model_id']); + $transaction = $this->user->transactions()->find((int) $data['attachable_id']); if (null === $transaction) { throw new FireflyException('Unexpectedly could not find transaction'); // @codeCoverageIgnore } - $data['model_id'] = $transaction->transaction_journal_id; + $data['attachable_id'] = $transaction->transaction_journal_id; $model = TransactionJournal::class; } @@ -63,7 +63,7 @@ class AttachmentFactory $attachment = Attachment::create( [ 'user_id' => $this->user->id, - 'attachable_id' => $data['model_id'], + 'attachable_id' => $data['attachable_id'], 'attachable_type' => $model, 'md5' => '', 'filename' => $data['filename'], diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 82abfd1a69..e691133828 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -596,10 +596,12 @@ class AccountRepository implements AccountRepositoryInterface [AccountType::CASH, AccountType::INITIAL_BALANCE, AccountType::IMPORT, AccountType::RECONCILIATION], ]; foreach ($sets as $set) { + Log::debug('Now in resetAccountOrder', $set); $list = $this->getAccountsByType($set); $index = 1; foreach ($list as $account) { - if ($index !== $account->order) { + if ($index !== (int)$account->order) { + Log::debug(sprintf('Account #%d ("%s"): order should %d be but is %d.', $account->id, $account->name, $index, $account->order)); $account->order = $index; $account->save(); } @@ -766,8 +768,27 @@ class AccountRepository implements AccountRepositoryInterface /** * @inheritDoc */ - public function maxOrder(array $types): int + public function maxOrder(string $type): int { - return (int)$this->getAccountsByType($types)->max('order'); + $sets = [ + AccountType::ASSET => [AccountType::DEFAULT, AccountType::ASSET], + AccountType::EXPENSE => [AccountType::EXPENSE, AccountType::BENEFICIARY], + AccountType::REVENUE => [AccountType::REVENUE], + AccountType::LOAN => [AccountType::LOAN, AccountType::DEBT, AccountType::CREDITCARD, AccountType::MORTGAGE], + AccountType::DEBT => [AccountType::LOAN, AccountType::DEBT, AccountType::CREDITCARD, AccountType::MORTGAGE], + AccountType::MORTGAGE => [AccountType::LOAN, AccountType::DEBT, AccountType::CREDITCARD, AccountType::MORTGAGE], + ]; + if (array_key_exists(ucfirst($type), $sets)) { + $order = (int)$this->getAccountsByType($sets[ucfirst($type)])->max('order'); + Log::debug(sprintf('Return max order of "%s" set: %d', $type, $order)); + + return $order; + } + $specials = [AccountType::CASH, AccountType::INITIAL_BALANCE, AccountType::IMPORT, AccountType::RECONCILIATION]; + + $order = (int)$this->getAccountsByType($specials)->max('order'); + Log::debug(sprintf('Return max order of "%s" set (specials!): %d', $type, $order)); + + return $order; } } diff --git a/app/Repositories/Account/AccountRepositoryInterface.php b/app/Repositories/Account/AccountRepositoryInterface.php index fe8af00155..7f2ba5b4e5 100644 --- a/app/Repositories/Account/AccountRepositoryInterface.php +++ b/app/Repositories/Account/AccountRepositoryInterface.php @@ -48,11 +48,11 @@ interface AccountRepositoryInterface public function count(array $types): int; /** - * @param array $types + * @param string $type * * @return int */ - public function maxOrder(array $types): int; + public function maxOrder(string $type): int; /** * Moved here from account CRUD. diff --git a/app/Repositories/Attachment/AttachmentRepository.php b/app/Repositories/Attachment/AttachmentRepository.php index 696f4b261a..5b791637ed 100644 --- a/app/Repositories/Attachment/AttachmentRepository.php +++ b/app/Repositories/Attachment/AttachmentRepository.php @@ -170,14 +170,19 @@ class AttachmentRepository implements AttachmentRepositoryInterface */ public function update(Attachment $attachment, array $data): Attachment { - $attachment->title = $data['title']; + if (array_key_exists('title', $data)) { + $attachment->title = $data['title']; + } - // update filename, if present and different: - if (isset($data['filename']) && '' !== $data['filename'] && $data['filename'] !== $attachment->filename) { - $attachment->filename = $data['filename']; + if (array_key_exists('filename', $data)) { + if ('' !== (string)$data['filename'] && $data['filename'] !== $attachment->filename) { + $attachment->filename = $data['filename']; + } } $attachment->save(); - $this->updateNote($attachment, $data['notes'] ?? ''); + if (array_key_exists('notes', $data)) { + $this->updateNote($attachment, (string)$data['notes']); + } return $attachment; } diff --git a/app/Services/Internal/Destroy/AccountDestroyService.php b/app/Services/Internal/Destroy/AccountDestroyService.php index f4b1f6b16c..80c8b1d301 100644 --- a/app/Services/Internal/Destroy/AccountDestroyService.php +++ b/app/Services/Internal/Destroy/AccountDestroyService.php @@ -39,18 +39,6 @@ use Log; */ class AccountDestroyService { - /** - * Constructor. - * - * @codeCoverageIgnore - */ - public function __construct() - { - if ('testing' === config('app.env')) { - Log::warning(sprintf('%s should not be instantiated in the TEST environment!', get_class($this))); - } - } - /** * @param Account $account * @param Account|null $moveTo diff --git a/app/Services/Internal/Support/JournalServiceTrait.php b/app/Services/Internal/Support/JournalServiceTrait.php index 8fd9c1ca61..7269164502 100644 --- a/app/Services/Internal/Support/JournalServiceTrait.php +++ b/app/Services/Internal/Support/JournalServiceTrait.php @@ -109,6 +109,7 @@ trait JournalServiceTrait $result = $this->findAccountByName($result, $data, $expectedTypes[$transactionType]); $result = $this->findAccountByIban($result, $data, $expectedTypes[$transactionType]); $result = $this->createAccount($result, $data, $expectedTypes[$transactionType][0]); + return $this->getCashAccount($result, $data, $expectedTypes[$transactionType]); } @@ -182,7 +183,7 @@ trait JournalServiceTrait */ protected function storeNotes(TransactionJournal $journal, ?string $notes): void { - $notes = (string) $notes; + $notes = (string)$notes; $note = $journal->notes()->first(); if ('' !== $notes) { if (null === $note) { @@ -222,11 +223,12 @@ trait JournalServiceTrait $set = []; if (!is_array($tags)) { Log::debug('Tags is not an array, break.'); + return; } Log::debug('Start of loop.'); foreach ($tags as $string) { - $string = (string) $string; + $string = (string)$string; Log::debug(sprintf('Now at tag "%s"', $string)); if ('' !== $string) { $tag = $this->tagFactory->findOrCreate($string); @@ -363,7 +365,7 @@ trait JournalServiceTrait throw new FireflyException('TransactionFactory: Cannot create asset account with these values', $data); } // fix name of account if only IBAN is given: - if ('' === (string) $data['name'] && '' !== (string) $data['iban']) { + if ('' === (string)$data['name'] && '' !== (string)$data['iban']) { Log::debug(sprintf('Account name is now IBAN ("%s")', $data['iban'])); $data['name'] = $data['iban']; } @@ -379,6 +381,7 @@ trait JournalServiceTrait 'active' => true, 'iban' => $data['iban'], 'currency_id' => $data['currency_id'] ?? null, + 'order' => $this->accountRepository->maxOrder($preferredType), ] ); // store BIC diff --git a/app/Services/Internal/Update/AccountUpdateService.php b/app/Services/Internal/Update/AccountUpdateService.php index df8980b979..acbd59960b 100644 --- a/app/Services/Internal/Update/AccountUpdateService.php +++ b/app/Services/Internal/Update/AccountUpdateService.php @@ -23,7 +23,6 @@ declare(strict_types=1); namespace FireflyIII\Services\Internal\Update; -use DB; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Models\Location; @@ -78,7 +77,7 @@ class AccountUpdateService */ public function update(Account $account, array $data): Account { - Log::debug(sprintf('Now in %s',__METHOD__)); + Log::debug(sprintf('Now in %s', __METHOD__)); $this->accountRepository->setUser($account->user); $this->user = $account->user; $account = $this->updateAccount($account, $data); @@ -185,17 +184,22 @@ class AccountUpdateService { // skip if no order info if (!array_key_exists('order', $data) || $data['order'] === $account->order) { + Log::debug(sprintf('Account order will not be touched because its not set or already at %d.', $account->order)); + return $account; } // skip if not of orderable type. $type = $account->accountType->type; if (!in_array($type, [AccountType::ASSET, AccountType::MORTGAGE, AccountType::LOAN, AccountType::DEBT], true)) { + Log::debug('Will not change order of this account.'); + return $account; } // get account type ID's because a join and an update is hard: $oldOrder = (int)$account->order; $newOrder = $data['order']; - $list = $this->getTypeIds([AccountType::MORTGAGE, AccountType::LOAN, AccountType::DEBT]); + Log::debug(sprintf('Order is set to be updated from %s to %s', $oldOrder, $newOrder)); + $list = $this->getTypeIds([AccountType::MORTGAGE, AccountType::LOAN, AccountType::DEBT]); if (in_array($type, [AccountType::ASSET], true)) { $list = $this->getTypeIds([AccountType::ASSET]); } @@ -204,8 +208,9 @@ class AccountUpdateService $this->user->accounts()->where('accounts.order', '<=', $newOrder)->where('accounts.order', '>', $oldOrder) ->where('accounts.id', '!=', $account->id) ->whereIn('accounts.account_type_id', $list) - ->decrement('order', 1); + ->decrement('order', 1); $account->order = $newOrder; + Log::debug(sprintf('Order of account #%d ("%s") is now %d', $account->id, $account->name, $newOrder)); $account->save(); return $account; @@ -214,8 +219,9 @@ class AccountUpdateService $this->user->accounts()->where('accounts.order', '>=', $newOrder)->where('accounts.order', '<', $oldOrder) ->where('accounts.id', '!=', $account->id) ->whereIn('accounts.account_type_id', $list) - ->increment('order',1); + ->increment('order', 1); $account->order = $newOrder; + Log::debug(sprintf('Order of account #%d ("%s") is now %d', $account->id, $account->name, $newOrder)); $account->save(); return $account; diff --git a/app/Transformers/AttachmentTransformer.php b/app/Transformers/AttachmentTransformer.php index a546445eae..34d2f4ade1 100644 --- a/app/Transformers/AttachmentTransformer.php +++ b/app/Transformers/AttachmentTransformer.php @@ -43,9 +43,6 @@ class AttachmentTransformer extends AbstractTransformer public function __construct() { $this->repository = app(AttachmentRepositoryInterface::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 07f7299065..ce3792034b 100644 --- a/tests/Api/Models/Account/StoreControllerTest.php +++ b/tests/Api/Models/Account/StoreControllerTest.php @@ -50,9 +50,9 @@ class StoreControllerTest extends TestCase /** * @param array $submission * - * X data Provider storeAccountDataProvider + * @dataProvider storeDataProvider * - * @dataProvider emptyDataProvider + * @ data Provider emptyDataProvider */ public function testStore(array $submission): void { @@ -76,7 +76,7 @@ class StoreControllerTest extends TestCase /** * @return array */ - public function storeAccountDataProvider(): array + public function storeDataProvider(): array { $minimalSets = $this->minimalSets(); $optionalSets = $this->optionalSets(); @@ -226,27 +226,4 @@ class StoreControllerTest extends TestCase ], ]; } - - /** - * @param string $area - * @param string $left - * @param string $right - * - * @return bool - */ - private function ignoreCombination(string $area, string $left, string $right): bool - { - Log::debug(sprintf('Must ignore %s: %s vs %s?', $area, $left, $right)); - if ('store-account' === $area) { - if ('expense' === $left && in_array($right, ['virtual_balance', 'opening_balance', 'opening_balance_date'])) { - Log::debug('Yes'); - - return true; - } - } - Log::debug('NO'); - - return false; - } - } \ No newline at end of file diff --git a/tests/Api/Models/Account/UpdateControllerTest.php b/tests/Api/Models/Account/UpdateControllerTest.php index 288e913b5e..a07ea37468 100644 --- a/tests/Api/Models/Account/UpdateControllerTest.php +++ b/tests/Api/Models/Account/UpdateControllerTest.php @@ -49,7 +49,7 @@ class UpdateControllerTest extends TestCase } /** - * @dataProvider updateSetDataProvider + * @dataProvider updateDataProvider */ public function testUpdate(array $submission): void { @@ -69,7 +69,7 @@ class UpdateControllerTest extends TestCase /** * @return array */ - public function updateSetDataProvider(): array + public function updateDataProvider(): array { $submissions = []; $all = $this->updateDataSet(); @@ -160,14 +160,14 @@ class UpdateControllerTest extends TestCase 'fields' => [ 'currency_id' => ['test_value' => (string)$faker->numberBetween(1, 10)], ], - 'extra_ignore' => [], + 'extra_ignore' => ['currency_code'], ], 'currency_code' => [ 'id' => 1, 'fields' => [ 'currency_code' => ['test_value' => $currencyCode], ], - 'extra_ignore' => [], + 'extra_ignore' => ['currency_id'], ], 'account_role' => [ 'id' => 1, diff --git a/tests/Api/Models/Attachment/StoreControllerTest.php b/tests/Api/Models/Attachment/StoreControllerTest.php new file mode 100644 index 0000000000..7a5ffdba1f --- /dev/null +++ b/tests/Api/Models/Attachment/StoreControllerTest.php @@ -0,0 +1,152 @@ +. + */ + +namespace Tests\Api\Models\Attachment; + + +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 + * + * @dataProvider storeDataProvider + * @ data Provider emptyDataProvider + */ + public function testStore(array $submission): void + { + if ([] === $submission) { + $this->markTestSkipped('Empty data provider'); + } + // run account store with a minimal data set: + $route = 'api.v1.attachments.store'; + $this->submitAndCompare($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->name; + // }, + // 'iban' => function () { + // $faker = Factory::create(); + // + // return $faker->iban(); + // }, + // 'account_number' => function () { + // $faker = Factory::create(); + // + // return $faker->iban(); + // }, + ]; + + return $this->genericDataProvider($minimalSets, $optionalSets, $regenConfig); + } + + + /** + * @return array + */ + private function minimalSets(): array + { + $faker = Factory::create(); + $types = [ + 'Account', + 'Budget', + 'Bill', + 'TransactionJournal', + 'PiggyBank', + 'Tag', + ]; + $type = $types[rand(0, count($types) - 1)]; + + return [ + 'default_file' => [ + 'fields' => [ + 'filename' => $faker->randomAscii, + 'attachable_type' => $type, + 'attachable_id' => '1', + ], + ], + ]; + } + + /** + * @return \array[][] + */ + private function optionalSets(): array + { + $faker = Factory::create(); + return [ + 'title' => [ + 'fields' => [ + 'title' => $faker->randomAscii, + ], + ], + 'notes' => [ + 'fields' => [ + 'notes' => join(' ', $faker->words(5)), + ], + ], + ]; + } +} \ No newline at end of file diff --git a/tests/Api/Models/Attachment/UpdateControllerTest.php b/tests/Api/Models/Attachment/UpdateControllerTest.php new file mode 100644 index 0000000000..a7551ae247 --- /dev/null +++ b/tests/Api/Models/Attachment/UpdateControllerTest.php @@ -0,0 +1,123 @@ +. + */ + +namespace Tests\Api\Models\Attachment; + + +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.attachments.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 = [ + 'filename' => [ + 'id' => 1, + 'fields' => [ + 'filename' => ['test_value' => $faker->text(64)], + ], + 'extra_ignore' => [], + ], + 'title' => [ + 'id' => 1, + 'fields' => [ + 'title' => ['test_value' => $faker->text(64)], + ], + 'extra_ignore' => [], + ], + 'notes' => [ + 'id' => 1, + 'fields' => [ + 'notes' => ['test_value' => join(' ', $faker->words(5))], + ], + 'extra_ignore' => [], + ], + 'model' => [ + 'id' => 1, + 'fields' => [ + 'attachable_type' => ['test_value' => 'TransactionJournal'], + 'attachable_id' => ['test_value' => (string)2], + ], + 'extra_ignore' => [], + ], + ]; + + return $set; + } + + +} \ No newline at end of file diff --git a/tests/Traits/TestHelpers.php b/tests/Traits/TestHelpers.php index bc19143839..08b630a453 100644 --- a/tests/Traits/TestHelpers.php +++ b/tests/Traits/TestHelpers.php @@ -120,10 +120,9 @@ trait TestHelpers // get original values: $response = $this->get($route, ['Accept' => 'application/json']); $response->assertStatus(200); - $originalString = $response->content(); - $originalArray = json_decode($originalString, true, 512, JSON_THROW_ON_ERROR); - - + $originalString = $response->content(); + $originalArray = json_decode($originalString, true, 512, JSON_THROW_ON_ERROR); + $originalAttributes = $originalArray['data']['attributes']; // submit whatever is in submission: // loop the fields we will update in Firefly III: $submissionArray = []; @@ -131,13 +130,18 @@ trait TestHelpers foreach ($fieldsToUpdate as $currentFieldName) { $submissionArray[$currentFieldName] = $submission['fields'][$currentFieldName]['test_value']; } - $response = $this->put($route, $submissionArray, ['Accept' => 'application/json']); + + Log::debug(sprintf('Will PUT %s to %s', json_encode($submissionArray), $route)); + + $response = $this->put($route, $submissionArray, ['Accept' => 'application/json']); $responseString = $response->content(); $response->assertStatus(200); - $responseArray = json_decode($responseString, true, 512, JSON_THROW_ON_ERROR); - + $responseArray = json_decode($responseString, true, 512, JSON_THROW_ON_ERROR); $responseAttributes = $responseArray['data']['attributes'] ?? []; + Log::debug(sprintf('Before: %s', json_encode($originalAttributes))); + Log::debug(sprintf('AFTER : %s', json_encode($responseAttributes))); + // loop it and compare: foreach ($responseAttributes as $rKey => $rValue) { // field should be ignored? @@ -165,10 +169,25 @@ trait TestHelpers // field in response was not in body, but should be the same: if (!array_key_exists($rKey, $submissionArray)) { // original has this key too: - if (array_key_exists($rKey, $originalArray)) { + if (array_key_exists($rKey, $originalAttributes)) { + // but we can ignore it! + if(in_array($rKey, $submission['extra_ignore'])) { + continue; + } // but it is different? - if ($originalArray[$rKey] !== $rValue) { - $message = 'Some other value not correct!'; + if ($originalAttributes[$rKey] !== $rValue) { + $message = sprintf( + "Untouched field '%s' should still be %s but changed to %s\nOriginal: %s\nSubmission: %s\nResult: %s", + $rKey, + var_export($originalAttributes[$rKey], true), + var_export($rValue, true), + $originalString, + json_encode($submissionArray), + $responseString + ); + + + $this->assertTrue(false, $message); } } @@ -242,7 +261,8 @@ trait TestHelpers // compare results: foreach ($responseJson['data']['attributes'] as $returnName => $returnValue) { if (array_key_exists($returnName, $submission)) { - if ($this->ignoreCombination('store-account', $submission['type'], $returnName)) { + // TODO still based on account routine: + if ($this->ignoreCombination($route, $submission['type'] ?? 'blank', $returnName)) { continue; } @@ -255,4 +275,25 @@ trait TestHelpers } } } + + /** + * Some specials: + * + * @param string $area + * @param string $left + * @param string $right + * + * @return bool + */ + protected function ignoreCombination(string $area, string $left, string $right): bool + { + if ('api.v1.attachments.store' === $area) { + if ('expense' === $left + && in_array($right, ['virtual_balance', 'opening_balance', 'opening_balance_date'])) { + return true; + } + } + + return false; + } }