Skip to content

Surface specific moderation errors for activity comments#4898

Open
GaryJones wants to merge 2 commits intobuddyboss:releasefrom
GaryJones:fix/activity-comment-moderation-error-type
Open

Surface specific moderation errors for activity comments#4898
GaryJones wants to merge 2 commits intobuddyboss:releasefrom
GaryJones:fix/activity-comment-moderation-error-type

Conversation

@GaryJones
Copy link
Copy Markdown

Problem

When a group activity comment triggers WordPress's built-in link moderation (Settings → Discussion → "Hold a comment in the queue if it contains X or more links"), users see the generic error:

"There was an error posting your reply. Please try again."

BuddyBoss's own moderation layer (bp_core_check_for_moderation()) produces a specific WP_Error with the message "You have posted too many links", but this error is silently discarded before it reaches the AJAX response. The same problem affects any error originating from bp_core_check_for_moderation(), including matches against the moderation word list.

Root cause

bp_nouveau_ajax_new_activity_comment() in bp-templates/bp-nouveau/includes/activity/ajax.php calls bp_activity_new_comment() without specifying error_type, so it defaults to 'bool'.

The call chain is:

  1. bp_nouveau_ajax_new_activity_comment()bp_activity_new_comment() (error_type: 'bool')
  2. bp_activity_new_comment()bp_activity_add() (error_type: 'bool')
  3. bp_activity_add()$activity->save()
  4. save() fires bp_activity_before_savebp_activity_check_moderation_keys()bp_core_check_for_moderation() returns WP_Error('bp_moderation_too_many_links', 'You have posted too many links')
  5. The callback stores the error on $activity->errors and sets $activity->component = false to force failure
  6. save() sees empty component, checks error_type — since it's 'bool', returns false (the WP_Error is discarded here)
  7. false propagates back up through bp_activity_add()bp_activity_new_comment() → the AJAX handler
  8. The AJAX handler checks $bp->activity->errors['new_comment'] but this was never populated (that only happens for errors within bp_activity_new_comment() itself), so the generic fallback message is sent

Fix

Two changes in bp_nouveau_ajax_new_activity_comment():

  1. Pass 'error_type' => 'wp_error' to bp_activity_new_comment() so WP_Error objects propagate through the call chain instead of being converted to false

  2. Check is_wp_error( $comment_id ) on the return value and extract the actual error message for the AJAX response

The existing $bp->activity->errors['new_comment'] fallback is preserved as an elseif for backwards compatibility.

This is consistent with how bp_activity_new_comment() was designed to work — the error_type parameter has been supported since BuddyPress 2.6.0.

Steps to reproduce

  1. Set Settings → Discussion → "Hold a comment in the queue if it contains 2 or more links
  2. Navigate to a group activity feed
  3. Post an activity comment containing 2+ links
  4. Observe "There was an error posting your reply. Please try again." instead of "You have posted too many links"

Test plan

  • With comment_max_links set to 2, post a group activity comment with 2+ links → should see "You have posted too many links"
  • Post a comment with fewer links → should succeed normally
  • Post a comment containing a moderation word list match → should see the appropriate moderation error
  • Post a normal comment with no links or moderation triggers → should succeed normally

The AJAX handler in bp_nouveau_ajax_new_activity_comment() calls
bp_activity_new_comment() with the default error_type of 'bool'.
This causes WP_Error objects from bp_core_check_for_moderation()
(e.g. "You have posted too many links") to be silently converted
to false before the AJAX handler can read them.

Users see the generic "There was an error posting your reply.
Please try again." instead of the specific moderation message.

Pass 'error_type' => 'wp_error' and check is_wp_error() on the
return value so that specific error messages are surfaced.
@welcome
Copy link
Copy Markdown

welcome bot commented Mar 2, 2026

Thanks for your pull request. It looks like this may be your first contribution to the BuddyBoss Platform open source project. Please note that this project and all contributions to it are public and bounded by the GPL v2.0 license, and that a record of the contribution (including all personal information you submit with it, including your full name and email address) is maintained indefinitely and may be redistributed with this project. If you are not okay with these terms, please close this pull request. Alternatively, you can let us know about your concerns by adding a comment to this pull request.

@chetansatasiya
Copy link
Copy Markdown
Contributor

@claude Review the PR

@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

Claude finished @chetansatasiya's task in 1m 33s —— View job


PR Review

  • Gather context and read changed files
  • Analyze the diff and review the implementation
  • Post review feedback

Summary

This is a clean, minimal, and well-targeted fix. The root cause analysis in the PR description is accurate, and the implementation correctly uses the existing error_type API that has been available since BuddyPress 2.6.0.


Code Review: src/bp-templates/bp-nouveau/includes/activity/ajax.php

Change 1 — 'error_type' => 'wp_error' (line 525)

Correct. Verified the full call chain:

  • bp_activity_new_comment() passes error_type through to bp_activity_add()
  • bp_activity_add() sets $activity->error_type and, at line 2157, returns the WP_Error from $activity->save() when error_type === 'wp_error'
  • bp_activity_new_comment() at line 3175 returns the WP_Error directly on failure

This change means bp_activity_new_comment() will now return a WP_Error on any failure instead of false, so the moderation error from bp_core_check_for_moderation() will propagate all the way up.

Change 2 — is_wp_error( $comment_id ) handling (lines 529–545)

Correct and well-structured. The elseif fallback to $bp->activity->errors['new_comment'] is preserved for backward compatibility, which is important since other code paths (e.g. filters or overrides) might still set that property.

The error message is correctly escaped via esc_html() before being placed in the HTML response.


Minor Observations (non-blocking)

Redundant ! $comment_id in the outer condition (line 529):

if ( ! $comment_id || is_wp_error( $comment_id ) ) {

With error_type => 'wp_error', bp_activity_new_comment() can never return false or 0 — all failure paths now return a WP_Error. The ! $comment_id part is effectively dead code in this new configuration. It's harmless and serves as a safety net, but worth noting.

unset( $bp->activity->errors['new_comment'] ) not needed in new branch:

The existing elseif branch (line 544) unsets $bp->activity->errors['new_comment'] after using it. The new is_wp_error( $comment_id ) branch doesn't need this because with error_type => 'wp_error', $bp->activity->errors is never populated. This is correct behaviour.


Verdict

The fix is correct, minimal, secure (error messages are escaped), and backward-compatible. It uses the designed extension point (error_type parameter) rather than hacking around the problem. No changes needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants