[4.2] Load the actionlog params from model instead of the helper

[laoneo]

Summary of Changes

Move the logic to fetch the log content type params of the action log component to a model. Models should do the database calls as they have access to it. Like that it is possible to inject the database object and it is not necessary to do a deprecated Factory::getDbo() call. This helps to write proper unit tests which improves the sustainability. Having the code tested with unit tests ensures that errors in further changes will be detected and potential BC breaks can be eliminated. This is only a small step but we need to head into that direction when we want to have a system where we can write easy system tests without a bunch of preparation setup.

Additionally it makes a small modification to the extension namespace mapper so it can be loaded during the bootstrap process of the unit test suite, similar to the libraries.

Testing Instructions

• Make sure the Joomla action log plugin is loaded
• Save an article

Actual result BEFORE applying this Pull Request

An entry is written to the user action log that an article is saved.

Expected result AFTER applying this Pull Request

An entry is written to the user action log that an article is saved.

[laoneo]

@brianteeman this is my idea how it should be when interacting with a component. The helpers should be replaced with models.

[brianteeman]

@brianteeman this is my idea how it should be when interacting with a component. The helpers should be replaced with models.

Without any explanation of why :(

[nibra]

Looks like a step towards better architecture to me. However, I would get the model in the constructor so that the (relatively) complex call only occurs once.

[nibra]

Without any explanation of why :(

That's not necessarily required, as there are a few obvious reasons.
a) Only models should ever interact with the persistence layer
b) Whenever you're tempted to name a class 'Helper', you're doing wrong

[laoneo]

Looks like a step towards better architecture to me. However, I would get the model in the constructor so that the (relatively) complex call only occurs once.

The drawback is that the model is then always loaded even when no function is called.

[brianteeman]

That's not necessarily required, as there are a few obvious reasons.

obvious to you but not everyone.

Remember that there is an entire ecosphere of extension deevlopers whose only knowledge of php etc is from the joomla core. Whenever there is an architecture change it should be fully documented and explained as a reference for them AND for future contributors to the core.

For example if naming a class "helper" is doing it wrong why are there any instances of this? If it is so obviously wrong then that code should have never been meerged. Without any explanation then you are keeping secret the reason why and only an inner clique will ever be able to contribute and no one else will bee able to learn

[laoneo]

I'v updated the description, is it more clear now?

[nibra]

The drawback is that the model is then always loaded even when no function is called.

Then the whole call should be moved to a private method getConfigParams() or the like.

[nibra]

Remember that there is an entire ecosphere of extension deevlopers whose only knowledge of php etc is from the joomla core. Whenever there is an architecture change it should be fully documented and explained as a reference for them AND for future contributors to the core.

Yes, agree. Actually that's one of the reasons for creating the RFC process (which needs to get more attention, but that's another story)

[nibra]

if naming a class "helper" is doing it wrong why are there any instances of this? If it is so obviously wrong then that code should have never been meerged.

Not everyone gives clean architecture the love it deserves. Helper' would be a reasonable name if one could say what one purpose a helper serves - after all, each class should serve only one purpose. But if it does, a more descriptive name for that class can always be found. Therefore, the name 'Helper' is a so-called code-smell.

[roland-d]

Thank you