Expand test coverage

This commit is contained in:
James Cole
2019-06-13 07:17:31 +02:00
parent aacd218056
commit 6bcb2ec144
7 changed files with 520 additions and 81 deletions

View File

@@ -70,10 +70,11 @@ class OtherCurrenciesCorrections extends Command
public function __construct()
{
parent::__construct();
$this->count = 0;
$this->accountRepos = app(AccountRepositoryInterface::class);
$this->currencyRepos = app(CurrencyRepositoryInterface::class);
$this->journalRepos = app(JournalRepositoryInterface::class);
$this->count = 0;
$this->accountCurrencies = [];
$this->accountRepos = app(AccountRepositoryInterface::class);
$this->currencyRepos = app(CurrencyRepositoryInterface::class);
$this->journalRepos = app(JournalRepositoryInterface::class);
}
/**
@@ -83,9 +84,6 @@ class OtherCurrenciesCorrections extends Command
*/
public function handle(): int
{
$this->accountCurrencies = [];
$start = microtime(true);
// @codeCoverageIgnoreStart
if ($this->isExecuted() && true !== $this->option('force')) {
@@ -98,12 +96,7 @@ class OtherCurrenciesCorrections extends Command
$this->updateOtherJournalsCurrencies();
$this->markAsExecuted();
if (0 === $this->count) {
$this->line('All transactions are correct.');
}
if (0 !== $this->count) {
$this->line(sprintf('Verified %d transaction(s) and journal(s).', $this->count));
}
$this->line(sprintf('Verified %d transaction(s) and journal(s).', $this->count));
$end = round(microtime(true) - $start, 2);
$this->info(sprintf('Verified and fixed transaction currencies in %s seconds.', $end));
@@ -119,7 +112,7 @@ class OtherCurrenciesCorrections extends Command
{
$accountId = $account->id;
if (isset($this->accountCurrencies[$accountId]) && 0 === $this->accountCurrencies[$accountId]) {
return null;
return null; // @codeCoverageIgnore
}
if (isset($this->accountCurrencies[$accountId]) && $this->accountCurrencies[$accountId] instanceof TransactionCurrency) {
return $this->accountCurrencies[$accountId];
@@ -127,9 +120,11 @@ class OtherCurrenciesCorrections extends Command
$currencyId = (int)$this->accountRepos->getMetaValue($account, 'currency_id');
$result = $this->currencyRepos->findNull($currencyId);
if (null === $result) {
// @codeCoverageIgnoreStart
$this->accountCurrencies[$accountId] = 0;
return null;
// @codeCoverageIgnoreEnd
}
$this->accountCurrencies[$accountId] = $result;
@@ -138,24 +133,6 @@ class OtherCurrenciesCorrections extends Command
}
/**
* @param TransactionJournal $journal
*
* @return Transaction|null
*/
private function getFirstAssetTransaction(TransactionJournal $journal): ?Transaction
{
$result = $journal->transactions->first(
static function (Transaction $transaction) {
// type can also be liability.
return AccountType::ASSET === $transaction->account->accountType->type;
}
);
return $result;
}
/**
* @return bool
*/
@@ -182,22 +159,31 @@ class OtherCurrenciesCorrections extends Command
*/
private function updateJournalCurrency(TransactionJournal $journal): void
{
$this->accountRepos->setUser($journal->user);
$this->journalRepos->setUser($journal->user);
$this->currencyRepos->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(
@@ -205,7 +191,6 @@ class OtherCurrenciesCorrections extends Command
if (null === $transaction->transaction_currency_id) {
$transaction->transaction_currency_id = $currency->id;
$transaction->save();
//$this->count++;
}
// when mismatch in transaction:
@@ -214,7 +199,6 @@ class OtherCurrenciesCorrections extends Command
$transaction->foreign_amount = $transaction->amount;
$transaction->transaction_currency_id = $currency->id;
$transaction->save();
//$this->count++;
}
}
);
@@ -231,8 +215,6 @@ class OtherCurrenciesCorrections extends Command
* Both source and destination must match the respective currency preference of the related asset account.
* So FF3 must verify all transactions.
*
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
private function updateOtherJournalsCurrencies(): void
{

View File

@@ -21,9 +21,9 @@
namespace FireflyIII\Console\Commands\Upgrade;
use DB;
use FireflyIII\Models\Transaction;
use FireflyIII\Models\TransactionJournal;
use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
use Illuminate\Console\Command;
use Illuminate\Database\QueryException;
use Log;
@@ -48,6 +48,19 @@ class TransactionIdentifier extends Command
*/
protected $signature = 'firefly-iii:transaction-identifiers {--F|force : Force the execution of this command.}';
/** @var JournalRepositoryInterface */
private $journalRepository;
/** @var int */
private $count;
public function __construct()
{
parent::__construct();
$this->journalRepository = app(JournalRepositoryInterface::class);
$this->count = 0;
}
/**
* 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.
@@ -61,7 +74,8 @@ class TransactionIdentifier extends Command
*/
public function handle(): int
{
$start = microtime(true);
$start = microtime(true);
// @codeCoverageIgnoreStart
if ($this->isExecuted() && true !== $this->option('force')) {
$this->warn('This command has already been executed.');
@@ -72,24 +86,22 @@ class TransactionIdentifier extends Command
if (!Schema::hasTable('transaction_journals')) {
return 0;
}
$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')]);
/** @noinspection PhpStrictTypeCheckingInspection */
$result = DB::table(DB::raw('(' . $subQuery->toSql() . ') AS derived'))
->mergeBindings($subQuery->getQuery())
->where('t_count', '>', 2)
->select(['id', 't_count']);
$journalIds = array_unique($result->pluck('id')->toArray());
$count= 0;
foreach ($journalIds as $journalId) {
$this->updateJournalIdentifiers((int)$journalId);
$count++;
// @codeCoverageIgnoreEnd
$journals = $this->journalRepository->getSplitJournals();
/** @var TransactionJournal $journal */
foreach ($journals as $journal) {
$this->updateJournalIdentifiers($journal);
}
if (0 === $this->count) {
$this->line('All split journal transaction identifiers are correct.');
}
if (0 !== $this->count) {
$this->line(sprintf('Fixed %d split journal transaction identifier(s).', $this->count));
}
$end = round(microtime(true) - $start, 2);
$this->info(sprintf('Verified and fixed %d transaction identifiers in %s seconds.', $count, $end));
$this->info(sprintf('Verified and fixed transaction identifiers in %s seconds.', $end));
$this->markAsExecuted();
return 0;
@@ -117,47 +129,63 @@ class TransactionIdentifier extends Command
}
/**
* grab all positive transactions from this journal that are not deleted. for each one, grab the negative opposing one
* 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.
*
* @param int $journalId
* @param TransactionJournal $transactionJournal
*/
private function updateJournalIdentifiers(int $journalId): void
private function updateJournalIdentifiers(TransactionJournal $transactionJournal): void
{
$identifier = 0;
$processed = [];
$transactions = Transaction::where('transaction_journal_id', $journalId)->where('amount', '>', 0)->get();
$exclude = []; // transactions already processed.
$transactions = $transactionJournal->transactions()->where('amount', '>', 0)->get();
/** @var Transaction $transaction */
foreach ($transactions as $transaction) {
// find opposing:
$amount = bcmul((string)$transaction->amount, '-1');
try {
/** @var Transaction $opposing */
$opposing = Transaction::where('transaction_journal_id', $journalId)
->where('amount', $amount)->where('identifier', '=', 0)
->whereNotIn('id', $processed)
->first();
} 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;
}
$opposing = $this->findOpposing($transaction, $exclude);
if (null !== $opposing) {
// give both a new identifier:
$transaction->identifier = $identifier;
$opposing->identifier = $identifier;
$transaction->save();
$opposing->save();
$processed[] = $transaction->id;
$processed[] = $opposing->id;
$exclude[] = $transaction->id;
$exclude[] = $opposing->id;
$this->count++;
}
++$identifier;
}
}
/**
* @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;
}
}

View File

@@ -28,6 +28,7 @@ use Illuminate\Console\Command;
/**
* Class UpgradeDatabase
* @codeCoverageIgnore
*/
class UpgradeDatabase extends Command
{
@@ -62,6 +63,7 @@ class UpgradeDatabase extends Command
public function handle(): int
{
$commands = [
// there are 11 upgrade commands.
'firefly-iii:transaction-identifiers',
'firefly-iii:account-currencies',
'firefly-iii:transfer-currencies',