Fix null activity objects causing PHP warnings

[GaryJones]

Problem

When activity rows are deleted from the database but their IDs are still
referenced (e.g. in cached query results), BP_Activity_Activity::get_activity_data()
adds false entries to the $activities array. This happens because
wp_cache_get() returns false for missing cache keys, and the result
is unconditionally appended to the array regardless of validity.

Downstream code then iterates over these null entries and attempts
property access, producing three PHP warnings:

PHP Warning: Attempt to read property "id" on null in .../class-bp-activity-activity.php on line 895
PHP Warning: Attempt to read property "mptt_right" on null in .../class-bp-activity-activity.php on line 1641
PHP Warning: Attempt to read property "component" on null in .../bp-friends-activity.php on line 221

Fix

1. get_activity_data() — wrap the $activities[] = $activity
   assignment with if ( ! empty( $activity ) ) so that only valid
   objects are added to the return array. This is the root cause fix
   and prevents all three warnings.

2. bp_friends_prefetch_activity_object_data() — add a defensive
   is_object() check to skip non-object entries, protecting against
   any code path that might pass invalid data into this function.

Testing

1. Delete an activity from the bp_activity table without clearing
   the object cache or any referencing queries
2. Load the activity feed — the three warnings above should no longer
   appear
3. Normal activity feed behaviour is unaffected

[chetansatasiya]

@claude Review the PR

[claude]

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

---

PR Review: Fix null activity objects causing PHP warnings

• Gather context and understand the request
• Read changed files and diff
• Analyze fix correctness and edge cases
• Review coding standards compliance
• Post final review feedback

---

Summary: This PR fixes three PHP warnings triggered when activity rows are deleted from the database but their IDs remain referenced (e.g. in a query result). The root cause is in get_activity_data(), where wp_cache_get() returns false for stale IDs and that false was unconditionally appended to $activities. A second defensive guard is added in bp_friends_prefetch_activity_object_data().

---

src/bp-activity/classes/class-bp-activity-activity.php

Root cause fix — correct and well-targeted.

Before the fix the loop was:

$activity = wp_cache_get( $activity_id, 'bp_activity' );
if ( ! empty( $activity ) ) {
    // integer casts, action string...
}
$activities[] = $activity;   // always appended — bug

wp_cache_get() returns false for a missing key, so deleted activities that have been evicted from the object cache (or simply never cached because they no longer exist in the DB) result in false being pushed into $activities. Downstream code at lines 894–895, 1641–1643, and bp-friends-activity.php:221 then attempts object-property access on false, producing the three warnings described.

The fix is correct. ! empty( false ) → true, so false is excluded.

Minor style observation (no defect):
After the fix there are now two consecutive if ( ! empty( $activity ) ) blocks in the same loop — one for casting, one for appending. These could be merged into a single block, which would be marginally cleaner:

$activity = wp_cache_get( $activity_id, 'bp_activity' );
if ( ! empty( $activity ) ) {
    $activity->id                = (int) $activity->id;
    // ... remaining casts ...
    if ( empty( $activity->action ) ) {
        $activity->action = bp_activity_generate_action_string( $activity );
    }
    $activities[] = $activity;
}

This is a refactor beyond the bug-fix scope, so keeping it as-is is a reasonable choice for a minimal, reviewable patch.

---

src/bp-friends/bp-friends-activity.php

Appropriate defensive guard.

if ( ! is_object( $activity ) ) {
    continue;
}

Using is_object() here (rather than ! empty()) is the right level of specificity — the subsequent code accesses $activity->component and $activity->secondary_item_id, so an actual object is required. If either false, null, a scalar, or an array slips through, this guard catches it.

With the root-cause fix in place this check will almost never fire in practice, but it is good defensive programming since bp_activity_prefetch_object_data is a public filter and external code could pass in non-object entries.

---

General observations

Area	Assessment
Correctness	✅ Fixes all three documented warnings
Coding standards	✅ WordPress tabs, ! empty() / is_object(), space after if
Backward compatibility	✅ Activities that exist continue to work; only missing/stale entries are skipped
Edge cases	✅ false, null, 0, "" all pass empty() and are excluded
Tests	⚠️ No unit test added for the regression. A test that mocks wp_cache_get returning false for a subset of IDs would lock this in and prevent regressions.
Negative caching	🔍 If a caller explicitly sets a cache entry to false (negative cache) for a known-missing ID, the fix correctly excludes it. This is the desired behaviour.

---

Verdict

The fix is correct, minimal, and safe to merge. The only gap worth noting is the absence of a unit test to prevent this from regressing. If the project's test suite can exercise get_activity_data() with a stale cache entry, a test would be a worthwhile follow-up.