Why is my WP_Query outputting my entries twice?

Question

I am using a small shortcode to output a list of movie titles arranged by decades. decades is my custom taxonomy with terms like 1930s 1940s etc.

Here is my shortcode:

[fashionfilms type=fashionfilms  tax=decades]

and here is how that shortcode is parsed:

add_shortcode('fashionfilms', 'cw_output_fashion_films');

function cw_output_fashion_films($atts){

    $post_type = $atts['type'];
    $tax = $atts['tax'];
    $tax_terms = get_terms($tax);

    if ($tax_terms) {
      echo "<ul>";
      foreach ($tax_terms  as $tax_term) {
        $args=array(
          'post_type' => $post_type,
          "$tax" => $tax_term->slug,
          'post_status' => 'publish',
          'posts_per_page' => -1,
          'ignore_sticky_posts'=> 1
        );

        $my_query = null;
        $my_query = new WP_Query($args);
        if( $my_query->have_posts() ) {
          echo "<li class='letter'>" . $tax_term->name . "</li>";
          while ($my_query->have_posts()) : $my_query->the_post(); ?>
            <li><a href="<?php the_permalink() ?>" rel="bookmark" title="Permanent Link to <?php the_title_attribute(); ?>"><?php the_title(); ?></a></li>
            <?php
          endwhile;
        }
        wp_reset_query();
      } //end foreach loop
      echo "</ul>";
    }
}

The problem is I am getting a doubled output for my entries.

To see the doubled outout, please check my live link here.

Here is my page template code too:

<div class="span9">
            <?php while ( have_posts() ) : the_post(); ?>
            <div <?php post_class(); ?>>
                <h1><?php the_title();?></h1>
                <div class="content">
                    <?php the_content() ?>
                </div>
            </div><!-- /.post_class -->
        <?php endwhile; ?>
        <?php bootstrapwp_content_nav('nav-below');?>

</div><!-- /.span9 -->

What am I doing wrong? How can I fix this?

Answer 1

You've used wp_reset_query, however, you should use wp_reset_postdata.

wp_reset_query takes the current query object, and replaces it with the main query. The problem here however, is that you're using WP_Query, which is a separate individual query object, the main query has not been touched and does not need preserving.

This means we don't need to call wp_reset_query, as the query is never changed.

We did call $my_query->the_post however. This means that the current post data is from your custom query, and needs resetting, so that's why we call wp_reset_postdata.

Calling wp_reset_query when you don't have to can have unintended consequences, in this case duplication of your posts

Answer 2

While the other answers are more specific, you have a somewhat more urgent problem:

Shortcodes should not create output.

By which, I mean that your shortcode handler function should never, ever, "echo" anything. It should build a string and return it. The shortcode processing code will take care of inserting it into the content properly and echo'ing it out for you.

If you echo things out yourself, then your shortcode will be incompatible with lots and lots of other things. Shortcodes are not processed only when you output the content, they can be processed by other code as well.

Echoing during a shortcode (or a filter) will sometimes cause your output to be output twice, or more, because your shortcode can be run twice (or more) on the same page load.

Change your function to return a string instead of echoing the content you're trying to produce.

More info: http://ottopress.com/2011/actions-and-filters-are-not-the-same-thing/

Answer 3

The problem is not wp_reset_query() by itself. However, as @TomJNowell has pointed out, and @hampusn originally suggested, you should should use wp_reset_postdata() as you are not altering the main query, so it doesn't need to be reset.

What wp_reset_query does...

On top of calling wp_reset_postdata(), wp_reset_query() just replaces the main query $wp_query with $wp_the_query (a stored copy). These two are linked by reference, and so should be identical... unless the link is broken and $wp_query is changed by query_posts() (for which wp_reset_query() is intended).

What happens when query_posts() is used...

Here's where the problems start. query_posts() changes the query $wp_query and breaks the reference. This is what you loop through. Lets suppose that the first post in this new loop is the one using your shortcode. In your call callback you use wp_reset_query().

$wp_query is now set back to the original query - which was for the post we've just printed. So the loop is essentially reset (in this my comment on the question was slightly wrong*) to the original query. This loop prints the post's content, and reaches the end.

Why does using wp_reset_postdata 'solve' it?

If you use query_posts() and the result is just the one post you're after - this will result in duplication.

Why doesn't using wp_reset_postdata 'solve' it?

If you use query_posts() and the result includes other posts then the first pass of the loop will include these other posts.

The solution:

In short the real problem using query_posts(). Don't use it. Using wp_reset_postdata, although the correct thing to use in this case, is just disguising the error.

But I'm not using query_posts()...

I can't be sure there are not other errors that would cause what you're seeing. But on a 'clean' install - you shouldn't see it with your above code (I've tested).

*If $wp_query and $wp_the_query are still linked by reference then my comment is in fact correct.

Answer 4

As suggested by @hampsun above, adding wp_reset_postdata() fixed the double output issue. The custom loop now reads:

[...]

function cw_output_fashion_films($atts){

    $post_type = $atts['type'];
    $tax = $atts['tax'];
    $tax_terms = get_terms($tax);

    if ($tax_terms) {
      echo "<ul>";
      foreach ($tax_terms  as $tax_term) {
        $args=array(
          'post_type' => $post_type,
          "$tax" => $tax_term->slug,
          'post_status' => 'publish',
          'posts_per_page' => -1,
          'ignore_sticky_posts'=> 1
        );

        $my_query = null;
        $my_query = new WP_Query($args);
        if( $my_query->have_posts() ) {
          echo "<li class='letter'>" . $tax_term->name . "</li>";
          while ($my_query->have_posts()) : $my_query->the_post(); ?>
            <li><a href="<?php the_permalink() ?>" rel="bookmark" title="Permanent Link to <?php the_title_attribute(); ?>"><?php the_title(); ?></a></li>
            <?php
          endwhile;
        }
        wp_reset_postdata();
      } //end foreach loop
      echo "</ul>";
    }
}

No idea after reading documentation as to why this works: http://digwp.com/2011/09/3-ways-to-reset-the-wordpress-loop/#wp_reset_postdata