? Pending

User tests: Successful: Unsuccessful:

avatar sakiss
sakiss
5 Sep 2019

The problem

Filtering the returned items is an essential functionality.
Trying to add some filters to the returned content (articles), it turns out that there is no way to have any control over the getList query that returns the items of a component.

Although plugin events are triggered for other basic functions (save, delete), there is no such an event for the read functionality, that returns a set of items

Suggested Solution

Added a plugin trigger at the end of _getListQuery function in the ListModel class. The function allows plugins to edit the query by adding their own conditions.
Since this is an abstract function that is called by all the components (at least those that do not overwritte that), this covers all the components.

Testing Instructions

Add a public function onAfterGetListQuery($context, $query) in a system plugin and check if it is triggered.

avatar sakiss sakiss - open - 5 Sep 2019
avatar sakiss sakiss - change - 5 Sep 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 5 Sep 2019
Category Libraries
avatar sakiss sakiss - change - 5 Sep 2019
The description was changed
avatar sakiss sakiss - edited - 5 Sep 2019
avatar sakiss sakiss - change - 5 Sep 2019
The description was changed
avatar sakiss sakiss - edited - 5 Sep 2019
avatar franz-wohlkoenig franz-wohlkoenig - change - 5 Sep 2019
Title
Trigger a plugin event on getListQuery
[4.0] Trigger a plugin event on getListQuery
avatar franz-wohlkoenig franz-wohlkoenig - edited - 5 Sep 2019
avatar sakiss sakiss - change - 5 Sep 2019
The description was changed
avatar sakiss sakiss - edited - 5 Sep 2019
avatar brianteeman
brianteeman - comment - 5 Sep 2019

The function allows plugins to edit the query by adding their own conditions.

Pretty sure it was stated before that plugins should not change the query. Maybe @mbabker can confirm that

avatar mbabker
mbabker - comment - 5 Sep 2019

Please please please please please please please no hooks for query manipulation.

avatar mbabker
mbabker - comment - 5 Sep 2019

The fix here is better object oriented architecture. Making it simpler for classes to be extended throughout core and not treating component classes as pseudo-final classes that cannot be extended. These types of hooks are bandaids at best and exposing just how bad parts of the architecture really are at worst.

(Not to mention that the second you introduce a plugin that fails to do proper context checking, or you introduce multiple plugins that try to hook the same query, you're screwed. And what if someone starts removing fields from the SELECT list and breaks the views. No. This is just asking for too much trouble.)

avatar sakiss
sakiss - comment - 5 Sep 2019

I agree with the above and were part of my consideration before this PR, though i opened it because:

  1. I see several plugin events manipulating objects.
    e.g. onExtensionBeforeSave, onExtensionAfterSave which manipulate the Table object.

  2. @mbabker would love to extend a component's Model. But i see that the called model (by the controller and the view) is tightly coupled with the passed view name. I see no any dependency injection mechanism in the component level, where i can pass my own classes.
    If you have any idea over that, i am open to it.

avatar mbabker
mbabker - comment - 5 Sep 2019

I see several plugin events manipulating objects.
e.g. onExtensionBeforeSave, onExtensionAfterSave which manipulate the Table object.

Different concepts. In these types of events you are directly manipulating the data of a single record, a hook point that is required for an extensible system to work correctly (otherwise, you might as well just rip out those events and tell people "tough luck", you figure out your own way to do something if an article was updated, notice the table classes don't have any type of hook for after the API has taken the data and built a SQL statement but before the statement is executed). Manipulating a general SQL statement though is a different concept altogether, and one that has a bigger risk of consequences and generally should be avoided. And realistically, even if you could hook the admin list view queries you really shouldn't be adding much more than what the models already support a state value to change and if what's in the models doesn't work for you the only thing you can realistically do is add more query filters (as long as the search tools form gets the same events the edit view form does); you can't add fields to the list to be shown on the view because the layout is hardcoded HTML with no way to customize the table.

I see no any dependency injection mechanism in the component level, where i can pass my own classes

This is a fundamental flaw in the Joomla architecture in general, and there isn't an easy solution here. In all reality, you might as well look at most component classes (at least the MVC + Table classes, helpers full of statics too) as having the "final" keyword on them because without some heavy handed mucking about the architecture was not designed with component level extensibility in mind (in 3.x and earlier because everything uses prioritized file path lookups, if you know what you're doing, you can force a custom class to load, but because the class names have to match such a rigid convention you can't subclass anything, only overload it).

avatar sakiss
sakiss - comment - 6 Sep 2019

@mbabker Thanks!
It turns out that the extension/overloading of a model class has 2 disadvantages:

  1. Is not so elegant, as it needs a new set of MVC classes, into a component.
  2. Is not global. Solves the problem only to a specific component.

Given that with the use of custom fields, any component can have items with multiple attributes, i find filtering quite essential. Also seems like the common way to extend a component's function is through a plugin.

I thought 2 alternative options on that:

  1. An event triggered before the getListQuery that returns an array of item ids (i.e. pk). This array will be used by the model's query in a WHERE IN clause.

  2. An event triggered after the getListQuery that returns the model's found items which can be filtered further by the plugin.

Any idea over that is welcome.

avatar mbabker
mbabker - comment - 6 Sep 2019

A pre-query hook is still resulting in query manipulation, just that scope
of what can be done is limited.

A post-hook is problematic because it will mess with pagination at a bare
minimum, you would practically have to stop using pagination queries and
filter it in PHP. More performance issues in an already
less-than-performing system.

To me, query manipulation really only belongs in a CCK like system. The
core data model nor custom fields are a CCK.

Giving special treatment to one query function allowing manipulation (when
you can’t reliably do much with it anyway because of the fragmented and
pseudo-final nature of components) just doesn’t come across as a good long
term thing.

Which leaves class inheritance, or finding some kind of middleware like
behavior. Yeah, it’s less pretty than just letting someone hook a
“special” query to do what they want with it, but it’s still
architecturally the debatably the most “right” way to go about business.
Unfortunately, Joomla isn’t designed with the ability to extend components
in mind. Even if you solve the filtering problem (which in all honesty
both the general query hook suggested here and the subclassing idea are
hacks at best, this is why core features should not be 100% delegated to
components where they can’t be extended or easily hook other components but
that’s another tangent for another day), you still have a lot of other
problems that need to be solved to make this type of extension point work
(i.e. adding data to list views).

On Fri, Sep 6, 2019 at 7:31 AM Sakis Terzis notifications@github.com
wrote:

@mbabker https://github.com/mbabker Thanks!
It turns out that the extension/overloading of a model class has 2
disadvantages:

  1. Is not so elegant, as it needs a new set of MVC classes, into a
    component.
  2. Is not global. Solves the problem only to a specific component.

Given that with the use of custom fields, any component can have items
with multiple attributes, i find filtering quite essential. Also seems like
the approved way to extend a component's function is through a plugin
trigger.

I thought 2 alternative options on that:

An event triggered before the getListQuery that returns an array of
item ids (i.e. pk). This array will be used by the query in a WHERE IN
clause.
2.

An event triggered after the getListQuery that returns the found
items which they can be filtered further.

Any idea over that is welcome.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/joomla/joomla-cms/pull/26179?email_source=notifications&email_token=AACZ7IOM4FO2M7QT6WETU6TQIJESXA5CNFSM4IUAIKTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6CWKXA#issuecomment-528835932,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AACZ7IJYC5XJQ7PPMRVOXJTQIJESXANCNFSM4IUAIKTA
.

--

  • Michael Please pardon any errors, this message was sent from my iPhone.
avatar wilsonge
wilsonge - comment - 6 Sep 2019

https://groups.google.com/forum/#!searchin/joomla-dev-cms/query%7Csort:date/joomla-dev-cms/KkGuhBYvoyo/sTILTLIqfXIJ

There's been many discussions about related events before, for example above. And it's been a longstanding policy not to introduce events on query objects. I've had a quick discussion with @HLeithner and both of us agree that there's no real new information here compared to the original posts and it also has the potential to become a black hole of possibilities. So I'm closing this

avatar wilsonge wilsonge - close - 6 Sep 2019
avatar wilsonge wilsonge - change - 6 Sep 2019
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2019-09-06 14:43:22
Closed_By wilsonge
Labels Added: ?

Add a Comment

Login with GitHub to post a comment