cleans up flash message keys. #33
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds username-or-email login support and introduces a viewKey() method for FlashMessageType; controllers and views updated to use view-specific flash keys. Tests added for email-based authentication and enum behavior; login form placeholder text updated. Changes
Sequence DiagramsequenceDiagram
participant Client
participant AuthService as Authentication Service
participant DB as Database
participant Session
Client->>AuthService: attempt(identifier, password)
AuthService->>AuthService: is identifier an email?
alt Email detected
AuthService->>DB: findByEmail(identifier)
else Username detected
AuthService->>DB: findByUsername(identifier)
end
DB-->>AuthService: returns User or null
alt User found
AuthService->>AuthService: verify password
alt Password valid
AuthService->>Session: set authenticated state
AuthService-->>Client: success
else Password invalid
AuthService-->>Client: failure
end
else User not found
AuthService-->>Client: failure
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)tests/Unit/Cms/Enums/FlashMessageTypeTest.php (1)
⏰ 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). (4)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting 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 |
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 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: 2
🤖 Fix all issues with AI agents
In `@src/Cms/Controllers/Admin/Events.php`:
- Around line 89-93: The controller is passing capitalized view keys via
FlashMessageType::SUCCESS->viewKey() and FlashMessageType::ERROR->viewKey(), but
the template expects lowercase $success and $error; update the Events controller
where ->with([...]) is built to use lowercase keys ('success' and 'error') and
assign the flash values from
$sessionManager->getFlash(FlashMessageType::SUCCESS->value) and
getFlash(FlashMessageType::ERROR->value) respectively (replace the current
FlashMessageType::...->viewKey() keys with 'success' and 'error') so the
template variables match.
In `@tests/Unit/Cms/Services/AuthenticationTest.php`:
- Around line 630-640: In testAttemptWithUsernameStillWorks remove the unused
local variable by calling createTestUser('usernametest', 'TestPass123') for its
side effects without assigning it to $user (or delete the $user declaration), so
the test keeps creating the user but no longer defines an unused $user variable
referenced in the test method.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
resources/views/auth/login/login.phpsrc/Cms/Controllers/Admin/EventCategories.phpsrc/Cms/Controllers/Admin/Events.phpsrc/Cms/Controllers/Admin/Media.phpsrc/Cms/Controllers/Admin/Pages.phpsrc/Cms/Controllers/Admin/Posts.phpsrc/Cms/Controllers/Admin/Profile.phpsrc/Cms/Controllers/Admin/Users.phpsrc/Cms/Controllers/Auth/Login.phpsrc/Cms/Enums/FlashMessageType.phpsrc/Cms/Services/Auth/Authentication.phptests/Unit/Cms/Services/AuthenticationTest.php
🧰 Additional context used
🧬 Code graph analysis (9)
src/Cms/Services/Auth/Authentication.php (2)
src/Cms/Repositories/DatabaseUserRepository.php (2)
findByEmail(59-62)findByUsername(51-54)src/Cms/Repositories/IUserRepository.php (2)
findByEmail(27-27)findByUsername(22-22)
src/Cms/Controllers/Auth/Login.php (3)
src/Cms/Enums/FlashMessageType.php (1)
viewKey(27-30)src/Cms/Controllers/Content.php (1)
getSessionManager(234-248)src/Cms/Auth/SessionManager.php (1)
getFlash(170-176)
src/Cms/Controllers/Admin/Posts.php (2)
src/Cms/Enums/FlashMessageType.php (1)
viewKey(27-30)src/Cms/Auth/SessionManager.php (1)
getFlash(170-176)
src/Cms/Controllers/Admin/Profile.php (3)
src/Cms/Enums/FlashMessageType.php (1)
viewKey(27-30)src/Cms/Controllers/Content.php (1)
getSessionManager(234-248)src/Cms/Auth/SessionManager.php (1)
getFlash(170-176)
src/Cms/Controllers/Admin/Users.php (2)
src/Cms/Enums/FlashMessageType.php (1)
viewKey(27-30)src/Cms/Auth/SessionManager.php (1)
getFlash(170-176)
src/Cms/Controllers/Admin/Events.php (2)
src/Cms/Enums/FlashMessageType.php (1)
viewKey(27-30)src/Cms/Auth/SessionManager.php (1)
getFlash(170-176)
src/Cms/Controllers/Admin/Pages.php (2)
src/Cms/Enums/FlashMessageType.php (1)
viewKey(27-30)src/Cms/Auth/SessionManager.php (1)
getFlash(170-176)
src/Cms/Controllers/Admin/EventCategories.php (2)
src/Cms/Enums/FlashMessageType.php (1)
viewKey(27-30)src/Cms/Auth/SessionManager.php (1)
getFlash(170-176)
src/Cms/Controllers/Admin/Media.php (2)
src/Cms/Enums/FlashMessageType.php (1)
viewKey(27-30)src/Cms/Auth/SessionManager.php (1)
getFlash(170-176)
🪛 PHPMD (2.15.0)
tests/Unit/Cms/Services/AuthenticationTest.php
632-632: Avoid unused local variables such as '$user'. (undefined)
(UnusedLocalVariable)
⏰ 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). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: build-test (postgres)
- GitHub Check: build-test (mysql)
🔇 Additional comments (13)
src/Cms/Enums/FlashMessageType.php (1)
19-30: LGTM: viewKey centralizes view-key mapping.Keeps view-specific keys derived from enum values in one place.
src/Cms/Controllers/Admin/Profile.php (1)
83-87: LGTM: consistent viewKey usage.Matches the new enum helper and keeps session keys unchanged.
src/Cms/Controllers/Admin/Posts.php (1)
103-107: LGTM: consistent viewKey usage.Matches the enum helper and keeps flash retrieval via lowercase keys.
src/Cms/Controllers/Admin/Pages.php (1)
89-93: LGTM: consistent viewKey usage.Matches the enum helper and keeps flash retrieval via lowercase keys.
src/Cms/Controllers/Admin/EventCategories.php (1)
75-76: LGTM: view-keyed flash data aligns with the new enum API.src/Cms/Controllers/Admin/Media.php (2)
117-118: LGTM: success-path flash keys now use viewKey().
138-139: LGTM: error-path flash keys also use viewKey(), keeping rendering consistent.src/Cms/Controllers/Admin/Users.php (1)
82-83: LGTM: users index uses viewKey() for flash data.resources/views/auth/login/login.php (1)
6-12: LGTM: copy now reflects username/email login.src/Cms/Services/Auth/Authentication.php (1)
47-58: No issue here—username validation prevents email-formatted usernames.Usernames are constrained by pattern
/^[a-zA-Z0-9_-]+$/, which explicitly disallows "@" and other special characters. Email-formatted usernames are impossible per the YAML validation rules. The authentication logic correctly routes based on whether the input matches email format—if it does, search by email; otherwise, search by username. No fallback is needed because the scenario you described cannot occur.Likely an incorrect or invalid review comment.
src/Cms/Controllers/Auth/Login.php (1)
78-79: LGTM!The refactoring correctly separates the concerns: using
viewKey()for view variable names (capitalized, e.g., 'Error') andvaluefor session storage keys (lowercase, e.g., 'error'). This improves consistency across the codebase and centralizes the key derivation logic in the enum.tests/Unit/Cms/Services/AuthenticationTest.php (2)
615-625: LGTM!Good test coverage for the email-based login feature. The test properly verifies:
- Authentication succeeds with email
- User is marked as authenticated
- The resolved username matches the expected value
645-664: LGTM!Good edge case coverage for the email-based authentication. These tests properly verify that:
- Nonexistent email addresses fail authentication
- Incorrect passwords fail even when using a valid email
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| FlashMessageType::SUCCESS->value => $sessionManager->getFlash( FlashMessageType::SUCCESS->value ), | ||
| FlashMessageType::ERROR->value => $sessionManager->getFlash( FlashMessageType::ERROR->value ) | ||
| FlashMessageType::SUCCESS->viewKey() => $sessionManager->getFlash( FlashMessageType::SUCCESS->value ), | ||
| FlashMessageType::ERROR->viewKey() => $sessionManager->getFlash( FlashMessageType::ERROR->value ) |
There was a problem hiding this comment.
Missing view updates break flash messages on multiple pages
High Severity
The controllers now pass flash message data using viewKey() which returns capitalized keys ('Success', 'Error'), but several view templates still expect lowercase variables ($success, $error). Only resources/views/admin/events/index.php was updated. The views for Event Categories, Media, Pages, and Profile still use lowercase variables and will silently fail to display any flash messages to users.
Note
Authentication and UX
Authentication::attemptnow accepts username or email (auto-detects email); added unit tests to cover email/username success/failure pathsFlash messaging consistency
FlashMessageType::viewKey()and updated controllers and views to pass/display flash messages using capitalized view keys (e.g.,$Success,$Error)resources/views/admin/events/index.phpaccordinglyFlashMessageType::viewKey()Written by Cursor Bugbot for commit 75710fe. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.