[IP-11]: Redirect to ivplv2 company dashboard after login#484
[IP-11]: Redirect to ivplv2 company dashboard after login#484nielsdrost7 wants to merge 4 commits into
Conversation
After authentication, both elevated and regular users are now sent to the ivplv2 company dashboard. For elevated users the ivplv2 company is looked up by search_code; for regular users it is preferred from their attached companies. Both paths fall back gracefully when ivplv2 is absent. Six PHPUnit tests cover the redirect behaviour across role types and company attachment scenarios. https://claude.ai/code/session_01KVV91ECY14zwgavRm99zPs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Fixed 2 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
📝 WalkthroughWalkthroughLoginResponse now differentiates elevated and non-elevated users when resolving tenant redirection, preferring a company matching a private DEFAULT_COMPANY_CODE constant (case-insensitive) with fallback to the oldest company. Session assignment and error handling were adjusted, and a new feature test suite validates the redirect behavior. ChangesLogin redirect with default company preference
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant LoginResponse
participant Company
participant Filament
User->>LoginResponse: submit login credentials
LoginResponse->>LoginResponse: check elevated role
alt elevated user
LoginResponse->>Company: query search_code = DEFAULT_COMPANY_CODE
alt found
Company-->>LoginResponse: return matching company
else not found
LoginResponse->>Company: fallback oldest company
Company-->>LoginResponse: return oldest company
end
LoginResponse->>Filament: setTenant(tenant)
else non-elevated user
LoginResponse->>Company: query user's companies for search_code match
alt found
Company-->>LoginResponse: return matching company
else not found
LoginResponse->>Company: fallback oldest user company
Company-->>LoginResponse: return oldest company
end
LoginResponse->>LoginResponse: session(['current_company_id' => tenant->id])
LoginResponse->>Filament: setTenant(tenant)
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nielsdrost7
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Modules/Core/Filament/Responses/LoginResponse.php`:
- Around line 45-46: Remove the unconditional session(['current_company_id' =>
$tenant->id]) and filament()->setTenant($tenant) calls in LoginResponse so the
method respects the existing elevated vs non-elevated branches; keep session
writes only in the non-elevated path and avoid duplicating setTenant after the
branch-specific logic has already finalized tenant state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7c94fa0-2278-448d-86ac-c7587bd92fa7
📒 Files selected for processing (1)
Modules/Core/Filament/Responses/LoginResponse.php
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Modules/Core/Filament/Responses/LoginResponse.php`:
- Around line 45-46: Remove the unconditional session(['current_company_id' =>
$tenant->id]) and filament()->setTenant($tenant) calls in LoginResponse so the
method respects the existing elevated vs non-elevated branches; keep session
writes only in the non-elevated path and avoid duplicating setTenant after the
branch-specific logic has already finalized tenant state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7c94fa0-2278-448d-86ac-c7587bd92fa7
📒 Files selected for processing (1)
Modules/Core/Filament/Responses/LoginResponse.php
🛑 Comments failed to post (1)
Modules/Core/Filament/Responses/LoginResponse.php (1)
45-46: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Redundant tenant/session writes leak
current_company_idfor elevated users.Both branches already finalize their own state: the elevated branch calls
filament()->setTenant($tenant)(Line 30) and intentionally omits any session write, while the non-elevated branch sets both the session and tenant (Lines 41–42). Lines 45–46 repeat these unconditionally, which (a) duplicates the setTenant call and (b) writescurrent_company_idinto the session for elevated users too — contradicting the design where session assignment is scoped to non-elevated users only. Removing these lines restores the intended contract.🔧 Proposed fix
session(['current_company_id' => $tenant->id]); filament()->setTenant($tenant); } - session(['current_company_id' => $tenant->id]); - filament()->setTenant($tenant); - return redirect()->route('filament.company.pages.dashboard', [ 'tenant' => Str::lower($tenant->search_code), ]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Core/Filament/Responses/LoginResponse.php` around lines 45 - 46, Remove the unconditional session(['current_company_id' => $tenant->id]) and filament()->setTenant($tenant) calls in LoginResponse so the method respects the existing elevated vs non-elevated branches; keep session writes only in the non-elevated path and avoid duplicating setTenant after the branch-specific logic has already finalized tenant state.
2 participants
After authentication, both elevated and regular users are now sent to
the ivplv2 company dashboard. For elevated users the ivplv2 company is
looked up by search_code; for regular users it is preferred from their
attached companies. Both paths fall back gracefully when ivplv2 is absent.
Six PHPUnit tests cover the redirect behaviour across role types and
company attachment scenarios.
Summary by CodeRabbit
Bug Fixes
Tests
addresses [Core]: Company Switcher #138 [Core]: Login screen #11
Summary by CodeRabbit
Bug Fixes
Tests