Skip to content

Add null safety checks to prevent PHP warnings#4905

Open
GaryJones wants to merge 2 commits intobuddyboss:releasefrom
GaryJones:fix/null-safety-php-warnings-v2
Open

Add null safety checks to prevent PHP warnings#4905
GaryJones wants to merge 2 commits intobuddyboss:releasefrom
GaryJones:fix/null-safety-php-warnings-v2

Conversation

@GaryJones
Copy link
Copy Markdown

Problem

Four places in the codebase lack null/undefined checks, producing PHP warnings or deprecation notices on PHP 8.1+.

1. Undefined title_required property on activity objects

In bp-groups/bp-groups-activity.php, when updating a group activity, the code accesses $activity->title_required without a fallback. If the property doesn't exist on the activity object, PHP emits an undefined property warning.

Fix: Add ?? false null-coalescing fallback.

2. Null passed to wp_kses_post() from notification icon

In bp-notifications/bp-notifications-functions.php, bb_notification_small_icon() can return null, which is then passed directly to wp_kses_post(). On PHP 8.1+, this triggers a deprecation warning since wp_kses_post() expects a string.

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

3. Missing progress data in profile completion widget

In class-bp-xprofile-profile-completion-widget.php, the widget accesses $user_progress['completion_percentage'] without checking whether $user_progress is populated or the key exists. When no applicable profile fields are configured, this produces undefined key warnings.

Fix: Add an early return when $user_progress is empty or missing the completion_percentage key.

4. Uninitialised $result variable in topics manager

In class-bb-topics-manager.php, the $result variable is only assigned inside a conditional block (if ( 'migrate' === $migrate_type && $new_topic_id )). When the condition is false, $result is undefined for any subsequent code that references it.

Fix: Initialise $result = array() before the conditional.

Test plan

  • Update a group activity where title_required is not set on the activity object — no PHP warning
  • View notifications where bb_notification_small_icon() returns null — no deprecation notice
  • Display the profile completion widget with no applicable xprofile fields configured — no undefined key warnings
  • Use the topics manager with a non-migrate action — no undefined variable notice

Four small fixes for undefined property/variable warnings and null
parameter deprecations on PHP 8.1+:

1. Null-coalescing fallback for missing title_required property on
   activity objects (bp-groups-activity.php).

2. Null-coalescing fallback before passing bb_notification_small_icon()
   result to wp_kses_post() (bp-notifications-functions.php).

3. Early return in profile completion widget when user_progress is
   empty or missing completion_percentage key
   (class-bp-xprofile-profile-completion-widget.php).

4. Initialise $result to empty array before conditional assignment
   in topics manager (class-bb-topics-manager.php).
@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 36s —— View job


PR Review: Add null safety checks to prevent PHP warnings

  • Gather context and understand request
  • Read changed files
  • Analyze code changes
  • Post review feedback

Overall assessment: ✅ Looks good — all four fixes are correct, minimal, and safe.

The PR addresses real PHP 8.1+ warnings/deprecation notices without changing behaviour on the happy path. Here are observations per file:


src/bp-groups/bp-groups-activity.phptitle_required property fallback

// Before
'title_required' => ! empty( $args['title_required'] ) ? $args['title_required'] : $activity->title_required,
// After
'title_required' => ! empty( $args['title_required'] ) ? $args['title_required'] : ( $activity->title_required ?? false ),

✅ Correct. The ?? operator properly handles an absent property (undefined properties evaluate to null in PHP, not to an undefined-variable warning when using ??). Using false as the fallback preserves the boolean semantics of the flag. The null-coalescing operator is already used 238+ times in the codebase, confirming it is an accepted pattern here.


src/bp-notifications/bp-notifications-functions.php — null return from bb_notification_small_icon()

// Before
echo wp_kses_post( bb_notification_small_icon( $component_action, true, $notification ) );
// After
echo wp_kses_post( bb_notification_small_icon( $component_action, true, $notification ) ?? '' );

✅ Correct. bb_notification_small_icon() has two bare return; exit paths (lines 1300 and 1317) that implicitly return null. The ?? '' correctly coalesces to an empty string before wp_kses_post() is called, avoiding the PHP 8.1 deprecation for passing null to a typed parameter.

Minor observation (not a blocker): The source function itself could declare string return type and return '' instead of bare return;. That would be a cleaner fix at the source, but fixing it at the call site is equally valid and keeps the change footprint small.


src/bp-xprofile/classes/class-bp-xprofile-profile-completion-widget.php — missing completion_percentage key

if ( empty( $user_progress ) || ! isset( $user_progress['completion_percentage'] ) ) {
    return;
}

✅ Correct. The guard is placed in the right spot — after the existing "nothing selected" early return — and checks both that $user_progress is populated and that the specific key exists. Silent bail-out (no output) matches the widget's existing behaviour for other edge cases.


src/bp-core/classes/class-bb-topics-manager.php — uninitialised $result

$result = array();
if ( 'migrate' === $migrate_type && $new_topic_id ) {
    $result = $this->bb_migrate_topic( ... );
    ...
}
...
wp_send_json_success( array( 'message' => ..., 'topic' => $result ) );

✅ Correct. $result is referenced unconditionally at the wp_send_json_success call on line 1803, so initialising it to array() before the conditional is the right fix.

Note (pre-existing, not introduced by this PR): When migrate_type !== 'migrate', the code will still reach wp_send_json_success with 'topic' => array(). This is likely fine for the current callers, but worth noting if the handler ever needs to support additional non-migrate action types.


General

  • Code follows WordPress coding standards (tabs, Yoda conditions, spacing).
  • All changes are minimal — no unrelated refactoring.
  • No security concerns introduced.
  • The ?? operator (PHP 7.0+) is consistent with 238 existing usages in the codebase.

No blocking issues found. The PR is ready to merge.

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