Skip to:
Content

bbPress.org

Opened 5 years ago

Last modified 4 years ago

#3234 new defect (bug)

Conflict with All in One SEO’s Sitemap query

Reported by: morgul's profile morgul Owned by:
Milestone: 2.7 Priority: normal
Severity: normal Version: 2.5.14
Component: Component - Forums Keywords: has-patch
Cc:

Description

There's an issue with having bbp_pre_get_posts_normalize_forum_visibility function attached to pre_get_posts action. It’s messing up a query from All in One SEO plugin’s Sitemap, which is for getting all the posts for its Sitemap generation. The issue is making this query return only bbPress topics, even the query was asking for posts from several post types (not only forum topics), which have matches (but they can't show up if ‘topic‘ post type was chosen amongst the post types for the Sitemap).

The reason is the meta_query that bbp_pre_get_posts_normalize_forum_visibility is adding to the query, in order to exclude private forums. Which ends up making the query to only return posts of type ‘topic’.

The condition it adds should end up turning into a SQL query that doesn’t force all posts to require a meta key bbp_forum_id. So it should be done in a way that it only limits the posts with that meta key to those that meet the condition, but allows posts without that meta key.

Steps to reproduce:
1) Install both bbPress and All in One SEO plugins.
2) Create and publish a public page.
3) Configure Sitemap generation on All in One SEO. Select forum and pages as the post types for the Sitemap generation.
4) Download the generated Sitemap. Check it to see how the public page is showing up.
5) Create a private bbPress forum.
6) Download the generated Sitemap again. Check it to see how the public page is no longer showing up.

Attachments (2)

non_invasive_meta_query.diff (1.0 KB) - added by morgul 5 years ago.
Fix for this issue, making the query be non-invasive to non-bbPress queries
3234.2.patch (1.5 KB) - added by johnjamesjacoby 5 years ago.

Download all attachments as: .zip

Change History (7)

@morgul
5 years ago

Fix for this issue, making the query be non-invasive to non-bbPress queries

#1 @johnjamesjacoby
5 years ago

  • Component changed from General - Integration to Component - Forums
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 2.6.1

Thanks for catching this. It's also possible this entire bit of code isn't working as intended anymore (maybe never?)

! array_diff( $post_types, bbp_get_post_types() )

That comparison will always return false, unless $post_types is specifically all three bbPress post types. The code comment above eludes to it trying to do the opposite:

// Some other post type besides Forums, Topics, or Replies

Can you include a var_dump() (or similar) of the query you're seeing All in One SEO generate?

#2 @johnjamesjacoby
5 years ago

Refreshed patch based on the original approach, to match the code comment style, and isolate all of the meta-query clauses together into their own array. Also removes the ! so it runs on more queries, in-case that was the original goal (not sure anymore?)

Last edited 5 years ago by johnjamesjacoby (previous) (diff)

#3 @morgul
5 years ago

Can you include a var_dump() (or similar) of the query you're seeing All in One SEO generate?

Hello, @johnjamesjacoby. Yes, here is the query it was generating:

<?php
array (
  'numberposts' => 50000,
  'offset' => 0,
  'category' => 0,
  'orderby' => 'post_date',
  'order' => 'DESC',
  'include' => 
  array (
  ),
  'exclude' => 
  array (
  ),
  'post_type' => 
  array (
    0 => 'post',
    1 => 'page',
    2 => 'forum',
    3 => 'topic',
    4 => 'product',
  ),
  'meta_key' => '_aioseop_sitemap_exclude',
  'meta_value' => 'on',
  'meta_compare' => '=',
  'meta_query' => 
  array (
    0 => '',
    1 => 
    array (
      'key' => '_bbp_forum_id',
      'value' => '162',
      'type' => 'numeric',
      'compare' => '!=',
    ),
  ),
  'cache_results' => false,
  'no_found_rows' => true,
  'post_status' => 'publish',
  'fields' => 'ids',
  'posts_per_page' => -1,
  'error' => '',
  'm' => '',
  'p' => 0,
  'post_parent' => '',
  'subpost' => '',
  'subpost_id' => '',
  'attachment' => '',
  'attachment_id' => 0,
  'name' => '',
  'static' => '',
  'pagename' => '',
  'page_id' => 0,
  'second' => '',
  'minute' => '',
  'hour' => '',
  'day' => 0,
  'monthnum' => 0,
  'year' => 0,
  'w' => 0,
  'category_name' => '',
  'tag' => '',
  'cat' => '',
  'tag_id' => '',
  'author' => '',
  'author_name' => '',
  'feed' => '',
  'tb' => '',
  'paged' => 0,
  'preview' => '',
  's' => '',
  'sentence' => '',
  'title' => '',
  'menu_order' => '',
  'embed' => '',
  'category__in' => 
  array (
  ),
  'category__not_in' => 
  array (
  ),
  'category__and' => 
  array (
  ),
  'post__in' => 
  array (
  ),
  'post__not_in' => 
  array (
  ),
  'post_name__in' => 
  array (
  ),
  'tag__in' => 
  array (
  ),
  'tag__not_in' => 
  array (
  ),
  'tag__and' => 
  array (
  ),
  'tag_slug__in' => 
  array (
  ),
  'tag_slug__and' => 
  array (
  ),
  'post_parent__in' => 
  array (
  ),
  'post_parent__not_in' => 
  array (
  ),
  'author__in' => 
  array (
  ),
  'author__not_in' => 
  array (
  ),
)

With my patch, the meta_query key on that array would instead be:

<?php
'meta_query' => 
  array (
    0 => '',
    'relation' => 'OR',
    1 => 
    array (
      'key' => '_bbp_forum_id',
      'compare' => 'NOT EXISTS',
      'value' => '',
    ),
    2 => 
    array (
      'key' => '_bbp_forum_id',
      'value' => '162',
      'type' => 'numeric',
      'compare' => '!=',
    ),
  ),

#4 @johnjamesjacoby
4 years ago

  • Milestone changed from 2.6.1 to 2.6.2

Move these to 2.6.2.

#5 @johnjamesjacoby
4 years ago

  • Milestone changed from 2.6.2 to 2.7
Note: See TracTickets for help on using tickets.