feat: Add recurrent transactions #71
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
app/Http/Controllers/API/v1/TransactionController.php (1)
417-467: Add support for updating recurring transaction fields.The update method doesn't include validation rules for recurring transaction fields, preventing users from modifying recurrence settings after creation. This is a significant limitation of the feature.
Would you like me to generate the necessary validation rules and logic for updating recurring transaction fields?
🧹 Nitpick comments (5)
database/migrations/2024_07_20_125117_create_transactions_table.php (1)
37-37: ConsideronUpdate('cascade')for the self-reference FKIf a parent transaction were ever re-keyed (unlikely but possible in some data-migration scenarios) the child rows would break.
AddingonUpdate('cascade')keeps parent/child IDs consistent.- $table->foreign('parent_transaction_id')->references('id')->on('transactions')->onDelete('cascade'); + $table->foreign('parent_transaction_id') + ->references('id')->on('transactions') + ->onDelete('cascade') + ->onUpdate('cascade');app/Console/Kernel.php (1)
15-17: Avoid double execution & respect timezoneRunning daily is fine, but adding
->withoutOverlapping()and an explicit timezone prevents accidental double runs on slow queues and makes behaviour deterministic across DST changes.- $schedule->command('transactions:generate-recurring')->daily(); + $schedule->command('transactions:generate-recurring') + ->daily() + ->timezone(config('app.timezone', 'UTC')) + ->withoutOverlapping();app/Models/Transaction.php (1)
67-72: Add attribute casting for new columnsCasting prevents subtle bugs when comparing booleans or Carbon dates.
protected $casts = [ // … + 'is_recurring' => 'boolean', + 'recurrence_interval' => 'integer', + 'recurrence_ends_at' => 'datetime', ];app/Console/Commands/GenerateRecurringTransactions.php (1)
57-65: Replicate categories & files to child transaction (optional)If users attach categories/files to the master transaction they likely expect them to propagate.
Consider cloning the relationships after creation.$child = Transaction::create([ /* existing attributes */ ]); // replicate relations $child->categories()->sync($transaction->categories->pluck('id')); $transaction->files->each(fn ($file) => $child->files()->create($file->only(['path','name'])));app/Http/Controllers/API/v1/TransactionController.php (1)
199-207: Simplify conditional validation using Laravel's built-in rules.The current approach uses a separate validation call which is redundant. Laravel provides
required_iffor this exact use case.Replace the conditional validation logic by updating the initial validation rules:
-'recurrence_period' => 'nullable|string|in:daily,weekly,monthly,yearly', +'recurrence_period' => 'nullable|required_if:is_recurring,true|string|in:daily,weekly,monthly,yearly',Then remove lines 199-207 as they become unnecessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/Console/Commands/GenerateRecurringTransactions.php(1 hunks)app/Console/Kernel.php(1 hunks)app/Enums/TransactionRecurringPeriod.php(1 hunks)app/Http/Controllers/API/v1/TransactionController.php(3 hunks)app/Models/Transaction.php(2 hunks)database/migrations/2024_07_20_125117_create_transactions_table.php(1 hunks)public/docs/api.json(2 hunks)tests/Feature/TransactionsTest.php(2 hunks)
🔇 Additional comments (7)
app/Enums/TransactionRecurringPeriod.php (1)
5-13: LGTM – concise enum definition
The enum cleanly centralises the allowed values. 👍app/Http/Controllers/API/v1/TransactionController.php (2)
145-148: LGTM! OpenAPI documentation properly added for recurring transaction fields.The new properties are well-documented with clear descriptions and appropriate types.
208-211: LGTM! Date format conversion is consistent with existing patterns.The conversion follows the same approach used for other date fields in the controller.
public/docs/api.json (3)
1293-1309: LGTM! API documentation properly updated for transaction creation.The recurring transaction fields are well-documented with clear descriptions and appropriate types.
2275-2295: LGTM! Transaction schema includes all recurring transaction fields.The schema properly documents all the new recurring transaction properties including the parent_transaction_id for tracking recurring instances.
1500-1545: Update endpoint schema is missing recurring transaction fields.The transaction update endpoint doesn't document support for modifying recurring transaction settings. This should be added once the controller supports updating these fields.
Likely an incorrect or invalid review comment.
tests/Feature/TransactionsTest.php (1)
363-400: LGTM! Well-structured helper method for creating transactions.The updated
createTransactionmethod cleanly supports both regular and recurring transactions with proper parameter handling.
database/migrations/2024_07_20_125117_create_transactions_table.php
Outdated
Show resolved
Hide resolved
kofimokome
force-pushed
the
feat/recurrent-transactions
branch
from
eb80cc6 to
96295a2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
tests/Feature/TransactionsTest.php (1)
402-415: Wallet & party are created for a different user – will violate auth/ownership checks
$this->walletand$this->partyare created before$this->user, and belong to other users generated by the factories.
Most routes (incl./api/v1/transactions) are expected to allow only resources owned by the authenticated user, so these tests can fail in environments enforcing that rule.Suggested fix:
- $this->wallet = User::factory()->create()->wallets()->create([ - 'name' => 'Wallet', - 'balance' => 1000, - ]); - - $this->party = User::factory()->create()->parties()->create([ - 'name' => 'Party', - 'type' => 'personal', - ]); - $this->user = User::factory()->create(); + $this->user = User::factory()->create(); + + $this->wallet = $this->user->wallets()->create([ + 'name' => 'Wallet', + 'balance'=> 1000, + ]); + + $this->party = $this->user->parties()->create([ + 'name' => 'Party', + 'type' => 'personal', + ]);This keeps resource ownership consistent with the acting user.
♻️ Duplicate comments (1)
tests/Feature/TransactionsTest.php (1)
21-333: Entire legacy test-suite is still commented out – restore or remove dead codeThe large
/* … */block keeps >300 lines of previously-approved tests inactive, leaving basic CRUD, validation, auth & file-handling paths totally uncovered again.
This was raised in the last review but hasn’t been addressed.Keeping the code commented means:
• CI passes while critical paths are untested.
• The file contains masses of dead code that will silently rot (e.g. undefined$contentvar, missingUploadedFileimport).Either:
- Un-comment the tests and update them to work with the new recurrence logic, or
- Delete them and create replacement coverage – but do not ship commented code.
🧹 Nitpick comments (1)
tests/Feature/TransactionsTest.php (1)
335-361: Time manipulation in recurrence test is ad-hoc – use Carbon helpers for determinismManually overwriting
created_atand calling the command works but couples the test to real time. Use Carbon’s test helpers (Carbon::setTestNow()/travel()) for clearer intent and reproducibility:Carbon::setTestNow(now()->subDays(2)); Artisan::call('transactions:generate-recurring'); Carbon::setTestNow(); // clearAlso ensure the API actually returns
is_recurring,recurrence_period, andrecurrence_interval; otherwise the earlier assertions will fail.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/Console/Commands/GenerateRecurringTransactions.php(1 hunks)app/Console/Kernel.php(1 hunks)app/Enums/TransactionRecurringPeriod.php(1 hunks)app/Http/Controllers/API/v1/TransactionController.php(6 hunks)app/Models/Transaction.php(2 hunks)database/migrations/2024_07_20_125117_create_transactions_table.php(1 hunks)public/docs/api.json(3 hunks)tests/Feature/TransactionsTest.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- app/Enums/TransactionRecurringPeriod.php
- app/Console/Kernel.php
- app/Console/Commands/GenerateRecurringTransactions.php
- database/migrations/2024_07_20_125117_create_transactions_table.php
- app/Models/Transaction.php
- public/docs/api.json
- app/Http/Controllers/API/v1/TransactionController.php
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run Tests
🔇 Additional comments (1)
tests/Feature/TransactionsTest.php (1)
363-400:createTransactionhelper omits recurrence fields from response assertionsThe helper asserts a fixed JSON structure that excludes
is_recurring,recurrence_period, andrecurrence_interval, yet tests later expect those keys. The mismatch will cause brittle/false-positive tests.- 'type', - 'amount', - 'description', - 'datetime', - 'party_id', - 'wallet_id', - 'user_id', - 'client_generated_id', + 'type', + 'amount', + 'description', + 'datetime', + 'party_id', + 'wallet_id', + 'user_id', + 'client_generated_id', + 'is_recurring', + 'recurrence_period', + 'recurrence_interval',Add these fields (or use
assertJsonStructure(['data' => array_merge([...], $extraFields)])to keep assertions in sync with expectations.
kofimokome
force-pushed
the
feat/recurrent-transactions
branch
from
96295a2 to
f2abb8b
Compare
There was a problem hiding this comment.
I request a couple of changes for better design and scalability.
Apart from the fact that, a Job is more suitable than a command and also that you should centralize the logic to create recurring transactions (preferably in a Service) are is a high level justification for the other change requests:
- Currently you need scan all transactions to filter which ones are recurring, whether is a command or a job, this is going to be additional load for the server.
- The transactions table is bloated needlessly
- Deleting original transaction would delete all recurrences (when it should simply remove the rule)
Verdict : Lets add RecurringTransactionRule and use that style instead.
database/migrations/2024_07_20_125117_create_transactions_table.php
Outdated
Show resolved
Hide resolved
database/migrations/2024_07_20_125117_create_transactions_table.php
Outdated
Show resolved
Hide resolved
kofimokome
force-pushed
the
feat/recurrent-transactions
branch
from
f2abb8b to
f3c3efe
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/Jobs/RecurrentTransactionJob.php (2)
31-32: Fix typo in log message."Handing" should be "Handling".
- logger()->info('Handing recurrent transaction'); + logger()->info('Handling recurrent transaction');
61-64: Improve error logging with more context.The current error logging lacks context about which recurring transaction failed.
} catch (\Exception $e) { - logger()->error('Exception occurred'); - logger()->error($e->getMessage()); + logger()->error('Exception occurred while processing recurring transaction '.$this->recurring_transaction_data['id'], [ + 'error' => $e->getMessage(), + 'trace' => $e->getTraceAsString() + ]); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/Console/Commands/GenerateRecurringTransactions.php(1 hunks)app/Console/Kernel.php(1 hunks)app/Enums/TransactionRecurringPeriod.php(1 hunks)app/Http/Controllers/API/v1/TransactionController.php(11 hunks)app/Jobs/RecurrentTransactionJob.php(1 hunks)app/Models/RecurringTransaction.php(1 hunks)app/Models/Transaction.php(5 hunks)app/helpers.php(2 hunks)config/queue.php(1 hunks)database/migrations/2024_07_20_125117_create_transactions_table.php(1 hunks)database/migrations/2025_06_19_125117_create_recurring_transactions_table.php(1 hunks)database/migrations/2025_06_20_125110_create_jobs_table.php(1 hunks)public/docs/api.json(5 hunks)tests/Feature/TransactionsTest.php(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- database/migrations/2025_06_20_125110_create_jobs_table.php
🚧 Files skipped from review as they are similar to previous changes (8)
- app/Console/Kernel.php
- database/migrations/2024_07_20_125117_create_transactions_table.php
- app/Enums/TransactionRecurringPeriod.php
- app/Console/Commands/GenerateRecurringTransactions.php
- app/Models/Transaction.php
- tests/Feature/TransactionsTest.php
- public/docs/api.json
- app/Http/Controllers/API/v1/TransactionController.php
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run Tests
🔇 Additional comments (5)
config/queue.php (1)
16-16: LGTM! Configuration change aligns with recurring transaction requirements.Switching from 'sync' to 'database' driver is necessary to support the delayed job processing required for recurring transactions.
app/Models/RecurringTransaction.php (2)
26-32: LGTM! Fillable attributes are properly defined.The fillable array correctly includes all necessary attributes for mass assignment.
34-37: LGTM! Relationship definition is correct.The belongsTo relationship with Transaction is properly defined.
database/migrations/2025_06_19_125117_create_recurring_transactions_table.php (2)
12-25: LGTM! Well-structured migration with proper constraints.The migration correctly defines:
- Appropriate data types and constraints
- Foreign key relationship with cascade delete
- Nullable fields where appropriate
- Default values for recurrence_interval
30-33: LGTM! Proper rollback implementation.The down method correctly drops the table if it exists.
kofimokome
force-pushed
the
feat/recurrent-transactions
branch
3 times, most recently
from
0b8f81f to
9c696be
Compare
kofimokome
force-pushed
the
feat/recurrent-transactions
branch
from
9c696be to
46c6a78
Compare
|
Having a memory leak in the test cases 😓 . Still trying to figure out why. It's coming from the transaction job. that is why the tests fail |
WalkthroughThis change introduces comprehensive support for recurring transactions in the application. It adds a new enum ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (6)
app/Enums/TransactionRecurringPeriod.php (1)
5-13: Well-structured enum for recurring periods.The enum provides good type safety and covers the most common recurrence patterns.
The past suggestion about adding a
CUSTOMcase remains valid for enhanced flexibility in future iterations.app/Jobs/RecurrentTransactionJob.php (1)
43-65: Remove duplicated condition check for recurrence end date.The condition checking if recurrence hasn't ended is duplicated on lines 48 and 55, making the code harder to maintain.
if ( ($this->recurrence_period == $recurring_transaction->recurrence_period) && ($this->recurrence_interval == $recurring_transaction->recurrence_interval) - && - ($recurring_transaction->recurrence_ends_at >= now() || is_null($recurring_transaction->recurrence_ends_at)) ) { - logger()->info('Running recurrent transaction with id '.$this->id); - $new_transaction = $recurring_transaction->transaction->replicate(); - $new_transaction->save(); + // Check if recurrence has ended + $hasEnded = $recurring_transaction->recurrence_ends_at && $recurring_transaction->recurrence_ends_at < now(); + + if (!$hasEnded) { + logger()->info('Running recurrent transaction with id '.$this->id); + $new_transaction = $recurring_transaction->transaction->replicate(); + $new_transaction->save(); - // 3. schedule next transaction - if ($recurring_transaction->recurrence_ends_at > now() || is_null($recurring_transaction->recurrence_ends_at)) { $next_date = get_next_transaction_schedule_date($recurring_transaction); logger()->info('Next transaction date for '.$this->id.": {$next_date}"); RecurrentTransactionJob::dispatch( id: $recurring_transaction->id, recurrence_period: $recurring_transaction->recurrence_period, recurrence_interval: $recurring_transaction->recurrence_interval )->delay($next_date); + } else { + logger()->info('Recurrent transaction '.$this->id.' has ended'); }app/Console/Commands/GenerateRecurringTransactions.php (3)
9-9: Duplicate of previous concern: Consider using a job instead of a command.This architectural concern was previously raised and remains valid. Commands cannot be queued, retried, or scaled horizontally like jobs can. For recurring transaction generation that may need to handle failures gracefully and process large datasets, a job would be more appropriate.
32-36: Duplicate of previous concern: Process large datasets in chunks.Loading all recurring transactions at once can cause memory exhaustion on systems with many recurring transactions. The previous suggestion to use
chunkByIdremains valid and should be implemented.
49-54: Duplicate of previous concern: Guard against invalid recurrence_period values.The
TransactionRecurringPeriod::from()call can still throw exceptions for invalid enum values, which would crash the entire command. The previous suggestion to add proper error handling remains valid.app/Http/Controllers/API/v1/TransactionController.php (1)
188-188: Add validation to ensure recurrence_ends_at is in the future.The current validation doesn't prevent users from setting a recurrence end date in the past, which would make the recurring transaction ineffective.
Update the validation rule to ensure the date is in the future:
-'recurrence_ends_at' => 'nullable|date', +'recurrence_ends_at' => 'nullable|date|after:today',Also applies to: 459-459
🧹 Nitpick comments (6)
app/Console/Kernel.php (1)
16-16: Good approach to have the scheduled command initially disabled.Keeping the recurring transaction generation command commented out allows for controlled deployment and testing before activation. Consider documenting when this should be enabled in production.
app/Models/RecurringTransactionRule.php (2)
13-13: Fix incorrect property description.The
idproperty description incorrectly refers to "ID of the transaction" when it should describe the recurring transaction rule's own ID.- new OA\Property(property: 'id', description: 'ID of the transaction', type: 'integer'), + new OA\Property(property: 'id', description: 'ID of the recurring transaction rule', type: 'integer'),
15-16: Improve property descriptions for clarity.The
recurrence_periodandrecurrence_intervalproperties have identical descriptions, which doesn't help distinguish their purposes.- new OA\Property(property: 'recurrence_period', description: 'Set how often the transaction should repeat', type: 'string'), - new OA\Property(property: 'recurrence_interval', description: 'Set how often the transaction should repeat', type: 'integer'), + new OA\Property(property: 'recurrence_period', description: 'The period type for recurrence (daily, weekly, monthly, yearly)', type: 'string'), + new OA\Property(property: 'recurrence_interval', description: 'The interval count for recurrence period (e.g., every 2 weeks)', type: 'integer'),database/migrations/2025_06_19_125117_create_recurring_transaction_rules_table.php (1)
16-16: Consider referencing enum class values for consistency.The enum values are hardcoded but should ideally reference the
TransactionRecurringPeriodenum class to maintain consistency and avoid drift.- $table->enum('recurrence_period', ['daily', 'weekly', 'monthly', 'yearly']); + $table->enum('recurrence_period', array_column(TransactionRecurringPeriod::cases(), 'value'));You'll need to import the enum at the top:
+use App\Enums\TransactionRecurringPeriod;app/Console/Commands/GenerateRecurringTransactions.php (1)
46-48: Improve the null check for recurrence_period.While the null check is present, it should be moved earlier to avoid unnecessary processing, and logging should be added for better observability.
+if ($transaction->recurrence_period == null) { + logger()->warning("Skipping transaction {$transaction->id}: missing recurrence_period"); + continue; +} + // Get the last recurring transaction based on parent or self $lastTransaction = Transaction::where('parent_transaction_id', $transaction->id) ->orWhere('id', $transaction->id) ->orderBy('id', 'desc') ->first(); -if ($transaction->recurrence_period == null) { - continue; -}tests/Feature/TransactionsTest.php (1)
382-396: Commented test should be completed or removed.The commented test for updating recurring rules should either be implemented and enabled, or removed if not needed. Leaving commented code reduces maintainability.
-/* public function test_api_user_can_update_the_recurring_rule_of_a_transaction() - { - $expense = $this->createTransaction('expense'); - - $response = $this->actingAs($this->user)->putJson('/api/v1/transactions/' . $expense['id'], [ - 'recurrence_period' => 'daily', - 'recurrence_interval' => 2, - 'is_recurring' => true, - ]); - - $transaction = $response->json('data'); - $this->assertEquals('daily', $transaction['recurring_rules']['recurrence_period']); - $this->assertEquals(1, $transaction['recurring_rules']['recurrence_interval']); - $this->assertEquals($transaction['id'], $transaction['recurring_rules']['transaction_id']); - }*/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/Console/Commands/GenerateRecurringTransactions.php(1 hunks)app/Console/Kernel.php(1 hunks)app/Enums/TransactionRecurringPeriod.php(1 hunks)app/Http/Controllers/API/v1/TransactionController.php(11 hunks)app/Jobs/RecurrentTransactionJob.php(1 hunks)app/Models/RecurringTransactionRule.php(1 hunks)app/Models/Transaction.php(5 hunks)app/helpers.php(2 hunks)config/queue.php(2 hunks)database/migrations/2024_07_20_125117_create_transactions_table.php(1 hunks)database/migrations/2025_06_19_125117_create_recurring_transaction_rules_table.php(1 hunks)database/migrations/2025_06_20_125110_create_jobs_table.php(1 hunks)public/docs/api.json(5 hunks)tests/Feature/TransactionsTest.php(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run Tests
🔇 Additional comments (7)
database/migrations/2024_07_20_125117_create_transactions_table.php (1)
25-25: LGTM! Formatting improvement enhances readability.The blank lines appropriately separate the table column definitions from the foreign key constraints, making the migration more readable.
Also applies to: 30-30
config/queue.php (2)
16-16: Good change to enable asynchronous job processing.Switching from 'sync' to 'database' queue driver enables proper background processing for recurring transaction jobs.
43-43: Appropriate memory allocation for queue workers.The 128MB memory limit is reasonable for processing recurring transaction jobs.
database/migrations/2025_06_20_125110_create_jobs_table.php (1)
14-22: Standard Laravel jobs table schema correctly implemented.The migration follows Laravel conventions with proper column types, indexing on the queue column for performance, and all necessary fields for queue job management.
app/Models/Transaction.php (1)
9-9: LGTM! Recurring transaction integration is well implemented.The Transaction model has been properly extended to support recurring transactions:
- Correct HasOne relationship to RecurringTransactionRule
- Proper attribute accessor following Laravel conventions
- OpenAPI schema appropriately updated
- Integration with model serialization via appends array
Also applies to: 39-43, 78-78, 139-147
tests/Feature/TransactionsTest.php (2)
29-46: Good enhancement to the test helper method.The addition of the optional
$recurrentDataparameter allows for flexible testing of both regular and recurring transactions. The implementation correctly sets the required recurring transaction fields.
20-365: Verify: Are all existing tests still functional?Previous reviews indicated that the entire test suite was commented out, removing critical test coverage. From the current code, the tests appear to be active, but I want to ensure no regression in test coverage occurred.
#!/bin/bash # Description: Verify that all transaction tests are functional and not commented out # Expected: All test methods should be active and runnable echo "Checking for commented test methods..." rg -n "^\s*/\*.*test_" tests/Feature/TransactionsTest.php || echo "No commented test methods found" echo "Counting active test methods..." rg -c "public function test_" tests/Feature/TransactionsTest.php echo "Listing all test method names..." rg -o "public function (test_[^(]+)" tests/Feature/TransactionsTest.php
kofimokome
force-pushed
the
feat/recurrent-transactions
branch
3 times, most recently
from
a50734d to
0a5386b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/checks.yml (1)
50-50: Suggest using GitHub Actionsenvblock instead ofsed.Chaining multiple
sedcommands to patch.envis brittle and can lead to hard-to-debug failures. Consider overriding these variables declaratively:jobs: tests: runs-on: ubuntu-latest env: APP_ENV: testing DB_HOST: 127.0.0.1 DB_PORT: 3306 DB_DATABASE: trakli QUEUE_CONNECTION: database DB_USERNAME: root steps: # ...This approach is more maintainable and reduces complexity in your CI steps.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/checks.yml(1 hunks)app/Console/Kernel.php(1 hunks)app/Enums/TransactionRecurringPeriod.php(1 hunks)app/Http/Controllers/API/v1/TransactionController.php(13 hunks)app/Jobs/RecurrentTransactionJob.php(1 hunks)app/Models/RecurringTransactionRule.php(1 hunks)app/Models/Transaction.php(5 hunks)app/helpers.php(2 hunks)config/queue.php(2 hunks)database/migrations/2024_07_20_125117_create_transactions_table.php(1 hunks)database/migrations/2025_06_19_125117_create_recurring_transaction_rules_table.php(1 hunks)database/migrations/2025_06_20_125110_create_jobs_table.php(1 hunks)phpunit.xml(1 hunks)public/docs/api.json(5 hunks)tests/Feature/TransactionsTest.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (14)
- app/Console/Kernel.php
- database/migrations/2024_07_20_125117_create_transactions_table.php
- phpunit.xml
- config/queue.php
- app/Jobs/RecurrentTransactionJob.php
- app/Models/RecurringTransactionRule.php
- app/helpers.php
- app/Models/Transaction.php
- database/migrations/2025_06_19_125117_create_recurring_transaction_rules_table.php
- app/Enums/TransactionRecurringPeriod.php
- public/docs/api.json
- tests/Feature/TransactionsTest.php
- database/migrations/2025_06_20_125110_create_jobs_table.php
- app/Http/Controllers/API/v1/TransactionController.php
🔇 Additional comments (1)
.github/workflows/checks.yml (1)
50-50: Align test queue driver todatabase.Updating
QUEUE_CONNECTIONtodatabasein the test.envensures that queued jobs are persisted in the database during CI runs, preventing them from executing synchronously and mitigating the memory leak in your tests related toRecurrentTransactionJob. This aligns with the newjobstable migration and the default queue connection inconfig/queue.php.
|
👀 |
kofimokome
force-pushed
the
feat/recurrent-transactions
branch
from
0a5386b to
23b6f33
Compare
Code Review Summary✨ This pull request introduces a new feature for recurring transactions, allowing users to define transactions that repeat daily, weekly, monthly, or yearly. It includes:
🚀 Key Improvements
💡 Minor Suggestions
🚨 Critical Issues
|
kofimokome
force-pushed
the
feat/recurrent-transactions
branch
from
23b6f33 to
a65de3d
Compare
|
@nfebe merge conflicts resolved |
kofimokome
force-pushed
the
feat/recurrent-transactions
branch
from
a65de3d to
30e74e8
Compare
![sourceant[bot]](https://avatars.githubusercontent.com/in/1161781?s=60&v=4){kind=small}
| private string $recurrence_period; | ||
|
|
||
| private int $recurrence_interval; | ||
|
|
There was a problem hiding this comment.
For consistency with other catch blocks in the application (e.g., TransactionController), it's recommended to catch Throwable instead of Exception. Throwable is the base interface for all errors and exceptions in PHP 7+, providing broader error handling.
| } catch (Throwable $e) { |
Fixes #44