From dde5181f6f17464e972e933b9c840dd0d2acba8c Mon Sep 17 00:00:00 2001 From: aaron Date: Sat, 4 May 2024 22:32:08 -0500 Subject: [PATCH 1/3] test: split presentation controller tests into describe blocks --- tests/Feature/PresentationControllerTest.php | 152 ++++++++++--------- 1 file changed, 79 insertions(+), 73 deletions(-) diff --git a/tests/Feature/PresentationControllerTest.php b/tests/Feature/PresentationControllerTest.php index 1f68be4..862027b 100644 --- a/tests/Feature/PresentationControllerTest.php +++ b/tests/Feature/PresentationControllerTest.php @@ -9,94 +9,109 @@ $this->user = User::factory()->create(); $this->guest = User::factory()->create(); - $this->publishedPresentation = Presentation::factory()->create([ - 'is_published' => true, - 'user_id' => $this->user->id, - ]); - $this->draftPresentation = Presentation::factory()->create([ 'is_published' => false, 'user_id' => $this->user->id, ]); }); -test('published presentation show screen can be rendered for unauthenticated user', function () { - $response = $this->get(route('presentations.show', [ - 'user' => $this->user->username, - 'slug' => $this->publishedPresentation->slug, - ])); - - $response->assertStatus(200); -}); - -test('published presentation show action generates daily view', function () { - $response = $this->get(route('presentations.show', [ - 'user' => $this->user->username, - 'slug' => $this->publishedPresentation->slug, - ])); +describe('published presentation', function () { + beforeEach(function () { + $this->publishedPresentation = Presentation::factory()->create([ + 'is_published' => true, + 'user_id' => $this->user->id, + ]); + }); - $this->assertDatabaseHas(DailyView::class, [ - 'presentation_id' => $this->publishedPresentation->id, - ]); -}); + test('show screen can be rendered for unauthenticated user', function () { + $response = $this->get(route('presentations.show', [ + 'user' => $this->user->username, + 'slug' => $this->publishedPresentation->slug, + ])); -test('published presentation show screen returns the right view and data', function () { - $response = $this->get(route('presentations.show', [ - 'user' => $this->user->username, - 'slug' => $this->publishedPresentation->slug, - ])); + $response->assertStatus(200); + }); - $response->assertInertia(fn (Assert $page) => $page - ->component('Slides') - ->where('content', $this->publishedPresentation->content) - ->has('meta', fn (Assert $page) => $page - ->where('title', $this->publishedPresentation->title) - ->where('description', $this->publishedPresentation->description) - ->where('imageUrl', $this->publishedPresentation->getFirstMediaUrl('thumbnail')) - ) - ); -}); + test('show action generates daily view', function () { + $response = $this->get(route('presentations.show', [ + 'user' => $this->user->username, + 'slug' => $this->publishedPresentation->slug, + ])); -test('draft presentation show screen is not rendered for unauthenticated user', function () { - $response = $this->get(route('presentations.show', [ - 'user' => $this->user->username, - 'slug' => $this->draftPresentation->slug, - ])); + $this->assertDatabaseHas(DailyView::class, [ + 'presentation_id' => $this->publishedPresentation->id, + ]); + }); - $response->assertStatus(403); -}); + test('show screen returns the right view and data', function () { + $response = $this->get(route('presentations.show', [ + 'user' => $this->user->username, + 'slug' => $this->publishedPresentation->slug, + ])); -test('draft presentation show action does not generate daily view', function () { - $response = $this->get(route('presentations.show', [ - 'user' => $this->user->username, - 'slug' => $this->draftPresentation->slug, - ])); + $response->assertInertia(fn (Assert $page) => $page + ->component('Slides') + ->where('content', $this->publishedPresentation->content) + ->has('meta', fn (Assert $page) => $page + ->where('title', $this->publishedPresentation->title) + ->where('description', $this->publishedPresentation->description) + ->where('imageUrl', $this->publishedPresentation->getFirstMediaUrl('thumbnail')) + ) + ); + }); + + test('non-existing username for user shows 404', function () { + $response = $this->get(route('presentations.show', [ + 'user' => 'foo', + 'slug' => $this->publishedPresentation->slug, + ])); - $this->assertDatabaseMissing(DailyView::class, [ - 'presentation_id' => $this->draftPresentation->id, - ]); + $response->assertStatus(404); + }); }); -test('draft presentation show screen can be rendered for author', function () { - $response = $this - ->actingAs($this->user) - ->get(route('presentations.show', [ +describe('draft presentation', function () { + test('show screen is not rendered for unauthenticated user', function () { + $response = $this->get(route('presentations.show', [ 'user' => $this->user->username, 'slug' => $this->draftPresentation->slug, ])); - $response->assertStatus(200); -}); + $response->assertStatus(403); + }); -test('draft presentation show screen is not rendered for non-author', function () { - $response = $this - ->actingAs($this->guest) - ->get(route('presentations.show', [ + test('show action does not generate daily view', function () { + $response = $this->get(route('presentations.show', [ 'user' => $this->user->username, 'slug' => $this->draftPresentation->slug, ])); - $response->assertStatus(403); + $this->assertDatabaseMissing(DailyView::class, [ + 'presentation_id' => $this->draftPresentation->id, + ]); + }); + + test('show screen can be rendered for author', function () { + $response = $this + ->actingAs($this->user) + ->get(route('presentations.show', [ + 'user' => $this->user->username, + 'slug' => $this->draftPresentation->slug, + ])); + + $response->assertStatus(200); + }); + + test('show screen is not rendered for non-author', function () { + $response = $this + ->actingAs($this->guest) + ->get(route('presentations.show', [ + 'user' => $this->user->username, + 'slug' => $this->draftPresentation->slug, + ])); + + $response->assertStatus(403); + }); }); test('non-existing slug for presentation shows 404', function () { @@ -107,12 +122,3 @@ $response->assertStatus(404); }); - -test('non-existing username for user shows 404', function () { - $response = $this->get(route('presentations.show', [ - 'user' => 'foo', - 'slug' => $this->publishedPresentation->slug, - ])); - - $response->assertStatus(404); -}); From 37cfff67acb1f8b2de9cffdef441052601a0622c Mon Sep 17 00:00:00 2001 From: aaron Date: Sat, 4 May 2024 22:55:44 -0500 Subject: [PATCH 2/3] docs: update static content on dashboard page --- app/Filament/Pages/Dashboard.php | 6 ++++++ app/Filament/Widgets/TopViews.php | 2 +- resources/views/filament/dashboard/footer.blade.php | 7 +++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 resources/views/filament/dashboard/footer.blade.php diff --git a/app/Filament/Pages/Dashboard.php b/app/Filament/Pages/Dashboard.php index 8690dfe..98c22ce 100644 --- a/app/Filament/Pages/Dashboard.php +++ b/app/Filament/Pages/Dashboard.php @@ -12,6 +12,7 @@ use Filament\Forms\Get; use Filament\Pages\Dashboard as BaseDashboard; use Filament\Pages\Dashboard\Concerns\HasFiltersForm; +use Illuminate\Contracts\View\View; class Dashboard extends BaseDashboard { @@ -22,6 +23,11 @@ public function getColumns(): int|string|array return 2; } + public function getFooter(): ?View + { + return view('filament.dashboard.footer'); + } + protected function getHeaderActions(): array { return [ diff --git a/app/Filament/Widgets/TopViews.php b/app/Filament/Widgets/TopViews.php index 0520e44..ca883a6 100644 --- a/app/Filament/Widgets/TopViews.php +++ b/app/Filament/Widgets/TopViews.php @@ -82,7 +82,7 @@ public function table(Table $table): Table ->emptyStateDescription(null) ->emptyStateActions([ Action::make('create') - ->label('Create Presentation') + ->label('New Presentation') ->url(route('filament.admin.resources.presentations.create')) ->icon('heroicon-m-plus') ->button(), diff --git a/resources/views/filament/dashboard/footer.blade.php b/resources/views/filament/dashboard/footer.blade.php new file mode 100644 index 0000000..1b4c7a0 --- /dev/null +++ b/resources/views/filament/dashboard/footer.blade.php @@ -0,0 +1,7 @@ + +

+ Note: Views are only tracked on published presentations. + Additionally, if you are logged in and view one of your own + presentations, a view will not be tracked. +

+ From 4c7ec460314525c5f78f87dbf6218695dc699cac Mon Sep 17 00:00:00 2001 From: aaron Date: Sat, 4 May 2024 22:56:36 -0500 Subject: [PATCH 3/3] feat: update rules for tracking presentation views Also refactor checking if a presentation can be viewed --- .../Controllers/PresentationController.php | 28 ++------ app/Models/Presentation.php | 68 +++++++++++++++++++ tests/Feature/PresentationControllerTest.php | 58 +++++++++++++--- 3 files changed, 124 insertions(+), 30 deletions(-) diff --git a/app/Http/Controllers/PresentationController.php b/app/Http/Controllers/PresentationController.php index f1d6241..e6f68a5 100644 --- a/app/Http/Controllers/PresentationController.php +++ b/app/Http/Controllers/PresentationController.php @@ -2,7 +2,6 @@ namespace App\Http\Controllers; -use App\Models\Presentation; use App\Models\User; use Inertia\Inertia; use Inertia\Response; @@ -16,13 +15,15 @@ public function show(User $user, string $slug): Response ->where('slug', $slug) ->firstOrFail(); - if (! $this->canViewPresentation($presentation)) { + if (! $presentation->canBeViewed) { abort(403); } - dispatch(function () use ($presentation) { - $presentation->addDailyView(); - })->afterResponse(); + if ($presentation->shouldTrackView) { + dispatch(function () use ($presentation) { + $presentation->addDailyView(); + })->afterResponse(); + } return Inertia::render('Slides', [ 'content' => $presentation->content, @@ -33,21 +34,4 @@ public function show(User $user, string $slug): Response ], ]); } - - private function canViewPresentation(Presentation $presentation): bool - { - // If the presentation is published, then anyone can see it. - if ($presentation->is_published) { - return true; - } - - // If the user is not logged in, then they can't see any draft - // presentations. - if (! auth()->check()) { - return false; - } - - // Default to the normal view policy function - return auth()->user()->can('view', $presentation); - } } diff --git a/app/Models/Presentation.php b/app/Models/Presentation.php index 63690b1..032ec5c 100644 --- a/app/Models/Presentation.php +++ b/app/Models/Presentation.php @@ -3,6 +3,7 @@ namespace App\Models; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; @@ -30,6 +31,73 @@ class Presentation extends Model implements HasMedia 'deleted_at', ]; + /** + * Determine if this presentation be viewed, based on published status and + * the authenticated user. + * + * @return Attribute + */ + protected function canBeViewed(): Attribute + { + return Attribute::make( + get: function (mixed $value, array $attributes): bool { + // If the presentation is published, then anyone can see it. + if ($this->is_published) { + return true; + } + + // If the user is not logged in, then they can't see any draft + // presentations. + if (! auth()->check()) { + return false; + } + + // Default to the normal view policy function + return auth()->user()->can('view', $this); + }, + ); + } + + /** + * Determine if this presentation should track a daily view, based on + * published status and the authenticated user. + * + * @return Attribute + */ + protected function shouldTrackView(): Attribute + { + return Attribute::make( + get: function (mixed $value, array $attributes): bool { + // If the presentation is not published, then a view should not + // be tracked. + if (! $this->is_published) { + return false; + } + + // If the user is not logged in, then a view should be tracked. + if (! auth()->check()) { + return true; + } + + // If the user is an admin, then a view should not be tracked. + if (auth()->user()->isAdministrator()) { + return false; + } + + // If the user is the creator of the presentation, then a view + // should not be tracked. + if (auth()->id() === $this->user_id) { + return false; + } + + // Otherwise, a user would be logged in, but not as an admin or + // the creator of the presentation, and thus should track a + // daily view. This would be a pretty rare case. + return true; + }, + ); + } + /** * Scope a query to only include presentations for the authenticated user. * diff --git a/tests/Feature/PresentationControllerTest.php b/tests/Feature/PresentationControllerTest.php index 862027b..d05cdbb 100644 --- a/tests/Feature/PresentationControllerTest.php +++ b/tests/Feature/PresentationControllerTest.php @@ -7,12 +7,6 @@ beforeEach(function () { $this->user = User::factory()->create(); - $this->guest = User::factory()->create(); - - $this->draftPresentation = Presentation::factory()->create([ - 'is_published' => false, - 'user_id' => $this->user->id, - ]); }); describe('published presentation', function () { @@ -32,7 +26,7 @@ $response->assertStatus(200); }); - test('show action generates daily view', function () { + test('show action generates daily view for unauthenticated user', function () { $response = $this->get(route('presentations.show', [ 'user' => $this->user->username, 'slug' => $this->publishedPresentation->slug, @@ -43,6 +37,47 @@ ]); }); + test('show action does not generate daily view for admin user', function () { + $adminUser = User::factory()->create(['is_admin' => true]); + + $response = $this + ->actingAs($adminUser) + ->get(route('presentations.show', [ + 'user' => $this->user->username, + 'slug' => $this->publishedPresentation->slug, + ])); + + $this->assertDatabaseMissing(DailyView::class, [ + 'presentation_id' => $this->publishedPresentation->id, + ]); + }); + + test('show action does not generate daily view for creating user', function () { + $response = $this + ->actingAs($this->user) + ->get(route('presentations.show', [ + 'user' => $this->user->username, + 'slug' => $this->publishedPresentation->slug, + ])); + + $this->assertDatabaseMissing(DailyView::class, [ + 'presentation_id' => $this->publishedPresentation->id, + ]); + }); + + test('show action does generate daily view for non-creating user', function () { + $response = $this + ->actingAs(User::factory()->create()) + ->get(route('presentations.show', [ + 'user' => $this->user->username, + 'slug' => $this->publishedPresentation->slug, + ])); + + $this->assertDatabaseHas(DailyView::class, [ + 'presentation_id' => $this->publishedPresentation->id, + ]); + }); + test('show screen returns the right view and data', function () { $response = $this->get(route('presentations.show', [ 'user' => $this->user->username, @@ -71,6 +106,13 @@ }); describe('draft presentation', function () { + beforeEach(function () { + $this->draftPresentation = Presentation::factory()->create([ + 'is_published' => false, + 'user_id' => $this->user->id, + ]); + }); + test('show screen is not rendered for unauthenticated user', function () { $response = $this->get(route('presentations.show', [ 'user' => $this->user->username, @@ -104,7 +146,7 @@ test('show screen is not rendered for non-author', function () { $response = $this - ->actingAs($this->guest) + ->actingAs(User::factory()->create()) ->get(route('presentations.show', [ 'user' => $this->user->username, 'slug' => $this->draftPresentation->slug,