diff --git a/app/Console/Commands/UpgradeDatabase.php b/app/Console/Commands/UpgradeDatabase.php index 066e4496d3..90d2434d76 100644 --- a/app/Console/Commands/UpgradeDatabase.php +++ b/app/Console/Commands/UpgradeDatabase.php @@ -20,7 +20,6 @@ use FireflyIII\Models\AccountMeta; use FireflyIII\Models\AccountType; use FireflyIII\Models\BudgetLimit; use FireflyIII\Models\LimitRepetition; -use FireflyIII\Models\PiggyBankEvent; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionCurrency; use FireflyIII\Models\TransactionJournal; @@ -71,76 +70,25 @@ class UpgradeDatabase extends Command { $this->setTransactionIdentifier(); $this->migrateRepetitions(); - $this->repairPiggyBanks(); $this->updateAccountCurrencies(); - $this->updateJournalCurrencies(); - $this->currencyInfoToTransactions(); - $this->verifyCurrencyInfo(); + $this->updateTransferCurrencies(); + $this->updateOtherCurrencies(); $this->info('Firefly III database is up to date.'); + return; + + + } /** - * Moves the currency id info to the transaction instead of the journal. - * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) // cannot be helped. + * Migrate budget repetitions to new format where the end date is in the budget limit as well, + * making the limit_repetition table obsolete. */ - private function currencyInfoToTransactions() + public function migrateRepetitions(): void { - $count = 0; - $set = TransactionJournal::with('transactions')->get(); - /** @var TransactionJournal $journal */ - foreach ($set as $journal) { - /** @var Transaction $transaction */ - foreach ($journal->transactions as $transaction) { - if (is_null($transaction->transaction_currency_id)) { - $transaction->transaction_currency_id = $journal->transaction_currency_id; - $transaction->save(); - $count++; - } - } - - // read and use the foreign amounts when present. - if ($journal->hasMeta('foreign_amount')) { - $amount = Steam::positive($journal->getMeta('foreign_amount')); - - // update both transactions: - foreach ($journal->transactions as $transaction) { - $transaction->foreign_amount = $amount; - if (bccomp($transaction->amount, '0') === -1) { - // update with negative amount: - $transaction->foreign_amount = bcmul($amount, '-1'); - } - // set foreign currency id: - $transaction->foreign_currency_id = intval($journal->getMeta('foreign_currency_id')); - $transaction->save(); - } - $journal->deleteMeta('foreign_amount'); - $journal->deleteMeta('foreign_currency_id'); - } - - } - - $this->line(sprintf('Updated currency information for %d transactions', $count)); - } - - /** - * Migrate budget repetitions to new format. - * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) // it's 5. - */ - private function migrateRepetitions() - { - if (!Schema::hasTable('budget_limits')) { - return; - } - // get all budget limits with end_date NULL $set = BudgetLimit::whereNull('end_date')->get(); - if ($set->count() > 0) { - $this->line(sprintf('Found %d budget limit(s) to update', $set->count())); - } /** @var BudgetLimit $budgetLimit */ foreach ($set as $budgetLimit) { - // get limit repetition (should be just one): /** @var LimitRepetition $repetition */ $repetition = $budgetLimit->limitrepetitions()->first(); if (!is_null($repetition)) { @@ -150,59 +98,30 @@ class UpgradeDatabase extends Command $repetition->delete(); } } + + return; } /** - * Make sure there are only transfers linked to piggy bank events. + * This method gives all transactions which are part of a split journal (so more than 2) a sort of "order" so they are easier + * to easier to match to their counterpart. When a journal is split, it has two or three transactions: -3, -4 and -5 for example. * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) // cannot be helped. + * In the database this is reflected as 6 transactions: -3/+3, -4/+4, -5/+5. + * + * When either of these are the same amount, FF3 can't keep them apart: +3/-3, +3/-3, +3/-3. This happens more often than you would + * think. So each set gets a number (1,2,3) to keep them apart. */ - private function repairPiggyBanks() - { - // if table does not exist, return false - if (!Schema::hasTable('piggy_bank_events')) { - return; - } - - $set = PiggyBankEvent::with(['PiggyBank', 'TransactionJournal', 'TransactionJournal.TransactionType'])->get(); - /** @var PiggyBankEvent $event */ - foreach ($set as $event) { - - if (is_null($event->transaction_journal_id)) { - continue; - } - /** @var TransactionJournal $journal */ - $journal = $event->transactionJournal()->first(); - if (is_null($journal)) { - continue; - } - - $type = $journal->transactionType->type; - if ($type !== TransactionType::TRANSFER) { - $event->transaction_journal_id = null; - $event->save(); - $this->line(sprintf('Piggy bank #%d was referenced by an invalid event. This has been fixed.', $event->piggy_bank_id)); - } - } - } - - /** - * This is strangely complex, because the HAVING modifier is a no-no. And subqueries in Laravel are weird. - */ - private function setTransactionIdentifier() + public function setTransactionIdentifier(): void { // if table does not exist, return false if (!Schema::hasTable('transaction_journals')) { return; } - - - $subQuery = TransactionJournal::leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') - ->whereNull('transaction_journals.deleted_at') - ->whereNull('transactions.deleted_at') - ->groupBy(['transaction_journals.id']) - ->select(['transaction_journals.id', DB::raw('COUNT(transactions.id) AS t_count')]); - + $subQuery = TransactionJournal::leftJoin('transactions', 'transactions.transaction_journal_id', '=', 'transaction_journals.id') + ->whereNull('transaction_journals.deleted_at') + ->whereNull('transactions.deleted_at') + ->groupBy(['transaction_journals.id']) + ->select(['transaction_journals.id', DB::raw('COUNT(transactions.id) AS t_count')]); $result = DB::table(DB::raw('(' . $subQuery->toSql() . ') AS derived')) ->mergeBindings($subQuery->getQuery()) ->where('t_count', '>', 2) @@ -210,55 +129,178 @@ class UpgradeDatabase extends Command $journalIds = array_unique($result->pluck('id')->toArray()); foreach ($journalIds as $journalId) { - $this->updateJournal(intval($journalId)); + $this->updateJournalidentifiers(intval($journalId)); } + + return; } /** - * Make sure all accounts have proper currency info. + * Each (asset) account must have a reference to a preferred currency. If the account does not have one, it's forced upon + * the account. */ - private function updateAccountCurrencies() + public function updateAccountCurrencies(): void { $accounts = Account::leftJoin('account_types', 'account_types.id', '=', 'accounts.account_type_id') ->whereIn('account_types.type', [AccountType::DEFAULT, AccountType::ASSET])->get(['accounts.*']); - /** @var Account $account */ - foreach ($accounts as $account) { - // get users preference, fall back to system pref. - $defaultCurrencyCode = Preferences::getForUser($account->user, 'currencyPreference', config('firefly.default_currency', 'EUR'))->data; - $defaultCurrency = TransactionCurrency::where('code', $defaultCurrencyCode)->first(); - $accountCurrency = intval($account->getMeta('currency_id')); - $openingBalance = $account->getOpeningBalance(); - $obCurrency = intval($openingBalance->transaction_currency_id); + $accounts->each( + function (Account $account) { + // get users preference, fall back to system pref. + $defaultCurrencyCode = Preferences::getForUser($account->user, 'currencyPreference', config('firefly.default_currency', 'EUR'))->data; + $defaultCurrency = TransactionCurrency::where('code', $defaultCurrencyCode)->first(); + $accountCurrency = intval($account->getMeta('currency_id')); + $openingBalance = $account->getOpeningBalance(); + $obCurrency = intval($openingBalance->transaction_currency_id); - // both 0? set to default currency: - if ($accountCurrency === 0 && $obCurrency === 0) { - AccountMeta::create(['account_id' => $account->id, 'name' => 'currency_id', 'data' => $defaultCurrency->id]); - $this->line(sprintf('Account #%d ("%s") now has a currency setting (%s).', $account->id, $account->name, $defaultCurrencyCode)); - continue; + // both 0? set to default currency: + if ($accountCurrency === 0 && $obCurrency === 0) { + AccountMeta::create(['account_id' => $account->id, 'name' => 'currency_id', 'data' => $defaultCurrency->id]); + $this->line(sprintf('Account #%d ("%s") now has a currency setting (%s).', $account->id, $account->name, $defaultCurrencyCode)); + + return true; + } + + // account is set to 0, opening balance is not? + if ($accountCurrency === 0 && $obCurrency > 0) { + AccountMeta::create(['account_id' => $account->id, 'name' => 'currency_id', 'data' => $obCurrency]); + $this->line(sprintf('Account #%d ("%s") now has a currency setting (%s).', $account->id, $account->name, $defaultCurrencyCode)); + + return true; + } + + // do not match and opening balance id is not null. + if ($accountCurrency !== $obCurrency && $openingBalance->id > 0) { + // update opening balance: + $openingBalance->transaction_currency_id = $accountCurrency; + $openingBalance->save(); + $this->line(sprintf('Account #%d ("%s") now has a correct currency for opening balance.', $account->id, $account->name)); + + return true; + } + + return true; } + ); - // account is set to 0, opening balance is not? - if ($accountCurrency === 0 && $obCurrency > 0) { - AccountMeta::create(['account_id' => $account->id, 'name' => 'currency_id', 'data' => $obCurrency]); - $this->line(sprintf('Account #%d ("%s") now has a currency setting (%s).', $account->id, $account->name, $defaultCurrencyCode)); - continue; + return; + } + + /** + * 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. + */ + public function updateOtherCurrencies() + { + /** @var CurrencyRepositoryInterface $repository */ + $repository = app(CurrencyRepositoryInterface::class); + $set = TransactionJournal + ::leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') + ->whereIn('transaction_types.type', [TransactionType::WITHDRAWAL, TransactionType::DEPOSIT, TransactionType::OPENING_BALANCE]) + ->get(['transaction_journals.*']); + + $set->each( + function (TransactionJournal $journal) use ($repository) { + // get the transaction with the asset account in it: + /** @var Transaction $transaction */ + $transaction = $journal->transactions() + ->leftJoin('accounts', 'accounts.id', '=', 'transactions.account_id') + ->leftJoin('account_types', 'account_types.id', '=', 'accounts.account_type_id') + ->whereIn('account_types.type', [AccountType::DEFAULT, AccountType::ASSET])->first(['transactions.*']); + /** @var Account $account */ + $account = $transaction->account; + $currency = $repository->find(intval($account->getMeta('currency_id'))); + $transactions = $journal->transactions()->get(); + $transactions->each( + function (Transaction $transaction) use ($currency) { + if (is_null($transaction->transaction_currency_id)) { + $transaction->transaction_currency_id = $currency->id; + $transaction->save(); + $this->line(sprintf('Transaction #%d is set to %s', $transaction->id, $currency->code)); + } + + // when mismatch in transaction: + if ($transaction->transaction_currency_id !== $currency->id) { + $this->line( + sprintf( + 'Transaction #%d is set to %s and foreign %s', $transaction->id, $currency->code, $transaction->transactionCurrency->code + ) + ); + $transaction->foreign_currency_id = $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; + $journal->save(); } + ); + return; + } - // do not match: - if ($accountCurrency !== $obCurrency) { - // update opening balance: - $openingBalance->transaction_currency_id = $accountCurrency; - $openingBalance->save(); - $this->line(sprintf('Account #%d ("%s") now has a correct currency for opening balance.', $account->id, $account->name)); - continue; + /** + * This routine verifies that transfers have the correct currency settings for the accounts they are linked to. + * For transfers, this is can be a destructive routine since we FORCE them into a currency setting whether they + * like it or not. Previous routines MUST have set the currency setting for both accounts for this to work. + * + * Both source and destination must match the respective currency preference. So FF3 must verify ALL + * transactions. + */ + public function updateTransferCurrencies() + { + /** @var CurrencyRepositoryInterface $repository */ + $repository = app(CurrencyRepositoryInterface::class); + $set = TransactionJournal + ::leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') + ->where('transaction_types.type', TransactionType::TRANSFER) + ->get(['transaction_journals.*']); + + $set->each( + function (TransactionJournal $transfer) use ($repository) { + /** @var Transaction $transaction */ + $transaction = $transfer->transactions()->where('amount', '<', 0)->first(); + $this->updateTransactionCurrency($transaction); + $this->updateJournalCurrency($transaction); + + + /** @var Transaction $transaction */ + $transaction = $transfer->transactions()->where('amount', '>', 0)->first(); + $this->updateTransactionCurrency($transaction); } + ); + } - // opening balance 0, account not zero? just continue: - // both are equal, just continue: + /** + * This method makes sure that the transaction journal uses the currency given in the transaction. + * + * @param Transaction $transaction + */ + private function updateJournalCurrency(Transaction $transaction): void + { + /** @var CurrencyRepositoryInterface $repository */ + $repository = app(CurrencyRepositoryInterface::class); + $currency = $repository->find(intval($transaction->account->getMeta('currency_id'))); + $journal = $transaction->transactionJournal; + + if ($currency->id !== $journal->transaction_currency_id) { + $this->line( + sprintf( + 'Transfer #%d ("%s") has been updated to use %s instead of %s.', $journal->id, $journal->description, $currency->code, + $journal->transactionCurrency->code + ) + ); + $journal->transaction_currency_id = $currency->id; + $journal->save(); } + return; } /** @@ -267,7 +309,7 @@ class UpgradeDatabase extends Command * * @param int $journalId */ - private function updateJournal(int $journalId) + private function updateJournalidentifiers(int $journalId): void { $identifier = 0; $processed = []; @@ -295,121 +337,45 @@ class UpgradeDatabase extends Command if (!is_null($opposing)) { // give both a new identifier: $transaction->identifier = $identifier; + $opposing->identifier = $identifier; $transaction->save(); - $opposing->identifier = $identifier; $opposing->save(); $processed[] = $transaction->id; $processed[] = $opposing->id; } $identifier++; } + + return; } /** - * Makes sure that withdrawals, deposits and transfers have - * a currency setting matching their respective accounts - */ - private function updateJournalCurrencies() - { - $types = [ - TransactionType::WITHDRAWAL => '<', - TransactionType::DEPOSIT => '>', - ]; - $repository = app(CurrencyRepositoryInterface::class); - $notification = '%s #%d uses %s but should use %s. It has been updated. Please verify this in Firefly III.'; - $transfer = 'Transfer #%d has been updated to use the correct currencies. Please verify this in Firefly III.'; - $driver = DB::connection()->getDriverName(); - $pgsql = ['pgsql', 'postgresql']; - - foreach ($types as $type => $operator) { - $query = TransactionJournal - ::leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id')->leftJoin( - 'transactions', function (JoinClause $join) use ($operator) { - $join->on('transaction_journals.id', '=', 'transactions.transaction_journal_id')->where('transactions.amount', $operator, '0'); - } - ) - ->leftJoin('accounts', 'accounts.id', '=', 'transactions.account_id') - ->leftJoin('account_meta', 'account_meta.account_id', '=', 'accounts.id') - ->where('transaction_types.type', $type) - ->where('account_meta.name', 'currency_id'); - if (in_array($driver, $pgsql)) { - $query->where('transaction_journals.transaction_currency_id', '!=', DB::raw('cast(account_meta.data as int)')); - } - if (!in_array($driver, $pgsql)) { - $query->where('transaction_journals.transaction_currency_id', '!=', DB::raw('account_meta.data')); - } - - $set = $query->get(['transaction_journals.*', 'account_meta.data as expected_currency_id', 'transactions.amount as transaction_amount']); - /** @var TransactionJournal $journal */ - foreach ($set as $journal) { - $expectedCurrency = $repository->find(intval($journal->expected_currency_id)); - $line = sprintf($notification, $type, $journal->id, $journal->transactionCurrency->code, $expectedCurrency->code); - - $journal->setMeta('foreign_amount', $journal->transaction_amount); - $journal->setMeta('foreign_currency_id', $journal->transaction_currency_id); - $journal->transaction_currency_id = $expectedCurrency->id; - $journal->save(); - $this->line($line); - } - } - /* - * For transfers it's slightly different. Both source and destination - * must match the respective currency preference. So we must verify ALL - * transactions. - */ - $set = TransactionJournal - ::leftJoin('transaction_types', 'transaction_types.id', '=', 'transaction_journals.transaction_type_id') - ->where('transaction_types.type', TransactionType::TRANSFER) - ->get(['transaction_journals.*']); - /** @var TransactionJournal $journal */ - foreach ($set as $journal) { - $updated = false; - /** @var Transaction $sourceTransaction */ - $sourceTransaction = $journal->transactions()->where('amount', '<', 0)->first(); - $sourceCurrency = $repository->find(intval($sourceTransaction->account->getMeta('currency_id'))); - - if ($sourceCurrency->id !== $journal->transaction_currency_id) { - $updated = true; - $journal->transaction_currency_id = $sourceCurrency->id; - $journal->save(); - } - - // destination - $destinationTransaction = $journal->transactions()->where('amount', '>', 0)->first(); - $destinationCurrency = $repository->find(intval($destinationTransaction->account->getMeta('currency_id'))); - - if ($destinationCurrency->id !== $journal->transaction_currency_id) { - $updated = true; - $journal->deleteMeta('foreign_amount'); - $journal->deleteMeta('foreign_currency_id'); - $journal->setMeta('foreign_amount', $destinationTransaction->amount); - $journal->setMeta('foreign_currency_id', $destinationCurrency->id); - } - if ($updated) { - $line = sprintf($transfer, $journal->id); - $this->line($line); - } - } - } - - /** + * This method makes sure that the tranaction uses the same currency as the main account does. + * If not, the currency is updated to include a reference to its original currency as the "foreign" currency. * + * @param Transaction $transaction */ - private function verifyCurrencyInfo() + private function updateTransactionCurrency(Transaction $transaction): void { - $count = 0; - $transactions = Transaction::get(); - /** @var Transaction $transaction */ - foreach ($transactions as $transaction) { - $currencyId = intval($transaction->transaction_currency_id); - $foreignId = intval($transaction->foreign_currency_id); - if ($currencyId === $foreignId) { - $transaction->foreign_currency_id = null; - $transaction->foreign_amount = null; - $transaction->save(); - $count++; - } + /** @var CurrencyRepositoryInterface $repository */ + $repository = app(CurrencyRepositoryInterface::class); + $currency = $repository->find(intval($transaction->account->getMeta('currency_id'))); + + if (is_null($transaction->transaction_currency_id)) { + $transaction->transaction_currency_id = $currency->id; + $transaction->save(); + $this->line(sprintf('Transaction #%d is set to %s', $transaction->id, $currency->code)); } - $this->line(sprintf('Updated currency information for %d transactions', $count)); + + // when mismatch in transaction: + if ($transaction->transaction_currency_id !== $currency->id) { + $this->line(sprintf('Transaction #%d is set to %s and foreign %s', $transaction->id, $currency->code, $transaction->transactionCurrency->code)); + $transaction->foreign_currency_id = $transaction->transaction_currency_id; + $transaction->foreign_amount = $transaction->amount; + $transaction->transaction_currency_id = $currency->id; + $transaction->save(); + } + + return; } } diff --git a/app/Console/Commands/VerifyDatabase.php b/app/Console/Commands/VerifyDatabase.php index 6e128e7f53..ffc8f1b667 100644 --- a/app/Console/Commands/VerifyDatabase.php +++ b/app/Console/Commands/VerifyDatabase.php @@ -17,6 +17,7 @@ use Crypt; use FireflyIII\Models\Account; use FireflyIII\Models\AccountType; use FireflyIII\Models\Budget; +use FireflyIII\Models\PiggyBankEvent; use FireflyIII\Models\Transaction; use FireflyIII\Models\TransactionJournal; use FireflyIII\Models\TransactionType; @@ -94,6 +95,40 @@ class VerifyDatabase extends Command // report on journals with the wrong types of accounts. $this->reportIncorrectJournals(); + // report (and fix) piggy banks + $this->repairPiggyBanks(); + + } + + /** + * Make sure there are only transfers linked to piggy bank events. + */ + private function repairPiggyBanks(): void + { + $set = PiggyBankEvent::with(['PiggyBank', 'TransactionJournal', 'TransactionJournal.TransactionType'])->get(); + $set->each( + function (PiggyBankEvent $event) { + if (is_null($event->transaction_journal_id)) { + return true; + } + /** @var TransactionJournal $journal */ + $journal = $event->transactionJournal()->first(); + if (is_null($journal)) { + return true; + } + + $type = $journal->transactionType->type; + if ($type !== TransactionType::TRANSFER) { + $event->transaction_journal_id = null; + $event->save(); + $this->line(sprintf('Piggy bank #%d was referenced by an invalid event. This has been fixed.', $event->piggy_bank_id)); + } + + return true; + } + ); + + return; } /**