Skip to content
This repository has been archived by the owner on Feb 4, 2022. It is now read-only.

JSON view overrides response content #203

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jebbdomingo
Copy link
Contributor

Closes #202

@jebbdomingo jebbdomingo added this to the 3.0.0-beta.2 milestone Jul 23, 2019
@jebbdomingo jebbdomingo self-assigned this Jul 23, 2019
Copy link
Contributor

@ercanozkaya ercanozkaya left a 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?

@@ -135,6 +135,12 @@ protected function _actionRender(ViewContext $context)
*/
protected function _fetchData(ViewContext $context)
{
$content = $this->getContent();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$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

Copy link
Contributor Author

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

Copy link
Contributor

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ercanozkaya
Copy link
Contributor

@johanjanssens this is an interesting one that @jebbdomingo caught. If you set the response directly in your controller like this:

$context->response->setContent(json_encode(['foo' => 'bar']), 'application/json');

ViewJson should only act on it right? Can you review the PR please?

@johanjanssens
Copy link
Member

@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?

@ercanozkaya
Copy link
Contributor

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

@johanjanssens
Copy link
Member

johanjanssens commented Jul 29, 2019

@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?

@johanjanssens
Copy link
Member

@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.

@amazeika amazeika modified the milestones: 3.0.0-beta.2, 3.0.0 Nov 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSON view overrides response content
4 participants