trac user mrclay wrote on 42302956-03-24

Wiki format fail! The plugin hook name would be something like query:alter_options and query_name would be the type.

ewinslow wrote on 42365541-01-05

Would this need go away if we made page handler scripts easier to override?

trac user mrclay wrote on 42365687-10-29

I can already override routes via plugin hook to intercept individual pages, but as a plugin dev I'd rather not have to take over full page building responsibility (or copy/paste a bunch of code) just to alter a query.

ewinslow wrote on 42377407-07-12

OK, I think I can appreciate the benefits of this. It would allow for simple things like changing the default limit for certain queries without overriding the whole page. I'm also thinking of something like river comments -- a plugin could change it to show 5 instead of 3 without having to override the whole view.

I'd still like to see a few more concrete examples before we dive right in and add an API to a low-level function like that. Are there any possible bad side effects if we go this route?

trac user mrclay wrote on 42400647-06-24

How about two hooks named: query:entities:before (passing $options through) and query:entities:after (passing back $options, raw data rows, an array of GUIDs returned, and passing through the entities).

By looking at $options['query_name'], a particular query can be modified, and the after hook would serve use cases described in #4296.

trac user mrclay wrote on 42400674-06-11

I think the addition of collections will provide the bulk of use cases; adding ordering to widgets on profile pages, sticky discussion topics/blog posts, etc. With the before hook this should be implementable without modifying any core mods.

Bad side effects include exposing low-level functionality to plugin devs, but having this all through hooks makes it a little less-coupled; we can stop triggering hooks if there's a need to break BC...

brettp wrote on 43030004-10-20

This makes me uncomfortable. We always encourage plugin devs to use the built in API and not drop to raw SQL. If we adopt these changes, we'd have to consider the SQL as a public API and the same restrictions would apply to it as other public APIs. That's a pretty big investment to promise we won't change SQL queries between bugfix releases.

brettp wrote on 43030009-10-05

Ignore the above. I somehow missed that this only passed the options array. I'm fine adding this, with the assumption it's only for "named" calls and that the raw SQL is never able to be changed.

trac user mrclay wrote on 43030229-04-28

Current state of this: I've outlined the basic idea [http://www.mrclay.org/2012/08/25/elgg-tip-make-your-display-queries-extensible-with-plugin-hooks/ in this blog post], and have a [https://github.com/mrclay/Elgg-leaf/blob/19-collections/engine/classes/ElggNamedQuery.php NamedQuery] class that wraps up the hook triggering.

Consider you want to list entities and give plugins the ability to alter the options:

$q = new ElggNamedQuery('pages:listing', array(
    // options
));
$listing = elgg_list_entities($q->getOptions());

However, I'm returning to the opinion that using this class inside ege() would be better. API:

$listing = elgg_list_entities(array(
    // options
    'query_name' => 'pages:listing',
));

• Allowing the extension of your options is dead easy, no learning about query modifier classes
• The query mod classes don't have to be public API

@mrclay @brettp Was thinking about this more today. Here's a related idea I had. Apologies for some of the non-existent APIs. Just experimenting here with how everything might look.

$entity->getEntities('comments', $options);
$entity->getEntity('location');

• I find it a bit cleaner
• $options would normally be limited to the kinds of things users could pass via URL (sort/limit/offset)
• Encourages reusable queries instead of just customizable queries
• Can extend entity controllers without modifying the class definition (simplifies our API)
• Easy to translate from restful URLs:

// /12345/comments
$entities->getEntity(12345)->getEntities('comments')

// /users/ewinslow/plugins
$users->getUser('ewinslow')->getEntities('plugins');

// /groups/54321/discussion
$groups->getGroup(54321)->getEntities('discussion')

More concrete examples:

// Pages plugin
$user->getEntities('pages')
$group->getEntities('pages')

// Community plugins
$user->getEntities('plugins');

// Groups
$site->getEntities('groups')

@ewinslow so there's a service that accepts a name and returns a querying strategy on demand. How do they get defined?

What I like about my proposal is that we can very quickly make almost every existing query extensible in a straightforward way.

BTW I think in Evan's code $entity represents a new querying service rather than an ElggEntity

No, $entity is an ElggEntity. Sorry that wasn't clear. This is the point about extending entity controllers. Instead of having to add a getComments method, you would say $entity->getEntities('comments'). The querying strategy could be defined by a plugin hook. Strawman example:

elgg_register_plugin_hook_handler('entity:query', 'comments', function($hook, $type, $params, $queryoptions) {
  return array(
    'container_guid' => $params['entity']->guid,
    'types' => array('object'),
    'subtypes' => array('comment'),
  );
});

$params['entity'] of course is the $entity in $entity->getEntities('comments')

Based on the options returned elgg could decide whether to use elgg_get_entities, elgg_get_entities_from_metadata, or elgg_get_entities_from_relationship

I do appreciate the ease of being able to just add in a new param to make existing queries extensible. I guess the motivation here goes beyond that to also being reusable.

One issue would be, what if the querying strategy uses two/more entities? or requires something else? I'd just suggest decoupling it from the ElggEntity API, or at least the dynamic (instance) API:

$comments = ElggEntity::fetch("comments", array(
    'entity' => $entity,
    'limit' => 5,
));

But my original proposition allows basically working the same way:

$comments = elgg_get_entities(array(
    'query_name' => 'comments',
    'entity' => $entity,
    'limit' => 5,
));

You can make the query more reusable by putting more in the plugin hook, but this API has the advantage of being able to drop extensibility into existing plugins trivially.

PR: #6256

It would be more a lot more powerful to hook into the listing functions. Here we could even allow changing the getter and viewer along with the options.

...because it's not enough to filter the options if you can't change the fetch function as necessary.

I propose we close this, as we are headed in a different direction