IntenseFinance/firefly-iii
3
0
Fork 0
mirror of https://github.com/firefly-iii/firefly-iii.git synced 2025-02-25 18:45:27 -06:00
Code Issues Packages Projects Releases Wiki Activity

Fix some edge cases in recurrences.

Browse Source
This commit is contained in:
James Cole 2021-03-15 19:51:55 +01:00
parent a0b46d9d8a
commit d5ee87ddee
No known key found for this signature in database
GPG Key ID: B5669F9493CDE38D
8 changed files with 92 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/Api/V1/Requests/Models/Recurrence/UpdateRequest.php
View File

@ -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;

2
app/Http/Controllers/Admin/UserController.php
View File

@ -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']) {

9
app/Models/RecurrenceTransaction.php
View File

@ -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);
}
}

11
app/Services/Internal/Support/RecurringTransactionTrait.php
View File

@ -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();

32
app/Services/Internal/Update/RecurrenceUpdateService.php
View File

@ -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,8 +250,26 @@ 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 = [
@ -256,6 +278,8 @@ class RecurrenceUpdateService
'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)) {

29
app/Validation/RecurrenceValidation.php
View File

@ -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,7 +38,8 @@ use Log;
*/
trait RecurrenceValidation
{
public function validateRecurringConfig(Validator $validator) {
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;
@ -44,11 +47,13 @@ trait RecurrenceValidation
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) {
$validator->errors()->add('nr_of_repetitions', trans('validation.require_repeat_until'));
$validator->errors()->add('repeat_until', trans('validation.require_repeat_until'));
return;
}
}
@ -65,6 +70,28 @@ trait RecurrenceValidation
$data = $validator->getData();
$transactionType = $data['type'] ?? 'invalid';
// 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 */

6
tests/Api/Models/Recurrence/UpdateControllerTest.php
View File

@ -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,

4
tests/Traits/TestHelpers.php
View File

@ -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');