-
Notifications
You must be signed in to change notification settings - Fork 0
Code refactoring and additional tests. #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@ljonesfl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds an email templating/sender system, an event-driven layer (events + listeners), service-based CRUD for users/categories, named route helpers and route renaming, DTO schemas, controller refactors for session/redirect helpers and updated return types, new homepage and email views, and extensive unit tests and docs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Browser
participant Controller
participant Service
participant Repo
participant EventEmitter
participant Listener
participant Sender
User->>Browser: submits form
Browser->>Controller: POST (form data)
Controller->>Service: validate & process
alt validation fails
Service-->>Controller: returns error
Controller->>Browser: redirect with flash(error)
else success
Service->>Repo: persist entity
Repo-->>Service: persisted entity
Service->>EventEmitter: emit(Event) %% new/changed interaction
EventEmitter->>Listener: dispatch Event
Listener->>Sender: prepare/send email (if applicable)
Listener-->>EventEmitter: handled
Controller->>Browser: redirect with flash(success)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Cms/Services/Email/Sender.php (1)
119-131: Close the output buffer on template failures.If
require $templateFile;throws, theob_start()buffer stays open and the method rethrows, leaving an extra output buffer level active for the rest of the request. That can swallow later output or leak memory. Please ensure the buffer is cleared in the exceptional path.Apply this diff:
- ob_start(); - require $templateFile; - $this->_body = ob_get_clean(); + ob_start(); + try + { + require $templateFile; + $this->_body = ob_get_clean(); + } + catch( \Throwable $renderingError ) + { + ob_end_clean(); + throw $renderingError; + }src/Cms/Controllers/Admin/PostController.php (1)
107-108: Define the session manager before reading flashes.
$sessionManageris never defined in this method, so attempting to read flashes here will trigger a fatal error as soon as the page loads. Please initialize it (e.g.,$sessionManager = $this->getSessionManager();) before building$viewData.Apply this diff near the top of
index():// Get all posts or filter by author if not admin if( $user->isAdmin() || $user->isEditor() ) { $posts = $this->_postRepository->all(); } else { $posts = $this->_postRepository->getByAuthor( $user->getUsername() ); } + $sessionManager = $this->getSessionManager(); + $viewData = [
🧹 Nitpick comments (16)
src/Cms/Dtos/CreateCategoryDto.yaml (1)
10-14: Consider adding slug format validation.The slug field is required but only has
IsNotEmptyvalidation. Slugs typically require specific formatting (e.g., lowercase alphanumeric with hyphens, no spaces or special characters). Consider adding a pattern validator to ensure valid URL-safe slugs.Apply this diff to add slug format validation:
slug: type: string required: true validators: - IsNotEmpty + - IsSlugOr if a pattern validator is available:
slug: type: string required: true validators: - IsNotEmpty + - IsPattern: + pattern: '/^[a-z0-9]+(?:-[a-z0-9]+)*$/'src/Cms/Dtos/UpdateUserDto.yaml (2)
16-21: Consider adding maximum length validation for password.While the minimum length of 8 characters is appropriate, password fields should also have an upper bound to prevent potential issues with password hashing or storage.
Apply this diff to add a maximum length:
password: type: string required: false validators: - IsStringLength: min: 8 + max: 128
22-26: Add validation for allowed role values.The role field only validates that it's not empty, but should validate against the allowed role constants defined in the User model (admin, editor, author, subscriber).
Apply this diff to add role value validation:
role: type: string required: true validators: - IsNotEmpty + - IsIn: + values: ['admin', 'editor', 'author', 'subscriber']src/Cms/Dtos/CreatePostDto.yaml (1)
17-21: Add validation for allowed status values.The status field only validates that it's not empty, but should validate against the allowed status constants defined in the Post model (draft, published, scheduled).
Apply this diff to add status value validation:
status: type: string required: true validators: - IsNotEmpty + - IsIn: + values: ['draft', 'published', 'scheduled']src/Cms/Controllers/Content.php (1)
237-247: Consider validating flash array structure before destructuring.The array destructuring
[$type, $message] = $flashassumes the flash array has exactly 2 elements at indices 0 and 1. If the caller passes a differently structured array (e.g., associative array or wrong number of elements), this will cause a runtime error.Apply this diff to add validation:
protected function redirect( string $routeName, array $parameters = [], ?array $flash = null ): never { if( $flash ) { + if( count( $flash ) !== 2 ) + { + throw new \InvalidArgumentException( 'Flash array must contain exactly 2 elements: [type, message]' ); + } [ $type, $message ] = $flash; $this->getSessionManager()->flash( $type, $message ); }Alternatively, use named keys for better clarity:
- if( $flash ) + if( $flash && isset( $flash['type'], $flash['message'] ) ) { - [ $type, $message ] = $flash; - $this->getSessionManager()->flash( $type, $message ); + $this->getSessionManager()->flash( $flash['type'], $flash['message'] ); }tests/Cms/ContentControllerTest.php (1)
199-216: Strengthen test assertion by verifying flash message was set.The test currently only checks that the session manager is an instance of
SessionManager, but doesn't verify that the flash message was actually stored. Consider checking the flash message content to ensure the method works as expected.Apply this diff to enhance the test:
// Verify flash was set in session manager $sessionManager = $getSessionMethod->invoke( $controller ); $this->assertInstanceOf( \Neuron\Cms\Auth\SessionManager::class, $sessionManager ); + + // Verify the flash message was actually set + $this->assertTrue( $sessionManager->hasFlash( 'success' ) ); + $this->assertEquals( 'Test message', $sessionManager->getFlash( 'success' ) ); }src/Cms/Dtos/UpdatePostDto.yaml (4)
10-16: Consider adding a maximum length constraint for the body field.The body field has a minimum length of 10 characters but no maximum. This could allow unbounded input, potentially leading to performance issues, storage concerns, or denial-of-service vulnerabilities.
Apply this diff to add a reasonable maximum length:
body: type: string required: true validators: - IsNotEmpty - IsStringLength: min: 10 + max: 100000
17-21: Add enum validation for the status field.The status field only validates that it's not empty but doesn't constrain the allowed values. This could allow invalid status values to be submitted.
Consider adding an enum validator to restrict status to valid values:
status: type: string required: true validators: - IsNotEmpty + - IsInList: + values: [draft, published, archived]
31-33: Consider validating categoryIds array contents.The categoryIds field accepts an array but has no validation on array elements or length. This could allow invalid or excessive category IDs.
Consider adding validators:
categoryIds: type: array required: false + validators: + - IsArray + - ArrayMaxLength: + max: 50
34-36: Consider adding length validation for the tags field.The tags field has no length constraints, which could allow unbounded input.
Apply this diff:
tags: type: string required: false + validators: + - IsStringLength: + max: 500docs/EMAIL_TEMPLATES.md (2)
36-36: Add language specifiers to fenced code blocks.To improve markdown rendering and comply with linting rules, add language identifiers to the fenced code blocks at lines 36, 150, and 391.
Apply these changes:
+```text
[INFO] TEST MODE - Email not sent
[INFO] To: newuser@example.com
...+```text
RuntimeException: Failed to render email template: emails/welcomeAlso applies to: 150-150, 391-391
87-87: Convert emphasis to proper heading.Line 87 uses emphasis (bold) instead of a proper markdown heading. Convert to a heading for better document structure.
Apply this change:
-**mail** (default) +#### mail (default)Similarly for other driver names (sendmail, smtp) if they follow the same pattern.
src/Cms/Controllers/Home.php (1)
26-36: Silence the unused-parameter warnings?Implementation looks fine, but PHPMD is already flagging
$parametersand$request. If you want a quiet report, consider either prefixing them with_, unsetting them, or adding a targeted suppression annotation.tests/Cms/Services/User/DeleterTest.php (1)
44-68: Add assertions to exercise delete outcomes.
testDeletesMultipleUserscurrently passes even ifDeleter::delete()stopped returningtrue, because the mock expectations succeed regardless. Capturing each call’s return value and asserting it staystruekeeps the test guarding the service contract, not just the repository interaction.Consider applying this diff:
- $this->_deleter->delete( 1 ); - $this->_deleter->delete( 2 ); - $this->_deleter->delete( 3 ); + $this->assertTrue( $this->_deleter->delete( 1 ) ); + $this->assertTrue( $this->_deleter->delete( 2 ) ); + $this->assertTrue( $this->_deleter->delete( 3 ) );resources/config/event-listeners.yaml (1)
15-35: Consider adding comments for empty listener arrays.The empty listener arrays for
post.created,category.created, andcategory.deletedare intentional placeholders, but adding brief comments would make the intent clearer for future maintainers.Example:
post.created: class: 'Neuron\Cms\Events\PostCreatedEvent' - listeners: [] + listeners: [] # Reserved for future listenerssrc/Cms/Services/Category/Updater.php (1)
73-79: Extract duplicated slug generation logic.The
generateSlugmethod is duplicated betweenCreator.php(lines 71-77) and thisUpdater.php(lines 73-79). Consider extracting to a shared location to follow DRY principles.Options:
- Extract to a trait (if only used by these two classes):
trait GeneratesSlug { private function generateSlug(string $name): string { $slug = strtolower(trim($name)); $slug = preg_replace('/[^a-z0-9-]/', '-', $slug); $slug = preg_replace('/-+/', '-', $slug); return trim($slug, '-'); } }
- Extract to a utility class (if used more broadly):
namespace Neuron\Cms\Utils; class SlugGenerator { public static function generate(string $name): string { $slug = strtolower(trim($name)); $slug = preg_replace('/[^a-z0-9-]/', '-', $slug); $slug = preg_replace('/-+/', '-', $slug); return trim($slug, '-'); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (83)
docs/EMAIL_TEMPLATES.md(1 hunks)docs/EVENTS.md(1 hunks)resources/app/Initializers/AuthInitializer.php(1 hunks)resources/app/Initializers/MaintenanceInitializer.php(1 hunks)resources/app/Initializers/PasswordResetInitializer.php(3 hunks)resources/config/email.yaml.example(1 hunks)resources/config/event-listeners.yaml(1 hunks)resources/config/routes.yaml(1 hunks)resources/views/admin/auth/login.php(1 hunks)resources/views/admin/categories/create.php(2 hunks)resources/views/admin/categories/edit.php(2 hunks)resources/views/admin/categories/index.php(2 hunks)resources/views/admin/dashboard/index.php(1 hunks)resources/views/admin/posts/create.php(2 hunks)resources/views/admin/posts/edit.php(2 hunks)resources/views/admin/posts/index.php(2 hunks)resources/views/admin/profile/edit.php(2 hunks)resources/views/admin/tags/create.php(2 hunks)resources/views/admin/tags/edit.php(2 hunks)resources/views/admin/tags/index.php(2 hunks)resources/views/admin/users/create.php(2 hunks)resources/views/admin/users/edit.php(2 hunks)resources/views/admin/users/index.php(2 hunks)resources/views/auth/forgot-password.php(2 hunks)resources/views/auth/login.php(2 hunks)resources/views/auth/reset-password.php(1 hunks)resources/views/blog/category.php(3 hunks)resources/views/blog/index.php(2 hunks)resources/views/blog/show.php(3 hunks)resources/views/blog/tag.php(3 hunks)resources/views/emails/password-reset.php(1 hunks)resources/views/emails/welcome.php(1 hunks)resources/views/home/index.php(1 hunks)resources/views/layouts/admin.php(3 hunks)resources/views/layouts/default.php(3 hunks)src/Cms/Auth/PasswordResetManager.php(4 hunks)src/Cms/Cli/Commands/Install/InstallCommand.php(1 hunks)src/Cms/Controllers/Admin/CategoryController.php(6 hunks)src/Cms/Controllers/Admin/DashboardController.php(1 hunks)src/Cms/Controllers/Admin/PostController.php(11 hunks)src/Cms/Controllers/Admin/ProfileController.php(5 hunks)src/Cms/Controllers/Admin/TagController.php(4 hunks)src/Cms/Controllers/Admin/UserController.php(8 hunks)src/Cms/Controllers/Auth/LoginController.php(2 hunks)src/Cms/Controllers/Content.php(3 hunks)src/Cms/Controllers/Home.php(1 hunks)src/Cms/Dtos/CreateCategoryDto.yaml(1 hunks)src/Cms/Dtos/CreatePostDto.yaml(1 hunks)src/Cms/Dtos/CreateUserDto.yaml(1 hunks)src/Cms/Dtos/UpdateCategoryDto.yaml(1 hunks)src/Cms/Dtos/UpdatePostDto.yaml(1 hunks)src/Cms/Dtos/UpdateUserDto.yaml(1 hunks)src/Cms/Events/CategoryCreatedEvent.php(1 hunks)src/Cms/Events/CategoryDeletedEvent.php(1 hunks)src/Cms/Events/CategoryUpdatedEvent.php(1 hunks)src/Cms/Events/PostCreatedEvent.php(1 hunks)src/Cms/Events/PostDeletedEvent.php(1 hunks)src/Cms/Events/PostPublishedEvent.php(1 hunks)src/Cms/Events/UserCreatedEvent.php(1 hunks)src/Cms/Events/UserDeletedEvent.php(1 hunks)src/Cms/Events/UserUpdatedEvent.php(1 hunks)src/Cms/Listeners/ClearCacheListener.php(1 hunks)src/Cms/Listeners/LogUserActivityListener.php(1 hunks)src/Cms/Listeners/SendWelcomeEmailListener.php(1 hunks)src/Cms/Services/Category/Creator.php(1 hunks)src/Cms/Services/Category/Deleter.php(1 hunks)src/Cms/Services/Category/Updater.php(1 hunks)src/Cms/Services/Email/Sender.php(4 hunks)src/Cms/Services/User/Creator.php(1 hunks)src/Cms/Services/User/Deleter.php(1 hunks)src/Cms/Services/User/Updater.php(1 hunks)src/Cms/View/helpers.php(1 hunks)tests/Cms/Auth/PasswordResetManagerTest.php(1 hunks)tests/Cms/ContentControllerTest.php(1 hunks)tests/Cms/Events/EventsTest.php(1 hunks)tests/Cms/Listeners/ListenersTest.php(1 hunks)tests/Cms/Services/Category/CreatorTest.php(1 hunks)tests/Cms/Services/Category/DeleterTest.php(1 hunks)tests/Cms/Services/Category/UpdaterTest.php(1 hunks)tests/Cms/Services/User/CreatorTest.php(1 hunks)tests/Cms/Services/User/DeleterTest.php(1 hunks)tests/Cms/Services/User/UpdaterTest.php(1 hunks)versionlog.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (62)
resources/views/blog/tag.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Post.php (2)
getSlug(82-85)getTitle(65-68)
resources/views/auth/reset-password.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
resources/views/blog/index.php (5)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Post.php (3)
Post(13-516)getSlug(82-85)getTitle(65-68)src/Cms/Models/Category.php (2)
getSlug(64-67)getName(47-50)src/Cms/Models/Tag.php (2)
getSlug(63-66)getName(46-49)src/Cms/Controllers/Blog.php (2)
category(199-230)tag(160-191)
resources/views/auth/forgot-password.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
src/Cms/Controllers/Home.php (2)
src/Cms/Controllers/Content.php (4)
Content(57-279)getTitle(138-141)getName(120-123)getDescription(156-159)src/Cms/Controllers/Admin/CategoryController.php (1)
index(54-78)
tests/Cms/Services/User/DeleterTest.php (2)
src/Cms/Models/User.php (1)
User(12-459)src/Cms/Services/User/Deleter.php (2)
Deleter(16-55)delete(32-54)
resources/views/admin/tags/create.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
src/Cms/Events/UserDeletedEvent.php (5)
src/Cms/Events/CategoryCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/CategoryDeletedEvent.php (2)
__construct(14-16)getName(18-21)src/Cms/Events/PostDeletedEvent.php (2)
__construct(14-16)getName(18-21)src/Cms/Events/UserCreatedEvent.php (1)
getName(19-22)src/Cms/Events/UserUpdatedEvent.php (1)
getName(19-22)
resources/views/admin/posts/index.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Post.php (2)
getSlug(82-85)getId(48-51)
tests/Cms/Services/Category/CreatorTest.php (2)
src/Cms/Models/Category.php (1)
Category(13-186)src/Cms/Services/Category/Creator.php (2)
Creator(18-79)create(36-64)
resources/views/admin/posts/create.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
resources/views/blog/category.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Post.php (2)
getSlug(82-85)getTitle(65-68)
resources/views/admin/users/create.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
src/Cms/Events/PostDeletedEvent.php (6)
src/Cms/Events/CategoryCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/CategoryDeletedEvent.php (2)
__construct(14-16)getName(18-21)src/Cms/Events/CategoryUpdatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/PostCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/PostPublishedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/UserDeletedEvent.php (1)
getName(18-21)
resources/views/admin/auth/login.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Auth/helpers.php (1)
csrf_field(122-126)
resources/views/admin/categories/create.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
src/Cms/Events/CategoryCreatedEvent.php (3)
src/Cms/Models/Category.php (1)
Category(13-186)src/Cms/Events/CategoryDeletedEvent.php (2)
__construct(14-16)getName(18-21)src/Cms/Events/CategoryUpdatedEvent.php (2)
__construct(15-17)getName(19-22)
tests/Cms/Services/Category/DeleterTest.php (2)
src/Cms/Models/Category.php (1)
Category(13-186)src/Cms/Services/Category/Deleter.php (2)
Deleter(16-55)delete(32-54)
resources/views/admin/profile/edit.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
src/Cms/Events/UserUpdatedEvent.php (7)
src/Cms/Models/User.php (1)
User(12-459)src/Cms/Events/CategoryCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/CategoryUpdatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/PostCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/PostPublishedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/UserCreatedEvent.php (1)
getName(19-22)src/Cms/Events/UserDeletedEvent.php (1)
getName(18-21)
src/Cms/Services/User/Deleter.php (3)
src/Cms/Events/UserDeletedEvent.php (1)
UserDeletedEvent(12-22)src/Cms/Services/Category/Deleter.php (2)
Deleter(16-55)delete(32-54)src/Cms/Controllers/Admin/UserController.php (1)
__construct(34-47)
tests/Cms/ContentControllerTest.php (2)
src/Cms/Controllers/Content.php (1)
Content(57-279)src/Cms/Auth/SessionManager.php (1)
SessionManager(13-185)
tests/Cms/Listeners/ListenersTest.php (8)
src/Cms/Events/UserCreatedEvent.php (1)
UserCreatedEvent(13-23)src/Cms/Events/UserUpdatedEvent.php (1)
UserUpdatedEvent(13-23)src/Cms/Events/PostPublishedEvent.php (1)
PostPublishedEvent(13-23)src/Cms/Listeners/SendWelcomeEmailListener.php (2)
SendWelcomeEmailListener(19-80)event(27-79)src/Cms/Listeners/LogUserActivityListener.php (2)
LogUserActivityListener(16-39)event(24-38)src/Cms/Listeners/ClearCacheListener.php (2)
ClearCacheListener(21-78)event(29-43)src/Cms/Models/User.php (2)
User(12-459)setUsername(78-82)src/Cms/Models/Post.php (1)
Post(13-516)
src/Cms/Controllers/Content.php (1)
src/Cms/Auth/SessionManager.php (3)
SessionManager(13-185)start(32-55)flash(132-136)
resources/views/blog/show.php (2)
src/Cms/Models/Post.php (2)
Post(13-516)getTags(368-371)src/Cms/View/helpers.php (1)
route_path(70-79)
src/Cms/Services/User/Creator.php (5)
src/Cms/Models/User.php (1)
User(12-459)src/Cms/Auth/PasswordHasher.php (4)
PasswordHasher(13-200)meetsRequirements(53-86)getValidationErrors(91-121)hash(24-30)src/Cms/Events/UserCreatedEvent.php (1)
UserCreatedEvent(13-23)src/Cms/Services/Category/Creator.php (2)
Creator(18-79)create(36-64)src/Cms/Controllers/Admin/UserController.php (2)
__construct(34-47)create(88-109)
src/Cms/Events/UserCreatedEvent.php (5)
src/Cms/Models/User.php (1)
User(12-459)src/Cms/Events/CategoryCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/PostCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/UserDeletedEvent.php (1)
getName(18-21)src/Cms/Events/UserUpdatedEvent.php (1)
getName(19-22)
src/Cms/Listeners/ClearCacheListener.php (3)
src/Cms/Events/PostPublishedEvent.php (2)
PostPublishedEvent(13-23)getName(19-22)src/Cms/Events/PostDeletedEvent.php (2)
PostDeletedEvent(12-22)getName(18-21)src/Cms/Events/CategoryUpdatedEvent.php (2)
CategoryUpdatedEvent(13-23)getName(19-22)
src/Cms/Listeners/SendWelcomeEmailListener.php (3)
src/Cms/Events/UserCreatedEvent.php (1)
UserCreatedEvent(13-23)src/Cms/Services/Email/Sender.php (5)
Sender(17-271)to(40-44)subject(67-71)template(105-132)send(137-197)src/Cms/Models/User.php (1)
getUsername(70-73)
resources/views/home/index.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
resources/views/admin/tags/index.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Tag.php (2)
getSlug(63-66)getId(29-32)
tests/Cms/Services/Category/UpdaterTest.php (2)
src/Cms/Models/Category.php (1)
Category(13-186)src/Cms/Services/Category/Updater.php (2)
Updater(18-80)update(37-65)
src/Cms/Services/Category/Deleter.php (3)
src/Cms/Events/CategoryDeletedEvent.php (2)
CategoryDeletedEvent(12-22)__construct(14-16)src/Cms/Services/User/Deleter.php (2)
Deleter(16-55)delete(32-54)src/Cms/Controllers/Admin/CategoryController.php (1)
__construct(33-45)
src/Cms/Events/PostCreatedEvent.php (7)
src/Cms/Models/Post.php (1)
Post(13-516)src/Cms/Events/CategoryCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/CategoryDeletedEvent.php (2)
__construct(14-16)getName(18-21)src/Cms/Events/CategoryUpdatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/PostDeletedEvent.php (2)
__construct(14-16)getName(18-21)src/Cms/Events/PostPublishedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/UserCreatedEvent.php (2)
__construct(15-17)getName(19-22)
src/Cms/Events/CategoryUpdatedEvent.php (3)
src/Cms/Models/Category.php (1)
Category(13-186)src/Cms/Events/CategoryCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/CategoryDeletedEvent.php (2)
__construct(14-16)getName(18-21)
src/Cms/Services/Category/Updater.php (4)
src/Cms/Models/Category.php (1)
Category(13-186)src/Cms/Events/CategoryUpdatedEvent.php (2)
CategoryUpdatedEvent(13-23)__construct(15-17)src/Cms/Controllers/Admin/CategoryController.php (2)
__construct(33-45)update(180-203)src/Cms/Services/Category/Creator.php (1)
generateSlug(72-78)
resources/views/layouts/default.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
resources/views/admin/users/edit.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
resources/views/admin/dashboard/index.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
src/Cms/Controllers/Admin/TagController.php (3)
src/Cms/Controllers/Admin/CategoryController.php (3)
store(117-132)update(180-203)destroy(212-225)src/Cms/Controllers/Content.php (1)
redirect(237-247)src/Cms/Repositories/DatabaseTagRepository.php (3)
update(110-145)findById(37-45)delete(150-157)
src/Cms/Services/Category/Creator.php (3)
src/Cms/Models/Category.php (1)
Category(13-186)src/Cms/Events/CategoryCreatedEvent.php (2)
CategoryCreatedEvent(13-23)__construct(15-17)src/Cms/Controllers/Admin/CategoryController.php (2)
__construct(33-45)create(87-108)
src/Cms/Auth/PasswordResetManager.php (2)
src/Cms/Services/Email/Sender.php (6)
Sender(17-271)__construct(31-35)to(40-44)subject(67-71)template(105-132)send(137-197)src/Cms/Email/helpers.php (1)
tests/Cms/Auth/PasswordResetManagerTest.php (5)
src/Cms/Auth/PasswordHasher.php (2)
PasswordHasher(13-200)hash(24-30)src/Cms/Auth/PasswordResetManager.php (6)
PasswordResetManager(20-225)setTokenExpirationMinutes(60-64)requestReset(75-106)validateToken(114-126)resetPassword(136-169)cleanupExpiredTokens(176-179)src/Cms/Models/PasswordResetToken.php (1)
PasswordResetToken(14-178)src/Cms/Models/User.php (3)
User(12-459)setPasswordHash(112-116)getPasswordHash(104-107)src/Cms/Email/helpers.php (1)
src/Cms/Listeners/LogUserActivityListener.php (4)
src/Cms/Events/UserCreatedEvent.php (1)
UserCreatedEvent(13-23)src/Cms/Events/UserUpdatedEvent.php (1)
UserUpdatedEvent(13-23)src/Cms/Events/UserDeletedEvent.php (1)
UserDeletedEvent(12-22)src/Cms/Models/User.php (1)
getUsername(70-73)
resources/views/auth/login.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
src/Cms/Controllers/Auth/LoginController.php (4)
src/Cms/Auth/SessionManager.php (3)
set(96-100)getFlash(141-147)flash(132-136)src/Cms/Auth/CsrfTokenManager.php (2)
getToken(36-44)validate(49-60)src/Cms/Controllers/Content.php (2)
redirect(237-247)flash(275-278)src/Cms/Auth/AuthManager.php (1)
logout(119-140)
src/Cms/Events/CategoryDeletedEvent.php (3)
src/Cms/Events/CategoryCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/CategoryUpdatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/PostDeletedEvent.php (2)
__construct(14-16)getName(18-21)
src/Cms/Events/PostPublishedEvent.php (3)
src/Cms/Models/Post.php (1)
Post(13-516)src/Cms/Events/CategoryCreatedEvent.php (2)
__construct(15-17)getName(19-22)src/Cms/Events/PostCreatedEvent.php (2)
__construct(15-17)getName(19-22)
resources/views/admin/posts/edit.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Post.php (1)
getId(48-51)
tests/Cms/Events/EventsTest.php (12)
src/Cms/Events/UserCreatedEvent.php (2)
UserCreatedEvent(13-23)getName(19-22)src/Cms/Events/UserUpdatedEvent.php (2)
UserUpdatedEvent(13-23)getName(19-22)src/Cms/Events/UserDeletedEvent.php (2)
UserDeletedEvent(12-22)getName(18-21)src/Cms/Events/PostCreatedEvent.php (2)
PostCreatedEvent(13-23)getName(19-22)src/Cms/Events/PostPublishedEvent.php (2)
PostPublishedEvent(13-23)getName(19-22)src/Cms/Events/PostDeletedEvent.php (2)
PostDeletedEvent(12-22)getName(18-21)src/Cms/Events/CategoryCreatedEvent.php (2)
CategoryCreatedEvent(13-23)getName(19-22)src/Cms/Events/CategoryUpdatedEvent.php (2)
CategoryUpdatedEvent(13-23)getName(19-22)src/Cms/Events/CategoryDeletedEvent.php (2)
CategoryDeletedEvent(12-22)getName(18-21)src/Cms/Models/User.php (3)
User(12-459)setUsername(78-82)getUsername(70-73)src/Cms/Models/Post.php (1)
Post(13-516)src/Cms/Models/Category.php (1)
Category(13-186)
src/Cms/Services/User/Updater.php (6)
src/Cms/Models/User.php (4)
User(12-459)setPasswordHash(112-116)setUsername(78-82)setRole(129-133)src/Cms/Auth/PasswordHasher.php (4)
PasswordHasher(13-200)meetsRequirements(53-86)getValidationErrors(91-121)hash(24-30)src/Cms/Events/UserUpdatedEvent.php (1)
UserUpdatedEvent(13-23)src/Cms/Services/Category/Updater.php (2)
Updater(18-80)update(37-65)src/Cms/Controllers/Admin/ProfileController.php (2)
__construct(30-41)update(84-129)src/Cms/Controllers/Admin/UserController.php (2)
__construct(34-47)update(184-208)
resources/views/admin/categories/edit.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Category.php (1)
getId(30-33)
tests/Cms/Services/User/UpdaterTest.php (5)
src/Cms/Models/User.php (7)
User(12-459)setUsername(78-82)setRole(129-133)setPasswordHash(112-116)getUsername(70-73)getRole(121-124)getPasswordHash(104-107)src/Cms/Auth/PasswordHasher.php (1)
PasswordHasher(13-200)src/Cms/Services/User/Updater.php (2)
Updater(18-77)update(43-76)src/Cms/Controllers/Admin/ProfileController.php (1)
update(84-129)src/Cms/Controllers/Admin/UserController.php (1)
update(184-208)
resources/views/admin/categories/index.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Category.php (2)
getSlug(64-67)getId(30-33)
resources/views/admin/tags/edit.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/Tag.php (1)
getId(29-32)
resources/views/admin/users/index.php (2)
src/Cms/View/helpers.php (1)
route_path(70-79)src/Cms/Models/User.php (1)
User(12-459)
tests/Cms/Services/User/CreatorTest.php (3)
src/Cms/Models/User.php (2)
User(12-459)isEmailVerified(195-198)src/Cms/Auth/PasswordHasher.php (1)
PasswordHasher(13-200)src/Cms/Services/User/Creator.php (2)
Creator(19-77)create(43-76)
src/Cms/Controllers/Admin/PostController.php (4)
src/Cms/Auth/CsrfTokenManager.php (1)
CsrfTokenManager(13-69)src/Cms/Controllers/Content.php (2)
getSessionManager(219-227)redirect(237-247)src/Cms/Auth/SessionManager.php (2)
destroy(69-91)getId(172-176)src/Cms/Services/Post/Deleter.php (1)
delete(30-38)
src/Cms/Controllers/Admin/UserController.php (8)
src/Cms/Services/User/Creator.php (2)
Creator(19-77)create(43-76)src/Cms/Services/User/Updater.php (2)
Updater(18-77)update(43-76)src/Cms/Services/User/Deleter.php (2)
Deleter(16-55)delete(32-54)src/Cms/Auth/PasswordHasher.php (1)
PasswordHasher(13-200)src/Cms/Auth/CsrfTokenManager.php (1)
CsrfTokenManager(13-69)src/Cms/Controllers/Content.php (2)
getSessionManager(219-227)redirect(237-247)src/Cms/Auth/SessionManager.php (4)
getFlash(141-147)get(105-109)destroy(69-91)getId(172-176)src/Cms/Repositories/DatabaseUserRepository.php (4)
create(89-130)findById(37-45)update(135-188)delete(193-199)
resources/views/layouts/admin.php (1)
src/Cms/View/helpers.php (1)
route_path(70-79)
src/Cms/Controllers/Admin/ProfileController.php (6)
src/Cms/Services/User/Updater.php (2)
Updater(18-77)update(43-76)src/Cms/Auth/CsrfTokenManager.php (2)
CsrfTokenManager(13-69)getToken(36-44)src/Cms/Controllers/Content.php (2)
getSessionManager(219-227)redirect(237-247)src/Cms/Auth/SessionManager.php (2)
set(96-100)getFlash(141-147)src/Cms/Repositories/DatabaseUserRepository.php (1)
update(135-188)src/Cms/Auth/PasswordHasher.php (1)
verify(35-38)
src/Cms/Controllers/Admin/CategoryController.php (6)
src/Cms/Models/Category.php (1)
Category(13-186)src/Cms/Services/Category/Creator.php (2)
Creator(18-79)create(36-64)src/Cms/Services/Category/Updater.php (2)
Updater(18-80)update(37-65)src/Cms/Services/Category/Deleter.php (2)
Deleter(16-55)delete(32-54)src/Cms/Repositories/DatabaseCategoryRepository.php (5)
DatabaseCategoryRepository(19-229)create(98-128)update(133-170)findById(37-45)delete(175-182)src/Cms/Controllers/Content.php (1)
redirect(237-247)
🪛 LanguageTool
versionlog.md
[style] ~5-~5: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...il system with PHPMailer integration. * Added EmailService class. * Added email helpe...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
docs/EMAIL_TEMPLATES.md
36-36: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
87-87: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
150-150: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
391-391: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 PHPMD (2.15.0)
src/Cms/Controllers/Home.php
26-26: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
26-26: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/TagController.php
108-108: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
216-216: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Auth/LoginController.php
93-93: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
136-136: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/PostController.php
159-159: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/UserController.php
117-117: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/ProfileController.php
84-84: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/CategoryController.php
117-117: Avoid unused parameters such as '$parameters'. (undefined)
(UnusedFormalParameter)
212-212: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/Cms/Services/User/CreatorTest.php (3)
89-199: Consider reducing test duplication.The four tests (admin role, editor role, active status, email verified) share identical mock setup patterns. Consider refactoring with a data provider or extracting a helper method for the common mock configuration.
Example with data provider:
/** * @dataProvider userPropertyProvider */ public function testSetsUserProperty(string $property, $expectedValue, callable $getter): void { $this->_mockPasswordHasher->method('meetsRequirements')->willReturn(true); $this->_mockPasswordHasher->method('hash')->willReturn('hashed_password'); $this->_mockUserRepository ->expects($this->once()) ->method('create') ->willReturnArgument(0); $result = $this->_creator->create('user', 'user@example.com', 'Pass123!', User::ROLE_SUBSCRIBER); $this->assertEquals($expectedValue, $getter($result)); } public function userPropertyProvider(): array { return [ 'active status' => ['status', User::STATUS_ACTIVE, fn($u) => $u->getStatus()], 'email verified' => ['emailVerified', true, fn($u) => $u->isEmailVerified()], ]; }
12-27: Consider testing event emission behavior.The Creator service emits a UserCreatedEvent after successful creation (per src/Cms/Services/User/Creator.php lines 56-62), but this behavior isn't covered by any test. Consider adding a test that verifies the event is emitted, or document if event emission is tested elsewhere.
If you'd like to test the event emission in unit tests, you would need to mock the Registry or refactor Creator to accept an optional EventEmitter dependency. Alternatively, this behavior could be covered by integration tests.
12-199: Consider adding edge case tests.The test suite focuses on happy paths and password validation but doesn't cover:
- Repository failure scenarios (e.g., database constraint violations)
- Invalid role values (if validation is Creator's responsibility)
- Empty or malformed username/email inputs
Adding tests for these scenarios would increase confidence in error handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Cms/Services/User/Updater.php(1 hunks)tests/Cms/Services/User/CreatorTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Cms/Services/User/Updater.php
🧰 Additional context used
🧬 Code graph analysis (1)
tests/Cms/Services/User/CreatorTest.php (3)
src/Cms/Models/User.php (5)
User(12-459)getUsername(70-73)getPasswordHash(104-107)getRole(121-124)isEmailVerified(195-198)src/Cms/Auth/PasswordHasher.php (1)
PasswordHasher(13-200)src/Cms/Services/User/Creator.php (2)
Creator(19-77)create(43-76)
🔇 Additional comments (3)
tests/Cms/Services/User/CreatorTest.php (3)
1-27: LGTM! Clean test setup.The test class structure and setUp method follow PHPUnit best practices, properly mocking dependencies and initializing the service under test.
29-64: LGTM! Comprehensive happy-path test.This test thoroughly verifies that Creator properly constructs a User with all required fields (username, email, hashed password, role, status, email verification, and timestamp) and persists it via the repository.
66-87: LGTM! Exception handling test is correct.The test properly validates the exception behavior for invalid passwords, and the regex pattern correctly handles the detailed error message format from Creator.
Co-authored-by: ljonesfl <1099983+ljonesfl@users.noreply.github.com>
Add validation to prevent empty username/email in UserController update
Continuing refactoring to introduce a more service centric implementation. Moved system emails into templates and introduced more system events.
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests