[4.2] Introduce helper factory aware interface and convert news module to services

[laoneo]

Pull Request for pr #37398.

Summary of Changes

Introduces a helper factory aware interface and converts the articles news site module to services. For backwards compatibility a new helper class was introduced and the old abstract one got deprecated.

Testing Instructions

• Create an article
• Create an Articles - Newsflash module

Actual result BEFORE applying this Pull Request

It works.

Expected result AFTER applying this Pull Request

It works.

[dgrammatiko]

@laoneo please add a line in the xml like <element>mod_article_news</element> so the module could be installable. Also remove the old file mod_article_news.php as it is moved inside the src folder, etc

[laoneo]

@laoneo please add a line in the xml like <element>mod_article_news</element> so the module could be installable.

No module has such an element tag. Why should I add it?

[dgrammatiko]

Because #33182

[laoneo]

This should be enough 13c1cf6. #33182 is basically obsolete as it would already work without it, see here #36042 (comment). Just one entry needs a module attribute, doesn't mater which one.

[dgrammatiko]

That's an undocumented hack and probably shouldn't be promoted! The docs are clear:

https://docs.joomla.org/Manifest_files

[laoneo]

Where do you see that in action the element tag? None of my extensions does have that element and the code which uses any of the file elements is there since Joomla 1.7, which do extensions use since ages. Your pr was for 4, so I guess you added a new behavior. I wouldn't call the original way a hack. Anyway, lets open an issue where we can discuss this in more deep if you want to get that sorted.

[dgrammatiko]

I think the project should have one unique way for defining the element especially for the namespaced extensions. When I did that PR (that was before your discovery?) I thought that normalising the optional element field and move it to the required ones would be self explanatory and also if that was to be standardised then the name field could be left free to be any type of string (translatable or not). This was somehow bite me with 4.1 as the templates name field is not explicitly defined correctly, even for the core ones, (should be tmpl_cassiopeia and not cassiopeia). Anyways either keep adding attributes to a folder or explicitly asking for an element field should be standardised and needs to be noted in the docs. People shouldn't reverse engineer code to figure out how things are working. End of off topic comments

[laoneo]

Show me one extension which exists before Joomla 4 was released that uses the element tag.

[dgrammatiko]

I can't, apart from the one I edited in that PR but maybe this Unit can:

joomla-cms/tests/Unit/Libraries/Cms/Installer/Adapter/ModuleAdapterTest.php

Lines 109 to 123 in 7c12dc7

	public function testgetElementFromElementTag()
	{
	$xml = file_get_contents(JPATH_ADMINISTRATOR . '/modules/mod_quickicon/mod_quickicon.xml');
	// Insert a Joomla 4 <module/> tag
	$xml = str_replace('<name>mod_quickicon</name>', '<name>mod_quickicon</name><element>mod_quickicon</element>', $xml);
	$xml = simplexml_load_string($xml);
	$this->moduleAdapter->setManifest($xml);
	$this->assertNotNull($this->moduleAdapter->manifest);
	$this->assertEquals('mod_quickicon', $this->moduleAdapter->getElement());
	$this->assertEquals('somethingElse', $this->moduleAdapter->getElement('somethingElse'));
	}

[laoneo]

Ok, we leave the pr as it is for now.

[Ruud68]

in your commit you have created a new ArticleNews.php in the src/Helper directory.
Will that replace the ArticleNewsHelper.php (as that file now only has the deprecated message)?

Asking because if that is the case, com_ajax is broken as it hardcoded looks for a module helperfile ending in 'Helper': so ArticleNewsHelper in this example.

[laoneo]

@Ruud68 you are right. Reverted to the original helper file and made it none abstract and added a new getArticles function which is none static. What do you think?

On the other side also like this it still doesn't work with com_ajax, because the function needs to end with "Ajax" https://github.com/joomla/joomla-cms/blob/4.1-dev/components/com_ajax/ajax.php#L99.

[Ruud68]

@Ruud68 you are right. Reverted to the original helper file and made it none abstract and added a new getArticles function which is none static. What do you think?

On the other side also like this it still doesn't work with com_ajax, because the function needs to end with "Ajax" https://github.com/joomla/joomla-cms/blob/4.1-dev/components/com_ajax/ajax.php#L99.

Cool, I think this will work. As for the Ajax in the function name that is not the issue. The issue was that the helperfile wasn't loaded. When that is loaded the method is called (ending in 'Ajax') so in the helper file you need to have that function otherwise you will get an error.

This is only relevant if you have a module that will service com_ajax requests because they have functionality that relies on that (which the Joomla core modules do not do)

What still 'bugs' me is that a module following this new approach will not work on J4.1 / 4.0.
For me as extension dev that complicates things as with every dependency on a Joomla version the 'market share' will drop :S
just saying :) should not hold back improvements.

[laoneo]

If you leave the old module file from J3 and load the helper the same way in the dispatcher as the quickicon module, then you have compatibility down to J3.

[laoneo]

@Ruud68 can you give this one here a test?

[Ruud68]

@laoneo so just set up a new nightly 4.2 environment (april 5th), pulled this PR and copied the files over the new test environment.
when going to [domain]/index.php?option=com_ajax&module=articles_news&method=test&format=raw

I get the message: RuntimeException: The file at mod_articles_news/helper.php does not exist.

have not tried to troubleshoot this

[laoneo]

Did you add the function testAjax to the helper?

[Ruud68]

Hi, that is not necessary. com_ajax first tries to load the helper and when it does then it tries to load the method.
The error I got is for loading the helper > so it didn't get to the loading of the method (testAjax)

[laoneo]

It does, but when the helper has no such function it tries the old way and when that doesn't succeed, it throws the old error message. You can see it here, it sets the result to null and doesn't throw an error message. If you want to add there an error, then a new pr is required.
Important for this pr is that you test the module on the ordinary way.

[Ruud68]

sorry about that, you are completely right.
So the module works, the helper works and when adding testAjax com_ajax also works for modules in this way!

[Ruud68]

I have tested this item ✅ successfully on b081abe

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37445.}

[SumCompanyInc]

I have tested this item ✅ successfully on 3ee5c32

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37445.}

[Ruud68]

I have tested this item ✅ successfully on 3ee5c32

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37445.}

[richard67]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37445.}

[roland-d]

Thanks everybody