bug with is_null conditional in BP_Docs_Groups_Integration::pre_query_args() ?

[gyrus]

I think there's a subtle bug here. I'm trying to track down why bp_docs_get_doc_count() seems to be returning the wrong number, and I've traced it here.

The conditional which adds the tax_query here hinges on ! is_null( $bp_docs_query->query_args['group_id'] ). However, testing shows that $bp_docs_query->query_args['group_id'] is in fact an empty string. So is_null() returns false, activating the conditional, and a tax query based on the term slug bp_docs_associated_group_0 is added. Obviously this doesn't exist, so zero is returned as the docs count.

Either I'm missing something or ! empty() would be a better conditional test here than ! is_null().

[boonebgorges]

Hi @gyrus - Thanks for your report. It does sound like something odd is happening around empty variables here. But instead of making the check looser empty() instead of is_null() when generating the query, it seems safer to me if we find the place where the query is generated, and make sure that the group ID is passed as null in that case.

So - can you clarify the code path that's leading to bp_docs_get_doc_count()? It looks like this is not currently in use in Docs, so it must be that you're calling it in a plugin or theme. It might look something like this:

$doc_group_id = bp_docs_get_associated_group_id( $doc_id );
$doc_group_count = bp_docs_get_doc_count( $doc_group_id, 'group' );

and bp_docs_get_associated_group_id() might return false or an empty string in some cases of failure. If this is happening in custom code, you might want to change this to something like:

$doc_group_id = bp_docs_get_associated_group_id( $doc_id );
if ( ! $doc_group_id ) {
    $doc_group_id = null;
}
$doc_group_count = bp_docs_get_doc_count( $doc_group_id, 'group' );

Internally, we could probably skip the query and go straight to 0 in cases where an empty() $item_id is passed to bp_docs_update_doc_count().

In any case, if you can provide more context, we can talk about how and whether anything needs fixing in Docs itself. Thanks!

[gyrus]

So in my custom theme I've got a function hooked to the filter bp_get_displayed_user_nav_document, which is in turn dynamically applied in bp_get_displayed_user_nav() (the _document part of the filter name is dynamic). All the hooked function does is adds the count for the user's docs using this:

bp_docs_get_doc_count( bp_displayed_user_id(), 'user' )

As far as I can tell bp_displayed_user_id() returns the right ID.

All of which seems irrelevant to your proposed solution, because it's based on a user's rather than a group's docs. I guess something else must be setting $bp_docs_query->query_args['group_id'] to an empty string instead of null?

Actually now I follow the code through again, I can see a probable source of this in the function bp_docs_update_doc_count()...

[boonebgorges]

Ah yes, you're right - group_id should be defaulting to null in that function. I'll send a PR here for you to test.

[boonebgorges]

@gyrus Have a look at https://github.com/boonebgorges/buddypress-docs/pull/606 and see if it seems right to you.

[gyrus]

@boonebgorges I've tried that new code, seems to work well!

[boonebgorges]

Thanks for confirming!

[gyrus]

@boonebgorges Am I right in thinking that fixing this function may not update already-inaccurate document counts for users? Is there something I need to do to retroactively update all user's docs counts? There seem to be some discrepancies on the live site still, with the new code.

[boonebgorges]

@gyrus Correct. The counts are cached in usermeta/groupmeta. If you need them all refreshed, try deleting all instances of bp_docs_count rows in usermeta, and/or all instances of bp-docs-count in groupmeta. (Back up first!) Counts are fetched in such a way that they will automatically be recounted if the cached value is not found (see eg bp_docs_get_doc_count())

[gyrus]

@boonebgorges Many thanks, sorted.