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

Ensure comments are not displayed on archive pages #2287

Merged
merged 2 commits into from Oct 29, 2018

Conversation

Projects
None yet
2 participants
@alexsanford
Contributor

alexsanford commented Oct 26, 2018

In some themes, the comments_template() WP function is called unconditionally in some templates. We found a bug in theme compatibility on archive pages when this happens:

  • WordPress tries, at that point, to load all comments.
  • Since archive pages use a dummy page object for rendering, the post_id is 0.
  • When WP_Comment_Query sees a post_id of 0, it loads all comments for all posts.

This was causing all of the comments across the site to be loaded and displayed on all Sensei archive pages on this themes. Any theme that used a condition (such as have_comments() or comments_open()) when calling comments_template() would not have this problem, since both would be false until after comments_template() is called.

Testing

  • Visit an archive page using an unsupported theme (e.g. /lesson-tag/<tag>.

  • In the theme's page.php template, find the call to comments_template(). If it within an if statement, modify the file so that it is not in an if statement. (If available, you may also use the Jannah theme, on which this bug was found).

  • Load the archive page, and ensure that no comments appear when using this branch.

@alexsanford alexsanford added this to the 1.12.1 milestone Oct 26, 2018

@alexsanford alexsanford self-assigned this Oct 26, 2018

@alexsanford alexsanford requested a review from donnapep Oct 26, 2018

@donnapep donnapep modified the milestones: 1.12.2, 1.12.1 Oct 27, 2018

@donnapep

Unrelated to this PR (as I tested before and after), but possibly related to theme compatibility is that, with the Jannah theme, on Sensei pages where comments should be visible (lessons, single message), I see the comments, then the form to submit a new comment, followed by the same comments again. Anything to be done there?

@@ -53,7 +53,7 @@ protected function prepare_wp_query( $wp_query, $object_to_copy, $post_params )
* creating the Page.
*/
protected function output_content_as_page( $content, $object_to_copy = null, $post_params = array() ) {
global $post, $wp_query;
global $post, $wp_query, $wp_the_query;

This comment has been minimized.

@donnapep

donnapep Oct 29, 2018

Contributor

Is this line related to this commit?

This comment has been minimized.

@alexsanford

alexsanford Oct 29, 2018

Contributor

No, but I thought I would sneak it in since I noticed it was missing. Otherwise, it is possible for it to cause issues in the future (since we use $wp_the_query later in this function).

This comment has been minimized.

@donnapep

donnapep Oct 29, 2018

Contributor

OK, may be best as a separate commit next time with its own explanation to avoid any possible confusion. 🙂

@alexsanford alexsanford force-pushed the fix/archive-page-comments branch from e7529c6 to 29d145f Oct 29, 2018

@alexsanford alexsanford force-pushed the fix/archive-page-comments branch from 29d145f to a071b76 Oct 29, 2018

@alexsanford

This comment has been minimized.

Contributor

alexsanford commented Oct 29, 2018

Unrelated to this PR (as I tested before and after), but possibly related to theme compatibility is that, with the Jannah theme, on Sensei pages where comments should be visible (lessons, single message), I see the comments, then the form to submit a new comment, followed by the same comments again. Anything to be done there?

Good catch! I took a look, and this was the exact same issue in code, manifested on the CPT page. It just wasn't as nasty because it wasn't loading and displaying all the comments across the entire site!

I've refactored this fix to share it between the two page types. Please have another look when you get a chance 🙂

@donnapep

I think we're good here! 🙂👍

@alexsanford alexsanford merged commit 1e54369 into master Oct 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alexsanford alexsanford deleted the fix/archive-page-comments branch Oct 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment