Refactored API code surrounding accounts to make transformers mockable.

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'));
+        $types    = $this->mapTransactionTypes($this->parameters->get('type'));
-        $manager = new Manager();
+        $manager  = new Manager();
-        $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1';
+        $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');
     }

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'),
+            'active'               => $active,
-            'include_net_worth'    => $this->boolean('include_net_worth'),
+            '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'),
+            'ccType'               => $this->string('credit_card_type'),
-            'ccMonthlyPaymentDate' => $this->string('cc_monthly_payment_date'),
+            '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',
+            'name'                 => 'required|min:1|uniqueAccountForUser',
-            'type'                    => 'required|in:' . $types,
+            'type'                 => 'required|in:' . $types,
-            'iban'                    => 'iban|nullable',
+            'iban'                 => 'iban|nullable',
-            'bic'                     => 'bic|nullable',
+            'bic'                  => 'bic|nullable',
-            'account_number'          => 'between:1,255|nullable|uniqueAccountNumberForUser',
+            'account_number'       => 'between:1,255|nullable|uniqueAccountNumberForUser',
-            'opening_balance'         => 'numeric|required_with:opening_balance_date|nullable',
+            'opening_balance'      => 'numeric|required_with:opening_balance_date|nullable',
-            'opening_balance_date'    => 'date|required_with:opening_balance|nullable',
+            'opening_balance_date' => 'date|required_with:opening_balance|nullable',
-            'virtual_balance'         => 'numeric|nullable',
+            'virtual_balance'      => 'numeric|nullable',
-            'currency_id'             => 'numeric|exists:transaction_currencies,id',
+            'currency_id'          => 'numeric|exists:transaction_currencies,id',
-            'currency_code'           => 'min:3|max:3|exists:transaction_currencies,code',
+            'currency_code'        => 'min:3|max:3|exists:transaction_currencies,code',
-            'active'                  => [new IsBoolean],
+            'active'               => [new IsBoolean],
-            'include_net_worth'       => [new IsBoolean],
+            'include_net_worth'    => [new IsBoolean],
-            'account_role'            => 'in:' . $accountRoles . '|required_if:type,asset',
+            'account_role'         => 'in:' . $accountRoles . '|required_if:type,asset',
-            'cc_type'                 => 'in:' . $ccPaymentTypes . '|required_if:account_role,ccAsset',
+            'credit_card_type'     => 'in:' . $ccPaymentTypes . '|required_if:account_role,ccAsset',
-            'cc_monthly_payment_date' => 'date' . '|required_if:account_role,ccAsset|required_if:cc_type,monthlyFull',
+            '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_type'       => 'required_if:type,liability|in:loan,debt,mortgage',
-            'liability_amount'        => 'required_if:type,liability|min:0|numeric',
+            'liability_amount'     => 'required_if:type,liability|min:0|numeric',
-            'liability_start_date'    => 'required_if:type,liability|date',
+            'liability_start_date' => 'required_if:type,liability|date',
-            'interest'                => 'required_if:type,liability|between:0,100|numeric',
+            'interest'             => 'required_if:type,liability|between:0,100|numeric',
-            'interest_period'         => 'required_if:type,liability|in:daily,monthly,yearly',
+            'interest_period'      => 'required_if:type,liability|in:daily,monthly,yearly',
-            'notes'                   => 'min:0|max:65536',
+            'notes'                => 'min:0|max:65536',
         ];
         switch ($this->method()) {
             default:

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

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

app/Transformers/AbstractTransformer.php

@@ -0,0 +1,53 @@
+<?php
+/**
+ * AbstractTransformer.php
+ * Copyright (c) 2018 thegrumpydictator@gmail.com
+ *
+ * This file is part of Firefly III.
+ *
+ * Firefly III is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Firefly III 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+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;
+    }
+}

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 Log;
-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;
 
 /**
  * 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;
+        // get account type:
-        $role = $this->repository->getMetaValue($account, 'accountRole');
+        $accountType = $this->repository->getAccountType($account);
-        if ($type !== AccountType::ASSET || '' === (string)$role) {
+
-            $role = null;
+        // 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) {
+        $currencySymbol = null;
-            $currency       = TransactionCurrency::find($currencyId);
+        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,
+            'type'                 => $accountType,
-            'account_role'         => $role,
+            '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,

database/factories/AccountFactory.php

@@ -0,0 +1,40 @@
+<?php
+/**
+ * AccountFactory.php
+ * Copyright (c) 2018 thegrumpydictator@gmail.com
+ *
+ * This file is part of Firefly III.
+ *
+ * Firefly III is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Firefly III 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Firefly III. If not, see <http://www.gnu.org/licenses/>.
+ */
+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,
+        ];
+    }
+);

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) {

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('');
-
+        $account = factory(Account::class)->make();
-        // 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,
-            ]
-        );
-
         $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']);