ITADN

Missing null guard in Elements::getRecentActivity() — TypeError when an activity row references a soft-deleted user

#19081Closedwatarutmnh 创建于 2026-06-12
W
watarutmnhcommented
### Description `craft\services\Elements::getRecentActivity()` constructs `new ElementActivity($users[$result['userId']], ...)` without checking that the user was loaded. When an `elementactivity` row's `userId` points to a soft-deleted (trashed) user, that user is absent from `$users`. In production (devMode off, `E_WARNING` suppressed) the undefined key yields `null`, which is passed to the non-nullable `$user` param and throws a `TypeError`. With devMode on, the undefined-key warning is promoted to an `ErrorException` before the constructor is reached. 6.x already guards this — the rewritten `src/Element/ElementActivity.php` (PR #18653) has `if (!isset($users[$result->userId])) { continue; }`. The 5.x (and 4.x) `Elements::getRecentActivity()` has no equivalent guard. ### Error (production) ``` TypeError: craft\models\ElementActivity::__construct(): Argument #1 ($user) must be of type craft\elements\User, null given, called in .../src/services/Elements.php on line 2958 ``` ### Root cause The user lookup (5.10.5, ~line 2902) uses `->status(null)` but not `->trashed(null)`: ```php $users = User::find()->id($userIds)->status(null)->indexBy('id')->all(); ``` `status(null)` only disables the status filter; `trashed` defaults to `false`, so `elements.dateDeleted IS NULL` still excludes soft-deleted users. The construction a few lines below (~line 2958) doesn't null-check `$users[$result['userId']]`, unlike the element lookup just above it (`if (!$resultElement) { …; continue; }`). Only *soft-deleted* users leave orphan rows: `elementactivity.userId → users.id` is `ON DELETE CASCADE`, and `users.id → elements.id` is `ON DELETE CASCADE`, so hard-deleting a user cascades its activity rows away; soft delete only sets `elements.dateDeleted`, leaving the rows. ### Steps to reproduce 1. As a CP user U, view/edit an element (creates an `elementactivity` row for U). 2. Delete user U (move to Trash) within the recent-activity window (rows with `timestamp > now() - 1 minute`). 3. As another user, open the same element's edit screen → the `elements/recent-activity` poll calls `getRecentActivity()` → 500. ### Affected versions 5.x (5.10.5, line 2958) and 4.x (`getRecentActivity()` is `@since 4.5.0`; same unguarded construction at `src/services/Elements.php` ~line 2496). PHP 8.4. ### Suggested fix Add the same guard 6.x uses, right before `new ElementActivity(...)`: ```php if (!isset($users[$result['userId']])) { continue; } ``` 6.x skips silently; the 5.x element guard a few lines above logs a `Craft::warning()`, so one could be added here for parity — happy to follow your preference. (Adding `->trashed(null)` to the user query would also avoid the null, but it would surface just-deleted users in the indicator; 6.x chose the skip-guard.) Happy to open a PR.
关闭于 2026-06-13 1 条评论