From 81dce5d7c72d2247945aca7069f093b6127e9f59 Mon Sep 17 00:00:00 2001 From: James Cole Date: Thu, 1 Aug 2019 06:22:07 +0200 Subject: [PATCH] Fix #2179 --- app/Exceptions/GracefulNotFoundHandler.php | 168 ++++++++++++++++++ app/Exceptions/Handler.php | 6 + .../Transaction/DeleteController.php | 112 ++++++++++++ .../TransactionGroupRepository.php | 13 +- .../TransactionGroupRepositoryInterface.php | 7 +- .../Http/Controllers/UserNavigation.php | 60 ++++--- resources/views/v1/errors/404.twig | 5 + resources/views/v1/transactions/delete.twig | 35 ++++ resources/views/v1/transactions/show.twig | 2 + routes/breadcrumbs.php | 9 +- 10 files changed, 384 insertions(+), 33 deletions(-) create mode 100644 app/Exceptions/GracefulNotFoundHandler.php create mode 100644 app/Http/Controllers/Transaction/DeleteController.php create mode 100644 resources/views/v1/transactions/delete.twig diff --git a/app/Exceptions/GracefulNotFoundHandler.php b/app/Exceptions/GracefulNotFoundHandler.php new file mode 100644 index 0000000000..dfb34a3fe1 --- /dev/null +++ b/app/Exceptions/GracefulNotFoundHandler.php @@ -0,0 +1,168 @@ +. + */ + +namespace FireflyIII\Exceptions; + + +use Exception; +use FireflyIII\Models\Account; +use FireflyIII\Models\TransactionGroup; +use FireflyIII\Models\TransactionJournal; +use FireflyIII\User; +use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; +use Illuminate\Http\Request; +use Log; + +/** + * Class GracefulNotFoundHandler + */ +class GracefulNotFoundHandler extends ExceptionHandler +{ + /** + * Render an exception into an HTTP response. + * + * @param Request $request + * @param Exception $exception + * @SuppressWarnings(PHPMD.CyclomaticComplexity) + * @SuppressWarnings(PHPMD.NPathComplexity) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + * + * @return mixed + */ + public function render($request, Exception $exception) + { + $route = $request->route(); + $name = $route->getName(); + if (!auth()->check()) { + return parent::render($request, $exception); + } + + switch ($name) { + default: + Log::error(sprintf('GracefulNotFoundHandler cannot handle route with name "%s"', $name)); + + return parent::render($request, $exception); + case 'accounts.show': + return $this->handleAccount($request, $exception); + case 'transactions.show': + return $this->handleGroup($request, $exception); + break; + case 'attachments.show': + return redirect(route('index')); + break; + case 'bills.show': + $request->session()->reflash(); + + return redirect(route('bills.index')); + break; + case 'currencies.show': + $request->session()->reflash(); + + return redirect(route('currencies.index')); + break; + case 'budgets.show': + $request->session()->reflash(); + + return redirect(route('budgets.index')); + break; + case 'categories.show': + $request->session()->reflash(); + + return redirect(route('categories.index')); + break; + case 'rules.edit': + $request->session()->reflash(); + + return redirect(route('rules.index')); + break; + case 'transactions.mass.edit': + case 'transactions.mass.delete': + case 'transactions.bulk.edit': + $request->session()->reflash(); + + return redirect(route('index')); + break; + } + } + + /** + * @param Request $request + * @param Exception $exception + * + * @return \Illuminate\Http\Response|\Symfony\Component\HttpFoundation\Response + */ + private function handleAccount($request, Exception $exception) + { + Log::debug('404 page is probably a deleted account. Redirect to overview of account types.'); + /** @var User $user */ + $user = auth()->user(); + $route = $request->route(); + $accountId = (int)$route->parameter('account'); + /** @var Account $account */ + $account = $user->accounts()->with(['accountType'])->withTrashed()->find($accountId); + if (null === $account) { + Log::error(sprintf('Could not find account %d, so give big fat error.', $accountId)); + + return parent::render($request, $exception); + } + $type = $account->accountType; + $shortType = config(sprintf('firefly.shortNamesByFullName.%s', $type->type)); + $request->session()->reflash(); + + return redirect(route('accounts.index', [$shortType])); + } + + /** + * @param $request + * @param Exception $exception + * + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Http\Response|\Illuminate\Routing\Redirector|\Symfony\Component\HttpFoundation\Response + */ + private function handleGroup($request, Exception $exception) + { + Log::debug('404 page is probably a deleted group. Redirect to overview of group types.'); + /** @var User $user */ + $user = auth()->user(); + $route = $request->route(); + $groupId = (int)$route->parameter('transactionGroup'); + + /** @var TransactionGroup $group */ + $group = $user->transactionGroups()->withTrashed()->find($groupId); + if (null === $group) { + Log::error(sprintf('Could not find group %d, so give big fat error.', $groupId)); + + return parent::render($request, $exception); + } + /** @var TransactionJournal $journal */ + $journal = $group->transactionJournals()->withTrashed()->first(); + if (null === $journal) { + Log::error(sprintf('Could not find journal for group %d, so give big fat error.', $groupId)); + + return parent::render($request, $exception); + } + $type = $journal->transactionType->type; + $request->session()->reflash(); + + return redirect(route('transactions.index', [strtolower($type)])); + + } + +} \ No newline at end of file diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 969defaede..5e6ca46764 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -92,6 +92,12 @@ class Handler extends ExceptionHandler return response()->json(['message' => 'Internal Firefly III Exception. See log files.', 'exception' => get_class($exception)], 500); } + if($exception instanceof NotFoundHttpException) { + $handler = app(GracefulNotFoundHandler::class); + return $handler->render($request, $exception); + } + + if ($exception instanceof FireflyException || $exception instanceof ErrorException || $exception instanceof OAuthServerException) { $isDebug = config('app.debug'); diff --git a/app/Http/Controllers/Transaction/DeleteController.php b/app/Http/Controllers/Transaction/DeleteController.php new file mode 100644 index 0000000000..4a926364ef --- /dev/null +++ b/app/Http/Controllers/Transaction/DeleteController.php @@ -0,0 +1,112 @@ +. + */ + +namespace FireflyIII\Http\Controllers\Transaction; + + +use FireflyIII\Http\Controllers\Controller; +use FireflyIII\Models\TransactionGroup; +use FireflyIII\Repositories\Journal\JournalRepositoryInterface; +use FireflyIII\Repositories\TransactionGroup\TransactionGroupRepositoryInterface; +use Illuminate\Http\RedirectResponse; +use Log; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use URL; + +/** + * Class DeleteController + */ +class DeleteController extends Controller +{ + /** @var TransactionGroupRepositoryInterface */ + private $repository; + + /** + * IndexController constructor. + * @codeCoverageIgnore + */ + public function __construct() + { + parent::__construct(); + + // translations: + $this->middleware( + function ($request, $next) { + app('view')->share('title', (string)trans('firefly.transactions')); + app('view')->share('mainTitleIcon', 'fa-repeat'); + + $this->repository = app(TransactionGroupRepositoryInterface::class); + + return $next($request); + } + ); + } + + /** + * Shows the form that allows a user to delete a transaction journal. + * + * @param TransactionGroup $group + * + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector|View + */ + public function delete(TransactionGroup $group) + { + Log::debug(sprintf('Start of delete view for group #%d', $group->id)); + + $journal = $group->transactionJournals->first(); + if (null === $journal) { + throw new NotFoundHttpException; + } + $objectType = strtolower($journal->transaction_type_type ?? $journal->transactionType->type); + $subTitle = (string)trans('firefly.delete_' . $objectType, ['description' => $group->title ?? $journal->description]); + $previous = URL::previous(route('index')); + // put previous url in session + Log::debug('Will try to remember previous URI'); + $this->rememberPreviousUri('transactions.delete.uri'); + + return view('transactions.delete', compact('group', 'journal', 'subTitle', 'objectType', 'previous')); + } + + /** + * Actually destroys the journal. + * + * @param TransactionGroup $group + * + * @return RedirectResponse + */ + public function destroy(TransactionGroup $group): RedirectResponse + { + $journal = $group->transactionJournals->first(); + if (null === $journal) { + throw new NotFoundHttpException; + } + $objectType = strtolower($journal->transaction_type_type ?? $journal->transactionType->type); + session()->flash('success', (string)trans('firefly.deleted_' . strtolower($objectType), ['description' => $group->title ?? $journal->description])); + + $this->repository->destroy($group); + + app('preferences')->mark(); + + return redirect($this->getPreviousUri('transactions.delete.uri')); + } + + +} \ No newline at end of file diff --git a/app/Repositories/TransactionGroup/TransactionGroupRepository.php b/app/Repositories/TransactionGroup/TransactionGroupRepository.php index c98255e2e5..679d203136 100644 --- a/app/Repositories/TransactionGroup/TransactionGroupRepository.php +++ b/app/Repositories/TransactionGroup/TransactionGroupRepository.php @@ -39,6 +39,7 @@ use FireflyIII\Models\TransactionGroup; use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionJournalLink; use FireflyIII\Models\TransactionType; +use FireflyIII\Services\Internal\Destroy\TransactionGroupDestroyService; use FireflyIII\Services\Internal\Update\GroupUpdateService; use FireflyIII\Support\NullArrayObject; use Illuminate\Database\Eloquent\Builder; @@ -46,7 +47,7 @@ use Illuminate\Database\Eloquent\Builder; /** * Class TransactionGroupRepository */ -class TransactionGroupRepository implements TransactionGroupRepositoryInterface +class TransactionGroupRepository implements TransactionGroupRepositoryInterface { private $user; @@ -364,4 +365,14 @@ class TransactionGroupRepository implements TransactionGroupRepositoryInterface return $return; } + + /** + * @param TransactionGroup $group + */ + public function destroy(TransactionGroup $group): void + { + /** @var TransactionGroupDestroyService $service */ + $service = new TransactionGroupDestroyService; + $service->destroy($group); + } } \ No newline at end of file diff --git a/app/Repositories/TransactionGroup/TransactionGroupRepositoryInterface.php b/app/Repositories/TransactionGroup/TransactionGroupRepositoryInterface.php index 034b802fcf..b47713dd43 100644 --- a/app/Repositories/TransactionGroup/TransactionGroupRepositoryInterface.php +++ b/app/Repositories/TransactionGroup/TransactionGroupRepositoryInterface.php @@ -23,8 +23,8 @@ declare(strict_types=1); namespace FireflyIII\Repositories\TransactionGroup; -use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\TransactionGroup; +use FireflyIII\Services\Internal\Destroy\TransactionGroupDestroyService; use FireflyIII\Support\NullArrayObject; use FireflyIII\User; @@ -43,6 +43,11 @@ interface TransactionGroupRepositoryInterface */ public function getAttachments(TransactionGroup $group): array; + /** + * @param TransactionGroup $group + */ + public function destroy(TransactionGroup $group): void; + /** * Return all journal links for all journals in the group. * diff --git a/app/Support/Http/Controllers/UserNavigation.php b/app/Support/Http/Controllers/UserNavigation.php index a40f3254fa..2127771398 100644 --- a/app/Support/Http/Controllers/UserNavigation.php +++ b/app/Support/Http/Controllers/UserNavigation.php @@ -50,42 +50,46 @@ trait UserNavigation */ protected function getPreviousUri(string $identifier): string { - // Log::debug(sprintf('Trying to retrieve URL stored under "%s"', $identifier)); - // "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)); - + Log::debug(sprintf('Trying to retrieve URL stored under "%s"', $identifier)); $uri = (string)session($identifier); - //Log::debug(sprintf('The URI is %s', $uri)); - if ( - !(false === strpos($identifier, 'delete')) - && !(false === strpos($uri, $forbidden))) { - $uri = $this->redirectUri; - //Log::debug(sprintf('URI is now %s (identifier contains "delete")', $uri)); - } + Log::debug(sprintf('The URI is %s', $uri)); + if (!(false === strpos($uri, 'jscript'))) { $uri = $this->redirectUri; // @codeCoverageIgnore - //Log::debug(sprintf('URI is now %s (uri contains jscript)', $uri)); + 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; } diff --git a/resources/views/v1/errors/404.twig b/resources/views/v1/errors/404.twig index ee684195fb..5091a89d0a 100644 --- a/resources/views/v1/errors/404.twig +++ b/resources/views/v1/errors/404.twig @@ -39,6 +39,11 @@ The page you have requested does not exist. Please check that you have not entered the wrong URL. Did you make a typo perhaps?

+

+ If you were redirected to this page automatically, please accept my apologies. + There is a mention of this error in your log files and I would be grateful if you sent + me the error to me. +

diff --git a/resources/views/v1/transactions/delete.twig b/resources/views/v1/transactions/delete.twig new file mode 100644 index 0000000000..5d0b80117f --- /dev/null +++ b/resources/views/v1/transactions/delete.twig @@ -0,0 +1,35 @@ +{% extends "./layout/default" %} + +{% block breadcrumbs %} + {{ Breadcrumbs.render(Route.getCurrentRoute.getName, group) }} +{% endblock %} + +{% block content %} +
+ + +
+
+
+
+

{{ trans('form.delete_journal', {'description': group.title|default(journal.description)}) }}

+
+
+

+ {{ trans('form.permDeleteWarning') }} +

+ +

+ {{ trans('form.journal_areYouSure', {'description': group.title|default(journal.description)}) }} +

+
+
+ + {{ trans('form.cancel') }} +
+
+
+
+ +
+{% endblock %} diff --git a/resources/views/v1/transactions/show.twig b/resources/views/v1/transactions/show.twig index 80b8d0e487..bd3e40fff7 100644 --- a/resources/views/v1/transactions/show.twig +++ b/resources/views/v1/transactions/show.twig @@ -71,6 +71,8 @@
{{ 'edit'|_ }} + {{ 'delete'|_ }} + {% if groupArray.transactions[0].type != 'withdrawal' %} parent('transactions.show', $journal); + static function (BreadcrumbsGenerator $breadcrumbs, TransactionGroup $group) { + $breadcrumbs->parent('transactions.show', $group); + + $journal = $group->transactionJournals->first(); $breadcrumbs->push( - trans('breadcrumbs.delete_journal', ['description' => limitStringLength($journal->description)]), route('transactions.delete', [$journal->id]) + trans('breadcrumbs.delete_group', ['description' => limitStringLength($group->title ?? $journal->description)]), + route('transactions.delete', [$group->id]) ); } );