From 2d7b7c2f3f8739cd0e0380b557a55bc3d9f252c4 Mon Sep 17 00:00:00 2001 From: James Cole Date: Fri, 29 Jun 2018 19:27:07 +0200 Subject: [PATCH] Expand recurring transactions API --- .../V1/Controllers/PreferenceController.php | 14 - .../V1/Controllers/RecurrenceController.php | 38 +- app/Api/V1/Requests/RecurrenceRequest.php | 480 ++++++++++++++++++ app/Api/V1/Requests/TransactionRequest.php | 10 +- app/Factory/RecurrenceFactory.php | 12 +- app/Jobs/CreateRecurringTransactions.php | 4 +- app/Models/RecurrenceTransaction.php | 6 +- app/Models/RecurrenceTransactionMeta.php | 2 +- .../Recurring/RecurringRepository.php | 1 - .../RecurringRepositoryInterface.php | 1 - .../Destroy/RecurrenceDestroyService.php | 14 + .../Support/RecurringTransactionTrait.php | 22 +- .../2018_06_08_200526_changes_for_v475.php | 8 +- public/js/ff/recurring/create.js | 36 +- public/js/ff/recurring/edit.js | 32 +- resources/lang/en_US/form.php | 120 ++--- resources/lang/en_US/validation.php | 4 + resources/views/recurring/create.twig | 8 +- resources/views/recurring/edit.twig | 8 +- resources/views/recurring/index.twig | 4 +- resources/views/recurring/show.twig | 8 +- 21 files changed, 670 insertions(+), 162 deletions(-) create mode 100644 app/Api/V1/Requests/RecurrenceRequest.php diff --git a/app/Api/V1/Controllers/PreferenceController.php b/app/Api/V1/Controllers/PreferenceController.php index 44363133d4..3efe8fa35a 100644 --- a/app/Api/V1/Controllers/PreferenceController.php +++ b/app/Api/V1/Controllers/PreferenceController.php @@ -56,20 +56,6 @@ class PreferenceController extends Controller ); } - /** - * Delete the resource. - * - * @param string $object - * - * @return JsonResponse - */ - public function delete(string $object): JsonResponse - { - // todo delete object. - - return response()->json([], 204); - } - /** * List all of them. * diff --git a/app/Api/V1/Controllers/RecurrenceController.php b/app/Api/V1/Controllers/RecurrenceController.php index e901c1a66d..84af2a235a 100644 --- a/app/Api/V1/Controllers/RecurrenceController.php +++ b/app/Api/V1/Controllers/RecurrenceController.php @@ -23,8 +23,9 @@ declare(strict_types=1); namespace FireflyIII\Api\V1\Controllers; +use FireflyIII\Api\V1\Requests\RecurrenceRequest; +use FireflyIII\Models\Recurrence; use FireflyIII\Repositories\Recurring\RecurringRepositoryInterface; -use FireflyIII\Transformers\PiggyBankTransformer; use FireflyIII\Transformers\RecurrenceTransformer; use FireflyIII\User; use Illuminate\Http\JsonResponse; @@ -33,6 +34,7 @@ use Illuminate\Pagination\LengthAwarePaginator; use League\Fractal\Manager; use League\Fractal\Pagination\IlluminatePaginatorAdapter; use League\Fractal\Resource\Collection as FractalCollection; +use League\Fractal\Resource\Item; use League\Fractal\Serializer\JsonApiSerializer; /** @@ -68,9 +70,9 @@ class RecurrenceController extends Controller * * @return JsonResponse */ - public function delete(string $object): JsonResponse + public function delete(Recurrence $recurrence): JsonResponse { - // todo delete object. + $this->repository->destroy($recurrence); return response()->json([], 204); } @@ -112,28 +114,44 @@ class RecurrenceController extends Controller /** * List single resource. * - * @param Request $request - * @param string $object + * @param Request $request + * @param Recurrence $recurrence * * @return JsonResponse */ - public function show(Request $request, string $object): JsonResponse + public function show(Request $request, Recurrence $recurrence): JsonResponse { - // todo implement me. + $manager = new Manager(); + // add include parameter: + $include = $request->get('include') ?? ''; + $manager->parseIncludes($include); + + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $manager->setSerializer(new JsonApiSerializer($baseUrl)); + + $resource = new Item($recurrence, new RecurrenceTransformer($this->parameters), 'recurrences'); + + return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); + } /** * Store new object. * - * @param Request $request + * @param RecurrenceRequest $request * * @return JsonResponse */ - public function store(Request $request): JsonResponse + public function store(RecurrenceRequest $request): JsonResponse { - // todo replace code and replace request object. + $recurrence = $this->repository->store($request->getAll()); + $manager = new Manager(); + $baseUrl = $request->getSchemeAndHttpHost() . '/api/v1'; + $manager->setSerializer(new JsonApiSerializer($baseUrl)); + $resource = new Item($recurrence, new RecurrenceTransformer($this->parameters), 'recurrences'); + return response()->json($manager->createData($resource)->toArray())->header('Content-Type', 'application/vnd.api+json'); } /** diff --git a/app/Api/V1/Requests/RecurrenceRequest.php b/app/Api/V1/Requests/RecurrenceRequest.php new file mode 100644 index 0000000000..6c7925fed6 --- /dev/null +++ b/app/Api/V1/Requests/RecurrenceRequest.php @@ -0,0 +1,480 @@ +. + */ + +declare(strict_types=1); + +namespace FireflyIII\Api\V1\Requests; + +use Carbon\Carbon; +use FireflyIII\Models\Account; +use FireflyIII\Models\AccountType; +use FireflyIII\Repositories\Account\AccountRepositoryInterface; +use FireflyIII\Rules\BelongsUser; +use Illuminate\Validation\Validator; +use InvalidArgumentException; + +/** + * Class RecurrenceRequest + */ +class RecurrenceRequest extends Request +{ + /** + * @return bool + */ + public function authorize(): bool + { + // Only allow authenticated users + return auth()->check(); + } + + /** + * @return array + */ + public function getAll(): array + { + $return = [ + 'recurrence' => [ + 'type' => $this->string('type'), + 'title' => $this->string('title'), + 'description' => $this->string('description'), + 'first_date' => $this->date('first_date'), + 'repeat_until' => $this->date('repeat_until'), + 'repetitions' => $this->integer('nr_of_repetitions'), + 'apply_rules' => $this->boolean('apply_rules'), + 'active' => $this->boolean('active'), + ], + 'meta' => [ + 'piggy_bank_id' => $this->integer('piggy_bank_id'), + 'piggy_bank_name' => $this->string('piggy_bank_name'), + 'tags' => explode(',', $this->string('tags')), + ], + 'transactions' => [], + 'repetitions' => [], + ]; + + // repetition data: + /** @var array $repetitions */ + $repetitions = $this->get('repetitions'); + /** @var array $repetition */ + foreach ($repetitions as $repetition) { + $return['repetitions'][] = [ + 'type' => $repetition['type'], + 'moment' => $repetition['moment'], + 'skip' => (int)$repetition['skip'], + 'weekend' => (int)$repetition['weekend'], + ]; + } + // transaction data: + /** @var array $transactions */ + $transactions = $this->get('transactions'); + /** @var array $transaction */ + foreach ($transactions as $transaction) { + $return['transactions'][] = [ + 'amount' => $transaction['amount'], + + 'currency_id' => isset($transaction['currency_id']) ? (int)$transaction['currency_id'] : null, + 'currency_code' => $transaction['currency_code'] ?? null, + + 'foreign_amount' => $transaction['foreign_amount'] ?? null, + 'foreign_currency_id' => isset($transaction['foreign_currency_id']) ? (int)$transaction['foreign_currency_id'] : null, + 'foreign_currency_code' => $transaction['foreign_currency_code'] ?? null, + + 'budget_id' => isset($transaction['budget_id']) ? (int)$transaction['budget_id'] : null, + 'budget_name' => $transaction['budget_name'] ?? null, + 'category_id' => isset($transaction['category_id']) ? (int)$transaction['category_id'] : null, + 'category_name' => $transaction['category_name'] ?? null, + + 'source_id' => isset($transaction['source_id']) ? (int)$transaction['source_id'] : null, + 'source_name' => isset($transaction['source_name']) ? (string)$transaction['source_name'] : null, + 'destination_id' => isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null, + 'destination_name' => isset($transaction['destination_name']) ? (string)$transaction['destination_name'] : null, + + 'description' => $transaction['description'], + ]; + } + + return $return; + } + + /** + * @return array + */ + public function rules(): array + { + $today = new Carbon; + $today->addDay(); + + return [ + 'type' => 'required|in:withdrawal,transfer,deposit', + 'title' => 'required|between:1,255', + 'description' => 'between:1,65000', + 'first_date' => sprintf('required|date|after:%s', $today->format('Y-m-d')), + 'repeat_until' => sprintf('date|after:%s', $today->format('Y-m-d')), + 'nr_of_repetitions' => 'numeric|between:1,31', + 'apply_rules' => 'required|boolean', + 'active' => 'required|boolean', + + // rules for meta values: + 'tags' => 'between:1,64000', + 'piggy_bank_id' => 'numeric', + + // rules for repetitions. + 'repetitions.*.type' => 'required|in:daily,weekly,ndom,monthly,yearly', + 'repetitions.*.moment' => 'between:0,10', + 'repetitions.*.skip' => 'required|between:0,31', + 'repetitions.*.weekend' => 'required|between:1,4', + + // rules for transactions. + 'transactions.*.currency_id' => 'numeric|exists:transaction_currencies,id|required_without:transactions.*.currency_code', + 'transactions.*.currency_code' => 'min:3|max:3|exists:transaction_currencies,code|required_without:transactions.*.currency_id', + 'transactions.*.foreign_amount' => 'numeric|more:0', + 'transactions.*.foreign_currency_id' => 'numeric|exists:transaction_currencies,id', + 'transactions.*.foreign_currency_code' => 'min:3|max:3|exists:transaction_currencies,code', + 'transactions.*.budget_id' => ['mustExist:budgets,id', new BelongsUser], + 'transactions.*.category_name' => 'between:1,255|nullable', + 'transactions.*.source_id' => ['numeric', 'nullable', new BelongsUser], + 'transactions.*.source_name' => 'between:1,255|nullable', + 'transactions.*.destination_id' => ['numeric', 'nullable', new BelongsUser], + 'transactions.*.destination_name' => 'between:1,255|nullable', + 'transactions.*.amount' => 'required|numeric|more:0', + 'transactions.*.description' => 'required|between:1,255', + ]; + } + + /** + * Configure the validator instance. + * + * @param Validator $validator + * + * @return void + */ + public function withValidator(Validator $validator): void + { + $validator->after( + function (Validator $validator) { + $this->atLeastOneTransaction($validator); + $this->atLeastOneRepetition($validator); + $this->validRepeatsUntil($validator); + $this->validRepetitionMoment($validator); + $this->foreignCurrencyInformation($validator); + $this->validateAccountInformation($validator); + } + ); + } + + /** + * Throws an error when this asset account is invalid. + * + * @noinspection MoreThanThreeArgumentsInspection + * + * @param Validator $validator + * @param int|null $accountId + * @param null|string $accountName + * @param string $idField + * @param string $nameField + * + * @return null|Account + */ + protected function assetAccountExists(Validator $validator, ?int $accountId, ?string $accountName, string $idField, string $nameField): ?Account + { + $accountId = (int)$accountId; + $accountName = (string)$accountName; + // both empty? hard exit. + if ($accountId < 1 && '' === $accountName) { + $validator->errors()->add($idField, trans('validation.filled', ['attribute' => $idField])); + + return null; + } + // ID belongs to user and is asset account: + /** @var AccountRepositoryInterface $repository */ + $repository = app(AccountRepositoryInterface::class); + $repository->setUser(auth()->user()); + $set = $repository->getAccountsById([$accountId]); + if ($set->count() === 1) { + /** @var Account $first */ + $first = $set->first(); + if ($first->accountType->type !== AccountType::ASSET) { + $validator->errors()->add($idField, trans('validation.belongs_user')); + + return null; + } + + // we ignore the account name at this point. + return $first; + } + + $account = $repository->findByName($accountName, [AccountType::ASSET]); + if (null === $account) { + $validator->errors()->add($nameField, trans('validation.belongs_user')); + + return null; + } + + return $account; + } + + /** + * Adds an error to the validator when there are no repetitions in the array of data. + * + * @param Validator $validator + */ + protected function atLeastOneRepetition(Validator $validator): void + { + $data = $validator->getData(); + $repetitions = $data['repetitions'] ?? []; + // need at least one transaction + if (\count($repetitions) === 0) { + $validator->errors()->add('description', trans('validation.at_least_one_repetition')); + } + } + + /** + * Adds an error to the validator when there are no transactions in the array of data. + * + * @param Validator $validator + */ + protected function atLeastOneTransaction(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + // need at least one transaction + if (\count($transactions) === 0) { + $validator->errors()->add('description', trans('validation.at_least_one_transaction')); + } + } + + /** + * TODO can be made a rule? + * If the transactions contain foreign amounts, there must also be foreign currency information. + * + * @param Validator $validator + */ + protected function foreignCurrencyInformation(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + foreach ($transactions as $index => $transaction) { + // must have currency info. + if (isset($transaction['foreign_amount']) + && !(isset($transaction['foreign_currency_id']) + || isset($transaction['foreign_currency_code']))) { + $validator->errors()->add( + 'transactions.' . $index . '.foreign_amount', + trans('validation.require_currency_info') + ); + } + } + } + + /** + * Throws an error when the given opposing account (of type $type) is invalid. + * Empty data is allowed, system will default to cash. + * + * @noinspection MoreThanThreeArgumentsInspection + * + * @param Validator $validator + * @param string $type + * @param int|null $accountId + * @param null|string $accountName + * @param string $idField + * + * @return null|Account + */ + protected function opposingAccountExists(Validator $validator, string $type, ?int $accountId, ?string $accountName, string $idField): ?Account + { + $accountId = (int)$accountId; + $accountName = (string)$accountName; + // both empty? done! + if ($accountId < 1 && \strlen($accountName) === 0) { + return null; + } + if ($accountId !== 0) { + // ID belongs to user and is $type account: + /** @var AccountRepositoryInterface $repository */ + $repository = app(AccountRepositoryInterface::class); + $repository->setUser(auth()->user()); + $set = $repository->getAccountsById([$accountId]); + if ($set->count() === 1) { + /** @var Account $first */ + $first = $set->first(); + if ($first->accountType->type !== $type) { + $validator->errors()->add($idField, trans('validation.belongs_user')); + + return null; + } + + // we ignore the account name at this point. + return $first; + } + } + + // not having an opposing account by this name is NOT a problem. + return null; + } + + /** + * TODO can be a rule? + * + * Validates the given account information. Switches on given transaction type. + * + * @param Validator $validator + */ + protected function validateAccountInformation(Validator $validator): void + { + $data = $validator->getData(); + $transactions = $data['transactions'] ?? []; + $idField = 'description'; + $transactionType = $data['type'] ?? 'false'; + foreach ($transactions as $index => $transaction) { + $sourceId = isset($transaction['source_id']) ? (int)$transaction['source_id'] : null; + $sourceName = $transaction['source_name'] ?? null; + $destinationId = isset($transaction['destination_id']) ? (int)$transaction['destination_id'] : null; + $destinationName = $transaction['destination_name'] ?? null; + $sourceAccount = null; + $destinationAccount = null; + switch ($transactionType) { + case 'withdrawal': + $idField = 'transactions.' . $index . '.source_id'; + $nameField = 'transactions.' . $index . '.source_name'; + $sourceAccount = $this->assetAccountExists($validator, $sourceId, $sourceName, $idField, $nameField); + $idField = 'transactions.' . $index . '.destination_id'; + $destinationAccount = $this->opposingAccountExists($validator, AccountType::EXPENSE, $destinationId, $destinationName, $idField); + break; + case 'deposit': + $idField = 'transactions.' . $index . '.source_id'; + $sourceAccount = $this->opposingAccountExists($validator, AccountType::REVENUE, $sourceId, $sourceName, $idField); + + $idField = 'transactions.' . $index . '.destination_id'; + $nameField = 'transactions.' . $index . '.destination_name'; + $destinationAccount = $this->assetAccountExists($validator, $destinationId, $destinationName, $idField, $nameField); + break; + case 'transfer': + $idField = 'transactions.' . $index . '.source_id'; + $nameField = 'transactions.' . $index . '.source_name'; + $sourceAccount = $this->assetAccountExists($validator, $sourceId, $sourceName, $idField, $nameField); + + $idField = 'transactions.' . $index . '.destination_id'; + $nameField = 'transactions.' . $index . '.destination_name'; + $destinationAccount = $this->assetAccountExists($validator, $destinationId, $destinationName, $idField, $nameField); + break; + default: + $validator->errors()->add($idField, trans('validation.invalid_account_info')); + + return; + + } + // add some errors in case of same account submitted: + if (null !== $sourceAccount && null !== $destinationAccount && $sourceAccount->id === $destinationAccount->id) { + $validator->errors()->add($idField, trans('validation.source_equals_destination')); + } + } + } + + /** + * @param Validator $validator + */ + private function validRepeatsUntil(Validator $validator): void + { + $data = $validator->getData(); + $repetitions = $data['nr_of_repetitions'] ?? null; + $repeatUntil = $data['repeat_until'] ?? null; + if (null !== $repetitions && null !== $repeatUntil) { + // expect a date OR count: + $validator->errors()->add('repeat_until', trans('validation.require_repeat_until')); + $validator->errors()->add('repetitions', trans('validation.require_repeat_until')); + + return; + } + } + + /** + * TODO merge this in a rule somehow. + * + * @param Validator $validator + */ + private function validRepetitionMoment(Validator $validator): void + { + $data = $validator->getData(); + $repetitions = $data['repetitions'] ?? []; + /** + * @var int $index + * @var array $repetition + */ + foreach ($repetitions as $index => $repetition) { + switch ($repetition['type']) { + default: + $validator->errors()->add(sprintf('repetitions.%d.type', $index), trans('validation.valid_recurrence_rep_type')); + + return; + case 'daily': + if ('' !== (string)$repetition['moment']) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + } + + return; + case 'monthly': + $dayOfMonth = (int)$repetition['moment']; + if ($dayOfMonth < 1 || $dayOfMonth > 31) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + } + + return; + case 'ndom': + $parameters = explode(',', $repetition['moment']); + if (\count($parameters) !== 2) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + + return; + } + $nthDay = (int)($parameters[0] ?? 0.0); + $dayOfWeek = (int)($parameters[1] ?? 0.0); + if ($nthDay < 1 || $nthDay > 5) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + + return; + } + if ($dayOfWeek < 1 || $dayOfWeek > 7) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + + return; + } + + return; + case 'weekly': + $dayOfWeek = (int)$repetition['moment']; + if ($dayOfWeek < 1 || $dayOfWeek > 7) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + + return; + } + break; + case 'yearly': + try { + Carbon::createFromFormat('Y-m-d', $repetition['moment']); + } catch (InvalidArgumentException $e) { + $validator->errors()->add(sprintf('repetitions.%d.moment', $index), trans('validation.valid_recurrence_rep_moment')); + + return; + } + } + } + } +} \ No newline at end of file diff --git a/app/Api/V1/Requests/TransactionRequest.php b/app/Api/V1/Requests/TransactionRequest.php index ab03ec561a..ce0c27e1be 100644 --- a/app/Api/V1/Requests/TransactionRequest.php +++ b/app/Api/V1/Requests/TransactionRequest.php @@ -188,6 +188,8 @@ class TransactionRequest extends Request /** * Throws an error when this asset account is invalid. * + * @noinspection MoreThanThreeArgumentsInspection + * * @param Validator $validator * @param int|null $accountId * @param null|string $accountName @@ -256,7 +258,7 @@ class TransactionRequest extends Request * * @param Validator $validator */ - protected function checkValidDescriptions(Validator $validator) + protected function checkValidDescriptions(Validator $validator): void { $data = $validator->getData(); $transactions = $data['transactions'] ?? []; @@ -317,6 +319,8 @@ class TransactionRequest extends Request } /** + * TODO can be made a rule? + * * If the transactions contain foreign amounts, there must also be foreign currency information. * * @param Validator $validator @@ -342,6 +346,8 @@ class TransactionRequest extends Request * Throws an error when the given opposing account (of type $type) is invalid. * Empty data is allowed, system will default to cash. * + * @noinspection MoreThanThreeArgumentsInspection + * * @param Validator $validator * @param string $type * @param int|null $accountId @@ -454,7 +460,7 @@ class TransactionRequest extends Request * * @throws FireflyException */ - protected function validateSplitAccounts(Validator $validator) + protected function validateSplitAccounts(Validator $validator): void { $data = $validator->getData(); $count = isset($data['transactions']) ? \count($data['transactions']) : 0; diff --git a/app/Factory/RecurrenceFactory.php b/app/Factory/RecurrenceFactory.php index b4cd95d86d..f144022ed7 100644 --- a/app/Factory/RecurrenceFactory.php +++ b/app/Factory/RecurrenceFactory.php @@ -30,6 +30,7 @@ use FireflyIII\Services\Internal\Support\RecurringTransactionTrait; use FireflyIII\Services\Internal\Support\TransactionServiceTrait; use FireflyIII\Services\Internal\Support\TransactionTypeTrait; use FireflyIII\User; +use Log; /** * Class RecurrenceFactory @@ -44,12 +45,17 @@ class RecurrenceFactory /** * @param array $data * - * @throws FireflyException * @return Recurrence */ - public function create(array $data): Recurrence + public function create(array $data): ?Recurrence { - $type = $this->findTransactionType(ucfirst($data['recurrence']['type'])); + try { + $type = $this->findTransactionType(ucfirst($data['recurrence']['type'])); + } catch (FireflyException $e) { + Log::error($e->getMessage()); + + return null; + } $repetitions = (int)$data['recurrence']['repetitions']; $recurrence = new Recurrence( [ diff --git a/app/Jobs/CreateRecurringTransactions.php b/app/Jobs/CreateRecurringTransactions.php index 05b036b5ed..281d2e146c 100644 --- a/app/Jobs/CreateRecurringTransactions.php +++ b/app/Jobs/CreateRecurringTransactions.php @@ -150,9 +150,9 @@ class CreateRecurringTransactions implements ShouldQueue 'budget_name' => null, 'category_id' => null, 'category_name' => $this->repository->getCategory($transaction), - 'source_id' => $transaction->source_account_id, + 'source_id' => $transaction->source_id, 'source_name' => null, - 'destination_id' => $transaction->destination_account_id, + 'destination_id' => $transaction->destination_id, 'destination_name' => null, 'foreign_currency_id' => $transaction->foreign_currency_id, 'foreign_currency_code' => null, diff --git a/app/Models/RecurrenceTransaction.php b/app/Models/RecurrenceTransaction.php index a9ad116f02..8687733929 100644 --- a/app/Models/RecurrenceTransaction.php +++ b/app/Models/RecurrenceTransaction.php @@ -35,8 +35,8 @@ use Illuminate\Database\Eloquent\SoftDeletes; * * @property int $transaction_currency_id, * @property int $foreign_currency_id - * @property int $source_account_id - * @property int $destination_account_id + * @property int $source_id + * @property int $destination_id * @property string $amount * @property string $foreign_amount * @property string $description @@ -62,7 +62,7 @@ class RecurrenceTransaction extends Model ]; /** @var array */ protected $fillable - = ['recurrence_id', 'transaction_currency_id', 'foreign_currency_id', 'source_account_id', 'destination_account_id', 'amount', 'foreign_amount', + = ['recurrence_id', 'transaction_currency_id', 'foreign_currency_id', 'source_id', 'destination_id', 'amount', 'foreign_amount', 'description']; /** @var string */ protected $table = 'recurrences_transactions'; diff --git a/app/Models/RecurrenceTransactionMeta.php b/app/Models/RecurrenceTransactionMeta.php index 66d8cdd85c..aa246529cf 100644 --- a/app/Models/RecurrenceTransactionMeta.php +++ b/app/Models/RecurrenceTransactionMeta.php @@ -39,7 +39,7 @@ class RecurrenceTransactionMeta extends Model use SoftDeletes; /** @var array */ protected $casts - = [ + = [ 'created_at' => 'datetime', 'updated_at' => 'datetime', 'deleted_at' => 'datetime', diff --git a/app/Repositories/Recurring/RecurringRepository.php b/app/Repositories/Recurring/RecurringRepository.php index 1046fb41f3..b4d1ae1e91 100644 --- a/app/Repositories/Recurring/RecurringRepository.php +++ b/app/Repositories/Recurring/RecurringRepository.php @@ -508,7 +508,6 @@ class RecurringRepository implements RecurringRepositoryInterface /** * @param array $data * - * @throws FireflyException * @return Recurrence */ public function store(array $data): Recurrence diff --git a/app/Repositories/Recurring/RecurringRepositoryInterface.php b/app/Repositories/Recurring/RecurringRepositoryInterface.php index 98751b3f39..92f824d25a 100644 --- a/app/Repositories/Recurring/RecurringRepositoryInterface.php +++ b/app/Repositories/Recurring/RecurringRepositoryInterface.php @@ -155,7 +155,6 @@ interface RecurringRepositoryInterface * * @param array $data * - * @throws FireflyException * @return Recurrence */ public function store(array $data): Recurrence; diff --git a/app/Services/Internal/Destroy/RecurrenceDestroyService.php b/app/Services/Internal/Destroy/RecurrenceDestroyService.php index 968f249235..6f59892f4f 100644 --- a/app/Services/Internal/Destroy/RecurrenceDestroyService.php +++ b/app/Services/Internal/Destroy/RecurrenceDestroyService.php @@ -25,6 +25,7 @@ namespace FireflyIII\Services\Internal\Destroy; use Exception; use FireflyIII\Models\Recurrence; +use FireflyIII\Models\RecurrenceTransaction; use Log; /** @@ -39,6 +40,19 @@ class RecurrenceDestroyService public function destroy(Recurrence $recurrence): void { try { + // delete all meta data + $recurrence->recurrenceMeta()->delete(); + + // delete all transactions. + /** @var RecurrenceTransaction $transaction */ + foreach($recurrence->recurrenceTransactions as $transaction) { + $transaction->recurrenceTransactionMeta()->delete(); + $transaction->delete(); + } + // delete all repetitions + $recurrence->recurrenceRepetitions()->delete(); + + // delete recurrence $recurrence->delete(); } catch (Exception $e) { // @codeCoverageIgnore Log::error(sprintf('Could not delete recurrence: %s', $e->getMessage())); // @codeCoverageIgnore diff --git a/app/Services/Internal/Support/RecurringTransactionTrait.php b/app/Services/Internal/Support/RecurringTransactionTrait.php index 4de55141ce..c57dce1f82 100644 --- a/app/Services/Internal/Support/RecurringTransactionTrait.php +++ b/app/Services/Internal/Support/RecurringTransactionTrait.php @@ -66,8 +66,6 @@ trait RecurringTransactionTrait /** * @param Recurrence $recurrence * @param array $transactions - * - * @throws FireflyException */ public function createTransactions(Recurrence $recurrence, array $transactions): void { @@ -76,19 +74,17 @@ trait RecurringTransactionTrait $source = null; $destination = null; switch ($recurrence->transactionType->type) { - default: - throw new FireflyException(sprintf('Cannot create "%s".', $recurrence->transactionType->type)); case TransactionType::WITHDRAWAL: - $source = $this->findAccount(AccountType::ASSET, $array['source_account_id'], null); - $destination = $this->findAccount(AccountType::EXPENSE, null, $array['destination_account_name']); + $source = $this->findAccount(AccountType::ASSET, $array['source_id'], $array['source_name'] ?? null); + $destination = $this->findAccount(AccountType::EXPENSE, $array['destination_id'] ?? null, $array['destination_name']); break; case TransactionType::DEPOSIT: - $source = $this->findAccount(AccountType::REVENUE, null, $array['source_account_name']); - $destination = $this->findAccount(AccountType::ASSET, $array['destination_account_id'], null); + $source = $this->findAccount(AccountType::REVENUE, $array['source_id'] ?? null, $array['source_name']); + $destination = $this->findAccount(AccountType::ASSET, $array['destination_id'], $array['destination_name'] ?? null); break; case TransactionType::TRANSFER: - $source = $this->findAccount(AccountType::ASSET, $array['source_account_id'], null); - $destination = $this->findAccount(AccountType::ASSET, $array['destination_account_id'], null); + $source = $this->findAccount(AccountType::ASSET, $array['source_id'], $array['source_name'] ?? null); + $destination = $this->findAccount(AccountType::ASSET, $array['destination_id'], $array['destination_name'] ?? null); break; } @@ -96,9 +92,9 @@ trait RecurringTransactionTrait [ 'recurrence_id' => $recurrence->id, 'transaction_currency_id' => $array['transaction_currency_id'], - 'foreign_currency_id' => '' === (string)$array['foreign_amount'] ? null : $array['foreign_currency_id'], - 'source_account_id' => $source->id, - 'destination_account_id' => $destination->id, + 'foreign_currency_id' => '' === (string)$array['foreign_currency_id'] ? null : $array['foreign_currency_id'], + 'source_id' => $source->id, + 'destination_id' => $destination->id, 'amount' => $array['amount'], 'foreign_amount' => '' === (string)$array['foreign_amount'] ? null : (string)$array['foreign_amount'], 'description' => $array['description'], diff --git a/database/migrations/2018_06_08_200526_changes_for_v475.php b/database/migrations/2018_06_08_200526_changes_for_v475.php index 366ea260f9..fa85ed5456 100644 --- a/database/migrations/2018_06_08_200526_changes_for_v475.php +++ b/database/migrations/2018_06_08_200526_changes_for_v475.php @@ -66,8 +66,8 @@ class ChangesForV475 extends Migration $table->integer('recurrence_id', false, true); $table->integer('transaction_currency_id', false, true); $table->integer('foreign_currency_id', false, true)->nullable(); - $table->integer('source_account_id', false, true); - $table->integer('destination_account_id', false, true); + $table->integer('source_id', false, true); + $table->integer('destination_id', false, true); $table->decimal('amount', 22, 12); $table->decimal('foreign_amount', 22, 12)->nullable(); @@ -77,8 +77,8 @@ class ChangesForV475 extends Migration $table->foreign('recurrence_id')->references('id')->on('recurrences')->onDelete('cascade'); $table->foreign('transaction_currency_id')->references('id')->on('transaction_currencies')->onDelete('cascade'); $table->foreign('foreign_currency_id')->references('id')->on('transaction_currencies')->onDelete('set null'); - $table->foreign('source_account_id')->references('id')->on('accounts')->onDelete('cascade'); - $table->foreign('destination_account_id')->references('id')->on('accounts')->onDelete('cascade'); + $table->foreign('source_id')->references('id')->on('accounts')->onDelete('cascade'); + $table->foreign('destination_id')->references('id')->on('accounts')->onDelete('cascade'); } ); diff --git a/public/js/ff/recurring/create.js b/public/js/ff/recurring/create.js index 7e1deebfff..4d47190f70 100644 --- a/public/js/ff/recurring/create.js +++ b/public/js/ff/recurring/create.js @@ -155,15 +155,15 @@ function initializeAutoComplete() { ); }); - if ($('input[name="destination_account_name"]').length > 0) { + if ($('input[name="destination_name"]').length > 0) { $.getJSON('json/expense-accounts').done(function (data) { - $('input[name="destination_account_name"]').typeahead({source: data, autoSelect: false}); + $('input[name="destination_name"]').typeahead({source: data, autoSelect: false}); }); } - if ($('input[name="source_account_name"]').length > 0) { + if ($('input[name="source_name"]').length > 0) { $.getJSON('json/revenue-accounts').done(function (data) { - $('input[name="source_account_name"]').typeahead({source: data, autoSelect: false}); + $('input[name="source_name"]').typeahead({source: data, autoSelect: false}); }); } @@ -208,16 +208,16 @@ function updateFormFields() { if (transactionType === 'withdrawal') { // hide source account name: - $('#source_account_name_holder').hide(); + $('#source_name_holder').hide(); // show source account ID: - $('#source_account_id_holder').show(); - - // show destination name: - $('#destination_account_name_holder').show(); + $('#source_id_holder').show(); // hide destination ID: - $('#destination_account_id_holder').hide(); + $('#destination_id_holder').hide(); + + // show destination name: + $('#destination_name_holder').show(); // show budget $('#budget_id_holder').show(); @@ -227,19 +227,19 @@ function updateFormFields() { } if (transactionType === 'deposit') { - $('#source_account_name_holder').show(); - $('#source_account_id_holder').hide(); - $('#destination_account_name_holder').hide(); - $('#destination_account_id_holder').show(); + $('#source_name_holder').show(); + $('#source_id_holder').hide(); + $('#destination_name_holder').hide(); + $('#destination_id_holder').show(); $('#budget_id_holder').hide(); $('#piggy_bank_id_holder').hide(); } if (transactionType === 'transfer') { - $('#source_account_name_holder').hide(); - $('#source_account_id_holder').show(); - $('#destination_account_name_holder').hide(); - $('#destination_account_id_holder').show(); + $('#source_name_holder').hide(); + $('#source_id_holder').show(); + $('#destination_name_holder').hide(); + $('#destination_id_holder').show(); $('#budget_id_holder').hide(); $('#piggy_bank_id_holder').show(); } diff --git a/public/js/ff/recurring/edit.js b/public/js/ff/recurring/edit.js index f499280b57..24ee67c1e8 100644 --- a/public/js/ff/recurring/edit.js +++ b/public/js/ff/recurring/edit.js @@ -156,15 +156,15 @@ function initializeAutoComplete() { ); }); - if ($('input[name="destination_account_name"]').length > 0) { + if ($('input[name="destination_name"]').length > 0) { $.getJSON('json/expense-accounts').done(function (data) { - $('input[name="destination_account_name"]').typeahead({source: data, autoSelect: false}); + $('input[name="destination_name"]').typeahead({source: data, autoSelect: false}); }); } - if ($('input[name="source_account_name"]').length > 0) { + if ($('input[name="source_name"]').length > 0) { $.getJSON('json/revenue-accounts').done(function (data) { - $('input[name="source_account_name"]').typeahead({source: data, autoSelect: false}); + $('input[name="source_name"]').typeahead({source: data, autoSelect: false}); }); } @@ -209,16 +209,16 @@ function updateFormFields() { if (transactionType === 'withdrawal') { // hide source account name: - $('#source_account_name_holder').hide(); + $('#source_name_holder').hide(); // show source account ID: - $('#source_account_id_holder').show(); + $('#source_id_holder').show(); // show destination name: - $('#destination_account_name_holder').show(); + $('#destination_name_holder').show(); // hide destination ID: - $('#destination_account_id_holder').hide(); + $('#destination_id_holder').hide(); // show budget $('#budget_id_holder').show(); @@ -228,19 +228,19 @@ function updateFormFields() { } if (transactionType === 'deposit') { - $('#source_account_name_holder').show(); - $('#source_account_id_holder').hide(); - $('#destination_account_name_holder').hide(); - $('#destination_account_id_holder').show(); + $('#source_name_holder').show(); + $('#source_id_holder').hide(); + $('#destination_name_holder').hide(); + $('#destination_id_holder').show(); $('#budget_id_holder').hide(); $('#piggy_bank_id_holder').hide(); } if (transactionType === 'transfer') { - $('#source_account_name_holder').hide(); - $('#source_account_id_holder').show(); - $('#destination_account_name_holder').hide(); - $('#destination_account_id_holder').show(); + $('#source_name_holder').hide(); + $('#source_id_holder').show(); + $('#destination_name_holder').hide(); + $('#destination_id_holder').show(); $('#budget_id_holder').hide(); $('#piggy_bank_id_holder').show(); } diff --git a/resources/lang/en_US/form.php b/resources/lang/en_US/form.php index 773f713fa0..fe18364b8e 100644 --- a/resources/lang/en_US/form.php +++ b/resources/lang/en_US/form.php @@ -24,66 +24,66 @@ declare(strict_types=1); return [ // new user: - 'bank_name' => 'Bank name', - 'bank_balance' => 'Balance', - 'savings_balance' => 'Savings balance', - 'credit_card_limit' => 'Credit card limit', - 'automatch' => 'Match automatically', - 'skip' => 'Skip', - 'name' => 'Name', - 'active' => 'Active', - 'amount_min' => 'Minimum amount', - 'amount_max' => 'Maximum amount', - 'match' => 'Matches on', - 'strict' => 'Strict mode', - 'repeat_freq' => 'Repeats', - 'journal_currency_id' => 'Currency', - 'currency_id' => 'Currency', - 'transaction_currency_id' => 'Currency', - 'external_ip' => 'Your server\'s external IP', - 'attachments' => 'Attachments', - 'journal_amount' => 'Amount', - 'journal_source_account_name' => 'Revenue account (source)', - 'journal_source_account_id' => 'Asset account (source)', - 'BIC' => 'BIC', - 'verify_password' => 'Verify password security', - 'source_account' => 'Source account', - 'destination_account' => 'Destination account', - 'journal_destination_account_id' => 'Asset account (destination)', - 'asset_destination_account' => 'Asset account (destination)', - 'asset_source_account' => 'Asset account (source)', - 'journal_description' => 'Description', - 'note' => 'Notes', - 'split_journal' => 'Split this transaction', - 'split_journal_explanation' => 'Split this transaction in multiple parts', - 'currency' => 'Currency', - 'account_id' => 'Asset account', - 'budget_id' => 'Budget', - 'openingBalance' => 'Opening balance', - 'tagMode' => 'Tag mode', - 'tag_position' => 'Tag location', - 'virtualBalance' => 'Virtual balance', - 'targetamount' => 'Target amount', - 'accountRole' => 'Account role', - 'openingBalanceDate' => 'Opening balance date', - 'ccType' => 'Credit card payment plan', - 'ccMonthlyPaymentDate' => 'Credit card monthly payment date', - 'piggy_bank_id' => 'Piggy bank', - 'returnHere' => 'Return here', - 'returnHereExplanation' => 'After storing, return here to create another one.', - 'returnHereUpdateExplanation' => 'After updating, return here.', - 'description' => 'Description', - 'expense_account' => 'Expense account', - 'revenue_account' => 'Revenue account', - 'decimal_places' => 'Decimal places', - 'exchange_rate_instruction' => 'Foreign currencies', - 'source_amount' => 'Amount (source)', - 'destination_amount' => 'Amount (destination)', - 'native_amount' => 'Native amount', - 'new_email_address' => 'New email address', - 'verification' => 'Verification', - 'api_key' => 'API key', - 'remember_me' => 'Remember me', + 'bank_name' => 'Bank name', + 'bank_balance' => 'Balance', + 'savings_balance' => 'Savings balance', + 'credit_card_limit' => 'Credit card limit', + 'automatch' => 'Match automatically', + 'skip' => 'Skip', + 'name' => 'Name', + 'active' => 'Active', + 'amount_min' => 'Minimum amount', + 'amount_max' => 'Maximum amount', + 'match' => 'Matches on', + 'strict' => 'Strict mode', + 'repeat_freq' => 'Repeats', + 'journal_currency_id' => 'Currency', + 'currency_id' => 'Currency', + 'transaction_currency_id' => 'Currency', + 'external_ip' => 'Your server\'s external IP', + 'attachments' => 'Attachments', + 'journal_amount' => 'Amount', + 'journal_source_name' => 'Revenue account (source)', + 'journal_source_id' => 'Asset account (source)', + 'BIC' => 'BIC', + 'verify_password' => 'Verify password security', + 'source_account' => 'Source account', + 'destination_account' => 'Destination account', + 'journal_destination_id' => 'Asset account (destination)', + 'asset_destination_account' => 'Asset account (destination)', + 'asset_source_account' => 'Asset account (source)', + 'journal_description' => 'Description', + 'note' => 'Notes', + 'split_journal' => 'Split this transaction', + 'split_journal_explanation' => 'Split this transaction in multiple parts', + 'currency' => 'Currency', + 'account_id' => 'Asset account', + 'budget_id' => 'Budget', + 'openingBalance' => 'Opening balance', + 'tagMode' => 'Tag mode', + 'tag_position' => 'Tag location', + 'virtualBalance' => 'Virtual balance', + 'targetamount' => 'Target amount', + 'accountRole' => 'Account role', + 'openingBalanceDate' => 'Opening balance date', + 'ccType' => 'Credit card payment plan', + 'ccMonthlyPaymentDate' => 'Credit card monthly payment date', + 'piggy_bank_id' => 'Piggy bank', + 'returnHere' => 'Return here', + 'returnHereExplanation' => 'After storing, return here to create another one.', + 'returnHereUpdateExplanation' => 'After updating, return here.', + 'description' => 'Description', + 'expense_account' => 'Expense account', + 'revenue_account' => 'Revenue account', + 'decimal_places' => 'Decimal places', + 'exchange_rate_instruction' => 'Foreign currencies', + 'source_amount' => 'Amount (source)', + 'destination_amount' => 'Amount (destination)', + 'native_amount' => 'Native amount', + 'new_email_address' => 'New email address', + 'verification' => 'Verification', + 'api_key' => 'API key', + 'remember_me' => 'Remember me', 'source_account_asset' => 'Source account (asset account)', 'destination_account_expense' => 'Destination account (expense account)', diff --git a/resources/lang/en_US/validation.php b/resources/lang/en_US/validation.php index 66df306ae7..f19b66f0e0 100644 --- a/resources/lang/en_US/validation.php +++ b/resources/lang/en_US/validation.php @@ -37,6 +37,8 @@ return [ 'invalid_selection' => 'Your selection is invalid', 'belongs_user' => 'This value is invalid for this field.', 'at_least_one_transaction' => 'Need at least one transaction.', + 'at_least_one_repetition' => 'Need at least one repetition.', + 'require_repeat_until' => 'Require either a number of repetitions, or an end date (repeats_until). Not both.', 'require_currency_info' => 'The content of this field is invalid without currency information.', 'equal_description' => 'Transaction description should not equal global description.', 'file_invalid_mime' => 'File ":name" is of type ":mime" which is not accepted as a new upload.', @@ -115,6 +117,8 @@ return [ 'unique_piggy_bank_for_user' => 'The name of the piggy bank must be unique.', 'secure_password' => 'This is not a secure password. Please try again. For more information, visit http://bit.ly/FF3-password-security', 'valid_recurrence_rep_type' => 'Invalid repetition type for recurring transactions', + 'valid_recurrence_rep_moment' => 'Invalid repetition moment for this type of repetition.', + 'invalid_account_info' => 'Invalid account information', 'attributes' => [ 'email' => 'email address', 'description' => 'description', diff --git a/resources/views/recurring/create.twig b/resources/views/recurring/create.twig index d88455cfaa..1c098a5d87 100644 --- a/resources/views/recurring/create.twig +++ b/resources/views/recurring/create.twig @@ -89,16 +89,16 @@ {{ ExpandedForm.amountNoCurrency('amount', []) }} {# source account if withdrawal, or if transfer: #} - {{ ExpandedForm.activeAssetAccountList('source_account_id', null, {label: trans('form.asset_source_account')}) }} + {{ ExpandedForm.activeAssetAccountList('source_id', null, {label: trans('form.asset_source_account')}) }} {# source account name for deposits: #} - {{ ExpandedForm.text('source_account_name', null, {label: trans('form.revenue_account')}) }} + {{ ExpandedForm.text('source_name', null, {label: trans('form.revenue_account')}) }} {# destination if deposit or transfer: #} - {{ ExpandedForm.activeAssetAccountList('destination_account_id', null, {label: trans('form.asset_destination_account')} ) }} + {{ ExpandedForm.activeAssetAccountList('destination_id', null, {label: trans('form.asset_destination_account')} ) }} {# destination account name for withdrawals #} - {{ ExpandedForm.text('destination_account_name', null, {label: trans('form.expense_account')}) }} + {{ ExpandedForm.text('destination_name', null, {label: trans('form.expense_account')}) }} diff --git a/resources/views/recurring/edit.twig b/resources/views/recurring/edit.twig index cba7f8e113..03a5c0b4a3 100644 --- a/resources/views/recurring/edit.twig +++ b/resources/views/recurring/edit.twig @@ -88,16 +88,16 @@ {{ ExpandedForm.amountNoCurrency('amount', array.transactions[0].amount) }} {# source account if withdrawal, or if transfer: #} - {{ ExpandedForm.assetAccountList('source_account_id', array.transactions[0].source_account_id, {label: trans('form.asset_source_account')}) }} + {{ ExpandedForm.assetAccountList('source_id', array.transactions[0].source_id, {label: trans('form.asset_source_account')}) }} {# source account name for deposits: #} - {{ ExpandedForm.text('source_account_name', array.transactions[0].source_account_name, {label: trans('form.revenue_account')}) }} + {{ ExpandedForm.text('source_name', array.transactions[0].source_name, {label: trans('form.revenue_account')}) }} {# destination if deposit or transfer: #} - {{ ExpandedForm.assetAccountList('destination_account_id', array.transactions[0].destination_account_id, {label: trans('form.asset_destination_account')} ) }} + {{ ExpandedForm.assetAccountList('destination_id', array.transactions[0].destination_id, {label: trans('form.asset_destination_account')} ) }} {# destination account name for withdrawals #} - {{ ExpandedForm.text('destination_account_name', array.transactions[0].destination_account_name, {label: trans('form.expense_account')}) }} + {{ ExpandedForm.text('destination_name', array.transactions[0].destination_name, {label: trans('form.expense_account')}) }} diff --git a/resources/views/recurring/index.twig b/resources/views/recurring/index.twig index a4f39bd1d7..03fa6e5476 100644 --- a/resources/views/recurring/index.twig +++ b/resources/views/recurring/index.twig @@ -75,9 +75,9 @@ {% if null != rtt['foreign_amount'] %} ({{ formatAmountBySymbol(rtt['foreign_amount'],rtt['foreign_currency_symbol'],rtt['foreign_currency_dp']) }}), {% endif %} - {{ rtt['source_account_name'] }} + {{ rtt['source_name'] }} → - {{ rtt['destination_account_name'] }} + {{ rtt['destination_name'] }} {% endfor %} diff --git a/resources/views/recurring/show.twig b/resources/views/recurring/show.twig index 54936e929d..85703b3282 100644 --- a/resources/views/recurring/show.twig +++ b/resources/views/recurring/show.twig @@ -101,11 +101,11 @@ {% for transaction in array.transactions %} - - {{ transaction.source_account_name }} + + {{ transaction.source_name }} - - {{ transaction.destination_account_name }} + + {{ transaction.destination_name }} {{ formatAmountBySymbol(transaction.amount,transaction.currency_symbol,transaction.currency_dp) }}