diff --git a/app/Api/V1/Controllers/AccountController.php b/app/Api/V1/Controllers/AccountController.php index e94727ccdf..5a0208c481 100644 --- a/app/Api/V1/Controllers/AccountController.php +++ b/app/Api/V1/Controllers/AccountController.php @@ -30,7 +30,6 @@ use FireflyIII\Models\Account; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; -use FireflyIII\Repositories\Journal\JournalRepositoryInterface; use FireflyIII\Support\Http\Api\AccountFilter; use FireflyIII\Support\Http\Api\TransactionFilter; use FireflyIII\Transformers\AccountTransformer; @@ -128,14 +127,18 @@ class AccountController extends Controller // present to user. $manager->setSerializer(new JsonApiSerializer($baseUrl)); - $resource = new FractalCollection($accounts, new AccountTransformer($this->parameters), 'accounts'); + + /** @var AccountTransformer $transformer */ + $transformer = app(AccountTransformer::class); + $transformer->setParameters($this->parameters); + + $resource = new FractalCollection($accounts, $transformer, 'accounts'); $resource->setPaginator(new IlluminatePaginatorAdapter($paginator)); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); } - /** * List all of them. * @@ -164,7 +167,12 @@ class AccountController extends Controller // present to user. $manager->setSerializer(new JsonApiSerializer($baseUrl)); - $resource = new FractalCollection($piggyBanks, new PiggyBankTransformer($this->parameters), 'piggy_banks'); + + /** @var PiggyBankTransformer $transformer */ + $transformer = app(PiggyBankTransformer::class); + $transformer->setParameters($this->parameters); + + $resource = new FractalCollection($piggyBanks, $transformer, 'piggy_banks'); $resource->setPaginator(new IlluminatePaginatorAdapter($paginator)); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); @@ -184,7 +192,11 @@ class AccountController extends Controller $manager = new Manager; $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; $manager->setSerializer(new JsonApiSerializer($baseUrl)); - $resource = new Item($account, new AccountTransformer($this->parameters), 'accounts'); + + /** @var AccountTransformer $transformer */ + $transformer = app(AccountTransformer::class); + $transformer->setParameters($this->parameters); + $resource = new Item($account, $transformer, 'accounts'); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); } @@ -209,7 +221,11 @@ class AccountController extends Controller $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; $manager->setSerializer(new JsonApiSerializer($baseUrl)); - $resource = new Item($account, new AccountTransformer($this->parameters), 'accounts'); + /** @var AccountTransformer $transformer */ + $transformer = app(AccountTransformer::class); + $transformer->setParameters($this->parameters); + + $resource = new Item($account, $transformer, 'accounts'); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); } @@ -228,9 +244,12 @@ class AccountController extends Controller $type = $request->get('type') ?? 'default'; $this->parameters->set('type', $type); - $types = $this->mapTransactionTypes($this->parameters->get('type')); - $manager = new Manager(); - $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $types = $this->mapTransactionTypes($this->parameters->get('type')); + $manager = new Manager(); + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + + + $manager->setSerializer(new JsonApiSerializer($baseUrl)); /** @var User $admin */ @@ -258,7 +277,12 @@ class AccountController extends Controller $paginator = $collector->getPaginatedTransactions(); $paginator->setPath(route('api.v1.accounts.transactions', [$account->id]) . $this->buildParams()); $transactions = $paginator->getCollection(); - $resource = new FractalCollection($transactions, new TransactionTransformer($this->parameters), 'transactions'); + + /** @var TransactionTransformer $transformer */ + $transformer = app(TransactionTransformer::class); + $transformer->setParameters($this->parameters); + + $resource = new FractalCollection($transactions, $transformer, 'transactions'); $resource->setPaginator(new IlluminatePaginatorAdapter($paginator)); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); @@ -287,7 +311,10 @@ class AccountController extends Controller $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; $manager->setSerializer(new JsonApiSerializer($baseUrl)); - $resource = new Item($account, new AccountTransformer($this->parameters), 'accounts'); + /** @var AccountTransformer $transformer */ + $transformer = app(AccountTransformer::class); + $transformer->setParameters($this->parameters); + $resource = new Item($account, $transformer, 'accounts'); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); } diff --git a/app/Api/V1/Requests/AccountRequest.php b/app/Api/V1/Requests/AccountRequest.php index 36c85e8f8c..3485c75eaa 100644 --- a/app/Api/V1/Requests/AccountRequest.php +++ b/app/Api/V1/Requests/AccountRequest.php @@ -50,11 +50,19 @@ class AccountRequest extends Request */ public function getAll(): array { + $active = true; + $includeNetWorth = true; + if (null !== $this->get('active')) { + $active = $this->boolean('active'); + } + if (null !== $this->get('include_net_worth')) { + $includeNetWorth = $this->boolean('include_net_worth'); + } $data = [ 'name' => $this->string('name'), - 'active' => $this->boolean('active'), - 'include_net_worth' => $this->boolean('include_net_worth'), + 'active' => $active, + 'include_net_worth' => $includeNetWorth, 'accountType' => $this->string('type'), 'account_type_id' => null, 'currency_id' => $this->integer('currency_id'), @@ -66,8 +74,8 @@ class AccountRequest extends Request 'accountRole' => $this->string('account_role'), 'openingBalance' => $this->string('opening_balance'), 'openingBalanceDate' => $this->date('opening_balance_date'), - 'ccType' => $this->string('cc_type'), - 'ccMonthlyPaymentDate' => $this->string('cc_monthly_payment_date'), + 'ccType' => $this->string('credit_card_type'), + 'ccMonthlyPaymentDate' => $this->string('monthly_payment_date'), 'notes' => $this->string('notes'), 'interest' => $this->string('interest'), 'interest_period' => $this->string('interest_period'), @@ -94,27 +102,27 @@ class AccountRequest extends Request $types = implode(',', array_keys(config('firefly.subTitlesByIdentifier'))); $ccPaymentTypes = implode(',', array_keys(config('firefly.ccTypes'))); $rules = [ - 'name' => 'required|min:1|uniqueAccountForUser', - 'type' => 'required|in:' . $types, - 'iban' => 'iban|nullable', - 'bic' => 'bic|nullable', - 'account_number' => 'between:1,255|nullable|uniqueAccountNumberForUser', - 'opening_balance' => 'numeric|required_with:opening_balance_date|nullable', - 'opening_balance_date' => 'date|required_with:opening_balance|nullable', - 'virtual_balance' => 'numeric|nullable', - 'currency_id' => 'numeric|exists:transaction_currencies,id', - 'currency_code' => 'min:3|max:3|exists:transaction_currencies,code', - 'active' => [new IsBoolean], - 'include_net_worth' => [new IsBoolean], - 'account_role' => 'in:' . $accountRoles . '|required_if:type,asset', - 'cc_type' => 'in:' . $ccPaymentTypes . '|required_if:account_role,ccAsset', - 'cc_monthly_payment_date' => 'date' . '|required_if:account_role,ccAsset|required_if:cc_type,monthlyFull', - 'liability_type' => 'required_if:type,liability|in:loan,debt,mortgage', - 'liability_amount' => 'required_if:type,liability|min:0|numeric', - 'liability_start_date' => 'required_if:type,liability|date', - 'interest' => 'required_if:type,liability|between:0,100|numeric', - 'interest_period' => 'required_if:type,liability|in:daily,monthly,yearly', - 'notes' => 'min:0|max:65536', + 'name' => 'required|min:1|uniqueAccountForUser', + 'type' => 'required|in:' . $types, + 'iban' => 'iban|nullable', + 'bic' => 'bic|nullable', + 'account_number' => 'between:1,255|nullable|uniqueAccountNumberForUser', + 'opening_balance' => 'numeric|required_with:opening_balance_date|nullable', + 'opening_balance_date' => 'date|required_with:opening_balance|nullable', + 'virtual_balance' => 'numeric|nullable', + 'currency_id' => 'numeric|exists:transaction_currencies,id', + 'currency_code' => 'min:3|max:3|exists:transaction_currencies,code', + 'active' => [new IsBoolean], + 'include_net_worth' => [new IsBoolean], + 'account_role' => 'in:' . $accountRoles . '|required_if:type,asset', + 'credit_card_type' => 'in:' . $ccPaymentTypes . '|required_if:account_role,ccAsset', + 'monthly_payment_date' => 'date' . '|required_if:account_role,ccAsset|required_if:credit_card_type,monthlyFull', + 'liability_type' => 'required_if:type,liability|in:loan,debt,mortgage', + 'liability_amount' => 'required_if:type,liability|min:0|numeric', + 'liability_start_date' => 'required_if:type,liability|date', + 'interest' => 'required_if:type,liability|between:0,100|numeric', + 'interest_period' => 'required_if:type,liability|in:daily,monthly,yearly', + 'notes' => 'min:0|max:65536', ]; switch ($this->method()) { default: diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 923a72cba7..ea233c95c8 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -27,6 +27,7 @@ use FireflyIII\Exceptions\FireflyException; use FireflyIII\Factory\AccountFactory; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; +use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\Services\Internal\Destroy\AccountDestroyService; @@ -51,7 +52,7 @@ class AccountRepository implements AccountRepositoryInterface */ public function __construct() { - if ('testing' === env('APP_ENV')) { + if ('testing' === config('app.env')) { Log::warning(sprintf('%s should not be instantiated in the TEST environment!', \get_class($this))); } } @@ -178,6 +179,31 @@ class AccountRepository implements AccountRepositoryInterface return $this->user->accounts()->find($accountId); } + /** + * @param Account $account + * + * @return TransactionCurrency|null + */ + public function getAccountCurrency(Account $account): ?TransactionCurrency + { + $currencyId = (int)$this->getMetaValue($account, 'currency_id'); + if ($currencyId > 0) { + return TransactionCurrency::find($currencyId); + } + + return null; + } + + /** + * @param Account $account + * + * @return string + */ + public function getAccountType(Account $account): string + { + return $account->accountType->type; + } + /** * Return account type or null if not found. * diff --git a/app/Repositories/Account/AccountRepositoryInterface.php b/app/Repositories/Account/AccountRepositoryInterface.php index b70582453c..1f60fa2cec 100644 --- a/app/Repositories/Account/AccountRepositoryInterface.php +++ b/app/Repositories/Account/AccountRepositoryInterface.php @@ -25,6 +25,7 @@ namespace FireflyIII\Repositories\Account; use Carbon\Carbon; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; +use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionJournal; use FireflyIII\User; use Illuminate\Support\Collection; @@ -44,6 +45,20 @@ interface AccountRepositoryInterface */ public function count(array $types): int; + /** + * @param Account $account + * + * @return string + */ + public function getAccountType(Account $account): string; + + /** + * @param Account $account + * + * @return TransactionCurrency|null + */ + public function getAccountCurrency(Account $account): ?TransactionCurrency; + /** * Moved here from account CRUD. * diff --git a/app/Transformers/AbstractTransformer.php b/app/Transformers/AbstractTransformer.php new file mode 100644 index 0000000000..d7e6ee4954 --- /dev/null +++ b/app/Transformers/AbstractTransformer.php @@ -0,0 +1,53 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Transformers; + +use League\Fractal\TransformerAbstract; +use Symfony\Component\HttpFoundation\ParameterBag; + +/** + * + * Class AbstractTransformer + */ +class AbstractTransformer extends TransformerAbstract +{ + /** @var ParameterBag */ + protected $parameters; + + /** + * @return ParameterBag + */ + public function getParameters(): ParameterBag + { + return $this->parameters; + } + + /** + * @param ParameterBag $parameters + */ + public function setParameters(ParameterBag $parameters): void + { + $this->parameters = $parameters; + } +} \ No newline at end of file diff --git a/app/Transformers/AccountTransformer.php b/app/Transformers/AccountTransformer.php index 7a22253f4d..b05f283309 100644 --- a/app/Transformers/AccountTransformer.php +++ b/app/Transformers/AccountTransformer.php @@ -25,26 +25,16 @@ namespace FireflyIII\Transformers; use Carbon\Carbon; -use FireflyIII\Helpers\Collector\TransactionCollectorInterface; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; -use FireflyIII\Models\TransactionCurrency; use FireflyIII\Repositories\Account\AccountRepositoryInterface; -use FireflyIII\Repositories\Journal\JournalRepositoryInterface; -use Illuminate\Support\Collection; -use League\Fractal\Resource\Collection as FractalCollection; -use League\Fractal\Resource\Item; -use League\Fractal\TransformerAbstract; -use Symfony\Component\HttpFoundation\ParameterBag; +use Log; /** * Class AccountTransformer */ -class AccountTransformer extends TransformerAbstract +class AccountTransformer extends AbstractTransformer { - /** @var ParameterBag */ - protected $parameters; - /** @var AccountRepositoryInterface */ protected $repository; @@ -53,13 +43,14 @@ class AccountTransformer extends TransformerAbstract * AccountTransformer constructor. * * @codeCoverageIgnore - * - * @param ParameterBag $parameters */ - public function __construct(ParameterBag $parameters) + 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(AccountRepositoryInterface::class); - $this->parameters = $parameters; } /** @@ -73,17 +64,23 @@ class AccountTransformer extends TransformerAbstract { $this->repository->setUser($account->user); - $type = $account->accountType->type; - $role = $this->repository->getMetaValue($account, 'accountRole'); - if ($type !== AccountType::ASSET || '' === (string)$role) { - $role = null; + // get account type: + $accountType = $this->repository->getAccountType($account); + + // get account role (will only work if the type is asset. TODO test me. + $accountRole = $this->repository->getMetaValue($account, 'accountRole'); + if ($accountType !== AccountType::ASSET || '' === (string)$accountRole) { + $accountRole = null; } - $currencyId = (int)$this->repository->getMetaValue($account, 'currency_id'); + + // get currency. If not 0, get from repository. TODO test me. + $currency = $this->repository->getAccountCurrency($account); + $currencyId = null; $currencyCode = null; - $currencySymbol = null; $decimalPlaces = 2; - if ($currencyId > 0) { - $currency = TransactionCurrency::find($currencyId); + $currencySymbol = null; + if (null !== $currency) { + $currencyId = $currency->id; $currencyCode = $currency->code; $decimalPlaces = $currency->decimal_places; $currencySymbol = $currency->symbol; @@ -94,20 +91,16 @@ class AccountTransformer extends TransformerAbstract $date = $this->parameters->get('date'); } - if (0 === $currencyId) { - $currencyId = null; - } - $monthlyPaymentDate = null; $creditCardType = null; - if ('ccAsset' === $role && $type === AccountType::ASSET) { + if ('ccAsset' === $accountRole && $accountType === AccountType::ASSET) { $creditCardType = $this->repository->getMetaValue($account, 'ccType'); $monthlyPaymentDate = $this->repository->getMetaValue($account, 'ccMonthlyPaymentDate'); } $openingBalance = null; $openingBalanceDate = null; - if (\in_array($type, [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], true)) { + if (\in_array($accountType, [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], true)) { $amount = $this->repository->getOpeningBalanceAmount($account); $openingBalance = null === $amount ? null : round($amount, $decimalPlaces); $openingBalanceDate = $this->repository->getOpeningBalanceDate($account); @@ -120,10 +113,10 @@ class AccountTransformer extends TransformerAbstract 'id' => (int)$account->id, 'created_at' => $account->created_at->toAtomString(), 'updated_at' => $account->updated_at->toAtomString(), - 'active' => 1 === (int)$account->active, + 'active' => $account->active, 'name' => $account->name, - 'type' => $type, - 'account_role' => $role, + 'type' => $accountType, + 'account_role' => $accountRole, 'currency_id' => $currencyId, 'currency_code' => $currencyCode, 'currency_symbol' => $currencySymbol, @@ -139,7 +132,7 @@ class AccountTransformer extends TransformerAbstract 'virtual_balance' => round($account->virtual_balance, $decimalPlaces), 'opening_balance' => $openingBalance, 'opening_balance_date' => $openingBalanceDate, - 'liability_type' => $type, + 'liability_type' => $accountType, 'liability_amount' => $openingBalance, 'liability_start_date' => $openingBalanceDate, 'interest' => $interest, diff --git a/database/factories/AccountFactory.php b/database/factories/AccountFactory.php new file mode 100644 index 0000000000..da08fd82c2 --- /dev/null +++ b/database/factories/AccountFactory.php @@ -0,0 +1,40 @@ +. + */ +declare(strict_types=1); + +use Carbon\Carbon; + + + +$factory->define( + FireflyIII\Models\Account::class, + function (Faker\Generator $faker) { + return [ + //'id' => $faker->unique()->numberBetween(1000, 10000), + 'user_id' => 1, + 'created_at' => new Carbon, + 'updated_at' => new Carbon, + 'name' => $faker->words(3, true), + 'account_type_id' => random_int(2, 5), + 'active' => true, + ]; + } +); \ No newline at end of file diff --git a/database/factories/ModelFactory.php b/database/factories/ModelFactory.php index dd086000bc..936dc2d38e 100644 --- a/database/factories/ModelFactory.php +++ b/database/factories/ModelFactory.php @@ -217,21 +217,6 @@ $factory->define( } ); -$factory->define( - FireflyIII\Models\Account::class, - function (Faker\Generator $faker) { - return [ - 'id' => $faker->unique()->numberBetween(1000, 10000), - 'user_id' => 1, - 'created_at' => new Carbon, - 'updated_at' => new Carbon, - 'name' => $faker->words(3, true), - 'account_type_id' => 1, - 'active' => true, - ]; - } -); - $factory->define( FireflyIII\Models\Transaction::class, function (Faker\Generator $faker) { diff --git a/tests/Unit/Transformers/AccountTransformerTest.php b/tests/Unit/Transformers/AccountTransformerTest.php index 1fc689ab59..2eb863dd65 100644 --- a/tests/Unit/Transformers/AccountTransformerTest.php +++ b/tests/Unit/Transformers/AccountTransformerTest.php @@ -53,23 +53,15 @@ class AccountTransformerTest extends TestCase $accountRepos->shouldReceive('getOpeningBalanceDate')->andReturn(null); $accountRepos->shouldReceive('getMetaValue')->andReturn('1'); $accountRepos->shouldReceive('getNoteText')->andReturn(''); - - // make new account: - $account = Account::create( - [ - 'user_id' => $this->user()->id, - 'account_type_id' => 3, // asset account - 'name' => 'Random name #' . random_int(1, 10000), - 'virtual_balance' => 12.34, - 'iban' => 'NL85ABNA0466812694', - 'active' => 1, - 'encrypted' => 0, - ] - ); - + $account = factory(Account::class)->make(); $transformer = new AccountTransformer(new ParameterBag); $result = $transformer->transform($account); + + // verify all fields. + $this->assertEquals($account->id, $result['id']); + + $this->assertEquals($account->name, $result['name']); $this->assertEquals('Asset account', $result['type']); $this->assertEquals(12.34, $result['virtual_balance']);