Activity link moderation doesn't output useful error message to end users

[r-a-y]

Reported on the buddypress.org forums:

In a BP Group, when posting an activity update, commenting on an activity update, or replying to an existing activity update, if the post contains more than the allowed number of links for WP comment moderation (as set in Admin >> Settings >> Discussion >> Comment Moderation >> “Hold a comment in the queue if it contains X links”) the post is rejected with an unhelpful error message: “There was an error posting your reply. Please try again.”

​https://buddypress.org/support/topic/buddypress-groups-activity-posting-fails-based-on-wp-comment-moderation-setting/

I haven't verified the report, but this sounds about right.

The bare minimum we need to do is add a better error message.

This will require some form of WP_Error addition and handling, similar to what was done in #6535.

On the flip side, the most work that would need to be done is to introduce some form of moderation status:
​https://buddypress.trac.wordpress.org/browser/tags/2.3.4/src/bp-activity/bp-activity-filters.php?marks=149-152#L136

But that's another ticket for another day :)

[boonebgorges]

Marked #6819 as a duplicate.

[hnla]

Related aspect in comment_max_links & bp $num_links bailing out if '>=' rather than '>' on ticket: #6821

[r-a-y]

01.patch is a first pass at generating better error messages when someone trips up the moderation settings in the activity stream.

There are still some tweaks that need to be made to the moderation check such as grabbing the "Settings > Discussion" moderation settings from the BP root blog, but it's getting late here :)

I'll add some unit tests a bit later on, but let me know what you think of the approach or whether we should just suspend the moderation feature as discussed in #6822.

[r-a-y]

Refreshed for trunk.

[hnla]

@r-a-y
Tested refreshed patch. This is a vast improvement on what we had and addresses some of the concerns raised in #6822 vis a vis:

• Add a status update, exceed links allowed, receive error message, however the status update is actually accepted and displayed with no actual moderation and we can see the update in user screens and members loops.

Now we have stronger messages highlighting the exact reason.

For links we halt and notify there are too many however now it's clearer why the error and we do NOT publish those links regardless as we were before, user does have the option to review those links and amend as we have retained the textarea data.

We correctly stop blacklisted words from being displayed, although we still do not add the comment to the WP moderation queue - this behaviour is better than posting the comment though.

For WP spam list words we do not trap those and do publish them although these are meant to be held too.

Posting a comment to a WP post from the act stream is still problematical we acknowledge an issue with the comment but not what i.e bad-word, spam-word, or too many links.

Having thrown up the error message and not posted the comment in the act stream we find that in visiting the post in question directly the comment has been posted bypassing the WP moderation queue.

I think we have a real issue here in providing a means to circumvent the WP process and user set blacklists, links etc.

The error message provision doesn't tackle that aspect although it does improve the BP status update behaviour and puts in place a proper WP_Error handling class?

imho I do think we still need to either pull moderation altogether until we can correct things or decide to tackle this fully but I would like my cake and eat it and add the patch here as it does start to put in place proper error handling?

[r-a-y]

@hnla - Thanks for testing!

Posting a comment to a WP post from the act stream is still problematical we acknowledge an issue with the comment but not what i.e bad-word, spam-word, or too many links.

I missed doing activity comment checks for moderation. 02.patch should address this.

The "Comment Moderation" section from the "Settings > Discussion" page should now be implemented for activity items.

In BuddyPress, when an activity update or comment matches any of these rules, we do not record an activity item into the database.

In WordPress, if a post comment matches any of these rules, WP does record the post comment, but sets the 'comment_status' DB field to 0.

---

For WP spam list words we do not trap those and do publish them although these are meant to be held too.

I noticed that I missed implementing the "Comment Blacklist" check from the "Settings > Discussion" admin page. I hope this is what you meant by WP spam list words, hnla!

However, this also presents its own set of problems. BuddyPress currently sets activity items that have a blacklisted word as activity spam (#4284). Since the activity item is technically recorded, the way I have set up activity error handling means I cannot throw up a custom error message.

To outline what WordPress does, they mark blacklisted comments to either 'trash' or 'spam' in the 'comment_status' DB field. I don't believe they add a user message if a comment hits the blacklist. Needs testing.

We could workaround this another way by doing spam checks after an activity item is recorded and then adding a custom error message that way. This adds another DB query though.

Or we can simply stop recording activity items that match a blacklisted word.

---

imho I do think we still need to either pull moderation altogether until we can correct things or decide to tackle this fully but I would like my cake and eat it and add the patch here as it does start to put in place proper error handling?

Activity moderation is a big task. In order to match WP functionality, we would need to introduce an 'activity_status' DB column in the activity DB table and rework some of our functions to handle recording this new flag. There are also admin UI considerations that would need to be done to the Activity admin dashboard, as well as other things like whether we should tie activity moderation to WP comment moderation settings?

See #2786.

I do agree that the error handling in this ticket is far and away better than what we currently have and improves the UX greatly. But whether we want to hold off on this for #2786 requires dev feedback.

[hnla]

I noticed that I missed implementing the "Comment Blacklist"
hope this is what you meant by WP spam list words, hnla!

Sorry, yes I was confusing things with terminology, not sure why I used the word 'Spam', but WP provides for two status labels 'pending', and 'spam'

'Blacklist' == words prohibited and if found in comment trashed by WP and no message is shown to user - this is a silent action!.

'Comment Moderation' == kicks in on either exceeding a set number of links or inclusion of a proscribed work in moderation list and the offending comment is queued in the 'Pending' list ( not 'Spam'! .

We correctly trapped excessive links and words in the moderation panel ( previously my ref to 'spam list')

Yes missed the blacklist in this implementation (prior to .02 patch), blacklisted words that would cause WP to trash comment are published in the act stream.

However again crux of our issue with post comments remains that whether we publish act stream comment or trap it with error message we do pass them through to the post proper bypassing the WP moderation or blacklisting.

Activity moderation is a big task. In order to match WP functionality, we would need to introduce an 'activity_status'...

I began to realise this when I was looking at code base to see what could be done, in lieu of being able to tackle this we need to ensure if the moderation /blacklist check returns true we need to unlink the post sync.

Or we can simply stop recording activity items that match a blacklisted word.

Yes we need to do this for blacklist words, that will roughly equate to that which WP does (although we technically destruct data where WP retains it for admins to deal with in 'trash')

[hnla]

Testing the .02 patch:

Adding a comment to a post from the act stream with a blacklist word in results in:

The comment isn't trapped with error message and appears to be initially published to stream(i.e ajax wise before page refresh) subsequently comment is not published though!

Comment is seen later to be added to BP activity spam list in dashboard.

Comment however IS added to the WP post comment list.

Adding a comment with a moderated word to a post from the act stream results in:

BP trapping the comment with an error message.

NOT publishing the comment to the WP post ( good!).

NOT adding the comment to the BP activity spam list in dashboard.

[boonebgorges]

I feel the same way about this approach as I felt in ​https://buddypress.trac.wordpress.org/ticket/6535#comment:4 - it stinks that an error_type param is the only way to ensure backward compatibility, but so be it.

I have not tested all of @hnla's scenarios, but the general strategy looks good to me.

[hnla]

Lets commit what we have for 2.6!

But make a mental note that we still need to look at issues where we may push comments through to a post that WP settings wouldn't allow!

[r-a-y]

Adding a comment to a post from the act stream with a blacklist word in results in:
Comment is seen later to be added to BP activity spam list in dashboard.
Comment however IS added to the WP post comment list.

I've created #7107 for this specific issue.

hnla and I were discussing if the "Comment Blacklist" option should just block activity items from being recorded instead of marking them as spam as we do now. FWIW, for "Comment Moderation", we currently just block the activity items from being recorded.

[r-a-y]

In 10858:

Activity: Introduce error handling to the BP_Activity_Activity class.

To support frontend user messages for activity failures, we need the
ability to add errors.

This commit:

• Introduces the $errors object and $error_type property to the BP_Activity_Activity class.
• Adds an 'error_type' parameter to all applicable, activity functions.
• Modifies bp-legacy to output an error message for activity failures.

See #6719.

[r-a-y]

In 10859:

Moderation: Add WP error handling.

bp_core_check_for_moderation() and bp_core_check_for_blacklist() now
supports returning WP errors.

See #6719.

[r-a-y]

In 10860:

Activity: Set WP error if activity content hits a moderation filter.

One important change we are making here is if the activity content matches
a word from the "Comment Blacklist" discussion option from WordPress, we
now stop recording the activity item.

Previously, we were still recording the activity item, but marking it as
spam.

See #6719.

[r-a-y]

Okay, going to mark this one as fixed.

Thanks for the feedback everyone!

[r-a-y]

In 10869:

Activity: Bail out of activity post functions if an error occurred.

See #6719.

[mercime]

@r-a-y Might I suggest changing the word inputted to posted in the following error messages:

• 'You have inputted too many links' to 'You have posted too many links.'

• 'You have inputted an inappropriate word.' to 'You have posted an inappropriate word.'

Thanks for your work on this.

[r-a-y]

Sounds good, @mercime!

Will make those changes early next week. Also gives others a chance to chime in before I get to this.

[dcavins]

I prefer the strings suggested by @mercime. Even though 'inputted' is not technically incorrect, it sounds funny when I say it out loud. (Related: I've never putted something in my toolbox. You wonder where the words diverged, ha ha.)

[r-a-y]

In 10881:

Moderation: Change 'input' to 'posted' for error message strings.

Props mercime.

See #6719.