Also test attachments.

This commit is contained in:
James Cole
2021-03-13 14:33:48 +01:00
parent bd040c80b2
commit bdb298740a
19 changed files with 446 additions and 99 deletions

View File

@@ -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 */

View File

@@ -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);

View File

@@ -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();

View File

@@ -48,8 +48,8 @@ class StoreRequest extends FormRequest
'filename' => $this->string('filename'),
'title' => $this->string('title'),
'notes' => $this->nlString('notes'),
'model' => $this->string('attachable_type'),
'model_id' => $this->integer('attachable_id'),
'attachable_type' => $this->string('attachable_type'),
'attachable_id' => $this->integer('attachable_id'),
];
}

View File

@@ -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',
'attachable_type' => sprintf('in:%s', $models),
'attachable_id' => ['numeric', new IsValidAttachmentModel($model)],
];
}
}

View File

@@ -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;

View File

@@ -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'],

View File

@@ -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;
}
}

View File

@@ -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.

View File

@@ -170,14 +170,19 @@ class AttachmentRepository implements AttachmentRepositoryInterface
*/
public function update(Attachment $attachment, array $data): Attachment
{
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) {
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;
}

View File

@@ -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

View File

@@ -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]);
}
@@ -222,6 +223,7 @@ trait JournalServiceTrait
$set = [];
if (!is_array($tags)) {
Log::debug('Tags is not an array, break.');
return;
}
Log::debug('Start of loop.');
@@ -379,6 +381,7 @@ trait JournalServiceTrait
'active' => true,
'iban' => $data['iban'],
'currency_id' => $data['currency_id'] ?? null,
'order' => $this->accountRepository->maxOrder($preferredType),
]
);
// store BIC

View File

@@ -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;
@@ -185,16 +184,21 @@ 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'];
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]);
@@ -206,6 +210,7 @@ class AccountUpdateService
->whereIn('accounts.account_type_id', $list)
->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;
@@ -216,6 +221,7 @@ class AccountUpdateService
->whereIn('accounts.account_type_id', $list)
->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;

View File

@@ -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)));
}
}
/**

View File

@@ -50,7 +50,7 @@ class StoreControllerTest extends TestCase
/**
* @param array $submission
*
* X data Provider storeAccountDataProvider
* @dataProvider storeDataProvider
*
* @ data Provider emptyDataProvider
*/
@@ -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;
}
}

View File

@@ -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,

View File

@@ -0,0 +1,152 @@
<?php
/*
* StoreControllerTest.php
* Copyright (c) 2021 james@firefly-iii.org
*
* This file is part of Firefly III (https://github.com/firefly-iii).
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
namespace Tests\Api\Models\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)),
],
],
];
}
}

View File

@@ -0,0 +1,123 @@
<?php
/*
* UpdateControllerTest.php
* Copyright (c) 2021 james@firefly-iii.org
*
* This file is part of Firefly III (https://github.com/firefly-iii).
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
namespace Tests\Api\Models\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;
}
}

View File

@@ -122,8 +122,7 @@ trait TestHelpers
$response->assertStatus(200);
$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'];
}
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);
$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;
}
}