Improve split controller code.

app/Http/Controllers/Transaction/SplitController.php

@@ -29,7 +29,6 @@ use FireflyIII\Http\Controllers\Controller;
 use FireflyIII\Http\Requests\SplitJournalFormRequest;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
-use FireflyIII\Models\Note;
 use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Repositories\Account\AccountRepositoryInterface;
 use FireflyIII\Repositories\Budget\BudgetRepositoryInterface;
@@ -37,7 +36,6 @@ use FireflyIII\Repositories\Currency\CurrencyRepositoryInterface;
 use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
 use FireflyIII\Repositories\Journal\JournalTaskerInterface;
 use Illuminate\Http\Request;
-use Log;
 use Preferences;
 use Session;
 use Steam;
@@ -125,18 +123,8 @@ class SplitController extends Controller
         Session::forget('transactions.edit-split.fromUpdate');
 
         return view(
-            'transactions.split.edit',
+            'transactions.split.edit', compact(
-            compact(
+            'subTitleIcon', 'currencies', 'optionalFields', 'preFilled', 'subTitle', 'uploadSize', 'assetAccounts', 'budgets', 'journal', 'accountArray',
-                'subTitleIcon',
-                'currencies',
-                'optionalFields',
-                'preFilled',
-                'subTitle',
-                'uploadSize',
-                'assetAccounts',
-                'budgets',
-                'journal',
-                'accountArray',
             'previous'
         )
         );
@@ -153,8 +141,12 @@ class SplitController extends Controller
         if ($this->isOpeningBalance($journal)) {
             return $this->redirectToAccount($journal);
         }
-        $data    = $this->arrayFromInput($request);
+        $data    = $request->getAll();
-        $journal = $this->repository->updateSplitJournal($journal, $data);
+        $journal = $this->repository->update($journal, $data);
+        var_dump($request->all());
+        var_dump($data);
+        exit;
+
         /** @var array $files */
         $files = $request->hasFile('attachments') ? $request->file('attachments') : null;
         // save attachments:
@@ -168,7 +160,7 @@ class SplitController extends Controller
         }
         // @codeCoverageIgnoreEnd
 
-        $type = strtolower($journal->transactionTypeStr());
+        $type = strtolower($this->repository->getTransactionType($journal));
         Session::flash('success', strval(trans('firefly.updated_' . $type, ['description' => $data['journal_description']])));
         Preferences::mark();
 
@@ -185,39 +177,6 @@ class SplitController extends Controller
         return redirect($this->getPreviousUri('transactions.edit-split.uri'));
     }
 
-    /**
-     * @param SplitJournalFormRequest $request
-     *
-     * @return array
-     */
-    private function arrayFromInput(SplitJournalFormRequest $request): array
-    {
-        $tags  = null === $request->get('tags') ? '' : $request->get('tags');
-        $array = [
-            'journal_description'            => $request->get('journal_description'),
-            'journal_source_account_id'      => $request->get('journal_source_account_id'),
-            'journal_source_account_name'    => $request->get('journal_source_account_name'),
-            'journal_destination_account_id' => $request->get('journal_destination_account_id'),
-            'what'                           => $request->get('what'),
-            'date'                           => $request->get('date'),
-            // all custom fields:
-            'interest_date'                  => $request->get('interest_date'),
-            'book_date'                      => $request->get('book_date'),
-            'process_date'                   => $request->get('process_date'),
-            'due_date'                       => $request->get('due_date'),
-            'payment_date'                   => $request->get('payment_date'),
-            'invoice_date'                   => $request->get('invoice_date'),
-            'internal_reference'             => $request->get('internal_reference'),
-            'notes'                          => $request->get('notes'),
-            'tags'                           => explode(',', $tags),
-
-            // transactions.
-            'transactions'                   => $this->getTransactionDataFromRequest($request),
-        ];
-
-        return $array;
-    }
-
     /**
      * @param SplitJournalFormRequest|Request $request
      * @param TransactionJournal              $journal
@@ -226,15 +185,8 @@ class SplitController extends Controller
      */
     private function arrayFromJournal(Request $request, TransactionJournal $journal): array
     {
-        $sourceAccounts      = $journal->sourceAccountList();
+        $sourceAccounts      = $this->repository->getJournalSourceAccounts($journal);
-        $destinationAccounts = $journal->destinationAccountList();
+        $destinationAccounts = $this->repository->getJournalDestinationAccounts($journal);
-        $notes               = '';
-        /** @var Note $note */
-
-        $note = $this->repository->getNote($journal);
-        if (null !== $note) {
-            $notes = $note->text;
-        }
         $array               = [
             'journal_description'            => $request->old('journal_description', $journal->description),
             'journal_amount'                 => $journal->amountPositive(),
@@ -243,19 +195,19 @@ class SplitController extends Controller
             'journal_source_account_name'    => $request->old('journal_source_account_name', $sourceAccounts->first()->name),
             'journal_destination_account_id' => $request->old('journal_destination_account_id', $destinationAccounts->first()->id),
             'destinationAccounts'            => $destinationAccounts,
-            'what'                           => strtolower($journal->transactionTypeStr()),
+            'what'                           => strtolower($this->repository->getTransactionType($journal)),
-            'date'                           => $request->old('date', $journal->date->format('Y-m-d')),
+            'date'                           => $request->old('date', $this->repository->getJournalDate($journal, null)),
             'tags'                           => join(',', $journal->tags->pluck('tag')->toArray()),
 
             // all custom fields:
-            'interest_date'                  => $request->old('interest_date', $journal->getMeta('interest_date')),
+            'interest_date'                  => $request->old('interest_date', $this->repository->getMetaField($journal, 'interest_date')),
-            'book_date'                      => $request->old('book_date', $journal->getMeta('book_date')),
+            'book_date'                      => $request->old('book_date', $this->repository->getMetaField($journal, 'book_date')),
-            'process_date'                   => $request->old('process_date', $journal->getMeta('process_date')),
+            'process_date'                   => $request->old('process_date', $this->repository->getMetaField($journal, 'process_date')),
-            'due_date'                       => $request->old('due_date', $journal->getMeta('due_date')),
+            'due_date'                       => $request->old('due_date', $this->repository->getMetaField($journal, 'due_date')),
-            'payment_date'                   => $request->old('payment_date', $journal->getMeta('payment_date')),
+            'payment_date'                   => $request->old('payment_date', $this->repository->getMetaField($journal, 'payment_date')),
-            'invoice_date'                   => $request->old('invoice_date', $journal->getMeta('invoice_date')),
+            'invoice_date'                   => $request->old('invoice_date', $this->repository->getMetaField($journal, 'invoice_date')),
-            'internal_reference'             => $request->old('internal_reference', $journal->getMeta('internal_reference')),
+            'internal_reference'             => $request->old('internal_reference', $this->repository->getMetaField($journal, 'internal_reference')),
-            'notes'                          => $request->old('notes', $notes),
+            'notes'                          => $request->old('notes', $this->repository->getNoteText($journal)),
 
             // transactions.
             'transactions'                   => $this->getTransactionDataFromJournal($journal),
@@ -295,12 +247,14 @@ class SplitController extends Controller
                 'foreign_currency_code'       => $transaction['foreign_currency_code'],
                 'foreign_currency_symbol'     => $transaction['foreign_currency_symbol'],
             ];
-
             // set initial category and/or budget:
-            if (1 === count($transactions) && 0 === $index) {
+            if ($set['budget_id'] === 0) {
-                $set['budget_id'] = $journal->budgetId();
+                $set['budget_id'] = $this->repository->getJournalBudgetId($journal);
-                $set['category']  = $journal->categoryAsString();
             }
+            if (strlen($set['category']) === 0) {
+                $set['category'] = $this->repository->getJournalCategoryName($journal);
+            }
+
 
             $return[] = $set;
         }
@@ -308,35 +262,6 @@ class SplitController extends Controller
         return $return;
     }
 
-    /**
-     * @param SplitJournalFormRequest|Request $request
-     *
-     * @return array
-     */
-    private function getTransactionDataFromRequest(SplitJournalFormRequest $request): array
-    {
-        $return       = [];
-        $transactions = $request->get('transactions');
-        foreach ($transactions as $transaction) {
-            $return[] = [
-                'description'              => $transaction['description'],
-                'source_account_id'        => $transaction['source_account_id'] ?? 0,
-                'source_account_name'      => $transaction['source_account_name'] ?? '',
-                'destination_account_id'   => $transaction['destination_account_id'] ?? 0,
-                'destination_account_name' => $transaction['destination_account_name'] ?? '',
-                'amount'                   => round($transaction['amount'] ?? 0, 12),
-                'foreign_amount'           => !isset($transaction['foreign_amount']) ? null : round($transaction['foreign_amount'] ?? 0, 12),
-                'budget_id'                => isset($transaction['budget_id']) ? intval($transaction['budget_id']) : 0,
-                'category'                 => $transaction['category'] ?? '',
-                'transaction_currency_id'  => intval($transaction['transaction_currency_id']),
-                'foreign_currency_id'      => $transaction['foreign_currency_id'] ?? null,
-            ];
-        }
-        Log::debug(sprintf('Found %d splits in request data.', count($return)));
-
-        return $return;
-    }
-
     /**
      * @param $array
      * @param $old

app/Http/Requests/SplitJournalFormRequest.php

@@ -41,23 +41,64 @@ class SplitJournalFormRequest extends Request
     /**
      * @return array
      */
-    public function getSplitData(): array
+    public function getAll(): array
     {
         $data = [
-            'id'                               => $this->integer('id'),
+            'description'     => $this->string('description'),
-            'journal_description'              => $this->string('journal_description'),
+            'type'            => $this->string('what'),
-            'journal_currency_id'              => $this->integer('journal_currency_id'),
-            'journal_source_account_id'        => $this->integer('journal_source_account_id'),
-            'journal_source_account_name'      => $this->string('journal_source_account_name'),
-            'journal_destination_account_id'   => $this->integer('journal_destination_account_id'),
-            'journal_destination_account_name' => $this->string('journal_source_destination_name'),
             'date'            => $this->date('date'),
-            'what'                             => $this->string('what'),
+            'tags'            => explode(',', $this->string('tags')),
-            'interest_date'                    => $this->date('interest_date'),
+            'bill_id'         => null,
-            'book_date'                        => $this->date('book_date'),
+            'bill_name'       => null,
-            'process_date'                     => $this->date('process_date'),
+            'piggy_bank_id'   => null,
-            'transactions'                     => $this->getTransactionData(),
+            'piggy_bank_name' => null,
+            'notes'           => $this->string('notes'),
+            'transactions'    => [],
         ];
+        // switch type to get correct source / destination info:
+        $sourceId        = null;
+        $sourceName      = null;
+        $destinationId   = null;
+        $destinationName = null;
+
+        foreach ($this->get('transactions') as $index => $transaction) {
+            switch ($data['type']) {
+                case 'withdrawal':
+                    $sourceId        = $this->integer('journal_source_account_id');
+                    $destinationName = $transaction['destination_account_name'];
+                    break;
+                case 'deposit':
+                    $sourceName    = $transaction['source_account_name'];
+                    $destinationId = $this->integer('journal_destination_account_id');
+                    break;
+                case 'transfer':
+                    $sourceId      = $this->integer('journal_source_account_id');
+                    $destinationId = $this->integer('journal_destination_account_id');
+                    break;
+            }
+            $foreignAmount          = $transaction['foreign_amount'] ?? null;
+            $foreignCurrency        = isset($transaction['foreign_currency_id']) ? intval($transaction['foreign_currency_id']) : null;
+            $set                    = [
+                'source_id'             => $sourceId,
+                'source_name'           => $sourceName,
+                'destination_id'        => $destinationId,
+                'destination_name'      => $destinationName,
+                'foreign_amount'        => $foreignAmount,
+                'foreign_currency_id'   => $foreignCurrency,
+                'foreign_currency_code' => null,
+                'reconciled'            => false,
+                'identifier'            => $index,
+                'currency_id'           => $this->integer('journal_currency_id'),
+                'currency_code'         => null,
+                'description'           => $transaction['description'],
+                'amount'                => $transaction['amount'],
+                'budget_id'             => intval($transaction['budget_id'] ?? 0),
+                'budget_name'           => null,
+                'category_id'           => null,
+                'category_name'         => $transaction['category'],
+            ];
+            $data['transactions'][] = $set;
+        }
 
         return $data;
     }