fix: broken migrations and smoke test against real MariaDB#468
Conversation
- recurring_invoices: remove FK to non-existent document_groups table - payments: add explicit index on invoice_id so MariaDB handles the nullable ON DELETE SET NULL FK constraint (errno 150 fix) - report_templates: delete duplicate migration in ReportBuilder module (identical file already exists in Core) - smoke.yml: switch from SQLite to MariaDB 10.11 service so FK constraint errors are caught; add push/pull_request triggers on master and develop so it runs automatically - quickstart.yml: uncomment and wire up MariaDB service - phpunit.yml: bump PHP 8.3 → 8.4, add pdo_sqlite extension - yarn-update.yml: bump PHP 8.2 → 8.4, fix extension names https://claude.ai/code/session_01Q4fwmdZcuFrrB6UQDeGWKd
…rop SQLite recurring_invoices: - rename document_group_id → numbering_id; add FK to numbering table matching the same pattern invoices uses (onUpdate cascade, onDelete restrict) - update RecurringInvoiceFactory: Numbering instead of missing DocumentGroup - update RecurringInvoiceForm: numbering_id Select with relationship payments: - invoice_id is NOT NULL (payment requires an invoice) - restore explicit constraint name payments_invoice_id_foreign with onUpdate cascade / onDelete restrict testing env: - .env.testing.example: replace SQLite :memory: with MySQL settings - phpunit.yml: add MariaDB 10.11 service, set DB env vars, uncomment migration step so tests run against real MariaDB https://claude.ai/code/session_01Q4fwmdZcuFrrB6UQDeGWKd
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSwitches test environment from SQLite to MariaDB, updates CI workflows to PHP 8.4 and runs migrations in CI, replaces recurring-invoices' document_group_id with numbering_id across migration/factory/form, and makes payments.invoice_id non-nullable with a named foreign key. ChangesTest Environment Infrastructure
Recurring Invoices Foreign Key Refactoring
Payments Invoice ID Constraint Enforcement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/phpunit.yml:
- Line 36: The workflow is using a floating actions/checkout@v4 ref and leaves
persist-credentials enabled; update the checkout step (the line containing
"uses: actions/checkout@v4") to a pinned release (e.g., a specific patch tag or
the action's commit SHA instead of the floating `@v4`) and add the with:
persist-credentials: false option to the same step so the job does not retain
the repo auth token/SSH key after checkout.
In
`@Modules/Invoices/Filament/Company/Resources/RecurringInvoices/Schemas/RecurringInvoiceForm.php`:
- Around line 23-25: The Select field created with Select::make('numbering_id')
currently relies on Filament's auto-generated label; update this field to
include an explicit translated label using trans(), e.g. call
->label(trans('...')) on the Select instance so the user-facing text follows the
translation guideline while keeping the existing
->relationship('numbering','name') and ->default(null) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 115dd06d-09e3-4bd7-9208-aafcdc7b48e2
📒 Files selected for processing (10)
.env.testing.example.github/workflows/phpunit.yml.github/workflows/quickstart.yml.github/workflows/smoke.yml.github/workflows/yarn-update.ymlModules/Invoices/Database/Factories/RecurringInvoiceFactory.phpModules/Invoices/Database/Migrations/2010_01_01_000023_create_recurring_invoices_table.phpModules/Invoices/Filament/Company/Resources/RecurringInvoices/Schemas/RecurringInvoiceForm.phpModules/Payments/Database/Migrations/2010_01_01_000024_create_payments_table.phpModules/ReportBuilder/Database/Migrations/2025_10_26_create_report_templates_table.php
💤 Files with no reviewable changes (1)
- Modules/ReportBuilder/Database/Migrations/2025_10_26_create_report_templates_table.php
| DB_PASSWORD: root | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify unpinned action refs and checkout credential persistence in workflows
rg -nP '^\s*uses:\s*[^@\s]+@(v?\d+(\.\d+)*)\s*$' .github/workflows
rg -nP '^\s*-\s*uses:\s*actions/checkout@' .github/workflows -A4 -B1Repository: InvoicePlane/InvoicePlane-v2
Length of output: 2442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the phpunit workflow around the checkout step
echo "---- phpunit.yml (around checkout) ----"
nl -ba .github/workflows/phpunit.yml | sed -n '1,120p'
# Check whether persist-credentials is set anywhere (especially in phpunit.yml)
echo "---- persist-credentials occurrences ----"
rg -n "persist-credentials" .github/workflows .github/actions || true
# Confirm whether phpunit.yml sets a 'with:' block on the checkout step
echo "---- checkout step in phpunit.yml ----"
rg -n "uses:\s*actions/checkout@" .github/workflows/phpunit.yml -n -C 5Repository: InvoicePlane/InvoicePlane-v2
Length of output: 173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "phpunit.yml line count:"
wc -l .github/workflows/phpunit.yml
echo "---- phpunit.yml (first 120 lines) ----"
cat -n .github/workflows/phpunit.yml | sed -n '1,120p'
echo "---- persist-credentials occurrences ----"
rg -n "persist-credentials" .github/workflows .github/actions || true
echo "---- checkout step in phpunit.yml ----"
rg -n "uses:\s*actions/checkout@" .github/workflows/phpunit.yml -C 5 || trueRepository: InvoicePlane/InvoicePlane-v2
Length of output: 2243
🌐 Web query:
actions/checkout persist-credentials default value
💡 Result:
The default value for the persist-credentials input in actions/checkout is true [1][2][3]. When set to true, this setting configures the repository's local git configuration with the authentication token or SSH key used for the checkout [1][4][5]. This allows subsequent git commands (such as git fetch or git push) within the workflow to use these credentials automatically [1][3]. The action removes these credentials from the local configuration during the post-job cleanup process [1][4][5]. Users can set persist-credentials to false to opt out of this behavior [1][5].
Citations:
- 1: https://github.com/actions/checkout/?tab=readme-ov-file
- 2: actions/checkout@c170eef
- 3: https://github.com/actions/checkout
- 4: https://github.com/actions/checkout/blob/85e6279cec87321a52edac9c87bce653a07cf6c2/README.md
- 5: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
Pin actions/checkout and disable persisted credentials in phpunit.yml.
.github/workflows/phpunit.ymluses a floating ref (actions/checkout@v4) and does not setpersist-credentials, whose default istrue—so checkout keeps the auth token/SSH key in the local git config for the remainder of the job.
Current state:
- uses: actions/checkout@v4Suggested hardening patch
- - uses: actions/checkout@v4
+ - uses: actions/checkout@<full_commit_sha_for_v4>
+ with:
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[warning] 36-36: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 36-36: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/phpunit.yml at line 36, The workflow is using a floating
actions/checkout@v4 ref and leaves persist-credentials enabled; update the
checkout step (the line containing "uses: actions/checkout@v4") to a pinned
release (e.g., a specific patch tag or the action's commit SHA instead of the
floating `@v4`) and add the with: persist-credentials: false option to the same
step so the job does not retain the repo auth token/SSH key after checkout.
|
@coderabbitai autofix |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken:
Lines 33–39 DB_PASSWORD: root
steps:
- - uses: actions/checkout@v4
+ - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+ with:
+ persist-credentials: false
- name: Setup PHP with Composer
uses: ./.github/actions/setup-php-composer |
Fixed 1 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
2 participants
recurring_invoices: remove FK to non-existent document_groups table
payments: add explicit index on invoice_id so MariaDB handles the
nullable ON DELETE SET NULL FK constraint (errno 150 fix)
report_templates: delete duplicate migration in ReportBuilder module
(identical file already exists in Core)
smoke.yml: switch from SQLite to MariaDB 10.11 service so FK
constraint errors are caught; add push/pull_request triggers on
master and develop so it runs automatically
quickstart.yml: uncomment and wire up MariaDB service
phpunit.yml: bump PHP 8.3 → 8.4, add pdo_sqlite extension
yarn-update.yml: bump PHP 8.2 → 8.4, fix extension names
https://claude.ai/code/session_01Q4fwmdZcuFrrB6UQDeGWKd
Summary by CodeRabbit
Refactor
Bug Fixes / Improvements
Chores