Fix #2434

app/Http/Controllers/Account/DeleteController.php

@@ -27,6 +27,7 @@ namespace FireflyIII\Http\Controllers\Account;
 use FireflyIII\Http\Controllers\Controller;
 use FireflyIII\Models\Account;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
+use FireflyIII\Support\Http\Controllers\UserNavigation;
 use Illuminate\Http\Request;
 
 /**
@@ -34,6 +35,8 @@ use Illuminate\Http\Request;
  */
 class DeleteController extends Controller
 {
+    use UserNavigation;
+
     /** @var AccountRepositoryInterface The account repository */
     private $repository;
 
@@ -67,6 +70,10 @@ class DeleteController extends Controller
      */
     public function delete(Account $account)
     {
+        if (!$this->isEditableAccount($account)) {
+            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
+        }
+
         $typeName    = config(sprintf('firefly.shortNamesByFullName.%s', $account->accountType->type));
         $subTitle    = (string)trans(sprintf('firefly.delete_%s_account', $typeName), ['name' => $account->name]);
         $accountList = app('expandedform')->makeSelectListWithEmpty($this->repository->getAccountsByType([$account->accountType->type]));
@@ -89,6 +96,10 @@ class DeleteController extends Controller
      */
     public function destroy(Request $request, Account $account)
     {
+        if (!$this->isEditableAccount($account)) {
+            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
+        }
+
         $type     = $account->accountType->type;
         $typeName = config(sprintf('firefly.shortNamesByFullName.%s', $type));
         $name     = $account->name;

app/Http/Controllers/Account/EditController.php

@@ -30,6 +30,7 @@ use FireflyIII\Models\Account;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface;
 use FireflyIII\Support\Http\Controllers\ModelInformation;
+use FireflyIII\Support\Http\Controllers\UserNavigation;
 use Illuminate\Http\Request;
 
 /**
@@ -38,7 +39,7 @@ use Illuminate\Http\Request;
  */
 class EditController extends Controller
 {
-    use ModelInformation;
+    use ModelInformation, UserNavigation;
     /** @var CurrencyRepositoryInterface The currency repository */
     private $currencyRepos;
     /** @var AccountRepositoryInterface The account repository */
@@ -79,6 +80,10 @@ class EditController extends Controller
      */
     public function edit(Request $request, Account $account, AccountRepositoryInterface $repository)
     {
+        if (!$this->isEditableAccount($account)) {
+            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
+        }
+
         $objectType     = config('firefly.shortNamesByFullName')[$account->accountType->type];
         $subTitle       = (string)trans(sprintf('firefly.edit_%s_account', $objectType), ['name' => $account->name]);
         $subTitleIcon   = config(sprintf('firefly.subIconsByIdentifier.%s', $objectType));
@@ -144,6 +149,10 @@ class EditController extends Controller
      */
     public function update(AccountFormRequest $request, Account $account)
     {
+        if (!$this->isEditableAccount($account)) {
+            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
+        }
+
         $data = $request->getAccountData();
         $this->repository->update($account, $data);
 

app/Http/Controllers/Account/ReconcileController.php

@@ -86,6 +86,10 @@ class ReconcileController extends Controller
      */
     public function reconcile(Account $account, Carbon $start = null, Carbon $end = null)
     {
+        if (!$this->isEditableAccount($account)) {
+            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
+        }
+
         if (AccountType::ASSET !== $account->accountType->type) {
             // @codeCoverageIgnoreStart
             session()->flash('error', (string)trans('firefly.must_be_asset_account'));
@@ -146,6 +150,10 @@ class ReconcileController extends Controller
      */
     public function submit(ReconciliationStoreRequest $request, Account $account, Carbon $start, Carbon $end)
     {
+        if (!$this->isEditableAccount($account)) {
+            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
+        }
+
         Log::debug('In ReconcileController::submit()');
         $data = $request->getAll();
 
@@ -178,6 +186,10 @@ class ReconcileController extends Controller
      */
     private function createReconciliation(Account $account, Carbon $start, Carbon $end, string $difference): string
     {
+        if (!$this->isEditableAccount($account)) {
+            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
+        }
+
         $reconciliation = $this->accountRepos->getReconciliation($account);
         $currency       = $this->accountRepos->getAccountCurrency($account) ?? app('amount')->getDefaultCurrency();
         $source         = $reconciliation;

app/Http/Controllers/Account/ShowController.php

@@ -28,7 +28,6 @@ use Exception;
 use FireflyIII\Helpers\Collector\GroupCollectorInterface;
 use FireflyIII\Http\Controllers\Controller;
 use FireflyIII\Models\Account;
-use FireflyIII\Models\AccountType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface;
 use FireflyIII\Support\Http\Controllers\PeriodOverview;
@@ -87,8 +86,8 @@ class ShowController extends Controller
      */
     public function show(Request $request, Account $account, Carbon $start = null, Carbon $end = null)
     {
-        if (in_array($account->accountType->type, [AccountType::INITIAL_BALANCE, AccountType::RECONCILIATION], true)) {
+        if (!$this->isEditableAccount($account)) {
-            return $this->redirectToOriginalAccount($account); // @codeCoverageIgnore
+            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
         }
 
         /** @var Carbon $start */
@@ -145,9 +144,10 @@ class ShowController extends Controller
      */
     public function showAll(Request $request, Account $account)
     {
-        if (AccountType::INITIAL_BALANCE === $account->accountType->type) {
+        if (!$this->isEditableAccount($account)) {
-            return $this->redirectToOriginalAccount($account); // @codeCoverageIgnore
+            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
         }
+
         $isLiability  = $this->repository->isLiability($account);
         $objectType   = config(sprintf('firefly.shortNamesByFullName.%s', $account->accountType->type));
         $end          = new Carbon;

app/Http/Controllers/Transaction/ConvertController.php

@@ -35,6 +35,7 @@ use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
 use FireflyIII\Services\Internal\Update\JournalUpdateService;
 use FireflyIII\Support\Http\Controllers\ModelInformation;
+use FireflyIII\Support\Http\Controllers\UserNavigation;
 use FireflyIII\Transformers\TransactionGroupTransformer;
 use FireflyIII\Validation\AccountValidator;
 use Illuminate\Http\Request;
@@ -49,7 +50,7 @@ use View;
  */
 class ConvertController extends Controller
 {
-    use ModelInformation;
+    use ModelInformation, UserNavigation;
 
     /** @var JournalRepositoryInterface Journals and transactions overview */
     private $repository;
@@ -87,16 +88,16 @@ class ConvertController extends Controller
      */
     public function index(TransactionType $destinationType, TransactionGroup $group)
     {
+        if (!$this->isEditableGroup($group)) {
+            return $this->redirectGroupToAccount($group); // @codeCoverageIgnore
+        }
+
         /** @var TransactionGroupTransformer $transformer */
         $transformer = app(TransactionGroupTransformer::class);
 
         /** @var TransactionJournal $first */
         $first      = $group->transactionJournals()->first();
         $sourceType = $first->transactionType;
-        // return to account.
-        if (!in_array($sourceType->type, [TransactionType::WITHDRAWAL, TransactionType::TRANSFER, TransactionType::DEPOSIT], true)) {
-            return $this->redirectToAccount($first); // @codeCoverageIgnore
-        }
 
         $groupTitle   = $group->title ?? $first->description;
         $groupArray   = $transformer->transformObject($group);
@@ -144,6 +145,10 @@ class ConvertController extends Controller
      */
     public function postIndex(Request $request, TransactionType $destinationType, TransactionGroup $group)
     {
+        if (!$this->isEditableGroup($group)) {
+            return $this->redirectGroupToAccount($group); // @codeCoverageIgnore
+        }
+
         /** @var TransactionJournal $journal */
         foreach ($group->transactionJournals as $journal) {
             // catch FF exception.

app/Http/Controllers/Transaction/DeleteController.php

@@ -26,6 +26,7 @@ use FireflyIII\Http\Controllers\Controller;
 use FireflyIII\Models\TransactionGroup;
 use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
 use FireflyIII\Repositories\TransactionGroup\TransactionGroupRepositoryInterface;
+use FireflyIII\Support\Http\Controllers\UserNavigation;
 use Illuminate\Http\RedirectResponse;
 use Log;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
@@ -36,6 +37,7 @@ use URL;
  */
 class DeleteController extends Controller
 {
+    use UserNavigation;
     /** @var TransactionGroupRepositoryInterface */
     private $repository;
 
@@ -69,6 +71,10 @@ class DeleteController extends Controller
      */
     public function delete(TransactionGroup $group)
     {
+        if (!$this->isEditableGroup($group)) {
+            return $this->redirectGroupToAccount($group); // @codeCoverageIgnore
+        }
+
         Log::debug(sprintf('Start of delete view for group #%d', $group->id));
 
         $journal = $group->transactionJournals->first();
@@ -94,6 +100,10 @@ class DeleteController extends Controller
      */
     public function destroy(TransactionGroup $group): RedirectResponse
     {
+        if (!$this->isEditableGroup($group)) {
+            return $this->redirectGroupToAccount($group); // @codeCoverageIgnore
+        }
+
         $journal = $group->transactionJournals->first();
         if (null === $journal) {
             throw new NotFoundHttpException;

app/Http/Controllers/Transaction/EditController.php

@@ -25,13 +25,14 @@ namespace FireflyIII\Http\Controllers\Transaction;
 use FireflyIII\Http\Controllers\Controller;
 use FireflyIII\Models\TransactionGroup;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
+use FireflyIII\Support\Http\Controllers\UserNavigation;
 
 /**
  * Class EditController
  */
 class EditController extends Controller
 {
-
+    use UserNavigation;
     /**
      * EditController constructor.
      * @codeCoverageIgnore
@@ -66,6 +67,10 @@ class EditController extends Controller
      */
     public function edit(TransactionGroup $transactionGroup)
     {
+        if (!$this->isEditableGroup($transactionGroup)) {
+            return $this->redirectGroupToAccount($transactionGroup); // @codeCoverageIgnore
+        }
+
         /** @var AccountRepositoryInterface $repository */
         $repository           = app(AccountRepositoryInterface::class);
         $allowedOpposingTypes = config('firefly.allowed_opposing_types');

app/Support/Http/Controllers/UserNavigation.php

@@ -26,8 +26,9 @@ namespace FireflyIII\Support\Http\Controllers;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\Transaction;
+use FireflyIII\Models\TransactionGroup;
 use FireflyIII\Models\TransactionJournal;
-use Illuminate\Http\RedirectResponse;
+use FireflyIII\Models\TransactionType;
 use Illuminate\Support\ViewErrorBag;
 use Log;
 
@@ -37,6 +38,108 @@ use Log;
  */
 trait UserNavigation
 {
+
+    //if (!$this->isEditableAccount($account)) {
+    //            return $this->redirectAccountToAccount($account); // @codeCoverageIgnore
+    //        }
+
+    /**
+     * Will return false if you cant edit this account type.
+     *
+     * @param Account $account
+     *
+     * @return bool
+     */
+    protected function isEditableAccount(Account $account): bool
+    {
+        $editable = [AccountType::EXPENSE, AccountType::REVENUE, AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE];
+        $type     = $account->accountType->type;
+
+        return in_array($type, $editable, true);
+    }
+
+    /**
+     * @param TransactionGroup $group
+     *
+     * @return bool
+     */
+    protected function isEditableGroup(TransactionGroup $group): bool
+    {
+        /** @var TransactionJournal $journal */
+        $journal = $group->transactionJournals()->first();
+        if (null === $journal) {
+            return false;
+        }
+        $type     = $journal->transactionType->type;
+        $editable = [TransactionType::WITHDRAWAL, TransactionType::TRANSFER, TransactionType::DEPOSIT];
+
+        return in_array($type, $editable, true);
+    }
+
+    /**
+     * @param TransactionGroup $group
+     *
+     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
+     */
+    protected function redirectGroupToAccount(TransactionGroup $group)
+    {
+        /** @var TransactionJournal $journal */
+        $journal = $group->transactionJournals()->first();
+        if (null === $journal) {
+            Log::error(sprintf('No journals in group #%d', $group->id));
+
+            return redirect(route('index'));
+        }
+        // prefer redirect to everything but expense and revenue:
+        $transactions = $journal->transactions;
+        $ignore       = [AccountType::REVENUE, AccountType::EXPENSE, AccountType::RECONCILIATION, AccountType::INITIAL_BALANCE];
+        /** @var Transaction $transaction */
+        foreach ($transactions as $transaction) {
+            $type = $transaction->account->accountType->type;
+            if (!in_array($type, $ignore)) {
+                return redirect(route('accounts.show', [$transaction->account_id]));
+            }
+        }
+
+        return redirect(route('index'));
+    }
+
+    /**
+     * @param Account $account
+     *
+     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
+     */
+    protected function redirectAccountToAccount(Account $account)
+    {
+        $type = $account->accountType->type;
+        if (AccountType::RECONCILIATION === $type || AccountType::INITIAL_BALANCE === $type) {
+            // reconciliation must be stored somewhere in this account's transactions.
+
+            /** @var Transaction $transaction */
+            $transaction = $account->transactions()->first();
+            if (null === $transaction) {
+                Log::error(sprintf('Account #%d has no transactions. Dont know where it belongs.', $account->id));
+                session()->flash('error', trans('firefly.cant_find_redirect_account'));
+
+                return redirect(route('index'));
+            }
+            $journal = $transaction->transactionJournal;
+            /** @var Transaction $other */
+            $other = $journal->transactions()->where('id', '!=', $transaction->id)->first();
+            if (null === $other) {
+                Log::error(sprintf('Account #%d has no valid journals. Dont know where it belongs.', $account->id));
+                session()->flash('error', trans('firefly.cant_find_redirect_account'));
+
+                return redirect(route('index'));
+            }
+
+            return redirect(route('accounts.show', [$other->account_id]));
+        }
+
+        return redirect(route('index'));
+    }
+
+
     /**
      * Functionality:.
      *
@@ -59,94 +162,72 @@ trait UserNavigation
             Log::debug(sprintf('URI is now %s (uri contains jscript)', $uri));
         }
 
-        // "forbidden" words for specific identifiers:
-        // if these are in the previous URI, don't refer back there.
-        //        $array     = [
-        //            'accounts.delete.uri'          => '/accounts/show/',
-        //            'transactions.delete.uri'      => '/transactions/show/',
-        //            'attachments.delete.uri'       => '/attachments/show/',
-        //            'bills.delete.uri'             => '/bills/show/',
-        //            'budgets.delete.uri'           => '/budgets/show/',
-        //            'categories.delete.uri'        => '/categories/show/',
-        //            'currencies.delete.uri'        => '/currencies/show/',
-        //            'piggy-banks.delete.uri'       => '/piggy-banks/show/',
-        //            'tags.delete.uri'              => '/tags/show/',
-        //            'rules.delete.uri'             => '/rules/edit/',
-        //            'transactions.mass-delete.uri' => '/transactions/show/',
-        //        ];
-        //$forbidden = $array[$identifier] ?? '/show/';
-        //Log::debug(sprintf('The forbidden word for %s is "%s"', $identifier, $forbidden));
-
-
-        //        if (
-        //            !(false === strpos($identifier, 'delete'))
-        //            && !(false === strpos($uri, $forbidden))) {
-        //            $uri = $this->redirectUri;
-        //            //Log::debug(sprintf('URI is now %s (identifier contains "delete")', $uri));
-        //        }
-
-
-        // more debug notes:
-        //Log::debug(sprintf('strpos($identifier, "delete"): %s', var_export(strpos($identifier, 'delete'), true)));
-        //Log::debug(sprintf('strpos($uri, $forbidden): %s', var_export(strpos($uri, $forbidden), true)));
         Log::debug(sprintf('Return direct link %s', $uri));
         return $uri;
     }
-
+    //
-    /**
+    //    /**
-     * Redirect to asset account that transaction belongs to.
+    //     * Redirect to asset account that transaction belongs to.
-     *
+    //     *
-     * @param TransactionJournal $journal
+    //     * @param TransactionGroup $group
-     *
+    //     *
-     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
+    //     * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
-     * @codeCoverageIgnore
+    //     * @codeCoverageIgnore
-     */
+    //     */
-    protected function redirectToAccount(TransactionJournal $journal)
+    //    protected function redirectToAccount(TransactionGroup $group)
-    {
+    //    {
-        $valid        = [AccountType::DEFAULT, AccountType::ASSET];
+    //        $journals = $group->transactionJournals;
-        $transactions = $journal->transactions;
+    //        $first    = $journals->first();
-        /** @var Transaction $transaction */
+    //
-        foreach ($transactions as $transaction) {
+    //        if (null === $first) {
-            $account = $transaction->account;
+    //            return redirect(route('index'));
-            if (in_array($account->accountType->type, $valid, true)) {
+    //        }
-                return redirect(route('accounts.show', [$account->id]));
+    //
-            }
+    //
-        }
+    //        $valid        = [AccountType::DEFAULT, AccountType::ASSET];
-        // @codeCoverageIgnoreStart
+    //        $transactions = $journal->transactions;
-        session()->flash('error', (string)trans('firefly.cannot_redirect_to_account'));
+    //        /** @var Transaction $transaction */
-
+    //        foreach ($transactions as $transaction) {
-        return redirect(route('index'));
+    //            $account = $transaction->account;
-        // @codeCoverageIgnoreEnd
+    //            if (in_array($account->accountType->type, $valid, true)) {
-    }
+    //                return redirect(route('accounts.show', [$account->id]));
-
+    //            }
-    /**
+    //        }
-     * @param Account $account
+    //        // @codeCoverageIgnoreStart
-     *
+    //        session()->flash('error', (string)trans('firefly.cannot_redirect_to_account'));
-     * @return RedirectResponse|\Illuminate\Routing\Redirector
+    //
-     * @codeCoverageIgnore
+    //        return redirect(route('index'));
-     */
+    //        // @codeCoverageIgnoreEnd
-    protected function redirectToOriginalAccount(Account $account)
+    //    }
-    {
+    //
-        /** @var Transaction $transaction */
+    //    /**
-        $transaction = $account->transactions()->first();
+    //     * @param Account $account
-        if (null === $transaction) {
+    //     *
-            app('session')->flash('error', trans('firefly.account_missing_transaction', ['name' => e($account->name), 'id' => $account->id]));
+    //     * @return RedirectResponse|\Illuminate\Routing\Redirector
-            Log::error(sprintf('Expected a transaction. Account #%d has none. BEEP, error.', $account->id));
+    //     * @codeCoverageIgnore
-
+    //     */
-            return redirect(route('index'));
+    //    protected function redirectToOriginalAccount(Account $account)
-        }
+    //    {
-
+    //        /** @var Transaction $transaction */
-        $journal = $transaction->transactionJournal;
+    //        $transaction = $account->transactions()->first();
-        /** @var Transaction $opposingTransaction */
+    //        if (null === $transaction) {
-        $opposingTransaction = $journal->transactions()->where('transactions.id', '!=', $transaction->id)->first();
+    //            app('session')->flash('error', trans('firefly.account_missing_transaction', ['name' => e($account->name), 'id' => $account->id]));
-
+    //            Log::error(sprintf('Expected a transaction. Account #%d has none. BEEP, error.', $account->id));
-        if (null === $opposingTransaction) {
+    //
-            app('session')->flash('error', trans('firefly.account_missing_transaction', ['name' => e($account->name), 'id' => $account->id]));
+    //            return redirect(route('index'));
-            Log::error(sprintf('Expected an opposing transaction. Account #%d has none. BEEP, error.', $account->id));
+    //        }
-        }
+    //
-
+    //        $journal = $transaction->transactionJournal;
-        return redirect(route('accounts.show', [$opposingTransaction->account_id]));
+    //        /** @var Transaction $opposingTransaction */
-    }
+    //        $opposingTransaction = $journal->transactions()->where('transactions.id', '!=', $transaction->id)->first();
+    //
+    //        if (null === $opposingTransaction) {
+    //            app('session')->flash('error', trans('firefly.account_missing_transaction', ['name' => e($account->name), 'id' => $account->id]));
+    //            Log::error(sprintf('Expected an opposing transaction. Account #%d has none. BEEP, error.', $account->id));
+    //        }
+    //
+    //        return redirect(route('accounts.show', [$opposingTransaction->account_id]));
+    //    }
 
     /**
      * @param string $identifier

resources/lang/en_US/firefly.php

@@ -770,6 +770,7 @@ return [
     'start_reconcile'                  => 'Start reconciling',
     'cash_account_type'                => 'Cash',
     'cash'                             => 'cash',
+    'cant_find_redirect_account'       => 'Firefly III tried to redirect you but couldn\'t. Sorry about that. Back to the index.',
     'account_type'                     => 'Account type',
     'save_transactions_by_moving'      => 'Save these transaction(s) by moving them to another account:',
     'stored_new_account'               => 'New account ":name" stored!',

resources/views/v1/transactions/show.twig

@@ -176,7 +176,7 @@
                                        title="{{ journal.source_iban|default(journal.source_name) }}">{{ journal.source_name }}</a> &rarr;
                                     {% if type == 'Withdrawal' or type == 'Deposit' %}
                                         {{ formatAmountBySymbol(journal.amount*-1, journal.currency_symbol, journal.currency_decimal_places) }}
-                                    {% elseif type == 'Transfer' %}
+                                    {% elseif type == 'Transfer' or type == 'Opening balance' %}
                                         <span class="text-info">
                                         {{ formatAmountBySymbol(journal.amount, journal.currency_symbol, journal.currency_decimal_places, false) }}
                                     </span>
@@ -193,7 +193,6 @@
                                         {% endif %}
                                     {% endif %}
 
-
                                     &rarr;
                                     <a href="{{ route('accounts.show', journal.destination_id) }}"
                                        title="{{ journal.destination_iban|default(journal.destination_name) }}">{{ journal.destination_name }}</a>