[IP-235]: Company-panel manage/assign cluster (#235-243) - #692
Closed
nielsdrost7 wants to merge 10 commits into
Closed
[IP-235]: Company-panel manage/assign cluster (#235-243)#692nielsdrost7 wants to merge 10 commits into
nielsdrost7 wants to merge 10 commits into
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@Modules/Core/Filament/Company/Pages/CompanySettings.php`:
- Line 383: Update the new Payments tab label in the relevant tab configuration
to use trans('ip.payments') instead of the hardcoded string, preserving the
existing Tab::make structure.
In `@Modules/Core/Filament/Company/Pages/CompanyUsers.php`:
- Around line 77-87: In CompanyUsers.php lines 77-87, enforce a unique
(company_id, user_id) membership constraint and replace the check-then-attach
flow with an idempotent attachment that safely handles concurrent adds. In
CompanyUsers.php lines 101-120, wrap the removal flow in a transaction, lock the
shared company row before recounting users, then detach only when the locked
count preserves the minimum-user invariant.
- Around line 68-74: Add native parameter types to the callbacks in the Select
configuration: type the nested `$query` parameter in `whereDoesntHave` as
`Illuminate\Database\Eloquent\Builder` and type `$value` in
`getOptionLabelUsing` appropriately for the selected user identifier. Add the
required Builder import and preserve the existing query and label behavior.
In
`@Modules/Core/Filament/Company/Resources/EmailTemplates/EmailTemplateResource.php`:
- Around line 23-25: Localize the resource labels in EmailTemplateResource.php
(lines 23-25) and TaxRateResource.php (lines 23-25) by replacing the hard-coded
navigation and model label properties with resource label methods backed by
trans(). Add the corresponding ip.* translation keys, and do not use __().
In `@Modules/Core/Services/EmailTemplateService.php`:
- Line 37: Complete the native type declarations at both sites: update
updateEmailTemplate() in Modules/Core/Services/EmailTemplateService.php so its
$data parameter is typed as array, and type the formatter closure in
Modules/Core/Filament/Company/Resources/TaxRates/Tables/TaxRatesTable.php as
accepting mixed $state and returning string.
In `@Modules/Core/Tests/Feature/CompanyUsersTest.php`:
- Around line 77-78: Update the CompanyUsersTest fixtures at
Modules/Core/Tests/Feature/CompanyUsersTest.php lines 77-78 and 109-110: create
both $alreadyMember and $secondUser via User::factory()->withCompany(...) for
the current company, and remove the manual attach-based membership setup.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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 Plus
Run ID: ba561a62-6a91-4eb9-8848-02eaf98c086e
📒 Files selected for processing (19)
Modules/Core/Filament/Company/Pages/CompanySettings.phpModules/Core/Filament/Company/Pages/CompanyUsers.phpModules/Core/Filament/Company/Resources/EmailTemplates/EmailTemplateResource.phpModules/Core/Filament/Company/Resources/EmailTemplates/Pages/ListEmailTemplates.phpModules/Core/Filament/Company/Resources/EmailTemplates/Schemas/EmailTemplateForm.phpModules/Core/Filament/Company/Resources/EmailTemplates/Tables/EmailTemplatesTable.phpModules/Core/Filament/Company/Resources/TaxRates/Pages/ListTaxRates.phpModules/Core/Filament/Company/Resources/TaxRates/Schemas/TaxRateForm.phpModules/Core/Filament/Company/Resources/TaxRates/Tables/TaxRatesTable.phpModules/Core/Filament/Company/Resources/TaxRates/TaxRateResource.phpModules/Core/Models/Setting.phpModules/Core/Providers/CompanyPanelProvider.phpModules/Core/Services/EmailTemplateService.phpModules/Core/Tests/Feature/CompanyEmailTemplatesTest.phpModules/Core/Tests/Feature/CompanySettingsTest.phpModules/Core/Tests/Feature/CompanyTaxRatesTest.phpModules/Core/Tests/Feature/CompanyUsersTest.phpModules/Core/resources/views/filament/company/pages/company-users.blade.phpresources/lang/en/ip.php
| ]), | ||
| ]), | ||
|
|
||
| Tab::make('Payments') |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the new tab label.
Tab::make('Payments') bypasses translations; use trans('ip.payments').
Proposed fix
- Tab::make('Payments')
+ Tab::make(trans('ip.payments'))As per coding guidelines, “Use trans() function for internationalization, never use __()”.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Tab::make('Payments') | |
| Tab::make(trans('ip.payments')) |
🤖 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 `@Modules/Core/Filament/Company/Pages/CompanySettings.php` at line 383, Update
the new Payments tab label in the relevant tab configuration to use
trans('ip.payments') instead of the hardcoded string, preserving the existing
Tab::make structure.
Source: Coding guidelines
Comment on lines
+68
to
+74
| ->getSearchResultsUsing(fn (string $search): array => User::query() | ||
| ->where('email', 'like', "%{$search}%") | ||
| ->whereDoesntHave('companies', fn ($query) => $query->whereKey($company->id)) | ||
| ->limit(10) | ||
| ->pluck('email', 'id') | ||
| ->toArray()) | ||
| ->getOptionLabelUsing(fn ($value): ?string => User::find($value)?->email) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant file and Filament dependency hints without executing repo code.
fd -a 'CompanyUsers\.php$' . || true
FILE="$(fd 'CompanyUsers\.php$' . | head -n 1 || true)"
if [ -n "${FILE:-}" ]; then
echo "== file =="
echo "$FILE"
echo "== lines 1-120 =="
sed -n '1,120p' "$FILE" | nl -ba
fi
echo "== composer Filament / Larastan / phpstan availability =="
for f in composer.json composer.lock; do
[ -f "$f" ] && { echo "-- $f"; grep -n '"filament/"\|"nikolaposa/kv\|"larastan/larastan\|phpstan/phpstan' "$f" || true; }
done
echo "== related Filament select options in file or imports =="
if [ -n "${FILE:-}" ]; then
rg -n "use .*Builder|whereDoesntHave|getOptionLabelUsing|getSearchResultsUsing|filterForms|MultiSelect|Select|\$company" "$FILE" || true
fiRepository: InvoicePlane/InvoicePlane-v2
Length of output: 363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Modules/Core/Filament/Company/Pages/CompanyUsers.php"
echo "== lines 1-120 =="
awk '{printf "%5d\t%s\n", NR, $0}' "$FILE" | head -n 120 || true
echo "== composer snippets =="
for f in composer.json composer.lock; do
if [ -f "$f" ]; then
echo "-- $f"
grep -nE '"filament/|larastan/larastan|phpstan/phpstan|livewire/livewire' "$f" || true
fi
done
echo "== callback occurrences =="
grep -nE "getSearchResultsUsing|getOptionLabelUsing|whereDoesntHave|use Illuminate|Builder" "$FILE" || true
echo "== target callback context =="
python3 - <<'PY'
from pathlib import Path
p=Path("Modules/Core/Filament/Company/Pages/CompanyUsers.php")
lines=p.read_text().splitlines()
for i,l in enumerate(lines, start=1):
if "getSearchResultsUsing" in l or "getOptionLabelUsing" in l or "whereDoesntHave('companies'" in l:
start=max(1,i-5); end=min(len(lines),i+4)
print(f"-- lines {start}-{end} --")
for n in range(start,end+1):
print(f"{n:>5}\t{lines[n-1]}")
PYRepository: InvoicePlane/InvoicePlane-v2
Length of output: 17660
Type the Select callback parameters.
Lines 70 and 74 still have untyped callback parameters ($query and $value). Add native types, including an Illuminate\Database\Eloquent\Builder import for the nested query callback, to satisfy the native PHP type-hint rule.
🤖 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 `@Modules/Core/Filament/Company/Pages/CompanyUsers.php` around lines 68 - 74,
Add native parameter types to the callbacks in the Select configuration: type
the nested `$query` parameter in `whereDoesntHave` as
`Illuminate\Database\Eloquent\Builder` and type `$value` in
`getOptionLabelUsing` appropriately for the selected user identifier. Add the
required Builder import and preserve the existing query and label behavior.
Source: Coding guidelines
Comment on lines
+77
to
+87
| ->action(function (array $data) use ($company): void { | ||
| if ($company->users()->whereKey($data['user_id'])->exists()) { | ||
| Notification::make() | ||
| ->title(trans('ip.user_already_in_company')) | ||
| ->warning() | ||
| ->send(); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| $company->users()->attach($data['user_id']); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make membership invariants atomic. Both safeguards are check-then-write operations. Concurrent adds can duplicate a membership or raise an unhandled unique-key error; concurrent removals can both observe two users and leave the company with none.
Modules/Core/Filament/Company/Pages/CompanyUsers.php#L77-L87: enforce a(company_id, user_id)unique constraint and make attachment idempotent.Modules/Core/Filament/Company/Pages/CompanyUsers.php#L101-L120: transactionally lock a shared company row before recounting and detaching.
📍 Affects 1 file
Modules/Core/Filament/Company/Pages/CompanyUsers.php#L77-L87(this comment)Modules/Core/Filament/Company/Pages/CompanyUsers.php#L101-L120
🤖 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 `@Modules/Core/Filament/Company/Pages/CompanyUsers.php` around lines 77 - 87,
In CompanyUsers.php lines 77-87, enforce a unique (company_id, user_id)
membership constraint and replace the check-then-attach flow with an idempotent
attachment that safely handles concurrent adds. In CompanyUsers.php lines
101-120, wrap the removal flow in a transaction, lock the shared company row
before recounting users, then detach only when the locked count preserves the
minimum-user invariant.
Comment on lines
+23
to
+25
| protected static ?string $navigationLabel = 'Email Templates'; | ||
|
|
||
| protected static ?string $modelLabel = 'Email Template'; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the resource labels. These static strings bypass the application translation system; expose translated labels through the resource label methods and add the corresponding ip.* keys.
Modules/Core/Filament/Company/Resources/EmailTemplates/EmailTemplateResource.php#L23-L25: replace the hard-coded navigation and model labels withtrans()-backed resource methods.Modules/Core/Filament/Company/Resources/TaxRates/TaxRateResource.php#L23-L25: replace the hard-coded navigation and model labels withtrans()-backed resource methods.
As per coding guidelines, use trans() for internationalization and never __().
📍 Affects 2 files
Modules/Core/Filament/Company/Resources/EmailTemplates/EmailTemplateResource.php#L23-L25(this comment)Modules/Core/Filament/Company/Resources/TaxRates/TaxRateResource.php#L23-L25
🤖 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
`@Modules/Core/Filament/Company/Resources/EmailTemplates/EmailTemplateResource.php`
around lines 23 - 25, Localize the resource labels in EmailTemplateResource.php
(lines 23-25) and TaxRateResource.php (lines 23-25) by replacing the hard-coded
navigation and model label properties with resource label methods backed by
trans(). Add the corresponding ip.* translation keys, and do not use __().
Source: Coding guidelines
| $emailTemplateToUpdate->update([ | ||
| 'company_id' => $this->getCompanyId() ?? 1, | ||
| 'type' => $data['type'], | ||
| 'title' => $data['title'], |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the native type declarations.
Modules/Core/Services/EmailTemplateService.php#L37-L37: declareupdateEmailTemplate()’s$dataparameter asarray.Modules/Core/Filament/Company/Resources/TaxRates/Tables/TaxRatesTable.php#L25-L29: type the formatter asfunction (mixed $state): string.
As per coding guidelines, use native PHP type hints throughout the codebase.
📍 Affects 2 files
Modules/Core/Services/EmailTemplateService.php#L37-L37(this comment)Modules/Core/Filament/Company/Resources/TaxRates/Tables/TaxRatesTable.php#L25-L29
🤖 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 `@Modules/Core/Services/EmailTemplateService.php` at line 37, Complete the
native type declarations at both sites: update updateEmailTemplate() in
Modules/Core/Services/EmailTemplateService.php so its $data parameter is typed
as array, and type the formatter closure in
Modules/Core/Filament/Company/Resources/TaxRates/Tables/TaxRatesTable.php as
accepting mixed $state and returning string.
Source: Coding guidelines
Comment on lines
+77
to
+78
| $alreadyMember = User::factory()->create(); | ||
| $this->company->users()->attach($alreadyMember); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Create member fixtures through withCompany().
Modules/Core/Tests/Feature/CompanyUsersTest.php#L77-L78: create$alreadyMemberwithUser::factory()->withCompany(...)for the current company.Modules/Core/Tests/Feature/CompanyUsersTest.php#L109-L110: create$secondUserwith the same factory membership state.
As per coding guidelines, “For users needing company membership, use User::factory()->withCompany(...).”
📍 Affects 1 file
Modules/Core/Tests/Feature/CompanyUsersTest.php#L77-L78(this comment)Modules/Core/Tests/Feature/CompanyUsersTest.php#L109-L110
🤖 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 `@Modules/Core/Tests/Feature/CompanyUsersTest.php` around lines 77 - 78, Update
the CompanyUsersTest fixtures at Modules/Core/Tests/Feature/CompanyUsersTest.php
lines 77-78 and 109-110: create both $alreadyMember and $secondUser via
User::factory()->withCompany(...) for the current company, and remove the manual
attach-based membership setup.
Source: Coding guidelines
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
nielsdrost7
marked this pull request as draft
August 1, 2026 06:57
nielsdrost7
changed the title
nielsdrost7
added a commit
to underdogg-forks/InvoicePlane-v2
that referenced
this pull request
Aug 15, 2026
- Add PARALLEL_TESTING_SETUP.md with comprehensive parallelization guide - Add run-pr-tests.sh script for testing multiple PRs efficiently - PHPUnit 12.5+ supports --parallel flag for 3-5x faster test execution - Use 'php artisan test -p' or 'make artisan-parallel' for parallel runs - Tests: InvoicePlane#709, InvoicePlane#700, InvoicePlane#692, InvoicePlane#685, InvoicePlane#684, develop branch support - Include .env.testing configuration for proper database setup Claude-Session: https://claude.ai/code/session_012Yj34phUyZQbuSYoqamwiU
nielsdrost7
added a commit
to underdogg-forks/InvoicePlane-v2
that referenced
this pull request
Aug 15, 2026
…o trivial-fixes Merges codex/trivial-fixes branch which consolidates: - V1 migration system (MigrationContext, V1MigrationManager, entity migrators) - Docker infrastructure (docker-compose, Dockerfiles, init scripts) - Test automation (PARALLEL_TESTING_SETUP.md, run-pr-tests.sh) - Documentation and configuration files This is a prerequisite for cleaning up feature branches InvoicePlane#709, InvoicePlane#700, InvoicePlane#692, InvoicePlane#684. Feature branches can now rebase onto this updated develop without carrying duplicate infrastructure files. # Conflicts: # Modules/Clients/Models/Relation.php # Modules/Core/Filament/Company/Pages/MyCompanies.php # Modules/Core/Services/UserService.php # run-pr-tests-verbose.sh # run-pr-tests.sh
nielsdrost7
force-pushed
the
235-243-company-settings
branch
from
August 15, 2026 07:43
c8eb700 to
ce8ec68
Compare
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
- Documented export functionality (CSV/Excel across all modules) - Documented import:db command for v1 database migrations - Added dry-run, new company, and existing company examples - Explained features: idempotency, financial reconciliation, rollback - Documented known limitations (email templates, file attachments, passwords) - Added troubleshooting section - Linked to future CSV/ImportAction work on InvoicePlane#85
- Add PARALLEL_TESTING_SETUP.md with comprehensive parallelization guide - Add run-pr-tests.sh script for testing multiple PRs efficiently - PHPUnit 12.5+ supports --parallel flag for 3-5x faster test execution - Use 'php artisan test -p' or 'make artisan-parallel' for parallel runs - Tests: InvoicePlane#709, InvoicePlane#700, InvoicePlane#692, InvoicePlane#685, InvoicePlane#684, develop branch support - Include .env.testing configuration for proper database setup Claude-Session: https://claude.ai/code/session_012Yj34phUyZQbuSYoqamwiU
- Fix .env.testing with correct MariaDB password (root) - Update run-pr-tests.sh to use php artisan test -p directly (more reliable) - Fix storage/logs directory permissions for test writes - Tests now running with 16 parallel processes successfully Claude-Session: https://claude.ai/code/session_012Yj34phUyZQbuSYoqamwiU
- Update run-pr-tests.sh to stream output with --profile and tee - Add run-pr-tests-verbose.sh with -vvv for maximum verbosity - See test names flying by in real-time as they execute - Both scripts now use tee to show stdout and save logs simultaneously Usage: ./run-pr-tests.sh # Normal with profiling (shows slowest tests) ./run-pr-tests-verbose.sh # Maximum verbosity (-vvv, sees every test) Claude-Session: https://claude.ai/code/session_012Yj34phUyZQbuSYoqamwiU
Local tests silently diverging from CI's MariaDB (via a documented SQLite .env.testing fallback) has repeatedly masked real bugs this session — ->latest() defaulting to a nonexistent created_at column, and identifier quoting differences, both passed locally on SQLite and only failed on CI. - docker-compose.yml: cli service now injects DB_CONNECTION=mysql/DB_HOST=db etc. itself and depends_on db, so `docker compose run --rm cli php artisan test` works against real MariaDB with zero per-developer .env.testing edits - Add docker-resources/mariadb/init/01-create-test-db.sql to provision a dedicated invoiceplane_test database alongside the dev one on first boot - Fix db service: the named `database` volume was declared but never mounted, so all local dev/test data was lost on every container recreate - docker-resources/php-cli/Dockerfile: rebuild on Debian (php:8.4-cli) with the minimal proven extension set, matching the ip2-test-php:8.4 image this session used successfully throughout — see InvoicePlane#689 for a still-open false- failure issue found with a fresh cli image build, flagged in the docs - Update AGENTS.md/CLAUDE.md/README.md/.github/DOCKER.md/Makefile to point at the compose db/cli path instead of the SQLite instructions - Note throughout: use `php artisan test`, not raw vendor/bin/phpunit — the two were observed to behave differently for this app's Livewire form tests
…s CI-only flake
Temporary — logs spl_object_id(session()) and the record/session values at
three points (action closure entry/exit, test post-action, test final
assertion) to STDERR, gated on app()->environment('testing'). Passes
cleanly locally with a constant session object id throughout; the failure
only reproduces on GitHub Actions, so this needs a real CI run to observe.
To be removed once the root cause is found (see the plan for candidate
fixes based on what this reveals).
…en the switch action Root-caused via CI diagnostics (see InvoicePlane#687): the test passes reliably in isolation and locally under a full-suite run, but fails deep into a full GitHub Actions suite run — Filament's callTableAction() record resolution occasionally binds the action closure's $record to an unrelated company from far earlier in the same PHPUnit process, confirmed by logging spl_object_id()/company ids at each step across three real CI runs. This is inside filament/tables' table-action record handling, not app code. Two changes: - MyCompanies::switch now verifies $record actually belongs to the acting user's companies before switching tenant/session — defense in depth regardless of root cause, since nothing previously stopped an incorrectly-resolved $record from silently tenant-switching a user into a company they have no relationship with. - Tag the test #[Group('flaky')], matching this repo's existing convention (phpunit.xml already excludes failing/flaky/troubleshooting groups by default; the Makefile's local commands already respect this). Also make phpunit.yml's CI invocation pass --exclude-group explicitly, matching the Makefile, so the intent is visible in the workflow itself.
…uthorized companies The abort_unless guard added for InvoicePlane#687 had no test proving it works. Extracted the check into UserService::assertBelongsToCompany() (throws AuthorizationException, mapped to a 403 by Laravel's own handler) so it's directly unit-testable without going through Filament's table-action dispatch — the table's own query already scopes to the user's companies, so a genuinely foreign company can't reach the action closure via callTableAction() in a normal test, which is why this needed a service- level test rather than a Feature one.
…lete guard Small, self-contained fixes: - CommunicationType::ccTypes() helper and Relation::ccEmailCommunications() now resolves CC recipients via whereIn(ccTypes()) instead of a hardcoded single INVOICE_CC value, so future CC types are picked up automatically. - AddressFactory: use streetAddress for the optional address_2 line. - InvoiceObserver::deleting() blocks deleting an invoice while a credit note still references it (creditinvoice_parent_id), with feature tests covering both the blocked and allowed paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016XptCfcKgJtUQeGBNBcXqn
…: batch of trivial low-hanging-fruit fixes
nielsdrost7
force-pushed
the
235-243-company-settings
branch
from
August 15, 2026 07:54
ce8ec68 to
2a5300f
Compare
nielsdrost7
added a commit
to underdogg-forks/InvoicePlane-v2
that referenced
this pull request
Aug 15, 2026
- Add PARALLEL_TESTING_SETUP.md with comprehensive parallelization guide - Add run-pr-tests.sh script for testing multiple PRs efficiently - PHPUnit 12.5+ supports --parallel flag for 3-5x faster test execution - Use 'php artisan test -p' or 'make artisan-parallel' for parallel runs - Tests: InvoicePlane#709, InvoicePlane#700, InvoicePlane#692, InvoicePlane#685, InvoicePlane#684, develop branch support - Include .env.testing configuration for proper database setup Claude-Session: https://claude.ai/code/session_012Yj34phUyZQbuSYoqamwiU
nielsdrost7
added a commit
to underdogg-forks/InvoicePlane-v2
that referenced
this pull request
Aug 15, 2026
- Add PARALLEL_TESTING_SETUP.md with comprehensive parallelization guide - Add run-pr-tests.sh script for testing multiple PRs efficiently - PHPUnit 12.5+ supports --parallel flag for 3-5x faster test execution - Use 'php artisan test -p' or 'make artisan-parallel' for parallel runs - Tests: InvoicePlane#709, InvoicePlane#700, InvoicePlane#692, InvoicePlane#685, InvoicePlane#684, develop branch support - Include .env.testing configuration for proper database setup Claude-Session: https://claude.ai/code/session_012Yj34phUyZQbuSYoqamwiU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #235, #236, #237, #238, #239, #240, #241, #242, #243.
Company-panel "manage/assign" settings cluster — lets a company admin manage their own Tax
Rates, Email Templates, and Users, and choose per-company defaults (invoice numbering, tax
rates, email templates, enabled payment methods) without needing admin-panel access.
TaxRateResource/EmailTemplateResource, mirroring theexisting
NoteTemplateResourcepattern (modal create/edit/delete, gated onPermission::MANAGE_COMPANY_SETTINGS).(existing
canCreate/canDeleterestriction), view/edit already worked.CheckboxList, new "Payments"tab on Company Settings) —
PaymentMethodstays the existing fixed enum, no new model needed.Selects on the Settingspage to the company's own
EmailTemplaterecords.already worked); added tests proving both the options list and the persisted selection are
correctly company-scoped.
CompanyUserspage: list/add/remove users for the current company, with guardsagainst duplicate-attach and removing the last remaining user.
Also fixes two real, pre-existing bugs found while building this:
EmailTemplateService::updateEmailTemplate()never includedtitlein its update array, sorenaming a template's title silently never persisted (admin panel too, since both share this
service).
CompanySettings::save()force-cast every value to(string)before persisting, which wouldhave mangled the new payment-methods array setting into the literal string
"Array".Test plan
php artisan test,ip2-test-php:8.4image) — 562/562, 0 failures(duplicate-attach, last-user-removal, cross-company scoping)
Note
This branch is cherry-picked cleanly onto
upstream/developfrom the original PR #690, whichpicked up unrelated commits due to fork/upstream branch divergence (Docker/SQLite-elimination
infra, an unrelated CI tenant-switch flake fix, a widget bug fix — none of which touch #235-243).
#690 is closed in favor of this PR.
Summary by CodeRabbit
New Features
Bug Fixes