From e3161a8b9c2c68b2c0632f081ef62df1e95a63a5 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 7 Mar 2021 16:19:14 +0100 Subject: [PATCH] Can now update order of accounts efficiently. --- .../Models/Account/ShowController.php | 1 + .../Requests/Models/Account/UpdateRequest.php | 4 +- .../Account/AccountRepository.php | 89 ++++++++++++++---- .../Account/AccountRepositoryInterface.php | 23 +++-- .../Internal/Update/AccountUpdateService.php | 92 ++++++++++++++++++- 5 files changed, 179 insertions(+), 30 deletions(-) diff --git a/app/Api/V1/Controllers/Models/Account/ShowController.php b/app/Api/V1/Controllers/Models/Account/ShowController.php index aa31be9ee5..68a7353789 100644 --- a/app/Api/V1/Controllers/Models/Account/ShowController.php +++ b/app/Api/V1/Controllers/Models/Account/ShowController.php @@ -82,6 +82,7 @@ class ShowController extends Controller $pageSize = (int)app('preferences')->getForUser(auth()->user(), 'listPageSize', 50)->data; // get list of accounts. Count it and split it. + $this->repository->sortAccounts(); $collection = $this->repository->getAccountsByType($types); $count = $collection->count(); $accounts = $collection->slice(($this->parameters->get('page') - 1) * $pageSize, $pageSize); diff --git a/app/Api/V1/Requests/Models/Account/UpdateRequest.php b/app/Api/V1/Requests/Models/Account/UpdateRequest.php index 14754093dc..4ccb8cd887 100644 --- a/app/Api/V1/Requests/Models/Account/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Account/UpdateRequest.php @@ -62,7 +62,6 @@ class UpdateRequest extends FormRequest 'account_type' => $this->nullableString('type'), 'account_type_id' => null, 'currency_id' => $this->nullableInteger('currency_id'), - 'order' => $this->integer('order'), 'currency_code' => $this->nullableString('currency_code'), 'virtual_balance' => $this->nullableString('virtual_balance'), 'iban' => $this->nullableString('iban'), @@ -77,6 +76,9 @@ class UpdateRequest extends FormRequest 'interest' => $this->nullableString('interest'), 'interest_period' => $this->nullableString('interest_period'), ]; + if(null !== $this->get('order')) { + $data['order'] = $this->integer('order'); + } $data = $this->appendLocationData($data, null); diff --git a/app/Repositories/Account/AccountRepository.php b/app/Repositories/Account/AccountRepository.php index 666315aa6d..d7be2d54a0 100644 --- a/app/Repositories/Account/AccountRepository.php +++ b/app/Repositories/Account/AccountRepository.php @@ -37,8 +37,8 @@ use FireflyIII\Models\TransactionType; use FireflyIII\Services\Internal\Destroy\AccountDestroyService; use FireflyIII\Services\Internal\Update\AccountUpdateService; use FireflyIII\User; +use Illuminate\Database\Eloquent\Builder as EloquentBuilder; use Illuminate\Database\Eloquent\Relations\HasMany; -use \Illuminate\Database\Eloquent\Builder as EloquentBuilder; use Illuminate\Support\Collection; use Log; use Storage; @@ -193,7 +193,7 @@ class AccountRepository implements AccountRepositoryInterface */ public function getAccountCurrency(Account $account): ?TransactionCurrency { - $currencyId = (int) $this->getMetaValue($account, 'currency_id'); + $currencyId = (int)$this->getMetaValue($account, 'currency_id'); if ($currencyId > 0) { return TransactionCurrency::find($currencyId); } @@ -303,15 +303,18 @@ class AccountRepository implements AccountRepositoryInterface */ public function getMetaValue(Account $account, string $field): ?string { - $result = $account->accountMeta->filter(function (AccountMeta $meta) use ($field) { - return strtolower($meta->name) === strtolower($field); - }); + $result = $account->accountMeta->filter( + function (AccountMeta $meta) use ($field) { + return strtolower($meta->name) === strtolower($field); + } + ); if (0 === $result->count()) { return null; } if (1 === $result->count()) { - return (string) $result->first()->data; + return (string)$result->first()->data; } + return null; } @@ -369,7 +372,7 @@ class AccountRepository implements AccountRepositoryInterface return null; } - return (string) $transaction->amount; + return (string)$transaction->amount; } /** @@ -487,7 +490,7 @@ class AccountRepository implements AccountRepositoryInterface ->orderBy('transaction_journals.id', 'ASC') ->first(['transaction_journals.id']); if (null !== $first) { - return TransactionJournal::find((int) $first->id); + return TransactionJournal::find((int)$first->id); } return null; @@ -577,6 +580,7 @@ class AccountRepository implements AccountRepositoryInterface { /** @var AccountUpdateService $service */ $service = app(AccountUpdateService::class); + return $service->update($account, $data); } @@ -641,8 +645,8 @@ class AccountRepository implements AccountRepositoryInterface $info = $account->transactions()->get(['transaction_currency_id', 'foreign_currency_id'])->toArray(); $currencyIds = []; foreach ($info as $entry) { - $currencyIds[] = (int) $entry['transaction_currency_id']; - $currencyIds[] = (int) $entry['foreign_currency_id']; + $currencyIds[] = (int)$entry['transaction_currency_id']; + $currencyIds[] = (int)$entry['foreign_currency_id']; } $currencyIds = array_unique($currencyIds); @@ -682,13 +686,17 @@ class AccountRepository implements AccountRepositoryInterface $parts = explode(' ', $query); foreach ($parts as $part) { $search = sprintf('%%%s%%', $part); - $dbQuery->where(function (EloquentBuilder $q1) use ($search) { - $q1->where('accounts.iban', 'LIKE', $search); - $q1->orWhere(function (EloquentBuilder $q2) use ($search) { - $q2->where('account_meta.name', '=', 'account_number'); - $q2->where('account_meta.data', 'LIKE', $search); - }); - }); + $dbQuery->where( + function (EloquentBuilder $q1) use ($search) { + $q1->where('accounts.iban', 'LIKE', $search); + $q1->orWhere( + function (EloquentBuilder $q2) use ($search) { + $q2->where('account_meta.name', '=', 'account_number'); + $q2->where('account_meta.data', 'LIKE', $search); + } + ); + } + ); } } if (!empty($types)) { @@ -698,4 +706,51 @@ class AccountRepository implements AccountRepositoryInterface return $dbQuery->take($limit)->get(['accounts.*']); } + + /** + * @inheritDoc + */ + public function sortAccounts(): void + { + // sort assets + $list = $this->user->accounts() + ->leftJoin('account_types', 'accounts.account_type_id', 'account_types.id') + ->where('account_types.type', AccountType::ASSET) + ->orderBy('accounts.order', 'ASC') + ->orderBy('accounts.name', 'ASC') + ->orderBy('accounts.created_at', 'ASC')->get(['accounts.id', 'accounts.order']); + $index = 1; + /** @var Account $account */ + foreach ($list as $account) { + if ($account->order !== $index) { + $account->order = $index; + $account->save(); + } + $index++; + } + + // sort liabilities + $list = $this->user->accounts() + ->leftJoin('account_types', 'accounts.account_type_id', 'account_types.id') + ->whereIn('account_types.type', [AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]) + ->orderBy('accounts.order', 'ASC') + ->orderBy('accounts.name', 'ASC') + ->orderBy('accounts.created_at', 'ASC')->get(['accounts.id', 'accounts.order']); + $index = 1; + /** @var Account $account */ + foreach ($list as $account) { + if ($account->order !== $index) { + $account->order = $index; + $account->save(); + } + $index++; + } + + // set the rest to zero: + $this->user->accounts() + ->leftJoin('account_types', 'accounts.account_type_id', 'account_types.id') + ->whereNotIn('account_types.type', [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]) + ->update(['order' => '0']); + + } } diff --git a/app/Repositories/Account/AccountRepositoryInterface.php b/app/Repositories/Account/AccountRepositoryInterface.php index 0befebe3de..38699eb1d6 100644 --- a/app/Repositories/Account/AccountRepositoryInterface.php +++ b/app/Repositories/Account/AccountRepositoryInterface.php @@ -61,6 +61,11 @@ interface AccountRepositoryInterface */ public function getUsedCurrencies(Account $account): Collection; + /** + * Sort accounts (and fix the sort if necessary). + */ + public function sortAccounts(): void; + /** * @param Account $account * @@ -80,7 +85,7 @@ interface AccountRepositoryInterface /** * Moved here from account CRUD. * - * @param Account $account + * @param Account $account * @param Account|null $moveTo * * @return bool @@ -98,7 +103,7 @@ interface AccountRepositoryInterface /** * @param string $iban - * @param array $types + * @param array $types * * @return Account|null */ @@ -106,7 +111,7 @@ interface AccountRepositoryInterface /** * @param string $name - * @param array $types + * @param array $types * * @return Account|null */ @@ -172,7 +177,7 @@ interface AccountRepositoryInterface * Return meta value for account. Null if not found. * * @param Account $account - * @param string $field + * @param string $field * * @return null|string */ @@ -265,8 +270,8 @@ interface AccountRepositoryInterface /** * @param string $query - * @param array $types - * @param int $limit + * @param array $types + * @param int $limit * * @return Collection */ @@ -274,8 +279,8 @@ interface AccountRepositoryInterface /** * @param string $query - * @param array $types - * @param int $limit + * @param array $types + * @param int $limit * * @return Collection */ @@ -295,7 +300,7 @@ interface AccountRepositoryInterface /** * @param Account $account - * @param array $data + * @param array $data * * @return Account */ diff --git a/app/Services/Internal/Update/AccountUpdateService.php b/app/Services/Internal/Update/AccountUpdateService.php index 8b345a2790..033567941b 100644 --- a/app/Services/Internal/Update/AccountUpdateService.php +++ b/app/Services/Internal/Update/AccountUpdateService.php @@ -74,6 +74,7 @@ class AccountUpdateService $this->accountRepository->setUser($account->user); $this->user = $account->user; $account = $this->updateAccount($account, $data); + $account = $this->updateAccountOrder($account, $data); // find currency, or use default currency instead. if (isset($data['currency_id']) && (null !== $data['currency_id'] || null !== $data['currency_code'])) { @@ -106,7 +107,8 @@ class AccountUpdateService * @param Account $account * @param array $data */ - private function updateLocation(Account $account, array $data): void { + private function updateLocation(Account $account, array $data): void + { $updateLocation = $data['update_location'] ?? false; // location must be updated? if (true === $updateLocation) { @@ -169,7 +171,6 @@ class AccountUpdateService $account->name = $data['name'] ?? $account->name; $account->active = $data['active'] ?? $account->active; $account->iban = $data['iban'] ?? $account->iban; - $account->order = $data['order'] ?? $account->order; // if account type is a liability, the liability type (account type) // can be updated to another one. @@ -202,7 +203,7 @@ class AccountUpdateService $array = $preference->data; Log::debug('Current list of accounts: ', $array); Log::debug(sprintf('Going to remove account #%d', $removeAccountId)); - $filtered = array_filter( + $filtered = array_filter( $array, function ($accountId) use ($removeAccountId) { return (int)$accountId !== $removeAccountId; } @@ -210,9 +211,11 @@ class AccountUpdateService Log::debug('Left with accounts', array_values($filtered)); app('preferences')->setForUser($account->user, 'frontpageAccounts', array_values($filtered)); app('preferences')->forget($account->user, 'frontpageAccounts'); + return; } Log::debug("Found no frontpageAccounts preference, do nothing."); + return; } Log::debug('Account was not marked as inactive, do nothing.'); @@ -241,4 +244,87 @@ class AccountUpdateService } } } + + /** + * @param Account $account + * @param array $data + * + * @return Account + */ + private function updateAccountOrder(Account $account, array $data): Account + { + // skip if no order info + if (!array_key_exists('order', $data) || $data['order'] === $account->order) { + return $account; + } + // skip if not of orderable type. + $type = $account->accountType->type; + if (!in_array($type, [AccountType::ASSET, AccountType::MORTGAGE, AccountType::LOAN, AccountType::DEBT], true)) { + return $account; + } + // get account type ID's because a join and an update is hard: + $list = $this->getTypeIds([AccountType::MORTGAGE, AccountType::LOAN, AccountType::DEBT]); + $oldOrder = (int)$account->order; + $newOrder = $data['order']; + if (in_array($type, [AccountType::ASSET], true)) { + $list = $this->getTypeIds([AccountType::ASSET]); + } + if ($oldOrder > $newOrder) { + // say you move from 9 (old) to 3 (new) + // everything that's 3 or higher moves up one spot. + // that leaves a gap for nr 3 later on. + // 1 2 (!) 4 5 6 7 8 9 10 11 12 13 14 + $this->user->accounts() + ->whereIn('accounts.account_type_id', $list) + ->where('accounts.order', '>=', $newOrder) + ->update(['accounts.order' => \DB::raw('accounts.order + 1')]); + + // update the account and save it: + // nummer 9 (now 10!) will move to nr 3. + // a gap appears on spot 10. + // 1 2 3 4 5 6 7 8 9 11 12 13 14 + $account->order = $newOrder; + $account->save(); + + // everything over 9 (old) drops one spot + // 1 2 3 4 5 6 7 8 9 10 11 12 13 14 + $this->user->accounts() + ->whereIn('accounts.account_type_id', $list) + ->where('accounts.order', '>', $oldOrder) + ->update(['accounts.order' => \DB::raw('accounts.order - 1')]); + + return $account; + } + + if ($oldOrder < $newOrder) { + // if it goes from 3 (old) to 9 (new), + // 1 2 3 4 5 6 7 8 9 10 11 12 13 14 + // everything that is between 3 and 9 (incl) - 1 spot + // 1 2 2 3 4 5 6 7 8 10 11 12 13 14 + $this->user->accounts() + ->whereIn('accounts.account_type_id', $list) + ->where('accounts.order', '>=', $oldOrder) + ->where('accounts.order', '<=', $newOrder) + ->update(['accounts.order' => \DB::raw('accounts.order - 1')]); + // then set order to 9 + // 1 2 3 4 5 6 7 8 9 10 11 12 13 14 + $account->order = $newOrder; + $account->save(); + } + + return $account; + } + + private function getTypeIds(array $array): array + { + $return = []; + /** @var string $type */ + foreach ($array as $type) { + /** @var AccountType $type */ + $type = AccountType::whereType($type)->first(); + $return[] = (int)$type->id; + } + + return $return; + } }