Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 2 years ago

#23833 closed enhancement (duplicate)

Add a unique ID or NAME to target queries .

Reported by: krembo99's profile krembo99 Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Query Keywords:
Focuses: Cc:

Description

I am not sure if this is enhancement or feature request .

wordpress allows us to change / filter queries with various methods , for example the pre_get_posts(). ( e.g. $query->set ($args) ).

But in today´s wp ecosystem, there are so many themes, plugins, widgets etc. that modify the query or add custom queries, that it had become very difficult to target a specific query, where The available conditions might not be enough. (is_admin, is_main_query etc.. )

The suggestion here is to add an identifier (ID or specific NAME) to a query , much like with textdomains or actions.

In this manner, targeting a query becomes a breeze something along the lines of

$query->set($query_id_or_name,$args)

Each developer has struggled one time or another with custom queries and with targeting 3rd party queries .
This feature / enhancement could potentially change all that by allowing a simple way to address custom queries .

Change History (13)

#1 @krembo99
11 years ago

Add examples :

right now we can assign a local var to the query

$my_query = new WP_Query($args)
while($my_query->have_posts()) {
      // Loop in here
   }

but changing it to

$my_query = new WP_Query('my_query_id',$args)

will give an additional layer to be used by third party developers (and self in a later stage)

another example / scenario would be to potentially change 10 named queries at once which is right now not possible .

Last edited 11 years ago by krembo99 (previous) (diff)

#2 @nacin
11 years ago

bbPress 1.x's BB_Query had query identifiers. I agree these can be helpful. I think this has been proposed previously, a year or two ago.

#4 follow-up: @prettyboymp
11 years ago

  • Cc mpretty@… added

I don't like the idea of names or context being referenced within WP_Query (or even query filters) because a name or context isn't explicit. It's already bad enough that WP_Query checks globals to modify the query for is_admin and is_main_query. The query_vars passed to wp_query should be modified before the WP_Query instance is created.

At this layer in the platform, the query_vars for a WP_Query instance should be explicit about what data the query will return. When a WP_Query instance is made with a set of query_vars it should ALWAYS return the same data, regardless of the context.

Ideally, there should be a factory method for WP_Query. With this, a factory could accept a context and append/modify the query_vars before creating the WP_Query instance. This would allow plugin authors to watch for a specific context in the factory method, apply query_vars that explicitly say how they're modifying the query, and add filters that react to those query_vars.

#5 in reply to: ↑ 4 @MikeSchinkel
11 years ago

  • Cc mike@… added

Replying to prettyboymp:

At this layer in the platform, the query_vars for a WP_Query instance should be explicit about what data the query will return. When a WP_Query instance is made with a set of query_vars it should ALWAYS return the same data, regardless of the context.

What justification do you have for "should ALWAYS?" I'm asking honestly, because at the moment I don't know what the justification for this would be but maybe I don't know what I don't know.

Without knowing of a justification for why it would not be a good idea I can envision that contexts would be very helpful to change the output. For example a context of 'main-loop' might return different values than a context of 'sidebar' (I'm being convenient in my examples.)

Or do you simply mean that a context should be included in the query_vars? i.e.:

$my_query = new WP_Query(array( 
   'context' => 'my_query_id',
   '...' => '...', 
));

Ideally, there should be a factory method for WP_Query....

That sounds like it might meet the needs but it's not clear to me unless I could see the implementation and apply to some of my use-cases.

BTW, I see a context as being used like classes in CSS, so there is a need to be able to have multiple contexts in a WP_Query if the use-case calls for it. :)

#6 @prettyboymp
11 years ago

Sorry it took me so long to get back to this. After going back and reading my first post, I realized my response is a bit confusing because I'm addressing multiple issues I have with WP_Query as it is, though they're are all related to this ticket.

I don't have a direct issue with adding 'context' as a query_var. I agree that it is probably the best way for a plugin to know how to alter a specific query. But I believe it should only be allowed as an intermediate query_var. A query_var like 'context' should be converted into one or more query_vars that explicitly express the filters they are adding to the query result. Without that conversion, any plugin that may replace the request to WPDB with some other backend, like a search provider, has no way to reimplement the logic added by a plugin that directly altered the MySQL query based on 'context'. However, if the plugin took {'context' => 'sidebar'} and converted it into {'meta_key' => 'feature_sidebar', 'meta_value' => '1'}, other plugins could understand it.

An example implementation would be.

$my_query = get_query(array('context'=> 'sidebar', 'posts_per_page' => 5));

function get_query($args) {
    $query_vars = apply_filters('parse_query_vars', $args); //allows plugins to translate 'context' into explicit query vars
    ....//core code taken out of WP_Query to fill in any unset with defaults
    return new WP_Query($query_vars);
}

With the above, WP_Query would be changed so that it only paid attention to explicit query vars. It wouldn't know what to do with something like 'context' since 'context' has no applicable meaning.

#7 follow-up: @MikeSchinkel
11 years ago

@prettyboymp - Thanks for the follow up.

I think I understand your concern, but only on an abstract basis, i.e. to make sure that 3rd party hooks are not affected by context. What I can't envision is an example of the thing you are trying to guard against. Your examples show how to do it right, but without a corresponding how-to-do-it-wrong example it's unclear where the problem would be.

BTW, the OP suggested a context and then @SergeyBiryukov references my related ticket #15063 so my interest may be different than the OPs; he'll have to explain himself better for me to understand what his use-cases really are.

My use-case need for contexts was about queries that WordPress itself generates, typically from within the admin but also within widgets, and by definition the contexts supplied would need to be cumulative, i.e. code calling a high level function might set context to 'admin_page_list' and then WP->query_posts() adds wp_query_posts to context, and so on down the chain. A perfect example need for context is the code in WP_Posts_List_Table that displays a category dropdown for the Quick Edit:

if ( 'top' == $which && !is_singular() ) {

  $this->months_dropdown( $this->screen->post_type );

  if ( is_object_in_taxonomy( $this->screen->post_type, 'category' ) ) {
    $dropdown_options = array(
      'show_option_all' => __( 'View all categories' ),
      'hide_empty' => 0,
      'hierarchical' => 1,
      'show_count' => 0,
      'orderby' => 'name',
      'selected' => $cat
    );
    wp_dropdown_categories( $dropdown_options );
  }
  do_action( 'restrict_manage_posts' );
  submit_button( __( 'Filter' ), 'button', false, false, array( 'id' => 'post-query-submit' ) );
}

If I need to modify the WP_Query that wp_dropdown_categories() ultimately calls, let's say I want to limit categories to only list the parent categories for some reason, it can be error prone and hard to know for certain that I'm only modifying what I want to modify and not accidentally modifying some other use of wp_dropdown_categories(). I can inspect $pagenow and verify that it is edit.php but what happen if some other plugin also uses wp_dropdown_categories() on the same page but for a different purpose? My code will break theirs.

However, if WordPress were to establish a "standard" for an optional 'context' array then WordPress itself could change the code above to include a context argument such as 'context' => array( 'admin_post_list_quick_edit' ):

if ( 'top' == $which && !is_singular() ) {

  $this->months_dropdown( $this->screen->post_type );

  if ( is_object_in_taxonomy( $this->screen->post_type, 'category' ) ) {
    $dropdown_options = array(
      'show_option_all' => __( 'View all categories' ),
      'hide_empty' => 0,
      'hierarchical' => 1,
      'show_count' => 0,
      'orderby' => 'name',
      'selected' => $cat,
      'context' => array( 'admin_post_list_quick_edit' )
    );
    wp_dropdown_categories( $dropdown_options );
  }
  do_action( 'restrict_manage_posts' );
  submit_button( __( 'Filter' ), 'button', false, false, array( 'id' => 'post-query-submit' ) );
}

With this a plugin developer could be sure to target changes to only work when 'admin_post_list_quick_edit' is in $query->context and thus not break other plugin's use of wp_dropdown_categories(). And other plugins could also add a context so that still others that target their contexts could do so explicitly.

BTW, as far as I can envision the benefits are not really needed when you are the developer writing the WP_Query() call because you can already do it yourself. The benefits are instead two-fold and if/when WordPress core decorates its code with contexts:

  1. To enable plugin developers to target queries that they did not write and for which there are no hooks that allow them to modify the query args before it is called _(even if there were/are hooks using hooks for this means multiple collaborating hooks to effect just one change.)_
  2. To establish a best practice for plugin developers to follow so their specific queries can be targeted by other plugin developers too.

Given my use-case can you envision it causing the kind of problems you are concerned about and if so can you give specifics to help me better understand? Thanks in advance.

#8 @alex-ye
11 years ago

  • Cc nashwan.doaqan@… added

#9 in reply to: ↑ 7 ; follow-up: @prettyboymp
11 years ago

Replying to MikeSchinkel:

If I need to modify the WP_Query that wp_dropdown_categories() ultimately calls, let's say I want to limit categories to only list the parent categories for some reason

I don't think wp_dropdown_categories() has a call to WP_Query in its stack.

#10 in reply to: ↑ 9 @MikeSchinkel
11 years ago

Replying to prettyboymp:

I don't think wp_dropdown_categories() has a call to WP_Query in its stack.

You are correct. Bad example on my part for WP_Query; I researched in haste and was not paying enough attention to the specifics.

But still a good example for need of 'context' argument. And there are also good examples for need for 'context' passed by WordPress core for calls to new WP_Query() in:

  • wp_ajax_query_attachments()
  • wp_link_query()
  • wp_dashboard_quick_press()
  • wp_dashboard_recent_drafts()
  • WP_Widget_Recent_Posts->widget()
  • wp_nav_menu_item_post_type_meta_box()
  • wp_get_associated_nav_menu_items()
  • get_popular_posts()
  • delete_search_results_template()
  • get_posts()
  • wp_get_post_autosave()
  • query_posts()
  • url_to_postid()
  • wp-settings.php

Then of course there is the large number of places that get_posts() is called as well, and several other functions that ultimately call new WP_Query() and/or WP_Query->get_posts().

Not having an explicit context means that the plugin code has to "guess" at it's context using URL parameters, $pagenow, $typenow, $current_screen and any number of other environmental clues that can result in a false positive, the worst being debug_backtrace(). By adding an explicit 'context' for all calls to generic functions throughout WordPress core plugin authors could precisely "scope" their hooks to operate only when needed and not when they are "best guess" needed.

#11 @wonderboymusic
11 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

The patch on #17019 adds a query_context query var

#12 @SergeyBiryukov
3 years ago

#52069 was marked as a duplicate.

#13 @SergeyBiryukov
2 years ago

#55644 was marked as a duplicate.

Note: See TracTickets for help on using tickets.