diff --git a/app/Api/V1/Requests/Models/Recurrence/UpdateRequest.php b/app/Api/V1/Requests/Models/Recurrence/UpdateRequest.php index b632c4ddcc..9077bb38d9 100644 --- a/app/Api/V1/Requests/Models/Recurrence/UpdateRequest.php +++ b/app/Api/V1/Requests/Models/Recurrence/UpdateRequest.php @@ -93,7 +93,7 @@ class UpdateRequest extends FormRequest } /** @var array $transaction */ foreach ($transactions as $transaction) { - $return[] = $this->getSingleRecurrenceData($transaction); + $return[] = $this->getSingleTransactionData($transaction); } return $return; diff --git a/app/Http/Controllers/Admin/UserController.php b/app/Http/Controllers/Admin/UserController.php index 23d1e4e683..cdefb8f96a 100644 --- a/app/Http/Controllers/Admin/UserController.php +++ b/app/Http/Controllers/Admin/UserController.php @@ -194,7 +194,7 @@ class UserController extends Controller Log::debug('Actually here'); $data = $request->getUserData(); - var_dump($data); + //var_dump($data); // update password if (array_key_exists('password', $data) && '' !== $data['password']) { diff --git a/app/Models/RecurrenceTransaction.php b/app/Models/RecurrenceTransaction.php index dcb1c804b0..137d59ef73 100644 --- a/app/Models/RecurrenceTransaction.php +++ b/app/Models/RecurrenceTransaction.php @@ -76,6 +76,7 @@ use Illuminate\Support\Collection; * @mixin Eloquent * @property int|null $transaction_type_id * @method static \Illuminate\Database\Eloquent\Builder|RecurrenceTransaction whereTransactionTypeId($value) + * @property-read \FireflyIII\Models\TransactionType|null $transactionType */ class RecurrenceTransaction extends Model { @@ -154,4 +155,12 @@ class RecurrenceTransaction extends Model { return $this->belongsTo(TransactionCurrency::class); } + /** + * @codeCoverageIgnore + * @return BelongsTo + */ + public function transactionType(): BelongsTo + { + return $this->belongsTo(TransactionType::class); + } } diff --git a/app/Services/Internal/Support/RecurringTransactionTrait.php b/app/Services/Internal/Support/RecurringTransactionTrait.php index 1699ed945d..b0787c3e05 100644 --- a/app/Services/Internal/Support/RecurringTransactionTrait.php +++ b/app/Services/Internal/Support/RecurringTransactionTrait.php @@ -111,7 +111,9 @@ trait RecurringTransactionTrait */ protected function createTransactions(Recurrence $recurrence, array $transactions): void { - foreach ($transactions as $array) { + Log::debug('Now in createTransactions()'); + foreach ($transactions as $index => $array) { + Log::debug(sprintf('Now at transaction #%d', $index)); $sourceTypes = config(sprintf('firefly.expected_source_types.source.%s', $recurrence->transactionType->type)); $destTypes = config(sprintf('firefly.expected_source_types.destination.%s', $recurrence->transactionType->type)); $source = $this->findAccount($sourceTypes, $array['source_id'], null); @@ -125,11 +127,17 @@ trait RecurringTransactionTrait $currency = app('amount')->getDefaultCurrencyByUser($recurrence->user); } + Log::debug( + sprintf('Will set the validator type to %s based on the type of the recurrence (#%d).', $recurrence->transactionType->type, $recurrence->id) + ); + // once the accounts have been determined, we still verify their validity: /** @var AccountValidator $validator */ $validator = app(AccountValidator::class); $validator->setUser($recurrence->user); $validator->setTransactionType($recurrence->transactionType->type); + + if (!$validator->validateSource($source->id, null, null)) { throw new FireflyException(sprintf('Source invalid: %s', $validator->sourceError)); // @codeCoverageIgnore } @@ -335,6 +343,7 @@ trait RecurringTransactionTrait */ protected function deleteTransactions(Recurrence $recurrence): void { + Log::debug('deleteTransactions()'); /** @var RecurrenceTransaction $transaction */ foreach ($recurrence->recurrenceTransactions as $transaction) { $transaction->recurrenceTransactionMeta()->delete(); diff --git a/app/Services/Internal/Update/RecurrenceUpdateService.php b/app/Services/Internal/Update/RecurrenceUpdateService.php index c3d04b8bd7..503430dcab 100644 --- a/app/Services/Internal/Update/RecurrenceUpdateService.php +++ b/app/Services/Internal/Update/RecurrenceUpdateService.php @@ -25,6 +25,7 @@ namespace FireflyIII\Services\Internal\Update; use Exception; use FireflyIII\Exceptions\FireflyException; +use FireflyIII\Factory\TransactionCurrencyFactory; use FireflyIII\Models\Note; use FireflyIII\Models\Recurrence; use FireflyIII\Models\RecurrenceRepetition; @@ -155,7 +156,7 @@ class RecurrenceUpdateService } // user added or removed repetitions, delete all and recreate: if ($originalCount !== count($repetitions)) { - Log::debug('Del + recreate'); + Log::debug('Delete existing repetitions and create new ones.'); $this->deleteRepetitions($recurrence); $this->createRepetitions($recurrence, $repetitions); @@ -232,12 +233,15 @@ class RecurrenceUpdateService } // user added or removed repetitions, delete all and recreate: if ($originalCount !== count($transactions)) { - Log::debug('Del + recreate'); + Log::debug('Delete existing transactions and create new ones.'); $this->deleteTransactions($recurrence); $this->createTransactions($recurrence, $transactions); return; } + $currencyFactory = app(TransactionCurrencyFactory::class); + + // loop all and try to match them: if ($originalCount === count($transactions)) { Log::debug('Loop and find'); @@ -246,16 +250,36 @@ class RecurrenceUpdateService if (null === $match) { throw new FireflyException('Cannot match recurring transaction to existing transaction. Not sure what to do. Break.'); } - // TODO find currency - // TODO find foreign currency + $currency = null; + $foreignCurrency = null; + if (array_key_exists('currency_id', $current) || array_key_exists('currency_code', $current)) { + $currency = $currencyFactory->find($current['currency_id'] ?? null, $currency['currency_code'] ?? null); + } + if (null === $currency) { + unset($current['currency_id'], $currency['currency_code']); + } + if (null !== $currency) { + $current['currency_id'] = (int)$currency->id; + } + if (array_key_exists('foreign_currency_id', $current) || array_key_exists('foreign_currency_code', $current)) { + $foreignCurrency = $currencyFactory->find($current['foreign_currency_id'] ?? null, $currency['foreign_currency_code'] ?? null); + } + if (null === $foreignCurrency) { + unset($current['foreign_currency_id'], $currency['foreign_currency_code']); + } + if (null !== $foreignCurrency) { + $current['foreign_currency_id'] = (int)$foreignCurrency->id; + } // update fields $fields = [ - 'source_id' => 'source_id', - 'destination_id' => 'destination_id', - 'amount' => 'amount', - 'foreign_amount' => 'foreign_amount', - 'description' => 'description', + 'source_id' => 'source_id', + 'destination_id' => 'destination_id', + 'amount' => 'amount', + 'foreign_amount' => 'foreign_amount', + 'description' => 'description', + 'currency_id' => 'transaction_currency_id', + 'foreign_currency_id' => 'foreign_currency_id', ]; foreach ($fields as $field => $column) { if (array_key_exists($field, $current)) { diff --git a/app/Validation/RecurrenceValidation.php b/app/Validation/RecurrenceValidation.php index a58d4d2033..e4a1f8e2c0 100644 --- a/app/Validation/RecurrenceValidation.php +++ b/app/Validation/RecurrenceValidation.php @@ -24,6 +24,8 @@ declare(strict_types=1); namespace FireflyIII\Validation; use Carbon\Carbon; +use FireflyIII\Models\Recurrence; +use FireflyIII\Models\RecurrenceTransaction; use Illuminate\Validation\Validator; use InvalidArgumentException; use Log; @@ -36,19 +38,22 @@ use Log; */ trait RecurrenceValidation { - public function validateRecurringConfig(Validator $validator) { - $data = $validator->getData(); - $reps = array_key_exists('nr_of_repetitions', $data) ? (int)$data['nr_of_repetitions'] : null; + public function validateRecurringConfig(Validator $validator) + { + $data = $validator->getData(); + $reps = array_key_exists('nr_of_repetitions', $data) ? (int)$data['nr_of_repetitions'] : null; $repeatUntil = array_key_exists('repeat_until', $data) ? new Carbon($data['repeat_until']) : null; - if(null === $reps && null === $repeatUntil) { + if (null === $reps && null === $repeatUntil) { $validator->errors()->add('nr_of_repetitions', trans('validation.require_repeat_until')); $validator->errors()->add('repeat_until', trans('validation.require_repeat_until')); + return; } - if($reps > 0 && null !== $repeatUntil) { + if ($reps > 0 && null !== $repeatUntil) { $validator->errors()->add('nr_of_repetitions', trans('validation.require_repeat_until')); $validator->errors()->add('repeat_until', trans('validation.require_repeat_until')); + return; } } @@ -65,7 +70,29 @@ trait RecurrenceValidation $data = $validator->getData(); $transactionType = $data['type'] ?? 'invalid'; - $transactions = $data['transactions'] ?? []; + + // grab model from parameter and try to set the transaction type from it + if ('invalid' === $transactionType) { + Log::debug('Type is invalid but we will search for it.'); + /** @var Recurrence $recurrence */ + $recurrence = $this->route()->parameter('recurrence'); + if (null !== $recurrence) { + Log::debug('There is a recurrence in the route.'); + // ok so we have a recurrence should be able to extract type somehow. + /** @var RecurrenceTransaction $first */ + $first = $recurrence->recurrenceTransactions()->first(); + if (null !== $first) { + $transactionType = $first->transactionType ? $first->transactionType->type : 'withdrawal'; + Log::debug(sprintf('Determined type to be %s.', $transactionType)); + } + if(null === $first) { + Log::warning('Just going to assume type is a withdrawal.'); + $transactionType = 'withdrawal'; + } + } + } + + $transactions = $data['transactions'] ?? []; /** @var AccountValidator $accountValidator */ $accountValidator = app(AccountValidator::class); diff --git a/tests/Api/Models/Recurrence/UpdateControllerTest.php b/tests/Api/Models/Recurrence/UpdateControllerTest.php index 70a5cc7f5d..dcf1f28bcc 100644 --- a/tests/Api/Models/Recurrence/UpdateControllerTest.php +++ b/tests/Api/Models/Recurrence/UpdateControllerTest.php @@ -212,10 +212,10 @@ class UpdateControllerTest extends TestCase // now loop fields, enough to create sets I guess? // TODO maybe do some permutation stuff here? $extraTransaction = [ - 'currency_id' => $faker->numberBetween(1, 4), - 'foreign_currency_id' => $faker->numberBetween(4, 6), + 'currency_id' => (string)$faker->numberBetween(1, 4), + 'foreign_currency_id' => (string)$faker->numberBetween(4, 6), 'source_id' => $faker->numberBetween(1, 3), - 'destination_id' => $faker->numberBetween(8), + 'destination_id' => $faker->numberBetween(8, 8), 'amount' => number_format($faker->randomFloat(2, 10, 100), 2), 'foreign_amount' => number_format($faker->randomFloat(2, 10, 100), 2), 'description' => $faker->uuid, diff --git a/tests/Traits/TestHelpers.php b/tests/Traits/TestHelpers.php index c93dd04d85..3e442db9bf 100644 --- a/tests/Traits/TestHelpers.php +++ b/tests/Traits/TestHelpers.php @@ -196,7 +196,7 @@ trait TestHelpers if (array_key_exists($rKey, $submissionArray)) { // comparison must be on array: if (is_array($submissionArray[$rKey]) && is_array($rValue)) { - $this->compareArray($originalAttributes, $rKey, $submissionArray[$rKey], $rValue); + $this->compareArray($submissionArray, $rKey, $submissionArray[$rKey], $rValue); } if (!is_array($submissionArray[$rKey]) && !is_array($rValue)) { @@ -261,7 +261,7 @@ trait TestHelpers $responseBody = $response->content(); $responseJson = json_decode($responseBody, true); $status = $response->getStatusCode(); - $this->assertEquals($status, 200, sprintf("Submission: %s\nResponse: %s", json_encode($submission), $responseBody)); + $this->assertEquals($status, 200, sprintf("Submission:\n%s\nResponse: %s", json_encode($submission), $responseBody)); $response->assertHeader('Content-Type', 'application/vnd.api+json');