From f6e28dc88f9344085e28ea3507ad10c42693c399 Mon Sep 17 00:00:00 2001 From: James Cole Date: Mon, 1 Apr 2024 19:59:21 +0200 Subject: [PATCH] Fine tune preferences to handle multi-administration options. --- .../Controllers/PreferencesController.php | 54 +++--- app/Models/Preference.php | 36 ++-- app/Support/Preferences.php | 162 ++++++------------ app/Transformers/PreferenceTransformer.php | 12 +- config/firefly.php | 44 ++--- 5 files changed, 134 insertions(+), 174 deletions(-) diff --git a/app/Http/Controllers/PreferencesController.php b/app/Http/Controllers/PreferencesController.php index e5d1974824..90e5421f57 100644 --- a/app/Http/Controllers/PreferencesController.php +++ b/app/Http/Controllers/PreferencesController.php @@ -66,14 +66,14 @@ class PreferencesController extends Controller */ public function index(AccountRepositoryInterface $repository) { - $accounts = $repository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]); - $isDocker = env('IS_DOCKER', false); - $groupedAccounts = []; + $accounts = $repository->getAccountsByType([AccountType::DEFAULT, AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]); + $isDocker = env('IS_DOCKER', false); + $groupedAccounts = []; /** @var Account $account */ foreach ($accounts as $account) { - $type = $account->accountType->type; - $role = sprintf('opt_group_%s', $repository->getMetaValue($account, 'account_role')); + $type = $account->accountType->type; + $role = sprintf('opt_group_%s', $repository->getMetaValue($account, 'account_role')); if (in_array($type, [AccountType::MORTGAGE, AccountType::DEBT, AccountType::LOAN], true)) { $role = sprintf('opt_group_l_%s', $type); @@ -94,23 +94,23 @@ class PreferencesController extends Controller if (!is_array($frontPageAccounts)) { $frontPageAccounts = $accountIds; } - $language = app('steam')->getLanguage(); - $languages = config('firefly.languages'); - $locale = app('preferences')->get('locale', config('firefly.default_locale', 'equal'))->data; - $listPageSize = app('preferences')->get('listPageSize', 50)->data; - $darkMode = app('preferences')->get('darkMode', 'browser')->data; - $slackUrl = app('preferences')->get('slack_webhook_url', '')->data; - $customFiscalYear = app('preferences')->get('customFiscalYear', 0)->data; - $fiscalYearStartStr = app('preferences')->get('fiscalYearStart', '01-01')->data; + $language = app('steam')->getLanguage(); + $languages = config('firefly.languages'); + $locale = app('preferences')->get('locale', config('firefly.default_locale', 'equal'))->data; + $listPageSize = app('preferences')->get('listPageSize', 50)->data; + $darkMode = app('preferences')->get('darkMode', 'browser')->data; + $slackUrl = app('preferences')->get('slack_webhook_url', '')->data; + $customFiscalYear = app('preferences')->get('customFiscalYear', 0)->data; + $fiscalYearStartStr = app('preferences')->get('fiscalYearStart', '01-01')->data; if (is_array($fiscalYearStartStr)) { $fiscalYearStartStr = '01-01'; } - $fiscalYearStart = sprintf('%s-%s', date('Y'), (string)$fiscalYearStartStr); - $tjOptionalFields = app('preferences')->get('transaction_journal_optional_fields', [])->data; - $availableDarkModes = config('firefly.available_dark_modes'); + $fiscalYearStart = sprintf('%s-%s', date('Y'), (string)$fiscalYearStartStr); + $tjOptionalFields = app('preferences')->get('transaction_journal_optional_fields', [])->data; + $availableDarkModes = config('firefly.available_dark_modes'); // notification preferences (single value for each): - $notifications = []; + $notifications = []; foreach (config('firefly.available_notifications') as $notification) { $notifications[$notification] = app('preferences')->get(sprintf('notification_%s', $notification), true)->data; } @@ -125,7 +125,7 @@ class PreferencesController extends Controller app('log')->error($e->getMessage()); $locales = []; } - $locales = ['equal' => (string)trans('firefly.equal_to_language')] + $locales; + $locales = ['equal' => (string)trans('firefly.equal_to_language')] + $locales; // an important fallback is that the frontPageAccount array gets refilled automatically // when it turns up empty. if (0 === count($frontPageAccounts)) { @@ -164,7 +164,7 @@ class PreferencesController extends Controller } // extract notifications: - $all = $request->all(); + $all = $request->all(); foreach (config('firefly.available_notifications') as $option) { $key = sprintf('notification_%s', $option); if (array_key_exists($key, $all)) { @@ -194,8 +194,8 @@ class PreferencesController extends Controller } // custom fiscal year - $customFiscalYear = 1 === (int)$request->get('customFiscalYear'); - $string = strtotime((string)$request->get('fiscalYearStart')); + $customFiscalYear = 1 === (int)$request->get('customFiscalYear'); + $string = strtotime((string)$request->get('fiscalYearStart')); if (false !== $string) { $fiscalYearStart = date('m-d', $string); app('preferences')->set('customFiscalYear', $customFiscalYear); @@ -204,15 +204,15 @@ class PreferencesController extends Controller // save page size: app('preferences')->set('listPageSize', 50); - $listPageSize = (int)$request->get('listPageSize'); + $listPageSize = (int)$request->get('listPageSize'); if ($listPageSize > 0 && $listPageSize < 1337) { app('preferences')->set('listPageSize', $listPageSize); } // language: /** @var Preference $currentLang */ - $currentLang = app('preferences')->get('language', 'en_US'); - $lang = $request->get('language'); + $currentLang = app('preferences')->get('language', 'en_US'); + $lang = $request->get('language'); if (array_key_exists($lang, config('firefly.languages'))) { app('preferences')->set('language', $lang); } @@ -229,8 +229,8 @@ class PreferencesController extends Controller } // optional fields for transactions: - $setOptions = $request->get('tj') ?? []; - $optionalTj = [ + $setOptions = $request->get('tj') ?? []; + $optionalTj = [ 'interest_date' => array_key_exists('interest_date', $setOptions), 'book_date' => array_key_exists('book_date', $setOptions), 'process_date' => array_key_exists('process_date', $setOptions), @@ -247,7 +247,7 @@ class PreferencesController extends Controller app('preferences')->set('transaction_journal_optional_fields', $optionalTj); // dark mode - $darkMode = $request->get('darkMode') ?? 'browser'; + $darkMode = $request->get('darkMode') ?? 'browser'; if (in_array($darkMode, config('firefly.available_dark_modes'), true)) { app('preferences')->set('darkMode', $darkMode); } diff --git a/app/Models/Preference.php b/app/Models/Preference.php index bcf0a2b640..708d6c31ef 100644 --- a/app/Models/Preference.php +++ b/app/Models/Preference.php @@ -54,7 +54,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; * @method static Builder|Preference whereUpdatedAt($value) * @method static Builder|Preference whereUserId($value) * - * @property mixed $user_group_id + * @property mixed $user_group_id * * @mixin Eloquent */ @@ -64,7 +64,7 @@ class Preference extends Model use ReturnsIntegerUserIdTrait; protected $casts - = [ + = [ 'created_at' => 'datetime', 'updated_at' => 'datetime', 'data' => 'array', @@ -81,22 +81,38 @@ class Preference extends Model { if (auth()->check()) { /** @var User $user */ - $user = auth()->user(); + $user = auth()->user(); - /** @var null|Preference $preference */ - $preference = $user->preferences()->where('name', $value)->first(); + // some preferences do not have an administration ID. + // some need it, to make sure the correct one is selected. + $userGroupId = (int)$user->user_group_id; + $userGroupId = $userGroupId === 0 ? null : $userGroupId; + /** @var Preference|null $preference */ + $preference = null; + $items = config('firefly.admin_specific_prefs'); + if (null !== $userGroupId && in_array($value, $items, true)) { + // find a preference with a specific user_group_id + $preference = $user->preferences()->where('user_group_id', $userGroupId)->where('name', $value)->first(); + } + if (!in_array($value, $items, true)) { + // find any one. + $preference = $user->preferences()->where('name', $value)->first(); + } + + // try again with ID, but this time don't care about the preferred user_group_id if (null === $preference) { $preference = $user->preferences()->where('id', (int)$value)->first(); } if (null !== $preference) { return $preference; } - $default = config('firefly.default_preferences'); + $default = config('firefly.default_preferences'); if (array_key_exists($value, $default)) { - $preference = new self(); - $preference->name = $value; - $preference->data = $default[$value]; - $preference->user_id = (int)$user->id; + $preference = new self(); + $preference->name = $value; + $preference->data = $default[$value]; + $preference->user_id = (int)$user->id; + $preference->user_group_id = in_array($value, $items, true) ? $userGroupId : null; $preference->save(); return $preference; diff --git a/app/Support/Preferences.php b/app/Support/Preferences.php index e1795e64b7..63aea0e04f 100644 --- a/app/Support/Preferences.php +++ b/app/Support/Preferences.php @@ -28,6 +28,8 @@ use FireflyIII\Models\Preference; use FireflyIII\User; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\Session; /** * Class Preferences. @@ -42,24 +44,15 @@ class Preferences } return Preference::where('user_id', $user->id) - ->where(function(Builder $q) use($user) { - $q->whereNull('user_group_id'); - $q->orWhere('user_group_id', $user->user_group_id); - }) + ->where(function (Builder $q) use ($user) { + $q->whereNull('user_group_id'); + $q->orWhere('user_group_id', $user->user_group_id); + }) ->get(); } - /** - * @param mixed $default - * - * @throws FireflyException - */ - public function get(string $name, $default = null): ?Preference + public function get(string $name, null | array | bool | int | string $default = null): ?Preference { - if ('currencyPreference' === $name) { - throw new FireflyException('No longer supports "currencyPreference", please refactor me.'); - } - /** @var null|User $user */ $user = auth()->user(); if (null === $user) { @@ -72,20 +65,12 @@ class Preferences return $this->getForUser($user, $name, $default); } - /** - * @throws FireflyException - */ - public function getForUser(User $user, string $name, null|array|bool|int|string $default = null): ?Preference + public function getForUser(User $user, string $name, null | array | bool | int | string $default = null): ?Preference { - if ('currencyPreference' === $name) { - throw new FireflyException('No longer supports "currencyPreference", please refactor me.'); - } - $preference = Preference:: - where(function(Builder $q) use($user) { - $q->whereNull('user_group_id'); - $q->orWhere('user_group_id', $user->user_group_id); - }) - ->where('user_id', $user->id)->where('name', $name)->first(['id', 'user_id', 'name', 'data', 'updated_at', 'created_at']); + // don't care about user group ID, except for some specific preferences. + $userGroupId = $this->getUserGroupId($user, $name); + $preference = Preference::where('user_group_id', $userGroupId)->where('user_id', $user->id)->where('name', $name)->first(['id', 'user_id', 'name', 'data', 'updated_at', 'created_at']); + if (null !== $preference && null === $preference->data) { $preference->delete(); $preference = null; @@ -103,17 +88,11 @@ class Preferences return $this->setForUser($user, $name, $default); } - /** - * @throws FireflyException - */ public function delete(string $name): bool { - if ('currencyPreference' === $name) { - throw new FireflyException('No longer supports "currencyPreference", please refactor me.'); - } $fullName = sprintf('preference%s%s', auth()->user()->id, $name); - if (\Cache::has($fullName)) { - \Cache::forget($fullName); + if (Cache::has($fullName)) { + Cache::forget($fullName); } Preference::where('user_id', auth()->user()->id)->where('name', $name)->delete(); @@ -122,35 +101,19 @@ class Preferences public function forget(User $user, string $name): void { - if ('currencyPreference' === $name) { - throw new FireflyException('No longer supports "currencyPreference", please refactor me.'); - } $key = sprintf('preference%s%s', $user->id, $name); - \Cache::forget($key); - \Cache::put($key, '', 5); + Cache::forget($key); + Cache::put($key, '', 5); } - /** - * @param mixed $value - * - * @throws FireflyException - */ - public function setForUser(User $user, string $name, $value): Preference + public function setForUser(User $user, string $name, null | array | bool | int | string $value): Preference { - if ('currencyPreference' === $name) { - throw new FireflyException('No longer supports "currencyPreference", please refactor me.'); - } - $fullName = sprintf('preference%s%s', $user->id, $name); - \Cache::forget($fullName); - - $groupId = null; - $items = config('firefly.admin_specific_prefs'); - if(in_array($name, $items, true)) { - $groupId = (int)$user->user_group_id; - } + $fullName = sprintf('preference%s%s', $user->id, $name); + $groupId = $this->getUserGroupId($user, $name); + Cache::forget($fullName); /** @var null|Preference $pref */ - $pref = Preference::where('user_group_id', $groupId)->where('user_id', $user->id)->where('name', $name)->first(['id', 'name', 'data', 'updated_at', 'created_at']); + $pref = Preference::where('user_group_id', $groupId)->where('user_id', $user->id)->where('name', $name)->first(['id', 'name', 'data', 'updated_at', 'created_at']); if (null !== $pref && null === $value) { $pref->delete(); @@ -161,41 +124,42 @@ class Preferences return new Preference(); } if (null === $pref) { - $pref = new Preference(); - $pref->user_id = (int)$user->id; + $pref = new Preference(); + $pref->user_id = (int)$user->id; $pref->user_group_id = $groupId; - $pref->name = $name; + $pref->name = $name; } $pref->data = $value; - - try { - $pref->save(); - } catch (\PDOException $e) { - throw new FireflyException(sprintf('Could not save preference: %s', $e->getMessage()), 0, $e); - } - \Cache::forever($fullName, $pref); + $pref->save(); + Cache::forever($fullName, $pref); return $pref; } public function beginsWith(User $user, string $search): Collection { - return Preference::where('user_id', $user->id)->where('name', 'LIKE', $search.'%')->get(); + return Preference::where('user_id', $user->id)->where('name', 'LIKE', $search . '%')->get(); } + /** + * Find by name, has no user ID in it, because the method is called from an unauthenticated route any way. + */ public function findByName(string $name): Collection { - if ('currencyPreference' === $name) { - throw new FireflyException('No longer supports "currencyPreference", please refactor me.'); - } - return Preference::where('name', $name)->get(); } public function getArrayForUser(User $user, array $list): array { $result = []; - $preferences = Preference::where('user_id', $user->id)->whereIn('name', $list)->get(['id', 'name', 'data']); + $preferences = Preference + ::where('user_id', $user->id) + ->where(function (Builder $q) use ($user) { + $q->whereNull('user_group_id'); + $q->orWhere('user_group_id', $user->user_group_id); + }) + ->whereIn('name', $list) + ->get(['id', 'name', 'data']); /** @var Preference $preference */ foreach ($preferences as $preference) { @@ -210,17 +174,8 @@ class Preferences return $result; } - /** - * @param mixed $default - * - * @throws FireflyException - */ - public function getFresh(string $name, $default = null): ?Preference + public function getFresh(string $name, null | array | bool | int | string $default = null): ?Preference { - if ('currencyPreference' === $name) { - throw new FireflyException('No longer supports "currencyPreference", please refactor me.'); - } - /** @var null|User $user */ $user = auth()->user(); if (null === $user) { @@ -230,22 +185,6 @@ class Preferences return $preference; } - return $this->getFreshForUser($user, $name, $default); - } - - /** - * TODO remove me. - * - * @param null $default - * - * @throws FireflyException - */ - public function getFreshForUser(User $user, string $name, $default = null): ?Preference - { - if ('currencyPreference' === $name) { - throw new FireflyException('No longer supports "currencyPreference", please refactor me.'); - } - return $this->getForUser($user, $name, $default); } @@ -270,19 +209,12 @@ class Preferences public function mark(): void { $this->set('lastActivity', microtime()); - \Session::forget('first'); + Session::forget('first'); } - /** - * @param mixed $value - * - * @throws FireflyException - */ - public function set(string $name, $value): Preference + public function set(string $name, null | array | bool | int | string $value): Preference { - if ('currencyPreference' === $name) { - throw new FireflyException('No longer supports "currencyPreference", please refactor me.'); - } + /** @var User $user */ $user = auth()->user(); if (null === $user) { // make new preference, return it: @@ -295,4 +227,14 @@ class Preferences return $this->setForUser($user, $name, $value); } + + private function getUserGroupId(User $user, string $preferenceName): ?int + { + $groupId = null; + $items = config('firefly.admin_specific_prefs'); + if (in_array($preferenceName, $items, true)) { + $groupId = (int)$user->user_group_id; + } + return $groupId; + } } diff --git a/app/Transformers/PreferenceTransformer.php b/app/Transformers/PreferenceTransformer.php index a944a0bea9..235ef383c4 100644 --- a/app/Transformers/PreferenceTransformer.php +++ b/app/Transformers/PreferenceTransformer.php @@ -35,12 +35,14 @@ class PreferenceTransformer extends AbstractTransformer */ public function transform(Preference $preference): array { + $userGroupId = $preference->user_group_id === 0 ? null : $preference->user_group_id; return [ - 'id' => $preference->id, - 'created_at' => $preference->created_at->toAtomString(), - 'updated_at' => $preference->updated_at->toAtomString(), - 'name' => $preference->name, - 'data' => $preference->data, + 'id' => $preference->id, + 'created_at' => $preference->created_at->toAtomString(), + 'updated_at' => $preference->updated_at->toAtomString(), + 'user_group_id' => $userGroupId, + 'name' => $preference->name, + 'data' => $preference->data, ]; } } diff --git a/config/firefly.php b/config/firefly.php index 0d541533df..9ec7bb1fb5 100644 --- a/config/firefly.php +++ b/config/firefly.php @@ -216,7 +216,7 @@ return [ ], // administration specific preferences - 'admin_specific_prefs' => ['frontpageAccounts'], + 'admin_specific_prefs' => ['frontPageAccounts', 'lastActivity'], // default user-related values 'darkMode' => 'browser', @@ -437,7 +437,7 @@ return [ 'transfers' => 'fa-exchange', ], - 'bindables' => [ + 'bindables' => [ // models 'account' => Account::class, 'attachment' => Attachment::class, @@ -495,7 +495,7 @@ return [ 'userGroupBill' => UserGroupBill::class, 'userGroup' => UserGroup::class, ], - 'rule-actions' => [ + 'rule-actions' => [ 'set_category' => SetCategory::class, 'clear_category' => ClearCategory::class, 'set_budget' => SetBudget::class, @@ -529,7 +529,7 @@ return [ // 'set_foreign_amount' => SetForeignAmount::class, // 'set_foreign_currency' => SetForeignCurrency::class, ], - 'context-rule-actions' => [ + 'context-rule-actions' => [ 'set_category', 'set_budget', 'add_tag', @@ -548,13 +548,13 @@ return [ 'convert_transfer', ], - 'test-triggers' => [ + 'test-triggers' => [ 'limit' => 10, 'range' => 200, ], // expected source types for each transaction type, in order of preference. - 'expected_source_types' => [ + 'expected_source_types' => [ 'source' => [ TransactionTypeModel::WITHDRAWAL => [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], TransactionTypeEnum::DEPOSIT->value => [AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE, AccountType::REVENUE, AccountType::CASH], @@ -599,7 +599,7 @@ return [ TransactionTypeModel::LIABILITY_CREDIT => [AccountType::LIABILITY_CREDIT, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], ], ], - 'allowed_opposing_types' => [ + 'allowed_opposing_types' => [ 'source' => [ AccountType::ASSET => [ AccountType::ASSET, @@ -689,7 +689,7 @@ return [ ], ], // depending on the account type, return the allowed transaction types: - 'allowed_transaction_types' => [ + 'allowed_transaction_types' => [ 'source' => [ AccountType::ASSET => [ TransactionTypeModel::WITHDRAWAL, @@ -758,7 +758,7 @@ return [ ], // having the source + dest will tell you the transaction type. - 'account_to_transaction' => [ + 'account_to_transaction' => [ AccountType::ASSET => [ AccountType::ASSET => TransactionTypeModel::TRANSFER, AccountType::CASH => TransactionTypeModel::WITHDRAWAL, @@ -823,7 +823,7 @@ return [ ], // allowed source -> destination accounts. - 'source_dests' => [ + 'source_dests' => [ TransactionTypeModel::WITHDRAWAL => [ AccountType::ASSET => [AccountType::EXPENSE, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE, AccountType::CASH], AccountType::LOAN => [AccountType::EXPENSE, AccountType::CASH], @@ -862,7 +862,7 @@ return [ ], ], // if you add fields to this array, don't forget to update the export routine (ExportDataGenerator). - 'journal_meta_fields' => [ + 'journal_meta_fields' => [ // sepa 'sepa_cc', 'sepa_ct_op', @@ -896,33 +896,33 @@ return [ 'recurrence_count', 'recurrence_date', ], - 'webhooks' => [ + 'webhooks' => [ 'max_attempts' => env('WEBHOOK_MAX_ATTEMPTS', 3), ], - 'can_have_virtual_amounts' => [AccountType::ASSET], - 'can_have_opening_balance' => [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], - 'dynamic_creation_allowed' => [ + 'can_have_virtual_amounts' => [AccountType::ASSET], + 'can_have_opening_balance' => [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], + 'dynamic_creation_allowed' => [ AccountType::EXPENSE, AccountType::REVENUE, AccountType::INITIAL_BALANCE, AccountType::RECONCILIATION, AccountType::LIABILITY_CREDIT, ], - 'valid_asset_fields' => ['account_role', 'account_number', 'currency_id', 'BIC', 'include_net_worth'], - 'valid_cc_fields' => ['account_role', 'cc_monthly_payment_date', 'cc_type', 'account_number', 'currency_id', 'BIC', 'include_net_worth'], - 'valid_account_fields' => ['account_number', 'currency_id', 'BIC', 'interest', 'interest_period', 'include_net_worth', 'liability_direction'], + 'valid_asset_fields' => ['account_role', 'account_number', 'currency_id', 'BIC', 'include_net_worth'], + 'valid_cc_fields' => ['account_role', 'cc_monthly_payment_date', 'cc_type', 'account_number', 'currency_id', 'BIC', 'include_net_worth'], + 'valid_account_fields' => ['account_number', 'currency_id', 'BIC', 'interest', 'interest_period', 'include_net_worth', 'liability_direction'], // dynamic date ranges are as follows: - 'dynamic_date_ranges' => ['last7', 'last30', 'last90', 'last365', 'MTD', 'QTD', 'YTD'], + 'dynamic_date_ranges' => ['last7', 'last30', 'last90', 'last365', 'MTD', 'QTD', 'YTD'], // only used in v1 - 'allowed_sort_parameters' => ['order', 'name', 'iban'], + 'allowed_sort_parameters' => ['order', 'name', 'iban'], // preselected account lists possibilities: - 'preselected_accounts' => ['all', 'assets', 'liabilities'], + 'preselected_accounts' => ['all', 'assets', 'liabilities'], // allowed sort columns for API's - 'sorting' => [ + 'sorting' => [ 'allowed' => [ 'transactions' => ['description', 'amount'], 'accounts' => ['name', 'active', 'iban', 'balance', 'last_activity'],