From b0c9fc0792b96588c1fb242df1aa99f965042d16 Mon Sep 17 00:00:00 2001 From: James Cole Date: Tue, 17 Mar 2020 14:54:25 +0100 Subject: [PATCH] Code cleanup that (hopefully) matches style CI --- .../Commands/Correction/CorrectDatabase.php | 3 +- .../Correction/CreateAccessTokens.php | 2 +- .../Commands/Correction/DeleteEmptyGroups.php | 8 +- .../Correction/DeleteEmptyJournals.php | 6 +- .../Correction/DeleteOrphanedTransactions.php | 8 +- .../Commands/Correction/DeleteZeroAmount.php | 1 + .../Commands/Correction/EnableCurrencies.php | 8 +- .../Commands/Correction/FixAccountTypes.php | 37 +- .../Correction/FixLongDescriptions.php | 3 +- .../Commands/Correction/FixUnevenAmount.php | 6 +- app/Console/Commands/DecryptDatabase.php | 9 +- app/Console/Commands/Export/ExportData.php | 18 +- .../Commands/Import/CreateCSVImport.php | 198 +++--- .../Commands/Integrity/ReportIntegrity.php | 1 + app/Console/Commands/Integrity/ReportSum.php | 2 +- app/Console/Commands/ScanAttachments.php | 2 +- app/Console/Commands/Tools/ApplyRules.php | 218 +++--- app/Console/Commands/Tools/Cron.php | 4 +- .../Commands/Upgrade/AccountCurrencies.php | 61 +- .../Commands/Upgrade/BackToJournals.php | 18 +- .../Commands/Upgrade/BudgetLimitCurrency.php | 2 +- .../Commands/Upgrade/CCLiabilities.php | 2 +- .../Commands/Upgrade/MigrateAttachments.php | 4 +- .../Commands/Upgrade/MigrateJournalNotes.php | 2 +- .../Upgrade/MigrateRecurrenceMeta.php | 4 +- .../Commands/Upgrade/MigrateTagLocations.php | 2 +- .../Commands/Upgrade/MigrateToGroups.php | 178 ++--- .../Commands/Upgrade/MigrateToRules.php | 124 ++-- .../Upgrade/OtherCurrenciesCorrections.php | 256 +++---- .../Commands/Upgrade/RenameAccountMeta.php | 2 +- .../Upgrade/TransactionIdentifier.php | 90 ++- .../Upgrade/TransferCurrenciesCorrections.php | 643 +++++++++--------- .../Commands/Upgrade/UpgradeDatabase.php | 5 +- .../Commands/UpgradeFireflyInstructions.php | 4 +- app/Console/Commands/VerifiesAccessToken.php | 8 +- 35 files changed, 980 insertions(+), 959 deletions(-) diff --git a/app/Console/Commands/Correction/CorrectDatabase.php b/app/Console/Commands/Correction/CorrectDatabase.php index 01b6a9c3f6..d2e88f4725 100644 --- a/app/Console/Commands/Correction/CorrectDatabase.php +++ b/app/Console/Commands/Correction/CorrectDatabase.php @@ -30,6 +30,7 @@ use Schema; /** * Class CorrectDatabase + * * @codeCoverageIgnore */ class CorrectDatabase extends Command @@ -73,7 +74,7 @@ class CorrectDatabase extends Command 'firefly-iii:fix-ob-currencies', 'firefly-iii:fix-long-descriptions', 'firefly-iii:fix-recurring-transactions', - 'firefly-iii:restore-oauth-keys' + 'firefly-iii:restore-oauth-keys', ]; foreach ($commands as $command) { $this->line(sprintf('Now executing %s', $command)); diff --git a/app/Console/Commands/Correction/CreateAccessTokens.php b/app/Console/Commands/Correction/CreateAccessTokens.php index 6286c3398c..9dd515c17a 100644 --- a/app/Console/Commands/Correction/CreateAccessTokens.php +++ b/app/Console/Commands/Correction/CreateAccessTokens.php @@ -50,8 +50,8 @@ class CreateAccessTokens extends Command /** * Execute the console command. * - * @return int * @throws Exception + * @return int */ public function handle(): int { diff --git a/app/Console/Commands/Correction/DeleteEmptyGroups.php b/app/Console/Commands/Correction/DeleteEmptyGroups.php index f03796d3ce..abf75ac2a7 100644 --- a/app/Console/Commands/Correction/DeleteEmptyGroups.php +++ b/app/Console/Commands/Correction/DeleteEmptyGroups.php @@ -49,15 +49,15 @@ class DeleteEmptyGroups extends Command /** * Execute the console command. * - * @return mixed * @throws Exception; + * @return mixed */ public function handle(): int { Log::debug(sprintf('Now in %s', __METHOD__)); - $start = microtime(true); - $groupIds = - TransactionGroup + $start = microtime(true); + $groupIds + = TransactionGroup ::leftJoin('transaction_journals', 'transaction_groups.id', '=', 'transaction_journals.transaction_group_id') ->whereNull('transaction_journals.id')->get(['transaction_groups.id'])->pluck('id')->toArray(); diff --git a/app/Console/Commands/Correction/DeleteEmptyJournals.php b/app/Console/Commands/Correction/DeleteEmptyJournals.php index 5704b3fdc3..034aeb89d3 100644 --- a/app/Console/Commands/Correction/DeleteEmptyJournals.php +++ b/app/Console/Commands/Correction/DeleteEmptyJournals.php @@ -101,18 +101,18 @@ class DeleteEmptyJournals extends Command ->get([DB::raw('COUNT(transactions.transaction_journal_id) as the_count'), 'transaction_journal_id']); $total = 0; foreach ($set as $row) { - $count = (int)$row->the_count; + $count = (int) $row->the_count; if (1 === $count % 2) { // uneven number, delete journal and transactions: try { - TransactionJournal::find((int)$row->transaction_journal_id)->delete(); + TransactionJournal::find((int) $row->transaction_journal_id)->delete(); // @codeCoverageIgnoreStart } catch (Exception $e) { Log::info(sprintf('Could not delete journal: %s', $e->getMessage())); } // @codeCoverageIgnoreEnd - Transaction::where('transaction_journal_id', (int)$row->transaction_journal_id)->delete(); + Transaction::where('transaction_journal_id', (int) $row->transaction_journal_id)->delete(); $this->info(sprintf('Deleted transaction journal #%d because it had an uneven number of transactions.', $row->transaction_journal_id)); $total++; } diff --git a/app/Console/Commands/Correction/DeleteOrphanedTransactions.php b/app/Console/Commands/Correction/DeleteOrphanedTransactions.php index f741da152e..c95007b60f 100644 --- a/app/Console/Commands/Correction/DeleteOrphanedTransactions.php +++ b/app/Console/Commands/Correction/DeleteOrphanedTransactions.php @@ -51,8 +51,8 @@ class DeleteOrphanedTransactions extends Command /** * Execute the console command. * - * @return int * @throws Exception + * @return int */ public function handle(): int { @@ -79,7 +79,7 @@ class DeleteOrphanedTransactions extends Command /** @var Transaction $transaction */ foreach ($set as $transaction) { // delete journals - $journal = TransactionJournal::find((int)$transaction->transaction_journal_id); + $journal = TransactionJournal::find((int) $transaction->transaction_journal_id); if ($journal) { try { $journal->delete(); @@ -89,7 +89,7 @@ class DeleteOrphanedTransactions extends Command } // @codeCoverageIgnoreEnd } - Transaction::where('transaction_journal_id', (int)$transaction->transaction_journal_id)->delete(); + Transaction::where('transaction_journal_id', (int) $transaction->transaction_journal_id)->delete(); $this->line( sprintf( 'Deleted transaction journal #%d because account #%d was already deleted.', @@ -123,7 +123,7 @@ class DeleteOrphanedTransactions extends Command ); /** @var stdClass $entry */ foreach ($set as $entry) { - $transaction = Transaction::find((int)$entry->transaction_id); + $transaction = Transaction::find((int) $entry->transaction_id); $transaction->delete(); $this->info( sprintf( diff --git a/app/Console/Commands/Correction/DeleteZeroAmount.php b/app/Console/Commands/Correction/DeleteZeroAmount.php index afae462e17..f96b426eee 100644 --- a/app/Console/Commands/Correction/DeleteZeroAmount.php +++ b/app/Console/Commands/Correction/DeleteZeroAmount.php @@ -49,6 +49,7 @@ class DeleteZeroAmount extends Command /** * Execute the console command. + * * @return int */ public function handle(): int diff --git a/app/Console/Commands/Correction/EnableCurrencies.php b/app/Console/Commands/Correction/EnableCurrencies.php index 87f5fe4ef7..f31d8d102a 100644 --- a/app/Console/Commands/Correction/EnableCurrencies.php +++ b/app/Console/Commands/Correction/EnableCurrencies.php @@ -62,28 +62,28 @@ class EnableCurrencies extends Command /** @var Collection $meta */ $meta = AccountMeta::where('name', 'currency_id')->groupBy('data')->get(['data']); foreach ($meta as $entry) { - $found[] = (int)$entry->data; + $found[] = (int) $entry->data; } // get all from journals: /** @var Collection $journals */ $journals = TransactionJournal::groupBy('transaction_currency_id')->get(['transaction_currency_id']); foreach ($journals as $entry) { - $found[] = (int)$entry->transaction_currency_id; + $found[] = (int) $entry->transaction_currency_id; } // get all from transactions /** @var Collection $transactions */ $transactions = Transaction::groupBy('transaction_currency_id')->get(['transaction_currency_id']); foreach ($transactions as $entry) { - $found[] = (int)$entry->transaction_currency_id; + $found[] = (int) $entry->transaction_currency_id; } // get all from budget limits /** @var Collection $limits */ $limits = BudgetLimit::groupBy('transaction_currency_id')->get(['transaction_currency_id']); foreach ($limits as $entry) { - $found[] = (int)$entry->transaction_currency_id; + $found[] = (int) $entry->transaction_currency_id; } $found = array_unique($found); diff --git a/app/Console/Commands/Correction/FixAccountTypes.php b/app/Console/Commands/Correction/FixAccountTypes.php index 6f1e8adb4d..8b6bd26d8e 100644 --- a/app/Console/Commands/Correction/FixAccountTypes.php +++ b/app/Console/Commands/Correction/FixAccountTypes.php @@ -49,20 +49,20 @@ class FixAccountTypes extends Command * @var string */ protected $signature = 'firefly-iii:fix-account-types'; + /** @var int */ + private $count; /** @var array */ private $expected; /** @var AccountFactory */ private $factory; /** @var array */ private $fixable; - /** @var int */ - private $count; /** * Execute the console command. * - * @return int * @throws FireflyException + * @return int */ public function handle(): int { @@ -109,23 +109,12 @@ class FixAccountTypes extends Command return 0; } - /** - * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is - * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should - * be called from the handle method instead of using the constructor to initialize the command. - * - * @codeCoverageIgnore - */ - private function stupidLaravel(): void - { - $this->count = 0; - } - /** * @param TransactionJournal $journal - * @param string $type - * @param Transaction $source - * @param Transaction $dest + * @param string $type + * @param Transaction $source + * @param Transaction $dest + * * @throws FireflyException */ private function fixJournal(TransactionJournal $journal, string $type, Transaction $source, Transaction $dest): void @@ -278,4 +267,16 @@ class FixAccountTypes extends Command $this->fixJournal($journal, $type, $sourceTransaction, $destTransaction); } } + + /** + * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is + * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should + * be called from the handle method instead of using the constructor to initialize the command. + * + * @codeCoverageIgnore + */ + private function stupidLaravel(): void + { + $this->count = 0; + } } diff --git a/app/Console/Commands/Correction/FixLongDescriptions.php b/app/Console/Commands/Correction/FixLongDescriptions.php index f43b3dd6ab..264c728d58 100644 --- a/app/Console/Commands/Correction/FixLongDescriptions.php +++ b/app/Console/Commands/Correction/FixLongDescriptions.php @@ -51,7 +51,7 @@ class FixLongDescriptions extends Command */ public function handle(): int { - $start = microtime(true); + $start = microtime(true); $journals = TransactionJournal::get(['id', 'description']); /** @var TransactionJournal $journal */ foreach ($journals as $journal) { @@ -73,6 +73,7 @@ class FixLongDescriptions extends Command } $end = round(microtime(true) - $start, 2); $this->info(sprintf('Verified all transaction group and journal title lengths in %s seconds.', $end)); + return 0; } } diff --git a/app/Console/Commands/Correction/FixUnevenAmount.php b/app/Console/Commands/Correction/FixUnevenAmount.php index 6e7ead58a5..d545d2f11e 100644 --- a/app/Console/Commands/Correction/FixUnevenAmount.php +++ b/app/Console/Commands/Correction/FixUnevenAmount.php @@ -63,8 +63,8 @@ class FixUnevenAmount extends Command ->get(['transaction_journal_id', DB::raw('SUM(amount) AS the_sum')]); /** @var stdClass $entry */ foreach ($journals as $entry) { - if (0 !== bccomp((string)$entry->the_sum, '0')) { - $this->fixJournal((int)$entry->transaction_journal_id); + if (0 !== bccomp((string) $entry->the_sum, '0')) { + $this->fixJournal((int) $entry->transaction_journal_id); $count++; } } @@ -105,7 +105,7 @@ class FixUnevenAmount extends Command return; } - $amount = bcmul('-1', (string)$source->amount); + $amount = bcmul('-1', (string) $source->amount); // fix amount of destination: /** @var Transaction $destination */ diff --git a/app/Console/Commands/DecryptDatabase.php b/app/Console/Commands/DecryptDatabase.php index a3733fd1bf..8d90e1134b 100644 --- a/app/Console/Commands/DecryptDatabase.php +++ b/app/Console/Commands/DecryptDatabase.php @@ -54,8 +54,8 @@ class DecryptDatabase extends Command /** * Execute the console command. * - * @return int * @throws FireflyException + * @return int */ public function handle(): int { @@ -96,7 +96,7 @@ class DecryptDatabase extends Command Log::debug(sprintf('Decrypted field "%s" "%s" to "%s" in table "%s" (row #%d)', $field, $original, print_r($value, true), $table, $id)); /** @var Preference $object */ - $object = Preference::find((int)$id); + $object = Preference::find((int) $id); if (null !== $object) { $object->data = $value; $object->save(); @@ -131,7 +131,7 @@ class DecryptDatabase extends Command $configName = sprintf('is_decrypted_%s', $table); $configVar = app('fireflyconfig')->get($configName, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; @@ -142,8 +142,9 @@ class DecryptDatabase extends Command * Tries to decrypt data. Will only throw an exception when the MAC is invalid. * * @param $value - * @return string + * * @throws FireflyException + * @return string */ private function tryDecrypt($value) { diff --git a/app/Console/Commands/Export/ExportData.php b/app/Console/Commands/Export/ExportData.php index 017c30a505..6b38974478 100644 --- a/app/Console/Commands/Export/ExportData.php +++ b/app/Console/Commands/Export/ExportData.php @@ -22,6 +22,7 @@ namespace FireflyIII\Console\Commands\Export; use Carbon\Carbon; +use Exception; use FireflyIII\Console\Commands\VerifiesAccessToken; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\AccountType; @@ -32,6 +33,7 @@ use FireflyIII\User; use Illuminate\Console\Command; use Illuminate\Support\Collection; use InvalidArgumentException; +use League\Csv\CannotInsertRecord; use Log; /** @@ -79,9 +81,9 @@ class ExportData extends Command /** * Execute the console command. * - * @return int * @throws FireflyException - * @throws \League\Csv\CannotInsertRecord + * @throws CannotInsertRecord + * @return int */ public function handle(): int { @@ -162,8 +164,8 @@ class ExportData extends Command } /** - * @return Collection * @throws FireflyException + * @return Collection */ private function getAccountsParameter(): Collection { @@ -171,7 +173,7 @@ class ExportData extends Command $accounts = new Collection; $accountList = $this->option('accounts'); $types = [AccountType::ASSET, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE]; - if (null !== $accountList && '' !== (string)$accountList) { + if (null !== $accountList && '' !== (string) $accountList) { $accountIds = explode(',', $accountList); $accounts = $this->accountRepository->getAccountsById($accountIds); } @@ -195,9 +197,9 @@ class ExportData extends Command /** * @param string $field * - * @return Carbon * @throws FireflyException - * @throws \Exception + * @throws Exception + * @return Carbon */ private function getDateParameter(string $field): Carbon { @@ -231,8 +233,8 @@ class ExportData extends Command } /** - * @return string * @throws FireflyException + * @return string */ private function getExportDirectory(): string { @@ -248,8 +250,8 @@ class ExportData extends Command } /** - * @return array * @throws FireflyException + * @return array */ private function parseOptions(): array { diff --git a/app/Console/Commands/Import/CreateCSVImport.php b/app/Console/Commands/Import/CreateCSVImport.php index 5af1f03c41..c6390e440d 100644 --- a/app/Console/Commands/Import/CreateCSVImport.php +++ b/app/Console/Commands/Import/CreateCSVImport.php @@ -39,6 +39,7 @@ use Log; /** * Class CreateCSVImport. + * * @deprecated * @codeCoverageIgnore */ @@ -62,12 +63,12 @@ class CreateCSVImport extends Command {configuration? : The configuration file to use for the import.} {--user=1 : The user ID that the import should import for.} {--token= : The user\'s access token.}'; - /** @var UserRepositoryInterface */ - private $userRepository; - /** @var ImportJobRepositoryInterface */ - private $importRepository; /** @var ImportJob */ private $importJob; + /** @var ImportJobRepositoryInterface */ + private $importRepository; + /** @var UserRepositoryInterface */ + private $userRepository; /** * Run the command. @@ -89,9 +90,9 @@ class CreateCSVImport extends Command } // @codeCoverageIgnoreEnd /** @var User $user */ - $user = $this->userRepository->findNull((int)$this->option('user')); - $file = (string)$this->argument('file'); - $configuration = (string)$this->argument('configuration'); + $user = $this->userRepository->findNull((int) $this->option('user')); + $file = (string) $this->argument('file'); + $configuration = (string) $this->argument('configuration'); $this->importRepository->setUser($user); @@ -149,21 +150,9 @@ class CreateCSVImport extends Command } /** - * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is - * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should - * be called from the handle method instead of using the constructor to initialize the command. - * - * @codeCoverageIgnore - */ - private function stupidLaravel(): void - { - $this->userRepository = app(UserRepositoryInterface::class); - $this->importRepository = app(ImportJobRepositoryInterface::class); - } - - /** - * @param string $message + * @param string $message * @param array|null $data + * * @codeCoverageIgnore */ private function errorLine(string $message, array $data = null): void @@ -173,9 +162,34 @@ class CreateCSVImport extends Command } + /** + * + */ + private function giveFeedback(): void + { + $this->infoLine('Job has finished.'); + + + if (null !== $this->importJob->tag) { + $this->infoLine(sprintf('%d transaction(s) have been imported.', $this->importJob->tag->transactionJournals->count())); + $this->infoLine(sprintf('You can find your transactions under tag "%s"', $this->importJob->tag->tag)); + } + + if (null === $this->importJob->tag) { + $this->errorLine('No transactions have been imported :(.'); + } + if (count($this->importJob->errors) > 0) { + $this->infoLine(sprintf('%d error(s) occurred:', count($this->importJob->errors))); + foreach ($this->importJob->errors as $err) { + $this->errorLine('- ' . $err); + } + } + } + /** * @param string $message - * @param array $data + * @param array $data + * * @codeCoverageIgnore */ private function infoLine(string $message, array $data = null): void @@ -184,65 +198,6 @@ class CreateCSVImport extends Command $this->line($message); } - /** - * Verify user inserts correct arguments. - * - * @noinspection MultipleReturnStatementsInspection - * @return bool - * @codeCoverageIgnore - */ - private function validArguments(): bool - { - $file = (string)$this->argument('file'); - $configuration = (string)$this->argument('configuration'); - $cwd = getcwd(); - $enabled = (bool)config('import.enabled.file'); - - if (false === $enabled) { - $this->errorLine('CSV Provider is not enabled.'); - - return false; - } - - if (!file_exists($file)) { - $this->errorLine(sprintf('Firefly III cannot find file "%s" (working directory: "%s").', $file, $cwd)); - - return false; - } - - if (!file_exists($configuration)) { - $this->errorLine(sprintf('Firefly III cannot find configuration file "%s" (working directory: "%s").', $configuration, $cwd)); - - return false; - } - - $configurationData = json_decode(file_get_contents($configuration), true, 512, JSON_THROW_ON_ERROR); - if (null === $configurationData) { - $this->errorLine(sprintf('Firefly III cannot read the contents of configuration file "%s" (working directory: "%s").', $configuration, $cwd)); - - return false; - } - - return true; - } - - /** - * Store the supplied file as an attachment to this job. - * - * @param string $file - * @throws FireflyException - */ - private function storeFile(string $file): void - { - // store file as attachment. - if ('' !== $file) { - $messages = $this->importRepository->storeCLIUpload($this->importJob, 'import_file', $file); - if ($messages->count() > 0) { - throw new FireflyException($messages->first()); - } - } - } - /** * Keep repeating import call until job lands on "provider_finished". * @@ -309,26 +264,75 @@ class CreateCSVImport extends Command } /** + * Store the supplied file as an attachment to this job. * + * @param string $file + * + * @throws FireflyException */ - private function giveFeedback(): void + private function storeFile(string $file): void { - $this->infoLine('Job has finished.'); - - - if (null !== $this->importJob->tag) { - $this->infoLine(sprintf('%d transaction(s) have been imported.', $this->importJob->tag->transactionJournals->count())); - $this->infoLine(sprintf('You can find your transactions under tag "%s"', $this->importJob->tag->tag)); - } - - if (null === $this->importJob->tag) { - $this->errorLine('No transactions have been imported :(.'); - } - if (count($this->importJob->errors) > 0) { - $this->infoLine(sprintf('%d error(s) occurred:', count($this->importJob->errors))); - foreach ($this->importJob->errors as $err) { - $this->errorLine('- ' . $err); + // store file as attachment. + if ('' !== $file) { + $messages = $this->importRepository->storeCLIUpload($this->importJob, 'import_file', $file); + if ($messages->count() > 0) { + throw new FireflyException($messages->first()); } } } + + /** + * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is + * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should + * be called from the handle method instead of using the constructor to initialize the command. + * + * @codeCoverageIgnore + */ + private function stupidLaravel(): void + { + $this->userRepository = app(UserRepositoryInterface::class); + $this->importRepository = app(ImportJobRepositoryInterface::class); + } + + /** + * Verify user inserts correct arguments. + * + * @noinspection MultipleReturnStatementsInspection + * @return bool + * @codeCoverageIgnore + */ + private function validArguments(): bool + { + $file = (string) $this->argument('file'); + $configuration = (string) $this->argument('configuration'); + $cwd = getcwd(); + $enabled = (bool) config('import.enabled.file'); + + if (false === $enabled) { + $this->errorLine('CSV Provider is not enabled.'); + + return false; + } + + if (!file_exists($file)) { + $this->errorLine(sprintf('Firefly III cannot find file "%s" (working directory: "%s").', $file, $cwd)); + + return false; + } + + if (!file_exists($configuration)) { + $this->errorLine(sprintf('Firefly III cannot find configuration file "%s" (working directory: "%s").', $configuration, $cwd)); + + return false; + } + + $configurationData = json_decode(file_get_contents($configuration), true, 512, JSON_THROW_ON_ERROR); + if (null === $configurationData) { + $this->errorLine(sprintf('Firefly III cannot read the contents of configuration file "%s" (working directory: "%s").', $configuration, $cwd)); + + return false; + } + + return true; + } } diff --git a/app/Console/Commands/Integrity/ReportIntegrity.php b/app/Console/Commands/Integrity/ReportIntegrity.php index a30e0c5e1f..62e45ed110 100644 --- a/app/Console/Commands/Integrity/ReportIntegrity.php +++ b/app/Console/Commands/Integrity/ReportIntegrity.php @@ -30,6 +30,7 @@ use Schema; /** * Class ReportIntegrity + * * @codeCoverageIgnore */ class ReportIntegrity extends Command diff --git a/app/Console/Commands/Integrity/ReportSum.php b/app/Console/Commands/Integrity/ReportSum.php index 45934f0fb8..2f24e1f00a 100644 --- a/app/Console/Commands/Integrity/ReportSum.php +++ b/app/Console/Commands/Integrity/ReportSum.php @@ -69,7 +69,7 @@ class ReportSum extends Command /** @var User $user */ foreach ($userRepository->all() as $user) { - $sum = (string)$user->transactions()->sum('amount'); + $sum = (string) $user->transactions()->sum('amount'); if (0 !== bccomp($sum, '0')) { $message = sprintf('Error: Transactions for user #%d (%s) are off by %s!', $user->id, $user->email, $sum); $this->error($message); diff --git a/app/Console/Commands/ScanAttachments.php b/app/Console/Commands/ScanAttachments.php index 60077a1573..8ba285ff00 100644 --- a/app/Console/Commands/ScanAttachments.php +++ b/app/Console/Commands/ScanAttachments.php @@ -63,7 +63,7 @@ class ScanAttachments extends Command $disk = Storage::disk('upload'); /** @var Attachment $attachment */ foreach ($attachments as $attachment) { - $fileName = $attachment->fileName(); + $fileName = $attachment->fileName(); try { $encryptedContent = $disk->get($fileName); } catch (FileNotFoundException $e) { diff --git a/app/Console/Commands/Tools/ApplyRules.php b/app/Console/Commands/Tools/ApplyRules.php index e61398da3a..419b1ec3d2 100644 --- a/app/Console/Commands/Tools/ApplyRules.php +++ b/app/Console/Commands/Tools/ApplyRules.php @@ -68,35 +68,32 @@ class ApplyRules extends Command {--all_rules : If set, will overrule both settings and simply apply ALL of your rules.} {--start_date= : The date of the earliest transaction to be included (inclusive). If omitted, will be your very first transaction ever. Format: YYYY-MM-DD} {--end_date= : The date of the latest transaction to be included (inclusive). If omitted, will be your latest transaction ever. Format: YYYY-MM-DD}'; - - /** @var Collection */ - private $accounts; /** @var array */ private $acceptedAccounts; + /** @var Collection */ + private $accounts; + /** @var bool */ + private $allRules; /** @var Carbon */ private $endDate; + /** @var Collection */ + private $groups; + /** @var RuleGroupRepositoryInterface */ + private $ruleGroupRepository; /** @var array */ private $ruleGroupSelection; + /** @var RuleRepositoryInterface */ + private $ruleRepository; /** @var array */ private $ruleSelection; /** @var Carbon */ private $startDate; - /** @var Collection */ - private $groups; - /** @var bool */ - private $allRules; - - /** @var RuleRepositoryInterface */ - private $ruleRepository; - - /** @var RuleGroupRepositoryInterface */ - private $ruleGroupRepository; /** * Execute the console command. * - * @return int * @throws FireflyException + * @return int */ public function handle(): int { @@ -169,6 +166,49 @@ class ApplyRules extends Command return 0; } + /** + * @return array + */ + private function getRulesToApply(): array + { + $rulesToApply = []; + /** @var RuleGroup $group */ + foreach ($this->groups as $group) { + $rules = $this->ruleGroupRepository->getActiveStoreRules($group); + /** @var Rule $rule */ + foreach ($rules as $rule) { + // if in rule selection, or group in selection or all rules, it's included. + $test = $this->includeRule($rule, $group); + if (true === $test) { + Log::debug(sprintf('Will include rule #%d "%s"', $rule->id, $rule->title)); + $rulesToApply[] = $rule->id; + } + } + } + + return $rulesToApply; + } + + /** + */ + private function grabAllRules(): void + { + $this->groups = $this->ruleGroupRepository->getActiveGroups(); + } + + /** + * @param Rule $rule + * @param RuleGroup $group + * + * @return bool + */ + private function includeRule(Rule $rule, RuleGroup $group): bool + { + return in_array($group->id, $this->ruleGroupSelection, true) + || in_array($rule->id, $this->ruleSelection, true) + || $this->allRules; + } + /** * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should @@ -189,8 +229,8 @@ class ApplyRules extends Command } /** - * @return bool * @throws FireflyException + * @return bool */ private function verifyInput(): bool { @@ -212,8 +252,8 @@ class ApplyRules extends Command } /** - * @return bool * @throws FireflyException + * @return bool */ private function verifyInputAccounts(): bool { @@ -240,7 +280,7 @@ class ApplyRules extends Command foreach ($accountList as $accountId) { - $accountId = (int)$accountId; + $accountId = (int) $accountId; $account = $accountRepository->findNull($accountId); if (null !== $account && in_array($account->accountType->type, $this->acceptedAccounts, true)) { $finalList->push($account); @@ -258,66 +298,6 @@ class ApplyRules extends Command } - /** - * @return bool - */ - private function verifyInputRuleGroups(): bool - { - $ruleGroupString = $this->option('rule_groups'); - if (null === $ruleGroupString || '' === $ruleGroupString) { - // can be empty. - return true; - } - $ruleGroupList = explode(',', $ruleGroupString); - // @codeCoverageIgnoreStart - if (0 === count($ruleGroupList)) { - // can be empty. - return true; - } - // @codeCoverageIgnoreEnd - foreach ($ruleGroupList as $ruleGroupId) { - $ruleGroup = $this->ruleGroupRepository->find((int)$ruleGroupId); - if ($ruleGroup->active) { - $this->ruleGroupSelection[] = $ruleGroup->id; - } - if (false === $ruleGroup->active) { - $this->warn(sprintf('Will ignore inactive rule group #%d ("%s")', $ruleGroup->id, $ruleGroup->title)); - } - } - - return true; - } - - /** - * @return bool - */ - private function verifyInputRules(): bool - { - $ruleString = $this->option('rules'); - if (null === $ruleString || '' === $ruleString) { - // can be empty. - return true; - } - $ruleList = explode(',', $ruleString); - - // @codeCoverageIgnoreStart - if (0 === count($ruleList)) { - // can be empty. - - return true; - } - // @codeCoverageIgnoreEnd - - foreach ($ruleList as $ruleId) { - $rule = $this->ruleRepository->find((int)$ruleId); - if (null !== $rule && $rule->active) { - $this->ruleSelection[] = $rule->id; - } - } - - return true; - } - /** * @throws FireflyException */ @@ -355,44 +335,62 @@ class ApplyRules extends Command } /** - */ - private function grabAllRules(): void - { - $this->groups = $this->ruleGroupRepository->getActiveGroups(); - } - - /** - * @param Rule $rule - * @param RuleGroup $group * @return bool */ - private function includeRule(Rule $rule, RuleGroup $group): bool + private function verifyInputRuleGroups(): bool { - return in_array($group->id, $this->ruleGroupSelection, true) || - in_array($rule->id, $this->ruleSelection, true) || - $this->allRules; - } - - /** - * @return array - */ - private function getRulesToApply(): array - { - $rulesToApply = []; - /** @var RuleGroup $group */ - foreach ($this->groups as $group) { - $rules = $this->ruleGroupRepository->getActiveStoreRules($group); - /** @var Rule $rule */ - foreach ($rules as $rule) { - // if in rule selection, or group in selection or all rules, it's included. - $test = $this->includeRule($rule, $group); - if (true === $test) { - Log::debug(sprintf('Will include rule #%d "%s"', $rule->id, $rule->title)); - $rulesToApply[] = $rule->id; - } + $ruleGroupString = $this->option('rule_groups'); + if (null === $ruleGroupString || '' === $ruleGroupString) { + // can be empty. + return true; + } + $ruleGroupList = explode(',', $ruleGroupString); + // @codeCoverageIgnoreStart + if (0 === count($ruleGroupList)) { + // can be empty. + return true; + } + // @codeCoverageIgnoreEnd + foreach ($ruleGroupList as $ruleGroupId) { + $ruleGroup = $this->ruleGroupRepository->find((int) $ruleGroupId); + if ($ruleGroup->active) { + $this->ruleGroupSelection[] = $ruleGroup->id; + } + if (false === $ruleGroup->active) { + $this->warn(sprintf('Will ignore inactive rule group #%d ("%s")', $ruleGroup->id, $ruleGroup->title)); } } - return $rulesToApply; + return true; + } + + /** + * @return bool + */ + private function verifyInputRules(): bool + { + $ruleString = $this->option('rules'); + if (null === $ruleString || '' === $ruleString) { + // can be empty. + return true; + } + $ruleList = explode(',', $ruleString); + + // @codeCoverageIgnoreStart + if (0 === count($ruleList)) { + // can be empty. + + return true; + } + // @codeCoverageIgnoreEnd + + foreach ($ruleList as $ruleId) { + $rule = $this->ruleRepository->find((int) $ruleId); + if (null !== $rule && $rule->active) { + $this->ruleSelection[] = $rule->id; + } + } + + return true; } } diff --git a/app/Console/Commands/Tools/Cron.php b/app/Console/Commands/Tools/Cron.php index 1f2b77ce78..31fc056f76 100644 --- a/app/Console/Commands/Tools/Cron.php +++ b/app/Console/Commands/Tools/Cron.php @@ -27,8 +27,8 @@ namespace FireflyIII\Console\Commands\Tools; use Carbon\Carbon; use Exception; use FireflyIII\Exceptions\FireflyException; -use FireflyIII\Support\Cronjobs\RecurringCronjob; use FireflyIII\Support\Cronjobs\AutoBudgetCronjob; +use FireflyIII\Support\Cronjobs\RecurringCronjob; use Illuminate\Console\Command; use InvalidArgumentException; use Log; @@ -68,7 +68,7 @@ class Cron extends Command $this->error(sprintf('"%s" is not a valid date', $this->option('date'))); $e->getMessage(); } - $force = (bool)$this->option('force'); + $force = (bool) $this->option('force'); /* * Fire recurring transaction cron job. diff --git a/app/Console/Commands/Upgrade/AccountCurrencies.php b/app/Console/Commands/Upgrade/AccountCurrencies.php index cbb6d77b1b..113e1d829a 100644 --- a/app/Console/Commands/Upgrade/AccountCurrencies.php +++ b/app/Console/Commands/Upgrade/AccountCurrencies.php @@ -54,10 +54,10 @@ class AccountCurrencies extends Command protected $signature = 'firefly-iii:account-currencies {--F|force : Force the execution of this command.}'; /** @var AccountRepositoryInterface */ private $accountRepos; - /** @var UserRepositoryInterface */ - private $userRepos; /** @var int */ private $count; + /** @var UserRepositoryInterface */ + private $userRepos; /** * Each (asset) account must have a reference to a preferred currency. If the account does not have one, it's forced upon the account. @@ -90,6 +90,27 @@ class AccountCurrencies extends Command return 0; } + /** + * @return bool + */ + private function isExecuted(): bool + { + $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); + if (null !== $configVar) { + return (bool) $configVar->data; + } + + return false; // @codeCoverageIgnore + } + + /** + * + */ + private function markAsExecuted(): void + { + app('fireflyconfig')->set(self::CONFIG_NAME, true); + } + /** * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should @@ -105,29 +126,7 @@ class AccountCurrencies extends Command } /** - * @return bool - */ - private function isExecuted(): bool - { - $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); - if (null !== $configVar) { - return (bool)$configVar->data; - } - - return false; // @codeCoverageIgnore - } - - - /** - * - */ - private function markAsExecuted(): void - { - app('fireflyconfig')->set(self::CONFIG_NAME, true); - } - - /** - * @param Account $account + * @param Account $account * @param TransactionCurrency $currency */ private function updateAccount(Account $account, TransactionCurrency $currency): void @@ -135,13 +134,13 @@ class AccountCurrencies extends Command Log::debug(sprintf('Now in updateAccount(%d, %s)', $account->id, $currency->code)); $this->accountRepos->setUser($account->user); - $accountCurrency = (int)$this->accountRepos->getMetaValue($account, 'currency_id'); + $accountCurrency = (int) $this->accountRepos->getMetaValue($account, 'currency_id'); Log::debug(sprintf('Account currency is #%d', $accountCurrency)); - $openingBalance = $this->accountRepos->getOpeningBalance($account); - $obCurrency = 0; + $openingBalance = $this->accountRepos->getOpeningBalance($account); + $obCurrency = 0; if (null !== $openingBalance) { - $obCurrency = (int)$openingBalance->transaction_currency_id; + $obCurrency = (int) $openingBalance->transaction_currency_id; Log::debug('Account has opening balance.'); } Log::debug(sprintf('Account OB currency is #%d.', $obCurrency)); @@ -195,7 +194,7 @@ class AccountCurrencies extends Command { Log::debug('Now in updateAccountCurrencies()'); $users = $this->userRepos->all(); - $defaultCurrencyCode = (string)config('firefly.default_currency', 'EUR'); + $defaultCurrencyCode = (string) config('firefly.default_currency', 'EUR'); Log::debug(sprintf('Default currency is %s', $defaultCurrencyCode)); foreach ($users as $user) { $this->updateCurrenciesForUser($user, $defaultCurrencyCode); @@ -203,7 +202,7 @@ class AccountCurrencies extends Command } /** - * @param User $user + * @param User $user * @param string $systemCurrencyCode */ private function updateCurrenciesForUser(User $user, string $systemCurrencyCode): void diff --git a/app/Console/Commands/Upgrade/BackToJournals.php b/app/Console/Commands/Upgrade/BackToJournals.php index 198e7d4968..bc6a2aead2 100644 --- a/app/Console/Commands/Upgrade/BackToJournals.php +++ b/app/Console/Commands/Upgrade/BackToJournals.php @@ -111,9 +111,9 @@ class BackToJournals extends Command $chunks = array_chunk($transactions, 500); foreach ($chunks as $chunk) { - $set = DB::table('transactions') - ->whereIn('transactions.id', $chunk) - ->get(['transaction_journal_id'])->pluck('transaction_journal_id')->toArray(); + $set = DB::table('transactions') + ->whereIn('transactions.id', $chunk) + ->get(['transaction_journal_id'])->pluck('transaction_journal_id')->toArray(); /** @noinspection SlowArrayOperationsInLoopInspection */ $array = array_merge($array, $set); } @@ -128,7 +128,7 @@ class BackToJournals extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore @@ -141,7 +141,7 @@ class BackToJournals extends Command { $configVar = app('fireflyconfig')->get(MigrateToGroups::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore @@ -212,7 +212,7 @@ class BackToJournals extends Command // both have a budget, but they don't match. if (null !== $budget && null !== $journalBudget && $budget->id !== $journalBudget->id) { // sync to journal: - $journal->budgets()->sync([(int)$budget->id]); + $journal->budgets()->sync([(int) $budget->id]); return; } @@ -220,7 +220,7 @@ class BackToJournals extends Command // transaction has a budget, but the journal doesn't. if (null !== $budget && null === $journalBudget) { // sync to journal: - $journal->budgets()->sync([(int)$budget->id]); + $journal->budgets()->sync([(int) $budget->id]); } } @@ -271,12 +271,12 @@ class BackToJournals extends Command // both have a category, but they don't match. if (null !== $category && null !== $journalCategory && $category->id !== $journalCategory->id) { // sync to journal: - $journal->categories()->sync([(int)$category->id]); + $journal->categories()->sync([(int) $category->id]); } // transaction has a category, but the journal doesn't. if (null !== $category && null === $journalCategory) { - $journal->categories()->sync([(int)$category->id]); + $journal->categories()->sync([(int) $category->id]); } } } diff --git a/app/Console/Commands/Upgrade/BudgetLimitCurrency.php b/app/Console/Commands/Upgrade/BudgetLimitCurrency.php index e25f36356c..990b02bbf8 100644 --- a/app/Console/Commands/Upgrade/BudgetLimitCurrency.php +++ b/app/Console/Commands/Upgrade/BudgetLimitCurrency.php @@ -102,7 +102,7 @@ class BudgetLimitCurrency extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore diff --git a/app/Console/Commands/Upgrade/CCLiabilities.php b/app/Console/Commands/Upgrade/CCLiabilities.php index 489bf934fa..e5904a7198 100644 --- a/app/Console/Commands/Upgrade/CCLiabilities.php +++ b/app/Console/Commands/Upgrade/CCLiabilities.php @@ -101,7 +101,7 @@ class CCLiabilities extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore diff --git a/app/Console/Commands/Upgrade/MigrateAttachments.php b/app/Console/Commands/Upgrade/MigrateAttachments.php index eab27f6941..9e2270d7d2 100644 --- a/app/Console/Commands/Upgrade/MigrateAttachments.php +++ b/app/Console/Commands/Upgrade/MigrateAttachments.php @@ -71,7 +71,7 @@ class MigrateAttachments extends Command foreach ($attachments as $att) { // move description: - $description = (string)$att->description; + $description = (string) $att->description; if ('' !== $description) { // find or create note: @@ -111,7 +111,7 @@ class MigrateAttachments extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore diff --git a/app/Console/Commands/Upgrade/MigrateJournalNotes.php b/app/Console/Commands/Upgrade/MigrateJournalNotes.php index 46f3128fcc..a234c2948f 100644 --- a/app/Console/Commands/Upgrade/MigrateJournalNotes.php +++ b/app/Console/Commands/Upgrade/MigrateJournalNotes.php @@ -110,7 +110,7 @@ class MigrateJournalNotes extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore diff --git a/app/Console/Commands/Upgrade/MigrateRecurrenceMeta.php b/app/Console/Commands/Upgrade/MigrateRecurrenceMeta.php index 396a2a27b7..8ec5d2b47e 100644 --- a/app/Console/Commands/Upgrade/MigrateRecurrenceMeta.php +++ b/app/Console/Commands/Upgrade/MigrateRecurrenceMeta.php @@ -81,7 +81,7 @@ class MigrateRecurrenceMeta extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore @@ -103,7 +103,7 @@ class MigrateRecurrenceMeta extends Command */ private function migrateEntry(RecurrenceMeta $meta): int { - $recurrence = $meta->recurrence; + $recurrence = $meta->recurrence; if (null === $recurrence) { return 0; } diff --git a/app/Console/Commands/Upgrade/MigrateTagLocations.php b/app/Console/Commands/Upgrade/MigrateTagLocations.php index eec8699002..9989e10b4e 100644 --- a/app/Console/Commands/Upgrade/MigrateTagLocations.php +++ b/app/Console/Commands/Upgrade/MigrateTagLocations.php @@ -84,7 +84,7 @@ class MigrateTagLocations extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore diff --git a/app/Console/Commands/Upgrade/MigrateToGroups.php b/app/Console/Commands/Upgrade/MigrateToGroups.php index c0f1c68f94..f953b784ed 100644 --- a/app/Console/Commands/Upgrade/MigrateToGroups.php +++ b/app/Console/Commands/Upgrade/MigrateToGroups.php @@ -59,21 +59,21 @@ class MigrateToGroups extends Command * @var string */ protected $signature = 'firefly-iii:migrate-to-groups {--F|force : Force the migration, even if it fired before.}'; + /** @var JournalCLIRepositoryInterface */ + private $cliRepository; + private $count; /** @var TransactionGroupFactory */ private $groupFactory; /** @var JournalRepositoryInterface */ private $journalRepository; - /** @var JournalCLIRepositoryInterface */ - private $cliRepository; /** @var JournalDestroyService */ private $service; - private $count; /** * Execute the console command. * - * @return int * @throws Exception + * @return int */ public function handle(): int { @@ -116,25 +116,9 @@ class MigrateToGroups extends Command return 0; } - /** - * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is - * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should - * be called from the handle method instead of using the constructor to initialize the command. - * - * @codeCoverageIgnore - */ - private function stupidLaravel(): void - { - $this->count = 0; - $this->journalRepository = app(JournalRepositoryInterface::class); - $this->service = app(JournalDestroyService::class); - $this->groupFactory = app(TransactionGroupFactory::class); - $this->cliRepository = app(JournalCLIRepositoryInterface::class); - } - /** * @param TransactionJournal $journal - * @param Transaction $transaction + * @param Transaction $transaction * * @return Transaction|null */ @@ -142,7 +126,7 @@ class MigrateToGroups extends Command { $set = $journal->transactions->filter( static function (Transaction $subject) use ($transaction) { - $amount = (float)$transaction->amount * -1 === (float)$subject->amount; + $amount = (float) $transaction->amount * -1 === (float) $subject->amount; $identifier = $transaction->identifier === $subject->identifier; Log::debug(sprintf('Amount the same? %s', var_export($amount, true))); Log::debug(sprintf('ID the same? %s', var_export($identifier, true))); @@ -168,6 +152,72 @@ class MigrateToGroups extends Command ); } + /** + * @param Transaction $left + * @param Transaction $right + * + * @return int|null + */ + private function getTransactionBudget(Transaction $left, Transaction $right): ?int + { + Log::debug('Now in getTransactionBudget()'); + + // try to get a budget ID from the left transaction: + /** @var Budget $budget */ + $budget = $left->budgets()->first(); + if (null !== $budget) { + Log::debug(sprintf('Return budget #%d, from transaction #%d', $budget->id, $left->id)); + + return (int) $budget->id; + } + + // try to get a budget ID from the right transaction: + /** @var Budget $budget */ + $budget = $right->budgets()->first(); + if (null !== $budget) { + Log::debug(sprintf('Return budget #%d, from transaction #%d', $budget->id, $right->id)); + + return (int) $budget->id; + } + Log::debug('Neither left or right have a budget, return NULL'); + + // if all fails, return NULL. + return null; + } + + /** + * @param Transaction $left + * @param Transaction $right + * + * @return int|null + */ + private function getTransactionCategory(Transaction $left, Transaction $right): ?int + { + Log::debug('Now in getTransactionCategory()'); + + // try to get a category ID from the left transaction: + /** @var Category $category */ + $category = $left->categories()->first(); + if (null !== $category) { + Log::debug(sprintf('Return category #%d, from transaction #%d', $category->id, $left->id)); + + return (int) $category->id; + } + + // try to get a category ID from the left transaction: + /** @var Category $category */ + $category = $right->categories()->first(); + if (null !== $category) { + Log::debug(sprintf('Return category #%d, from transaction #%d', $category->id, $category->id)); + + return (int) $category->id; + } + Log::debug('Neither left or right have a category, return NULL'); + + // if all fails, return NULL. + return null; + } + /** * @param array $array */ @@ -192,7 +242,7 @@ class MigrateToGroups extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore @@ -383,72 +433,6 @@ class MigrateToGroups extends Command ); } - /** - * @param Transaction $left - * @param Transaction $right - * - * @return int|null - */ - private function getTransactionBudget(Transaction $left, Transaction $right): ?int - { - Log::debug('Now in getTransactionBudget()'); - - // try to get a budget ID from the left transaction: - /** @var Budget $budget */ - $budget = $left->budgets()->first(); - if (null !== $budget) { - Log::debug(sprintf('Return budget #%d, from transaction #%d', $budget->id, $left->id)); - - return (int)$budget->id; - } - - // try to get a budget ID from the right transaction: - /** @var Budget $budget */ - $budget = $right->budgets()->first(); - if (null !== $budget) { - Log::debug(sprintf('Return budget #%d, from transaction #%d', $budget->id, $right->id)); - - return (int)$budget->id; - } - Log::debug('Neither left or right have a budget, return NULL'); - - // if all fails, return NULL. - return null; - } - - /** - * @param Transaction $left - * @param Transaction $right - * - * @return int|null - */ - private function getTransactionCategory(Transaction $left, Transaction $right): ?int - { - Log::debug('Now in getTransactionCategory()'); - - // try to get a category ID from the left transaction: - /** @var Category $category */ - $category = $left->categories()->first(); - if (null !== $category) { - Log::debug(sprintf('Return category #%d, from transaction #%d', $category->id, $left->id)); - - return (int)$category->id; - } - - // try to get a category ID from the left transaction: - /** @var Category $category */ - $category = $right->categories()->first(); - if (null !== $category) { - Log::debug(sprintf('Return category #%d, from transaction #%d', $category->id, $category->id)); - - return (int)$category->id; - } - Log::debug('Neither left or right have a category, return NULL'); - - // if all fails, return NULL. - return null; - } - /** * */ @@ -456,4 +440,20 @@ class MigrateToGroups extends Command { app('fireflyconfig')->set(self::CONFIG_NAME, true); } + + /** + * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is + * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should + * be called from the handle method instead of using the constructor to initialize the command. + * + * @codeCoverageIgnore + */ + private function stupidLaravel(): void + { + $this->count = 0; + $this->journalRepository = app(JournalRepositoryInterface::class); + $this->service = app(JournalDestroyService::class); + $this->groupFactory = app(TransactionGroupFactory::class); + $this->cliRepository = app(JournalCLIRepositoryInterface::class); + } } diff --git a/app/Console/Commands/Upgrade/MigrateToRules.php b/app/Console/Commands/Upgrade/MigrateToRules.php index f1ecbd56ec..d751ffa7cd 100644 --- a/app/Console/Commands/Upgrade/MigrateToRules.php +++ b/app/Console/Commands/Upgrade/MigrateToRules.php @@ -53,22 +53,21 @@ class MigrateToRules extends Command * @var string */ protected $signature = 'firefly-iii:bills-to-rules {--F|force : Force the execution of this command.}'; - - /** @var UserRepositoryInterface */ - private $userRepository; - /** @var RuleGroupRepositoryInterface */ - private $ruleGroupRepository; /** @var BillRepositoryInterface */ private $billRepository; + private $count; + /** @var RuleGroupRepositoryInterface */ + private $ruleGroupRepository; /** @var RuleRepositoryInterface */ private $ruleRepository; - private $count; + /** @var UserRepositoryInterface */ + private $userRepository; /** * Execute the console command. * - * @return int * @throws FireflyException + * @return int */ public function handle(): int { @@ -103,22 +102,6 @@ class MigrateToRules extends Command return 0; } - /** - * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is - * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should - * be called from the handle method instead of using the constructor to initialize the command. - * - * @codeCoverageIgnore - */ - private function stupidLaravel(): void - { - $this->count = 0; - $this->userRepository = app(UserRepositoryInterface::class); - $this->ruleGroupRepository = app(RuleGroupRepositoryInterface::class); - $this->billRepository = app(BillRepositoryInterface::class); - $this->ruleRepository = app(RuleRepositoryInterface::class); - } - /** * @return bool */ @@ -126,7 +109,7 @@ class MigrateToRules extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore @@ -140,42 +123,6 @@ class MigrateToRules extends Command app('fireflyconfig')->set(self::CONFIG_NAME, true); } - /** - * Migrate bills to new rule structure for a specific user. - * - * @param User $user - * @throws FireflyException - */ - private function migrateUser(User $user): void - { - $this->ruleGroupRepository->setUser($user); - $this->billRepository->setUser($user); - $this->ruleRepository->setUser($user); - - /** @var Preference $lang */ - $lang = app('preferences')->getForUser($user, 'language', 'en_US'); - $groupTitle = (string)trans('firefly.rulegroup_for_bills_title', [], $lang->data); - $ruleGroup = $this->ruleGroupRepository->findByTitle($groupTitle); - //$currency = $this->getCurrency($user); - - if (null === $ruleGroup) { - $ruleGroup = $this->ruleGroupRepository->store( - [ - 'title' => (string)trans('firefly.rulegroup_for_bills_title', [], $lang->data), - 'description' => (string)trans('firefly.rulegroup_for_bills_description', [], $lang->data), - 'active' => true, - ] - ); - } - $bills = $this->billRepository->getBills(); - - /** @var Bill $bill */ - foreach ($bills as $bill) { - $this->migrateBill($ruleGroup, $bill, $lang); - } - - } - /** * @param RuleGroup $ruleGroup * @param Bill $bill @@ -194,8 +141,8 @@ class MigrateToRules extends Command 'active' => true, 'strict' => false, 'stop_processing' => false, // field is no longer used. - 'title' => (string)trans('firefly.rule_for_bill_title', ['name' => $bill->name], $language->data), - 'description' => (string)trans('firefly.rule_for_bill_description', ['name' => $bill->name], $language->data), + 'title' => (string) trans('firefly.rule_for_bill_title', ['name' => $bill->name], $language->data), + 'description' => (string) trans('firefly.rule_for_bill_description', ['name' => $bill->name], $language->data), 'trigger' => 'store-journal', 'triggers' => [ [ @@ -246,4 +193,57 @@ class MigrateToRules extends Command $this->billRepository->update($bill, $newBillData); $this->count++; } + + /** + * Migrate bills to new rule structure for a specific user. + * + * @param User $user + * + * @throws FireflyException + */ + private function migrateUser(User $user): void + { + $this->ruleGroupRepository->setUser($user); + $this->billRepository->setUser($user); + $this->ruleRepository->setUser($user); + + /** @var Preference $lang */ + $lang = app('preferences')->getForUser($user, 'language', 'en_US'); + $groupTitle = (string) trans('firefly.rulegroup_for_bills_title', [], $lang->data); + $ruleGroup = $this->ruleGroupRepository->findByTitle($groupTitle); + //$currency = $this->getCurrency($user); + + if (null === $ruleGroup) { + $ruleGroup = $this->ruleGroupRepository->store( + [ + 'title' => (string) trans('firefly.rulegroup_for_bills_title', [], $lang->data), + 'description' => (string) trans('firefly.rulegroup_for_bills_description', [], $lang->data), + 'active' => true, + ] + ); + } + $bills = $this->billRepository->getBills(); + + /** @var Bill $bill */ + foreach ($bills as $bill) { + $this->migrateBill($ruleGroup, $bill, $lang); + } + + } + + /** + * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is + * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should + * be called from the handle method instead of using the constructor to initialize the command. + * + * @codeCoverageIgnore + */ + private function stupidLaravel(): void + { + $this->count = 0; + $this->userRepository = app(UserRepositoryInterface::class); + $this->ruleGroupRepository = app(RuleGroupRepositoryInterface::class); + $this->billRepository = app(BillRepositoryInterface::class); + $this->ruleRepository = app(RuleRepositoryInterface::class); + } } diff --git a/app/Console/Commands/Upgrade/OtherCurrenciesCorrections.php b/app/Console/Commands/Upgrade/OtherCurrenciesCorrections.php index 7a6a37e5b9..2c40887fc9 100644 --- a/app/Console/Commands/Upgrade/OtherCurrenciesCorrections.php +++ b/app/Console/Commands/Upgrade/OtherCurrenciesCorrections.php @@ -57,14 +57,14 @@ class OtherCurrenciesCorrections extends Command private $accountCurrencies; /** @var AccountRepositoryInterface */ private $accountRepos; - /** @var CurrencyRepositoryInterface */ - private $currencyRepos; - /** @var JournalRepositoryInterface */ - private $journalRepos; /** @var JournalCLIRepositoryInterface */ private $cliRepos; /** @var int */ private $count; + /** @var CurrencyRepositoryInterface */ + private $currencyRepos; + /** @var JournalRepositoryInterface */ + private $journalRepos; /** * Execute the console command. @@ -93,23 +93,6 @@ class OtherCurrenciesCorrections extends Command return 0; } - /** - * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is - * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should - * be called from the handle method instead of using the constructor to initialize the command. - * - * @codeCoverageIgnore - */ - private function stupidLaravel(): void - { - $this->count = 0; - $this->accountCurrencies = []; - $this->accountRepos = app(AccountRepositoryInterface::class); - $this->currencyRepos = app(CurrencyRepositoryInterface::class); - $this->journalRepos = app(JournalRepositoryInterface::class); - $this->cliRepos = app(JournalCLIRepositoryInterface::class); - } - /** * @param Account $account * @@ -137,112 +120,6 @@ class OtherCurrenciesCorrections extends Command return $currency; } - /** - * @return bool - */ - private function isExecuted(): bool - { - $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); - if (null !== $configVar) { - return (bool)$configVar->data; - } - - return false; // @codeCoverageIgnore - } - - /** - * - */ - private function markAsExecuted(): void - { - app('fireflyconfig')->set(self::CONFIG_NAME, true); - } - - /** - * @param TransactionJournal $journal - */ - private function updateJournalCurrency(TransactionJournal $journal): void - { - $this->accountRepos->setUser($journal->user); - $this->journalRepos->setUser($journal->user); - $this->currencyRepos->setUser($journal->user); - $this->cliRepos->setUser($journal->user); - - $leadTransaction = $this->getLeadTransaction($journal); - - if (null === $leadTransaction) { - // @codeCoverageIgnoreStart - $this->error(sprintf('Could not reliably determine which transaction is in the lead for transaction journal #%d.', $journal->id)); - - return; - // @codeCoverageIgnoreEnd - } - - /** @var Account $account */ - $account = $leadTransaction->account; - $currency = $this->getCurrency($account); - if (null === $currency) { - // @codeCoverageIgnoreStart - $this->error(sprintf( - 'Account #%d ("%s") has no currency preference, so transaction journal #%d can\'t be corrected', - $account->id, - $account->name, - $journal->id - )); - $this->count++; - - return; - // @codeCoverageIgnoreEnd - } - // fix each transaction: - $journal->transactions->each( - static function (Transaction $transaction) use ($currency) { - if (null === $transaction->transaction_currency_id) { - $transaction->transaction_currency_id = $currency->id; - $transaction->save(); - } - - // when mismatch in transaction: - if (!((int)$transaction->transaction_currency_id === (int)$currency->id)) { - $transaction->foreign_currency_id = (int)$transaction->transaction_currency_id; - $transaction->foreign_amount = $transaction->amount; - $transaction->transaction_currency_id = $currency->id; - $transaction->save(); - } - } - ); - // also update the journal, of course: - $journal->transaction_currency_id = $currency->id; - $this->count++; - $journal->save(); - } - - /** - * This routine verifies that withdrawals, deposits and opening balances have the correct currency settings for - * the accounts they are linked to. - * - * Both source and destination must match the respective currency preference of the related asset account. - * So FF3 must verify all transactions. - * - */ - private function updateOtherJournalsCurrencies(): void - { - $set = - $this->cliRepos->getAllJournals( - [ - TransactionType::WITHDRAWAL, - TransactionType::DEPOSIT, - TransactionType::OPENING_BALANCE, - TransactionType::RECONCILIATION, - ] - ); - - /** @var TransactionJournal $journal */ - foreach ($set as $journal) { - $this->updateJournalCurrency($journal); - } - } - /** * Gets the transaction that determines the transaction that "leads" and will determine * the currency to be used by all transactions, and the journal itself. @@ -282,4 +159,129 @@ class OtherCurrenciesCorrections extends Command return $lead; } + + /** + * @return bool + */ + private function isExecuted(): bool + { + $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); + if (null !== $configVar) { + return (bool) $configVar->data; + } + + return false; // @codeCoverageIgnore + } + + /** + * + */ + private function markAsExecuted(): void + { + app('fireflyconfig')->set(self::CONFIG_NAME, true); + } + + /** + * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is + * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should + * be called from the handle method instead of using the constructor to initialize the command. + * + * @codeCoverageIgnore + */ + private function stupidLaravel(): void + { + $this->count = 0; + $this->accountCurrencies = []; + $this->accountRepos = app(AccountRepositoryInterface::class); + $this->currencyRepos = app(CurrencyRepositoryInterface::class); + $this->journalRepos = app(JournalRepositoryInterface::class); + $this->cliRepos = app(JournalCLIRepositoryInterface::class); + } + + /** + * @param TransactionJournal $journal + */ + private function updateJournalCurrency(TransactionJournal $journal): void + { + $this->accountRepos->setUser($journal->user); + $this->journalRepos->setUser($journal->user); + $this->currencyRepos->setUser($journal->user); + $this->cliRepos->setUser($journal->user); + + $leadTransaction = $this->getLeadTransaction($journal); + + if (null === $leadTransaction) { + // @codeCoverageIgnoreStart + $this->error(sprintf('Could not reliably determine which transaction is in the lead for transaction journal #%d.', $journal->id)); + + return; + // @codeCoverageIgnoreEnd + } + + /** @var Account $account */ + $account = $leadTransaction->account; + $currency = $this->getCurrency($account); + if (null === $currency) { + // @codeCoverageIgnoreStart + $this->error( + sprintf( + 'Account #%d ("%s") has no currency preference, so transaction journal #%d can\'t be corrected', + $account->id, + $account->name, + $journal->id + ) + ); + $this->count++; + + return; + // @codeCoverageIgnoreEnd + } + // fix each transaction: + $journal->transactions->each( + static function (Transaction $transaction) use ($currency) { + if (null === $transaction->transaction_currency_id) { + $transaction->transaction_currency_id = $currency->id; + $transaction->save(); + } + + // when mismatch in transaction: + if (!((int) $transaction->transaction_currency_id === (int) $currency->id)) { + $transaction->foreign_currency_id = (int) $transaction->transaction_currency_id; + $transaction->foreign_amount = $transaction->amount; + $transaction->transaction_currency_id = $currency->id; + $transaction->save(); + } + } + ); + // also update the journal, of course: + $journal->transaction_currency_id = $currency->id; + $this->count++; + $journal->save(); + } + + /** + * This routine verifies that withdrawals, deposits and opening balances have the correct currency settings for + * the accounts they are linked to. + * + * Both source and destination must match the respective currency preference of the related asset account. + * So FF3 must verify all transactions. + * + */ + private function updateOtherJournalsCurrencies(): void + { + $set + = $this->cliRepos->getAllJournals( + [ + TransactionType::WITHDRAWAL, + TransactionType::DEPOSIT, + TransactionType::OPENING_BALANCE, + TransactionType::RECONCILIATION, + ] + ); + + /** @var TransactionJournal $journal */ + foreach ($set as $journal) { + $this->updateJournalCurrency($journal); + } + } } diff --git a/app/Console/Commands/Upgrade/RenameAccountMeta.php b/app/Console/Commands/Upgrade/RenameAccountMeta.php index fef408cc3a..6a6e369a8d 100644 --- a/app/Console/Commands/Upgrade/RenameAccountMeta.php +++ b/app/Console/Commands/Upgrade/RenameAccountMeta.php @@ -101,7 +101,7 @@ class RenameAccountMeta extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore diff --git a/app/Console/Commands/Upgrade/TransactionIdentifier.php b/app/Console/Commands/Upgrade/TransactionIdentifier.php index 72861c099b..a60db0b924 100644 --- a/app/Console/Commands/Upgrade/TransactionIdentifier.php +++ b/app/Console/Commands/Upgrade/TransactionIdentifier.php @@ -50,15 +50,12 @@ class TransactionIdentifier extends Command * @var string */ protected $signature = 'firefly-iii:transaction-identifiers {--F|force : Force the execution of this command.}'; - - /** @var JournalRepositoryInterface */ - private $journalRepository; - /** @var JournalCLIRepositoryInterface */ private $cliRepository; - /** @var int */ private $count; + /** @var JournalRepositoryInterface */ + private $journalRepository; /** * This method gives all transactions which are part of a split journal (so more than 2) a sort of "order" so they are easier @@ -108,17 +105,36 @@ class TransactionIdentifier extends Command } /** - * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is - * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should - * be called from the handle method instead of using the constructor to initialize the command. + * @param Transaction $transaction + * @param array $exclude * - * @codeCoverageIgnore + * @return Transaction|null */ - private function stupidLaravel(): void + private function findOpposing(Transaction $transaction, array $exclude): ?Transaction { - $this->journalRepository = app(JournalRepositoryInterface::class); - $this->cliRepository = app(JournalCLIRepositoryInterface::class); - $this->count = 0; + // find opposing: + $amount = bcmul((string) $transaction->amount, '-1'); + + try { + /** @var Transaction $opposing */ + $opposing = Transaction::where('transaction_journal_id', $transaction->transaction_journal_id) + ->where('amount', $amount)->where('identifier', '=', 0) + ->whereNotIn('id', $exclude) + ->first(); + // @codeCoverageIgnoreStart + } catch (QueryException $e) { + Log::error($e->getMessage()); + $this->error('Firefly III could not find the "identifier" field in the "transactions" table.'); + $this->error(sprintf('This field is required for Firefly III version %s to run.', config('firefly.version'))); + $this->error('Please run "php artisan migrate" to add this field to the table.'); + $this->info('Then, run "php artisan firefly:upgrade-database" to try again.'); + + return null; + } + + // @codeCoverageIgnoreEnd + + return $opposing; } /** @@ -128,7 +144,7 @@ class TransactionIdentifier extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore @@ -142,6 +158,20 @@ class TransactionIdentifier extends Command app('fireflyconfig')->set(self::CONFIG_NAME, true); } + /** + * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is + * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should + * be called from the handle method instead of using the constructor to initialize the command. + * + * @codeCoverageIgnore + */ + private function stupidLaravel(): void + { + $this->journalRepository = app(JournalRepositoryInterface::class); + $this->cliRepository = app(JournalCLIRepositoryInterface::class); + $this->count = 0; + } + /** * Grab all positive transactions from this journal that are not deleted. for each one, grab the negative opposing one * which has 0 as an identifier and give it the same identifier. @@ -171,36 +201,4 @@ class TransactionIdentifier extends Command } } - - /** - * @param Transaction $transaction - * @param array $exclude - * @return Transaction|null - */ - private function findOpposing(Transaction $transaction, array $exclude): ?Transaction - { - // find opposing: - $amount = bcmul((string)$transaction->amount, '-1'); - - try { - /** @var Transaction $opposing */ - $opposing = Transaction::where('transaction_journal_id', $transaction->transaction_journal_id) - ->where('amount', $amount)->where('identifier', '=', 0) - ->whereNotIn('id', $exclude) - ->first(); - // @codeCoverageIgnoreStart - } catch (QueryException $e) { - Log::error($e->getMessage()); - $this->error('Firefly III could not find the "identifier" field in the "transactions" table.'); - $this->error(sprintf('This field is required for Firefly III version %s to run.', config('firefly.version'))); - $this->error('Please run "php artisan migrate" to add this field to the table.'); - $this->info('Then, run "php artisan firefly:upgrade-database" to try again.'); - - return null; - } - - // @codeCoverageIgnoreEnd - - return $opposing; - } } diff --git a/app/Console/Commands/Upgrade/TransferCurrenciesCorrections.php b/app/Console/Commands/Upgrade/TransferCurrenciesCorrections.php index 13a674112a..90023cbb92 100644 --- a/app/Console/Commands/Upgrade/TransferCurrenciesCorrections.php +++ b/app/Console/Commands/Upgrade/TransferCurrenciesCorrections.php @@ -57,27 +57,26 @@ class TransferCurrenciesCorrections extends Command private $accountCurrencies; /** @var AccountRepositoryInterface */ private $accountRepos; - /** @var CurrencyRepositoryInterface */ - private $currencyRepos; - /** @var JournalRepositoryInterface */ - private $journalRepos; /** @var JournalCLIRepositoryInterface */ private $cliRepos; /** @var int */ private $count; - - /** @var Transaction The source transaction of the current journal. */ - private $sourceTransaction; - /** @var Account The source account of the current journal. */ - private $sourceAccount; - /** @var TransactionCurrency The currency preference of the source account of the current journal. */ - private $sourceCurrency; - /** @var Transaction The destination transaction of the current journal. */ - private $destinationTransaction; + /** @var CurrencyRepositoryInterface */ + private $currencyRepos; /** @var Account The destination account of the current journal. */ private $destinationAccount; /** @var TransactionCurrency The currency preference of the destination account of the current journal. */ private $destinationCurrency; + /** @var Transaction The destination transaction of the current journal. */ + private $destinationTransaction; + /** @var JournalRepositoryInterface */ + private $journalRepos; + /** @var Account The source account of the current journal. */ + private $sourceAccount; + /** @var TransactionCurrency The currency preference of the source account of the current journal. */ + private $sourceCurrency; + /** @var Transaction The source transaction of the current journal. */ + private $sourceTransaction; /** * Execute the console command. @@ -116,21 +115,216 @@ class TransferCurrenciesCorrections extends Command } /** - * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is - * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should - * be called from the handle method instead of using the constructor to initialize the command. - * - * @codeCoverageIgnore + * The destination transaction must have a currency. If not, it will be added by + * taking it from the destination account's preference. */ - private function stupidLaravel(): void + private function fixDestNoCurrency(): void { - $this->count = 0; - $this->accountRepos = app(AccountRepositoryInterface::class); - $this->currencyRepos = app(CurrencyRepositoryInterface::class); - $this->journalRepos = app(JournalRepositoryInterface::class); - $this->cliRepos = app(JournalCLIRepositoryInterface::class); - $this->accountCurrencies = []; - $this->resetInformation(); + if (null === $this->destinationTransaction->transaction_currency_id && null !== $this->destinationCurrency) { + $this->destinationTransaction + ->transaction_currency_id + = (int) $this->destinationCurrency->id; + $message = sprintf( + 'Transaction #%d has no currency setting, now set to %s.', + $this->destinationTransaction->id, + $this->destinationCurrency->code + ); + Log::debug($message); + $this->line($message); + $this->count++; + $this->destinationTransaction->save(); + } + } + + /** + * If the foreign amount of the destination transaction is null, but that of the other isn't, use this piece of code + * to restore it. + */ + private function fixDestNullForeignAmount(): void + { + if (null === $this->destinationTransaction->foreign_amount && null !== $this->sourceTransaction->foreign_amount) { + $this->destinationTransaction->foreign_amount = bcmul((string) $this->sourceTransaction->foreign_amount, '-1'); + $this->destinationTransaction->save(); + $this->count++; + Log::debug( + sprintf( + 'Restored foreign amount of destination transaction #%d to %s', + $this->destinationTransaction->id, + $this->destinationTransaction->foreign_amount + ) + ); + } + } + + /** + * The destination transaction must have the correct currency. If not, it will be set by + * taking it from the destination account's preference. + */ + private function fixDestinationUnmatchedCurrency(): void + { + if (null !== $this->destinationCurrency + && null === $this->destinationTransaction->foreign_amount + && (int) $this->destinationTransaction->transaction_currency_id !== (int) $this->destinationCurrency->id + ) { + $message = sprintf( + 'Transaction #%d has a currency setting #%d that should be #%d. Amount remains %s, currency is changed.', + $this->destinationTransaction->id, + $this->destinationTransaction->transaction_currency_id, + $this->destinationAccount->id, + $this->destinationTransaction->amount + ); + Log::debug($message); + $this->line($message); + $this->count++; + $this->destinationTransaction->transaction_currency_id = (int) $this->destinationCurrency->id; + $this->destinationTransaction->save(); + } + } + + /** + * If the destination account currency is the same as the source currency, + * both foreign_amount and foreign_currency_id fields must be NULL + * for both transactions (because foreign currency info would not make sense) + * + */ + private function fixInvalidForeignCurrency(): void + { + if ((int) $this->destinationCurrency->id === (int) $this->sourceCurrency->id) { + // update both transactions to match: + $this->sourceTransaction->foreign_amount = null; + $this->sourceTransaction->foreign_currency_id = null; + + $this->destinationTransaction->foreign_amount = null; + $this->destinationTransaction->foreign_currency_id = null; + + $this->sourceTransaction->save(); + $this->destinationTransaction->save(); + + Log::debug( + sprintf( + 'Currency for account "%s" is %s, and currency for account "%s" is also + %s, so transactions #%d and #%d has been verified to be to %s exclusively.', + $this->destinationAccount->name, + $this->destinationCurrency->code, + $this->sourceAccount->name, + $this->sourceCurrency->code, + $this->sourceTransaction->id, + $this->destinationTransaction->id, + $this->sourceCurrency->code + ) + ); + } + } + + /** + * If destination account currency is different from source account currency, + * then both transactions must get the source account's currency as normal currency + * and the opposing account's currency as foreign currency. + */ + private function fixMismatchedForeignCurrency(): void + { + if ((int) $this->sourceCurrency->id !== (int) $this->destinationCurrency->id) { + $this->sourceTransaction->transaction_currency_id = $this->sourceCurrency->id; + $this->sourceTransaction->foreign_currency_id = $this->destinationCurrency->id; + $this->destinationTransaction->transaction_currency_id = $this->sourceCurrency->id; + $this->destinationTransaction->foreign_currency_id = $this->destinationCurrency->id; + + $this->sourceTransaction->save(); + $this->destinationTransaction->save(); + $this->count++; + Log::debug(sprintf('Verified foreign currency ID of transaction #%d and #%d', $this->sourceTransaction->id, $this->destinationTransaction->id)); + } + } + + /** + * The source transaction must have a currency. If not, it will be added by + * taking it from the source account's preference. + */ + private function fixSourceNoCurrency(): void + { + if (null === $this->sourceTransaction->transaction_currency_id && null !== $this->sourceCurrency) { + $this->sourceTransaction + ->transaction_currency_id + = (int) $this->sourceCurrency->id; + $message = sprintf( + 'Transaction #%d has no currency setting, now set to %s.', + $this->sourceTransaction->id, + $this->sourceCurrency->code + ); + Log::debug($message); + $this->line($message); + $this->count++; + $this->sourceTransaction->save(); + } + } + + /** + * If the foreign amount of the source transaction is null, but that of the other isn't, use this piece of code + * to restore it. + */ + private function fixSourceNullForeignAmount(): void + { + if (null === $this->sourceTransaction->foreign_amount && null !== $this->destinationTransaction->foreign_amount) { + $this->sourceTransaction->foreign_amount = bcmul((string) $this->destinationTransaction->foreign_amount, '-1'); + $this->sourceTransaction->save(); + $this->count++; + Log::debug( + sprintf( + 'Restored foreign amount of source transaction #%d to %s', + $this->sourceTransaction->id, + $this->sourceTransaction->foreign_amount + ) + ); + } + } + + /** + * The source transaction must have the correct currency. If not, it will be set by + * taking it from the source account's preference. + */ + private function fixSourceUnmatchedCurrency(): void + { + if (null !== $this->sourceCurrency + && null === $this->sourceTransaction->foreign_amount + && (int) $this->sourceTransaction->transaction_currency_id !== (int) $this->sourceCurrency->id + ) { + $message = sprintf( + 'Transaction #%d has a currency setting #%d that should be #%d. Amount remains %s, currency is changed.', + $this->sourceTransaction->id, + $this->sourceTransaction->transaction_currency_id, + $this->sourceAccount->id, + $this->sourceTransaction->amount + ); + Log::debug($message); + $this->line($message); + $this->count++; + $this->sourceTransaction->transaction_currency_id = (int) $this->sourceCurrency->id; + $this->sourceTransaction->save(); + } + } + + /** + * This method makes sure that the transaction journal uses the currency given in the source transaction. + * + * @param TransactionJournal $journal + */ + private function fixTransactionJournalCurrency(TransactionJournal $journal): void + { + if ((int) $journal->transaction_currency_id !== (int) $this->sourceCurrency->id) { + $oldCurrencyCode = $journal->transactionCurrency->code ?? '(nothing)'; + $journal->transaction_currency_id = $this->sourceCurrency->id; + $message = sprintf( + 'Transfer #%d ("%s") has been updated to use %s instead of %s.', + $journal->id, + $journal->description, + $this->sourceCurrency->code, + $oldCurrencyCode + ); + $this->count++; + $this->line($message); + Log::debug($message); + $journal->save(); + } } /** @@ -148,7 +342,7 @@ class TransferCurrenciesCorrections extends Command return $this->accountCurrencies[$accountId]; // @codeCoverageIgnore } // TODO we can use getAccountCurrency() instead - $currencyId = (int)$this->accountRepos->getMetaValue($account, 'currency_id'); + $currencyId = (int) $this->accountRepos->getMetaValue($account, 'currency_id'); $result = $this->currencyRepos->findNull($currencyId); if (null === $result) { // @codeCoverageIgnoreStart @@ -162,6 +356,20 @@ class TransferCurrenciesCorrections extends Command return $result; } + /** + * Extract destination transaction, destination account + destination account currency from the journal. + * + * @param TransactionJournal $journal + * + * @codeCoverageIgnore + */ + private function getDestinationInformation(TransactionJournal $journal): void + { + $this->destinationTransaction = $this->getDestinationTransaction($journal); + $this->destinationAccount = null === $this->destinationTransaction ? null : $this->destinationTransaction->account; + $this->destinationCurrency = null === $this->destinationAccount ? null : $this->getCurrency($this->destinationAccount); + } + /** * @param TransactionJournal $transfer * @@ -173,6 +381,20 @@ class TransferCurrenciesCorrections extends Command return $transfer->transactions()->where('amount', '>', 0)->first(); } + /** + * Extract source transaction, source account + source account currency from the journal. + * + * @param TransactionJournal $journal + * + * @codeCoverageIgnore + */ + private function getSourceInformation(TransactionJournal $journal): void + { + $this->sourceTransaction = $this->getSourceTransaction($journal); + $this->sourceAccount = null === $this->sourceTransaction ? null : $this->sourceTransaction->account; + $this->sourceCurrency = null === $this->sourceAccount ? null : $this->getCurrency($this->sourceAccount); + } + /** * @param TransactionJournal $transfer * @@ -184,6 +406,19 @@ class TransferCurrenciesCorrections extends Command return $transfer->transactions()->where('amount', '<', 0)->first(); } + /** + * Is either the source or destination transaction NULL? + * + * @return bool + * @codeCoverageIgnore + */ + private function isEmptyTransactions(): bool + { + return null === $this->sourceTransaction || null === $this->destinationTransaction + || null === $this->sourceAccount + || null === $this->destinationAccount; + } + /** * @return bool */ @@ -191,12 +426,56 @@ class TransferCurrenciesCorrections extends Command { $configVar = app('fireflyconfig')->get(self::CONFIG_NAME, false); if (null !== $configVar) { - return (bool)$configVar->data; + return (bool) $configVar->data; } return false; // @codeCoverageIgnore } + /** + * @return bool + * @codeCoverageIgnore + */ + private function isNoCurrencyPresent(): bool + { + // source account must have a currency preference. + if (null === $this->sourceCurrency) { + $message = sprintf('Account #%d ("%s") must have currency preference but has none.', $this->sourceAccount->id, $this->sourceAccount->name); + Log::error($message); + $this->error($message); + + return true; + } + + // destination account must have a currency preference. + if (null === $this->destinationCurrency) { + $message = sprintf( + 'Account #%d ("%s") must have currency preference but has none.', + $this->destinationAccount->id, + $this->destinationAccount->name + ); + Log::error($message); + $this->error($message); + + return true; + } + + return false; + } + + /** + * Is this a split transaction journal? + * + * @param TransactionJournal $transfer + * + * @return bool + * @codeCoverageIgnore + */ + private function isSplitJournal(TransactionJournal $transfer): bool + { + return $transfer->transactions->count() > 2; + } + /** * */ @@ -206,27 +485,18 @@ class TransferCurrenciesCorrections extends Command } /** - * This method makes sure that the transaction journal uses the currency given in the source transaction. + * Reset all the class fields for the current transfer. * - * @param TransactionJournal $journal + * @codeCoverageIgnore */ - private function fixTransactionJournalCurrency(TransactionJournal $journal): void + private function resetInformation(): void { - if ((int)$journal->transaction_currency_id !== (int)$this->sourceCurrency->id) { - $oldCurrencyCode = $journal->transactionCurrency->code ?? '(nothing)'; - $journal->transaction_currency_id = $this->sourceCurrency->id; - $message = sprintf( - 'Transfer #%d ("%s") has been updated to use %s instead of %s.', - $journal->id, - $journal->description, - $this->sourceCurrency->code, - $oldCurrencyCode - ); - $this->count++; - $this->line($message); - Log::debug($message); - $journal->save(); - } + $this->sourceTransaction = null; + $this->sourceAccount = null; + $this->sourceCurrency = null; + $this->destinationTransaction = null; + $this->destinationAccount = null; + $this->destinationCurrency = null; } /** @@ -249,43 +519,21 @@ class TransferCurrenciesCorrections extends Command } /** - * Reset all the class fields for the current transfer. - * @codeCoverageIgnore - */ - private function resetInformation(): void - { - $this->sourceTransaction = null; - $this->sourceAccount = null; - $this->sourceCurrency = null; - $this->destinationTransaction = null; - $this->destinationAccount = null; - $this->destinationCurrency = null; - } - - /** - * Extract source transaction, source account + source account currency from the journal. + * Laravel will execute ALL __construct() methods for ALL commands whenever a SINGLE command is + * executed. This leads to noticeable slow-downs and class calls. To prevent this, this method should + * be called from the handle method instead of using the constructor to initialize the command. * - * @param TransactionJournal $journal * @codeCoverageIgnore */ - private function getSourceInformation(TransactionJournal $journal): void + private function stupidLaravel(): void { - $this->sourceTransaction = $this->getSourceTransaction($journal); - $this->sourceAccount = null === $this->sourceTransaction ? null : $this->sourceTransaction->account; - $this->sourceCurrency = null === $this->sourceAccount ? null : $this->getCurrency($this->sourceAccount); - } - - /** - * Extract destination transaction, destination account + destination account currency from the journal. - * - * @param TransactionJournal $journal - * @codeCoverageIgnore - */ - private function getDestinationInformation(TransactionJournal $journal): void - { - $this->destinationTransaction = $this->getDestinationTransaction($journal); - $this->destinationAccount = null === $this->destinationTransaction ? null : $this->destinationTransaction->account; - $this->destinationCurrency = null === $this->destinationAccount ? null : $this->getCurrency($this->destinationAccount); + $this->count = 0; + $this->accountRepos = app(AccountRepositoryInterface::class); + $this->currencyRepos = app(CurrencyRepositoryInterface::class); + $this->journalRepos = app(JournalRepositoryInterface::class); + $this->cliRepos = app(JournalCLIRepositoryInterface::class); + $this->accountCurrencies = []; + $this->resetInformation(); } /** @@ -352,241 +600,4 @@ class TransferCurrenciesCorrections extends Command // fix journal itself: $this->fixTransactionJournalCurrency($transfer); } - - /** - * The source transaction must have a currency. If not, it will be added by - * taking it from the source account's preference. - */ - private function fixSourceNoCurrency(): void - { - if (null === $this->sourceTransaction->transaction_currency_id && null !== $this->sourceCurrency) { - $this->sourceTransaction - ->transaction_currency_id = (int)$this->sourceCurrency->id; - $message = sprintf( - 'Transaction #%d has no currency setting, now set to %s.', - $this->sourceTransaction->id, - $this->sourceCurrency->code - ); - Log::debug($message); - $this->line($message); - $this->count++; - $this->sourceTransaction->save(); - } - } - - /** - * The destination transaction must have a currency. If not, it will be added by - * taking it from the destination account's preference. - */ - private function fixDestNoCurrency(): void - { - if (null === $this->destinationTransaction->transaction_currency_id && null !== $this->destinationCurrency) { - $this->destinationTransaction - ->transaction_currency_id = (int)$this->destinationCurrency->id; - $message = sprintf( - 'Transaction #%d has no currency setting, now set to %s.', - $this->destinationTransaction->id, - $this->destinationCurrency->code - ); - Log::debug($message); - $this->line($message); - $this->count++; - $this->destinationTransaction->save(); - } - } - - /** - * The source transaction must have the correct currency. If not, it will be set by - * taking it from the source account's preference. - */ - private function fixSourceUnmatchedCurrency(): void - { - if (null !== $this->sourceCurrency && - null === $this->sourceTransaction->foreign_amount && - (int)$this->sourceTransaction->transaction_currency_id !== (int)$this->sourceCurrency->id - ) { - $message = sprintf( - 'Transaction #%d has a currency setting #%d that should be #%d. Amount remains %s, currency is changed.', - $this->sourceTransaction->id, - $this->sourceTransaction->transaction_currency_id, - $this->sourceAccount->id, - $this->sourceTransaction->amount - ); - Log::debug($message); - $this->line($message); - $this->count++; - $this->sourceTransaction->transaction_currency_id = (int)$this->sourceCurrency->id; - $this->sourceTransaction->save(); - } - } - - /** - * The destination transaction must have the correct currency. If not, it will be set by - * taking it from the destination account's preference. - */ - private function fixDestinationUnmatchedCurrency(): void - { - if (null !== $this->destinationCurrency && - null === $this->destinationTransaction->foreign_amount && - (int)$this->destinationTransaction->transaction_currency_id !== (int)$this->destinationCurrency->id - ) { - $message = sprintf( - 'Transaction #%d has a currency setting #%d that should be #%d. Amount remains %s, currency is changed.', - $this->destinationTransaction->id, - $this->destinationTransaction->transaction_currency_id, - $this->destinationAccount->id, - $this->destinationTransaction->amount - ); - Log::debug($message); - $this->line($message); - $this->count++; - $this->destinationTransaction->transaction_currency_id = (int)$this->destinationCurrency->id; - $this->destinationTransaction->save(); - } - } - - /** - * Is this a split transaction journal? - * - * @param TransactionJournal $transfer - * @return bool - * @codeCoverageIgnore - */ - private function isSplitJournal(TransactionJournal $transfer): bool - { - return $transfer->transactions->count() > 2; - } - - /** - * Is either the source or destination transaction NULL? - * @return bool - * @codeCoverageIgnore - */ - private function isEmptyTransactions(): bool - { - return null === $this->sourceTransaction || null === $this->destinationTransaction || - null === $this->sourceAccount || null === $this->destinationAccount; - } - - /** - * If the destination account currency is the same as the source currency, - * both foreign_amount and foreign_currency_id fields must be NULL - * for both transactions (because foreign currency info would not make sense) - * - */ - private function fixInvalidForeignCurrency(): void - { - if ((int)$this->destinationCurrency->id === (int)$this->sourceCurrency->id) { - // update both transactions to match: - $this->sourceTransaction->foreign_amount = null; - $this->sourceTransaction->foreign_currency_id = null; - - $this->destinationTransaction->foreign_amount = null; - $this->destinationTransaction->foreign_currency_id = null; - - $this->sourceTransaction->save(); - $this->destinationTransaction->save(); - - Log::debug( - sprintf( - 'Currency for account "%s" is %s, and currency for account "%s" is also - %s, so transactions #%d and #%d has been verified to be to %s exclusively.', - $this->destinationAccount->name, - $this->destinationCurrency->code, - $this->sourceAccount->name, - $this->sourceCurrency->code, - $this->sourceTransaction->id, - $this->destinationTransaction->id, - $this->sourceCurrency->code - ) - ); - } - } - - /** - * If destination account currency is different from source account currency, - * then both transactions must get the source account's currency as normal currency - * and the opposing account's currency as foreign currency. - */ - private function fixMismatchedForeignCurrency(): void - { - if ((int)$this->sourceCurrency->id !== (int)$this->destinationCurrency->id) { - $this->sourceTransaction->transaction_currency_id = $this->sourceCurrency->id; - $this->sourceTransaction->foreign_currency_id = $this->destinationCurrency->id; - $this->destinationTransaction->transaction_currency_id = $this->sourceCurrency->id; - $this->destinationTransaction->foreign_currency_id = $this->destinationCurrency->id; - - $this->sourceTransaction->save(); - $this->destinationTransaction->save(); - $this->count++; - Log::debug(sprintf('Verified foreign currency ID of transaction #%d and #%d', $this->sourceTransaction->id, $this->destinationTransaction->id)); - } - } - - /** - * If the foreign amount of the source transaction is null, but that of the other isn't, use this piece of code - * to restore it. - */ - private function fixSourceNullForeignAmount(): void - { - if (null === $this->sourceTransaction->foreign_amount && null !== $this->destinationTransaction->foreign_amount) { - $this->sourceTransaction->foreign_amount = bcmul((string)$this->destinationTransaction->foreign_amount, '-1'); - $this->sourceTransaction->save(); - $this->count++; - Log::debug(sprintf( - 'Restored foreign amount of source transaction #%d to %s', - $this->sourceTransaction->id, - $this->sourceTransaction->foreign_amount - )); - } - } - - /** - * If the foreign amount of the destination transaction is null, but that of the other isn't, use this piece of code - * to restore it. - */ - private function fixDestNullForeignAmount(): void - { - if (null === $this->destinationTransaction->foreign_amount && null !== $this->sourceTransaction->foreign_amount) { - $this->destinationTransaction->foreign_amount = bcmul((string)$this->sourceTransaction->foreign_amount, '-1'); - $this->destinationTransaction->save(); - $this->count++; - Log::debug(sprintf( - 'Restored foreign amount of destination transaction #%d to %s', - $this->destinationTransaction->id, - $this->destinationTransaction->foreign_amount - )); - } - } - - /** - * @return bool - * @codeCoverageIgnore - */ - private function isNoCurrencyPresent(): bool - { - // source account must have a currency preference. - if (null === $this->sourceCurrency) { - $message = sprintf('Account #%d ("%s") must have currency preference but has none.', $this->sourceAccount->id, $this->sourceAccount->name); - Log::error($message); - $this->error($message); - - return true; - } - - // destination account must have a currency preference. - if (null === $this->destinationCurrency) { - $message = sprintf( - 'Account #%d ("%s") must have currency preference but has none.', - $this->destinationAccount->id, - $this->destinationAccount->name - ); - Log::error($message); - $this->error($message); - - return true; - } - - return false; - } } diff --git a/app/Console/Commands/Upgrade/UpgradeDatabase.php b/app/Console/Commands/Upgrade/UpgradeDatabase.php index 13b72703b5..9c330c9554 100644 --- a/app/Console/Commands/Upgrade/UpgradeDatabase.php +++ b/app/Console/Commands/Upgrade/UpgradeDatabase.php @@ -30,6 +30,7 @@ use Illuminate\Console\Command; /** * Class UpgradeDatabase + * * @codeCoverageIgnore */ class UpgradeDatabase extends Command @@ -112,9 +113,9 @@ class UpgradeDatabase extends Command echo $result; } // set new DB version. - app('fireflyconfig')->set('db_version', (int)config('firefly.db_version')); + app('fireflyconfig')->set('db_version', (int) config('firefly.db_version')); // index will set FF3 version. - app('fireflyconfig')->set('ff3_version', (string)config('firefly.version')); + app('fireflyconfig')->set('ff3_version', (string) config('firefly.version')); return 0; } diff --git a/app/Console/Commands/UpgradeFireflyInstructions.php b/app/Console/Commands/UpgradeFireflyInstructions.php index ae9f1e5d35..83c7f1ee6d 100644 --- a/app/Console/Commands/UpgradeFireflyInstructions.php +++ b/app/Console/Commands/UpgradeFireflyInstructions.php @@ -50,10 +50,10 @@ class UpgradeFireflyInstructions extends Command */ public function handle(): int { - if ('update' === (string)$this->argument('task')) { + if ('update' === (string) $this->argument('task')) { $this->updateInstructions(); } - if ('install' === (string)$this->argument('task')) { + if ('install' === (string) $this->argument('task')) { $this->installInstructions(); } diff --git a/app/Console/Commands/VerifiesAccessToken.php b/app/Console/Commands/VerifiesAccessToken.php index bbab0837bc..9c7afe03e6 100644 --- a/app/Console/Commands/VerifiesAccessToken.php +++ b/app/Console/Commands/VerifiesAccessToken.php @@ -38,12 +38,12 @@ use Log; trait VerifiesAccessToken { /** - * @return User * @throws FireflyException + * @return User */ public function getUser(): User { - $userId = (int)$this->option('user'); + $userId = (int) $this->option('user'); /** @var UserRepositoryInterface $repository */ $repository = app(UserRepositoryInterface::class); $user = $repository->findNull($userId); @@ -70,8 +70,8 @@ trait VerifiesAccessToken */ protected function verifyAccessToken(): bool { - $userId = (int)$this->option('user'); - $token = (string)$this->option('token'); + $userId = (int) $this->option('user'); + $token = (string) $this->option('token'); /** @var UserRepositoryInterface $repository */ $repository = app(UserRepositoryInterface::class); $user = $repository->findNull($userId);