Refactor Module Templates to use DTOs in Services and Controllers#444
Merged
armanist merged 3 commits intosoftberg:masterfrom Apr 2, 2026
Conversation
📝 WalkthroughWalkthroughIntroduces PostDTO, CommentDTO, and UserDTO templates and updates controllers and services across DemoApi and DemoWeb templates to construct and pass DTOs (via factories/constructors) instead of assembling raw arrays; service signatures now accept DTOs and use DTO->toArray() when persisting. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant DTO as "DTO (Post/Comment/User)"
participant Service
participant ModelDB as "Model / DB"
Client->>Controller: HTTP request (validated)
Controller->>DTO: DTO::fromRequest(request, ..., ...) / new DTO(...)
DTO-->>Controller: DTO instance
Controller->>Service: addXxx(DTO) / updateXxx(uuid, DTO)
Service->>ModelDB: create() or find(uuid)
Service->>ModelDB: fill(array_merge(['uuid'=>...], DTO.toArray())) or fill(DTO.toArray())
ModelDB-->>Service: persisted model
Service-->>Controller: persisted model / array
Controller-->>Client: JSON response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #444 +/- ##
=========================================
Coverage 82.58% 82.58%
Complexity 2831 2831
=========================================
Files 243 243
Lines 7545 7545
=========================================
Hits 6231 6231
Misses 1314 1314 ☔ 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: 1
🧹 Nitpick comments (5)
src/Module/Templates/DemoWeb/src/DTOs/PostDTO.php.tpl (1)
69-77: Inconsistent request data handling compared to CommentDTO.
PostDTO::fromRequest()uses$request->get('title', null, true)withraw=true, bypassing framework sanitization. Meanwhile,CommentDTO::fromRequest()appliestrim()without the raw flag. This inconsistency may be intentional (e.g., posts allow HTML content) but should be documented or standardized.If raw input is intentional for posts, consider adding a brief doc comment explaining why. If not, consider aligning the approach:
Option: Apply trim like CommentDTO
public static function fromRequest(Request $request, ?string $userUuid = null, ?string $image = null): self { return new self( - $request->get('title', null, true), - $request->get('content', null, true), + trim($request->get('title', '')), + trim($request->get('content', '')), $userUuid, $image ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/DTOs/PostDTO.php.tpl` around lines 69 - 77, PostDTO::fromRequest currently calls $request->get('title', null, true) and $request->get('content', null, true) which bypasses framework sanitization unlike CommentDTO::fromRequest that trims input; either (1) document intent by adding a brief docblock to PostDTO::fromRequest stating that raw=true is intentional because posts may include HTML and must be stored raw, or (2) standardize behavior by removing the raw=true and applying trim() (as in CommentDTO) to title/content before constructing the DTO; locate the method named fromRequest in class PostDTO and implement one of these two options (docblock explaining raw input or change to trimmed/sanitized input) to resolve the inconsistency.src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl (1)
84-98: Verify intent: empty string vs null for image increate.When no image is uploaded,
$imageNameis set to''(empty string). SincePostDTO::toArray()only filtersnullvalues,'image' => ''will be included and persisted to the database.If the intent is to store no image, this works but stores an empty string rather than
NULL. If storingNULLis preferred when no image exists, initialize$imageNametonullinstead:Optional: Use null for no image
public function create(Request $request) { - $imageName = ''; + $imageName = null; if ($request->hasFile('image')) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl` around lines 84 - 98, The create method sets $imageName = '' which causes PostDTO::fromRequest (and later PostDTO::toArray()) to persist an empty string for 'image' instead of NULL; change the initialization to null and ensure the branch that assigns from $this->postService->saveImage still returns a string (or null) so that when no file is uploaded $imageName remains null and the image field is omitted/persisted as NULL; update references in create, the $imageName variable, and validate saveImage behavior in PostService to return null when no image is saved.src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tpl (2)
93-103:fromRequest()does not populate theimageproperty.The factory method doesn't set
image, so it defaults to''. SincetoArray()filters empty strings,imagewill never appear in the output array when usingfromRequest(). If the auth service or model expects animagekey, it will be missing.Verify this is the intended behavior for signup flows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tpl` around lines 93 - 103, The fromRequest factory in UserDTO currently doesn't set the image property, so instances created via UserDTO::fromRequest(...) leave image as '' and toArray() filters it out; update UserDTO::fromRequest to read and pass the image value from the incoming Request (e.g., $request->get('image') or null/optional) into the constructor so the image property is populated consistently with how toArray() and the auth/model layers expect it; ensure the parameter/order matches the UserDTO constructor signature and handle missing values (null or empty) the same way other callers do.
144-157: InconsistenttoArray()filtering compared toPostDTO.
UserDTO::toArray()filters out bothnulland empty string ('') values, whilePostDTO::toArray()only filtersnull. This inconsistency could cause subtle behavioral differences:
- In
UserDTO: emptyroleorimagewill be excluded from the array- In
PostDTO: empty string values are retainedConsider aligning the filtering strategy across all DTOs for predictable behavior.
Option A: Filter only null (match PostDTO)
public function toArray(): array { return array_filter([ 'uuid' => $this->uuid, 'email' => $this->email, 'password' => $this->password, 'firstname' => $this->firstname, 'lastname' => $this->lastname, 'role' => $this->role, 'image' => $this->image, ], function ($value) { - return $value !== null && $value !== ''; + return $value !== null; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tpl` around lines 144 - 157, UserDTO::toArray currently removes both null and empty string values (the closure returns $value !== null && $value !== ''), causing behavior drift from PostDTO::toArray which only filters nulls; update the closure in UserDTO::toArray (in src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tpl) to only exclude nulls (i.e., return $value !== null) so empty strings like role or image remain consistent with PostDTO::toArray.src/Module/Templates/DemoApi/src/DTOs/PostDTO.php.tpl (1)
69-77: Consider documenting the third parameter behavior.The
fromRequest()method passestrueas the third argument to$request->get(). This likely enables HTML entity decoding or sanitization. A brief docblock note would clarify this behavior for future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/DTOs/PostDTO.php.tpl` around lines 69 - 77, The fromRequest() method passes true as the third argument to $request->get() for the 'title' and 'content' fields; add a short docblock above fromRequest() explaining what that third parameter does (e.g., enables HTML entity decoding or sanitization/stripping depending on Request::get implementation), how it affects the returned values for $title and $content, and any security considerations for storing/displaying those fields; reference the function name fromRequest and the parameters 'title' and 'content' so maintainers can quickly locate and understand the behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Module/Templates/DemoApi/src/DTOs/CommentDTO.php.tpl`:
- Around line 61-64: The fromRequest factory (CommentDTO::fromRequest) calls
trim($request->get('content')) which can receive null and throw a TypeError;
update fromRequest to defensively handle missing content by normalizing to a
string (e.g., coalesce to '' or cast to string) before calling trim and then
pass the sanitized value into the constructor (new self($postUuid, $userUuid,
$sanitizedContent)); also add/update the method docblock to state that content
will be a string even when missing.
---
Nitpick comments:
In `@src/Module/Templates/DemoApi/src/DTOs/PostDTO.php.tpl`:
- Around line 69-77: The fromRequest() method passes true as the third argument
to $request->get() for the 'title' and 'content' fields; add a short docblock
above fromRequest() explaining what that third parameter does (e.g., enables
HTML entity decoding or sanitization/stripping depending on Request::get
implementation), how it affects the returned values for $title and $content, and
any security considerations for storing/displaying those fields; reference the
function name fromRequest and the parameters 'title' and 'content' so
maintainers can quickly locate and understand the behavior.
In `@src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tpl`:
- Around line 93-103: The fromRequest factory in UserDTO currently doesn't set
the image property, so instances created via UserDTO::fromRequest(...) leave
image as '' and toArray() filters it out; update UserDTO::fromRequest to read
and pass the image value from the incoming Request (e.g., $request->get('image')
or null/optional) into the constructor so the image property is populated
consistently with how toArray() and the auth/model layers expect it; ensure the
parameter/order matches the UserDTO constructor signature and handle missing
values (null or empty) the same way other callers do.
- Around line 144-157: UserDTO::toArray currently removes both null and empty
string values (the closure returns $value !== null && $value !== ''), causing
behavior drift from PostDTO::toArray which only filters nulls; update the
closure in UserDTO::toArray (in
src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tpl) to only exclude nulls
(i.e., return $value !== null) so empty strings like role or image remain
consistent with PostDTO::toArray.
In
`@src/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tpl`:
- Around line 84-98: The create method sets $imageName = '' which causes
PostDTO::fromRequest (and later PostDTO::toArray()) to persist an empty string
for 'image' instead of NULL; change the initialization to null and ensure the
branch that assigns from $this->postService->saveImage still returns a string
(or null) so that when no file is uploaded $imageName remains null and the image
field is omitted/persisted as NULL; update references in create, the $imageName
variable, and validate saveImage behavior in PostService to return null when no
image is saved.
In `@src/Module/Templates/DemoWeb/src/DTOs/PostDTO.php.tpl`:
- Around line 69-77: PostDTO::fromRequest currently calls $request->get('title',
null, true) and $request->get('content', null, true) which bypasses framework
sanitization unlike CommentDTO::fromRequest that trims input; either (1)
document intent by adding a brief docblock to PostDTO::fromRequest stating that
raw=true is intentional because posts may include HTML and must be stored raw,
or (2) standardize behavior by removing the raw=true and applying trim() (as in
CommentDTO) to title/content before constructing the DTO; locate the method
named fromRequest in class PostDTO and implement one of these two options
(docblock explaining raw input or change to trimmed/sanitized input) to resolve
the inconsistency.
🪄 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: c5a90f30-b7e3-4fc3-a458-411e6d18d38e
📒 Files selected for processing (16)
src/Module/Templates/DemoApi/src/Controllers/AuthController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/CommentController.php.tplsrc/Module/Templates/DemoApi/src/Controllers/PostManagementController.php.tplsrc/Module/Templates/DemoApi/src/DTOs/CommentDTO.php.tplsrc/Module/Templates/DemoApi/src/DTOs/PostDTO.php.tplsrc/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tplsrc/Module/Templates/DemoApi/src/Services/CommentService.php.tplsrc/Module/Templates/DemoApi/src/Services/PostService.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/AuthController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/CommentController.php.tplsrc/Module/Templates/DemoWeb/src/Controllers/PostManagementController.php.tplsrc/Module/Templates/DemoWeb/src/DTOs/CommentDTO.php.tplsrc/Module/Templates/DemoWeb/src/DTOs/PostDTO.php.tplsrc/Module/Templates/DemoWeb/src/DTOs/UserDTO.php.tplsrc/Module/Templates/DemoWeb/src/Services/CommentService.php.tplsrc/Module/Templates/DemoWeb/src/Services/PostService.php.tpl
armanist
added
enhancement
armanist
requested review from
Arno2005,
andrey-smaelov,
charoyan88,
ematrito and
grigoryanmartin20
andrey-smaelov
approved these changes
Apr 2, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/Module/Templates/DemoWeb/src/DTOs/UserDTO.php.tpl (2)
105-138: Consider adding PHPDoc comments to getter methods for consistency.The constructor and
fromRequestmethods have PHPDoc blocks, but the getter methods lack them. Adding@returnannotations would improve consistency and IDE support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/DTOs/UserDTO.php.tpl` around lines 105 - 138, Add PHPDoc blocks with `@return` annotations to each getter (getEmail, getPassword, getFirstname, getLastname, getRole, getUuid, getImage) to match the existing constructor/fromRequest documentation style; for nullable return types use `@return` string|null on getUuid and `@return` string on the others, placing the docblock immediately above each method signature for IDE/typehint consistency.
93-103: Note:imagefield is not extracted from the request.The
fromRequestfactory method doesn't extract or pass theimagefield from the request. This appears intentional since user signup likely doesn't include image upload, but worth confirming if image should ever be populated during user creation via this factory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoWeb/src/DTOs/UserDTO.php.tpl` around lines 93 - 103, The fromRequest factory currently omits the image field when constructing UserDTO; update the fromRequest(Request $request, string $role, ?string $uuid = null): self method to extract the image (e.g., $request->get('image') or $request->file('image') as appropriate) and pass it into the UserDTO constructor (or the Image-related parameter) so the DTO's image property is populated during creation (adjust the constructor call/signature in UserDTO if needed).src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tpl (1)
93-102: Consider decoupling DTO creation from raw request accessors for improved type safety.While the
Signupmiddleware validates the request beforefromRequest()is called, the method still accepts untypedRequest::get()values that returnmixed. This creates a type contract mismatch: the constructor expects strictstringparameters, but the factory receivesmixed. For better type safety and clarity, consider accepting a validated array parameter instead, keeping transport parsing in the controller/middleware.♻️ Suggested refactor
- public static function fromRequest(Request $request, string $role, ?string $uuid = null): self + public static function fromValidated(array $data, string $role, ?string $uuid = null): self { return new self( - $request->get('email'), - $request->get('password'), - $request->get('firstname'), - $request->get('lastname'), + $data['email'], + $data['password'], + $data['firstname'], + $data['lastname'], $role, $uuid ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tpl` around lines 93 - 102, The fromRequest factory currently reads untyped values via Request::get and passes mixed into the UserDTO constructor; change fromRequest(Request $request, string $role, ?string $uuid = null): self to accept a validated array (e.g. array $validated) or a specific value-object so callers (Signup middleware / controller) pass typed strings, then map those typed values into the constructor; update the method signature and body of fromRequest and any call sites in Signup middleware/controller to use the validated array keys (email, password, firstname, lastname) instead of Request::get to restore strict string typing for the constructor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tpl`:
- Around line 93-102: The fromRequest factory currently reads untyped values via
Request::get and passes mixed into the UserDTO constructor; change
fromRequest(Request $request, string $role, ?string $uuid = null): self to
accept a validated array (e.g. array $validated) or a specific value-object so
callers (Signup middleware / controller) pass typed strings, then map those
typed values into the constructor; update the method signature and body of
fromRequest and any call sites in Signup middleware/controller to use the
validated array keys (email, password, firstname, lastname) instead of
Request::get to restore strict string typing for the constructor.
In `@src/Module/Templates/DemoWeb/src/DTOs/UserDTO.php.tpl`:
- Around line 105-138: Add PHPDoc blocks with `@return` annotations to each getter
(getEmail, getPassword, getFirstname, getLastname, getRole, getUuid, getImage)
to match the existing constructor/fromRequest documentation style; for nullable
return types use `@return` string|null on getUuid and `@return` string on the
others, placing the docblock immediately above each method signature for
IDE/typehint consistency.
- Around line 93-103: The fromRequest factory currently omits the image field
when constructing UserDTO; update the fromRequest(Request $request, string
$role, ?string $uuid = null): self method to extract the image (e.g.,
$request->get('image') or $request->file('image') as appropriate) and pass it
into the UserDTO constructor (or the Image-related parameter) so the DTO's image
property is populated during creation (adjust the constructor call/signature in
UserDTO if needed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 457d7e3e-876b-4e59-a20e-a40a37187eba
📒 Files selected for processing (2)
src/Module/Templates/DemoApi/src/DTOs/UserDTO.php.tplsrc/Module/Templates/DemoWeb/src/DTOs/UserDTO.php.tpl
armanist
deleted the
443-Implement-DTO-Pattern-for-Posts-Comments-and-Users-for-module-templates
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #443
Summary by CodeRabbit