Various code cleanup.

2025-02-25 18:45:27 -06:00 · 2018-03-02 16:31:02 +01:00 · 2018-03-02 16:31:02 +01:00 · 139c2284b8
commit 139c2284b8
parent 91909a70d7
11 changed files with 42 additions and 51 deletions
--- a/app/Api/V1/Controllers/TransactionController.php
+++ b/app/Api/V1/Controllers/TransactionController.php
@ -228,7 +228,6 @@ class TransactionController extends Controller

        /** @var JournalUpdateService $service */
        $service = app(JournalUpdateService::class);
-        $service->setUser(auth()->user());
        $journal = $service->update($transaction->transactionJournal, $data);

        $manager = new Manager();
--- a/app/Models/TransactionJournal.php
+++ b/app/Models/TransactionJournal.php
@ -26,6 +26,7 @@ use Carbon\Carbon;
 use Crypt;
 use FireflyIII\Support\CacheProperties;
 use FireflyIII\Support\Models\TransactionJournalTrait;
+use FireflyIII\User;
 use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsToMany;
@ -38,6 +39,7 @@ use Watson\Validating\ValidatingTrait;

 /**
 * Class TransactionJournal.
+ * @property User $user
 */
 class TransactionJournal extends Model
 {
--- a/app/Repositories/Journal/JournalRepository.php
+++ b/app/Repositories/Journal/JournalRepository.php
@ -23,8 +23,6 @@ declare(strict_types=1);
 namespace FireflyIII\Repositories\Journal;

 use Carbon\Carbon;
-use Exception;
-use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Factory\TransactionJournalFactory;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
@ -585,13 +583,7 @@ class JournalRepository implements JournalRepositoryInterface
    {
        /** @var JournalUpdateService $service */
        $service = app(JournalUpdateService::class);
-        $service->setUser($this->user);
-
-        try {
        $journal = $service->update($journal, $data);
-        } catch (FireflyException | Exception $e) {
-            throw new FireflyException($e->getMessage());
-        }

        return $journal;
    }
@ -608,7 +600,6 @@ class JournalRepository implements JournalRepositoryInterface
    {
        /** @var JournalUpdateService $service */
        $service = app(JournalUpdateService::class);
-        $service->setUser($this->user);

        return $service->updateBudget($journal, $budgetId);
    }
@ -625,7 +616,6 @@ class JournalRepository implements JournalRepositoryInterface
    {
        /** @var JournalUpdateService $service */
        $service = app(JournalUpdateService::class);
-        $service->setUser($this->user);

        return $service->updateCategory($journal, $category);
    }
@ -642,7 +632,6 @@ class JournalRepository implements JournalRepositoryInterface
    {
        /** @var JournalUpdateService $service */
        $service = app(JournalUpdateService::class);
-        $service->setUser($this->user);
        $service->connectTags($journal, $tags);

        return $journal;
--- a/app/Services/Internal/Destroy/AccountDestroyService.php
+++ b/app/Services/Internal/Destroy/AccountDestroyService.php
@ -24,8 +24,10 @@ declare(strict_types=1);
 namespace FireflyIII\Services\Internal\Destroy;

 use DB;
+use Exception;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\Transaction;
+use FireflyIII\Models\TransactionJournal;
 use Log;

 /**
@ -49,17 +51,19 @@ class AccountDestroyService
        /** @var Transaction $transaction */
        foreach ($account->transactions()->get() as $transaction) {
            Log::debug('Now at transaction #' . $transaction->id);
+            /** @var TransactionJournal $journal */
            $journal = $transaction->transactionJournal()->first();
            if (null !== $journal) {
                Log::debug('Call for deletion of journal #' . $journal->id);
-                $journal->delete();
+                /** @var JournalDestroyService $service */
+                $service = app(JournalDestroyService::class);
+                $service->destroy($journal);
            }
        }
        try {
            $account->delete();
-        } catch (\Exception $e) {
-            // don't care
-            Log::error($e->getMessage());
+        } catch (Exception $e) { // @codeCoverageIgnore
+            Log::error(sprintf('Could not delete account: %s',$e->getMessage())); // @codeCoverageIgnore
        }

        return true;
--- a/app/Services/Internal/Destroy/BillDestroyService.php
+++ b/app/Services/Internal/Destroy/BillDestroyService.php
@ -25,8 +25,9 @@ namespace FireflyIII\Services\Internal\Destroy;

 use Exception;
 use FireflyIII\Models\Bill;
-
+use Log;
 /**
+ * @codeCoverageIgnore
 * Class BillDestroyService
 */
 class BillDestroyService
@ -38,8 +39,8 @@ class BillDestroyService
    {
        try {
            $bill->delete();
-        } catch (Exception $e) {
-            // don't care.
+        } catch (Exception $e) { // @codeCoverageIgnore
+            Log::error(sprintf('Could not delete bill: %s',$e->getMessage())); // @codeCoverageIgnore
        }
    }

--- a/app/Services/Internal/Destroy/JournalDestroyService.php
+++ b/app/Services/Internal/Destroy/JournalDestroyService.php
@ -30,6 +30,7 @@ use FireflyIII\Models\TransactionJournalMeta;
 use Log;

 /**
+ * @codeCoverageIgnore
 * Class JournalDestroyService
 */
 class JournalDestroyService
@ -55,7 +56,7 @@ class JournalDestroyService
            }
            $journal->delete();
        } catch (Exception $e) {
-            // don't care
+            Log::error(sprintf('Could not delete bill: %s',$e->getMessage())); // @codeCoverageIgnore
        }

        return;
--- a/app/Services/Internal/Support/AccountServiceTrait.php
+++ b/app/Services/Internal/Support/AccountServiceTrait.php
@ -35,6 +35,7 @@ use FireflyIII\Models\Note;
 use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Models\TransactionType;
+use FireflyIII\Services\Internal\Destroy\JournalDestroyService;
 use FireflyIII\User;
 use Log;
 use Validator;
@ -66,11 +67,9 @@ trait AccountServiceTrait
        // opening balance data? update it!
        if (null !== $openingBalance) {
            Log::debug('Opening balance journal found, delete journal.');
-            try {
-                $openingBalance->delete();
-            } catch (Exception $e) {
-                Log::error(sprintf('Could not delete opening balance: %s', $e->getMessage()));
-            }
+            /** @var JournalDestroyService $service */
+            $service = app(JournalDestroyService::class);
+            $service->destroy($openingBalance);

            return true;
        }
@ -277,11 +276,9 @@ trait AccountServiceTrait
        Log::debug(sprintf('Submitted amount for opening balance to update is "%s"', $amount));
        if (0 === bccomp($amount, '0')) {
            Log::notice(sprintf('Amount "%s" is zero, delete opening balance.', $amount));
-            try {
-                $journal->delete();
-            } catch (Exception $e) {
-                Log::error(sprintf('Could not delete opening balance: %s', $e->getMessage()));
-            }
+            /** @var JournalDestroyService $service */
+            $service = app(JournalDestroyService::class);
+            $service->destroy($journal);


            return true;
--- a/app/Services/Internal/Support/JournalServiceTrait.php
+++ b/app/Services/Internal/Support/JournalServiceTrait.php
@ -47,7 +47,7 @@ trait JournalServiceTrait
    {
        /** @var BillFactory $factory */
        $factory = app(BillFactory::class);
-        $factory->setUser($this->user);
+        $factory->setUser($journal->user);
        $bill = $factory->find($data['bill_id'], $data['bill_name']);

        if (!is_null($bill)) {
--- a/app/Services/Internal/Update/AccountUpdateService.php
+++ b/app/Services/Internal/Update/AccountUpdateService.php
@ -48,8 +48,6 @@ class AccountUpdateService
     * @param array   $data
     *
     * @return Account
-     * @throws FireflyException
-     * @throws Exception
     */
    public function update(Account $account, array $data): Account
    {
--- a/app/Services/Internal/Update/BillUpdateService.php
+++ b/app/Services/Internal/Update/BillUpdateService.php
@ -27,6 +27,7 @@ use FireflyIII\Models\Bill;
 use FireflyIII\Services\Internal\Support\BillServiceTrait;

 /**
+ * @codeCoverageIgnore
 * Class BillUpdateService
 */
 class BillUpdateService
@ -36,6 +37,8 @@ class BillUpdateService
    /**
     * @param Bill  $bill
     * @param array $data
+     *
+     * @return Bill
     */
    public function update(Bill $bill, array $data): Bill
    {
--- a/app/Services/Internal/Update/JournalUpdateService.php
+++ b/app/Services/Internal/Update/JournalUpdateService.php
@ -27,8 +27,8 @@ use FireflyIII\Factory\TransactionFactory;
 use FireflyIII\Models\Transaction;
 use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Services\Internal\Support\JournalServiceTrait;
-use FireflyIII\User;
 use Illuminate\Support\Collection;
+use Log;

 /**
 * Class to centralise code that updates a journal given the input by system.
@ -38,24 +38,12 @@ use Illuminate\Support\Collection;
 class JournalUpdateService
 {
    use JournalServiceTrait;
-    /** @var User */
-    private $user;
-
-    /**
-     * @param User $user
-     */
-    public function setUser(User $user): void
-    {
-        $this->user = $user;
-    }

    /**
     * @param TransactionJournal $journal
     * @param array              $data
     *
     * @return TransactionJournal
-     * @throws \FireflyIII\Exceptions\FireflyException
-     * @throws \Exception
     */
    public function update(TransactionJournal $journal, array $data): TransactionJournal
    {
@ -67,12 +55,14 @@ class JournalUpdateService
        // update transactions:
        /** @var TransactionUpdateService $service */
        $service = app(TransactionUpdateService::class);
-        $service->setUser($this->user);
+        $service->setUser($journal->user);

        // create transactions
        /** @var TransactionFactory $factory */
        $factory = app(TransactionFactory::class);
-        $factory->setUser($this->user);
+        $factory->setUser($journal->user);
+
+        Log::debug(sprintf('Found %d rows in array (should result in %d transactions', count($data['transactions']), count($data['transactions']) * 2));

        /**
         * @var int   $identifier
@ -82,25 +72,32 @@ class JournalUpdateService
            // exists transaction(s) with this identifier? update!
            /** @var Collection $existing */
            $existing = $journal->transactions()->where('identifier', $identifier)->get();
+            Log::debug(sprintf('Found %d transactions with identifier %d', $existing->count(), $identifier));
            if ($existing->count() > 0) {
                $existing->each(
                    function (Transaction $transaction) use ($service, $trData) {
+                        Log::debug(sprintf('Update transaction #%d (identifier %d)', $transaction->id, $trData['identifier']));
                        $service->update($transaction, $trData);
                    }
                );
                continue;
            }
+            Log::debug('Found none, so create a pair.');
            // otherwise, create!
            $factory->createPair($journal, $trData);
        }
        // could be that journal has more transactions than submitted (remove split)
        $transactions = $journal->transactions()->where('amount', '>', 0)->get();
+        Log::debug(sprintf('Journal #%d has %d transactions', $journal->id, $transactions->count()));
        /** @var Transaction $transaction */
        foreach ($transactions as $transaction) {
+            Log::debug(sprintf('Now at transaction %d with identifier %d', $transaction->id, $transaction->identifier));
            if (!isset($data['transactions'][$transaction->identifier])) {
+                Log::debug('No such entry in array, delete this set of transactions.');
                $journal->transactions()->where('identifier', $transaction->identifier)->delete();
            }
        }
+        Log::debug(sprintf('New count is %d, transactions array held %d items', $journal->transactions()->count(), count($data['transactions'])));

        // connect bill:
        $this->connectBill($journal, $data);
@ -137,7 +134,7 @@ class JournalUpdateService
    {
        /** @var TransactionUpdateService $service */
        $service = app(TransactionUpdateService::class);
-        $service->setUser($this->user);
+        $service->setUser($journal->user);

        /** @var Transaction $transaction */
        foreach ($journal->transactions as $transaction) {
@ -159,7 +156,7 @@ class JournalUpdateService
    {
        /** @var TransactionUpdateService $service */
        $service = app(TransactionUpdateService::class);
-        $service->setUser($this->user);
+        $service->setUser($journal->user);

        /** @var Transaction $transaction */
        foreach ($journal->transactions as $transaction) {