-
Notifications
You must be signed in to change notification settings - Fork 1
[Website Settings]: Update fails for type Text and field data #1215
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
[Website Settings]: Update fails for type Text and field data #1215
Conversation
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.
Pull Request Overview
Adds a custom denormalizer to correctly map data
payloads for website settings updates and registers it as a serializer service.
- Introduces
WebsiteSettingsUpdateDenormalizer
to handle array, string, and boolean payloads fordata
- Tags the new denormalizer in
config/website_settings.yaml
- Cleans up minor formatting in the update controller
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/WebsiteSetting/Request/WebsiteSettingsUpdateDenormalizer.php | New denormalizer class that constructs WebsiteSettingsUpdate from request payloads |
src/WebsiteSetting/Controller/UpdateController.php | Removed an extra blank line |
config/website_settings.yaml | Registered the new denormalizer service tag |
Comments suppressed due to low confidence (2)
src/WebsiteSetting/Request/WebsiteSettingsUpdateDenormalizer.php:43
- The
denormalize()
method can throwInvalidArgumentException
; please add a@throws InvalidArgumentException
annotation to the docblock to document this behavior.
public function denormalize(
src/WebsiteSetting/Request/WebsiteSettingsUpdateDenormalizer.php:1
- This new denormalizer logic isn’t covered by existing tests. Please add unit tests in
tests/Unit
for cases wheredata
is an array, string, boolean, and invalid type to verify expected behavior.
<?php
src/WebsiteSetting/Request/WebsiteSettingsUpdateDenormalizer.php
Outdated
Show resolved
Hide resolved
|
Changes in this pull request
Resolves #1210
Additional info
Symfony maps data always to the object (ElementParameter) when using MapRequestPayload.
Introduced custom denormalizer to tackle this issue.