From a8d96554a78960d7de6ba9f26c96089061fba8d7 Mon Sep 17 00:00:00 2001 From: myrmidex Date: Thu, 13 Aug 2026 18:06:49 +0200 Subject: [PATCH] 83 - Tighten dashboard analytics after review --- app/Dashboard/Stats/BreakdownResult.php | 5 ---- app/Dashboard/Stats/Stat.php | 2 +- .../Api/V1/DashboardController.php | 4 +-- app/Livewire/Activity.php | 9 +------ app/Livewire/Dashboard.php | 9 +------ app/Models/ActivityLog.php | 14 ++++++++++ .../Api/V1/DashboardControllerTest.php | 16 +++++++++++ tests/Feature/Livewire/DashboardTest.php | 27 +++++++++++++++++++ .../Dashboard/Stats/BreakdownResultTest.php | 15 ----------- .../Unit/Models/RouteArticleDecisionTest.php | 20 ++++++++++++++ 10 files changed, 82 insertions(+), 39 deletions(-) diff --git a/app/Dashboard/Stats/BreakdownResult.php b/app/Dashboard/Stats/BreakdownResult.php index 3ef41ce7..8b2264e1 100644 --- a/app/Dashboard/Stats/BreakdownResult.php +++ b/app/Dashboard/Stats/BreakdownResult.php @@ -22,9 +22,4 @@ public function shareOf(Breakdown $row): float return $total > 0 ? round(($row->count / $total) * 100, 1) : 0.0; } - - public function isEmpty(): bool - { - return $this->total() === 0; - } } diff --git a/app/Dashboard/Stats/Stat.php b/app/Dashboard/Stats/Stat.php index bc6c70a4..6a304303 100644 --- a/app/Dashboard/Stats/Stat.php +++ b/app/Dashboard/Stats/Stat.php @@ -5,7 +5,7 @@ interface Stat { /** - * Stable identifier, also used as the island name for this stat's panel. + * Stable identifier. Island names in dashboard.blade.php are written to match by hand, not derived from this. */ public function key(): string; diff --git a/app/Http/Controllers/Api/V1/DashboardController.php b/app/Http/Controllers/Api/V1/DashboardController.php index e88704fb..4aa65c90 100644 --- a/app/Http/Controllers/Api/V1/DashboardController.php +++ b/app/Http/Controllers/Api/V1/DashboardController.php @@ -20,8 +20,8 @@ public function __construct( public function stats(Request $request): JsonResponse { $validated = $request->validate([ - 'from' => ['nullable', 'date'], - 'to' => ['nullable', 'date', 'after_or_equal:from'], + 'from' => ['nullable', 'date', 'required_with:to'], + 'to' => ['nullable', 'date', 'after_or_equal:from', 'required_with:from'], ]); $range = isset($validated['from'], $validated['to']) diff --git a/app/Livewire/Activity.php b/app/Livewire/Activity.php index 07efe099..1754727d 100644 --- a/app/Livewire/Activity.php +++ b/app/Livewire/Activity.php @@ -4,12 +4,9 @@ use App\Enums\ActivityTypeEnum; use App\Models\ActivityLog; -use App\Models\Article; use App\Services\Activity\ActivitySummary; use Illuminate\Contracts\View\View; use Illuminate\Database\Eloquent\Builder; -use Illuminate\Database\Eloquent\Relations\MorphTo; -use Illuminate\Database\Eloquent\Relations\Relation; use Livewire\Component; use Livewire\WithPagination; @@ -37,11 +34,7 @@ public function updatedDays(): void private function query(): Builder { return ActivityLog::query() - ->with(['subject' => function (Relation $morphTo): void { - if ($morphTo instanceof MorphTo) { - $morphTo->morphWith([Article::class => ['feed']]); - } - }]) + ->withSubjectDetails() ->since(now()->subDays($this->days)) ->when( $this->type !== '', diff --git a/app/Livewire/Dashboard.php b/app/Livewire/Dashboard.php index 778fc2c8..8af9e0da 100644 --- a/app/Livewire/Dashboard.php +++ b/app/Livewire/Dashboard.php @@ -11,13 +11,10 @@ use App\Dashboard\Stats\PublishSuccessRate; use App\Dashboard\Stats\SeriesResult; use App\Models\ActivityLog; -use App\Models\Article; use App\Services\DashboardStatsService; use App\Support\DateRange; use Illuminate\Contracts\View\View; use Illuminate\Database\Eloquent\Collection; -use Illuminate\Database\Eloquent\Relations\MorphTo; -use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Support\Carbon; use InvalidArgumentException; use Livewire\Attributes\Computed; @@ -195,11 +192,7 @@ private function breakdown(string $stat): BreakdownResult public function recentActivity(): Collection { return ActivityLog::query() - ->with(['subject' => function (Relation $morphTo): void { - if ($morphTo instanceof MorphTo) { - $morphTo->morphWith([Article::class => ['feed']]); - } - }]) + ->withSubjectDetails() ->latestFirst() ->limit(self::RECENT_ACTIVITY_LIMIT) ->get(); diff --git a/app/Models/ActivityLog.php b/app/Models/ActivityLog.php index 60b626a4..857ace30 100644 --- a/app/Models/ActivityLog.php +++ b/app/Models/ActivityLog.php @@ -8,6 +8,7 @@ use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\MorphTo; +use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Support\Carbon; /** @@ -77,4 +78,17 @@ public function scopeLatestFirst(Builder $query): Builder { return $query->orderByDesc('logged_at')->orderByDesc('id'); } + + /** + * @param Builder $query + * @return Builder + */ + public function scopeWithSubjectDetails(Builder $query): Builder + { + return $query->with(['subject' => function (Relation $morphTo): void { + if ($morphTo instanceof MorphTo) { + $morphTo->morphWith([Article::class => ['feed']]); + } + }]); + } } diff --git a/tests/Feature/Http/Controllers/Api/V1/DashboardControllerTest.php b/tests/Feature/Http/Controllers/Api/V1/DashboardControllerTest.php index 85e1f2e1..0d26f0e3 100644 --- a/tests/Feature/Http/Controllers/Api/V1/DashboardControllerTest.php +++ b/tests/Feature/Http/Controllers/Api/V1/DashboardControllerTest.php @@ -99,6 +99,22 @@ public function test_stats_rejects_a_malformed_date(): void ->assertStatus(422); } + public function test_stats_rejects_a_from_without_a_to(): void + { + $this + ->getJson('/api/v1/dashboard/stats?from=2026-07-01') + ->assertStatus(422) + ->assertJsonValidationErrors('to'); + } + + public function test_stats_rejects_a_to_without_a_from(): void + { + $this + ->getJson('/api/v1/dashboard/stats?to=2026-07-31') + ->assertStatus(422) + ->assertJsonValidationErrors('from'); + } + public function test_stats_with_sample_data(): void { $feed = Feed::factory()->create(['is_active' => true]); diff --git a/tests/Feature/Livewire/DashboardTest.php b/tests/Feature/Livewire/DashboardTest.php index 539c48b8..98acf89a 100644 --- a/tests/Feature/Livewire/DashboardTest.php +++ b/tests/Feature/Livewire/DashboardTest.php @@ -13,6 +13,7 @@ use App\Models\RouteArticle; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\DB; use Livewire\Features\SupportTesting\Testable; use Livewire\Livewire; use Tests\TestCase; @@ -552,6 +553,32 @@ public function test_it_renders_an_activity_entry_whose_subject_was_deleted(): v Livewire::test(Dashboard::class)->assertSee('Orphaned entry'); } + private function dashboardQueryCount(int $activityEntries): int + { + $feed = Feed::factory()->create(); + + foreach (range(1, $activityEntries) as $index) { + $article = Article::factory()->create(['feed_id' => $feed->id]); + ActivityLog::factory()->forSubject($article)->create(['message' => "Entry {$index}"]); + } + + DB::flushQueryLog(); + DB::enableQueryLog(); + Livewire::test(Dashboard::class); + $queries = count(DB::getQueryLog()); + DB::disableQueryLog(); + + return $queries; + } + + public function test_it_does_not_fan_out_queries_per_activity_entry(): void + { + $this->assertSame( + $this->dashboardQueryCount(1), + $this->dashboardQueryCount(20), + ); + } + public function test_every_range_dependent_island_re_renders_on_a_range_change(): void { $fragments = $this->islandFragments( diff --git a/tests/Unit/Dashboard/Stats/BreakdownResultTest.php b/tests/Unit/Dashboard/Stats/BreakdownResultTest.php index d6af4ccf..cd39af4c 100644 --- a/tests/Unit/Dashboard/Stats/BreakdownResultTest.php +++ b/tests/Unit/Dashboard/Stats/BreakdownResultTest.php @@ -40,19 +40,4 @@ public function test_it_reports_a_zero_share_when_nothing_was_counted(): void $this->assertSame(0.0, $result->shareOf($result->rows[0])); } - - public function test_it_has_no_data_without_rows(): void - { - $this->assertTrue((new BreakdownResult([]))->isEmpty()); - } - - public function test_it_has_no_data_when_every_row_is_zero(): void - { - $this->assertTrue((new BreakdownResult([new Breakdown('A', 0)]))->isEmpty()); - } - - public function test_it_has_data_when_any_row_is_counted(): void - { - $this->assertFalse((new BreakdownResult([new Breakdown('A', 1)]))->isEmpty()); - } } diff --git a/tests/Unit/Models/RouteArticleDecisionTest.php b/tests/Unit/Models/RouteArticleDecisionTest.php index 7c4f89e5..95e6c08a 100644 --- a/tests/Unit/Models/RouteArticleDecisionTest.php +++ b/tests/Unit/Models/RouteArticleDecisionTest.php @@ -67,6 +67,26 @@ public function test_re_approving_does_not_move_the_decision_time(): void ); } + public function test_re_rejecting_does_not_move_the_decision_time(): void + { + /** @var RouteArticle $routeArticle */ + $routeArticle = RouteArticle::factory()->create([ + 'approval_status' => ApprovalStatusEnum::PENDING, + 'decided_at' => null, + ]); + + $routeArticle->reject(); + $first = $routeArticle->fresh()->decided_at; + + $this->travel(1)->hours(); + $routeArticle->fresh()->reject(); + + $this->assertSame( + $first->format('Y-m-d H:i:s'), + $routeArticle->fresh()->decided_at->format('Y-m-d H:i:s') + ); + } + public function test_rejecting_an_approved_article_moves_the_decision_time(): void { /** @var RouteArticle $routeArticle */