From a5fd821e0c55324d1bb73e7eaad393036f35abb5 Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 7 Mar 2018 10:18:22 +0100 Subject: [PATCH] Code to implement #1168 and #1197. --- app/Http/Controllers/PiggyBankController.php | 26 ++----- .../Transaction/ConvertController.php | 8 -- .../Transaction/SingleController.php | 57 +------------- .../Transaction/SplitController.php | 8 +- app/Support/ExpandedForm.php | 56 ++++++++++++++ config/twigbridge.php | 2 +- resources/views/piggy-banks/create.twig | 2 +- resources/views/piggy-banks/edit.twig | 2 +- resources/views/transactions/convert.twig | 6 +- .../views/transactions/single/create.twig | 4 +- resources/views/transactions/single/edit.twig | 4 +- resources/views/transactions/split/edit.twig | 11 +-- .../Controllers/PiggyBankControllerTest.php | 58 ++++++++------- .../Transaction/ConvertControllerTest.php | 74 +++++++++++++------ .../Transaction/SingleControllerTest.php | 29 +++----- .../Transaction/SplitControllerTest.php | 23 +++--- 16 files changed, 190 insertions(+), 180 deletions(-) diff --git a/app/Http/Controllers/PiggyBankController.php b/app/Http/Controllers/PiggyBankController.php index 7ba0315b5c..afc8279d20 100644 --- a/app/Http/Controllers/PiggyBankController.php +++ b/app/Http/Controllers/PiggyBankController.php @@ -23,11 +23,8 @@ declare(strict_types=1); namespace FireflyIII\Http\Controllers; use Carbon\Carbon; -use ExpandedForm; use FireflyIII\Http\Requests\PiggyBankFormRequest; -use FireflyIII\Models\AccountType; use FireflyIII\Models\PiggyBank; -use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface; use Illuminate\Http\Request; use Illuminate\Pagination\LengthAwarePaginator; @@ -99,29 +96,20 @@ class PiggyBankController extends Controller } /** - * @param AccountRepositoryInterface $repository - * - * @return View + * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View */ - public function create(AccountRepositoryInterface $repository) + public function create() { - $accounts = ExpandedForm::makeSelectList($repository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET])); $subTitle = trans('firefly.new_piggy_bank'); $subTitleIcon = 'fa-plus'; - if (0 === count($accounts)) { - Session::flash('error', strval(trans('firefly.need_at_least_one_account'))); - - return redirect(route('new-user.index')); - } - // put previous url in session if not redirect from store (not "create another"). if (true !== session('piggy-banks.create.fromStore')) { $this->rememberPreviousUri('piggy-banks.create.uri'); } Session::forget('piggy-banks.create.fromStore'); - return view('piggy-banks.create', compact('accounts', 'subTitle', 'subTitleIcon')); + return view('piggy-banks.create', compact('subTitle', 'subTitleIcon')); } /** @@ -155,14 +143,12 @@ class PiggyBankController extends Controller } /** - * @param AccountRepositoryInterface $repository - * @param PiggyBank $piggyBank + * @param PiggyBank $piggyBank * * @return View */ - public function edit(AccountRepositoryInterface $repository, PiggyBank $piggyBank) + public function edit(PiggyBank $piggyBank) { - $accounts = ExpandedForm::makeSelectList($repository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET])); $subTitle = trans('firefly.update_piggy_title', ['name' => $piggyBank->name]); $subTitleIcon = 'fa-pencil'; $targetDate = null; @@ -191,7 +177,7 @@ class PiggyBankController extends Controller } Session::forget('piggy-banks.edit.fromUpdate'); - return view('piggy-banks.edit', compact('subTitle', 'subTitleIcon', 'piggyBank', 'accounts', 'preFilled')); + return view('piggy-banks.edit', compact('subTitle', 'subTitleIcon', 'piggyBank', 'preFilled')); } /** diff --git a/app/Http/Controllers/Transaction/ConvertController.php b/app/Http/Controllers/Transaction/ConvertController.php index 9541162376..5905de782f 100644 --- a/app/Http/Controllers/Transaction/ConvertController.php +++ b/app/Http/Controllers/Transaction/ConvertController.php @@ -22,11 +22,9 @@ declare(strict_types=1); namespace FireflyIII\Http\Controllers\Transaction; -use ExpandedForm; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Http\Controllers\Controller; use FireflyIII\Models\Account; -use FireflyIII\Models\AccountType; use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; use FireflyIII\Repositories\Account\AccountRepositoryInterface; @@ -40,9 +38,6 @@ use View; */ class ConvertController extends Controller { - /** @var AccountRepositoryInterface */ - private $accounts; - /** @var JournalRepositoryInterface */ private $repository; @@ -56,7 +51,6 @@ class ConvertController extends Controller // some useful repositories: $this->middleware( function ($request, $next) { - $this->accounts = app(AccountRepositoryInterface::class); $this->repository = app(JournalRepositoryInterface::class); app('view')->share('title', trans('firefly.transactions')); @@ -81,7 +75,6 @@ class ConvertController extends Controller } // @codeCoverageIgnoreEnd $positiveAmount = $this->repository->getJournalTotal($journal); - $assetAccounts = ExpandedForm::makeSelectList($this->accounts->getActiveAccountsByType([AccountType::DEFAULT, AccountType::ASSET])); $sourceType = $journal->transactionType; $subTitle = trans('firefly.convert_to_' . $destinationType->type, ['description' => $journal->description]); $subTitleIcon = 'fa-exchange'; @@ -110,7 +103,6 @@ class ConvertController extends Controller 'sourceType', 'destinationType', 'journal', - 'assetAccounts', 'positiveAmount', 'sourceAccount', 'destinationAccount', diff --git a/app/Http/Controllers/Transaction/SingleController.php b/app/Http/Controllers/Transaction/SingleController.php index 8dd3023290..d2b36e97e9 100644 --- a/app/Http/Controllers/Transaction/SingleController.php +++ b/app/Http/Controllers/Transaction/SingleController.php @@ -29,13 +29,10 @@ use FireflyIII\Events\UpdatedTransactionJournal; use FireflyIII\Helpers\Attachments\AttachmentHelperInterface; use FireflyIII\Http\Controllers\Controller; use FireflyIII\Http\Requests\JournalFormRequest; -use FireflyIII\Models\Account; -use FireflyIII\Models\AccountType; use FireflyIII\Models\Note; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; -use FireflyIII\Repositories\Account\AccountRepositoryInterface; use FireflyIII\Repositories\Budget\BudgetRepositoryInterface; use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; use FireflyIII\Repositories\Journal\JournalRepositoryInterface; @@ -51,9 +48,6 @@ use View; */ class SingleController extends Controller { - /** @var AccountRepositoryInterface */ - private $accounts; - /** @var AttachmentHelperInterface */ private $attachments; @@ -82,7 +76,6 @@ class SingleController extends Controller // some useful repositories: $this->middleware( function ($request, $next) { - $this->accounts = app(AccountRepositoryInterface::class); $this->budgets = app(BudgetRepositoryInterface::class); $this->piggyBanks = app(PiggyBankRepositoryInterface::class); $this->attachments = app(AttachmentHelperInterface::class); @@ -163,7 +156,6 @@ class SingleController extends Controller { $what = strtolower($what); $what = $request->old('what') ?? $what; - $assetAccounts = $this->groupedActiveAccountList(); $budgets = ExpandedForm::makeSelectListWithEmpty($this->budgets->getActiveBudgets()); $piggyBanks = $this->piggyBanks->getPiggyBanksWithAmount(); $piggies = ExpandedForm::makeSelectListWithEmpty($piggyBanks); @@ -192,7 +184,7 @@ class SingleController extends Controller return view( 'transactions.single.create', - compact('assetAccounts', 'subTitleIcon', 'budgets', 'what', 'piggies', 'subTitle', 'optionalFields', 'preFilled') + compact('subTitleIcon', 'budgets', 'what', 'piggies', 'subTitle', 'optionalFields', 'preFilled') ); } @@ -268,9 +260,8 @@ class SingleController extends Controller return redirect(route('transactions.split.edit', [$journal->id])); } - $what = strtolower($transactionType); - $assetAccounts = $this->groupedAccountList(); - $budgetList = ExpandedForm::makeSelectListWithEmpty($this->budgets->getBudgets()); + $what = strtolower($transactionType); + $budgetList = ExpandedForm::makeSelectListWithEmpty($this->budgets->getBudgets()); // view related code $subTitle = trans('breadcrumbs.edit_journal', ['description' => $journal->description]); @@ -330,7 +321,7 @@ class SingleController extends Controller return view( 'transactions.single.edit', - compact('journal', 'optionalFields', 'assetAccounts', 'what', 'budgetList', 'subTitle') + compact('journal', 'optionalFields', 'what', 'budgetList', 'subTitle') )->with('data', $preFilled); } @@ -440,46 +431,6 @@ class SingleController extends Controller return redirect($this->getPreviousUri('transactions.edit.uri')); } - /** - * @return array - */ - private function groupedAccountList(): array - { - $accounts = $this->accounts->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET]); - $return = []; - /** @var Account $account */ - foreach ($accounts as $account) { - $type = $account->getMeta('accountRole'); - if (0 === strlen($type)) { - $type = 'no_account_type'; // @codeCoverageIgnore - } - $key = strval(trans('firefly.opt_group_' . $type)); - $return[$key][$account->id] = $account->name; - } - - return $return; - } - - /** - * @return array - */ - private function groupedActiveAccountList(): array - { - $accounts = $this->accounts->getActiveAccountsByType([AccountType::DEFAULT, AccountType::ASSET]); - $return = []; - /** @var Account $account */ - foreach ($accounts as $account) { - $type = $account->getMeta('accountRole'); - if (0 === strlen($type)) { - $type = 'no_account_type'; // @codeCoverageIgnore - } - $key = strval(trans('firefly.opt_group_' . $type)); - $return[$key][$account->id] = $account->name; - } - - return $return; - } - /** * @param TransactionJournal $journal * diff --git a/app/Http/Controllers/Transaction/SplitController.php b/app/Http/Controllers/Transaction/SplitController.php index b82633bef4..20c3fb9403 100644 --- a/app/Http/Controllers/Transaction/SplitController.php +++ b/app/Http/Controllers/Transaction/SplitController.php @@ -100,15 +100,13 @@ class SplitController extends Controller $uploadSize = min(Steam::phpBytes(ini_get('upload_max_filesize')), Steam::phpBytes(ini_get('post_max_size'))); $currencies = $this->currencies->get(); - $accountList = $this->accounts->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET]); - $assetAccounts = ExpandedForm::makeSelectList($accountList); $optionalFields = Preferences::get('transaction_journal_optional_fields', [])->data; $budgets = ExpandedForm::makeSelectListWithEmpty($this->budgets->getActiveBudgets()); $preFilled = $this->arrayFromJournal($request, $journal); $subTitle = trans('breadcrumbs.edit_journal', ['description' => $journal->description]); $subTitleIcon = 'fa-pencil'; - - $accountArray = []; + $accountList = $this->accounts->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT]); + $accountArray = []; // account array to display currency info: /** @var Account $account */ foreach ($accountList as $account) { @@ -159,7 +157,7 @@ class SplitController extends Controller // @codeCoverageIgnoreEnd $type = strtolower($this->repository->getTransactionType($journal)); - Session::flash('success', strval(trans('firefly.updated_' . $type, ['description' => $data['description']]))); + Session::flash('success', strval(trans('firefly.updated_' . $type, ['description' => $journal->description]))); Preferences::mark(); // @codeCoverageIgnoreStart diff --git a/app/Support/ExpandedForm.php b/app/Support/ExpandedForm.php index b04e92b11d..6af1139624 100644 --- a/app/Support/ExpandedForm.php +++ b/app/Support/ExpandedForm.php @@ -25,6 +25,10 @@ namespace FireflyIII\Support; use Amount as Amt; use Carbon\Carbon; use Eloquent; +use FireflyIII\Models\Account; +use FireflyIII\Models\AccountType; +use FireflyIII\Repositories\Account\AccountRepositoryInterface; +use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; use Illuminate\Support\Collection; use Illuminate\Support\MessageBag; use RuntimeException; @@ -61,6 +65,58 @@ class ExpandedForm return $this->currencyField($name, 'amount-small', $value, $options); } + /** + * @param string $name + * @param null $value + * @param array $options + * + * @return string + * @throws \Throwable + */ + public function assetAccountList(string $name, $value = null, array $options = []): string + { + // properties for cache + $cache = new CacheProperties; + $cache->addProperty('exp-form-asset-list'); + $cache->addProperty($name); + $cache->addProperty($value); + $cache->addProperty($options); + + if ($cache->has()) { + return $cache->get(); + } + // make repositories + /** @var AccountRepositoryInterface $repository */ + $repository = app(AccountRepositoryInterface::class); + /** @var CurrencyRepositoryInterface $currencyRepos */ + $currencyRepos = app(CurrencyRepositoryInterface::class); + + $assetAccounts = $repository->getAccountsByType([AccountType::ASSET, AccountType::DEFAULT]); + $defaultCurrency = app('amount')->getDefaultCurrency(); + $grouped = []; + // group accounts: + /** @var Account $account */ + foreach ($assetAccounts as $account) { + $balance = app('steam')->balance($account, new Carbon); + $currencyId = intval($account->getMeta('currency_id')); + $currency = $currencyRepos->findNull($currencyId); + $role = $account->getMeta('accountRole'); + if (0 === strlen($role)) { + $role = 'no_account_type'; // @codeCoverageIgnore + } + if (is_null($currency)) { + $currency = $defaultCurrency; + } + + $key = strval(trans('firefly.opt_group_' . $role)); + $grouped[$key][$account->id] = $account->name . ' (' . app('amount')->formatAnything($currency, $balance, false) . ')'; + } + $res = $this->select($name, $grouped, $value, $options); + $cache->store($res); + + return $res; + } + /** * @param $name * @param null $value diff --git a/config/twigbridge.php b/config/twigbridge.php index 5d7467be5b..2433a298c2 100644 --- a/config/twigbridge.php +++ b/config/twigbridge.php @@ -176,7 +176,7 @@ return [ 'is_safe' => [ 'date', 'text', 'select', 'balance', 'optionsList', 'checkbox', 'amount', 'tags', 'integer', 'textarea', 'location', 'multiRadio', 'file', 'multiCheckbox', 'staticText', 'amountSmall', 'password', 'nonSelectableBalance', 'nonSelectableAmount', - 'number', + 'number', 'assetAccountList', ], ], 'Form' => [ diff --git a/resources/views/piggy-banks/create.twig b/resources/views/piggy-banks/create.twig index 439177c5d7..ecedf82dc2 100644 --- a/resources/views/piggy-banks/create.twig +++ b/resources/views/piggy-banks/create.twig @@ -18,7 +18,7 @@
{{ ExpandedForm.text('name') }} - {{ ExpandedForm.select('account_id',accounts,null,{'label' : 'saveOnAccount'|_}) }} + {{ ExpandedForm.assetAccountList('account_id', null, {label: 'saveOnAccount'|_ }) }} {{ ExpandedForm.amount('targetamount') }}
diff --git a/resources/views/piggy-banks/edit.twig b/resources/views/piggy-banks/edit.twig index 54dbb62500..427a9670a5 100644 --- a/resources/views/piggy-banks/edit.twig +++ b/resources/views/piggy-banks/edit.twig @@ -19,7 +19,7 @@
{{ ExpandedForm.text('name') }} - {{ ExpandedForm.select('account_id',accounts,null,{'label' : 'saveOnAccount'|_}) }} + {{ ExpandedForm.assetAccountList('account_id', null, {label: 'saveOnAccount'|_ }) }} {{ ExpandedForm.amount('targetamount') }}
diff --git a/resources/views/transactions/convert.twig b/resources/views/transactions/convert.twig index f51af285fe..8ab8cedb31 100644 --- a/resources/views/transactions/convert.twig +++ b/resources/views/transactions/convert.twig @@ -92,8 +92,7 @@

- {{ ExpandedForm.select('destination_account_asset', assetAccounts) }} - + {{ ExpandedForm.assetAccountList('destination_account_asset', null) }} {% endif %} @@ -146,8 +145,7 @@

- - {{ ExpandedForm.select('source_account_asset', assetAccounts) }} + {{ ExpandedForm.assetAccountList('source_account_asset', null) }} {% endif %} {# FIVE #} diff --git a/resources/views/transactions/single/create.twig b/resources/views/transactions/single/create.twig index e8f2f7bde5..bdad680bf9 100644 --- a/resources/views/transactions/single/create.twig +++ b/resources/views/transactions/single/create.twig @@ -33,7 +33,7 @@ {{ ExpandedForm.text('description') }} {# SELECTABLE SOURCE ACCOUNT ONLY FOR WITHDRAWALS AND TRANSFERS #} - {{ ExpandedForm.select('source_account_id', assetAccounts, null, {label: trans('form.asset_source_account')}) }} + {{ ExpandedForm.assetAccountList('source_account_id', null, {label: trans('form.asset_source_account')}) }} {# FREE FORMAT SOURCE ACCOUNT ONLY FOR DEPOSITS #} {{ ExpandedForm.text('source_account_name', null, {label: trans('form.revenue_account')}) }} @@ -42,7 +42,7 @@ {{ ExpandedForm.text('destination_account_name', null, {label: trans('form.expense_account')}) }} {# SELECTABLE DESTINATION ACCOUNT ONLY FOR TRANSFERS AND DEPOSITS #} - {{ ExpandedForm.select('destination_account_id', assetAccounts, null, {label: trans('form.asset_destination_account')} ) }} + {{ ExpandedForm.assetAccountList('destination_account_id', null, {label: trans('form.asset_destination_account')} ) }} {# ALWAYS SHOW AMOUNT #} {{ ExpandedForm.amount('amount') }} diff --git a/resources/views/transactions/single/edit.twig b/resources/views/transactions/single/edit.twig index b3c2241db2..aba0cb3a51 100644 --- a/resources/views/transactions/single/edit.twig +++ b/resources/views/transactions/single/edit.twig @@ -38,7 +38,7 @@ {# SELECTABLE SOURCE ACCOUNT ONLY FOR WITHDRAWALS AND TRANSFERS #} {% if what == 'transfer' or what == 'withdrawal' %} - {{ ExpandedForm.select('source_account_id',assetAccounts, data.source_account_id, {label: trans('form.asset_source_account')}) }} + {{ ExpandedForm.assetAccountList('source_account_id', data.source_account_id, {label: trans('form.asset_source_account')}) }} {% endif %} {# FREE FORMAT SOURCE ACCOUNT ONLY FOR DEPOSITS #} @@ -53,7 +53,7 @@ {# SELECTABLE DESTINATION ACCOUNT ONLY FOR TRANSFERS AND DEPOSITS #} {% if what == 'transfer' or what == 'deposit' %} - {{ ExpandedForm.select('destination_account_id',assetAccounts, data.destination_account_id, {label: trans('form.asset_destination_account')} ) }} + {{ ExpandedForm.assetAccountList('destination_account_id', data.destination_account_id, {label: trans('form.asset_destination_account')} ) }} {% endif %} {# ALWAYS SHOW AMOUNT #} diff --git a/resources/views/transactions/split/edit.twig b/resources/views/transactions/split/edit.twig index 5859ec3f6f..e26cc3b229 100644 --- a/resources/views/transactions/split/edit.twig +++ b/resources/views/transactions/split/edit.twig @@ -43,17 +43,12 @@ {# show source if withdrawal or transfer #} {% if preFilled.what == 'withdrawal' or preFilled.what == 'transfer' %} - {{ ExpandedForm.select('journal_source_account_id', assetAccounts, preFilled.journal_source_account_id) }} + {{ ExpandedForm.assetAccountList('journal_source_account_id', preFilled.journal_source_account_id) }} {% endif %} {# show destination account id, if deposit (is asset): #} - {% if preFilled.what == 'deposit' %} - {{ ExpandedForm.select('journal_destination_account_id', assetAccounts, preFilled.journal_destination_account_id) }} - {% endif %} - - {# show static destination if transfer #} - {% if preFilled.what == 'transfer' %} - {{ ExpandedForm.select('journal_destination_account_id', assetAccounts, preFilled.journal_destination_account_id) }} + {% if preFilled.what == 'deposit' or preFilled.what == 'transfer' %} + {{ ExpandedForm.assetAccountList('journal_destination_account_id', preFilled.journal_destination_account_id) }} {% endif %} {# TOTAL AMOUNT IS STATIC TEXT #} diff --git a/tests/Feature/Controllers/PiggyBankControllerTest.php b/tests/Feature/Controllers/PiggyBankControllerTest.php index 90460a3bb7..61b8294b02 100644 --- a/tests/Feature/Controllers/PiggyBankControllerTest.php +++ b/tests/Feature/Controllers/PiggyBankControllerTest.php @@ -22,11 +22,14 @@ declare(strict_types=1); namespace Tests\Feature\Controllers; +use Amount; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Models\PiggyBank; +use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionJournal; use FireflyIII\Repositories\Account\AccountRepositoryInterface; +use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface; use FireflyIII\Repositories\Journal\JournalRepositoryInterface; use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface; use Illuminate\Support\Collection; @@ -80,12 +83,25 @@ class PiggyBankControllerTest extends TestCase public function testCreate() { // mock stuff - $account = factory(Account::class)->make(); - $accountRepos = $this->mock(AccountRepositoryInterface::class); + + $journalRepos = $this->mock(JournalRepositoryInterface::class); - $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); + + // new account list thing. + $currency = TransactionCurrency::first(); + $account = factory(Account::class)->make(); + $currencyRepos = $this->mock(CurrencyRepositoryInterface::class); + $currencyRepos->shouldReceive('findNull')->andReturn($currency); + + $accountRepos = $this->mock(AccountRepositoryInterface::class); $accountRepos->shouldReceive('getAccountsByType') - ->withArgs([[AccountType::DEFAULT, AccountType::ASSET]])->andReturn(new Collection([$account]))->once(); + ->withArgs([[AccountType::ASSET, AccountType::DEFAULT]])->andReturn(new Collection([$account]))->once(); + + Amount::shouldReceive('getDefaultCurrency')->andReturn($currency); + Amount::shouldReceive('balance')->andReturn('0'); + + $journalRepos->shouldReceive('first')->once()->andReturn(new TransactionJournal); + $this->be($this->user()); $response = $this->get(route('piggy-banks.create')); @@ -93,25 +109,6 @@ class PiggyBankControllerTest extends TestCase $response->assertSee('