mirror of https://github.com/pixelfed/pixelfed
Merge pull request #7216 from pixelfed/fix/notifications-web-loading-7195
Fix/notifications to match previous item_typepull/7193/head
Shlee
2 weeks ago
committed by
GitHub
commit
bce33a51d8
@ -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();
|
||||
});
|
||||
@ -0,0 +1,62 @@
|
||||
<?php
|
||||
|
||||
use App\Jobs\InternalPipeline\NotificationEpochUpdatePipeline;
|
||||
use App\Models\Notification;
|
||||
use App\Services\NotificationService;
|
||||
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
|
||||
uses(LazilyRefreshDatabase::class);
|
||||
|
||||
/*
|
||||
|--------------------------------------------------------------------------
|
||||
| NotificationService epoch fallback
|
||||
|--------------------------------------------------------------------------
|
||||
|
|
||||
| When the cached epoch id is missing/expired, getEpochId() must return a
|
||||
| recent notification id (computed inline) so notification queries only scan
|
||||
| the last ~6 months. Returning 1 would force `WHERE id > 1`, scanning the
|
||||
| entire notifications table.
|
||||
|
|
||||
*/
|
||||
|
||||
beforeEach(function () {
|
||||
Queue::fake();
|
||||
Cache::forget(NotificationService::EPOCH_CACHE_KEY.'6');
|
||||
});
|
||||
|
||||
it('returns a recent notification id on cache miss instead of 1', function () {
|
||||
$old = Notification::create([
|
||||
'profile_id' => 1,
|
||||
'actor_id' => 2,
|
||||
'action' => 'like',
|
||||
'created_at' => now()->subMonths(9),
|
||||
]);
|
||||
|
||||
$recent = Notification::create([
|
||||
'profile_id' => 1,
|
||||
'actor_id' => 2,
|
||||
'action' => 'like',
|
||||
'created_at' => now()->subMonth(),
|
||||
]);
|
||||
|
||||
$epoch = NotificationService::getEpochId();
|
||||
|
||||
expect($epoch)->toBe($recent->id);
|
||||
expect($epoch)->not->toBe(1);
|
||||
expect($epoch)->toBeGreaterThan($old->id);
|
||||
|
||||
Queue::assertPushed(NotificationEpochUpdatePipeline::class);
|
||||
});
|
||||
|
||||
it('falls back to 1 when no recent notifications exist', function () {
|
||||
Notification::create([
|
||||
'profile_id' => 1,
|
||||
'actor_id' => 2,
|
||||
'action' => 'like',
|
||||
'created_at' => now()->subYear(),
|
||||
]);
|
||||
|
||||
expect(NotificationService::getEpochId())->toBe(1);
|
||||
});
|
||||
Loading…
Reference in New Issue