From 7138b0fa83d04133ec83c4c119a18058abff85eb Mon Sep 17 00:00:00 2001 From: sonali-dudhia Date: Fri, 30 Oct 2026 15:50:38 +0530 Subject: [PATCH 1/2] Explain why a plugin can't be approved instead of greying out Approve Approve was disabled whenever a pending plugin failed a required review check. The failing checks were only listed in the confirmation popup, which a disabled button can't open, so admins (super admins included) saw a faded Approve button that did nothing. Keep the button clickable. When checks fail, the popup lists them and offers only Close, and the action refuses to approve on the server. Co-Authored-By: Claude Opus 5.5 --- .../PluginResource/Pages/EditPlugin.php | 17 +++- .../Filament/PluginApproveActionTest.php | 84 +++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 tests/Feature/Filament/PluginApproveActionTest.php diff --git a/app/Filament/Resources/PluginResource/Pages/EditPlugin.php b/app/Filament/Resources/PluginResource/Pages/EditPlugin.php index 052ee945..eec8078e 100644 --- a/app/Filament/Resources/PluginResource/Pages/EditPlugin.php +++ b/app/Filament/Resources/PluginResource/Pages/EditPlugin.php @@ -53,9 +53,22 @@ protected function getHeaderActions(): array ->icon('heroicon-o-check') ->color('success') ->visible(fn () => $this->record->isPending()) - ->disabled(fn () => ! $this->record->passesRequiredReviewChecks()) - ->action(fn () => $this->record->approve(auth()->id())) + ->action(function (): void { + if (! $this->record->passesRequiredReviewChecks()) { + Notification::make() + ->title('Required review checks are failing') + ->body(implode(', ', $this->record->getFailingRequiredChecks())) + ->danger() + ->send(); + + return; + } + + $this->record->approve(auth()->id()); + }) ->requiresConfirmation() + ->modalSubmitAction(fn (Actions\Action $action): Actions\Action|false => $this->record->passesRequiredReviewChecks() ? $action : false) + ->modalCancelActionLabel(fn (): string => $this->record->passesRequiredReviewChecks() ? 'Cancel' : 'Close') ->modalHeading('Approve Plugin') ->modalDescription(fn () => ! $this->record->passesRequiredReviewChecks() ? "Cannot approve '{$this->record->name}' — required checks are failing: ".implode(', ', $this->record->getFailingRequiredChecks()) diff --git a/tests/Feature/Filament/PluginApproveActionTest.php b/tests/Feature/Filament/PluginApproveActionTest.php new file mode 100644 index 00000000..d7373d3d --- /dev/null +++ b/tests/Feature/Filament/PluginApproveActionTest.php @@ -0,0 +1,84 @@ + ['env-admin@test.com']]); + Filament::setCurrentPanel('admin'); + } + + public function test_super_admin_role_can_approve_a_pending_plugin_that_passes_review(): void + { + $superAdmin = User::factory()->create(); + $superAdmin->assignRole(Role::findOrCreate(config('filament-shield.super_admin.name'))); + + $this->assertApproves($superAdmin, $this->reviewablePlugin()); + } + + public function test_filament_users_admin_can_approve_a_pending_plugin_that_passes_review(): void + { + $this->assertApproves(User::factory()->create(['email' => 'env-admin@test.com']), $this->reviewablePlugin()); + } + + public function test_approve_stays_clickable_and_explains_failing_checks(): void + { + $superAdmin = User::factory()->create(['email' => 'env-admin@test.com']); + $plugin = $this->reviewablePlugin(); + $plugin->update(['demo_video_url' => null, 'demo_video_attested_at' => null]); + + Livewire::actingAs($superAdmin) + ->test(EditPlugin::class, ['record' => $plugin->getRouteKey()]) + ->assertActionVisible('approve') + ->assertActionEnabled('approve') + ->mountAction('approve') + ->assertMountedActionModalSee('required checks are failing') + ->assertMountedActionModalSee('Demo video showing the plugin working') + ->callMountedAction(); + + $this->assertSame(PluginStatus::Pending, $plugin->fresh()->status); + } + + private function assertApproves(User $user, Plugin $plugin): void + { + Livewire::actingAs($user) + ->test(EditPlugin::class, ['record' => $plugin->getRouteKey()]) + ->assertActionVisible('approve') + ->assertActionEnabled('approve') + ->callAction('approve') + ->assertHasNoActionErrors(); + + $plugin->refresh(); + + $this->assertSame(PluginStatus::Approved, $plugin->status); + $this->assertSame($user->id, $plugin->approved_by); + } + + private function reviewablePlugin(): Plugin + { + return Plugin::factory()->pending()->free()->create([ + 'webhook_installed' => true, + 'review_checks' => ['has_license_file' => true, 'has_release_version' => true, 'release_version' => '1.0.0'], + ]); + } +} From 0c65c2d9a94cd00a89f893d0622661cabf64cfa3 Mon Sep 17 00:00:00 2001 From: sonali-dudhia Date: Fri, 30 Oct 2026 15:59:28 +0530 Subject: [PATCH 2/2] Grandfather plugins submitted before demo videos were required Plugins first sent for review before #534 (25 Sep 2026) couldn't be approved, because the new demo video check can never pass for them. They no longer need a demo video; plugins first submitted after that still do. The first submission comes from the plugin's activity log, falling back to its creation date for older plugins that left draft before submissions were logged. Drafts that were never submitted still need a video. Admin review checks, the developer's checklist and submit button, and the review-checks email all use the same rule. Co-Authored-By: Claude Opus 5.5 --- app/Filament/Resources/PluginResource.php | 9 +- app/Livewire/Customer/Plugins/Show.php | 2 +- app/Models/Plugin.php | 46 +++++++++- .../PluginReviewChecksIncomplete.php | 2 +- .../livewire/customer/plugins/show.blade.php | 2 +- .../Filament/PluginApproveActionTest.php | 83 ++++++++++++++++++- tests/Feature/PluginDemoVideoTest.php | 2 + 7 files changed, 137 insertions(+), 9 deletions(-) diff --git a/app/Filament/Resources/PluginResource.php b/app/Filament/Resources/PluginResource.php index 5848c3dc..ffc80ca2 100644 --- a/app/Filament/Resources/PluginResource.php +++ b/app/Filament/Resources/PluginResource.php @@ -229,6 +229,7 @@ public static function form(Schema $schema): Schema ->label('Demo Video (required)') ->content(fn (?Plugin $record) => match (true) { $record?->hasDemoVideo() => '✅ '.$record->demoVideo()->providerLabel().' — attested '.$record->demo_video_attested_at->diffForHumans(), + $record?->requiresDemoVideo() === false => '✅ Not required — submitted before demo videos were required', $record?->demoVideo() !== null => '❌ Not attested by developer', default => '❌ Missing', }), @@ -608,7 +609,11 @@ public static function table(Table $table): Table $lines = collect([ ['License file *', $checks['has_license_file']], ['Release version *', $checks['has_release_version'] ? $checks['release_version'] : false], - ['Demo video *', $record->hasDemoVideo() ? $record->demoVideo()->providerLabel() : false], + ['Demo video *', match (true) { + $record->hasDemoVideo() => $record->demoVideo()->providerLabel(), + ! $record->requiresDemoVideo() => 'not required', + default => false, + }], ['iOS support', $checks['supports_ios']], ['Android support', $checks['supports_android']], ['JS support', $checks['supports_js']], @@ -632,7 +637,7 @@ public static function table(Table $table): Table 'supports_ios', 'supports_android', 'supports_js', 'requires_mobile_sdk', 'has_ios_min_version', 'has_android_min_version', - ])->filter()->count() + ($record->hasDemoVideo() ? 1 : 0); + ])->filter()->count() + ($record->meetsDemoVideoRequirement() ? 1 : 0); Notification::make() ->title("Review checks complete ({$passed}/9 passed)") diff --git a/app/Livewire/Customer/Plugins/Show.php b/app/Livewire/Customer/Plugins/Show.php index f9de3970..2add408d 100644 --- a/app/Livewire/Customer/Plugins/Show.php +++ b/app/Livewire/Customer/Plugins/Show.php @@ -196,7 +196,7 @@ public function submitForReview(): void return; } - if (! $this->plugin->hasDemoVideo()) { + if (! $this->plugin->meetsDemoVideoRequirement()) { Flux::toast(variant: 'danger', text: 'Please add a demo video showing your plugin working and confirm it before submitting for review.'); return; diff --git a/app/Models/Plugin.php b/app/Models/Plugin.php index 598cb033..b44d72f1 100644 --- a/app/Models/Plugin.php +++ b/app/Models/Plugin.php @@ -36,6 +36,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\HasOne; +use Illuminate\Support\Carbon; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Notification; @@ -501,7 +502,7 @@ public function passesRequiredReviewChecks(): bool return ! empty($checks['has_license_file']) && ! empty($checks['has_release_version']) && $this->webhook_installed - && $this->hasDemoVideo(); + && $this->meetsDemoVideoRequirement(); } /** @@ -512,6 +513,42 @@ public function hasDemoVideo(): bool return $this->demoVideo() !== null && $this->demo_video_attested_at !== null; } + /** + * Plugins first submitted for review before demo videos became a requirement + * (#534) are grandfathered in and can be approved without one. + */ + public function requiresDemoVideo(): bool + { + $firstSubmittedAt = $this->firstSubmittedAt(); + + return $firstSubmittedAt === null + || $firstSubmittedAt->gte(Carbon::parse(self::DEMO_VIDEO_REQUIRED_FROM, 'UTC')); + } + + public function meetsDemoVideoRequirement(): bool + { + return $this->hasDemoVideo() || ! $this->requiresDemoVideo(); + } + + /** + * When the plugin was first sent for review. Plugins that left draft before + * submissions were logged as activity fall back to when they were created. + */ + public function firstSubmittedAt(): ?Carbon + { + $submittedAt = $this->activities() + ->reorder() + ->where('type', PluginActivityType::Submitted) + ->oldest() + ->value('created_at'); + + if ($submittedAt !== null) { + return Carbon::parse($submittedAt); + } + + return $this->status === PluginStatus::Draft ? null : $this->created_at; + } + public function demoVideo(): ?DemoVideo { return DemoVideo::fromUrl($this->demo_video_url); @@ -547,7 +584,7 @@ public function getFailingRequiredChecks(): array $failing[] = 'Webhook configured'; } - if (! $this->hasDemoVideo()) { + if (! $this->meetsDemoVideoRequirement()) { $failing[] = 'Demo video showing the plugin working'; } @@ -693,6 +730,11 @@ public function hasCustomIcon(): bool */ public const DEFAULT_ICON_NAME = 'cube'; + /** + * When #534 made a demo video required for review (UTC). + */ + public const DEMO_VIDEO_REQUIRED_FROM = '2026-09-25 16:23:52'; + /** * Whether the given name matches an outline Heroicon we can actually render. * diff --git a/app/Notifications/PluginReviewChecksIncomplete.php b/app/Notifications/PluginReviewChecksIncomplete.php index cf05cdc1..6aa826e3 100644 --- a/app/Notifications/PluginReviewChecksIncomplete.php +++ b/app/Notifications/PluginReviewChecksIncomplete.php @@ -182,7 +182,7 @@ private function isCheckPassing(string $key, array $checks): bool { return match ($key) { 'webhook_configured' => (bool) $this->plugin->webhook_installed, - 'demo_video' => $this->plugin->hasDemoVideo(), + 'demo_video' => $this->plugin->meetsDemoVideoRequirement(), default => ! empty($checks[$key]), }; } diff --git a/resources/views/livewire/customer/plugins/show.blade.php b/resources/views/livewire/customer/plugins/show.blade.php index abaa2ff9..23b81ba1 100644 --- a/resources/views/livewire/customer/plugins/show.blade.php +++ b/resources/views/livewire/customer/plugins/show.blade.php @@ -113,7 +113,7 @@ @php $isPassing = match ($check['key']) { 'webhook_configured' => $plugin->webhook_installed, - 'demo_video' => $plugin->hasDemoVideo(), + 'demo_video' => $plugin->meetsDemoVideoRequirement(), default => $plugin->review_checks[$check['key']] ?? false, }; @endphp diff --git a/tests/Feature/Filament/PluginApproveActionTest.php b/tests/Feature/Filament/PluginApproveActionTest.php index d7373d3d..7988c095 100644 --- a/tests/Feature/Filament/PluginApproveActionTest.php +++ b/tests/Feature/Filament/PluginApproveActionTest.php @@ -2,12 +2,14 @@ namespace Tests\Feature\Filament; +use App\Enums\PluginActivityType; use App\Enums\PluginStatus; use App\Filament\Resources\PluginResource\Pages\EditPlugin; use App\Models\Plugin; use App\Models\User; use Filament\Facades\Filament; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Notification; use Illuminate\Support\Facades\Queue; use Livewire\Livewire; @@ -44,8 +46,8 @@ public function test_filament_users_admin_can_approve_a_pending_plugin_that_pass public function test_approve_stays_clickable_and_explains_failing_checks(): void { $superAdmin = User::factory()->create(['email' => 'env-admin@test.com']); - $plugin = $this->reviewablePlugin(); - $plugin->update(['demo_video_url' => null, 'demo_video_attested_at' => null]); + $plugin = $this->pluginWithoutDemoVideo(); + $this->recordSubmission($plugin, $this->afterRequirement()); Livewire::actingAs($superAdmin) ->test(EditPlugin::class, ['record' => $plugin->getRouteKey()]) @@ -59,6 +61,51 @@ public function test_approve_stays_clickable_and_explains_failing_checks(): void $this->assertSame(PluginStatus::Pending, $plugin->fresh()->status); } + public function test_plugin_submitted_before_demo_videos_were_required_can_be_approved_without_one(): void + { + $plugin = $this->pluginWithoutDemoVideo(); + $this->recordSubmission($plugin, $this->beforeRequirement()); + + $this->assertFalse($plugin->requiresDemoVideo()); + $this->assertTrue($plugin->passesRequiredReviewChecks()); + $this->assertNotContains('Demo video showing the plugin working', $plugin->getFailingRequiredChecks()); + + $this->assertApproves(User::factory()->create(['email' => 'env-admin@test.com']), $plugin); + } + + public function test_plugin_submitted_after_demo_videos_were_required_still_needs_one(): void + { + $plugin = $this->pluginWithoutDemoVideo(); + $this->recordSubmission($plugin, $this->afterRequirement()); + + $this->assertTrue($plugin->requiresDemoVideo()); + $this->assertFalse($plugin->passesRequiredReviewChecks()); + $this->assertContains('Demo video showing the plugin working', $plugin->getFailingRequiredChecks()); + } + + public function test_resubmitting_after_the_cutoff_keeps_the_original_submission_date(): void + { + $plugin = $this->pluginWithoutDemoVideo(); + $this->recordSubmission($plugin, $this->beforeRequirement()); + $this->recordSubmission($plugin, $this->afterRequirement(), PluginActivityType::Resubmitted); + + $this->assertFalse($plugin->requiresDemoVideo()); + } + + public function test_old_pending_plugin_without_a_logged_submission_falls_back_to_its_creation_date(): void + { + $plugin = $this->pluginWithoutDemoVideo(['created_at' => $this->beforeRequirement()]); + + $this->assertFalse($plugin->requiresDemoVideo()); + } + + public function test_old_draft_that_was_never_submitted_needs_a_demo_video(): void + { + $plugin = $this->pluginWithoutDemoVideo(['status' => PluginStatus::Draft, 'created_at' => $this->beforeRequirement()]); + + $this->assertTrue($plugin->requiresDemoVideo()); + } + private function assertApproves(User $user, Plugin $plugin): void { Livewire::actingAs($user) @@ -81,4 +128,36 @@ private function reviewablePlugin(): Plugin 'review_checks' => ['has_license_file' => true, 'has_release_version' => true, 'release_version' => '1.0.0'], ]); } + + /** + * @param array $attributes + */ + private function pluginWithoutDemoVideo(array $attributes = []): Plugin + { + $plugin = $this->reviewablePlugin(); + $plugin->forceFill(['demo_video_url' => null, 'demo_video_attested_at' => null, ...$attributes])->save(); + + return $plugin->fresh(); + } + + private function recordSubmission(Plugin $plugin, Carbon $at, PluginActivityType $type = PluginActivityType::Submitted): void + { + $plugin->activities()->create([ + 'type' => $type, + 'from_status' => PluginStatus::Draft, + 'to_status' => PluginStatus::Pending, + 'causer_id' => $plugin->user_id, + 'created_at' => $at, + ]); + } + + private function beforeRequirement(): Carbon + { + return Carbon::parse(Plugin::DEMO_VIDEO_REQUIRED_FROM, 'UTC')->subDay(); + } + + private function afterRequirement(): Carbon + { + return Carbon::parse(Plugin::DEMO_VIDEO_REQUIRED_FROM, 'UTC')->addDay(); + } } diff --git a/tests/Feature/PluginDemoVideoTest.php b/tests/Feature/PluginDemoVideoTest.php index 1c357b71..096ec71e 100644 --- a/tests/Feature/PluginDemoVideoTest.php +++ b/tests/Feature/PluginDemoVideoTest.php @@ -312,6 +312,7 @@ public function test_required_checks_fail_without_demo_video_even_when_repo_chec $plugin = Plugin::factory()->pending()->withoutDemoVideo()->create([ 'review_checks' => self::PASSING_REPO_CHECKS, 'webhook_installed' => true, + 'created_at' => now(), ]); $this->assertFalse($plugin->passesRequiredReviewChecks()); @@ -365,6 +366,7 @@ public function test_review_checks_email_lists_missing_demo_video(): void $plugin = Plugin::factory()->pending()->withoutDemoVideo()->for($user)->create([ 'review_checks' => self::PASSING_REPO_CHECKS, 'webhook_installed' => true, + 'created_at' => now(), ]); $rendered = (string) (new PluginReviewChecksIncomplete($plugin))->toMail($user)->render();