diff --git a/app/Api/V1/Requests/TransactionRequest.php b/app/Api/V1/Requests/TransactionRequest.php index 69cbacf63e..37d5e0d711 100644 --- a/app/Api/V1/Requests/TransactionRequest.php +++ b/app/Api/V1/Requests/TransactionRequest.php @@ -90,9 +90,9 @@ class TransactionRequest extends Request 'category_id' => isset($transaction['category_id']) ? intval($transaction['category_id']) : null, 'category_name' => $transaction['category_name'] ?? null, 'source_id' => isset($transaction['source_id']) ? intval($transaction['source_id']) : null, - 'source_name' => $transaction['source_name'] ?? null, + 'source_name' => isset($transaction['source_name']) ? strval($transaction['source_name']) : null, 'destination_id' => isset($transaction['destination_id']) ? intval($transaction['destination_id']) : null, - 'destination_name' => $transaction['destination_name'] ?? null, + 'destination_name' => isset($transaction['destination_name']) ? strval($transaction['destination_name']) : null, 'reconciled' => $transaction['reconciled'] ?? false, 'identifier' => $index, ]; @@ -356,11 +356,8 @@ class TransactionRequest extends Request // we ignore the account name at this point. return; } - $account = $repository->findByName($accountName, [$type]); - if (is_null($account)) { - $validator->errors()->add($nameField, trans('validation.belongs_user')); - } + // not having an opposing account by this name is NOT a problem. return; } diff --git a/app/Factory/AccountFactory.php b/app/Factory/AccountFactory.php new file mode 100644 index 0000000000..6e543fbb74 --- /dev/null +++ b/app/Factory/AccountFactory.php @@ -0,0 +1,95 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Factory; + +use FireflyIII\Models\Account; +use FireflyIII\Models\AccountType; +use FireflyIII\User; + +/** + * Class AccountFactory + */ +class AccountFactory +{ + /** @var User */ + private $user; + + /** + * TagFactory constructor. + */ + public function __construct() + { + + } + + /** + * @param array $data + * + * @return Account + */ + public function create(array $data): Account + { + return Account::create($data); + } + + /** + * @param string $accountName + * @param string $accountType + * + * @return Account + */ + public function findOrCreate(string $accountName, string $accountType): Account + { + $type = AccountType::whereType($accountType)->first(); + $accounts = $this->user->accounts()->where('account_type_id', $type->id)->get(['accounts.*']); + + /** @var Account $object */ + foreach ($accounts as $object) { + if ($object->name === $accountName) { + return $object; + } + } + + $newAccount = $this->create( + [ + 'user_id' => $this->user->id, + 'name' => $accountName, + 'account_type_id' => $type->id, + 'virtual_balance' => '0', + 'iban' => null, + 'active' => true, + ] + ); + return $newAccount; + } + + /** + * @param User $user + */ + public function setUser(User $user): void + { + $this->user = $user; + } + +} \ No newline at end of file diff --git a/app/Factory/TransactionFactory.php b/app/Factory/TransactionFactory.php index 154c733c5b..1a3d98b019 100644 --- a/app/Factory/TransactionFactory.php +++ b/app/Factory/TransactionFactory.php @@ -241,8 +241,10 @@ class TransactionFactory return $this->accountRepository->findNull($accountId); } if (strlen($accountName) > 0) { - // alternatively, return by name. Validator should catch invalid names. - return $this->accountRepository->findByName($accountName, [AccountType::EXPENSE]); + /** @var AccountFactory $factory */ + $factory = app(AccountFactory::class); + $factory->setUser($this->user); + return $factory->findOrCreate($accountName, AccountType::EXPENSE); } // return cash account: @@ -254,8 +256,11 @@ class TransactionFactory return $this->accountRepository->findNull($accountId); } if (strlen($accountName) > 0) { - // alternatively, return by name. Validator should catch invalid names. - return $this->accountRepository->findByName($accountName, [AccountType::REVENUE]); + // alternatively, return by name. + /** @var AccountFactory $factory */ + $factory = app(AccountFactory::class); + $factory->setUser($this->user); + return $factory->findOrCreate($accountName, AccountType::REVENUE); } // return cash account: diff --git a/tests/Api/V1/Controllers/TransactionControllerTest.php b/tests/Api/V1/Controllers/TransactionControllerTest.php index a830b1c5b4..e682e21c7e 100644 --- a/tests/Api/V1/Controllers/TransactionControllerTest.php +++ b/tests/Api/V1/Controllers/TransactionControllerTest.php @@ -77,6 +77,326 @@ class TransactionControllerTest extends TestCase } /** + * Empty descriptions + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testFailEmptyDescriptions() + { + $account = auth()->user()->accounts()->where('account_type_id', 3)->first(); + $data = [ + 'description' => '', + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_id' => $account->id, + 'description' => '', + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'description' => [ + 'The description field is required.', + 'The description must be between 1 and 255 characters.', + ], + 'transactions.0.description' => [ + 'Transaction description should not equal journal description.', + ], + ], + ] + ); + } + + /** + * Submit all empty descriptions for transactions. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testFailEmptySplitDescriptions() + { + $account = auth()->user()->accounts()->where('account_type_id', 3)->first(); + $data = [ + 'description' => 'Split journal #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_id' => $account->id, + 'description' => '', + ], + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_id' => $account->id, + 'description' => '', + ], + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'transactions.0.description' => [ + 'The transaction description field is required.', + ], + 'transactions.1.description' => [ + 'The transaction description field is required.', + ], + + ], + ] + ); + } + + /** + * Submitted expense account instead of asset account. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testFailExpenseID() + { + $account = $this->user()->accounts()->where('account_type_id', 4)->first(); + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_id' => $account->id, + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'transactions.0.source_id' => [ + 'This value is invalid for this field.', + ], + ], + ] + ); + } + + /** + * Submitted expense account name instead of asset account name. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testFailExpenseName() + { + $account = $this->user()->accounts()->where('account_type_id', 4)->first(); + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_name' => $account->name, + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'transactions.0.source_name' => [ + 'This value is invalid for this field.', + ], + ], + ] + ); + } + + /** + * Submit no asset account info at all. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testFailNoAsset() + { + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'transactions.0.source_id' => [ + 'The transactions.0.source_id field is required.', + ], + ], + ] + ); + } + + /** + * Submit no transactions. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testFailNoData() + { + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'description' => [ + 'Need at least one transaction.', + ], + ], + ] + ); + } + + /** + * Submit foreign currency without foreign currency info. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testFailNoForeignCurrencyInfo() + { + $account = auth()->user()->accounts()->where('account_type_id', 3)->first(); + $data = [ + 'description' => 'Split journal #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'foreign_amount' => 10, + 'source_id' => $account->id, + ], + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'transactions.0.foreign_amount' => [ + 'The content of this field is invalid without currency information.', + ], + + ], + ] + ); + } + + /** + * Submit revenue ID instead of expense ID. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testFailOpposingRevenueID() + { + $account = auth()->user()->accounts()->where('account_type_id', 3)->first(); + $opposing = auth()->user()->accounts()->where('account_type_id', 5)->first(); + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_id' => $account->id, + 'destination_id' => $opposing->id, + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'transactions.0.destination_id' => [ + 'This value is invalid for this field.', + ], + + ], + ] + ); + } + + /** + * Show index. + * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::__construct * @covers \FireflyIII\Api\V1\Controllers\TransactionController::index * @covers \FireflyIII\Api\V1\Controllers\TransactionController::mapTypes @@ -290,6 +610,7 @@ class TransactionControllerTest extends TestCase * Submit a transaction (withdrawal) with attached bill ID * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessBillId() { @@ -337,12 +658,12 @@ class TransactionControllerTest extends TestCase ); } - /** * Submit a transaction (withdrawal) with attached bill ID * TODO also test deposit / transfer (should be ignored). * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessBillName() { @@ -394,6 +715,55 @@ class TransactionControllerTest extends TestCase * Submit the minimum amount of data required to create a withdrawal. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testSuccessStoreAccountName() + { + $account = auth()->user()->accounts()->where('account_type_id', 3)->first(); + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_name' => $account->name, + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(200); + $response->assertJson( + [ + 'data' => [ + 'type' => 'transactions', + 'attributes' => [ + 'description' => $data['description'], + 'date' => $data['date'], + 'source_id' => $account->id, + 'source_name' => $account->name, + 'type' => 'Withdrawal', + 'source_type' => 'Asset account', + 'destination_name' => 'Cash account', + 'destination_type' => 'Cash account', + 'amount' => -10, + ], + 'links' => true, + ], + ] + ); + } + + /** + * Submit the minimum amount of data required to create a withdrawal. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreBasic() { @@ -437,10 +807,59 @@ class TransactionControllerTest extends TestCase ); } + /** + * Submit the minimum amount of data required to create a deposit. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testSuccessStoreBasicDeposit() + { + $account = auth()->user()->accounts()->where('account_type_id', 3)->first(); + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'deposit', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'destination_id' => $account->id, + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(200); + $response->assertJson( + [ + 'data' => [ + 'type' => 'transactions', + 'attributes' => [ + 'description' => $data['description'], + 'date' => $data['date'], + 'destination_id' => $account->id, + 'destination_name' => $account->name, + 'destination_type' => 'Asset account', + 'type' => 'Deposit', + 'source_name' => 'Cash account', + 'source_type' => 'Cash account', + 'amount' => 10, + ], + 'links' => true, + ], + ] + ); + } + /** * Submit with existing budget ID, see it reflected in output. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreBudgetId() { @@ -492,6 +911,7 @@ class TransactionControllerTest extends TestCase * Submit with existing budget name, see it reflected in output. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreBudgetName() { @@ -543,6 +963,7 @@ class TransactionControllerTest extends TestCase * Submit with existing category ID, see it reflected in output. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreCategoryID() { @@ -594,6 +1015,7 @@ class TransactionControllerTest extends TestCase * Submit with existing category ID, see it reflected in output. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreCategoryName() { @@ -645,6 +1067,7 @@ class TransactionControllerTest extends TestCase * Add foreign amount information. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreForeignAmount() { @@ -699,6 +1122,7 @@ class TransactionControllerTest extends TestCase * Add all available meta data fields. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreMetaData() { @@ -764,10 +1188,61 @@ class TransactionControllerTest extends TestCase ); } + /** + * Add opposing account by name. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testSuccessStoreNewOpposingName() + { + $account = auth()->user()->accounts()->where('account_type_id', 3)->first(); + $name = 'New opposing account #' . rand(1, 10000); + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_id' => $account->id, + 'destination_name' => $name, + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(200); + $response->assertJson( + [ + 'data' => [ + 'type' => 'transactions', + 'attributes' => [ + 'description' => $data['description'], + 'date' => $data['date'], + 'source_id' => $account->id, + 'source_name' => $account->name, + 'type' => 'Withdrawal', + 'source_type' => 'Asset account', + 'destination_name' => $name, + 'destination_type' => 'Expense account', + 'amount' => -10, + ], + 'links' => true, + ], + ] + ); + } + /** * Submit the minimum amount of data required to create a withdrawal. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreNotes() { @@ -814,11 +1289,114 @@ class TransactionControllerTest extends TestCase ); } + /** + * Add opposing account by ID. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testSuccessStoreOpposingID() + { + $account = auth()->user()->accounts()->where('account_type_id', 3)->first(); + $opposing = auth()->user()->accounts()->where('account_type_id', 4)->first(); + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_id' => $account->id, + 'destination_id' => $opposing->id, + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(200); + $response->assertJson( + [ + 'data' => [ + 'type' => 'transactions', + 'attributes' => [ + 'description' => $data['description'], + 'date' => $data['date'], + 'source_id' => $account->id, + 'source_name' => $account->name, + 'type' => 'Withdrawal', + 'source_type' => 'Asset account', + 'destination_name' => $opposing->name, + 'destination_id' => $opposing->id, + 'destination_type' => 'Expense account', + 'amount' => -10, + ], + 'links' => true, + ], + ] + ); + } + + /** + * Add opposing account by name. + * + * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest + */ + public function testSuccessStoreOpposingName() + { + $account = auth()->user()->accounts()->where('account_type_id', 3)->first(); + $opposing = auth()->user()->accounts()->where('account_type_id', 4)->first(); + $data = [ + 'description' => 'Some transaction #' . rand(1, 1000), + 'date' => '2018-01-01', + 'type' => 'withdrawal', + 'transactions' => [ + [ + 'amount' => '10', + 'currency_id' => 1, + 'source_id' => $account->id, + 'destination_name' => $opposing->name, + ], + + + ], + ]; + + // test API + $response = $this->post('/api/v1/transactions', $data, ['Accept' => 'application/json']); + $response->assertStatus(200); + $response->assertJson( + [ + 'data' => [ + 'type' => 'transactions', + 'attributes' => [ + 'description' => $data['description'], + 'date' => $data['date'], + 'source_id' => $account->id, + 'source_name' => $account->name, + 'type' => 'Withdrawal', + 'source_type' => 'Asset account', + 'destination_name' => $opposing->name, + 'destination_id' => $opposing->id, + 'destination_type' => 'Expense account', + 'amount' => -10, + ], + 'links' => true, + ], + ] + ); + } + /** * Submit the minimum amount of data required to create a withdrawal. * When sending a piggy bank by name, this must be reflected in the output. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStorePiggyId() { @@ -878,6 +1456,7 @@ class TransactionControllerTest extends TestCase * TODO only when sending a transfer. Ignore it with withdrawals. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStorePiggyName() { @@ -935,6 +1514,7 @@ class TransactionControllerTest extends TestCase * Set a different reconciled var * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreReconciled() { @@ -985,6 +1565,7 @@ class TransactionControllerTest extends TestCase * Add some tags as well. Expect to see them in the result. * * @covers \FireflyIII\Api\V1\Controllers\TransactionController::store + * @covers \FireflyIII\Api\V1\Requests\TransactionRequest */ public function testSuccessStoreTags() {