Refactor reconciliation routine

app/Http/Controllers/Account/ReconcileController.php

@@ -24,9 +24,11 @@ namespace FireflyIII\Http\Controllers\Account;
 
 use Carbon\Carbon;
 use FireflyIII\Exceptions\FireflyException;
+use FireflyIII\Factory\TransactionJournalFactory;
 use FireflyIII\Helpers\Collector\JournalCollectorInterface;
 use FireflyIII\Http\Controllers\Controller;
-use FireflyIII\Http\Requests\ReconciliationFormRequest;
+use FireflyIII\Http\Requests\ReconciliationStoreRequest;
+use FireflyIII\Http\Requests\ReconciliationUpdateRequest;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
 use FireflyIII\Models\Transaction;
@@ -35,8 +37,10 @@ use FireflyIII\Models\TransactionType;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface;
 use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
+use FireflyIII\Services\Internal\Update\TransactionUpdateService;
 use Illuminate\Http\Request;
 use Illuminate\Support\Collection;
+use Log;
 use Preferences;
 use Response;
 use Session;
@@ -80,7 +84,7 @@ class ReconcileController extends Controller
         $subTitle = trans('breadcrumbs.edit_journal', ['description' => $journal->description]);
 
         // journal related code
-        $pTransaction = $journal->positiveTransaction();
+        $pTransaction = $journal->positiveTransaction(); // TODO replace
         $preFilled    = [
             'date'     => $journal->dateAsString(),
             'category' => $journal->categoryAsString(),
@@ -182,7 +186,7 @@ class ReconcileController extends Controller
         /** @var CurrencyRepositoryInterface $currencyRepos */
         $currencyRepos = app(CurrencyRepositoryInterface::class);
         $currencyId    = intval($account->getMeta('currency_id'));
-        $currency      = $currencyRepos->find($currencyId);
+        $currency      = $currencyRepos->findNull($currencyId);
         if (0 === $currencyId) {
             $currency = app('amount')->getDefaultCurrency(); // @codeCoverageIgnore
         }
@@ -227,6 +231,7 @@ class ReconcileController extends Controller
      */
     public function show(JournalRepositoryInterface $repository, TransactionJournal $journal)
     {
+
         if (TransactionType::RECONCILIATION !== $journal->transactionType->type) {
             return redirect(route('transactions.show', [$journal->id]));
         }
@@ -240,51 +245,90 @@ class ReconcileController extends Controller
     }
 
     /**
-     * @param Request $request
+     * @param ReconciliationStoreRequest $request
-     * @param Account $account
+     * @param Account                    $account
-     * @param Carbon  $start
+     * @param Carbon                     $start
-     * @param Carbon  $end
+     * @param Carbon                     $end
      *
      * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
+     * @throws FireflyException
      */
-    public function submit(Request $request, Account $account, Carbon $start, Carbon $end)
+    public function submit(ReconciliationStoreRequest $request, Account $account, Carbon $start, Carbon $end)
     {
-        /** @var JournalRepositoryInterface $repository */
+        Log::debug('In ReconcileController::submit()');
-        $repository   = app(JournalRepositoryInterface::class);
+        $data = $request->getAll();
-        $transactions = $repository->getTransactionsById($request->get('transactions') ?? []);
+        /** @var TransactionUpdateService $service */
+        $service = app(TransactionUpdateService::class); // todo move to repos
+
         /** @var Transaction $transaction */
-        foreach ($transactions as $transaction) {
+        foreach ($data['transactions'] as $transactionId) {
-            $repository->reconcile($transaction); // mark as reconciled.
+            $service->reconcile(intval($transactionId));
         }
+        Log::debug('Reconciled all transactions.');
 
         // create reconciliation transaction (if necessary):
-        if ('create' === $request->get('reconcile')) {
+        if ('create' === $data['reconcile']) {
+            // get "opposing" account.
             /** @var AccountRepositoryInterface $accountRepos */
             $accountRepos   = app(AccountRepositoryInterface::class);
             $reconciliation = $accountRepos->getReconciliation($account);
-            $difference     = $request->get('difference');
 
-            // store journal between these two.
+
-            $data    = [
+            $difference  = $data['difference'];
-                'what'        => 'Reconciliation',
+            $source      = $reconciliation;
-                'source'      => $account,
+            $destination = $account;
-                'destination' => $reconciliation,
+            if (bccomp($difference, '0') === 1) {
-                'category'    => '',
+                // amount is positive. Add it to reconciliation?
-                'budget_id'   => 0,
+                $source      = $account;
-                'amount'      => $difference,
+                $destination = $reconciliation;
-                'currency_id' => $account->getMeta('currency_id'),
+
-                'description' => trans(
+            }
-                    'firefly.reconcilliation_transaction_title',
+
-                    ['from' => $start->formatLocalized($this->monthAndDayFormat), 'to' => $end->formatLocalized($this->monthAndDayFormat)]
+            // data for journal
-                ),
+            $description = trans(
-                'date'        => $request->get('end'),
+                'firefly.reconcilliation_transaction_title',
-                'notes'       => join(',', $transactions->pluck('id')->toArray()),
+                ['from' => $start->formatLocalized($this->monthAndDayFormat), 'to' => $end->formatLocalized($this->monthAndDayFormat)]
+            );
+            $journalData = [
+                'type'            => 'Reconciliation',
+                'description'     => $description,
+                'user'            => auth()->user()->id,
+                'date'            => $data['end'],
+                'bill_id'         => null,
+                'bill_name'       => null,
+                'piggy_bank_id'   => null,
+                'piggy_bank_name' => null,
+                'tags'            => null,
+                'interest_date'   => null,
+                'transactions'    => [[
+                                          'currency_id'           => intval($account->getMeta('currency_id')),
+                                          'currency_code'         => null,
+                                          'description'           => null,
+                                          'amount'                => app('steam')->positive($difference),
+                                          'source_id'             => $source->id,
+                                          'source_name'           => null,
+                                          'destination_id'        => $destination->id,
+                                          'destination_name'      => null,
+                                          'reconciled'            => true,
+                                          'identifier'            => 0,
+                                          'foreign_currency_id'   => null,
+                                          'foreign_currency_code' => null,
+                                          'foreign_amount'        => null,
+                                          'budget_id'             => null,
+                                          'budget_name'           => null,
+                                          'category_id'           => null,
+                                          'category_name'         => null,
+                                      ],
+                ],
+                'notes'           => join(', ', $data['transactions']),
             ];
-            $journal = $repository->store($data);
+
-            // reconcile this transaction too:
+            /** @var TransactionJournalFactory $factory */
-            $transaction = $journal->transactions()->first();
+            $factory = app(TransactionJournalFactory::class); // todo move to repos
-            $repository->reconcile($transaction);
+            $factory->setUser(auth()->user());
+            $journal = $factory->create($journalData);
         }
+        Log::debug('End of routine.');
 
         Session::flash('success', trans('firefly.reconciliation_stored'));
 
@@ -339,13 +383,13 @@ class ReconcileController extends Controller
     }
 
     /**
-     * @param ReconciliationFormRequest  $request
+     * @param ReconciliationUpdateRequest $request
-     * @param AccountRepositoryInterface $repository
+     * @param JournalRepositoryInterface  $repository
-     * @param TransactionJournal         $journal
+     * @param TransactionJournal          $journal
      *
      * @return $this|\Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
      */
-    public function update(ReconciliationFormRequest $request, AccountRepositoryInterface $repository, TransactionJournal $journal)
+    public function update(ReconciliationUpdateRequest $request, JournalRepositoryInterface $repository, TransactionJournal $journal)
     {
         if (TransactionType::RECONCILIATION !== $journal->transactionType->type) {
             return redirect(route('transactions.show', [$journal->id]));
@@ -356,8 +400,53 @@ class ReconcileController extends Controller
             return redirect(route('accounts.reconcile.edit', [$journal->id]))->withInput();
         }
         // update journal using account repository. Keep it consistent.
-        $data = $request->getJournalData();
+        $submitted = $request->getJournalData();
-        $repository->updateReconciliation($journal, $data);
+
+        // amount pos neg influences the accounts:
+        $source      = $repository->getSourceAccount($journal);
+        $destination = $repository->getDestinationAccount($journal);
+        if (bccomp($submitted['amount'], '0') === 1) {
+            // amount is positive, switch accounts:
+            list($source, $destination) = [$destination, $source];
+
+        }
+        // expand data with journal data:
+        $data = [
+            'type'            => $journal->transactionType->type,
+            'description'     => $journal->description,
+            'user'            => $journal->user_id,
+            'date'            => $journal->date,
+            'bill_id'         => null,
+            'bill_name'       => null,
+            'piggy_bank_id'   => null,
+            'piggy_bank_name' => null,
+            'tags'            => $submitted['tags'],
+            'interest_date'   => null,
+            'book_date'       => null,
+            'transactions'    => [[
+                                      'currency_id'           => $journal->transaction_currency_id,
+                                      'currency_code'         => null,
+                                      'description'           => null,
+                                      'amount'                => app('steam')->positive($submitted['amount']),
+                                      'source_id'             => $source->id,
+                                      'source_name'           => null,
+                                      'destination_id'        => $destination->id,
+                                      'destination_name'      => null,
+                                      'reconciled'            => true,
+                                      'identifier'            => 0,
+                                      'foreign_currency_id'   => null,
+                                      'foreign_currency_code' => null,
+                                      'foreign_amount'        => null,
+                                      'budget_id'             => null,
+                                      'budget_name'           => null,
+                                      'category_id'           => null,
+                                      'category_name'         => $submitted['category'],
+                                  ],
+            ],
+            'notes'           => $repository->getNote($journal),
+        ];
+
+        $repository->update($journal, $data);
 
         // @codeCoverageIgnoreStart
         if (1 === intval($request->get('return_to_edit'))) {

app/Http/Requests/ReconciliationStoreRequest.php

@@ -0,0 +1,81 @@
+<?php
+/**
+ * ReconciliationStoreRequest.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\Http\Requests;
+
+use FireflyIII\Rules\ValidTransactions;
+use Log;
+
+/**
+ * Class ReconciliationStoreRequest
+ */
+class ReconciliationStoreRequest extends Request
+{
+    /**
+     * @return bool
+     */
+    public function authorize(): bool
+    {
+        // Only allow logged in users
+        return auth()->check();
+    }
+
+    /**
+     * @return array
+     */
+    public function getAll(): array
+    {
+        $transactions = $this->get('transactions');
+        if (!is_array($transactions)) {
+            $transactions = [];
+        }
+        $data = [
+            'start'         => $this->date('start'),
+            'end'           => $this->date('end'),
+            'start_balance' => $this->string('startBalance'),
+            'end_balance'   => $this->string('endBalance'),
+            'difference'    => $this->string('difference'),
+            'transactions'  => $transactions,
+            'reconcile'     => $this->string('reconcile'),
+        ];
+        Log::debug('In ReconciliationStoreRequest::getAll(). Will now return data.');
+
+        return $data;
+    }
+
+    /**
+     * @return array
+     */
+    public function rules(): array
+    {
+        return [
+            'start'        => 'required|date',
+            'end'          => 'required|date',
+            'startBalance' => 'numeric',
+            'endBalance'   => 'numeric',
+            'difference'   => 'required|numeric',
+            'transactions' => [new ValidTransactions],
+            'reconcile'    => 'required|in:create,nothing',
+        ];
+    }
+}

app/Http/Requests/ReconciliationUpdateRequest.php

@@ -1,6 +1,6 @@
 <?php
 /**
- * ReconciliationFormRequest.php
+ * ReconciliationUpdateRequest.php
  * Copyright (c) 2017 thegrumpydictator@gmail.com
  *
  * This file is part of Firefly III.
@@ -23,10 +23,9 @@ declare(strict_types=1);
 namespace FireflyIII\Http\Requests;
 
 /**
- * @codeCoverageIgnore
+ * Class ReconciliationUpdateRequest.
- * Class ReconciliationFormRequest.
  */
-class ReconciliationFormRequest extends Request
+class ReconciliationUpdateRequest extends Request
 {
     /**
      * @return bool