fix(session):Added Session::flush() + Session::regenerate() at the en…

[smarcet]

Summary by CodeRabbit

• Chores

  • Streamlined session management by centralizing logout cleanup operations for improved consistency and security.

• Tests

  • Added comprehensive test suite validating session cleanup, cookie handling, and security context clearing during logout across different user scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

Session management operations are being consolidated from multiple controllers into the AuthService. Session::flush() and Session::regenerate() calls are removed from HomeController and UserController, then centralized in AuthService::logout(). A comprehensive test suite validates the refactored behavior.

Changes

Cohort / File(s)	Summary
Controller Session Cleanup Removal app/Http/Controllers/HomeController.php, app/Http/Controllers/UserController.php	Removed Session facade import and explicit Session::flush()/Session::regenerate() calls from guest check and logout logic, respectively.
Auth Service Session Management app/libs/Auth/AuthService.php	Added Session::flush() and Session::regenerate() calls to logout() method, centralizing session cleanup at the service layer after authentication logout and cookie issuance.
Logout Test Suite tests/AuthServiceLogoutTest.php	New comprehensive test class with 8 test methods validating session invalidation, session ID capture, cookie deletion, security context clearing, principal service cleanup, and action logging order across guest and authenticated scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 Hops and rejoices!
Sessions now flutter from controller to service,
Where cleanup happens cleanly and purposefully,
Tests dance in formation to prove it all works—
Order and clarity bloom in every line! 🌿✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is truncated and incomplete, ending with 'en…' without specifying what stage of the process the session flush/regenerate should occur in.	Complete the pull request title to clearly indicate the specific location or context where Session::flush() and Session::regenerate() are being added (e.g., 'fix(session): Add Session::flush() and Session::regenerate() in AuthService logout').

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch hotfix/session-cleanup

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/AuthServiceLogoutTest.php (1)

79-81: Minor: debug_msg is not a Laravel Log facade method.

Line 81 mocks debug_msg which doesn't exist on Laravel's Log facade. This is harmless since Mockery will just ignore calls that don't happen, but it's unnecessary noise.

🧹 Suggested cleanup

         // Log calls are always allowed
         $this->log_mock->shouldReceive('debug')->zeroOrMoreTimes();
-        $this->log_mock->shouldReceive('debug_msg')->zeroOrMoreTimes();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/AuthServiceLogoutTest.php` around lines 79 - 81, Remove the unnecessary
mock for the non-existent Log method by deleting the shouldReceive('debug_msg')
line in the test setup where $this->log_mock is configured; keep the valid
shouldReceive('debug')->zeroOrMoreTimes() call so logging remains allowed, and
ensure no other tests rely on a custom debug_msg expectation.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/AuthServiceLogoutTest.php`:
- Around line 79-81: Remove the unnecessary mock for the non-existent Log method
by deleting the shouldReceive('debug_msg') line in the test setup where
$this->log_mock is configured; keep the valid
shouldReceive('debug')->zeroOrMoreTimes() call so logging remains allowed, and
ensure no other tests rely on a custom debug_msg expectation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 302357c8-3e6c-4ae2-a3d8-2b1246bc4234

📥 Commits

Reviewing files that changed from the base of the PR and between fe5432a and 4992042.

📒 Files selected for processing (4)

• app/Http/Controllers/HomeController.php
• app/Http/Controllers/UserController.php
• app/libs/Auth/AuthService.php
• tests/AuthServiceLogoutTest.php

💤 Files with no reviewable changes (2)

• app/Http/Controllers/UserController.php
• app/Http/Controllers/HomeController.php