fatal error when trashing emails

[DJPaul]

When clicking “rebuild emails…” and clicking “empty trash” in Emails -> All emails -> Trash
i get error:
Fatal error: Call to undefined function bp_blogs_delete_activity() in ..../public_html/wp-content/plugins/buddypress/bp-blogs/bp-blogs-functions.php on line 914

From ​https://buddypress.org/support/topic/fatal-error-bp_blogs_delete_activity-after-clicking-empty-trash-in-emails/

[DJPaul]

This might be if the activity component is disabled (bp-blogs-activity.php is only loaded then). Have asked reporter to confirm.

[DJPaul]

I'm guessing this was introduced possibly by the post type comment tracking change.

[DJPaul]

See also ​https://buddypress.org/support/topic/php-fatal-error-possible-bug/ for similar but maybe the same root cause.

[mi423]

Activity is disabled.

[nyodulf]

Replying to DJPaul:

See also ​https://buddypress.org/support/topic/php-fatal-error-possible-bug/ for similar but maybe the same root cause.

The activity streams component is disabled on the site where that error occurred.

[imath]

Thanks a lot for your comments @mi423 I'm currently working on a patch.

What i'm amazed of is : how come you didn't noticed it in previous versions of BuddyPress ?

I'm pretty sure this could happen with BuddyPress 2.4.3 having the same components activated.

I mean in the end bp_blogs_delete_activity() is using bp_activity_delete_by_item_id() which is located in the Activity component.

I'm guessing this was introduced possibly by the post type comment tracking change.

@DJPaul The only change that could have caused this (although i think it's there for quite a while) is the fact we include /bp-blogs/bp-blogs-activity.php only if the Activity component is active. The function bp_blogs_remove_post() wasn't touched during 2.5.0 dev cycle.

[mi423]

Replying to imath:

What i'm amazed of is : how come you didn't noticed it in previous versions of BuddyPress ?

I'm pretty sure this could happen with BuddyPress 2.4.3 having the same components activated.

I have no idea :) I do not remember if I've seen this error before.

[imath]

@DJPaul @mi423

Sorry I spoke too quick.

I see now what caused this. A bunch of if ( ! bp_is_active( 'activity' ) ) { checks were removed inside functions such as bp_blogs_delete_activity() because the whole file is now loaded only if the activity component is active.

I now see it was a really bad idea.

[imath]

Here's a first patch. I'm going to run some tests and will add a new comment to explain my choice once done.

[imath]

So here's a detailled explanation

In r10544 we did two things:

• Only load bp-blogs/bp-blogs-activity.php if the Activity Component was active
• Remove the bp_is_active( 'activity' ) checks inside bp_blogs_record_activity() and bp_blogs_delete_activity()

This means before this commit, the file was loaded and the checks were making sure the activity component was active before using an Activity component function. Reason why this trouble appeared in 2.5.0 and not before.

I keep on thinking only loading the bp-blogs/bp-blogs-activity.php if the Activity Component is active is a good improvement as it avoids loading a bunch of code that is not used anyway. But this means we now need to do the bp_is_active( 'activity' ) before using bp_blogs_record_activity() or bp_blogs_delete_activity().

That's what is doing ​6937.patch. I've tested it having the Blogs component active and the Activity one deactivated on multisite/non multisite and it's fixing the issue.

[mi423]

​6937.patchworks for me.
No error on empting trash ;)

cheers
GJ

[r-a-y]

I keep on thinking only loading the bp-blogs/bp-blogs-activity.php if the Activity Component is active is a good improvement as it avoids loading a bunch of code that is not used anyway.

Darn it! This was my fault! :(

Here's an alternate patch that moves the activity function calls from bp-blogs-functions.php to bp-blogs-activity.php.

Benefits are no more bp_is_active() checks and code is hooked. The code changes may be too extreme for v2.5.1, so I'm thinking we roll with imath's fix for v2.5.1 and look at my patch for v2.6.0 instead.

To whomever commits, feel free to give me anti-props! :)

[imath]

Replying to r-a-y:

Darn it! This was my fault! :(

I should have checked too, so we're on the same boat :)

I really like 6937.activity.patch 's approach. Thanks for adding it.

[DJPaul]

🚣

[DJPaul]

What's the proposed plan? imath's patch for 2.5.1, r-a-y's for 2.6, or what?

I have not tested either patch but both patches look good, so please proceed.

[imath]

I think it's safest to go with 6937.patch for 2.5.1 as @mi423 confirmed it was fixing the issue.

Once committed, i'll leave this ticket open so that we can improve it the way @r-a-y suggested in his patch.

[imath]

In 10631:

Make sure the Activity Component is active before using functions relative to it inside the Blogs component.

r10544 introduced a side effect on the BuddyPress Emails feature when the Blogs component is activated and the Activity one is deactivated.

This changeset removed some bp_is_active() checks inside bp_blogs_delete_activity() and bp_blogs_record_activity() in favor a conditional load of the whole bp-blogs/bp-blogs-activity.php file.

Unfortunately, an issue appeared when the Activity component is not active because these functions are used when some WordPress hooks are fired, for instance when a post type is deleted.

For the 2.5 Branch we are bringing back the bp_is_active() checks before using the two functions mentioned earlier. For next release, we will carry on moving all the code specific to the activity component inside the bp-blogs/bp-blogs-activity.php file.

See #6937 (branch 2.5)

[imath]

In 10632:

Make sure the Activity Component is active before using functions relative to it inside the Blogs component.

r10544 introduced a side effect on the BuddyPress Emails feature when the Blogs component is activated and the Activity one is deactivated.

This changeset removed some bp_is_active() checks inside bp_blogs_delete_activity() and bp_blogs_record_activity() in favor a conditional load of the whole bp-blogs/bp-blogs-activity.php file.

Unfortunately, an issue appeared when the Activity component is not active because these functions are used when some WordPress hooks are fired, for instance when a post type is deleted.

For the 2.5 Branch we are bringing back the bp_is_active() checks before using the two functions mentioned earlier. For next release, we will carry on moving all the code specific to the activity component inside the bp-blogs/bp-blogs-activity.php file.

See #6937 (trunk)

[DJPaul]

Is 6937.activity.patch ready for review/testing, or does it require further work?

[DJPaul]

Let's refresh Ray's patch and get it in.

[r-a-y]

activity.patch is refreshed.

Before committing, this patch does introduce three new functions for hook purposes:

• bp_blogs_record_activity_on_site_creation()
• bp_blogs_delete_new_blog_activity_for_site()
• bp_blogs_delete_activity_for_site()

Let me know if any of these functions should be renamed to something more palatable.

[DJPaul]

Is this done? Can we commit it?

[r-a-y]

See what I wrote in comment:23. I just need a quick check on the names of the new functions before committing.

[DJPaul]

@r-a-y Names seem fine, nice and descriptive!

[r-a-y]

In 10730:

Blogs: Introduce new parameter - $no_activity to 'bp_blogs_new_blog' action.

See #6937.

[r-a-y]

In 10731:

Blogs: Move activity-related code from bp-blogs-functions.php to bp-blogs-activity.php.

Instead of hardcoding activity code and needing to check for the Activity
component in bp-blogs-functions.php, this commit moves this activity code
code to bp-blogs-activity.php and also hooks this code to appropriate
blog actions.

This allows developers to easily remove said activity hook if needed.

Fixes #6937.