New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[com_ajax] to support templates #16532

Merged
merged 12 commits into from Aug 20, 2017

Conversation

@bassmanpaul
Contributor

bassmanpaul commented Jun 5, 2017

See this discussion for why template support is needed in com_ajax.

I have basically copied the module logic which requires a helper file in the root of the extension with specific class & method names.

See the below code to quickly test support for this from the front end & the back end.

Front end test:

templates/protostar/helper.php:

class tplProtostarHelper
{
	public static function getAjax()
	{
		return __METHOD__ . "\n\n" . JText::_( 'TPL_PROTOSTAR_XML_DESCRIPTION' );
	}

	public static function mySuperAwesomeMethodToTriggerAjax()
	{
		return __METHOD__ . "\n\n" . JText::_( 'TPL_PROTOSTAR_XML_DESCRIPTION' );
	}
}

Ajax call test:

// Default method:
jQuery.ajax( 'index.php?option=com_ajax&template=protostar&format=json' );

// Custom method:
jQuery.ajax( 'index.php?option=com_ajax&template=protostar&format=json&method=mySuperAwesomeMethodToTrigger' );

Expected results:

  • When called from the front end in protostar template: ✔ success.
  • When called from the front end in another template: ✖ unauthorised.
  • When called from the back end in any template: ✔ success.

Back end admin test:

templates/hathor/helper.php:

class tplHathorHelper
{
	public static function getAjax()
	{
		return __METHOD__ . "\n\n" . JText::_( 'TPL_HATHOR_XML_DESCRIPTION' );
	}

	public static function mySuperAwesomeMethodToTriggerAjax()
	{
		return __METHOD__ . "\n\n" . JText::_( 'TPL_HATHOR_XML_DESCRIPTION' );
	}
}

Ajax call test:

// Default method:
jQuery.ajax( 'index.php?option=com_ajax&template=hathor&format=json' );

// Custom method:
jQuery.ajax( 'index.php?option=com_ajax&template=hathor&format=json&method=mySuperAwesomeMethodToTrigger' );

Expected results:

  • When called from the front end in any template: ✖ unauthorised.
  • When called from the back end in any template: ✔ success.

bassmanpaul added some commits Jun 5, 2017

Open up access to back end templates
Allow templates to make calls when they aren't the current menu item. 

Aka when viewing Hathor's config screen, allow it to make ajax calls even though isis is the current active admin template.
Allow back end calls to look up front end template
e.g. Allow the back end the template manager's configuration page (e.g. protostar) to make com_ajax calls.
@bassmanpaul

This comment has been minimized.

Show comment
Hide comment
@bassmanpaul

bassmanpaul Jun 5, 2017

Contributor

When then is implemented the documentation will also need updating:

https://docs.joomla.org/Using_Joomla_Ajax_Interface

Contributor

bassmanpaul commented Jun 5, 2017

When then is implemented the documentation will also need updating:

https://docs.joomla.org/Using_Joomla_Ajax_Interface

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Jun 5, 2017

Contributor

I wouldn't restrict it to the template of the current menuitem. I don't see a reason for that restriction. For modules we also only check for the published state and not if it's actually visible.
I would just check if the template is published.

Contributor

Bakual commented Jun 5, 2017

I wouldn't restrict it to the template of the current menuitem. I don't see a reason for that restriction. For modules we also only check for the published state and not if it's actually visible.
I would just check if the template is published.

@bassmanpaul

This comment has been minimized.

Show comment
Hide comment
@bassmanpaul

bassmanpaul Jun 6, 2017

Contributor

Done.

Is retrieving the template published state best accessed through JTable::getInstance('extension');?

Contributor

bassmanpaul commented Jun 6, 2017

Done.

Is retrieving the template published state best accessed through JTable::getInstance('extension');?

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Jun 6, 2017

Contributor

Since we don't seem to have a JTemplateHelper::isEnabled method (like we have for components, plugins and modules), I would probably just use a direct database query. But using the table should work as well.

Contributor

Bakual commented Jun 6, 2017

Since we don't seem to have a JTemplateHelper::isEnabled method (like we have for components, plugins and modules), I would probably just use a direct database query. But using the table should work as well.

@bassmanpaul

This comment has been minimized.

Show comment
Hide comment
@bassmanpaul

bassmanpaul Jun 6, 2017

Contributor

Yeah, I did wonder whether it would be helpful to suggest creating a Template Helper as modules, plugins & languages all have their own helpers & classes.

I've used:

$table		= JTable::getInstance('extension');
$templateid	= $table->find(array('type' => 'template', 'element' => $template));

if ($templateid && $table->load($templateid) && $table->enabled)
...
Contributor

bassmanpaul commented Jun 6, 2017

Yeah, I did wonder whether it would be helpful to suggest creating a Template Helper as modules, plugins & languages all have their own helpers & classes.

I've used:

$table		= JTable::getInstance('extension');
$templateid	= $table->find(array('type' => 'template', 'element' => $template));

if ($templateid && $table->load($templateid) && $table->enabled)
...
@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Jun 6, 2017

Contributor

For consistency it would make sense, yes. Not sure how useful it will be.

Contributor

Bakual commented Jun 6, 2017

For consistency it would make sense, yes. Not sure how useful it will be.

bassmanpaul added some commits Jun 6, 2017

Update indentation syntax
As requested by @matrikular.
Revisit how front-end template is accessed from back-end
Look to the client_id to determine where the template is rather than using fallbacks.
@matrikular

This comment has been minimized.

Show comment
Hide comment
@matrikular

matrikular Jun 6, 2017

Contributor

I have tested this item successfully on 8bbf94d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16532.

Thanks for your effort and patience.

Contributor

matrikular commented Jun 6, 2017

I have tested this item successfully on 8bbf94d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16532.

Thanks for your effort and patience.

@bassmanpaul

This comment has been minimized.

Show comment
Hide comment
@bassmanpaul

bassmanpaul Jun 9, 2017

Contributor

What's the next step for this feature? Do I need to run more tests or is it pending the next Joomla release?

Contributor

bassmanpaul commented Jun 9, 2017

What's the next step for this feature? Do I need to run more tests or is it pending the next Joomla release?

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Jun 9, 2017

Contributor

Usually, we want two people testing a PR. Your own test will not count as we assume you tested your own code prior to submitting it 😄

When those tests are done, it's the call of the release leader to include it into the next release. earliest possible release will be 3.8.0 since it is a new feature.

Contributor

Bakual commented Jun 9, 2017

Usually, we want two people testing a PR. Your own test will not count as we assume you tested your own code prior to submitting it 😄

When those tests are done, it's the call of the release leader to include it into the next release. earliest possible release will be 3.8.0 since it is a new feature.

@bassmanpaul

This comment has been minimized.

Show comment
Hide comment
@bassmanpaul

bassmanpaul Jun 9, 2017

Contributor

Okay, sure. Thanks for letting me know :)

Contributor

bassmanpaul commented Jun 9, 2017

Okay, sure. Thanks for letting me know :)

@grig0ry

This comment has been minimized.

Show comment
Hide comment
@grig0ry

grig0ry Aug 19, 2017

Contributor

I have tested this item successfully on 8bbf94d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16532.

Contributor

grig0ry commented Aug 19, 2017

I have tested this item successfully on 8bbf94d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16532.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Aug 19, 2017

RTC after two successful tests.

franz-wohlkoenig commented Aug 19, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC label Aug 19, 2017

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Aug 19, 2017

Contributor

@mbabker do you want to have this in 3.8?

Contributor

zero-24 commented Aug 19, 2017

@mbabker do you want to have this in 3.8?

@grig0ry

This comment has been minimized.

Show comment
Hide comment
@grig0ry

grig0ry Aug 19, 2017

Contributor

@zero-24 It would be great.

Contributor

grig0ry commented Aug 19, 2017

@zero-24 It would be great.

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Aug 19, 2017

Contributor

It would be great.

@Grigory90 I'm not the release leader so I can't take the decision on that. Lets give Michael a bit time to decide on that.

Contributor

zero-24 commented Aug 19, 2017

It would be great.

@Grigory90 I'm not the release leader so I can't take the decision on that. Lets give Michael a bit time to decide on that.

@mbabker mbabker added this to the Joomla 3.8.0 milestone Aug 20, 2017

@mbabker mbabker merged commit 0ddd64b into joomla:staging Aug 20, 2017

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
hound No violations found. Woof!

@joomla-cms-bot joomla-cms-bot removed the RTC label Aug 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment