Skip to content

Conversation

@dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Dec 29, 2022

Pull Request for Issue # .

Summary of Changes

The PR #39011 introduced namespace to the templates. This obviously solves the Fields problem but there is one more that needs to be tackled: com_ajax.
This PR is using the existing conventions but allows to load the namespaced helper

Testing Instructions

Existing functionality Front end templates

  • Add a file named helper.php in the templates\cassiopeia folder with the following content
<?php

defined('_JEXEC') || die;


class TplCassiopeiaHelper
{
    public static function templateAjax(): array
    {
        return ['is' => true, 'namespaced' => false];
    }
}
  • point your browser to index.php?option=com_ajax&method=template&format=json&template=cassiopeia and you should have a message: {"success":true,"message":null,"messages":null,"data":{"is":true,"namespaced":false}}

Proposed code

  • Add <namespace path="src">Joomla\Template\Cassiopeia</namespace> to the templateDetails.xml of Cassiopeia
  • Delete the file administrator/cache/autoload_psr4.php
  • create a file named templates/cassiopeia/src/Helper/AjaxHelper.php with the following contents:
<?php

namespace Joomla\Template\Cassiopeia\Site\Helper;

defined('_JEXEC') || die;


class AjaxHelper
{
    public static function templateAjax(): array
    {
        return ['is' => true, 'namespaced' => true];
    }
}
  • visit again the same URL as before index.php?option=com_ajax&method=template&format=json&template=cassiopeia and you should have a message: {"success":true,"message":null,"messages":null,"data":{"is":true,"namespaced":true}}

Existing functionality Back end templates

  • Add a file named helper.php in the administrator/templates/atum folder
  • Add the following content
<?php

defined('_JEXEC') || die;


class TplAtumHelper
{
    public static function templateAjax(): array
    {
        return ['is' => true, 'namespaced' => false];
    }
}
  • point your browser to administrator/index.php?option=com_ajax&method=template&format=json&template=atum and you should have a message: {"success":true,"message":null,"messages":null,"data":{"is":true,"namespaced":false}}

Proposed code

  • Add <namespace path="src">Joomla\Template\Atum</namespace> to the templateDetails.xml of the Atum template
  • Delete the file administrator/cache/autoload_psr4.php
  • create a file named administrator/templates/atum/src/Helper/AjaxHelper.php with the following contents:
<?php

namespace Joomla\Template\Atum\Administrator\Helper;

defined('_JEXEC') || die;


class AjaxHelper
{
    public static function templateAjax(): array
    {
        return ['is' => true, 'namespaced' => true];
    }
}
  • visit again the same URL as before administrator/index.php?option=com_ajax&method=template&format=json&template=atum and you should have a message: {"success":true,"message":null,"messages":null,"data":{"is":true,"namespaced":true}}

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Requesting a review from @HLeithner @laoneo @wilsonge

@SharkyKZ
Copy link
Contributor

Class names must not be hardcoded. That's why template interface is needed for something like this.

@dgrammatiko
Copy link
Contributor Author

That's why template interface is needed for something like this.

You're welcome to propose a better solution, I'm just monkey patching things here so that a namespaced template could still use the com_ajax functionality...

@HLeithner
Copy link
Member

I thought about this and think it's completely the wrong approach, the helper class has zero context. Has no idea in which application context it's running. This is the opposite as the way joomla is going.

I'm against this PR and removing the com_ajax template at some point in time. The preferred way to do this is using a plugin.
Also the module is questionable, because it's also context less. Looking at the way how webservices work I think it's better to follow a event system why and not allow to by-pass the single entry point approach.

I know I said it doesn't go away anytime soon, but that doesn't mean we have to support it in new code. I will bring this up in maintainers chat and try to get some feedback

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented Jan 1, 2023

I'm against this PR and removing the com_ajax template at some point in time. The preferred way to do this is using a plugin. Also the module is questionable, because it's also context less.

The "questionable" com_ajax for all the supported parts (plugins, modules and templates) would be nice to go away IF Joomla itself provided an easy way to produce the same output (JSON or HTML) using partials, as WP does. Joomla's ability to render partials, atm, requires an extra entry file on the active template and even in that case a component is always needed in the front end (the routing part) so rendering a module similar to com_ajax requires quite some work. The idea to remove those helpers without a viable alternative doesn't look that appealing, esp for people that don't want to write a tonne of PHP (plugins, extra entry files on the template, etc)

I know I said it doesn't go away anytime soon, but that doesn't mean we have to support it in new code.

It would be nice to either have complete namespace support (eg also the helper) or properly deprecate the thing and remove it ASAP.

In conclusion: I have code that requires com_ajax (both for modules and templates, thus the PR), so a definite answer here would be nice. Keep in mind that the helper in all 3 contexts IS a very easy escape hatch for front enders that need some server side code without going too deep creating components or plugins. Requiring a plugin for those cases pushes front enders away from the project rather than inviting them...

My 2c, and Happy new year 🎉

@dgrammatiko dgrammatiko deleted the 4.3-dev_tpl_helper branch January 15, 2023 17:10
@dgrammatiko dgrammatiko restored the 4.3-dev_tpl_helper branch July 29, 2023 12:41
@dgrammatiko dgrammatiko deleted the 4.3-dev_tpl_helper branch July 29, 2023 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants