-
Notifications
You must be signed in to change notification settings - Fork 10
JSON view overrides response content #203
base: master
Are you sure you want to change the base?
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.
@jebbdomingo can you try my suggestion please?
code/view/json.php
Outdated
@@ -135,6 +135,12 @@ protected function _actionRender(ViewContext $context) | |||
*/ | |||
protected function _fetchData(ViewContext $context) | |||
{ | |||
$content = $this->getContent(); |
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.
$content = $this->getContent(); | |
$content = $context->content; |
@jebbdomingo can you try the above suggestion? It should play better with other behaviors that could affect the content
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.
@ercanozkaya $context->content
is null
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.
@jebbdomingo Can you please show me the code in Docman that is affected by this so that I can properly check it out ?
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.
@johanjanssens this is an interesting one that @jebbdomingo caught. If you set the response directly in your controller like this:
ViewJson should only act on it right? Can you review the PR please? |
@ercanozkaya @jebbdomingo Trying to follow the logic for this one, we are setting string content in the response using json, Json view is called, additional question would be since the content is already a string do we still need to call the view, or just bypass that alltogether? |
We could still run it through ViewJson::render so that everything would go through the same route. We could also not support this at all and require Response::send() to be called explicitly in the controller. But that might lead to early termination. I'm good either way |
@ercanozkaya These is an issue here, application/json is not the mimetype for our build in json view, this should be application/vnd.api+json as per spec. Pushing it to the view for handling wouldn't be really correct. Could you explain why you are returnig json here, is this to support the uploader? You are sending a json encoded redirect as part of a payload after a POST request? Something similar is happening here: https://github.com/joomlatools/foliolabs-docman/blob/465714568990d3ea546b66edbad31784aeb135f0/code/site/controller/submit.php#L210 This controllers seems to support both json and html? |
@jebbdomingo Discussed this with Ercan will be working on a better and more flexible solution for this in the framework. In the mean time suggest you override the json view in DOCman and apply the fix you proposed here. That way things will work and we don't need to make Kodekit changes. |
Closes #202