CUNY Academic Commons

I'm sorry to be a nudge, but . . .

Marilyn, as noted above, the problem here is deeply complex. Please extend our apologies to the user over the spacing issue, but this is not something that can be quickly solved. Please have patience while the team tries to get to the bottom of the issue. The timeline for this is weeks, not days.

Will do.

I spent a bit of time at the end of the week trying to debug the is_front_page / is_home issue, but didn't get much further. The good news is this issue can be duplicated on cdev. If you are logged in as an administrator, you can set an authentication cookie with Query Monitor to do some logged-out debugging. I've disabled Litespeed Cache for the following group -- https://cunyac.reclaimhosting.dev/groups/ray-test-group/ -- so that might help with debugging.

Might it make sense to run my workaround fix on production until we can pinpoint the exact issue here?

Ray, I think we should go ahead with your workaround.

Should it be run only on the main site? Seems like that's the only place where bp_parse_query will ever fire anyway?

My mind is more fresh today :)

Let's go all the way back to #22668:

For some reason, though, is_front_page() is returning true for URLs like https://commons.gc.cuny.edu/members/admin/docs/edited/library/

I've put a fix in place such that the access query is run when a BP component is detected, which should solve the problem with false negatives like this. This has been deployed, and the relevant caches have been flushed.

When the WP_Query in that buddypress-docs function runs, it somehow results in the is_home flag being tripped.

When we added the bp_current_component() check on our custom 'pre_get_posts' hook, this created the current set of is_home() side-effects.

Since this is_home problem is only an issue on BuddyPress Docs user pages, I'm altering the conditional to check if we're on a member Docs page. I've done this in https://github.com/cuny-academic-commons/cac/commit/bad99e7a49f027330398ec85c314911bce29aba1 and pushed the fix to production. This should address this CV issue and the group "Manage" issue mentioned in https://redmine.gc.cuny.edu/issues/22069#note-11.

Side note: We already worked around the is_front_page() and /members/X/docs/edited/library problem by redirecting away from these member doc pages in https://redmine.gc.cuny.edu/issues/22668#note-4. There is still an underlying problem with private doc info (like the title and excerpt) being displayed in the doc loop on a member's Docs page for all other BuddyPress Docs installs...

===

More braindump about the bp_docs_general_access_protection() function for Boone:

This function affects all WP_Query loops by adding a NOT IN clause. There is a reason mentioned in the comments, but is this necessary outside of the bp_doc post type queries, general search queries and sitemap generation? The commit that introduced this change mentions a conflict with bbPress. Is that still the case?

1. Do we need to run the function when the query is for a BuddyPress query like 'bp_groups' or 'bp_members'? If not, we can add the following conditional in bp_docs_general_access_protection() at this juncture to bail from the lookup:

    // Don't do this for a BuddyPress query.
    if ( $query->get( 'bp_groups' ) || $query->get( 'bp_members' ) ) {
        return;
    }

2. Since the function currently does a WP_Query loop on the 'pre_get_posts' hook for the NOT IN clause. If you run a WP_Query loop on the 'pre_get_posts' hook, all the hooks that take place in a WP_Query loop will run again. Something about this is causing all these unintended problems like what you mentioned in the first comment of this thread. Doing a direct DB query to fetch the doc IDs would be preferable, but again is an ugly workaround. The caching set up that's already in place in the get_doc_ids() method can help to mitigate this if we switched to using a DB query here.

Thanks! The user is happy

Ray, thanks for the braindump.

The commit that introduced this change mentions a conflict with bbPress. Is that still the case?

God, I have no idea. I wish I'd opened a ticket or something that spelled out the actual issue.

I do remember that, in adding this $filter_query logic, I was trying to be conservative. That is, if I couldn't be 100% sure that the query in question would match bp_doc items, then I'd go ahead and run the NOT IN query. But this broadness appears to be the source of the issue, and I wonder if I could switch from the conservative "always filter except..." to the laxer "only filter when...". And the "when" there would be: (a) post_type=bp_doc, (b) post_type CONTAINS bp_doc, (c) post_type=any. Cases you mentioned would be covered by this: sitemaps are built by fetching each 'public=true' post type and then doing a query for each one (ie post_type=bp_doc), and search works by building an array of post_type that excludes 'exclude_from_search' post types (ie post_type=post,page,bp_doc).

So the questions are: (1) Would this change introduce privacy bugs? It's not clear to me that it would. And (2) would this change make the current set of problems go away, by ensuring that this additional NOT IN query doesn't run willy-nilly? My understanding is that it would, and we could possibly even do away with the whole 'pre_get_posts' filter in cac-functions.php.

And (2) would this change make the current set of problems go away, by ensuring that this additional NOT IN query doesn't run willy-nilly? My understanding is that it would, and we could possibly even do away with the whole 'pre_get_posts' filter in cac-functions.php.

Removing our custom 'pre_get_posts' hook and moving the conditional logic to the main bp_docs_general_access_protection() function should address this specific instance with is_home() and BuddyPress Docs.

However, after looking at my 'bp_parse_query' workaround again, I still think this is a general BuddyPress bug because when we do checks for is_home() on early hooks like 'pre_get_posts', is_home() will always return true on BuddyPress pages:

add_action(
    'pre_get_posts',
    function( $query ) {
        if ( ! $query->is_main_query() ) {
            return;
        }

        // This returns true on BuddyPress pages at this juncture.
        var_dump( $query->is_home() );
    }
);

// Runs later in the WP bootstrap.
add_action( 'wp', function() {
    // This returns false on BuddyPress pages at this juncture.
    var_dump( is_home() );
} );

So it is not reliable to do is_home() checks on early hooks, I'll most likely write up a BuddyPress ticket about this.

Ray, you talked about opening a BuddyPress ticket with your findings and with a suggestion about how to fix the is_home behavior in BP itself

I've posted the ticket to BuddyPress Trac here: https://buddypress.trac.wordpress.org/ticket/9300. I've also committed the code snippet I suggested in comment 1 in https://github.com/cuny-academic-commons/cac/commit/1147821c5e90bc871d44bd0868ab60a6c07c6331 as I feel comfortable running it after reviewing this issue again while posting the BP ticket. I've also removed a previous is_home() workaround for certain group event pages in https://github.com/cuny-academic-commons/cac/commit/c82d7f355977b689cf611f5a40a997e78690a642, since this is no longer required with the new fix.