feat: add multi-organization support#275
Conversation
Introduce organization-scoped resource isolation. Users belong to organizations with per-org roles (admin, member, viewer). A global super_admin flag grants cross-org access. - Organization model with ULID, global scope for data isolation - CurrentOrganization service + middleware (cookie/header/query param) - Organization switcher in sidebar, management in Configuration tab - Per-org roles on pivot table, replacing flat role column on users - User management: invite new users or add existing users to an org - API: ?org= query parameter for org selection (documented in Scramble) - Updated all factories, seeders, policies, and tests for org context - Three-step migration: schema, data backfill, finalize
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds first-class multi-organization support: Organization model, migrations and factories, per-organization pivot roles plus a global super_admin flag, CurrentOrganization service and middleware, OrganizationScope, UI for managing/switching orgs, authorization updates, and pervasive scoping/wiring of resources and tests to an active organization context. ChangesMulti-Tenant Organization System
Sequence Diagram(s)sequenceDiagram
participant Req as HTTP Request
participant MW as SetCurrentOrganization
participant Auth as Auth::user()
participant Current as CurrentOrganization
participant Org as Organization
Req->>MW: handle($request)
MW->>Auth: identify actor
alt actor is Agent
MW->>Current: set(agent->organization)
else actor is User
MW->>Req: read ?org_id or X-Organization-Id
MW->>Org: lookup requested org (if provided)
MW->>Current: resolveForUser(user, $orgId)
Current->>Current: use cookie/membership/super-admin fallback
end
MW->>Req: continue request
sequenceDiagram
participant Controller as Livewire/Controller
participant Current as CurrentOrganization
participant Model as ResourceModel
participant DB as Database
Controller->>Current: id()
Current-->>Controller: org_id
Controller->>Model: create(data + organization_id)
Model->>DB: insert row (organization_id)
DB-->>Model: created record
Model-->>Controller: instance
Note over Model,Current: Reads use OrganizationScope -> CurrentOrganization
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #275 +/- ##
============================================
+ Coverage 91.32% 91.59% +0.26%
- Complexity 2179 2287 +108
============================================
Files 192 199 +7
Lines 7818 8186 +368
============================================
+ Hits 7140 7498 +358
- Misses 678 688 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
app/Livewire/Dashboard/JobStatusGrid.php (1)
68-82:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame cross-org data leakage as
LatestJobs:selectedJobqueries without org scoping.
jobs()is correctly scoped viaforCurrentOrg(), but theselectedJobcomputed property (line 75) usesBackupJob::with(...)->find($this->selectedJobId)with no org restriction. Combined with the unlockedpublic ?string $selectedJobId(line 18) and the publicly callableviewLogs(string $id)action, an authenticated user can read job detail data from another organization.🔒 Proposed fix
+use Livewire\Attributes\Locked; public bool $showLogsModal = false; +#[Locked] public ?string $selectedJobId = null; // ... #[Computed] public function selectedJob(): ?BackupJob { if (! $this->selectedJobId) { return null; } - return BackupJob::with([ + return BackupJob::forCurrentOrg()->with([ 'snapshot.databaseServer', 'snapshot.triggeredBy', 'restore.snapshot.databaseServer', 'restore.targetServer', 'restore.triggeredBy', ])->find($this->selectedJobId); }🤖 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 `@app/Livewire/Dashboard/JobStatusGrid.php` around lines 68 - 82, The computed property selectedJob uses BackupJob::with(...)->find($this->selectedJobId) with no organization scoping, allowing cross-org data access; update selectedJob to apply the same forCurrentOrg() scope used by jobs() (e.g. BackupJob::forCurrentOrg()->with(...)->find(...)) and/or validate that $this->selectedJobId belongs to the current org before returning, and ensure viewLogs and the public property selectedJobId cannot bypass this check (reference selectedJob, BackupJob::with, forCurrentOrg(), selectedJobId, and viewLogs).tests/Feature/Mcp/McpServerTest.php (1)
188-196:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Organization::first()->idwithout null safety risks aTypeErrorif no org is seeded.Both lines 195 and 258 call
Organization::first()->iddirectly. If no organization exists at test execution time,Organization::first()returnsnulland PHP throws aTypeError. The factory (line 28 ofDatabaseServerSshConfigFactory.php) consistently uses the null-safeOrganization::first()?->id— the test should match. UsingOrganization::main()->id(or at minimum the null-safe operator /firstOrFail()) is more explicit and resilient.🛡️ Proposed fix
- 'organization_id' => \App\Models\Organization::first()->id, + 'organization_id' => \App\Models\Organization::main()->id,Apply the same change at both line 195 and line 258.
Also applies to: 248-259
🤖 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 `@tests/Feature/Mcp/McpServerTest.php` around lines 188 - 196, The test uses Organization::first()->id which can throw a TypeError when no Organization exists; update both occurrences (the DatabaseServer creation at the block using \App\Models\DatabaseServer::create and the other call around line 258) to use a null-safe or explicit main/fail approach (e.g., Organization::first()?->id or Organization::main()->id or Organization::firstOrFail()->id) so the test no longer assumes a non-null Organization and mirrors the factory's null-safe use; ensure both instances are changed consistently.docs/docs/user-guide/permissions.md (1)
51-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winContradiction between the Users permissions table and the User Deletion Restrictions section.
Line 56 shows that an Admin can "Delete user" (✅), but lines 63–66 state "Only super admins can delete users." If "Delete user" in the table means remove from organization and the super-admin restriction applies to permanently deleting an account, the documentation should use different labels to make this distinction explicit (e.g., "Remove from organization" vs. "Delete account"). As written, the two sections directly conflict.
🤖 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 `@docs/docs/user-guide/permissions.md` around lines 51 - 66, The permissions table entry "Delete user" conflicts with the "User Deletion Restrictions" section; update the docs to disambiguate by renaming the table action and/or the restriction header: change the table action "Delete user" to "Remove from organization" (keeping Admin = ✅) and add a new row "Permanently delete account" with Viewer = ❌ Member = ❌ Admin = ❌ Super Admin = ✅, and update the "User Deletion Restrictions" heading to explicitly reference "Permanently delete account" (and keep the two bullets about self-deletion and last-super-admin restrictions) so the table and the special rules are consistent.app/Livewire/Dashboard/LatestJobs.php (1)
58-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCross-org data leakage:
getSelectedJobProperty()fetches jobs without org scoping.
fetchJobs()is now correctly scoped toforCurrentOrg(), butgetSelectedJobProperty()uses a plainBackupJob::with(...)->find($this->selectedJobId). Any public method in a Livewire component can be called from the client-side, even without an associated wire:click handler — users can still trigger the action from the browser's DevTools. SinceviewLogs(string $id)accepts any string from the client and sets$this->selectedJobId, andselectedJobIdis also an unprotected public property that should be treated as un-trusted input from the user, an authenticated user in Org A can read job logs (including eager-loaded snapshot and database server data) belonging to Org B.Add org scoping to the lookup and add
#[Locked]onselectedJobIdas defence-in-depth:🔒 Proposed fix
+use Livewire\Attributes\Locked; public bool $showLogsModal = false; +#[Locked] public ?string $selectedJobId = null; // ... public function getSelectedJobProperty(): ?BackupJob { if (! $this->selectedJobId) { return null; } - return BackupJob::with([ + return BackupJob::forCurrentOrg()->with([ 'snapshot.databaseServer', 'snapshot.triggeredBy', 'restore.snapshot.databaseServer', 'restore.targetServer', 'restore.triggeredBy', ])->find($this->selectedJobId); }🤖 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 `@app/Livewire/Dashboard/LatestJobs.php` around lines 58 - 71, getSelectedJobProperty currently loads a BackupJob by id without org scoping and selectedJobId is unprotected; update getSelectedJobProperty to use the org scope (e.g. BackupJob::forCurrentOrg()->with(...)->find($this->selectedJobId)) and return null if not found, and add the #[Locked] attribute to the selectedJobId property declaration to treat it as untrusted input (also ensure viewLogs uses the same guarded property). This ensures lookups are constrained to the current org and selectedJobId is hardened as defence-in-depth.tests/Feature/DatabaseServer/SshTunnelTest.php (1)
16-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
Volume::factory()->create()instead of rawVolume::create(), and guard against a nullOrganization::first().Two issues appear in all four test cases:
Coding guideline:
Volume::create([...])bypasses the factory. TheVolumeFactoryalready defaultsorganization_idcorrectly (Organization::first()?->id ?? Organization::factory()->main()), so switching to the factory removes the duplication and keeps tests consistent.Potential NPE:
Organization::first()->idhas no null-safe guard. If migrations don't seed a main org (e.g. in an isolated run), this throws aTypeError. The factories and other test helpers use?->idor a fallback — these tests should do the same.🛠️ Proposed fix (shown for the first occurrence; apply identically to lines 65-70, 109-114, and 191-196)
- $volume = Volume::create([ - 'name' => 'Test Volume', - 'type' => 'local', - 'config' => ['path' => '/var/backups'], - 'organization_id' => \App\Models\Organization::first()->id, - ]); + $volume = Volume::factory()->create();As per coding guidelines: "When creating models for tests, use factories. Check if the factory has custom states before manually setting up the model."
Also applies to: 65-70, 109-114, 191-196
🤖 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 `@tests/Feature/DatabaseServer/SshTunnelTest.php` around lines 16 - 21, Replace raw Volume::create([...]) calls with Volume::factory()->create() so the VolumeFactory defaults (including organization_id) are used; if you must set organization_id explicitly, guard against a null Organization by using Organization::first()?->id ?? Organization::factory()->main() (or create()) instead of Organization::first()->id. Update the occurrences around the Volume creation in SshTunnelTest (the lines using Volume::create and Organization::first()) and apply the same change to the other three blocks mentioned.tests/Feature/User/IndexTest.php (1)
59-78:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe test body never asserts the actual protection it is named for.
The test "admin cannot delete the last admin" only proves a non-final admin can be deleted. After deleting
$anotherAdminand returning to one super admin, the test exits without attempting to delete$this->adminand verifying that the action is forbidden. The comment on line 77 even acknowledges the gap.✅ Proposed completion of the test
expect(User::find($anotherAdmin->id))->toBeNull(); - // Now try to delete the last admin - should fail on authorize - // We need to create another user to try from (but they need to be admin) - // Actually, the test should verify you can't delete when count = 1 + // Create a second super admin to act as the deleter, then attempt + // to delete the only remaining super admin — must be rejected. + $secondAdmin = User::factory()->superAdmin()->create(); + Livewire::actingAs($secondAdmin) + ->test(Index::class) + ->call('confirmDelete', $this->admin->id) + ->assertForbidden(); });🤖 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 `@tests/Feature/User/IndexTest.php` around lines 59 - 78, The test never actually attempts to delete the final super admin; after removing $anotherAdmin it should try to delete $this->admin using the same Livewire flow and assert the action is forbidden. Update the "admin cannot delete the last admin" test to, after deleting $anotherAdmin, call Livewire::test(Index::class)->call('confirmDelete', $this->admin->id)->call('delete') (or the equivalent flow used by Index::class), then assert the deletion was prevented by checking User::find($this->admin->id) is not null and by asserting the Livewire component signaled failure (e.g., unauthorized/forbidden event, validation error, or emitted error message) so the test verifies confirmDelete/delete on $this->admin is rejected.app/Queries/BackupJobQuery.php (1)
25-49:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd org scoping to
make()to fix data isolation breach in API endpoint.
make()returns unscoped queries that include BackupJobs from all organizations.BackupJobdoes not register a globalOrganizationScope—it only provides a localscopeForCurrentOrg()method that must be explicitly called. Meanwhile,buildFromParams()correctly callsforCurrentOrg()on line 70.The API endpoint
BackupJobController::index()callsBackupJobQuery::make()->paginate()directly without additional scoping, allowing callers to retrieve BackupJobs across all tenants. Add->forCurrentOrg()to the chain inmake().🤖 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 `@app/Queries/BackupJobQuery.php` around lines 25 - 49, The make() query builder currently returns unscoped BackupJob::class results; to enforce tenant isolation add the model scope call forCurrentOrg() into the fluent chain inside BackupJobQuery::make() (the same scope exposed by BackupJob::scopeForCurrentOrg()/forCurrentOrg()), so the chain becomes QueryBuilder::for(BackupJob::class)->forCurrentOrg()->with(self::RELATIONSHIPS).... This mirrors buildFromParams() and prevents BackupJobController::index() from returning jobs across organizations.tests/Feature/Services/Backup/LocalVolumeStorageTest.php (1)
38-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
Organization::main()instead ofOrganization::first()for consistency.Other test files in this PR (e.g.,
DemoModeTest.phpline 215,CreateTest.phpline 59) use\App\Models\Organization::main()to reference the main organization.Organization::first()is order-dependent and diverges from the established pattern.♻️ Proposed fix
- 'organization_id' => \App\Models\Organization::first()->id, + 'organization_id' => \App\Models\Organization::main()->id,Apply the same change to both occurrences (lines 42 and 55).
🤖 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 `@tests/Feature/Services/Backup/LocalVolumeStorageTest.php` around lines 38 - 56, Replace the order-dependent Organization::first() calls used when creating test volumes with the canonical Organization::main() helper; specifically update the Organization lookup in the Volume::create calls that set 'organization_id' (the two occurrences near the Volume::create for $volumeWithRoot and $volumeWithPath) to use \App\Models\Organization::main()->id so the tests follow the established pattern.tests/Feature/DatabaseServer/EditTest.php (1)
14-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
Organization::main()instead ofOrganization::first()for consistency.Same issue as
LocalVolumeStorageTest.php:Organization::first()is order-dependent and inconsistent withOrganization::main()used throughout the rest of this PR's test changes.♻️ Proposed fix
- 'organization_id' => \App\Models\Organization::first()->id, + 'organization_id' => \App\Models\Organization::main()->id,Apply to all four occurrences (lines 18, 25, 105, 116).
Also applies to: 22-26, 101-106, 109-117
🤖 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 `@tests/Feature/DatabaseServer/EditTest.php` around lines 14 - 19, Replace uses of Organization::first()->id with Organization::main()->id in EditTest.php to ensure consistent, order-independent test data; specifically update the four occurrences that build Volume records (the Volume::create calls and any other places where organization_id is set using Organization::first()) so they call Organization::main()->id instead (search for Organization::first() in the file and swap to Organization::main()).app/Services/DemoBackupService.php (1)
14-63:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
DemoBackupService::createDemoBackup()will crash at runtime during first-user registration becauseCurrentOrganizationis never resolved.During registration, the
SetCurrentOrganizationmiddleware runs before the user exists, so it cannot resolve an organization.CreateNewUserthen creates the main organization and attaches the user, but never calls$currentOrganization->set()or$currentOrganization->switchTo()to set the resolved context. WhencreateDemoBackup()is subsequently called, it invokes$this->currentOrganization->id()on lines 42, 51, and 63, which dereferences$this->organization->idwithout a null-check, resulting in aTypeErrorbecause$organizationis stillnull.The registration test masks this crash by mocking the entire service via
$this->app->instance(), so the real code path is never executed.Fix: Call
$this->currentOrganization->set($mainOrg)after creating and attaching the user to the main organization, or add a guard check increateDemoBackup()before callingid().🤖 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 `@app/Services/DemoBackupService.php` around lines 14 - 63, The createDemoBackup() method in DemoBackupService calls $this->currentOrganization->id() assuming CurrentOrganization is resolved, which can be null during first-user registration; fix by setting the current organization immediately after creating/attaching the main organization in the user-registration flow (e.g., in CreateNewUser after you create $mainOrg and attach the user, call CurrentOrganization::set($mainOrg) or switchTo($mainOrg)) so DemoBackupService::createDemoBackup() always sees a resolved organization, or alternatively add a null-guard in DemoBackupService::createDemoBackup() that throws a clear RuntimeException if CurrentOrganization has no organization set before calling id().tests/Feature/User/EditTest.php (1)
14-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse factory states for role assignment instead of manual attributes.
The factory properly handles the post-migration virtual
'role'attribute throughnewModel()interception andafterCreating()pivot attachment, so the tests will work. However, per the coding guideline to check for custom states before manually setting up models, replace direct attribute passing with the available factory states:->viewer(),->admin(),->demo(). For member role, omit the attribute entirely since it is the default.Lines 15, 24, 38, 90 and similar patterns should use:
User::factory()->viewer()->create()instead of->create(['role' => 'viewer']), andUser::factory()->create()instead of->create(['role' => 'member']).🤖 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 `@tests/Feature/User/EditTest.php` around lines 14 - 39, Replace direct role attribute creation in the User factory calls with the provided factory states and defaults: use User::factory()->viewer()->create() (and ->admin() or ->demo() where appropriate) for explicit viewer/admin/demo users, and simply User::factory()->create() for default member users; apply this change in the tests named "non-admin users cannot access edit user page", "viewer cannot access edit user page", "admin can edit another user" (and any similar tests) so tests rely on the factory states/newModel interception and afterCreating pivot logic rather than manually passing ['role' => ...].
🟡 Minor comments (6)
config/oauth.php-15-25 (1)
15-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
OIDC_prefix is inconsistent with all other OAuth config env varsThe config comment says "auto-created OAuth/OIDC users" (covering Google, GitHub, GitLab, and OIDC alike), but the env variable is named
OIDC_DEFAULT_ORGANIZATION_ID. Every other option in this file uses theOAUTH_prefix. An operator configuring Google or GitHub OAuth will not expect to look for anOIDC_-prefixed variable to control organization assignment.🔧 Suggested rename
- 'default_organization_id' => env('OIDC_DEFAULT_ORGANIZATION_ID'), + 'default_organization_id' => env('OAUTH_DEFAULT_ORGANIZATION_ID'),Update
.env.exampleand the documentation page accordingly.🤖 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 `@config/oauth.php` around lines 15 - 25, The env var for the config key default_organization_id is incorrectly using the OIDC_ prefix (OIDC_DEFAULT_ORGANIZATION_ID) which is inconsistent with other options; change the env lookup to use OAUTH_DEFAULT_ORGANIZATION_ID in the config (default_organization_id => env('OAUTH_DEFAULT_ORGANIZATION_ID')), and update any related artifacts (e.g., .env.example and documentation) to reflect the new OAUTH_ name so Google/GitHub/GitLab operators can find the setting.tests/Feature/DatabaseServer/CreateTest.php-17-18 (1)
17-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Organization::first()->idwill fatal-error if no Organization exists.Every
Volume::create()call uses\App\Models\Organization::first()->id(without the null-safe operator). If an Organization isn't in the database at the moment this line executes — due to test isolation, a changed seeding order, or a future test refactor — PHP will throw a fatal error trying to access->idonnull. TheDatabaseServerFactoryalready uses the defensive formOrganization::first()?->id ??.🛡️ Proposed fix (representative example — apply to all occurrences)
-'organization_id' => \App\Models\Organization::first()->id, +'organization_id' => \App\Models\Organization::first()?->id ?? \App\Models\Organization::factory()->main()->create()->id,Alternatively, extract a shared
mainOrganization()test helper (similar to the existingdailySchedule()/weeklySchedule()helpers visible in the file) to avoid repeating this across every test.Also applies to: 110-111, 149-150, 268-269, 296-296, 313-313, 330-330, 356-357, 433-434, 474-475, 480-481
🤖 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 `@tests/Feature/DatabaseServer/CreateTest.php` around lines 17 - 18, Replace unsafe Organization::first()->id usages in Volume::create() calls with a null-safe fallback to ensure tests don't fatal when no Organization exists; specifically update each occurrence of Organization::first()->id to use Organization::first()?->id ?? Organization::factory()->create()->id (or call a shared helper like mainOrganization() that returns an existing id), and mirror the defensive pattern already used in DatabaseServerFactory so all referenced locations (e.g., the Volume::create(...) calls) use the null-safe operator plus a create fallback.app/Services/CurrentOrganization.php-130-135 (1)
130-135:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Organization::main()may return null;switchTo(null)will TypeError.If
main()returnsnull(e.g., the backfill migration failed to seed it, or a super_admin was created against a freshly-migrated schema where seeding didn't run),switchTo(Organization::main())violatesswitchTo(Organization $org). Same null-safety story applies if the main org row was manually deleted in production. A defensiveif ($main = Organization::main())plus log-or-throw on the else branch keeps the failure mode comprehensible.if ($user->isSuperAdmin()) { - $this->switchTo(Organization::main()); - - return; + $main = Organization::main(); + if ($main !== null) { + $this->switchTo($main); + } + + return; }🤖 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 `@app/Services/CurrentOrganization.php` around lines 130 - 135, The call to switchTo(Organization::main()) can pass null and cause a TypeError; update the super-admin fallback in CurrentOrganization (the block using Organization::main() and switchTo(Organization $org)) to first assign and guard the result (e.g., if ($main = Organization::main()) { $this->switchTo($main); return; } else { /* log error or throw a clear exception */ }), ensuring you either log a descriptive error or throw a specific exception when main() returns null instead of calling switchTo with null.app/Livewire/Forms/UserForm.php-89-114 (1)
89-114:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReturning
falseon last-super-admin demotion swallows feedback.
update()short-circuits withreturn falsewhen the only super admin tries to demote themselves, but the form does not surface an error message to the user. Whether the calling Livewire component displays a meaningful failure depends on its boilerplate handling of the boolean. UseaddError()(or throw aValidationException) so the message reliably reaches the UI and the form state stays consistent.♻️ Proposed fix
if (auth()->user()->isSuperAdmin()) { // Cannot remove the last super admin if ($this->user->isSuperAdmin() && ! $this->superAdmin) { if (User::where('super_admin', true)->count() === 1) { - return false; + $this->addError('superAdmin', __('Cannot remove the last super admin.')); + return false; } }🤖 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 `@app/Livewire/Forms/UserForm.php` around lines 89 - 114, The code currently returns false when preventing the last super admin from being demoted (inside the auth()->user()->isSuperAdmin() branch checking $this->user->isSuperAdmin() && ! $this->superAdmin), which silently fails; replace that return false with a proper UI-facing error by calling $this->addError('super_admin', 'Cannot remove the last super admin') (or throw a ValidationException) so the Livewire form surfaces the message and halts further updates; keep the rest of the update logic (the $this->user->update(...) calls and the pivot updateExistingPivot on organizations()) unchanged and ensure you exit the method after adding the error.database/factories/UserFactory.php-64-75 (1)
64-75:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Organization::first()may not be the main org.
afterCreatingattaches the user to whichever organization happens to come first by primary key, even if it is not flaggedis_main. In tests that pre-seed multiple orgs and rely on the user being a member of the main one (e.g. assertions usingroleIn(Organization::main())), this can quietly fail. Prefer querying the main org explicitly or fall back to creating one viaOrganizationFactory::main().♻️ Proposed fix
- $org = Organization::first() ?? Organization::factory()->main()->create(); + $org = Organization::where('is_main', true)->first() + ?? Organization::factory()->main()->create();🤖 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 `@database/factories/UserFactory.php` around lines 64 - 75, In configure()'s afterCreating closure, stop using Organization::first() and instead query for the main organization explicitly (e.g. Organization::where('is_main', true)->first() or Organization::main()->first()) and if none exists fall back to Organization::factory()->main()->create(); update the reference to Organization::first() in the closure to use this main-org lookup so tests that expect membership in the main org (and the factory method Organization::factory()->main()) reliably attach the user to the main organization.app/Models/Organization.php-104-112 (1)
104-112:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
hasResources()omitssshConfigsfrom the check, though cascade delete protects them.SSH configs are deleted when the organization is deleted (via
cascadeOnDelete()in the schema), so there's no data loss risk. However, since the method gates the delete button visibility, operators see "no resources" even when SSH configs exist. IncludesshConfigsin the check for clarity—so the UI accurately reflects all organization-scoped assets being removed.♻️ Proposed fix
public function hasResources(): bool { return $this->databaseServers()->exists() || $this->volumes()->exists() - || $this->agents()->exists(); + || $this->agents()->exists() + || $this->sshConfigs()->exists(); }🤖 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 `@app/Models/Organization.php` around lines 104 - 112, The hasResources() method currently checks databaseServers(), volumes(), and agents() but omits sshConfigs(), causing the delete UI to report no resources when SSH configs exist; update hasResources() to also include $this->sshConfigs()->exists() (i.e., return $this->databaseServers()->exists() || $this->volumes()->exists() || $this->agents()->exists() || $this->sshConfigs()->exists()) so the method reflects all organization-scoped assets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9128dfa-4f65-464b-80df-856fde8fc0b6
📒 Files selected for processing (77)
app/Actions/Fortify/CreateNewUser.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Http/Controllers/Api/V1/VolumeController.phpapp/Http/Middleware/DemoModeMiddleware.phpapp/Http/Middleware/SetCurrentOrganization.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/Dashboard/JobStatusGrid.phpapp/Livewire/Dashboard/JobsActivityChart.phpapp/Livewire/Dashboard/LatestJobs.phpapp/Livewire/Dashboard/SnapshotsCard.phpapp/Livewire/Dashboard/StorageCard.phpapp/Livewire/Dashboard/StorageDistributionChart.phpapp/Livewire/Dashboard/SuccessRateCard.phpapp/Livewire/Forms/AgentForm.phpapp/Livewire/Forms/DatabaseServerForm.phpapp/Livewire/Forms/UserForm.phpapp/Livewire/Forms/VolumeForm.phpapp/Livewire/OrganizationSwitcher.phpapp/Livewire/User/Create.phpapp/Livewire/User/Edit.phpapp/Livewire/User/Index.phpapp/Models/Agent.phpapp/Models/BackupJob.phpapp/Models/DatabaseServer.phpapp/Models/DatabaseServerSshConfig.phpapp/Models/Organization.phpapp/Models/Scopes/OrganizationScope.phpapp/Models/Snapshot.phpapp/Models/User.phpapp/Models/Volume.phpapp/Policies/OrganizationPolicy.phpapp/Policies/UserPolicy.phpapp/Providers/AppServiceProvider.phpapp/Queries/BackupJobQuery.phpapp/Queries/SnapshotQuery.phpapp/Services/CurrentOrganization.phpapp/Services/DemoBackupService.phpapp/Services/OAuthService.phpbootstrap/app.phpconfig/oauth.phpdatabase/factories/AgentFactory.phpdatabase/factories/DatabaseServerFactory.phpdatabase/factories/DatabaseServerSshConfigFactory.phpdatabase/factories/OrganizationFactory.phpdatabase/factories/UserFactory.phpdatabase/factories/VolumeFactory.phpdatabase/migrations/2026_05_05_000001_create_organizations_tables.phpdatabase/migrations/2026_05_05_000002_backfill_organizations_data.phpdatabase/migrations/2026_05_05_000003_finalize_organizations_schema.phpdatabase/seeders/DatabaseSeeder.phpdocs/docs/user-guide/organizations.mddocs/docs/user-guide/permissions.mdresources/views/layouts/app.blade.phpresources/views/livewire/api-token/index.blade.phpresources/views/livewire/configuration/_tabs.blade.phpresources/views/livewire/configuration/organization.blade.phpresources/views/livewire/organization-switcher.blade.phpresources/views/livewire/user/create.blade.phpresources/views/livewire/user/edit.blade.phpresources/views/livewire/user/index.blade.phproutes/web.phptests/Feature/Auth/OAuthTest.phptests/Feature/Auth/RegistrationTest.phptests/Feature/Dashboard/SnapshotsCardTest.phptests/Feature/DatabaseServer/CreateTest.phptests/Feature/DatabaseServer/EditTest.phptests/Feature/DatabaseServer/SshTunnelTest.phptests/Feature/DemoModeTest.phptests/Feature/Mcp/McpServerTest.phptests/Feature/Services/Backup/LocalVolumeStorageTest.phptests/Feature/Services/DemoBackupServiceTest.phptests/Feature/User/CreateTest.phptests/Feature/User/EditTest.phptests/Feature/User/IndexTest.phptests/Feature/Volume/VolumeTest.phptests/Pest.phptests/Support/IntegrationTestHelpers.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci
🧰 Additional context used
📓 Path-based instructions (6)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
app/Policies/OrganizationPolicy.phpapp/Livewire/Dashboard/JobsActivityChart.phpapp/Livewire/Dashboard/StorageDistributionChart.phptests/Feature/Dashboard/SnapshotsCardTest.phpapp/Livewire/Forms/AgentForm.phpresources/views/livewire/configuration/_tabs.blade.phpconfig/oauth.phptests/Feature/Mcp/McpServerTest.phpresources/views/livewire/organization-switcher.blade.phpdatabase/factories/DatabaseServerSshConfigFactory.phpresources/views/layouts/app.blade.phptests/Feature/Services/Backup/LocalVolumeStorageTest.phpapp/Livewire/Dashboard/LatestJobs.phpdatabase/factories/VolumeFactory.phproutes/web.phpdatabase/migrations/2026_05_05_000003_finalize_organizations_schema.phpapp/Actions/Fortify/CreateNewUser.phpapp/Models/BackupJob.phpapp/Livewire/Dashboard/JobStatusGrid.phpdatabase/factories/DatabaseServerFactory.phpdatabase/factories/AgentFactory.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Models/DatabaseServerSshConfig.phpapp/Livewire/Dashboard/StorageCard.phptests/Feature/DatabaseServer/EditTest.phpdatabase/factories/OrganizationFactory.phpresources/views/livewire/configuration/organization.blade.phpapp/Models/Agent.phpapp/Livewire/Forms/DatabaseServerForm.phpdatabase/migrations/2026_05_05_000001_create_organizations_tables.phpapp/Livewire/Dashboard/SuccessRateCard.phptests/Feature/DemoModeTest.phptests/Feature/DatabaseServer/SshTunnelTest.phpapp/Models/Snapshot.phpapp/Http/Middleware/SetCurrentOrganization.phpapp/Models/Volume.phptests/Feature/Services/DemoBackupServiceTest.phpapp/Http/Controllers/Api/V1/VolumeController.phpresources/views/livewire/api-token/index.blade.phpapp/Models/Organization.phptests/Pest.phpapp/Livewire/User/Edit.phpbootstrap/app.phpapp/Queries/SnapshotQuery.phpapp/Livewire/Dashboard/SnapshotsCard.phptests/Feature/User/IndexTest.phpapp/Models/DatabaseServer.phpapp/Livewire/Configuration/Organization.phpapp/Services/DemoBackupService.phptests/Support/IntegrationTestHelpers.phpdatabase/migrations/2026_05_05_000002_backfill_organizations_data.phpresources/views/livewire/user/index.blade.phpapp/Policies/UserPolicy.phpapp/Services/CurrentOrganization.phpdatabase/seeders/DatabaseSeeder.phpapp/Livewire/User/Index.phpapp/Providers/AppServiceProvider.phptests/Feature/DatabaseServer/CreateTest.phpapp/Models/Scopes/OrganizationScope.phpapp/Livewire/Forms/VolumeForm.phptests/Feature/User/CreateTest.phptests/Feature/Auth/OAuthTest.phpapp/Http/Middleware/DemoModeMiddleware.phpapp/Livewire/User/Create.phpapp/Livewire/Forms/UserForm.phpdatabase/factories/UserFactory.phpapp/Livewire/OrganizationSwitcher.phpapp/Models/User.phpapp/Services/OAuthService.phpresources/views/livewire/user/edit.blade.phptests/Feature/Volume/VolumeTest.phpapp/Queries/BackupJobQuery.phptests/Feature/Auth/RegistrationTest.phptests/Feature/User/EditTest.phpresources/views/livewire/user/create.blade.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
app/Policies/OrganizationPolicy.phpapp/Livewire/Dashboard/JobsActivityChart.phpapp/Livewire/Dashboard/StorageDistributionChart.phptests/Feature/Dashboard/SnapshotsCardTest.phpapp/Livewire/Forms/AgentForm.phpresources/views/livewire/configuration/_tabs.blade.phpconfig/oauth.phptests/Feature/Mcp/McpServerTest.phpresources/views/livewire/organization-switcher.blade.phpdatabase/factories/DatabaseServerSshConfigFactory.phpresources/views/layouts/app.blade.phptests/Feature/Services/Backup/LocalVolumeStorageTest.phpapp/Livewire/Dashboard/LatestJobs.phpdatabase/factories/VolumeFactory.phproutes/web.phpdatabase/migrations/2026_05_05_000003_finalize_organizations_schema.phpapp/Actions/Fortify/CreateNewUser.phpapp/Models/BackupJob.phpapp/Livewire/Dashboard/JobStatusGrid.phpdatabase/factories/DatabaseServerFactory.phpdatabase/factories/AgentFactory.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Models/DatabaseServerSshConfig.phpapp/Livewire/Dashboard/StorageCard.phptests/Feature/DatabaseServer/EditTest.phpdatabase/factories/OrganizationFactory.phpresources/views/livewire/configuration/organization.blade.phpapp/Models/Agent.phpapp/Livewire/Forms/DatabaseServerForm.phpdatabase/migrations/2026_05_05_000001_create_organizations_tables.phpapp/Livewire/Dashboard/SuccessRateCard.phptests/Feature/DemoModeTest.phptests/Feature/DatabaseServer/SshTunnelTest.phpapp/Models/Snapshot.phpapp/Http/Middleware/SetCurrentOrganization.phpapp/Models/Volume.phptests/Feature/Services/DemoBackupServiceTest.phpapp/Http/Controllers/Api/V1/VolumeController.phpresources/views/livewire/api-token/index.blade.phpapp/Models/Organization.phptests/Pest.phpapp/Livewire/User/Edit.phpbootstrap/app.phpapp/Queries/SnapshotQuery.phpapp/Livewire/Dashboard/SnapshotsCard.phptests/Feature/User/IndexTest.phpapp/Models/DatabaseServer.phpapp/Livewire/Configuration/Organization.phpapp/Services/DemoBackupService.phptests/Support/IntegrationTestHelpers.phpdatabase/migrations/2026_05_05_000002_backfill_organizations_data.phpresources/views/livewire/user/index.blade.phpapp/Policies/UserPolicy.phpapp/Services/CurrentOrganization.phpdatabase/seeders/DatabaseSeeder.phpapp/Livewire/User/Index.phpapp/Providers/AppServiceProvider.phptests/Feature/DatabaseServer/CreateTest.phpapp/Models/Scopes/OrganizationScope.phpapp/Livewire/Forms/VolumeForm.phptests/Feature/User/CreateTest.phptests/Feature/Auth/OAuthTest.phpapp/Http/Middleware/DemoModeMiddleware.phpapp/Livewire/User/Create.phpapp/Livewire/Forms/UserForm.phpdatabase/factories/UserFactory.phpapp/Livewire/OrganizationSwitcher.phpapp/Models/User.phpapp/Services/OAuthService.phpresources/views/livewire/user/edit.blade.phptests/Feature/Volume/VolumeTest.phpapp/Queries/BackupJobQuery.phptests/Feature/Auth/RegistrationTest.phptests/Feature/User/EditTest.phpresources/views/livewire/user/create.blade.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Livewire/**/*.php: In Livewire components, use#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data.
In Livewire components, useSession::flash()for one-time messages and show them via@if (session('success'))in Blade templates.
In Livewire components, return$this->redirect()withnavigate: truefor SPA-like navigation.
In Mary UI modals, add a boolean property to the component class and usewire:modelin the Blade template.
Keep state server-side in Livewire components so the UI reflects it. Validate and authorize in actions as you would in HTTP requests.
Files:
app/Livewire/Dashboard/JobsActivityChart.phpapp/Livewire/Dashboard/StorageDistributionChart.phpapp/Livewire/Forms/AgentForm.phpapp/Livewire/Dashboard/LatestJobs.phpapp/Livewire/Dashboard/JobStatusGrid.phpapp/Livewire/Dashboard/StorageCard.phpapp/Livewire/Forms/DatabaseServerForm.phpapp/Livewire/Dashboard/SuccessRateCard.phpapp/Livewire/User/Edit.phpapp/Livewire/Dashboard/SnapshotsCard.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/User/Index.phpapp/Livewire/Forms/VolumeForm.phpapp/Livewire/User/Create.phpapp/Livewire/Forms/UserForm.phpapp/Livewire/OrganizationSwitcher.php
tests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.php: When creating models for tests, use factories. Check if the factory has custom states before manually setting up the model.
Use faker methods such as$this->faker->word()orfake()->randomDigit()when generating test data. Follow existing conventions on whether to use$this->fakerorfake().
Create Pest tests usingphp artisan make:test --pest {name}without including the test suite directory. Usephp artisan make:test --pest SomeFeatureTestinstead ofphp artisan make:test --pest Feature/SomeFeatureTest.
Files:
tests/Feature/Dashboard/SnapshotsCardTest.phptests/Feature/Mcp/McpServerTest.phptests/Feature/Services/Backup/LocalVolumeStorageTest.phptests/Feature/DatabaseServer/EditTest.phptests/Feature/DemoModeTest.phptests/Feature/DatabaseServer/SshTunnelTest.phptests/Feature/Services/DemoBackupServiceTest.phptests/Pest.phptests/Feature/User/IndexTest.phptests/Support/IntegrationTestHelpers.phptests/Feature/DatabaseServer/CreateTest.phptests/Feature/User/CreateTest.phptests/Feature/Auth/OAuthTest.phptests/Feature/Volume/VolumeTest.phptests/Feature/Auth/RegistrationTest.phptests/Feature/User/EditTest.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
All Livewire component blade files should contain only view markup; all PHP logic must be in the component class.
Files:
resources/views/livewire/configuration/_tabs.blade.phpresources/views/livewire/organization-switcher.blade.phpresources/views/livewire/configuration/organization.blade.phpresources/views/livewire/api-token/index.blade.phpresources/views/livewire/user/index.blade.phpresources/views/livewire/user/edit.blade.phpresources/views/livewire/user/create.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/**/*.blade.php: All UI components in Mary UI should be prefixed withx-(e.g.,<x-button>,<x-input>,<x-card>) and use Heroicons for icons (e.g.,icon="o-user"for outline,icon="s-user"for solid).
In Mary UI select components, use the:optionsprop with array format[['id' => 'value', 'name' => 'Label']].
In Mary UI alerts, useclass="alert-success",class="alert-error", etc. (notvariantprop).
Use Alpine.js for client-side interactions in Livewire instead of JavaScript frameworks.
In Blade templates, use:attrbinding (dynamic syntax) instead of{{ }}interpolation when passing translated strings to component attributes to avoid double-encoding special characters.
Files:
resources/views/livewire/configuration/_tabs.blade.phpresources/views/livewire/organization-switcher.blade.phpresources/views/layouts/app.blade.phpresources/views/livewire/configuration/organization.blade.phpresources/views/livewire/api-token/index.blade.phpresources/views/livewire/user/index.blade.phpresources/views/livewire/user/edit.blade.phpresources/views/livewire/user/create.blade.php
🧠 Learnings (10)
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
app/Policies/OrganizationPolicy.phpapp/Livewire/Dashboard/JobsActivityChart.phpapp/Livewire/Dashboard/StorageDistributionChart.phptests/Feature/Dashboard/SnapshotsCardTest.phpapp/Livewire/Forms/AgentForm.phpresources/views/livewire/configuration/_tabs.blade.phpconfig/oauth.phptests/Feature/Mcp/McpServerTest.phpresources/views/livewire/organization-switcher.blade.phpdatabase/factories/DatabaseServerSshConfigFactory.phpresources/views/layouts/app.blade.phptests/Feature/Services/Backup/LocalVolumeStorageTest.phpapp/Livewire/Dashboard/LatestJobs.phpdatabase/factories/VolumeFactory.phproutes/web.phpdatabase/migrations/2026_05_05_000003_finalize_organizations_schema.phpapp/Actions/Fortify/CreateNewUser.phpapp/Models/BackupJob.phpapp/Livewire/Dashboard/JobStatusGrid.phpdatabase/factories/DatabaseServerFactory.phpdatabase/factories/AgentFactory.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Models/DatabaseServerSshConfig.phpapp/Livewire/Dashboard/StorageCard.phptests/Feature/DatabaseServer/EditTest.phpdatabase/factories/OrganizationFactory.phpresources/views/livewire/configuration/organization.blade.phpapp/Models/Agent.phpapp/Livewire/Forms/DatabaseServerForm.phpdatabase/migrations/2026_05_05_000001_create_organizations_tables.phpapp/Livewire/Dashboard/SuccessRateCard.phptests/Feature/DemoModeTest.phptests/Feature/DatabaseServer/SshTunnelTest.phpapp/Models/Snapshot.phpapp/Http/Middleware/SetCurrentOrganization.phpapp/Models/Volume.phptests/Feature/Services/DemoBackupServiceTest.phpapp/Http/Controllers/Api/V1/VolumeController.phpresources/views/livewire/api-token/index.blade.phpapp/Models/Organization.phptests/Pest.phpapp/Livewire/User/Edit.phpbootstrap/app.phpapp/Queries/SnapshotQuery.phpapp/Livewire/Dashboard/SnapshotsCard.phptests/Feature/User/IndexTest.phpapp/Models/DatabaseServer.phpapp/Livewire/Configuration/Organization.phpapp/Services/DemoBackupService.phptests/Support/IntegrationTestHelpers.phpdatabase/migrations/2026_05_05_000002_backfill_organizations_data.phpresources/views/livewire/user/index.blade.phpapp/Policies/UserPolicy.phpapp/Services/CurrentOrganization.phpdatabase/seeders/DatabaseSeeder.phpapp/Livewire/User/Index.phpapp/Providers/AppServiceProvider.phptests/Feature/DatabaseServer/CreateTest.phpapp/Models/Scopes/OrganizationScope.phpapp/Livewire/Forms/VolumeForm.phptests/Feature/User/CreateTest.phptests/Feature/Auth/OAuthTest.phpapp/Http/Middleware/DemoModeMiddleware.phpapp/Livewire/User/Create.phpapp/Livewire/Forms/UserForm.phpdatabase/factories/UserFactory.phpapp/Livewire/OrganizationSwitcher.phpapp/Models/User.phpapp/Services/OAuthService.phpresources/views/livewire/user/edit.blade.phptests/Feature/Volume/VolumeTest.phpapp/Queries/BackupJobQuery.phptests/Feature/Auth/RegistrationTest.phptests/Feature/User/EditTest.phpresources/views/livewire/user/create.blade.php
📚 Learning: 2026-02-18T09:45:52.485Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 116
File: app/Livewire/DatabaseServer/ConnectionStatus.php:18-18
Timestamp: 2026-02-18T09:45:52.485Z
Learning: In Livewire components, Eloquent model properties (e.g., public DatabaseServer $server) are automatically locked by the framework to prevent client-side ID tampering. The #[Locked] attribute is only needed for scalar properties (int, string, bool, etc.) that require protection from client-side mutation. Apply this guidance to all Livewire PHP components; use #[Locked] only on primitive properties that you want to shield from client manipulation, and rely on automatic locking for Eloquent model properties.
Applied to files:
app/Livewire/Dashboard/JobsActivityChart.phpapp/Livewire/Dashboard/StorageDistributionChart.phpapp/Livewire/Forms/AgentForm.phpapp/Livewire/Dashboard/LatestJobs.phpapp/Livewire/Dashboard/JobStatusGrid.phpapp/Livewire/Dashboard/StorageCard.phpapp/Livewire/Forms/DatabaseServerForm.phpapp/Livewire/Dashboard/SuccessRateCard.phpapp/Livewire/User/Edit.phpapp/Livewire/Dashboard/SnapshotsCard.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/User/Index.phpapp/Livewire/Forms/VolumeForm.phpapp/Livewire/User/Create.phpapp/Livewire/Forms/UserForm.phpapp/Livewire/OrganizationSwitcher.php
📚 Learning: 2026-02-13T11:05:37.072Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 105
File: tests/Feature/Services/Backup/Databases/SqliteDatabaseTest.php:8-11
Timestamp: 2026-02-13T11:05:37.072Z
Learning: Adopt a global afterEach hook in tests/Pest.php (or equivalent Pest bootstrap) to clean up temporary directories created during tests. Specifically handle temp dirs named with the prefixes sqlite-db-test-*, backup-task-test-*, restore-task-test-*, and volume-test-*, so individual test files don’t need their own cleanup logic. This applies to all PHP test files under the tests directory.
Applied to files:
tests/Feature/Dashboard/SnapshotsCardTest.phptests/Feature/Mcp/McpServerTest.phptests/Feature/Services/Backup/LocalVolumeStorageTest.phptests/Feature/DatabaseServer/EditTest.phptests/Feature/DemoModeTest.phptests/Feature/DatabaseServer/SshTunnelTest.phptests/Feature/Services/DemoBackupServiceTest.phptests/Pest.phptests/Feature/User/IndexTest.phptests/Support/IntegrationTestHelpers.phptests/Feature/DatabaseServer/CreateTest.phptests/Feature/User/CreateTest.phptests/Feature/Auth/OAuthTest.phptests/Feature/Volume/VolumeTest.phptests/Feature/Auth/RegistrationTest.phptests/Feature/User/EditTest.php
📚 Learning: 2026-04-09T13:59:25.873Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 205
File: tests/Feature/ConfigurationTest.php:165-176
Timestamp: 2026-04-09T13:59:25.873Z
Learning: In this repository, `Notification::fake()` is already performed globally in the base test class (`tests/TestCase.php`) within `setUp()` before each test. When reviewing individual test files under `tests/`, do not flag missing `Notification::fake()` calls, since they are handled by the shared base test setup.
Applied to files:
tests/Feature/Dashboard/SnapshotsCardTest.phptests/Feature/Mcp/McpServerTest.phptests/Feature/Services/Backup/LocalVolumeStorageTest.phptests/Feature/DatabaseServer/EditTest.phptests/Feature/DemoModeTest.phptests/Feature/DatabaseServer/SshTunnelTest.phptests/Feature/Services/DemoBackupServiceTest.phptests/Pest.phptests/Feature/User/IndexTest.phptests/Support/IntegrationTestHelpers.phptests/Feature/DatabaseServer/CreateTest.phptests/Feature/User/CreateTest.phptests/Feature/Auth/OAuthTest.phptests/Feature/Volume/VolumeTest.phptests/Feature/Auth/RegistrationTest.phptests/Feature/User/EditTest.php
📚 Learning: 2026-01-30T22:27:46.107Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 61
File: resources/views/livewire/volume/connectors/s3-config.blade.php:1-13
Timestamp: 2026-01-30T22:27:46.107Z
Learning: In Blade template files (any .blade.php) within the databasement project, allow using alert-info for informational content inside <x-alert> components. The guideline that permits alert-success and alert-error does not exclude using alert-info for informational purposes. Apply this consistently to all Blade components that render alerts; ensure semantic usage and accessibility.
Applied to files:
resources/views/livewire/configuration/_tabs.blade.phpresources/views/livewire/organization-switcher.blade.phpresources/views/layouts/app.blade.phpresources/views/livewire/configuration/organization.blade.phpresources/views/livewire/api-token/index.blade.phpresources/views/livewire/user/index.blade.phpresources/views/livewire/user/edit.blade.phpresources/views/livewire/user/create.blade.php
📚 Learning: 2026-02-06T10:34:43.585Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 75
File: resources/views/livewire/backup-job/_filters.blade.php:36-40
Timestamp: 2026-02-06T10:34:43.585Z
Learning: In Blade template files, when creating compact inline filter controls, prefer using native <input type="checkbox"> elements with daisyUI classes (e.g., checkbox checkbox-warning checkbox-xs) over the Mary UI <x-checkbox> component. The <x-checkbox> component adds wrapper markup (e.g., <div><fieldset><label> with gap-3) that can break tight inline flex layouts. Use the native input approach for compact inline controls, but reserve <x-checkbox> for form fields that require labels, hints, and errors.
Applied to files:
resources/views/livewire/configuration/_tabs.blade.phpresources/views/livewire/organization-switcher.blade.phpresources/views/layouts/app.blade.phpresources/views/livewire/configuration/organization.blade.phpresources/views/livewire/api-token/index.blade.phpresources/views/livewire/user/index.blade.phpresources/views/livewire/user/edit.blade.phpresources/views/livewire/user/create.blade.php
📚 Learning: 2026-02-04T12:26:27.374Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 72
File: routes/web.php:66-81
Timestamp: 2026-02-04T12:26:27.374Z
Learning: In Laravel Livewire v4, prefer routing full-page Livewire components using Route::livewire('/path', ComponentClass::class) for all full-page components (whether class-based or view-based). While Route::get('/path', ComponentClass::class) remains technically functional for backward compatibility, it is not recommended in v4. Apply this guideline in all route files under routes (e.g., routes/web.php, routes/api.php) to ensure consistent, future-proof routing for Livewire components.
Applied to files:
routes/web.php
📚 Learning: 2026-04-10T08:46:55.182Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 205
File: database/migrations/2026_04_09_000001_add_notification_channels.php:143-156
Timestamp: 2026-04-10T08:46:55.182Z
Learning: In David-Crty/databasement, treat Laravel migrations as forward-only for production. For migrations in database/migrations/*.php, it is acceptable to delete or transform legacy data in the migration’s up() (e.g., removing app_configs rows) without restoring it in down(), even if down() cannot fully reverse the state. During code review, do not flag missing rollback restoration of migrated/deleted data as a production risk for this repo’s migrations.
Applied to files:
database/migrations/2026_05_05_000003_finalize_organizations_schema.phpdatabase/migrations/2026_05_05_000001_create_organizations_tables.phpdatabase/migrations/2026_05_05_000002_backfill_organizations_data.php
📚 Learning: 2026-03-23T11:45:15.846Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 178
File: app/Http/Controllers/Api/V1/VolumeController.php:116-120
Timestamp: 2026-03-23T11:45:15.846Z
Learning: For the API v1 `testConnection` endpoints in this repo (e.g., `VolumeController::testConnection`, `DatabaseServerController::testConnection`), keep using `$this->authorize('view', $model)` for authorization. This is an intentional maintainer design choice; do not flag it as a bug or as insufficiently strict authorization in future reviews for these `testConnection` actions.
Applied to files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Http/Controllers/Api/V1/VolumeController.php
📚 Learning: 2026-03-28T22:33:16.333Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 195
File: app/Notifications/CLAUDE.md:7-12
Timestamp: 2026-03-28T22:33:16.333Z
Learning: In this repository (David-Crty/databasement), do not flag fenced Markdown code blocks that omit a language identifier (i.e., MD040: missing language after ```). The project treats unlabeled fenced code blocks as plain text by default, so missing language tags are acceptable for Markdown files.
Applied to files:
docs/docs/user-guide/permissions.mddocs/docs/user-guide/organizations.md
🪛 markdownlint-cli2 (0.22.1)
docs/docs/user-guide/organizations.md
[warning] 65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| @php $tokenRole = $token->tokenable->roleIn(app(\App\Services\CurrentOrganization::class)->model()); @endphp | ||
| <span class="badge badge-sm badge-ghost ml-1">{{ ucfirst($tokenRole ?? 'member') }}</span> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
PHP logic in Blade violates the project's Livewire view guideline.
The @php block resolves the container service and calls roleIn() directly in the template. Per the coding guideline: "All Livewire component blade files should contain only view markup; all PHP logic must be in the component class." This logic should be moved to the ApiToken\Index component class.
As an added concern, roleIn() (context snippet — app/Models/User.php:204-212) issues a database query per unique token owner that isn't already cached, creating an N+1 for token lists with many distinct owners.
♻️ Suggested approach
In the Livewire component class (app/Livewire/ApiToken/Index.php or equivalent), precompute roles before passing $tokens to the view:
// Example: expose a helper or eager-compute into a collection
public function getTokensWithRoles(): \Illuminate\Support\Collection
{
$org = app(\App\Services\CurrentOrganization::class)->model();
return $this->tokens->map(function ($token) use ($org) {
$token->currentOrgRole = $token->tokenable?->roleIn($org);
return $token;
});
}In the Blade template, replace the @php block with a simple property access:
-@php $tokenRole = $token->tokenable->roleIn(app(\App\Services\CurrentOrganization::class)->model()); `@endphp`
-<span class="badge badge-sm badge-ghost ml-1">{{ ucfirst($tokenRole ?? 'member') }}</span>
+<span class="badge badge-sm badge-ghost ml-1">{{ ucfirst($token->currentOrgRole ?? 'member') }}</span>As per coding guidelines, "All Livewire component blade files should contain only view markup; all PHP logic must be in the component class."
🤖 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 `@resources/views/livewire/api-token/index.blade.php` around lines 29 - 30,
Move the PHP logic out of the Blade and into the Livewire component class
(ApiToken\Index): compute each token owner's role against the current
organization (use app(\App\Services\CurrentOrganization::class)->model()) in a
method like getTokensWithRoles() or during component initialization by iterating
$this->tokens and setting a new attribute (e.g., currentOrgRole) on each token
using tokenable->roleIn($org) so the DB query is done once per unique owner and
cached; then update the view to simply read the precomputed property (e.g.,
$token->currentOrgRole or fallback 'member') instead of using the `@php` block
that calls roleIn() directly.
| @php | ||
| $roleClass = match($user->role) { | ||
| $currentOrg = app(\App\Services\CurrentOrganization::class); | ||
| $orgRole = $user->roleIn($currentOrg->model()); | ||
| $displayRole = $orgRole ?? 'member'; | ||
| $roleClass = match($displayRole) { | ||
| 'admin' => 'badge-primary', | ||
| 'member' => 'badge-info', | ||
| 'viewer' => 'badge-neutral', | ||
| default => 'badge-ghost', | ||
| }; | ||
| @endphp | ||
| <x-badge :value="ucfirst($user->role)" :icon="\App\Models\User::roleIcon($user->role)" class="{{ $roleClass }}" /> | ||
| <div class="flex items-center gap-1"> | ||
| @if($user->isSuperAdmin()) | ||
| <x-badge value="Super Admin" icon="o-star" class="badge-warning" /> | ||
| @endif | ||
| @if($orgRole) | ||
| <x-badge :value="ucfirst($displayRole)" :icon="\App\Models\User::roleIcon($displayRole)" class="{{ $roleClass }}" /> | ||
| @endif | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Move per-row role/badge logic out of the Livewire blade.
The @php block resolves a service and derives $orgRole, $displayRole, $roleClass for every user row. As per coding guidelines, "All Livewire component blade files should contain only view markup; all PHP logic must be in the component class." Surface the resolved values from App\Livewire\User\Index (e.g., a roleBadge(User $user): array helper, or pre-compute a $roleBadges keyed by user id during render()), and have the blade just render them.
♻️ Sketch of the refactor
In app/Livewire/User/Index.php:
/**
* `@return` array{role: ?string, label: string, class: string, icon: string|null}
*/
public function roleBadge(User $user): array
{
$orgRole = $user->roleIn($this->currentOrg->model());
$display = $orgRole ?? 'member';
return [
'role' => $orgRole,
'label' => ucfirst($display),
'class' => match ($display) {
'admin' => 'badge-primary',
'member' => 'badge-info',
'viewer' => 'badge-neutral',
default => 'badge-ghost',
},
'icon' => User::roleIcon($display),
];
}In the blade:
`@scope`('cell_role', $user)
- `@php`
- $currentOrg = app(\App\Services\CurrentOrganization::class);
- $orgRole = $user->roleIn($currentOrg->model());
- $displayRole = $orgRole ?? 'member';
- $roleClass = match($displayRole) {
- 'admin' => 'badge-primary',
- 'member' => 'badge-info',
- 'viewer' => 'badge-neutral',
- default => 'badge-ghost',
- };
- `@endphp`
+ `@php`($badge = $this->roleBadge($user))
<div class="flex items-center gap-1">
`@if`($user->isSuperAdmin())
- <x-badge value="Super Admin" icon="o-star" class="badge-warning" />
+ <x-badge :value="__('Super Admin')" icon="o-star" class="badge-warning" />
`@endif`
- `@if`($orgRole)
- <x-badge :value="ucfirst($displayRole)" :icon="\App\Models\User::roleIcon($displayRole)" class="{{ $roleClass }}" />
+ `@if`($badge['role'])
+ <x-badge :value="__($badge['label'])" :icon="$badge['icon']" class="{{ $badge['class'] }}" />
`@endif`
</div>
`@endscope`As per coding guidelines: "All Livewire component blade files should contain only view markup; all PHP logic must be in the component class." and "Translations should use __('...') helper function".
🤖 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 `@resources/views/livewire/user/index.blade.php` around lines 49 - 67, Extract
the per-row role/badge PHP into the Livewire component (App\Livewire\User\Index)
— add a method like roleBadge(User $user): array (or compute a $roleBadges keyed
by $user->id in render()) that resolves $orgRole via $this->currentOrg->model(),
derives display ('member' fallback), maps the class via the same match logic,
and returns label, class, role and icon (use User::roleIcon()). Then update the
blade (resources/views/livewire/user/index.blade.php) to only read the provided
array (e.g. $this->roleBadge($user) or $roleBadges[$user->id]) and render
badges; replace hard-coded strings with __('...') where applicable.
Delete unused methods (User::canPerformActionsIn, User::isViewer, CurrentOrganization::canPerformActions, UserForm::$orgAttachments) and fix Organization::hasResources() to bypass global scopes so it checks the target org's resources rather than the current session org. Add tests for OrganizationPolicy, Configuration\Organization CRUD, OrganizationSwitcher authorization, and User\Create::addExisting().
- Abort 403 when API client explicitly requests an invalid/inaccessible org via X-Organization-Id or ?org= instead of silently falling back - Fix withCount in Organization admin page to bypass OrganizationScope so counts are correct for non-current orgs - Use manageOrgMembership policy for addExisting() instead of create - Prevent org admins from editing super admin users in UserPolicy - Replace hardcoded role strings with User::ROLE_* constants - Remove unnecessary isResolved() guards in web-only code paths - Fix N+1 in user index by eager-loading organizations relation and making roleIn() use loaded collection when available
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
app/Services/CurrentOrganization.php (1)
44-55: ⚡ Quick winReuse
User::roleIn()for the current-org lookup.This method bypasses the eager-loaded/cache-aware path added in
User::roleIn(), souserRole()still does a fresh pivot query even whenorganizationsis already loaded. ReusingroleIn($this->organization)keeps a single source of truth and preserves the N+1 fix elsewhere in the PR.Proposed change
- $pivot = $user->organizations()->wherePivot('organization_id', $this->organization->id)->first(); - - return $pivot?->pivot?->role; // `@phpstan-ignore` property.notFound + return $user->roleIn($this->organization);🤖 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 `@app/Services/CurrentOrganization.php` around lines 44 - 55, Replace the manual pivot lookup in userRole() with the existing User::roleIn() helper to preserve eager-loaded/cache-aware behavior: obtain the current user (as you already do), return null if missing, then call $user->roleIn($this->organization) and return its result instead of performing the organizations()->wherePivot(...) pivot query; remove the manual pivot access so userRole() delegates to User::roleIn() as the single source of truth.app/Livewire/Configuration/Organization.php (1)
44-48: ⚡ Quick winOnly bypass
OrganizationScopein these count subqueries.
withoutGlobalScopes()fixes the org-count bug, but it also drops any other global scope those models may gain later. These counts only need to ignore tenant scoping.Proposed change
+use App\Models\Scopes\OrganizationScope; ... - 'databaseServers' => fn ($q) => $q->withoutGlobalScopes(), - 'volumes' => fn ($q) => $q->withoutGlobalScopes(), - 'agents' => fn ($q) => $q->withoutGlobalScopes(), + 'databaseServers' => fn ($q) => $q->withoutGlobalScope(OrganizationScope::class), + 'volumes' => fn ($q) => $q->withoutGlobalScope(OrganizationScope::class), + 'agents' => fn ($q) => $q->withoutGlobalScope(OrganizationScope::class),🤖 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 `@app/Livewire/Configuration/Organization.php` around lines 44 - 48, The count subqueries on OrganizationModel are calling withoutGlobalScopes(), which removes all global scopes (risking future scope loss); change each subquery for 'databaseServers', 'volumes', and 'agents' to only remove the tenant OrganizationScope (e.g., use withoutGlobalScope(OrganizationScope::class)) so the users/databaseServers/volumes/agents counts bypass only tenant scoping while preserving other global scopes; locate these calls on OrganizationModel::withCount([...]) and replace withoutGlobalScopes() with withoutGlobalScope(OrganizationScope::class).app/Livewire/Forms/UserForm.php (1)
21-22: ⚡ Quick winUse the role constants in the validators too.
roleOptions()already derives the allowed values fromUser::ROLE_*, but the validation rules still hardcode'viewer,member,admin'. That gives this form two separate definitions of the same contract.Proposed change
- #[Validate('required|in:viewer,member,admin')] + #[Validate('required|in:'.User::ROLE_VIEWER.','.User::ROLE_MEMBER.','.User::ROLE_ADMIN)] public string $role = User::ROLE_MEMBER; ... - 'role' => 'required|in:viewer,member,admin', + 'role' => 'required|in:'.User::ROLE_VIEWER.','.User::ROLE_MEMBER.','.User::ROLE_ADMIN, ... - 'role' => 'required|in:viewer,member,admin', + 'role' => 'required|in:'.User::ROLE_VIEWER.','.User::ROLE_MEMBER.','.User::ROLE_ADMIN,Also applies to: 40-44, 63-67
🤖 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 `@app/Livewire/Forms/UserForm.php` around lines 21 - 22, The Validate attribute on the $role property hardcodes allowed roles; remove that hardcoded attribute and move the rule into a dynamic rules() method (or update the existing rules provider) so it builds the in: list from the source of truth (User::roleOptions() or the User::ROLE_* constants) — e.g. return ['role' => 'required|in:' . implode(',', array_values(User::roleOptions()))]; apply the same change to the other occurrences noted (the other Validate usages at the referenced blocks) so validation always derives allowed values from User::ROLE_* rather than a hardcoded string.app/Livewire/User/Index.php (1)
142-156: ⚡ Quick winConsolidate redundant org filters and drop
whereRaw.When
roleFilteris set, the query emits two EXISTS subqueries onorganization_userfor the sameorganization_id— one fromwhereRelation(line 144) and one fromwhereHas(line 155). They can be folded into a singlewhereHasthat also covers the role predicate, removing the duplicate EXISTS and thewhereRaw.♻️ Proposed refactor
$currentOrg = app(CurrentOrganization::class); - $query = User::query(); - - $query->whereRelation('organizations', 'organization_id', $currentOrg->id()); - - $users = $query + $users = User::query() ->with('organizations') + ->whereHas('organizations', function ($q) use ($currentOrg) { + $q->where('organization_id', $currentOrg->id()); + if ($this->roleFilter !== '') { + $q->where('role', $this->roleFilter); + } + }) ->when($this->search, function ($query) { $query->where(function ($q) { $q->where('name', 'like', '%'.$this->search.'%') ->orWhere('email', 'like', '%'.$this->search.'%'); }); }) - ->when($this->roleFilter !== '', function ($query) use ($currentOrg) { - $query->whereHas('organizations', fn ($q) => $q->whereRaw('organization_id = ? and role = ?', [$currentOrg->id(), $this->roleFilter])); - }) ->when($this->statusFilter !== '', function ($query) {🤖 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 `@app/Livewire/User/Index.php` around lines 142 - 156, Remove the initial whereRelation call and consolidate the organization filter into the existing whereHas on the organizations relation: use User::query()->when($this->search, ...)->whereHas('organizations', function($q) use ($currentOrg) { $q->where('organization_id', $currentOrg->id()); if ($this->roleFilter !== '') { $q->where('role', $this->roleFilter); } }); this removes the duplicate EXISTS and replaces whereRaw with explicit where(...) calls on the organizations relation (keep the ->with('organizations') and existing search logic intact).tests/Feature/User/CreateTest.php (1)
59-79: 💤 Low valueNit: import
Organizationinstead of repeating the FQN.
\App\Models\Organizationis referenced three times (lines 59, 67, 78). Ausestatement at the top would match the convention used intests/Feature/Configuration/OrganizationTest.php(which aliases it asOrganizationModel).♻️ Proposed cleanup
use App\Livewire\User\Create; +use App\Models\Organization; use App\Models\User; use Livewire\Livewire; @@ - expect($user->roleIn(\App\Models\Organization::main()))->toBe('member'); + expect($user->roleIn(Organization::main()))->toBe('member'); @@ - $otherOrg = \App\Models\Organization::factory()->create(); + $otherOrg = Organization::factory()->create(); @@ - expect($existingUser->roleIn(\App\Models\Organization::main()))->toBe('viewer'); + expect($existingUser->roleIn(Organization::main()))->toBe('viewer');🤖 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 `@tests/Feature/User/CreateTest.php` around lines 59 - 79, Import the Organization model at the top and replace the three FQN usages to match the project convention; add a use statement for App\Models\Organization (or alias it as OrganizationModel to follow OrganizationTest) and change the three occurrences of \App\Models\Organization::main() in the CreateTest Livewire test to Organization::main() (or OrganizationModel::main() if aliased) so the test uses the imported symbol instead of the fully-qualified names.
🤖 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 `@app/Http/Middleware/SetCurrentOrganization.php`:
- Around line 45-51: The code only treats non-empty org query/header values as
explicit; change the $hasExplicitOrg check in SetCurrentOrganization to detect
presence even if blank by using the underlying parameter bags: use
$request->query->has('org') || $request->headers->has('X-Organization-Id') so an
explicit but empty selector (e.g. ?org= or empty header) will be considered
explicit and thus trigger the existing 403 logic after resolveApiOrgId and
currentOrganization->resolveForUser.
In `@app/Livewire/User/Create.php`:
- Around line 55-63: The attach call can race and violate the pivot unique
constraint; make the insert idempotent by replacing the
organizations()->attach(...) with an idempotent write (e.g.
organizations()->syncWithoutDetaching([$currentOrg->id() => ['role' =>
$this->existingUserRole]])) or wrap the organizations()->attach(...) in a
try/catch for QueryException and call $this->addError('existingUserId', __('This
user is already a member of this organization.')) when a duplicate-key error is
detected; locate this logic around User::findOrFail($this->existingUserId),
belongsToOrganization(...), organizations()->attach(...) and update accordingly.
In `@app/Models/User.php`:
- Around line 188-216: roleIn() memoizes per-organization roles in $cachedRoles
but that cache is only cleared by refresh(), so pivots updated via attach() or
updateExistingPivot() can leave stale values; add a small invalidation helper on
the User model (e.g., clearCachedRoleForOrg(int|string $orgId) or
clearCachedRoles(?int $orgId) that unsets $this->cachedRoles[$orgId] (or clears
the whole array if null) and invoke this helper immediately after any pivot
write in your write paths (calls that use attach() or updateExistingPivot()),
for example from UserForm::setUser() and UserForm::update() after they modify
the organization pivot so roleIn()/isAdmin() will reflect the new pivot without
needing refresh().
In `@app/Policies/UserPolicy.php`:
- Around line 86-89: The copyInvitationLink policy currently restricts access to
super admins only; update the UserPolicy::copyInvitationLink method to mirror
the logic used in update() by allowing either a super admin OR an org admin of
the same organization to copy the invitation link, while still requiring the
model to have a non-null invitation_token; use existing helpers like
isSuperAdmin() and isOrgAdmin() and compare the current user's org id to
$model->org_id to implement this relaxation.
In `@tests/Feature/OrganizationSwitcherTest.php`:
- Around line 19-27: The test calls OrganizationSwitcher::switchOrg but only
asserts no redirect; capture the component's initial currentOrgId (or the user's
first organization id) before invoking switchOrg and after the call assert that
the component's "currentOrgId" remains equal to that initial value (use
OrganizationSwitcher, the switchOrg method and the currentOrgId property) so the
negative path verifies the org did not change.
---
Nitpick comments:
In `@app/Livewire/Configuration/Organization.php`:
- Around line 44-48: The count subqueries on OrganizationModel are calling
withoutGlobalScopes(), which removes all global scopes (risking future scope
loss); change each subquery for 'databaseServers', 'volumes', and 'agents' to
only remove the tenant OrganizationScope (e.g., use
withoutGlobalScope(OrganizationScope::class)) so the
users/databaseServers/volumes/agents counts bypass only tenant scoping while
preserving other global scopes; locate these calls on
OrganizationModel::withCount([...]) and replace withoutGlobalScopes() with
withoutGlobalScope(OrganizationScope::class).
In `@app/Livewire/Forms/UserForm.php`:
- Around line 21-22: The Validate attribute on the $role property hardcodes
allowed roles; remove that hardcoded attribute and move the rule into a dynamic
rules() method (or update the existing rules provider) so it builds the in: list
from the source of truth (User::roleOptions() or the User::ROLE_* constants) —
e.g. return ['role' => 'required|in:' . implode(',',
array_values(User::roleOptions()))]; apply the same change to the other
occurrences noted (the other Validate usages at the referenced blocks) so
validation always derives allowed values from User::ROLE_* rather than a
hardcoded string.
In `@app/Livewire/User/Index.php`:
- Around line 142-156: Remove the initial whereRelation call and consolidate the
organization filter into the existing whereHas on the organizations relation:
use User::query()->when($this->search, ...)->whereHas('organizations',
function($q) use ($currentOrg) { $q->where('organization_id',
$currentOrg->id()); if ($this->roleFilter !== '') { $q->where('role',
$this->roleFilter); } }); this removes the duplicate EXISTS and replaces
whereRaw with explicit where(...) calls on the organizations relation (keep the
->with('organizations') and existing search logic intact).
In `@app/Services/CurrentOrganization.php`:
- Around line 44-55: Replace the manual pivot lookup in userRole() with the
existing User::roleIn() helper to preserve eager-loaded/cache-aware behavior:
obtain the current user (as you already do), return null if missing, then call
$user->roleIn($this->organization) and return its result instead of performing
the organizations()->wherePivot(...) pivot query; remove the manual pivot access
so userRole() delegates to User::roleIn() as the single source of truth.
In `@tests/Feature/User/CreateTest.php`:
- Around line 59-79: Import the Organization model at the top and replace the
three FQN usages to match the project convention; add a use statement for
App\Models\Organization (or alias it as OrganizationModel to follow
OrganizationTest) and change the three occurrences of
\App\Models\Organization::main() in the CreateTest Livewire test to
Organization::main() (or OrganizationModel::main() if aliased) so the test uses
the imported symbol instead of the fully-qualified names.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 442da78a-9ab4-4658-8a59-437c107f2bbc
📒 Files selected for processing (14)
app/Http/Middleware/SetCurrentOrganization.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/Forms/UserForm.phpapp/Livewire/User/Create.phpapp/Livewire/User/Index.phpapp/Models/BackupJob.phpapp/Models/Organization.phpapp/Models/Snapshot.phpapp/Models/User.phpapp/Policies/UserPolicy.phpapp/Services/CurrentOrganization.phptests/Feature/Configuration/OrganizationTest.phptests/Feature/OrganizationSwitcherTest.phptests/Feature/User/CreateTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- app/Models/Organization.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
app/Models/BackupJob.phptests/Feature/OrganizationSwitcherTest.phpapp/Http/Middleware/SetCurrentOrganization.phpapp/Services/CurrentOrganization.phpapp/Livewire/User/Index.phpapp/Policies/UserPolicy.phptests/Feature/User/CreateTest.phptests/Feature/Configuration/OrganizationTest.phpapp/Models/Snapshot.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/Forms/UserForm.phpapp/Models/User.phpapp/Livewire/User/Create.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
app/Models/BackupJob.phptests/Feature/OrganizationSwitcherTest.phpapp/Http/Middleware/SetCurrentOrganization.phpapp/Services/CurrentOrganization.phpapp/Livewire/User/Index.phpapp/Policies/UserPolicy.phptests/Feature/User/CreateTest.phptests/Feature/Configuration/OrganizationTest.phpapp/Models/Snapshot.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/Forms/UserForm.phpapp/Models/User.phpapp/Livewire/User/Create.php
tests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.php: When creating models for tests, use factories. Check if the factory has custom states before manually setting up the model.
Use faker methods such as$this->faker->word()orfake()->randomDigit()when generating test data. Follow existing conventions on whether to use$this->fakerorfake().
Create Pest tests usingphp artisan make:test --pest {name}without including the test suite directory. Usephp artisan make:test --pest SomeFeatureTestinstead ofphp artisan make:test --pest Feature/SomeFeatureTest.
Files:
tests/Feature/OrganizationSwitcherTest.phptests/Feature/User/CreateTest.phptests/Feature/Configuration/OrganizationTest.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Livewire/**/*.php: In Livewire components, use#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data.
In Livewire components, useSession::flash()for one-time messages and show them via@if (session('success'))in Blade templates.
In Livewire components, return$this->redirect()withnavigate: truefor SPA-like navigation.
In Mary UI modals, add a boolean property to the component class and usewire:modelin the Blade template.
Keep state server-side in Livewire components so the UI reflects it. Validate and authorize in actions as you would in HTTP requests.
Files:
app/Livewire/User/Index.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/Forms/UserForm.phpapp/Livewire/User/Create.php
🧠 Learnings (5)
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
app/Models/BackupJob.phptests/Feature/OrganizationSwitcherTest.phpapp/Http/Middleware/SetCurrentOrganization.phpapp/Services/CurrentOrganization.phpapp/Livewire/User/Index.phpapp/Policies/UserPolicy.phptests/Feature/User/CreateTest.phptests/Feature/Configuration/OrganizationTest.phpapp/Models/Snapshot.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/Forms/UserForm.phpapp/Models/User.phpapp/Livewire/User/Create.php
📚 Learning: 2026-02-13T11:05:37.072Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 105
File: tests/Feature/Services/Backup/Databases/SqliteDatabaseTest.php:8-11
Timestamp: 2026-02-13T11:05:37.072Z
Learning: Adopt a global afterEach hook in tests/Pest.php (or equivalent Pest bootstrap) to clean up temporary directories created during tests. Specifically handle temp dirs named with the prefixes sqlite-db-test-*, backup-task-test-*, restore-task-test-*, and volume-test-*, so individual test files don’t need their own cleanup logic. This applies to all PHP test files under the tests directory.
Applied to files:
tests/Feature/OrganizationSwitcherTest.phptests/Feature/User/CreateTest.phptests/Feature/Configuration/OrganizationTest.php
📚 Learning: 2026-04-09T13:59:25.873Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 205
File: tests/Feature/ConfigurationTest.php:165-176
Timestamp: 2026-04-09T13:59:25.873Z
Learning: In this repository, `Notification::fake()` is already performed globally in the base test class (`tests/TestCase.php`) within `setUp()` before each test. When reviewing individual test files under `tests/`, do not flag missing `Notification::fake()` calls, since they are handled by the shared base test setup.
Applied to files:
tests/Feature/OrganizationSwitcherTest.phptests/Feature/User/CreateTest.phptests/Feature/Configuration/OrganizationTest.php
📚 Learning: 2026-05-05T23:16:24.464Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 275
File: app/Services/CurrentOrganization.php:28-39
Timestamp: 2026-05-05T23:16:24.464Z
Learning: Enforce non-nullable return types for id(): string and model(): Organization in PHP service classes under app/Services. Ensure SetCurrentOrganization middleware always resolves an Organization in the web context before these methods are reached; do not reintroduce isResolved guards in callers. A null dereference due to a broken middleware chain is the intended failure signal, not silent null propagation. Document the contract in class docblocks and consider tests to verify the middleware guarantees.
Applied to files:
app/Services/CurrentOrganization.php
📚 Learning: 2026-02-18T09:45:52.485Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 116
File: app/Livewire/DatabaseServer/ConnectionStatus.php:18-18
Timestamp: 2026-02-18T09:45:52.485Z
Learning: In Livewire components, Eloquent model properties (e.g., public DatabaseServer $server) are automatically locked by the framework to prevent client-side ID tampering. The #[Locked] attribute is only needed for scalar properties (int, string, bool, etc.) that require protection from client-side mutation. Apply this guidance to all Livewire PHP components; use #[Locked] only on primitive properties that you want to shield from client manipulation, and rely on automatic locking for Eloquent model properties.
Applied to files:
app/Livewire/User/Index.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/Forms/UserForm.phpapp/Livewire/User/Create.php
🔇 Additional comments (2)
app/Policies/UserPolicy.php (1)
45-56: Super-admin guard onupdate()looks correct.Org admins can no longer edit super-admin users that happen to be members of their org, and the existing self/membership conditions are preserved. Matches the addressed feedback from prior reviews.
tests/Feature/Configuration/OrganizationTest.php (1)
12-102: LGTM — solid coverage of Organization management flows.Access control, validation, rename main/non-main, delete main/non-main/with-resources, and the empty-name validation paths are all exercised. The resource-scoping check at line 96 also confirms
Organization::hasResources()'s scope-bypass behavior referenced in the PR description.
…ols and invitation options
…pes and adding organization ID display
… invitation link management
…ulti-organization
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/views/layouts/app.blade.php (1)
62-62:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
"Super Admin"badge value is not wrapped in__()translation helper.The static string
value="Super Admin"bypasses the translation pipeline. Consistent with how all other user-facing strings are handled across the file, this should use the dynamic binding syntax.🔧 Proposed fix
- <x-badge value="Super Admin" icon="o-star" class="badge-warning whitespace-nowrap" /> + <x-badge :value="__('Super Admin')" icon="o-star" class="badge-warning whitespace-nowrap" />As per coding guidelines: "Translations should use
__('...')helper function" and "use:attrbinding (dynamic syntax) instead of{{ }}interpolation when passing translated strings to component attributes."🤖 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 `@resources/views/layouts/app.blade.php` at line 62, The "Super Admin" badge is using a static value attribute and not going through the translation helper; update the badge component usage (the element using value="Super Admin", e.g., the <x-badge> or similar component) to use the translation helper and dynamic binding by replacing value="Super Admin" with :value="__('Super Admin')" so the string is translatable and uses the :attr binding pattern consistent with other strings in this file.
🧹 Nitpick comments (8)
resources/views/livewire/user/create.blade.php (1)
60-74: 💤 Low valueDuplicate role label rendered on the group.
Both blocks render a manual
<label>("Role in current organization") immediately followed by<x-radio-card-group ... :label="__('Role')">, which causes the component to render its own internal "Role" label as well. Drop one of them so only a single, clear label is shown.♻️ Proposed fix
- <div> - <label class="label label-text font-semibold mb-2">{{ __('Role in current organization') }}</label> - <x-radio-card-group class="grid-cols-1 sm:grid-cols-3" :label="__('Role')"> + <div> + <x-radio-card-group class="grid-cols-1 sm:grid-cols-3" :label="__('Role in current organization')">Also applies to: 93-107
🤖 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 `@resources/views/livewire/user/create.blade.php` around lines 60 - 74, The UI renders two labels for the role field because a manual <label> element is placed immediately before the <x-radio-card-group> which also outputs its own label via the :label prop; remove one to avoid duplicate text—either delete the manual <label class="label label-text...">{{ __('Role in current organization') }}</label> or remove the :label="__('Role')" prop on the <x-radio-card-group> component (same change must be applied in the duplicate block near the bottom of the file that mirrors this role field).app/Livewire/User/Index.php (1)
147-161: 💤 Low valueConsolidate the org + role filter into one
whereHasand dropwhereRaw.As written, you produce two independent
EXISTSsubqueries againstorganization_user(one fromwhereRelation, another fromwhereHaswhenroleFilteris set), and the role-filter branch uses raw SQL where it isn't needed. Both filters target the same pivot relationship and can be expressed once with a parameterisedwhere:♻️ Proposed fix
- $query = User::query(); - - $query->whereRelation('organizations', 'organization_id', $currentOrg->id()); - - $users = $query - ->with('organizations') + $orgId = $currentOrg->id(); + + $users = User::query() + ->with('organizations') + ->whereHas('organizations', function ($q) use ($orgId) { + $q->where('organization_id', $orgId); + }) ->when($this->search, function ($query) { $query->where(function ($q) { $q->where('name', 'like', '%'.$this->search.'%') ->orWhere('email', 'like', '%'.$this->search.'%'); }); }) - ->when($this->roleFilter !== '', function ($query) use ($currentOrg) { - $query->whereHas('organizations', fn ($q) => $q->whereRaw('organization_id = ? and role = ?', [$currentOrg->id(), $this->roleFilter])); - }) + ->when($this->roleFilter !== '', function ($query) use ($orgId) { + $query->whereHas('organizations', fn ($q) => $q + ->where('organization_id', $orgId) + ->where('role', $this->roleFilter)); + })Note: the outer
whereHasand the role-scopedwhereHasstill produce twoEXISTSsubqueries. If you want a single one, you can keep only the role-scoped variant and make it the always-applied org filter, switching towhere('role', $this->roleFilter)only when set.🤖 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 `@app/Livewire/User/Index.php` around lines 147 - 161, Replace the separate whereRelation + conditional whereHas with a single whereHas on the 'organizations' relation: remove the call to whereRelation('organizations', ...) and instead call whereHas('organizations', function($q) use ($currentOrg) { $q->where('organization_id', $currentOrg->id()); if ($this->roleFilter !== '') { $q->where('role', $this->roleFilter); } }); — this drops the raw SQL, keeps the organization filter and only applies the role filter when set, preserving the existing with('organizations') and search logic.tests/Feature/User/CreateTest.php (1)
87-115: ⚡ Quick winTest relies on implicit factory side-effect (attachment to Main org).
Both tests assume
User::factory()->create()attaches the new user toOrganization::main()by default — line 92-94 deliberatelysyncs to a different org to remove that membership, while line 105-115 relies on it to assert "already a member". This implicit dependency on factory behaviour is fragile; if the default attachment ever changes, both tests will fail in non-obvious ways.Consider making the membership state explicit at the start of each test (e.g., explicitly attach/detach in arrange) to document the precondition.
🤖 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 `@tests/Feature/User/CreateTest.php` around lines 87 - 115, The tests for the Create Livewire component rely on User::factory()->create() implicitly attaching users to Organization::main(); make the membership explicit: in the "admin can add existing user" test ensure the created $existingUser is detached from Organization::main() (e.g., call $existingUser->organizations()->detach(Organization::main()->id) or sync to the other org) before calling Livewire::test(Create::class), and in the "rejects adding user already in organization" test explicitly attach the $existingUser to Organization::main() with the expected role (e.g., $existingUser->organizations()->attach(Organization::main()->id, ['role' => 'member'])) so the preconditions for Create::addExisting and the roleIn(Organization::main()) assertions are obvious and stable.tests/Feature/User/EditTest.php (1)
51-57: 💤 Low valuePrefer factory states over
['role' => $role]for consistency.Same pattern as
IndexTest::non-admin cannot access user index: the dataset values'member'/'viewer'correspond to existing factory states used elsewhere in this PR. Switching toUser::factory()->{$state}()->create()ensures the per-organization pivot role is set in addition to the legacy column.As per coding guidelines: "Check if the factory has custom states before manually setting up the model."
🤖 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 `@tests/Feature/User/EditTest.php` around lines 51 - 57, The test currently creates non-admin users with User::factory()->create(['role' => $role]) which bypasses factory states and misses per-organization pivot role; update the test 'non-admin cannot edit users' to use the factory states instead (e.g. User::factory()->member()->create() and User::factory()->viewer()->create()) so the per-organization pivot role is set correctly; keep the rest of the test (the actingAs($user) call and get(route('users.edit', $target))->assertForbidden()) unchanged and ensure the ->with([...]) data provider values match the state method names.resources/views/livewire/configuration/organization.blade.php (1)
12-12: 💤 Low valueAdd
rel="noopener noreferrer"to external_blanklink.External link with
target="_blank"should setrel="noopener noreferrer"to prevent the opened tab from accessingwindow.openerand to avoid leaking the referrer.♻️ Proposed fix
- <a href="https://david-crty.github.io/databasement/docs/user-guide/organizations" target="_blank" class="link link-primary">{{ __('Learn more') }}</a> + <a href="https://david-crty.github.io/databasement/docs/user-guide/organizations" target="_blank" rel="noopener noreferrer" class="link link-primary">{{ __('Learn more') }}</a>🤖 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 `@resources/views/livewire/configuration/organization.blade.php` at line 12, The external anchor tag rendering the "Learn more" link uses target="_blank" but is missing rel="noopener noreferrer"; update the <a> element in the organization Blade view (the anchor with class "link link-primary" and text "Learn more") to include rel="noopener noreferrer" so the new tab cannot access window.opener and the referrer is not leaked.app/Livewire/User/Create.php (1)
53-56: ⚡ Quick winKeep the existing-user rules on the component or form object.
addExisting()adds another inline rule array even though this component already usesUserFormfor the invite flow. Please moveexistingUserId/existingUserRoleto#[Validate]attributes or a form object so both modes follow the same Livewire pattern.As per coding guidelines: "In Livewire components, use
#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data."🤖 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 `@app/Livewire/User/Create.php` around lines 53 - 56, The inline validation for existingUserId and existingUserRole inside the Livewire component should be moved to the component's validation declarations instead of calling $this->validate() with an ad-hoc array in addExisting(); update the component to use #[Validate] attributes or include these fields on the existing UserForm so both invite and addExisting() flows reuse the same rules (reference existingUserId, existingUserRole, addExisting(), UserForm and #[Validate]) and remove the duplicate inline rule array so validation follows the Livewire form/attribute pattern.app/Livewire/Configuration/Organization.php (2)
76-76: ⚡ Quick winUse Livewire navigate redirects after these actions.
These plain redirects force a full page load. Prefer
$this->redirect(..., navigate: true)here so organization CRUD stays consistent with the app’s SPA-style navigation.↪️ Minimal change
- return $this->redirect(route('configuration.organizations')); + return $this->redirect(route('configuration.organizations'), navigate: true);As per coding guidelines: "In Livewire components, return
$this->redirect()withnavigate: truefor SPA-like navigation."Also applies to: 105-105, 129-129
🤖 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 `@app/Livewire/Configuration/Organization.php` at line 76, The redirect calls in the Livewire component Organization.php perform full page loads; replace each plain redirect call (the return $this->redirect(route('configuration.organizations')) instances at the three spots) with Livewire navigation-enabled redirects by calling $this->redirect(..., navigate: true) so the component uses SPA-style navigation; update the redirect invocations in the methods that currently return redirects (the Organization Livewire action methods around the redirect lines) accordingly.
65-67: ⚡ Quick winMove these organization-name rules into Livewire state.
Both actions define ad-hoc validation arrays inside the methods. Please keep
newOrgName/editOrgNameon#[Validate]attributes or a small form object so this component follows the repo’s Livewire validation pattern.As per coding guidelines: "In Livewire components, use
#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data."Also applies to: 96-98
🤖 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 `@app/Livewire/Configuration/Organization.php` around lines 65 - 67, The inline validation arrays for newOrgName (and similarly editOrgName) should be moved out of the method and defined as Livewire validation metadata — add #[Validate] attributes (or a small Form object) on the Organization component properties newOrgName and editOrgName with the rules 'required|string|max:255|unique:organizations,name' (and the corresponding edit rule), then remove the ad-hoc arrays from the methods (at lines calling $this->validate([...])) so the methods simply call $this->validate() to trigger the attribute-based validation; update any references in the create/edit handlers to rely on the validated properties.
🤖 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 `@app/Policies/UserPolicy.php`:
- Around line 74-75: The policy's check using $model->isSuperAdmin() and
User::where('super_admin', true)->count() is racy because it runs outside the
delete transaction; to fix, move the "last super admin" invariant into the
actual delete path (e.g., in the controller or User model deletion routine that
performs the delete), wrap the operation in a DB transaction and acquire a
pessimistic lock when counting (use the query with lockForUpdate()/SELECT ...
FOR UPDATE) to re-check the number of super admins before deleting, and
abort/throw an exception if the count would drop to zero; keep the policy for
quick UI hints but enforce correctness in the transactional delete code that
calls the model's delete method.
- Around line 92-99: The org-admin branch in UserPolicy currently authorizes any
pending user in the org, which lets an org admin act on a pending super-admin
account; update the return expression in the method (the block using
CurrentOrganization::class, isOrgAdmin(), and belongsToOrganization()) to also
require that the target $model is not a super-admin by adding a check like
!$model->isSuperAdmin() (or the equivalent role check on the User model) so org
admins cannot copy/claim super-admin invitation tokens.
In `@resources/views/layouts/app.blade.php`:
- Around line 38-39: Remove the unused resolution of $currentOrg: delete the
line that calls app(\App\Services\CurrentOrganization::class) since $currentOrg
is never used and SetCurrentOrganization middleware already initializes the
singleton; ensure no other logic depends on that resolved value and keep the
$showSwitcher line as-is (which uses $user->organizations()) to preserve
behavior.
In `@resources/views/livewire/configuration/organization.blade.php`:
- Around line 38-40: The Organization model's hasResources() currently issues
three exists() queries and should be refactored to use the preloaded withCount
attributes (database_servers_count, volumes_count, agents_count) to avoid the
N+1; update Organization::hasResources() to return true if any of
($this->database_servers_count ?? 0), ($this->volumes_count ?? 0), or
($this->agents_count ?? 0) are greater than zero so the Livewire component's
withCount(['databaseServers','volumes','agents']) is used instead of additional
exists() calls.
---
Outside diff comments:
In `@resources/views/layouts/app.blade.php`:
- Line 62: The "Super Admin" badge is using a static value attribute and not
going through the translation helper; update the badge component usage (the
element using value="Super Admin", e.g., the <x-badge> or similar component) to
use the translation helper and dynamic binding by replacing value="Super Admin"
with :value="__('Super Admin')" so the string is translatable and uses the :attr
binding pattern consistent with other strings in this file.
---
Nitpick comments:
In `@app/Livewire/Configuration/Organization.php`:
- Line 76: The redirect calls in the Livewire component Organization.php perform
full page loads; replace each plain redirect call (the return
$this->redirect(route('configuration.organizations')) instances at the three
spots) with Livewire navigation-enabled redirects by calling
$this->redirect(..., navigate: true) so the component uses SPA-style navigation;
update the redirect invocations in the methods that currently return redirects
(the Organization Livewire action methods around the redirect lines)
accordingly.
- Around line 65-67: The inline validation arrays for newOrgName (and similarly
editOrgName) should be moved out of the method and defined as Livewire
validation metadata — add #[Validate] attributes (or a small Form object) on the
Organization component properties newOrgName and editOrgName with the rules
'required|string|max:255|unique:organizations,name' (and the corresponding edit
rule), then remove the ad-hoc arrays from the methods (at lines calling
$this->validate([...])) so the methods simply call $this->validate() to trigger
the attribute-based validation; update any references in the create/edit
handlers to rely on the validated properties.
In `@app/Livewire/User/Create.php`:
- Around line 53-56: The inline validation for existingUserId and
existingUserRole inside the Livewire component should be moved to the
component's validation declarations instead of calling $this->validate() with an
ad-hoc array in addExisting(); update the component to use #[Validate]
attributes or include these fields on the existing UserForm so both invite and
addExisting() flows reuse the same rules (reference existingUserId,
existingUserRole, addExisting(), UserForm and #[Validate]) and remove the
duplicate inline rule array so validation follows the Livewire form/attribute
pattern.
In `@app/Livewire/User/Index.php`:
- Around line 147-161: Replace the separate whereRelation + conditional whereHas
with a single whereHas on the 'organizations' relation: remove the call to
whereRelation('organizations', ...) and instead call whereHas('organizations',
function($q) use ($currentOrg) { $q->where('organization_id',
$currentOrg->id()); if ($this->roleFilter !== '') { $q->where('role',
$this->roleFilter); } }); — this drops the raw SQL, keeps the organization
filter and only applies the role filter when set, preserving the existing
with('organizations') and search logic.
In `@resources/views/livewire/configuration/organization.blade.php`:
- Line 12: The external anchor tag rendering the "Learn more" link uses
target="_blank" but is missing rel="noopener noreferrer"; update the <a> element
in the organization Blade view (the anchor with class "link link-primary" and
text "Learn more") to include rel="noopener noreferrer" so the new tab cannot
access window.opener and the referrer is not leaked.
In `@resources/views/livewire/user/create.blade.php`:
- Around line 60-74: The UI renders two labels for the role field because a
manual <label> element is placed immediately before the <x-radio-card-group>
which also outputs its own label via the :label prop; remove one to avoid
duplicate text—either delete the manual <label class="label label-text...">{{
__('Role in current organization') }}</label> or remove the :label="__('Role')"
prop on the <x-radio-card-group> component (same change must be applied in the
duplicate block near the bottom of the file that mirrors this role field).
In `@tests/Feature/User/CreateTest.php`:
- Around line 87-115: The tests for the Create Livewire component rely on
User::factory()->create() implicitly attaching users to Organization::main();
make the membership explicit: in the "admin can add existing user" test ensure
the created $existingUser is detached from Organization::main() (e.g., call
$existingUser->organizations()->detach(Organization::main()->id) or sync to the
other org) before calling Livewire::test(Create::class), and in the "rejects
adding user already in organization" test explicitly attach the $existingUser to
Organization::main() with the expected role (e.g.,
$existingUser->organizations()->attach(Organization::main()->id, ['role' =>
'member'])) so the preconditions for Create::addExisting and the
roleIn(Organization::main()) assertions are obvious and stable.
In `@tests/Feature/User/EditTest.php`:
- Around line 51-57: The test currently creates non-admin users with
User::factory()->create(['role' => $role]) which bypasses factory states and
misses per-organization pivot role; update the test 'non-admin cannot edit
users' to use the factory states instead (e.g.
User::factory()->member()->create() and User::factory()->viewer()->create()) so
the per-organization pivot role is set correctly; keep the rest of the test (the
actingAs($user) call and get(route('users.edit', $target))->assertForbidden())
unchanged and ensure the ->with([...]) data provider values match the state
method names.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db754d77-afa1-4ed2-9f69-c7041244871c
📒 Files selected for processing (23)
app/Http/Middleware/SetCurrentOrganization.phpapp/Livewire/Configuration/Organization.phpapp/Livewire/User/Create.phpapp/Livewire/User/Index.phpapp/Models/User.phpapp/Policies/UserPolicy.phpapp/Providers/AppServiceProvider.phpapp/Services/CurrentOrganization.phpdocs/docs/user-guide/organizations.mdresources/views/layouts/app.blade.phpresources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/create.blade.phpresources/views/livewire/user/index.blade.phptests/Feature/Api/BackupApiTest.phptests/Feature/Api/BackupScheduleCrudApiTest.phptests/Feature/Api/DatabaseServerCrudApiTest.phptests/Feature/Api/OrganizationContextApiTest.phptests/Feature/Api/RestoreApiTest.phptests/Feature/Api/VolumeCrudApiTest.phptests/Feature/Configuration/OrganizationTest.phptests/Feature/User/CreateTest.phptests/Feature/User/EditTest.phptests/Feature/User/IndexTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Feature/Configuration/OrganizationTest.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci
🧰 Additional context used
📓 Path-based instructions (6)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
tests/Feature/Api/BackupScheduleCrudApiTest.phptests/Feature/Api/BackupApiTest.phpapp/Http/Middleware/SetCurrentOrganization.phptests/Feature/Api/VolumeCrudApiTest.phpresources/views/livewire/configuration/organization.blade.phptests/Feature/Api/DatabaseServerCrudApiTest.phptests/Feature/User/EditTest.phpapp/Services/CurrentOrganization.phptests/Feature/Api/RestoreApiTest.phpresources/views/livewire/user/index.blade.phptests/Feature/User/IndexTest.phpapp/Providers/AppServiceProvider.phpapp/Livewire/User/Index.phpresources/views/layouts/app.blade.phptests/Feature/Api/OrganizationContextApiTest.phpresources/views/livewire/user/create.blade.phpapp/Livewire/User/Create.phpapp/Policies/UserPolicy.phpapp/Models/User.phpapp/Livewire/Configuration/Organization.phptests/Feature/User/CreateTest.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
tests/Feature/Api/BackupScheduleCrudApiTest.phptests/Feature/Api/BackupApiTest.phpapp/Http/Middleware/SetCurrentOrganization.phptests/Feature/Api/VolumeCrudApiTest.phpresources/views/livewire/configuration/organization.blade.phptests/Feature/Api/DatabaseServerCrudApiTest.phptests/Feature/User/EditTest.phpapp/Services/CurrentOrganization.phptests/Feature/Api/RestoreApiTest.phpresources/views/livewire/user/index.blade.phptests/Feature/User/IndexTest.phpapp/Providers/AppServiceProvider.phpapp/Livewire/User/Index.phpresources/views/layouts/app.blade.phptests/Feature/Api/OrganizationContextApiTest.phpresources/views/livewire/user/create.blade.phpapp/Livewire/User/Create.phpapp/Policies/UserPolicy.phpapp/Models/User.phpapp/Livewire/Configuration/Organization.phptests/Feature/User/CreateTest.php
tests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.php: When creating models for tests, use factories. Check if the factory has custom states before manually setting up the model.
Use faker methods such as$this->faker->word()orfake()->randomDigit()when generating test data. Follow existing conventions on whether to use$this->fakerorfake().
Create Pest tests usingphp artisan make:test --pest {name}without including the test suite directory. Usephp artisan make:test --pest SomeFeatureTestinstead ofphp artisan make:test --pest Feature/SomeFeatureTest.
Files:
tests/Feature/Api/BackupScheduleCrudApiTest.phptests/Feature/Api/BackupApiTest.phptests/Feature/Api/VolumeCrudApiTest.phptests/Feature/Api/DatabaseServerCrudApiTest.phptests/Feature/User/EditTest.phptests/Feature/Api/RestoreApiTest.phptests/Feature/User/IndexTest.phptests/Feature/Api/OrganizationContextApiTest.phptests/Feature/User/CreateTest.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
All Livewire component blade files should contain only view markup; all PHP logic must be in the component class.
Files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/index.blade.phpresources/views/livewire/user/create.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/**/*.blade.php: All UI components in Mary UI should be prefixed withx-(e.g.,<x-button>,<x-input>,<x-card>) and use Heroicons for icons (e.g.,icon="o-user"for outline,icon="s-user"for solid).
In Mary UI select components, use the:optionsprop with array format[['id' => 'value', 'name' => 'Label']].
In Mary UI alerts, useclass="alert-success",class="alert-error", etc. (notvariantprop).
Use Alpine.js for client-side interactions in Livewire instead of JavaScript frameworks.
In Blade templates, use:attrbinding (dynamic syntax) instead of{{ }}interpolation when passing translated strings to component attributes to avoid double-encoding special characters.
Files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/index.blade.phpresources/views/layouts/app.blade.phpresources/views/livewire/user/create.blade.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Livewire/**/*.php: In Livewire components, use#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data.
In Livewire components, useSession::flash()for one-time messages and show them via@if (session('success'))in Blade templates.
In Livewire components, return$this->redirect()withnavigate: truefor SPA-like navigation.
In Mary UI modals, add a boolean property to the component class and usewire:modelin the Blade template.
Keep state server-side in Livewire components so the UI reflects it. Validate and authorize in actions as you would in HTTP requests.
Files:
app/Livewire/User/Index.phpapp/Livewire/User/Create.phpapp/Livewire/Configuration/Organization.php
🧠 Learnings (9)
📚 Learning: 2026-02-13T11:05:37.072Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 105
File: tests/Feature/Services/Backup/Databases/SqliteDatabaseTest.php:8-11
Timestamp: 2026-02-13T11:05:37.072Z
Learning: Adopt a global afterEach hook in tests/Pest.php (or equivalent Pest bootstrap) to clean up temporary directories created during tests. Specifically handle temp dirs named with the prefixes sqlite-db-test-*, backup-task-test-*, restore-task-test-*, and volume-test-*, so individual test files don’t need their own cleanup logic. This applies to all PHP test files under the tests directory.
Applied to files:
tests/Feature/Api/BackupScheduleCrudApiTest.phptests/Feature/Api/BackupApiTest.phptests/Feature/Api/VolumeCrudApiTest.phptests/Feature/Api/DatabaseServerCrudApiTest.phptests/Feature/User/EditTest.phptests/Feature/Api/RestoreApiTest.phptests/Feature/User/IndexTest.phptests/Feature/Api/OrganizationContextApiTest.phptests/Feature/User/CreateTest.php
📚 Learning: 2026-04-09T13:59:25.873Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 205
File: tests/Feature/ConfigurationTest.php:165-176
Timestamp: 2026-04-09T13:59:25.873Z
Learning: In this repository, `Notification::fake()` is already performed globally in the base test class (`tests/TestCase.php`) within `setUp()` before each test. When reviewing individual test files under `tests/`, do not flag missing `Notification::fake()` calls, since they are handled by the shared base test setup.
Applied to files:
tests/Feature/Api/BackupScheduleCrudApiTest.phptests/Feature/Api/BackupApiTest.phptests/Feature/Api/VolumeCrudApiTest.phptests/Feature/Api/DatabaseServerCrudApiTest.phptests/Feature/User/EditTest.phptests/Feature/Api/RestoreApiTest.phptests/Feature/User/IndexTest.phptests/Feature/Api/OrganizationContextApiTest.phptests/Feature/User/CreateTest.php
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
tests/Feature/Api/BackupScheduleCrudApiTest.phptests/Feature/Api/BackupApiTest.phpapp/Http/Middleware/SetCurrentOrganization.phptests/Feature/Api/VolumeCrudApiTest.phpresources/views/livewire/configuration/organization.blade.phptests/Feature/Api/DatabaseServerCrudApiTest.phptests/Feature/User/EditTest.phpapp/Services/CurrentOrganization.phptests/Feature/Api/RestoreApiTest.phpresources/views/livewire/user/index.blade.phptests/Feature/User/IndexTest.phpapp/Providers/AppServiceProvider.phpapp/Livewire/User/Index.phpresources/views/layouts/app.blade.phptests/Feature/Api/OrganizationContextApiTest.phpresources/views/livewire/user/create.blade.phpapp/Livewire/User/Create.phpapp/Policies/UserPolicy.phpapp/Models/User.phpapp/Livewire/Configuration/Organization.phptests/Feature/User/CreateTest.php
📚 Learning: 2026-05-06T10:46:47.009Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 275
File: app/Models/User.php:188-216
Timestamp: 2026-05-06T10:46:47.009Z
Learning: In app/Models/User.php, the $cachedRoles cache is intentionally request-scoped to prevent N+1 queries during rendering. Pivot writes (attach() in app/Livewire/User/Create.php, updateExistingPivot() in app/Livewire/Forms/UserForm.php) are terminal within a request, and the cache is not read after a write in the same request. Do not flag missing cache invalidation after pivot writes as a bug; this design is intentional. When reviewing related changes in PHP files under app/, assume this cache pattern and do not raise invalidation issues for these flows.
Applied to files:
app/Http/Middleware/SetCurrentOrganization.phpapp/Services/CurrentOrganization.phpapp/Providers/AppServiceProvider.phpapp/Livewire/User/Index.phpapp/Livewire/User/Create.phpapp/Policies/UserPolicy.phpapp/Models/User.phpapp/Livewire/Configuration/Organization.php
📚 Learning: 2026-01-30T22:27:46.107Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 61
File: resources/views/livewire/volume/connectors/s3-config.blade.php:1-13
Timestamp: 2026-01-30T22:27:46.107Z
Learning: In Blade template files (any .blade.php) within the databasement project, allow using alert-info for informational content inside <x-alert> components. The guideline that permits alert-success and alert-error does not exclude using alert-info for informational purposes. Apply this consistently to all Blade components that render alerts; ensure semantic usage and accessibility.
Applied to files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/index.blade.phpresources/views/layouts/app.blade.phpresources/views/livewire/user/create.blade.php
📚 Learning: 2026-02-06T10:34:43.585Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 75
File: resources/views/livewire/backup-job/_filters.blade.php:36-40
Timestamp: 2026-02-06T10:34:43.585Z
Learning: In Blade template files, when creating compact inline filter controls, prefer using native <input type="checkbox"> elements with daisyUI classes (e.g., checkbox checkbox-warning checkbox-xs) over the Mary UI <x-checkbox> component. The <x-checkbox> component adds wrapper markup (e.g., <div><fieldset><label> with gap-3) that can break tight inline flex layouts. Use the native input approach for compact inline controls, but reserve <x-checkbox> for form fields that require labels, hints, and errors.
Applied to files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/index.blade.phpresources/views/layouts/app.blade.phpresources/views/livewire/user/create.blade.php
📚 Learning: 2026-05-05T23:16:24.464Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 275
File: app/Services/CurrentOrganization.php:28-39
Timestamp: 2026-05-05T23:16:24.464Z
Learning: Enforce non-nullable return types for id(): string and model(): Organization in PHP service classes under app/Services. Ensure SetCurrentOrganization middleware always resolves an Organization in the web context before these methods are reached; do not reintroduce isResolved guards in callers. A null dereference due to a broken middleware chain is the intended failure signal, not silent null propagation. Document the contract in class docblocks and consider tests to verify the middleware guarantees.
Applied to files:
app/Services/CurrentOrganization.php
📚 Learning: 2026-02-18T09:45:52.485Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 116
File: app/Livewire/DatabaseServer/ConnectionStatus.php:18-18
Timestamp: 2026-02-18T09:45:52.485Z
Learning: In Livewire components, Eloquent model properties (e.g., public DatabaseServer $server) are automatically locked by the framework to prevent client-side ID tampering. The #[Locked] attribute is only needed for scalar properties (int, string, bool, etc.) that require protection from client-side mutation. Apply this guidance to all Livewire PHP components; use #[Locked] only on primitive properties that you want to shield from client manipulation, and rely on automatic locking for Eloquent model properties.
Applied to files:
app/Livewire/User/Index.phpapp/Livewire/User/Create.phpapp/Livewire/Configuration/Organization.php
📚 Learning: 2026-03-28T22:33:16.333Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 195
File: app/Notifications/CLAUDE.md:7-12
Timestamp: 2026-03-28T22:33:16.333Z
Learning: In this repository (David-Crty/databasement), do not flag fenced Markdown code blocks that omit a language identifier (i.e., MD040: missing language after ```). The project treats unlabeled fenced code blocks as plain text by default, so missing language tags are acceptable for Markdown files.
Applied to files:
docs/docs/user-guide/organizations.md
🪛 markdownlint-cli2 (0.22.1)
docs/docs/user-guide/organizations.md
[warning] 65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (14)
docs/docs/user-guide/organizations.md (1)
1-109: Comprehensive and consistent organizations guide — looks good to merge.The page is clear, well-structured, and covers the key multi-organization behaviors and constraints (roles, scoping, switching, API usage, and isolation) in a way that matches the feature scope.
resources/views/livewire/user/index.blade.php (1)
49-67: PHP logic in Livewire blade and N+1 query concern.The
@phpblock (service resolution, role derivation, badge class computation) and the per-rowroleIn()DB call were both flagged in previous review rounds. Marking as duplicate to keep tracking.tests/Feature/Api/OrganizationContextApiTest.php (1)
7-130: Comprehensive org-context API test coverage — LGTM.The suite covers the full spectrum: default org scoping, explicit
org_idoverride,X-Organization-Idheader parity, cross-org isolation, 403 on inaccessible/nonexistent org ID, super-admin bypass, and multi-org switching. The 403 (not 404) for nonexistentorg_id(line 49-55) is correct from a security standpoint.tests/Feature/Api/BackupApiTest.php (1)
17-17: Factory state standardization — LGTM.Replacing the direct role attribute with
->viewer()factory state is the correct approach and consistent with the rest of the test suite changes in this PR.tests/Feature/Api/RestoreApiTest.php (1)
18-18: Factory state standardization — LGTM.tests/Feature/Api/BackupScheduleCrudApiTest.php (1)
57-57: Factory state standardization across all three mutation tests — LGTM.Also applies to: 127-127, 186-186
app/Providers/AppServiceProvider.php (1)
37-37: Singleton binding and Scramble org_id injection — LGTM.
singleton(CurrentOrganization::class)is correct for the standard PHP-FPM lifecycle; thewithDocumentTransformerspattern for injectingorg_idinto all operations is idiomatic for Scramble 0.12+.Also applies to: 104-112
tests/Feature/Api/VolumeCrudApiTest.php (1)
75-75: Factory state standardization and Mockery-based filesystem isolation — LGTM.Both the
viewer()factory state adoption and theMockery::mock(FilesystemProvider::class)approach for connection tests are clean improvements over the previous approach.Also applies to: 136-136
resources/views/livewire/user/create.blade.php (1)
51-57: PHP authorization logic still in template.
auth()->user()->isSuperAdmin()is invoked directly in the Blade. Per the coding guideline that Livewire blade files contain only view markup, this should be exposed as a public component property (e.g.,$isSuperAdminset inmount()) and referenced as@if($isSuperAdmin).As per coding guidelines: "All Livewire component blade files should contain only view markup; all PHP logic must be in the component class."
tests/Feature/User/CreateTest.php (1)
29-34: 💤 Low valuePrefer factory states over
['role' => $role].Same observation as in
IndexTest/EditTest: use the existingmember()/viewer()factory states so both the legacy column and the per-organization pivot role are set.As per coding guidelines: "Check if the factory has custom states before manually setting up the model."
tests/Feature/Api/DatabaseServerCrudApiTest.php (1)
17-17: LGTM — viewer factory state usage.Switching to
User::factory()->viewer()->create()matches the PR's per-organization role model and follows the project convention of using factory states.Also applies to: 242-242, 370-370
app/Http/Middleware/SetCurrentOrganization.php (1)
44-51: ⚡ Quick winThe strict comparison is safe.
CurrentOrganization::id()has an explicit return type ofstring, andOrganizationusesHasUlids(generating string-based ULIDs). Both values being compared on line 49 are strings, so the strict equality check is appropriate.> Likely an incorrect or invalid review comment.tests/Feature/User/IndexTest.php (1)
29-35: ⚡ Quick winThe proposed factory state approach won't work—there is no
member()state method.The UserFactory contains state methods for
viewer(),admin(),superAdmin(), anddemo(), but notmember(). The current test code usingUser::factory()->create(['role' => $role])is actually correct by design: the factory'snewModel()method intercepts the virtual 'role' attribute and stages it viapendingPivotRoleso that theconfigure()hook can attach the user to the organization with the intended role. The default definition already providesrole=>User::ROLE_MEMBER, so passing['role' => $role]as an attribute is the intended pattern for setting per-organization roles.If you want to use factory states, the
viewer()state exists and can be used for viewers, but you'd need either a newmember()state method added to UserFactory (out of scope here) or conditional test logic.> Likely an incorrect or invalid review comment.app/Models/User.php (1)
108-115: Thesuper_adminfield is safe to keep in$fillablefor this codebase.All user write paths properly gate the
super_adminfield behind application-level authorization checks:
- CreateNewUser (first registration): hardcodes
super_admin => true- UserForm (admin user management): conditions
super_adminassignment onauth()->user()->isSuperAdmin()check- AcceptInvitation (invitation flow): excludes
super_adminfrom the update- Profile settings: validates only
nameandsuper_adminNo API endpoints, request validation, or form request classes accept
super_adminfrom user input. The concern in the original review is mitigated by proper application-level controls, though removing the field from$fillableand usingforceFill()explicitly where needed would reduce long-term maintenance risk.
…nd authorization logic
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
resources/views/livewire/user/create.blade.php (1)
51-57: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMove the super-admin authorization check into the Livewire component
Line 51 calls
auth()->user()->isSuperAdmin()directly in the Blade view. Expose a component property (e.g.,$isSuperAdmin) and render with@if($isSuperAdmin)so the template stays presentation-only.♻️ Proposed fix
- `@if`(auth()->user()->isSuperAdmin()) + `@if`($isSuperAdmin) <x-checkbox wire:model="form.superAdmin" :label="__('Super Admin')" :hint="__('Super admins can access all organizations and manage global settings.')" /> `@endif`// app/Livewire/User/Create.php public bool $isSuperAdmin = false; public function mount(): void { $this->isSuperAdmin = auth()->user()->isSuperAdmin(); }As per coding guidelines: "All Livewire component blade files should contain only view markup; all PHP logic must be in the component class."
🤖 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 `@resources/views/livewire/user/create.blade.php` around lines 51 - 57, The Blade view is calling auth()->user()->isSuperAdmin() directly; move that logic into the Livewire component by adding a public bool property $isSuperAdmin on the User\Create component (e.g., class Create) and set it in mount(): $this->isSuperAdmin = auth()->user()->isSuperAdmin(); then update the view to use `@if`($isSuperAdmin) instead of auth()->user()->isSuperAdmin() so the template remains presentation-only.
🤖 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 `@app/Livewire/Configuration/Organization.php`:
- Line 76: The redirect calls in createOrganization(), updateOrganization(), and
deleteOrganization() use $this->redirect(route(...)) which forces a full page
reload; change each to return
$this->redirect(route('configuration.organizations'), ['navigate' => true]) so
Livewire performs an SPA-style navigate transition. Alternatively, instead of
redirecting, reset component state and invalidate the organizations cache by
calling unset($this->organizations) (or $this->reset(...) as appropriate) after
the mutation to avoid any redirect; update the three methods
(createOrganization, updateOrganization, deleteOrganization) accordingly.
---
Duplicate comments:
In `@resources/views/livewire/user/create.blade.php`:
- Around line 51-57: The Blade view is calling auth()->user()->isSuperAdmin()
directly; move that logic into the Livewire component by adding a public bool
property $isSuperAdmin on the User\Create component (e.g., class Create) and set
it in mount(): $this->isSuperAdmin = auth()->user()->isSuperAdmin(); then update
the view to use `@if`($isSuperAdmin) instead of auth()->user()->isSuperAdmin() so
the template remains presentation-only.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0665648-3092-4528-bae7-14eb6d57da75
📒 Files selected for processing (6)
app/Livewire/Configuration/Organization.phpapp/Models/Organization.phpapp/Policies/UserPolicy.phpresources/views/layouts/app.blade.phpresources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/create.blade.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci
🧰 Additional context used
📓 Path-based instructions (5)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/create.blade.phpresources/views/layouts/app.blade.phpapp/Models/Organization.phpapp/Policies/UserPolicy.phpapp/Livewire/Configuration/Organization.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
All Livewire component blade files should contain only view markup; all PHP logic must be in the component class.
Files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/create.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/**/*.blade.php: All UI components in Mary UI should be prefixed withx-(e.g.,<x-button>,<x-input>,<x-card>) and use Heroicons for icons (e.g.,icon="o-user"for outline,icon="s-user"for solid).
In Mary UI select components, use the:optionsprop with array format[['id' => 'value', 'name' => 'Label']].
In Mary UI alerts, useclass="alert-success",class="alert-error", etc. (notvariantprop).
Use Alpine.js for client-side interactions in Livewire instead of JavaScript frameworks.
In Blade templates, use:attrbinding (dynamic syntax) instead of{{ }}interpolation when passing translated strings to component attributes to avoid double-encoding special characters.
Files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/create.blade.phpresources/views/layouts/app.blade.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/create.blade.phpresources/views/layouts/app.blade.phpapp/Models/Organization.phpapp/Policies/UserPolicy.phpapp/Livewire/Configuration/Organization.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Livewire/**/*.php: In Livewire components, use#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data.
In Livewire components, useSession::flash()for one-time messages and show them via@if (session('success'))in Blade templates.
In Livewire components, return$this->redirect()withnavigate: truefor SPA-like navigation.
In Mary UI modals, add a boolean property to the component class and usewire:modelin the Blade template.
Keep state server-side in Livewire components so the UI reflects it. Validate and authorize in actions as you would in HTTP requests.
Files:
app/Livewire/Configuration/Organization.php
🧠 Learnings (5)
📚 Learning: 2026-01-30T22:27:46.107Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 61
File: resources/views/livewire/volume/connectors/s3-config.blade.php:1-13
Timestamp: 2026-01-30T22:27:46.107Z
Learning: In Blade template files (any .blade.php) within the databasement project, allow using alert-info for informational content inside <x-alert> components. The guideline that permits alert-success and alert-error does not exclude using alert-info for informational purposes. Apply this consistently to all Blade components that render alerts; ensure semantic usage and accessibility.
Applied to files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/create.blade.phpresources/views/layouts/app.blade.php
📚 Learning: 2026-02-06T10:34:43.585Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 75
File: resources/views/livewire/backup-job/_filters.blade.php:36-40
Timestamp: 2026-02-06T10:34:43.585Z
Learning: In Blade template files, when creating compact inline filter controls, prefer using native <input type="checkbox"> elements with daisyUI classes (e.g., checkbox checkbox-warning checkbox-xs) over the Mary UI <x-checkbox> component. The <x-checkbox> component adds wrapper markup (e.g., <div><fieldset><label> with gap-3) that can break tight inline flex layouts. Use the native input approach for compact inline controls, but reserve <x-checkbox> for form fields that require labels, hints, and errors.
Applied to files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/create.blade.phpresources/views/layouts/app.blade.php
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
resources/views/livewire/configuration/organization.blade.phpresources/views/livewire/user/create.blade.phpresources/views/layouts/app.blade.phpapp/Models/Organization.phpapp/Policies/UserPolicy.phpapp/Livewire/Configuration/Organization.php
📚 Learning: 2026-05-06T10:46:47.009Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 275
File: app/Models/User.php:188-216
Timestamp: 2026-05-06T10:46:47.009Z
Learning: In app/Models/User.php, the $cachedRoles cache is intentionally request-scoped to prevent N+1 queries during rendering. Pivot writes (attach() in app/Livewire/User/Create.php, updateExistingPivot() in app/Livewire/Forms/UserForm.php) are terminal within a request, and the cache is not read after a write in the same request. Do not flag missing cache invalidation after pivot writes as a bug; this design is intentional. When reviewing related changes in PHP files under app/, assume this cache pattern and do not raise invalidation issues for these flows.
Applied to files:
app/Models/Organization.phpapp/Policies/UserPolicy.phpapp/Livewire/Configuration/Organization.php
📚 Learning: 2026-02-18T09:45:52.485Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 116
File: app/Livewire/DatabaseServer/ConnectionStatus.php:18-18
Timestamp: 2026-02-18T09:45:52.485Z
Learning: In Livewire components, Eloquent model properties (e.g., public DatabaseServer $server) are automatically locked by the framework to prevent client-side ID tampering. The #[Locked] attribute is only needed for scalar properties (int, string, bool, etc.) that require protection from client-side mutation. Apply this guidance to all Livewire PHP components; use #[Locked] only on primitive properties that you want to shield from client manipulation, and rely on automatic locking for Eloquent model properties.
Applied to files:
app/Livewire/Configuration/Organization.php
🔇 Additional comments (7)
resources/views/livewire/user/create.blade.php (1)
82-89:x-selectoptions wiring looks correctThe
:options="$availableUsers"binding matches the expected Mary UI options contract (id/name) and keeps option-building logic in the component.app/Models/Organization.php (1)
37-113: LGTM!The model is well typed, generics on relations are accurate, and
hasResources()correctly relies on preloadedwithCountattributes — all current callers (organizations(),confirmDelete(),deleteOrganization()) preload them withwithoutGlobalScope(OrganizationScope::class), so cross-org counts are accurate.resources/views/layouts/app.blade.php (2)
35-45: LGTM — past dead-code concern resolved.The previously flagged unused
$currentOrgresolution has been removed;$showSwitchershort-circuits onisSuperAdmin()so the count query only runs for regular multi-org users.
54-56: LGTM — Users menu correctly gated by policy.
@can('viewAny', \App\Models\User::class)properly defers toUserPolicy::viewAny, which now restricts access to admins (super or org). This keeps the sidebar consistent with the org-scoped permission model.app/Policies/UserPolicy.php (1)
8-117: LGTM — all previously flagged escalation paths are closed.
update()andcopyInvitationLink()correctly add the! $model->isSuperAdmin()guard on the org‑admin branch, thedelete()path is restricted to super admins with a last‑super‑admin safety check, andmanageOrgMembership()consistently applies the super‑admin OR org‑admin pattern viaCurrentOrganization. Authorization model is internally consistent.app/Livewire/Configuration/Organization.php (1)
44-53: LGTM — withCount/withoutGlobalScope correctly applied.The past issue with
OrganizationScopezeroing out cross-org counts is resolved by wrapping each scoped relation in a closure that strips the global scope, while leavingusers(unscoped) untouched. The same pattern is consistently applied inconfirmDelete()anddeleteOrganization().resources/views/livewire/configuration/organization.blade.php (1)
1-73: LGTM — guideline-compliant Mary UI markup.
x-prefixed components, Heroicons,__()translations,:attrbindings for translated strings, andwire:modelon modal booleans are all in line with the project guidelines. ThehasResources()/is_maingating in the actions cell mirrorsOrganizationPolicyconstraints, so the UI won't expose disabled actions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/Livewire/Configuration/Organization.php (1)
23-29: ⚡ Quick winMove the shared field rules onto the Livewire properties.
These inline rule arrays work, but they bypass the repo’s Livewire validation pattern and duplicate the non-unique parts of the rules. Put the shared
required|string|max:255checks on$newOrgName/$editOrgNamewith#[Validate](or a small Form object), then keep only the action-specificuniquerule inline.♻️ Minimal refactor
use Livewire\Attributes\Computed; use Livewire\Attributes\Title; +use Livewire\Attributes\Validate; use Livewire\Component; @@ - public string $newOrgName = ''; + #[Validate('required|string|max:255')] + public string $newOrgName = ''; @@ - public string $editOrgName = ''; + #[Validate('required|string|max:255')] + public string $editOrgName = ''; @@ $this->validate([ - 'newOrgName' => 'required|string|max:255|unique:organizations,name', + 'newOrgName' => 'unique:organizations,name', ]); @@ $this->validate([ - 'editOrgName' => 'required|string|max:255|unique:organizations,name,'.$org->id, + 'editOrgName' => 'unique:organizations,name,'.$org->id, ]);As per coding guidelines: "In Livewire components, use
#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data."Also applies to: 67-69, 100-102
🤖 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 `@app/Livewire/Configuration/Organization.php` around lines 23 - 29, Move the common "required|string|max:255" validation onto the Livewire properties by adding the #[Validate] attribute for $newOrgName and $editOrgName (or replace with a small Form object) so those checks are centralized; then remove those parts from the inline rule arrays in the create/update actions and leave only the action-specific "unique" constraint inline (e.g., in the methods that currently reference editingOrgId or create org); finally ensure each action calls $this->validate() before processing so validation runs.
🤖 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 `@app/Livewire/Configuration/Organization.php`:
- Around line 57-61: openCreateModal() and openEditModal() currently clear
fields but leave Livewire's validation error bag, causing stale validation
messages to appear when reopening the modal; call Livewire's resetValidation()
in both methods (in Organization::openCreateModal and
Organization::openEditModal) to clear the error bag when opening the modal so
the form starts clean.
---
Nitpick comments:
In `@app/Livewire/Configuration/Organization.php`:
- Around line 23-29: Move the common "required|string|max:255" validation onto
the Livewire properties by adding the #[Validate] attribute for $newOrgName and
$editOrgName (or replace with a small Form object) so those checks are
centralized; then remove those parts from the inline rule arrays in the
create/update actions and leave only the action-specific "unique" constraint
inline (e.g., in the methods that currently reference editingOrgId or create
org); finally ensure each action calls $this->validate() before processing so
validation runs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80eff8aa-1d40-455d-8392-3e2258449346
📒 Files selected for processing (2)
app/Livewire/Configuration/Organization.phpresources/views/livewire/configuration/organization.blade.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci
🧰 Additional context used
📓 Path-based instructions (5)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: In PHP files, always use curly braces for control structures, even for single-line bodies.
Use PHP 8 constructor property promotion:public function __construct(public GitHub $github) { }. Do not leave empty zero-parameter__construct()methods unless the constructor is private.
Use explicit return type declarations and type hints for all method parameters in PHP:function isAccessible(User $user, ?string $path = null): bool
Prefer PHPDoc blocks over inline comments. Only add inline comments for exceptionally complex logic.
Use array shape type definitions in PHPDoc blocks for PHP.
Files:
app/Livewire/Configuration/Organization.phpresources/views/livewire/configuration/organization.blade.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Livewire/**/*.php: In Livewire components, use#[Validate]attributes or Form objects for validation. Call$this->validate()before processing data.
In Livewire components, useSession::flash()for one-time messages and show them via@if (session('success'))in Blade templates.
In Livewire components, return$this->redirect()withnavigate: truefor SPA-like navigation.
In Mary UI modals, add a boolean property to the component class and usewire:modelin the Blade template.
Keep state server-side in Livewire components so the UI reflects it. Validate and authorize in actions as you would in HTTP requests.
Files:
app/Livewire/Configuration/Organization.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{php,blade.php}: Always use named routes and theroute()function when generating links to other pages.
Translations should use__('...')helper function and be stored in JSON translation files inlang/{locale}.json. Keep technical terms like 'Backup', 'Restore', 'Snapshot(s)' in English across all locales.
Files:
app/Livewire/Configuration/Organization.phpresources/views/livewire/configuration/organization.blade.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
All Livewire component blade files should contain only view markup; all PHP logic must be in the component class.
Files:
resources/views/livewire/configuration/organization.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/**/*.blade.php: All UI components in Mary UI should be prefixed withx-(e.g.,<x-button>,<x-input>,<x-card>) and use Heroicons for icons (e.g.,icon="o-user"for outline,icon="s-user"for solid).
In Mary UI select components, use the:optionsprop with array format[['id' => 'value', 'name' => 'Label']].
In Mary UI alerts, useclass="alert-success",class="alert-error", etc. (notvariantprop).
Use Alpine.js for client-side interactions in Livewire instead of JavaScript frameworks.
In Blade templates, use:attrbinding (dynamic syntax) instead of{{ }}interpolation when passing translated strings to component attributes to avoid double-encoding special characters.
Files:
resources/views/livewire/configuration/organization.blade.php
🧠 Learnings (5)
📚 Learning: 2026-02-18T09:45:52.485Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 116
File: app/Livewire/DatabaseServer/ConnectionStatus.php:18-18
Timestamp: 2026-02-18T09:45:52.485Z
Learning: In Livewire components, Eloquent model properties (e.g., public DatabaseServer $server) are automatically locked by the framework to prevent client-side ID tampering. The #[Locked] attribute is only needed for scalar properties (int, string, bool, etc.) that require protection from client-side mutation. Apply this guidance to all Livewire PHP components; use #[Locked] only on primitive properties that you want to shield from client manipulation, and rely on automatic locking for Eloquent model properties.
Applied to files:
app/Livewire/Configuration/Organization.php
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
app/Livewire/Configuration/Organization.phpresources/views/livewire/configuration/organization.blade.php
📚 Learning: 2026-05-06T10:46:47.009Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 275
File: app/Models/User.php:188-216
Timestamp: 2026-05-06T10:46:47.009Z
Learning: In app/Models/User.php, the $cachedRoles cache is intentionally request-scoped to prevent N+1 queries during rendering. Pivot writes (attach() in app/Livewire/User/Create.php, updateExistingPivot() in app/Livewire/Forms/UserForm.php) are terminal within a request, and the cache is not read after a write in the same request. Do not flag missing cache invalidation after pivot writes as a bug; this design is intentional. When reviewing related changes in PHP files under app/, assume this cache pattern and do not raise invalidation issues for these flows.
Applied to files:
app/Livewire/Configuration/Organization.php
📚 Learning: 2026-01-30T22:27:46.107Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 61
File: resources/views/livewire/volume/connectors/s3-config.blade.php:1-13
Timestamp: 2026-01-30T22:27:46.107Z
Learning: In Blade template files (any .blade.php) within the databasement project, allow using alert-info for informational content inside <x-alert> components. The guideline that permits alert-success and alert-error does not exclude using alert-info for informational purposes. Apply this consistently to all Blade components that render alerts; ensure semantic usage and accessibility.
Applied to files:
resources/views/livewire/configuration/organization.blade.php
📚 Learning: 2026-02-06T10:34:43.585Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 75
File: resources/views/livewire/backup-job/_filters.blade.php:36-40
Timestamp: 2026-02-06T10:34:43.585Z
Learning: In Blade template files, when creating compact inline filter controls, prefer using native <input type="checkbox"> elements with daisyUI classes (e.g., checkbox checkbox-warning checkbox-xs) over the Mary UI <x-checkbox> component. The <x-checkbox> component adds wrapper markup (e.g., <div><fieldset><label> with gap-3) that can break tight inline flex layouts. Use the native input approach for compact inline controls, but reserve <x-checkbox> for form fields that require labels, hints, and errors.
Applied to files:
resources/views/livewire/configuration/organization.blade.php
| public function openCreateModal(): void | ||
| { | ||
| $this->newOrgName = ''; | ||
| $this->showCreateModal = true; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the Organization.php Livewire component
find . -path "*/app/Livewire/Configuration/Organization.php" -type f | head -1Repository: David-Crty/databasement
Length of output: 113
🏁 Script executed:
# Get the file size first to determine how to read it
wc -l app/Livewire/Configuration/Organization.phpRepository: David-Crty/databasement
Length of output: 115
🏁 Script executed:
# Read the full file since we need to understand the context
cat -n app/Livewire/Configuration/Organization.phpRepository: David-Crty/databasement
Length of output: 6122
🏁 Script executed:
# Search for validation patterns in Livewire components
rg "resetValidation" --type php -C 2Repository: David-Crty/databasement
Length of output: 740
🏁 Script executed:
# Check if this is a Laravel/Livewire project by looking for config or composer
ls -la | grep -E "composer|artisan|laravel"Repository: David-Crty/databasement
Length of output: 259
Reset the validation error bag when opening modals to prevent stale errors from previous submissions.
The openCreateModal() and openEditModal() methods reset field values but leave Livewire's validation error bag intact. If a form submission fails validation, reopening the modal displays those stale errors on the fresh form.
🩹 Small fix
public function openCreateModal(): void
{
+ $this->resetValidation();
$this->newOrgName = '';
$this->showCreateModal = true;
}
public function openEditModal(string $orgId): void
{
+ $this->resetValidation();
$org = OrganizationModel::findOrFail($orgId);This pattern is already established elsewhere in the codebase (ConfigurationForm.php, NotificationChannelForm.php use resetValidation()). Aligns with the guideline to keep server-side state properly synchronized with the UI.
🤖 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 `@app/Livewire/Configuration/Organization.php` around lines 57 - 61,
openCreateModal() and openEditModal() currently clear fields but leave
Livewire's validation error bag, causing stale validation messages to appear
when reopening the modal; call Livewire's resetValidation() in both methods (in
Organization::openCreateModal and Organization::openEditModal) to clear the
error bag when opening the modal so the form starts clean.
…ulti-organization
- Revert Fortify auth to block all OAuth users from password login - Rename "oauth only" to "oauth" in test names and comments - Add profile email-blocking tests for OAuth users - Add OIDC_DEFAULT_ORGANIZATION_ID tests (default org, fallback, role sync) - Assert OIDC_DEFAULT_ORGANIZATION_ID in configuration page test
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- Skip password validation for OAuth users in DeleteUserForm - Move modal buttons inside <form> tag (x-slot:actions renders outside form) - Use tap/logout/delete pattern to avoid session race condition - Flash success message on login page after account deletion - Add test for OAuth passwordless account deletion
Redirecting to `/` caused a double redirect (/ → /login) which consumed the flash message. Redirect to the login route directly so the "Your account has been deleted." message is visible.
When a user has no organization (removed by admin, or orphaned account), the SetCurrentOrganization middleware now logs them out and shows an error on the login page instead of crashing with a TypeError. Also adds reset() to CurrentOrganization singleton to ensure fresh resolution on each request.
1 participant
Summary
?org=query parameter andX-Organization-Idheader for org selectionKey changes
Organizationmodel withOrganizationScopeglobal scopeCurrentOrganizationsingleton service resolved by middlewareSetCurrentOrganizationmiddleware handles web (cookie) and API (header/query param) org resolutionorgquery param into OpenAPI docsTest plan
?org=ProductionOrgreturns scoped resultsSummary by CodeRabbit
New Features
New Services / Policies
Improvements
Documentation
Tests