Fix web notifications not loading (#7195)

Notification status hydration compared item_type strictly against
Status::class (App\Models\Status). Rows created before the App\ ->
App\Models\ namespace migration store the legacy 'App\Status' morph-map
alias, so the comparison failed and favourite/comment/mention
notifications came back with no attached status. The web UI filters those
out client-side but keeps paginating (response never empty), leaving the
infinite-scroll loader spinning forever.

- NotificationTransformer + Mastodon NotificationTransformer: match both
  the legacy alias and the current FQCN when hydrating status.
- NotificationService::buildNotification: same alias-aware deleted-item guard.
- NotificationService::getMaxPage/getMinPage: filter out unrenderable
  notifications (status-type without a hydrated status) so the endpoint
  never returns rows the UI discards, fixing pagination termination; warn
  on unexpected notification types.
- Tests for transformer hydration (legacy + current), renderable filtering,
  the unexpected-type warning, and pagination termination.
pull/7216/head
Your Name 2 weeks ago
parent fb72788144
commit c0f29d4a4b

@ -108,7 +108,11 @@ class NotificationService
if (! $epoch) {
NotificationEpochUpdatePipeline::dispatch();
return 1;
$rec = Notification::whereDate('created_at', '>=', now()->subMonths($months)->format('Y-m-d'))
->orderBy('id')
->first();
return $rec ? $rec->id : 1;
}
return $epoch;
@ -142,12 +146,51 @@ class NotificationService
*/
public static function getMaxPage($id = false, $maxId = null, $limit = 10)
{
return self::fetchPage($id, $maxId, 'max', $limit);
return self::fetchPage($id, $maxId, 'max', $limit, self::renderableFilter());
}
public static function getMinPage($id = false, $minId = null, $limit = 10)
{
return self::fetchPage($id, $minId, 'min', $limit);
return self::fetchPage($id, $minId, 'min', $limit, self::renderableFilter());
}
protected static function renderableFilter(): callable
{
// Notification types that are meaningless without an attached status.
$statusTypes = ['comment', 'mention', 'share', 'reblog', 'favourite'];
// Notification types that don't have an attached status.
$otherTypes = array_merge($statusTypes, [
'follow',
'follow_request',
'direct',
'tagged',
'modlog',
'group',
'story:react',
'story:comment',
]);
return function ($n) use ($statusTypes, $otherTypes) {
if (! isset($n['account']['id'])) {
return null;
}
$type = $n['type'] ?? null;
if ($type !== null && ! in_array($type, $otherTypes)) {
Log::warning('NotificationService: unexpected notification type in renderableFilter', [
'type' => $type,
'notification_id' => $n['id'] ?? null,
]);
}
if (in_array($type, $statusTypes) && ! isset($n['status']['id'])) {
return null;
}
return $n;
};
}
/**
@ -527,7 +570,7 @@ class NotificationService
return null;
}
if ($n->item_id && $n->item_type === Status::class && ! $n->item) {
if ($n->item_id && in_array($n->item_type, ['App\Status', Status::class]) && ! $n->item) {
return null;
}

@ -30,7 +30,7 @@ class NotificationTransformer extends Fractal\TransformerAbstract
public function includeStatus(Notification $notification)
{
$item = $notification;
if ($item->item_id && $item->item_type == Status::class) {
if ($item->item_id && in_array($item->item_type, ['App\Status', Status::class])) {
$status = Status::with('media')->find($item->item_id);
if ($status) {
return $this->item($status, new StatusTransformer);

@ -30,7 +30,7 @@ class NotificationTransformer extends Fractal\TransformerAbstract
}
}
if ($n->item_id && $n->item_type == Status::class) {
if ($n->item_id && in_array($n->item_type, ['App\Status', Status::class])) {
$res['status'] = StatusService::get($n->item_id, false);
}

@ -7,9 +7,31 @@ use App\Models\Status;
use App\Models\User;
use App\Services\NotificationService;
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
use Illuminate\Support\Facades\Log;
uses(LazilyRefreshDatabase::class);
/**
* Persist a like notification while forcing the raw item_type string, so tests
* can reproduce legacy ('App\Status') vs current (App\Models\Status) rows.
*/
function makeLikeNotificationWithType(int $profileId, int $actorId, int $statusId, string $itemType): Notification
{
$n = new Notification;
$n->profile_id = $profileId;
$n->actor_id = $actorId;
$n->action = 'like';
$n->item_id = $statusId;
$n->item_type = $itemType;
$n->save();
// Mirror createNotification's cache registration so the page walk sees it.
NotificationService::setNotification($n);
NotificationService::set($n->profile_id, $n->id);
return $n;
}
describe('NotificationService::createNotification', function () {
it('creates a notification and persists it', function () {
$user = User::factory()->create();
@ -186,3 +208,187 @@ describe('NotificationService::firstOrCreateNotification', function () {
expect($first->id)->toBe($second->id);
});
});
describe('NotificationService::getMaxPage renderable filtering (pixelfed#7195)', function () {
it('excludes a like notification whose status has been deleted', function () {
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
$status = Status::factory()->create(['profile_id' => $user->profile_id, 'scope' => 'public']);
$n = NotificationService::createNotification(
$user->profile_id,
$actor->profile_id,
'like',
$status->id,
Status::class
);
// Delete the underlying status so it can no longer be hydrated.
$status->delete();
NotificationService::del($user->profile_id, $n->id);
$page = NotificationService::getMaxPage($user->profile_id, $n->id + 1, 20);
$ids = collect($page['data'])->pluck('id')->map(fn ($v) => (string) $v)->all();
expect($ids)->not->toContain((string) $n->id);
});
it('includes a follow notification (no status required)', function () {
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
$n = NotificationService::createNotification(
$user->profile_id,
$actor->profile_id,
'follow',
$actor->profile_id,
Profile::class
);
$page = NotificationService::getMaxPage($user->profile_id, $n->id + 1, 20);
$ids = collect($page['data'])->pluck('id')->map(fn ($v) => (string) $v)->all();
expect($ids)->toContain((string) $n->id);
});
it('includes a like notification whose status still exists', function () {
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
$status = Status::factory()->create(['profile_id' => $user->profile_id, 'scope' => 'public']);
$n = NotificationService::createNotification(
$user->profile_id,
$actor->profile_id,
'like',
$status->id,
Status::class
);
$page = NotificationService::getMaxPage($user->profile_id, $n->id + 1, 20);
$ids = collect($page['data'])->pluck('id')->map(fn ($v) => (string) $v)->all();
expect($ids)->toContain((string) $n->id);
});
});
describe('NotificationService legacy item_type alias (pixelfed#7195)', function () {
it('hydrates and includes a legacy App\\Status like notification', function () {
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
$status = Status::factory()->create(['profile_id' => $user->profile_id, 'scope' => 'public']);
// Legacy row: item_type stored as the pre-migration 'App\Status'.
$n = makeLikeNotificationWithType($user->profile_id, $actor->profile_id, $status->id, 'App\Status');
$page = NotificationService::getMaxPage($user->profile_id, $n->id + 1, 20);
$item = collect($page['data'])->firstWhere('id', (string) $n->id)
?? collect($page['data'])->firstWhere('id', $n->id);
expect($item)->not->toBeNull();
expect($item['status'] ?? null)->not->toBeNull();
expect((string) $item['status']['id'])->toBe((string) $status->id);
});
it('excludes a legacy App\\Status like notification whose status was deleted', function () {
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
$status = Status::factory()->create(['profile_id' => $user->profile_id, 'scope' => 'public']);
$n = makeLikeNotificationWithType($user->profile_id, $actor->profile_id, $status->id, 'App\Status');
$status->delete();
NotificationService::del($user->profile_id, $n->id);
$page = NotificationService::getMaxPage($user->profile_id, $n->id + 1, 20);
$ids = collect($page['data'])->pluck('id')->map(fn ($v) => (string) $v)->all();
expect($ids)->not->toContain((string) $n->id);
});
});
describe('NotificationService::renderableFilter unexpected type warning', function () {
it('logs a warning for a notification type outside the known set', function () {
Log::spy();
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
// An unknown action -> replaceTypeVerb passes it through unchanged, so
// the page walk sees a type not in the known list.
$n = NotificationService::createNotification(
$user->profile_id,
$actor->profile_id,
'totally-unknown-type',
$actor->profile_id,
Profile::class
);
NotificationService::getMaxPage($user->profile_id, $n->id + 1, 20);
Log::shouldHaveReceived('warning')
->withArgs(fn ($message) => str_contains($message, 'unexpected notification type'))
->atLeast()->once();
});
it('does not warn for a known follow notification', function () {
Log::spy();
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
$n = NotificationService::createNotification(
$user->profile_id,
$actor->profile_id,
'follow',
$actor->profile_id,
Profile::class
);
NotificationService::getMaxPage($user->profile_id, $n->id + 1, 20);
Log::shouldNotHaveReceived('warning');
});
});
describe('NotificationService pagination termination (pixelfed#7195)', function () {
it('returns empty data when every candidate is an unrenderable status notification', function () {
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
// Two like notifications whose statuses are all deleted -> nothing is
// renderable, so the page must come back empty and let the client stop
// paginating instead of looping forever.
$notifs = [];
foreach (range(1, 2) as $i) {
$status = Status::factory()->create(['profile_id' => $user->profile_id, 'scope' => 'public']);
$n = makeLikeNotificationWithType($user->profile_id, $actor->profile_id, $status->id, Status::class);
$status->delete();
NotificationService::del($user->profile_id, $n->id);
$notifs[] = $n;
}
$topId = max(array_map(fn ($n) => $n->id, $notifs)) + 1;
$page = NotificationService::getMaxPage($user->profile_id, $topId, 20);
expect($page['data'])->toBeEmpty();
});
});

@ -0,0 +1,114 @@
<?php
use App\Models\Notification;
use App\Models\Status;
use App\Models\User;
use App\Transformer\Api\NotificationTransformer;
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
use League\Fractal\Manager;
use League\Fractal\Resource\Item;
use League\Fractal\Serializer\ArraySerializer;
uses(LazilyRefreshDatabase::class);
/*
|--------------------------------------------------------------------------
| NotificationTransformer status hydration (pixelfed#7195)
|--------------------------------------------------------------------------
|
| Notification::item_type is stored verbatim. Rows created before the
| App\ -> App\Models\ namespace migration hold the legacy 'App\Status'
| value (now a morph-map alias), while new rows hold App\Models\Status.
| A strict `== Status::class` comparison silently skipped status hydration
| for the legacy rows, so favourite/comment/mention notifications came back
| with no attached status and the web UI dropped them (endless loading).
|
| These transform the notification directly (no Redis-backed service cache)
| so the assertion targets the transformer contract itself.
|
*/
function transformNotification(Notification $n): array
{
$fractal = new Manager;
$fractal->setSerializer(new ArraySerializer);
return $fractal->createData(new Item($n, new NotificationTransformer))->toArray();
}
/**
* Persist a like notification while forcing the raw item_type string, so we
* can reproduce legacy ('App\Status') vs current (App\Models\Status) rows.
*/
function makeLikeNotification(string $itemType): array
{
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
$status = Status::factory()->create([
'profile_id' => $user->profile_id,
'scope' => 'public',
]);
$n = new Notification;
$n->profile_id = $user->profile_id;
$n->actor_id = $actor->profile_id;
$n->action = 'like';
$n->item_id = $status->id;
$n->item_type = $itemType;
$n->save();
return [$n, $status];
}
it('hydrates status for a legacy App\\Status notification (regression)', function () {
[$n, $status] = makeLikeNotification('App\Status');
$res = transformNotification($n);
expect($res['type'])->toBe('favourite');
expect($res['status'])->not->toBeNull();
expect((string) $res['status']['id'])->toBe((string) $status->id);
});
it('hydrates status for a current App\\Models\\Status notification', function () {
[$n, $status] = makeLikeNotification(Status::class);
$res = transformNotification($n);
expect($res['status'])->not->toBeNull();
expect((string) $res['status']['id'])->toBe((string) $status->id);
});
it('returns a null status for a legacy notification whose status was deleted', function () {
[$n, $status] = makeLikeNotification('App\Status');
$status->delete();
$res = transformNotification($n);
// The comparison now matches, so hydration is attempted; a deleted status
// resolves to null rather than being skipped by an unmatched item_type.
expect($res['status'] ?? null)->toBeNull();
});
it('does not attach a status for a follow notification', function () {
$user = User::factory()->create();
$user->refresh();
$actor = User::factory()->create();
$actor->refresh();
$n = new Notification;
$n->profile_id = $user->profile_id;
$n->actor_id = $actor->profile_id;
$n->action = 'follow';
$n->item_id = $actor->profile_id;
$n->item_type = 'App\Profile';
$n->save();
$res = transformNotification($n);
expect($res['type'])->toBe('follow');
expect(array_key_exists('status', $res))->toBeFalse();
});
Loading…
Cancel
Save