`empty()` check for activity content after kses filtering

[oztaser]

The BP_Activity_Activity()->save() method inserts an activity with empty content If the whole activity content has not allowed HTML elements. Because bp_activity_filter_kses returns an empty value.

I think we should check the activity content before running update/insert query.

[imath]

Hi @oztaser, thanks for your suggestion and patch, I believe we are using activities with no content to trace things like the last time the user connected to the site or when a group is created.

I’ll need to check. I believe this check exists earlier in the process when someone posts from the activity post form.

[oztaser]

Hi @imath, thank you for your response. I checked and I found last_activity type with no content as you mention. It looks like we can not add a control for all types. But an empty activity_update looks like a problem to me, at least. Because if users post just an iframe, an empty activity is inserted. There is an empty check for activity_update type on bp_activity_post_update function but the content is not filtered there. Maybe we can filter not allowed HTML elements before checking content. Of course, you think this is an issue. I fixed my problem by using bp_activity_before_save action.

I have another suggestion, we must pass a message parameter for bp_activity_missing_component and bp_activity_missing_type errors. Because It's a required parameter for WP_Error->add(). I am not quite sure about what message should return so I am not preparing the patch.

Thanks again.

[imath]

You're welcome :) what about adding a check to the activity type ? eg:

if ( 'activity_update' === $this->type && ! $this->content ) {}

[oztaser]

Hi again @imath , What do you think about 8236.1.patch?

[imath]

Hi! This is shaping great, I think I would do the check a bit later, just after the statement about component and type:

if ( empty( $this->component ) || empty( $this->type ) )

This way you are sure about the fact type is not empty before checking it’s not activity_update.

Thanks a lot for your work on it 👌

[imath]

In 8236.2.patch :

• I'm checking the content once we are sure the type is set as explained in my previous comment.
• I'm introducing a filter bp_activity_type_requires_content to allow plugins to make their activity type's content required.
• I'm adding unit tests.

@oztaser could you test the patch before the end of march to confirm it's fixing your issue so that we can eventually include this in the 6.0.0 first beta release?

[oztaser]

Hi @imath, your patch looks good and fixed the problem. Thanks a lot for your time!

[imath]

In 12604:

Make sure activities requiring content are not inserted empty into db

The activity type activity_update requires a content. If it is not set (eg: this can happen after sanitization), the activity won't be inserted into the database. Plugin developers can use the bp_activity_type_requires_content filter to force the activity type their plugin generated to also require content.

Props oztaser

Fixes #8236