\NewsModel::countPublishedPids() cannot be extended without rewriting the whole method

[timgatzky]

The 3rd. argument is the commen $arrOptions array to pass custom query options but the NewsModell::countPublishedPids() method won't rewrite the arguments in the needed format the Model::countBy() method need.

class A extends \NewsModel
{
   public static function countPublishedByPids($arrPids, $blnFeatured=null, array $arrOptions=array())
   {
      $arrOptions['column'][] = 'tl_news.id=10';
      return parent::countPublishedByPids($arrPids, $blnFeatured, $arrOptions); // won't recognize that we only want the tl_news.id=10 record
  }
}

Extending and overwriting the Models classes is another big issue. Especially when there are 2 different extensions extending e.g. the \Contao\NewsModel.
If we don't know the last class in line extending, operations might get lost.
A good callback structure like a Hook - callback with passing arguments from one to another would be great.
Maybe I'm missing a point here but I cannot figure it out.
Posted it here before:
https://community.contao.org/de/showthread.php?47626-Models-Funktionen-sicher-%FCberschreiben

[beaniemonk]

+1 on a hook to alter a model's options before a query is built.

Starting in 3.3 there appear to be two new methods in the base Model class -- 'buildCountQuery()' and 'buildFindQuery()'. (Tim, I don't think this solves everything you bring up, but it will allow you to do what's in your example).

I think these would be ideal places for a 'buildCountQuery' hook and a 'findCountQuery' hook, respectively. Each hook would pass $arrOptions and static::$strTable to the callback, and receive $arrOptions as the return value, before passing on to QueryBuilder.

This would help extensions do things like add additional filters/sorting/etc. to listing modules without having to override the model class.

Sound like a good idea?

[timgatzky]

Yup, a good place would be right at the beginning of the QueryBuilder class, before the whole query string building starts.

A simple filter callback method would be great for the Model class as well. Just pass a list of ids (array) to the Model and it fetches only the records matching the ids. Simple but effective filter.
A little like the dca filter in the backend.

True, it does not solve the inheritance issue mentioned in the forum post. I wrote a ClassFinder class (in the post is quick draft) that circles through the classes registered by the ClassLoader and finds an inheritance chain for a given class.
Would be great if the ClassLoader itself comes with a method like: findParentClass('myClass')
I know it open a couple problemes with the Registry but that can be solved. The result would be a consistent inheritance chain from all extension/classes inside Contao.

[leofeyer]

What is the initial problem here?

[timgatzky]

Ich mach das mal auf deutsch...
Problem 1 ist direkt die Model::countBy() Methode.
Diese erwartet keinen 3. Parameter um individuelle SQL Optionen ($arrOptions) durchzureichen.

Problem 2 beschreibt die allgemeine Schwierigkeit sämtliche Methoden der Models gescheit zu erweitern. Der Forums Post beschreibt es am besten. Hier die Kurzfassung.
Hat man zwei Erweiterungen, die beide z.B. die \Contao\NewsModels mit ihren statischen Methoden überschreiben, wird nur die zu letzt initialisierte Erweiterung durchlaufen. Ist klar, weil alphabetisch.
Die 2. Erw. müsste also eigentlich von der ersten erben, diese über extends erweitern. Was in einem Szenario, wo man nicht weiss welche Erweiterungen von welcher erben so gut wie unmöglich ist.
Man bräuchte also ein dynamsiches extends, damit man phps Vererbung nutzen kann und nicht jede Methode neubauen muss. Hier hilft auch kein Parameter, der durchgereicht wird, weil die Vererbungskette sämtliche Klassen zwischen Ursprungsklasse und letztem Erbe überspringt.

Daher sollte eigentlich jede Methode ein Callback bieten, oder wie hier angepsrochen, spätestens die QueryBuilder, um wirklich mehrere sql Optionen über unbestimmt viele Erweiterungen zu akzeptieren.

[leofeyer]

Problem 1 können wir gerne in der nächste Minor-Version beheben. Problem 2 ist wohl etwas umfassender und sollte vorab auf Mumble besprochen werden.

[timgatzky]

Hi Leo, ja Problem 1 kann ja direkt in einem neuen Release gefixt werden.
Das Problem mit der Unflexibilität der Models wird sicher auch andere betreffen.

[leofeyer]

Behoben in 89cd48a.