[#43179] - [5.3] JSONApiView: Ensure type is set
- Pending
- Medium
- Build: 4.4-dev
- # 43179
- Diff
- Hackwar:4.4-jsonapiview
- Pending continuous-integration/drone/pr Build is pending Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #37305 .
Summary of Changes
When creating a new JSONApiView object, but not handing over the optional parameter 'contentType', the following line with the JoomlaSerializer throws an error.
Testing Instructions
Codereview
Link to documentations
Please select:
-
Documentation link for docs.joomla.org:
-
No documentation changes for docs.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
The intention here was type is a required parameter. Either via injecting through content type or setting it directly in the class depending on the use case. We might be better off throwing a legit 500 to symbolize that though, the serializer is required for both displayList and displayItem - so don't think this is the fix.
In this case we should do it the same as all other object we need.
So we need a SerializerAwareInterface requres a setSerializer function and SerializerAwareTrait this implements with a private variable and a getter function.
Similar to CurrentUserInterface
This pull request has been automatically rebased to 5.2-dev.
| Title |
|
||||||
This pull request has been automatically rebased to 5.3-dev.
| Title |
|
||||||
So, what should we do with this? @HLeithner says a new trait and interface, @wilsonge rather wants to throw an exception and is against the other proposal. I think having an interface and trait for this one thing seems overkill. I'd like to move forward here and solve this or close the PR. Any opinions?
Definitely agree trait is overkill. I'd rather my solution and bail. But if not aren't we just getting an error when we call display? Because the serializer is kind of required no?
I have tested this item ✅ successfully on 9c3c54e
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43179.