Add null safety checks in forums code

[GaryJones]

Problem

Three places in the forums code lack null safety checks, causing PHP warnings, deprecation notices, or potential fatal errors on PHP 8.1+.

1. Null post object access in forum group detection

In bp-forums/functions.php, get_post( $forum_id ) can return null when the forum post does not exist (e.g. it was deleted or the ID is invalid). The code immediately accesses $current_post->post_parent without checking, causing a fatal error.

Fix: Add $current_post && before accessing the property.

2. REST reply permission check logic error

In class-bp-rest-reply-endpoint.php, the update_item_permissions_check method calls $this->get_item_permissions_check() which may return a WP_Error. The code then unconditionally compares the reply author, even when $retval is already an error. This can cause unexpected behaviour or mask the original permission error.

Fix: Add true === $retval && to only evaluate the author check when the prior permission check passed.

3. Null passed to sanitize_title() for REST topic tag

In class-bp-rest-topics-endpoint.php, $request->get_param( 'tag' ) returns null when the parameter is not provided. Passing null to sanitize_title() triggers a deprecation warning on PHP 8.1+.

Fix: Add ?? '' null-coalescing fallback.

Test plan

• Access a forum where the forum post has been deleted — should not fatal
• Make an unauthenticated REST API request to update a reply — should return a proper permission error
• Make a REST API request to list topics without a tag parameter on PHP 8.1+ — no deprecation warning

[chetansatasiya]

@GaryJones Thanks for the PR and it will be added in our upcoming release.

[chetansatasiya]

@claude Review the PR

[claude]

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

---

PR Review: Add null safety checks in forums code

• Gather context and understand the request
• Read changed files and diff
• Review null safety fixes for correctness
• Check coding standards compliance
• Post review feedback

---

Overall, this is a well-scoped PR with three accurate, minimal fixes that address real PHP 8.1+ compatibility issues. All changes are correct and follow existing code style. Here's the analysis of each fix:

---

Fix 1 — src/bp-forums/functions.php:860 — Null post object access

- if ( $current_post->post_parent ) {
+ if ( $current_post && $current_post->post_parent ) {

Verdict: Correct.

get_post() returns null when a post doesn't exist (deleted or invalid ID). Accessing ->post_parent on null is a fatal error on PHP 8.0+. The added null guard is minimal and correct.

When $current_post is null, the condition short-circuits and falls into the else branch which sets $reached_the_top = true, cleanly exiting the while loop — the right behavior when a post no longer exists in the hierarchy.

---

Fix 2 — src/bp-forums/classes/class-bp-rest-reply-endpoint.php:1770 — REST reply permission short-circuit

- if ( bbp_get_user_id( 0, true, true ) !== $reply->post_author && ! current_user_can( 'edit_reply', $request['id'] ) ) {
+ if ( true === $retval && bbp_get_user_id( 0, true, true ) !== $reply->post_author && ! current_user_can( 'edit_reply', $request['id'] ) ) {

Verdict: Correct.

$this->get_item_permissions_check() can return a WP_Error (e.g. when private-network restriction is active and the user is not logged in). Without the guard, the author comparison would run even when $retval already holds an error, potentially masking the original permission error message.

The true === $retval strict check matches the pattern used consistently throughout get_item_permissions_check() itself (lines 506, 516, 526), so this is idiomatic and consistent with the existing code.

Minor observation (not a blocker): $reply = bbp_get_reply( $request->get_param( 'id' ) ) on the line above is still fetched unconditionally, meaning a DB lookup occurs even when $retval is already a WP_Error. A more complete fix would move that fetch inside the true === $retval branch to avoid the unnecessary query, but this doesn't affect correctness — only efficiency.

---

Fix 3 — src/bp-forums/classes/class-bp-rest-topics-endpoint.php:254 — Null coalescing for sanitize_title

- $tag = sanitize_title( $request->get_param( 'tag' ) );
+ $tag = sanitize_title( $request->get_param( 'tag' ) ?? '' );

Verdict: Correct.

$request->get_param( 'tag' ) returns null when the parameter is absent from the request. Passing null to sanitize_title() triggers a PHP 8.1 deprecation notice (Deprecated: Passing null to parameter #1 ($title) of type string). The ?? '' fallback is idiomatic, minimal, and sufficient — sanitize_title( '' ) returns '', so the subsequent ! empty( $tag ) check naturally skips the tag-based query.

---

Summary

File	Fix	Verdict
bp-forums/functions.php	Null guard before ->post_parent access	✅ Correct
class-bp-rest-reply-endpoint.php	Guard author check with true === $retval	✅ Correct
class-bp-rest-topics-endpoint.php	Null coalesce for sanitize_title() arg	✅ Correct

All three fixes are accurate and safe to merge. The only non-blocking observation is the unnecessary bbp_get_reply() call in Fix 2 when $retval is already a WP_Error, which could be addressed in a follow-up if desired.