[5.2] Initial thoughts on v5 View structure (Part 2)

[wilsonge]

Continuation of #38931 after me making a major mistake and pushing to the wrong remote.

OK So I had a bit of time on a train this morning and the stuff in #38926 prompted me to have a think about what would actually be involved in removing the View package's CMSObject dependencies and also moving it towards a better semblance of DI whilst we were at it for a v5.0 package by removing the config object in favour of child classes directly setting things in their own constructors - none of these properties are actually things that the controller really cares about. Finally I've removed the concept of helper classes as I can't see any use of these in core for like 10 years or more.

This is purely PoC for discussion. I'm know the branch tests will fail because I've not made the relevant changes to the MVCFactory to build the view with the new constructor. This is purely about opening a conversation with the aim of requirements gathering for what we do and don't need in the other PR to get us to this as a proposed end state. Some of this stuff will end up being backported into the 4.x series to give us b/c, docs will be needed in the manual. Again this is a PoC for discussion not something intended to be merged today or tomorrow.

I've been reasonably aggressive with my constructor refactoring here. I'm sure I'm going to get shouted down a bit by @nikosdion to be more practical and that's fine. I completely accept there will need to be revisions based on this (please don't kill me too much :D )

[nikosdion]

So, to sum up what I said in the previous PR:

• We need a DocumentAware Interface which declares getDocument and setDocument
• We need a DocumentAwareTrait which implements these two methods
• The MVCFactory needs to implement the DocumentAware interface and use the trait
• The AbstractView needs to implement the DocumentAware interface and use the trait
• The MVCFactory service provider needs to push the document to the MVCFactory instance
• The MVCFactory instance needs to push the document to the View instance in createView

Together with what I am seeing in the PR this would strike a good balance between b/c and forwards-looking, cleaner code. We no longer need to push a document from the Controller unless we are trying to do something weird. The document drips from the DI Container via the component service provider, MVCFactory service provider and MVCFactory itself into the View. The View also has a setDocument method making it (more) testable. The magic getter and setter in the View ensure nothing breaks in 5.x but does complain that the legacy code must go away. Everyone's happy and Joomla moves forward. Happy times all around.

[wilsonge]

MVCFactory is more tricky because the legacy factory has to be taken into account too. I'd rather leave that for another PR I think and have done the stuff on the abstractview and left the injection in the controller for now

[nikosdion]

if ($view instanceof DocumentAwareInterface) {
   $view->setDocument($document);
}

Fully b/c. We are using this pattern already in Joomla 4.2, IIRC to push the application to the Controller (off the top of my head, I didn't check the codebase, I'm about to go to bed).

[wilsonge]

Yeah yeah I get it - if you look I updated the controller code to that initially. I can add it in addition to the mvc factory. But legacy mvc factory wouldn’t have it which makes removing it from the controller (which was our aim hard). I could probably drop it in both classes but it’s not clear to me if there would be other issues - which is why I’m nervy about moving that part around - and you know I’m generally more liberal about changes than you are 😂

[wilsonge]

There's a few missing class properties that this latest code identified beyond the layouts which i've just direct backported to 4.3 with #38939

[HLeithner]

We need to add the __set and __get intro 4.3 for deprecation warning, maybe with the same pr #38939

Also is it possible to introduce a legacy class which could be loaded by the b/c plugin which has the __set, __get (cmsobject) functions and overloads the default view?

[HLeithner]

@wilsonge can you fix the merge conflicts please

[wilsonge]

Done. Because we've merged the DocumentAware Interface and trait into 4.4 since I wrote this some of the changes around where we call getDocument() look very trivial - but that's just cause me and Allon both put them into different places when writing the code independently. There's still some other changes in here (e.g. the reclassing of the CategoryFeed parent) and also removing the concept of helpers.

[HLeithner]

This pull request has been automatically rebased to 5.1-dev.

[HLeithner]

This pull request has been automatically rebased to 5.2-dev.