From 6c163ebef3966010466153321effc2fa9244be85 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 22 Feb 2020 11:05:16 +0100 Subject: [PATCH] Add debug information. --- .../Controllers/Search/AccountController.php | 6 ++++++ .../V1/Controllers/TransactionController.php | 2 ++ .../V1/Requests/TransactionStoreRequest.php | 5 ++++- app/Factory/TransactionGroupFactory.php | 8 +++++++- app/Factory/TransactionJournalFactory.php | 17 ++++++++++++++--- .../TransactionGroupRepository.php | 9 ++++++++- app/Support/Http/Api/AccountFilter.php | 3 ++- app/Support/Search/AccountSearch.php | 5 ++--- app/Validation/AccountValidator.php | 9 ++++++--- app/Validation/TransactionValidation.php | 18 +++++++++++++++++- 10 files changed, 68 insertions(+), 14 deletions(-) diff --git a/app/Api/V1/Controllers/Search/AccountController.php b/app/Api/V1/Controllers/Search/AccountController.php index 24feebcea8..6fcea80239 100644 --- a/app/Api/V1/Controllers/Search/AccountController.php +++ b/app/Api/V1/Controllers/Search/AccountController.php @@ -32,6 +32,7 @@ use Illuminate\Http\Response; use Illuminate\Pagination\LengthAwarePaginator; use League\Fractal\Pagination\IlluminatePaginatorAdapter; use League\Fractal\Resource\Collection as FractalCollection; +use Log; /** * Class AccountController @@ -62,6 +63,7 @@ class AccountController extends Controller */ public function search(Request $request) { + Log::debug('Now in account search()'); $manager = $this->getManager(); $query = $request->get('query'); $field = $request->get('field'); @@ -70,6 +72,8 @@ class AccountController extends Controller return response(null, 422); } $types = $this->mapAccountTypes($type); + Log::debug(sprintf('Going to search for "%s" in types', $query), $types); + /** @var AccountSearch $search */ $search = app(AccountSearch::class); $search->setUser(auth()->user()); @@ -79,6 +83,8 @@ class AccountController extends Controller $accounts = $search->search(); + Log::debug(sprintf('Found %d accounts', $accounts->count())); + /** @var AccountTransformer $transformer */ $transformer = app(AccountTransformer::class); $transformer->setParameters($this->parameters); diff --git a/app/Api/V1/Controllers/TransactionController.php b/app/Api/V1/Controllers/TransactionController.php index e5ce2eb4cd..728e3f0e8b 100644 --- a/app/Api/V1/Controllers/TransactionController.php +++ b/app/Api/V1/Controllers/TransactionController.php @@ -274,6 +274,7 @@ class TransactionController extends Controller */ public function store(TransactionStoreRequest $request): JsonResponse { + Log::debug('Now in API TransactionController::store()'); $data = $request->getAll(); $data['user'] = auth()->user()->id; @@ -283,6 +284,7 @@ class TransactionController extends Controller try { $transactionGroup = $this->groupRepository->store($data); } catch (DuplicateTransactionException $e) { + Log::warning('Caught a duplicate. Return error message.'); // return bad validation message. // TODO use Laravel's internal validation thing to do this. $response = [ diff --git a/app/Api/V1/Requests/TransactionStoreRequest.php b/app/Api/V1/Requests/TransactionStoreRequest.php index 967fae17e8..15d8bcfae8 100644 --- a/app/Api/V1/Requests/TransactionStoreRequest.php +++ b/app/Api/V1/Requests/TransactionStoreRequest.php @@ -30,7 +30,7 @@ use FireflyIII\Rules\IsDateOrTime; use FireflyIII\Support\NullArrayObject; use FireflyIII\Validation\TransactionValidation; use Illuminate\Validation\Validator; - +use Log; /** * Class TransactionStoreRequest @@ -46,6 +46,7 @@ class TransactionStoreRequest extends Request */ public function authorize(): bool { + Log::debug('Authorize TransactionStoreRequest'); // Only allow authenticated users return auth()->check(); } @@ -57,6 +58,7 @@ class TransactionStoreRequest extends Request */ public function getAll(): array { + Log::debug('get all data in TransactionStoreRequest'); $data = [ 'group_title' => $this->string('group_title'), 'error_if_duplicate_hash' => $this->boolean('error_if_duplicate_hash'), @@ -73,6 +75,7 @@ class TransactionStoreRequest extends Request */ public function rules(): array { + Log::debug('Collect rules of TransactionStoreRequest'); $rules = [ // basic fields for group: 'group_title' => 'between:1,1000|nullable', diff --git a/app/Factory/TransactionGroupFactory.php b/app/Factory/TransactionGroupFactory.php index 68c2ae09cb..27e6b619dc 100644 --- a/app/Factory/TransactionGroupFactory.php +++ b/app/Factory/TransactionGroupFactory.php @@ -26,6 +26,7 @@ namespace FireflyIII\Factory; use FireflyIII\Exceptions\DuplicateTransactionException; use FireflyIII\Models\TransactionGroup; use FireflyIII\User; +use Log; /** * Class TransactionGroupFactory @@ -60,7 +61,12 @@ class TransactionGroupFactory $this->journalFactory->setUser($this->user); $this->journalFactory->setErrorOnHash($data['error_if_duplicate_hash'] ?? false); - $collection = $this->journalFactory->create($data); + try { + $collection = $this->journalFactory->create($data); + } catch(DuplicateTransactionException $e) { + Log::warning('GroupFactory::create() caught journalFactory::create() with a duplicate!'); + throw new DuplicateTransactionException($e->getMessage()); + } $title = $data['group_title'] ?? null; $title = '' === $title ? null : $title; diff --git a/app/Factory/TransactionJournalFactory.php b/app/Factory/TransactionJournalFactory.php index a250310c6f..e1db66c70f 100644 --- a/app/Factory/TransactionJournalFactory.php +++ b/app/Factory/TransactionJournalFactory.php @@ -129,11 +129,11 @@ class TransactionJournalFactory public function create(array $data): Collection { // convert to special object. - $data = new NullArrayObject($data); + $dataObject = new NullArrayObject($data); Log::debug('Start of TransactionJournalFactory::create()'); $collection = new Collection; - $transactions = $data['transactions'] ?? []; + $transactions = $dataObject['transactions'] ?? []; if (0 === count($transactions)) { Log::error('There are no transactions in the array, the TransactionJournalFactory cannot continue.'); @@ -145,7 +145,12 @@ class TransactionJournalFactory Log::debug(sprintf('Now creating journal %d/%d', $index + 1, count($transactions))); Log::debug('Going to call createJournal', $row); - $journal = $this->createJournal(new NullArrayObject($row)); + try { + $journal = $this->createJournal(new NullArrayObject($row)); + } catch (DuplicateTransactionException|Exception $e) { + Log::warning('TransactionJournalFactory::create() caught a duplicate journal in createJournal()'); + throw new DuplicateTransactionException($e->getMessage()); + } if (null !== $journal) { $collection->push($journal); } @@ -395,17 +400,20 @@ class TransactionJournalFactory */ private function errorIfDuplicate(string $hash): void { + Log::debug(sprintf('In errorIfDuplicate(%s)', $hash)); if (false === $this->errorOnHash) { return; } $result = null; if ($this->errorOnHash) { + Log::debug('Will verify duplicate!'); /** @var TransactionJournalMeta $result */ $result = TransactionJournalMeta::where('data', json_encode($hash, JSON_THROW_ON_ERROR)) ->with(['transactionJournal', 'transactionJournal.transactionGroup']) ->first(); } if (null !== $result) { + Log::warning('Found a duplicate!'); throw new DuplicateTransactionException(sprintf('Duplicate of transaction #%d.', $result->transactionJournal->transaction_group_id)); } } @@ -526,6 +534,9 @@ class TransactionJournalFactory public function setErrorOnHash(bool $errorOnHash): void { $this->errorOnHash = $errorOnHash; + if (true === $errorOnHash) { + Log::info('Will trigger duplication alert for this journal.'); + } } diff --git a/app/Repositories/TransactionGroup/TransactionGroupRepository.php b/app/Repositories/TransactionGroup/TransactionGroupRepository.php index 34bc5bba21..2c000c5c66 100644 --- a/app/Repositories/TransactionGroup/TransactionGroupRepository.php +++ b/app/Repositories/TransactionGroup/TransactionGroupRepository.php @@ -45,6 +45,7 @@ use FireflyIII\Services\Internal\Update\GroupUpdateService; use FireflyIII\Support\NullArrayObject; use FireflyIII\User; use Illuminate\Database\Eloquent\Builder; +use Log; /** * Class TransactionGroupRepository @@ -337,8 +338,14 @@ class TransactionGroupRepository implements TransactionGroupRepositoryInterface /** @var TransactionGroupFactory $factory */ $factory = app(TransactionGroupFactory::class); $factory->setUser($this->user); + try { + return $factory->create($data); + } catch (DuplicateTransactionException $e) { + Log::warning('Group repository caught group factory with a duplicate exception!'); + throw new DuplicateTransactionException($e->getMessage()); + } + - return $factory->create($data); } /** diff --git a/app/Support/Http/Api/AccountFilter.php b/app/Support/Http/Api/AccountFilter.php index 06b15335f4..3c7378aec2 100644 --- a/app/Support/Http/Api/AccountFilter.php +++ b/app/Support/Http/Api/AccountFilter.php @@ -41,7 +41,8 @@ trait AccountFilter protected function mapAccountTypes(string $type): array { $types = [ - 'all' => [AccountType::DEFAULT, AccountType::CASH, AccountType::ASSET, AccountType::EXPENSE, AccountType::REVENUE, + 'all' => [AccountType::DEFAULT, AccountType::CASH, + AccountType::ASSET, AccountType::EXPENSE, AccountType::REVENUE, AccountType::INITIAL_BALANCE, AccountType::BENEFICIARY, AccountType::IMPORT, AccountType::RECONCILIATION, AccountType::LOAN, AccountType::DEBT, AccountType::MORTGAGE], 'asset' => [AccountType::DEFAULT, AccountType::ASSET,], diff --git a/app/Support/Search/AccountSearch.php b/app/Support/Search/AccountSearch.php index ec33585a09..9fff5294ea 100644 --- a/app/Support/Search/AccountSearch.php +++ b/app/Support/Search/AccountSearch.php @@ -82,8 +82,8 @@ class AccountSearch implements GenericSearchInterface $query->orWhere( static function (Builder $q) use ($originalQuery) { $json = json_encode($originalQuery, JSON_THROW_ON_ERROR); - $q->where('account_meta.name', 'account_number'); - $q->where('account_meta.data', $json); + $q->where('account_meta.name', '=', 'account_number'); + $q->where('account_meta.data', 'LIKE', $json); } ); break; @@ -107,7 +107,6 @@ class AccountSearch implements GenericSearchInterface ); break; } - return $query->distinct()->get(['accounts.*']); } diff --git a/app/Validation/AccountValidator.php b/app/Validation/AccountValidator.php index 9a70d05fcb..19da4ce24c 100644 --- a/app/Validation/AccountValidator.php +++ b/app/Validation/AccountValidator.php @@ -559,7 +559,7 @@ class AccountValidator // if both values are NULL we return false, // because the source of a withdrawal can't be created. $this->sourceError = (string)trans('validation.transfer_source_need_data'); - + Log::warning('Not a valid source, need more data.'); return false; } @@ -567,10 +567,11 @@ class AccountValidator $search = $this->findExistingAccount($validTypes, (int)$accountId, (string)$accountName); if (null === $search) { $this->sourceError = (string)trans('validation.transfer_source_bad_data', ['id' => $accountId, 'name' => $accountName]); - + Log::warning('Not a valid source, cant find it.', $validTypes); return false; } $this->source = $search; + Log::debug('Valid source!'); return true; } @@ -634,6 +635,7 @@ class AccountValidator // if both values are NULL we return false, // because the source of a withdrawal can't be created. $this->sourceError = (string)trans('validation.withdrawal_source_need_data'); + Log::warning('Not a valid source. Need more data.'); return false; } @@ -642,10 +644,11 @@ class AccountValidator $search = $this->findExistingAccount($validTypes, (int)$accountId, (string)$accountName); if (null === $search) { $this->sourceError = (string)trans('validation.withdrawal_source_bad_data', ['id' => $accountId, 'name' => $accountName]); - + Log::warning('Not a valid source. Cant find it.', $validTypes); return false; } $this->source = $search; + Log::debug('Valid source account!'); return true; } diff --git a/app/Validation/TransactionValidation.php b/app/Validation/TransactionValidation.php index f530c5322f..93f5aefb0d 100644 --- a/app/Validation/TransactionValidation.php +++ b/app/Validation/TransactionValidation.php @@ -42,6 +42,7 @@ trait TransactionValidation */ public function validateAccountInformation(Validator $validator): void { + Log::debug('Now in validateAccountInformation()'); $transactions = $this->getTransactionsArray($validator); $data = $validator->getData(); @@ -88,6 +89,7 @@ trait TransactionValidation */ public function validateAccountInformationUpdate(Validator $validator): void { + Log::debug('Now in validateAccountInformationUpdate()'); $transactions = $this->getTransactionsArray($validator); /** @var AccountValidator $accountValidator */ @@ -141,9 +143,10 @@ trait TransactionValidation */ public function validateDescriptions(Validator $validator): void { + Log::debug('Now in validateDescriptions()'); $transactions = $this->getTransactionsArray($validator); $validDescriptions = 0; - foreach ($transactions as $index => $transaction) { + foreach ($transactions as $transaction) { if ('' !== (string)($transaction['description'] ?? null)) { $validDescriptions++; } @@ -164,6 +167,7 @@ trait TransactionValidation */ public function validateForeignCurrencyInformation(Validator $validator): void { + Log::debug('Now in validateForeignCurrencyInformation()'); $transactions = $this->getTransactionsArray($validator); foreach ($transactions as $index => $transaction) { @@ -191,6 +195,7 @@ trait TransactionValidation */ public function validateGroupDescription(Validator $validator): void { + Log::debug('Now in validateGroupDescription()'); $data = $validator->getData(); $transactions = $this->getTransactionsArray($validator); @@ -207,6 +212,7 @@ trait TransactionValidation */ public function validateOneRecurrenceTransaction(Validator $validator): void { + Log::debug('Now in validateOneRecurrenceTransaction()'); $transactions = $this->getTransactionsArray($validator); // need at least one transaction @@ -222,6 +228,7 @@ trait TransactionValidation */ public function validateOneRecurrenceTransactionUpdate(Validator $validator): void { + Log::debug('Now in validateOneRecurrenceTransactionUpdate()'); $transactions = $this->getTransactionsArray($validator); // need at least one transaction if (0 === count($transactions)) { @@ -236,11 +243,15 @@ trait TransactionValidation */ public function validateOneTransaction(Validator $validator): void { + Log::debug('Now in validateOneTransaction()'); $transactions = $this->getTransactionsArray($validator); // need at least one transaction if (0 === count($transactions)) { $validator->errors()->add('transactions.0.description', (string)trans('validation.at_least_one_transaction')); + Log::debug('Added error: at_least_one_transaction.'); + return; } + Log::debug('Added NO errors.'); } /** @@ -250,6 +261,7 @@ trait TransactionValidation */ public function validateTransactionTypes(Validator $validator): void { + Log::debug('Now in validateTransactionTypes()'); $transactions = $this->getTransactionsArray($validator); $types = []; @@ -276,6 +288,7 @@ trait TransactionValidation */ public function validateTransactionTypesForUpdate(Validator $validator): void { + Log::debug('Now in validateTransactionTypesForUpdate()'); $transactions = $this->getTransactionsArray($validator); $types = []; foreach ($transactions as $index => $transaction) { @@ -383,6 +396,7 @@ trait TransactionValidation */ private function validateEqualAccounts(Validator $validator): void { + Log::debug('Now in validateEqualAccounts()'); $data = $validator->getData(); $transactions = $data['transactions'] ?? []; @@ -433,6 +447,7 @@ trait TransactionValidation */ private function validateEqualAccountsForUpdate(Validator $validator, TransactionGroup $transactionGroup): void { + Log::debug('Now in validateEqualAccountsForUpdate()'); $data = $validator->getData(); $transactions = $data['transactions'] ?? []; @@ -525,6 +540,7 @@ trait TransactionValidation */ private function validateJournalIds(Validator $validator, TransactionGroup $transactionGroup): void { + Log::debug('Now in validateJournalIds()'); $data = $validator->getData(); $transactions = $data['transactions'] ?? [];