Skip to content

Moving JResponse to its own package and adding JResponseJson #1470

Closed
wants to merge 5 commits into from

4 participants

@Chraneco

Hi,

the handling of Ajax requests in the installer of Joomla! is imho very good and I think it's time to standardize this a bit.
This pull request introduces a new class JResponseJson which is able to simplify things with Ajax requests.

  • Using the flag success the JavaScript code (whether Mootools JRequest.JSON or jQuery.getJSON) can check whether the task was successful or not and react accordingly (If the request itself failes the error event of the JavaScript API can be used).
  • Then, the actual response data (if any) can be retrieved from data in the JSON object and
  • optionally a main response message from message.
  • Additionally all gathered messages in the JApplication message queue are also automatically sent back in messages. I'm not sure what will happen with that part because the message queue is only in the legacy application class at the moment without any replacement, but it's very useful at this point. The dependencies are checked afore.

If you have a look at the 'strings.json' controller of language override manager in the CMS you can see how it is working (it is included there already as a helper class with another name).

Another advantage is that no $app->close(); is necessary anymore if the Ajax request is done with 'format=json' because the already existing API handles the rest. Just echo the response object at the end of the task.

With this pull request the class can be added to the platform so that it can be used for any application.
For example, in the CMS it could replace the current code in the installer and the language override manager. I saw that Finder and the media manager could also be adapted for using this class. If this pull request is accepted I can standardize this in the CMS.

I'm not sure whether the unit tests are written as expected (it's the first time I coded some).

@piotr-cz

Thanks, in my opinion this class is extremely important for webapps.
I've prepared almost same class (based on com_languages/JJsonResponse) for my project and realized that I should have done so long time ago.

My only concern is whether to include application messages queue handling.
Le'ts say the client-side doesn't have an ability to render messages received in the response (or messages have been added to the queue by other part of application and are irrelevant at the moment). However using JResponseJSON (JFactory::getApplication()->getMessageQueue();) resets the queue server-side so messages will be lost. I'd add an optional argument ignoreMessages = false to the consturctor.

@Chraneco

You are right, good point. I have added this parameter.

@pasamio
pasamio commented Oct 9, 2012

This pull request can't be merged, it'll need to be rebased so it can be merged again.

@eddieajau

Can you add a reference to where this is being used in the CMS? We need to make sure this type of thing is agnostic (for example, not every platform application will necessarily have a message queue). On a technical note, I'm not sure this adds a lot to JResponse but I'd certainly like to see this class extending it if it's to be used at all. It's probably worth looking at this GSOC project for how a JSON response is handled: https://github.com/stefanneculai/Web-service-API (look at the application and the controllers in particular).

For now I'm closing this pull request because it cannot be merged cleanly and some further thought needs to be given to the strategy presented. Please rebase, push up the changes and re-open the pull request so we can review it again. Thanks.

@eddieajau eddieajau closed this Oct 9, 2012
@piotr-cz
piotr-cz commented Oct 9, 2012

@eddieajau
Good point about the message queue, default value for $ignoreMessages should be then true.

References in CMS:

In mentioned GSOC code there's no such separate class, in each controller there is similar pattern repeated to handle it.

Shortened example of controller code:

if ($result == true)
{
    $returnedContent = new stdClass;
    $returnedContent->id = $comment_id;
    $this->app->setBody(json_encode($returnedContent));
}
// Throw error
else
{
    $this->app->errors->addError('202');
    $this->app->setBody(json_encode($this->app->errors->getErrors()));
    $this->app->setHeader('status', $this->app->errors->getResponseCode(), true);
    return;
}

vs com_languages -> strings.json.php:

echo new JJsonResponse($this->getModel('strings')->search());

Of course, there are differences: Joomla JSON controllers usually echo the response directly (to avoid malformation by content plugins).

@Chraneco

It's the first time a tried to rebase a branch and I think I messed it up a bit :-)

So here is a new and cleaned pull request: #1596.

@piotr-cz
Thanks for putting all that together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.