Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace each_connected() with something less confusing #348

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

scribu
Copy link
Owner

@scribu scribu commented Jun 12, 2013

each_connected() is confusing because it doesn't return anything; instead, it produces side-effects on the $wp_query object.

It would be better if it returned something which the user could explicitly manipulate.

Proposed syntax:

$connected_locations = p2p_type( 'posts_to_locations' )->connected_many( $wp_query );
$connected_pages = p2p_type( 'pages_to_posts' )->connected_many( $wp_query );

while ( have_posts() ) : the_post();

    foreach ( $connected_locations->for( $post ) as $post ) : setup_postdata( $post );
        the_title();
    endforeach;
    wp_reset_postdata();

    foreach ( $connected_pages->for( $post ) as $post ) : setup_postdata( $post );
        the_title();
    endforeach;
    wp_reset_postdata();

endwhile;

cc: @markjaquith, @aaroncampbell

History

Initially, all the querying was done through WP_Query query vars (including 'each_connected').

Then, the idea of explicit connection types came along and the query vars were replaced with P2P_Connection_Type->get_connected().

Then, the preference went back to query vars (#61).

@scribu
Copy link
Owner Author

scribu commented Apr 12, 2013

Right, can't use for() as a method name, so will have to find something else.

@funkatron82
Copy link

If you do use it to return something, perfect filter to use is the_posts. It's what I use currently to distribute connections onto posts of a query

@funkatron82
Copy link

Been thinking about this all week and to me, a good alternative to each_connected is to instead roll the functionality into query variable instead of a separate function. The query variable (I suggest include_connected) would be an array of arrays that would let you include multiple connections. Each sub-array would have the fields that each_connected uses: the connection type, the name of the variable your including in the post object and an array of extra qvs:

$results = new WP_Query( array(
    'post_type' => 'post',
    'include_connected' => array(
        array(
            'p2p_type' => 'page_to_post',
            'prop_name' => 'connected',
            'extra_qv' => array()
        ),
        array(
            'p2p_type' => 'news_to_post',
            'prop_name' => 'news',
            'extra_qv' => array()
        )
        //etc,etc
    )
) );

I think that this is a more organic way of including connected posts vs. using a separate function.
Another option I suggest is having include_connected as a new argument in the connection registration function.

<?php
function my_connection_types() {
    p2p_register_connection_type( array(
        'name' => 'posts_to_pages',
        'from' => 'post',
        'to' => 'page',
        'include_connected' => array(
            'from' => array(
                'prop_name' => 'pages'  
            ),
            'to' => array(
                'prop_name'=> 'posts'
            )           
        )
    ) );
}
add_action( 'p2p_init', 'my_connection_types' );
?>

Adding it would add all connected objects to every WP_Query call. Not something you would do all the time but I think it would be useful in many scenarios.

@funkatron82
Copy link

And of course, after posting my last comment, I noticed you posted something similar to my first suggestion last year. I think the second one would work well for the main loop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants