? Success

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
2 Oct 2013

can be a bit late, but but...

this pull request change the com_ajax to work in Joomla! way, using new MVC

and work like described http://docs.joomla.org/Xml-rpc_changes_in_Joomla!_1.6
here each format have a own controller and each "element" (plugin, module) have own controller.
It allow more simple extend this component in future, if will be need

call link changed to:
for plugins:
index.php?option=com_ajax&task=plugin.call&name=foo&format=json
index.php?option=com_ajax&task=plugin.call&name=foo&format=raw

for modules:
index.php?option=com_ajax&task=module.call&name=foo&format=json
index.php?option=com_ajax&task=module.call&name=foo&format=raw

Another formats we can add in future, by request.
Other changes:

  • response format from #1960 by @piotr-cz
  • format parameter now is required
  • own Exception handler that should fix previous problem with handling errors when "ajax" call,
  • own function for check "whether module available"
  • added content and system plugin groups

links:

avatar Fedik Fedik - open - 2 Oct 2013
avatar piotr-cz
piotr-cz - comment - 2 Oct 2013

What are reasons behind own AjaxModuleHelper instead of using native JModuleHelper and AjaxError?

Overall this feels much more right than current implementation

avatar Fedik
Fedik - comment - 2 Oct 2013

AjaxError for replace the default exception handler that Joomla! use libraries/cms/error/page.php ..
it need because, as you can see, it allow more simple handle the errors, and because default handler will render a HTML page, that no need in our case.

AjaxModuleHelper just small helper for prevent code duplication in the controllers.
Default JModuleHelper load the full modules list that enabled for a current page, that not very optimal, because for us just need to check the one module, and I not see big sense that the module should be exactly "for this page" or for "this language" ...
I think for us enough check whether module enabled and user have the access.

avatar wilsonge
wilsonge - comment - 2 Oct 2013

I have to say I'm not a massive fan of this. In this specific case I think splitting it up is creating more hassle than it's worth. I'm of the opinion for modules and plugins pretty much all we should support is json replies - and maybe raw if we are in debug mode. This seems like creating files for the sake of conforming to standards.

I support moving to JResponseJson (as in piotr's PR) but I think that's as far as we need to go.

I'd say this component will get removed as soon as we move to webservices (J5??) anyhow

avatar piotr-cz
piotr-cz - comment - 3 Oct 2013

@Fedik thanks for answers, this helps.
I'm not entirely convinced for the same reason as @wilsonge, on the other code has logic pattern now.

avatar elinw
elinw - comment - 4 Oct 2013

If we are going to do controllers let's do them new style for example just calll or callplugin, callmodule with options for format (and maybe even with options for plugin or module). The new style is designed for services and makes a lot more sense. Also use singular folder names so things autoload correctly and we don't need to mess around with includes.

avatar Fedik
Fedik - comment - 4 Oct 2013

@elinw there already task=module.call and task=plugin.call , and parameters format=raw, format=json also there.
Or I think I not very understand suggestion about "callplugin, callmodule".
Can you explain a bit more? :)

I know not to much about planed web-services, so I thought that extending the current realization by adding just a new controller in future, will be a cool idea ;)

and think, right, if do it compatible with web-services, then the current "trick" can be removed with the less pain in future when web-services will be ready ... but I know nothing about the plans around web-services :)
and whether web-services will allow interaction with the modules?

avatar betweenbrain
betweenbrain - comment - 4 Oct 2013

I appreciate the work that went into this PR, the code is well written and has some nice innovations like AjaxError. In no way do I want to discourage future contributions, but I'm also not convinced that introducing controllers is beneficial. @wilsonge is right in that it is very unlikely that we'll need to introduce other formats in the future and may introduce more maintenance than benefit.

avatar Fedik
Fedik - comment - 4 Oct 2013

I really see no problem with formats
check http://docs.joomla.org/Xml-rpc_changes_in_Joomla!_2.5
if need additional format, then just need add the new controller , and no need any changes in existing controller,

example for xml just need add module.xml.php where in the call method you can render xml, or use view for it

avatar piotr-cz
piotr-cz - comment - 4 Oct 2013

@Fedik Yes, you can do it in your own installation, but what about if you'd like to use formats like image or xml in a public extension?

avatar Fedik
Fedik - comment - 4 Oct 2013

answer is:

Another formats we can add in future, by request.

it can be added without big problem if it will be a really need ;)

but combination plugin + format=image can be problem (and maybe for other formats too), because the dispatcher can return multiple values ... but it a question how to call the plugin(s)

avatar Fedik
Fedik - comment - 5 Oct 2013

ok, added xml and image formats.
right, looks a bit "over coded" :)

avatar betweenbrain
betweenbrain - comment - 22 Nov 2013

Would it be safe to say that we can close this issue as there isn't consensus to the changes it introduces?

avatar Fedik
Fedik - comment - 1 Dec 2013

ok, changed to using new MVC,
and without change the request format, so can be tested with existing plugins/modules that already wrote for use com_ajax

thoughts? :)

avatar Fedik
Fedik - comment - 8 Feb 2014

new pull to staging #3071

avatar Fedik Fedik - close - 8 Feb 2014

Add a Comment

Login with GitHub to post a comment