From 28d7e24d3080052b03afda142ba2b0c4ef1e70ed Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 14 Dec 2024 20:16:08 +0100 Subject: [PATCH] Fix piggy bank orders --- app/Factory/PiggyBankFactory.php | 79 +++++++++++-------- app/Handlers/Observer/AccountObserver.php | 4 +- app/Http/Controllers/DebugController.php | 7 +- app/Http/Controllers/HomeController.php | 1 + app/Models/PiggyBank.php | 6 +- .../PiggyBank/ModifiesPiggyBanks.php | 21 +++-- .../PiggyBank/PiggyBankRepository.php | 1 - app/User.php | 1 + 8 files changed, 73 insertions(+), 47 deletions(-) diff --git a/app/Factory/PiggyBankFactory.php b/app/Factory/PiggyBankFactory.php index 4f58734d05..ac40191563 100644 --- a/app/Factory/PiggyBankFactory.php +++ b/app/Factory/PiggyBankFactory.php @@ -39,7 +39,8 @@ use Illuminate\Database\QueryException; class PiggyBankFactory { use CreatesObjectGroups; - public User $user { + + public User $user { set(User $value) { $this->user = $value; $this->currencyRepository->setUser($value); @@ -47,14 +48,14 @@ class PiggyBankFactory $this->piggyBankRepository->setUser($value); } } - private CurrencyRepositoryInterface $currencyRepository; - private AccountRepositoryInterface $accountRepository; + private CurrencyRepositoryInterface $currencyRepository; + private AccountRepositoryInterface $accountRepository; private PiggyBankRepositoryInterface $piggyBankRepository; public function __construct() { - $this->currencyRepository = app(CurrencyRepositoryInterface::class); - $this->accountRepository = app(AccountRepositoryInterface::class); + $this->currencyRepository = app(CurrencyRepositoryInterface::class); + $this->accountRepository = app(AccountRepositoryInterface::class); $this->piggyBankRepository = app(PiggyBankRepositoryInterface::class); } @@ -65,23 +66,24 @@ class PiggyBankFactory * * @return PiggyBank */ - public function store(array $data): PiggyBank { + public function store(array $data): PiggyBank + { - $piggyBankData =$data; + $piggyBankData = $data; // unset some fields - unset($piggyBankData['object_group_title'],$piggyBankData['transaction_currency_code'],$piggyBankData['transaction_currency_id'],$piggyBankData['accounts'], $piggyBankData['object_group_id'], $piggyBankData['notes']); + unset($piggyBankData['object_group_title'], $piggyBankData['transaction_currency_code'], $piggyBankData['transaction_currency_id'], $piggyBankData['accounts'], $piggyBankData['object_group_id'], $piggyBankData['notes']); // validate amount: - if (array_key_exists('target_amount', $piggyBankData) && '' === (string)$piggyBankData['target_amount']) { + if (array_key_exists('target_amount', $piggyBankData) && '' === (string) $piggyBankData['target_amount']) { $piggyBankData['target_amount'] = '0'; } - $piggyBankData['start_date_tz'] = $piggyBankData['start_date']?->format('e'); - $piggyBankData['target_date_tz'] = $piggyBankData['target_date']?->format('e'); - $piggyBankData['account_id'] = null; + $piggyBankData['start_date_tz'] = $piggyBankData['start_date']?->format('e'); + $piggyBankData['target_date_tz'] = $piggyBankData['target_date']?->format('e'); + $piggyBankData['account_id'] = null; $piggyBankData['transaction_currency_id'] = $this->getCurrency($data)->id; - $piggyBankData['order'] = 131337; + $piggyBankData['order'] = 131337; try { /** @var PiggyBank $piggyBank */ @@ -95,7 +97,7 @@ class PiggyBankFactory $this->linkToAccountIds($piggyBank, $data['accounts']); $this->piggyBankRepository->updateNote($piggyBank, $data['notes']); - $objectGroupTitle = $data['object_group_title'] ?? ''; + $objectGroupTitle = $data['object_group_title'] ?? ''; if ('' !== $objectGroupTitle) { $objectGroup = $this->findOrCreateObjectGroup($objectGroupTitle); if (null !== $objectGroup) { @@ -104,7 +106,7 @@ class PiggyBankFactory } } // try also with ID - $objectGroupId = (int)($data['object_group_id'] ?? 0); + $objectGroupId = (int) ($data['object_group_id'] ?? 0); if (0 !== $objectGroupId) { $objectGroup = $this->findObjectGroupById($objectGroupId); if (null !== $objectGroup) { @@ -118,15 +120,19 @@ class PiggyBankFactory public function find(?int $piggyBankId, ?string $piggyBankName): ?PiggyBank { - $piggyBankId = (int)$piggyBankId; - $piggyBankName = (string)$piggyBankName; + $piggyBankId = (int) $piggyBankId; + $piggyBankName = (string) $piggyBankName; if ('' === $piggyBankName && 0 === $piggyBankId) { return null; } // first find by ID: if ($piggyBankId > 0) { - /** @var null|PiggyBank $piggyBank */ - $piggyBank = $this->user->piggyBanks()->find($piggyBankId); + $piggyBank = PiggyBank + ::leftJoin('account_piggy_bank', 'account_piggy_bank.piggy_bank_id', '=', 'piggy_banks.id') + ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') + ->where('accounts.user_id', $this->user->id) + ->where('piggy_banks.id', $piggyBankId) + ->first(['piggy_banks.*']); if (null !== $piggyBank) { return $piggyBank; } @@ -146,26 +152,33 @@ class PiggyBankFactory public function findByName(string $name): ?PiggyBank { - return $this->user->piggyBanks()->where('piggy_banks.name', $name)->first(); + return PiggyBank + ::leftJoin('account_piggy_bank', 'account_piggy_bank.piggy_bank_id', '=', 'piggy_banks.id') + ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') + ->where('accounts.user_id', $this->user->id) + ->where('piggy_banks.name', $name) + ->first(['piggy_banks.*']); } - private function getCurrency(array $data): TransactionCurrency { + private function getCurrency(array $data): TransactionCurrency + { // currency: $defaultCurrency = app('amount')->getDefaultCurrency(); $currency = null; if (array_key_exists('transaction_currency_code', $data)) { - $currency = $this->currencyRepository->findByCode((string)($data['transaction_currency_code'] ?? '')); + $currency = $this->currencyRepository->findByCode((string) ($data['transaction_currency_code'] ?? '')); } if (array_key_exists('transaction_currency_id', $data)) { - $currency = $this->currencyRepository->find((int)($data['transaction_currency_id'] ?? 0)); + $currency = $this->currencyRepository->find((int) ($data['transaction_currency_id'] ?? 0)); } $currency ??= $defaultCurrency; return $currency; } - private function setOrder(PiggyBank $piggyBank, array $data): PiggyBank { + private function setOrder(PiggyBank $piggyBank, array $data): PiggyBank + { $this->resetOrder(); - $order = $this->getMaxOrder() + 1; + $order = $this->getMaxOrder() + 1; if (array_key_exists('order', $data)) { $order = $data['order']; } @@ -178,13 +191,12 @@ class PiggyBankFactory public function resetOrder(): void { // TODO duplicate code - $set = PiggyBank + $set = PiggyBank ::leftJoin('account_piggy_bank', 'account_piggy_bank.piggy_bank_id', '=', 'piggy_banks.id') ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') ->where('accounts.user_id', $this->user->id) ->with( [ - 'account', 'objectGroups', ] ) @@ -207,19 +219,20 @@ class PiggyBankFactory } - public function linkToAccountIds(PiggyBank $piggyBank, array $accounts): void { + public function linkToAccountIds(PiggyBank $piggyBank, array $accounts): void + { $toBeLinked = []; /** @var array $info */ - foreach($accounts as $info) { - $account = $this->accountRepository->find((int)($info['account_id'] ?? 0)); - if(null === $account) { + foreach ($accounts as $info) { + $account = $this->accountRepository->find((int) ($info['account_id'] ?? 0)); + if (null === $account) { continue; } - if(array_key_exists('current_amount',$info)) { + if (array_key_exists('current_amount', $info)) { $toBeLinked[$account->id] = ['current_amount' => $info['current_amount']]; //$piggyBank->accounts()->syncWithoutDetaching([$account->id => ['current_amount' => $info['current_amount'] ?? '0']]); } - if(!array_key_exists('current_amount', $info)) { + if (!array_key_exists('current_amount', $info)) { $toBeLinked[$account->id] = []; //$piggyBank->accounts()->syncWithoutDetaching([$account->id]); } diff --git a/app/Handlers/Observer/AccountObserver.php b/app/Handlers/Observer/AccountObserver.php index d93fd9d427..f140aaa84c 100644 --- a/app/Handlers/Observer/AccountObserver.php +++ b/app/Handlers/Observer/AccountObserver.php @@ -25,6 +25,7 @@ declare(strict_types=1); namespace FireflyIII\Handlers\Observer; use FireflyIII\Models\Account; +use FireflyIII\Models\PiggyBank; /** * Class AccountObserver @@ -38,8 +39,9 @@ class AccountObserver { app('log')->debug('Observe "deleting" of an account.'); $account->accountMeta()->delete(); + /** @var PiggyBank $piggy */ foreach ($account->piggyBanks()->get() as $piggy) { - $piggy->delete(); + $piggy->accounts()->detach($account); } foreach ($account->attachments()->get() as $attachment) { $attachment->delete(); diff --git a/app/Http/Controllers/DebugController.php b/app/Http/Controllers/DebugController.php index 350136fac7..5f2f4338c1 100644 --- a/app/Http/Controllers/DebugController.php +++ b/app/Http/Controllers/DebugController.php @@ -29,7 +29,9 @@ use Exception; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Http\Middleware\IsDemoUser; use FireflyIII\Models\AccountType; +use FireflyIII\Models\PiggyBank; use FireflyIII\Models\TransactionType; +use FireflyIII\Repositories\PiggyBank\PiggyBankRepositoryInterface; use FireflyIII\Support\Http\Controllers\GetConfigurationData; use FireflyIII\Support\Models\AccountBalanceCalculator; use FireflyIII\User; @@ -291,7 +293,10 @@ class DebugController extends Controller } // has piggies - if ($user->piggyBanks()->count() > 0) { + $repository = app(PiggyBankRepositoryInterface::class); + $repository->setUser($user); + + if ($repository->getPiggyBanks()->count() > 0) { $flags[] = ':pig:'; } diff --git a/app/Http/Controllers/HomeController.php b/app/Http/Controllers/HomeController.php index 72fc6ba722..d491b74ecf 100644 --- a/app/Http/Controllers/HomeController.php +++ b/app/Http/Controllers/HomeController.php @@ -120,6 +120,7 @@ class HomeController extends Controller */ public function index(AccountRepositoryInterface $repository): mixed { + $types = config('firefly.accountTypesByIdentifier.asset'); $count = $repository->count($types); Log::channel('audit')->info('User visits homepage.'); diff --git a/app/Models/PiggyBank.php b/app/Models/PiggyBank.php index b92e104bb8..a9554aa03a 100644 --- a/app/Models/PiggyBank.php +++ b/app/Models/PiggyBank.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Models; +use FireflyIII\Exceptions\FireflyException; use FireflyIII\Support\Models\ReturnsIntegerIdTrait; use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Model; @@ -55,7 +56,7 @@ class PiggyBank extends Model 'target_amount' => 'string', ]; - protected $fillable = ['name', 'account_id', 'order', 'target_amount', 'start_date', 'start_date_tz', 'target_date', 'target_date_tz', 'active', 'transaction_currency_id']; + protected $fillable = ['name', 'order', 'target_amount', 'start_date', 'start_date_tz', 'target_date', 'target_date_tz', 'active', 'transaction_currency_id']; /** * Route binder. Converts the key in the URL to the specified object (or throw 404). @@ -83,10 +84,9 @@ class PiggyBank extends Model { return $this->belongsTo(TransactionCurrency::class); } - public function account(): BelongsTo { - return $this->belongsTo(Account::class); + throw new FireflyException('This method is not available on PiggyBank.'); } public function attachments(): MorphMany diff --git a/app/Repositories/PiggyBank/ModifiesPiggyBanks.php b/app/Repositories/PiggyBank/ModifiesPiggyBanks.php index b7c53c9ea2..8fb38370c2 100644 --- a/app/Repositories/PiggyBank/ModifiesPiggyBanks.php +++ b/app/Repositories/PiggyBank/ModifiesPiggyBanks.php @@ -204,21 +204,26 @@ trait ModifiesPiggyBanks $oldOrder = $piggyBank->order; // app('log')->debug(sprintf('Will move piggy bank #%d ("%s") from %d to %d', $piggyBank->id, $piggyBank->name, $oldOrder, $newOrder)); if ($newOrder > $oldOrder) { - $this->user->piggyBanks()->where('piggy_banks.order', '<=', $newOrder)->where('piggy_banks.order', '>', $oldOrder) - ->where('piggy_banks.id', '!=', $piggyBank->id) - ->decrement('piggy_banks.order') - ; + PiggyBank::leftJoin('account_piggy_bank', 'account_piggy_bank.piggy_bank_id', '=', 'piggy_banks.id') + ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') + ->where('accounts.user_id', $this->user->id) + ->where('piggy_banks.order', '<=', $newOrder)->where('piggy_banks.order', '>', $oldOrder) + ->where('piggy_banks.id', '!=', $piggyBank->id) + ->distinct()->decrement('piggy_banks.order'); + $piggyBank->order = $newOrder; app('log')->debug(sprintf('[1] Order of piggy #%d ("%s") from %d to %d', $piggyBank->id, $piggyBank->name, $oldOrder, $newOrder)); $piggyBank->save(); return true; } - - $this->user->piggyBanks()->where('piggy_banks.order', '>=', $newOrder)->where('piggy_banks.order', '<', $oldOrder) + PiggyBank::leftJoin('account_piggy_bank', 'account_piggy_bank.piggy_bank_id', '=', 'piggy_banks.id') + ->leftJoin('accounts', 'accounts.id', '=', 'account_piggy_bank.account_id') + ->where('accounts.user_id', $this->user->id) + ->where('piggy_banks.order', '>=', $newOrder)->where('piggy_banks.order', '<', $oldOrder) ->where('piggy_banks.id', '!=', $piggyBank->id) - ->increment('piggy_banks.order') - ; + ->distinct()->increment('piggy_banks.order'); + $piggyBank->order = $newOrder; app('log')->debug(sprintf('[2] Order of piggy #%d ("%s") from %d to %d', $piggyBank->id, $piggyBank->name, $oldOrder, $newOrder)); $piggyBank->save(); diff --git a/app/Repositories/PiggyBank/PiggyBankRepository.php b/app/Repositories/PiggyBank/PiggyBankRepository.php index 7d7c8c3e2b..62687ed4aa 100644 --- a/app/Repositories/PiggyBank/PiggyBankRepository.php +++ b/app/Repositories/PiggyBank/PiggyBankRepository.php @@ -289,7 +289,6 @@ class PiggyBankRepository implements PiggyBankRepositoryInterface ->where('accounts.user_id', auth()->user()->id) ->with( [ - 'account', 'objectGroups', ] ) diff --git a/app/User.php b/app/User.php index c32c6d2ad6..9cf7a1b958 100644 --- a/app/User.php +++ b/app/User.php @@ -36,6 +36,7 @@ use FireflyIII\Models\Category; use FireflyIII\Models\CurrencyExchangeRate; use FireflyIII\Models\GroupMembership; use FireflyIII\Models\ObjectGroup; +use FireflyIII\Models\PiggyBank; use FireflyIII\Models\Preference; use FireflyIII\Models\Recurrence; use FireflyIII\Models\Role;