From 33eb96fdc7f8b46b82930a0db826f4c9327e9db6 Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Tue, 25 Aug 2026 14:57:35 +0100 Subject: [PATCH 1/2] Stop unknown plugin icon names from 500ing every page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit plugins.icon_name is free text typed by developers, but was only validated for shape, never for whether the Heroicon actually exists. Views rendered it via , which throws InvalidArgumentException at component resolution when the name is unknown — taking down the whole page. "image" (should be "photo") and "location" (should be "map-pin") were both saved in production. Add Plugin::getIconComponent(), which resolves the name through the blade-icons factory and falls back to heroicon-o-cube, and use it at all ten render sites (plugin card, public listing, cart, Ultra index, team page, purchased plugins, developer plugin page). This recovers the rows already broken in production. Also reject non-existent icon names in Show::updateIcon() so no new bad data lands, and point the input hint at heroicons.com instead of inviting guesses. Co-Authored-By: Claude Opus 5 (1M context) --- app/Livewire/Customer/Plugins/Show.php | 12 +- app/Models/Plugin.php | 43 +++++ resources/views/cart/show.blade.php | 2 +- .../views/components/plugin-card.blade.php | 2 +- resources/views/customer/team/show.blade.php | 2 +- .../views/customer/ultra/index.blade.php | 2 +- .../livewire/customer/plugins/show.blade.php | 12 +- .../customer/purchased-plugins.blade.php | 2 +- resources/views/plugin-show.blade.php | 2 +- tests/Feature/PluginIconFallbackTest.php | 159 ++++++++++++++++++ 10 files changed, 225 insertions(+), 13 deletions(-) create mode 100644 tests/Feature/PluginIconFallbackTest.php diff --git a/app/Livewire/Customer/Plugins/Show.php b/app/Livewire/Customer/Plugins/Show.php index 0e97eb1e..12f365c6 100644 --- a/app/Livewire/Customer/Plugins/Show.php +++ b/app/Livewire/Customer/Plugins/Show.php @@ -390,7 +390,17 @@ public function updateIcon(): void $this->validate([ 'iconGradient' => ['required', 'string', 'in:'.implode(',', array_keys(Plugin::gradientPresets()))], - 'iconName' => ['required', 'string', 'max:100', 'regex:/^[a-z0-9-]+$/'], + 'iconName' => [ + 'required', + 'string', + 'max:100', + 'regex:/^[a-z0-9-]+$/', + function (string $attribute, mixed $value, \Closure $fail) { + if (is_string($value) && ! Plugin::isValidIconName($value)) { + $fail('That isn\'t a Heroicon outline name. Browse the available icons at heroicons.com and use the name shown there, e.g. photo, map-pin, cube.'); + } + }, + ], ]); if ($this->plugin->logo_path) { diff --git a/app/Models/Plugin.php b/app/Models/Plugin.php index 4ea7381e..12490fa3 100644 --- a/app/Models/Plugin.php +++ b/app/Models/Plugin.php @@ -17,6 +17,8 @@ use App\Services\OgImageService; use App\Services\PluginSyncService; use App\Support\PluginReadme; +use BladeUI\Icons\Exceptions\SvgNotFound; +use BladeUI\Icons\Factory as IconFactory; use Illuminate\Database\Eloquent\Attributes\Scope; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Casts\Attribute; @@ -475,6 +477,47 @@ public function hasCustomIcon(): bool return $this->hasLogo() || $this->hasGradientIcon(); } + /** + * The Heroicon used when a plugin has no icon, or an unrecognised one. + */ + public const DEFAULT_ICON_NAME = 'cube'; + + /** + * Whether the given name matches an outline Heroicon we can actually render. + * + * Icon names are free text typed in by developers, so they regularly don't + * exist (e.g. "image" instead of "photo", or "location" instead of "map-pin"). + */ + public static function isValidIconName(?string $iconName): bool + { + if ($iconName === null || preg_match('/^[a-z0-9-]+$/', $iconName) !== 1) { + return false; + } + + try { + app(IconFactory::class)->svg('heroicon-o-'.$iconName); + } catch (SvgNotFound) { + return false; + } + + return true; + } + + /** + * The Blade component name for this plugin's gradient icon. + * + * Always resolvable: an unknown icon name would otherwise throw out of + * `` and take down the whole page. + */ + public function getIconComponent(): string + { + $iconName = self::isValidIconName($this->icon_name) + ? $this->icon_name + : self::DEFAULT_ICON_NAME; + + return 'heroicon-o-'.$iconName; + } + /** * Available gradient presets for plugin icons. * diff --git a/resources/views/cart/show.blade.php b/resources/views/cart/show.blade.php index 50883dbf..604d26de 100644 --- a/resources/views/cart/show.blade.php +++ b/resources/views/cart/show.blade.php @@ -210,7 +210,7 @@ class="flex gap-4 p-6" {{ $item->plugin->name }} @elseif ($item->plugin->hasGradientIcon())
- +
@else
diff --git a/resources/views/components/plugin-card.blade.php b/resources/views/components/plugin-card.blade.php index dd3663df..9d9f0807 100644 --- a/resources/views/components/plugin-card.blade.php +++ b/resources/views/components/plugin-card.blade.php @@ -13,7 +13,7 @@ class="size-12 shrink-0 rounded-xl object-cover" /> @elseif ($plugin->hasGradientIcon())
- +
@else
diff --git a/resources/views/customer/team/show.blade.php b/resources/views/customer/team/show.blade.php index f6d70085..a93f526a 100644 --- a/resources/views/customer/team/show.blade.php +++ b/resources/views/customer/team/show.blade.php @@ -43,7 +43,7 @@ {{ $plugin->name }} @elseif($plugin->hasGradientIcon())
- +
@else
diff --git a/resources/views/customer/ultra/index.blade.php b/resources/views/customer/ultra/index.blade.php index b475f467..b83b5c48 100644 --- a/resources/views/customer/ultra/index.blade.php +++ b/resources/views/customer/ultra/index.blade.php @@ -141,7 +141,7 @@ {{ $plugin->name }} @elseif($plugin->hasGradientIcon())
- +
@else
diff --git a/resources/views/livewire/customer/plugins/show.blade.php b/resources/views/livewire/customer/plugins/show.blade.php index 1f7214c5..1e82cf91 100644 --- a/resources/views/livewire/customer/plugins/show.blade.php +++ b/resources/views/livewire/customer/plugins/show.blade.php @@ -387,7 +387,7 @@ {{ $plugin->name }} logo @elseif ($plugin->hasGradientIcon())
- +
@endif Remove icon @@ -421,7 +421,7 @@ class="peer sr-only" wire:model="iconName" label="Heroicon name" placeholder="cube" - description="Enter a Heroicon outline name, e.g., cube, sparkles, bolt." + description="Enter an outline icon name exactly as it appears on heroicons.com, e.g., cube, sparkles, bolt, photo, map-pin." /> @error('iconName') {{ $message }} @@ -567,7 +567,7 @@ class="block text-sm text-gray-500 file:mr-4 file:rounded-md file:border-0 file: {{ $plugin->name }} logo @elseif ($plugin->hasGradientIcon())
- +
@endif Remove icon @@ -601,7 +601,7 @@ class="peer sr-only" wire:model="iconName" label="Heroicon name" placeholder="cube" - description="Enter a Heroicon outline name, e.g., cube, sparkles, bolt." + description="Enter an outline icon name exactly as it appears on heroicons.com, e.g., cube, sparkles, bolt, photo, map-pin." /> @error('iconName') {{ $message }} @@ -675,7 +675,7 @@ class="block text-sm text-gray-500 file:mr-4 file:rounded-md file:border-0 file: {{ $plugin->name }} logo @elseif ($plugin->hasGradientIcon())
- +
@else
@@ -825,7 +825,7 @@ class="block text-sm text-gray-500 file:mr-4 file:rounded-md file:border-0 file: {{ $plugin->name }} logo @elseif ($plugin->hasGradientIcon())
- +
@else
diff --git a/resources/views/livewire/customer/purchased-plugins.blade.php b/resources/views/livewire/customer/purchased-plugins.blade.php index 0eb8ec75..2f228caa 100644 --- a/resources/views/livewire/customer/purchased-plugins.blade.php +++ b/resources/views/livewire/customer/purchased-plugins.blade.php @@ -34,7 +34,7 @@ {{ $pluginLicense->plugin->name }} @elseif($pluginLicense->plugin->hasGradientIcon())
- +
@else
diff --git a/resources/views/plugin-show.blade.php b/resources/views/plugin-show.blade.php index 2bb6e7d0..e3cbff3a 100644 --- a/resources/views/plugin-show.blade.php +++ b/resources/views/plugin-show.blade.php @@ -73,7 +73,7 @@ class="size-16 shrink-0 rounded-2xl object-cover" /> @elseif ($plugin->hasGradientIcon())
- +
@else
diff --git a/tests/Feature/PluginIconFallbackTest.php b/tests/Feature/PluginIconFallbackTest.php new file mode 100644 index 00000000..bcc06a76 --- /dev/null +++ b/tests/Feature/PluginIconFallbackTest.php @@ -0,0 +1,159 @@ +create([ + 'github_id' => '12345', + 'github_token' => encrypt('fake-token'), + ]); + DeveloperAccount::factory()->withAcceptedTerms()->create([ + 'user_id' => $user->id, + ]); + + return $user; + } + + public function test_it_recognises_real_heroicon_outline_names(): void + { + $this->assertTrue(Plugin::isValidIconName('cube')); + $this->assertTrue(Plugin::isValidIconName('photo')); + $this->assertTrue(Plugin::isValidIconName('map-pin')); + } + + public function test_it_rejects_names_that_are_not_heroicons(): void + { + // The names from the reported 500s: neither exists in Heroicons. + $this->assertFalse(Plugin::isValidIconName('image')); + $this->assertFalse(Plugin::isValidIconName('location')); + + $this->assertFalse(Plugin::isValidIconName(null)); + $this->assertFalse(Plugin::isValidIconName('')); + $this->assertFalse(Plugin::isValidIconName('Cube')); + $this->assertFalse(Plugin::isValidIconName('../../secret')); + } + + public function test_icon_component_uses_the_stored_name_when_it_is_valid(): void + { + $plugin = Plugin::factory()->approved()->create([ + 'icon_gradient' => 'blue-cyan', + 'icon_name' => 'map-pin', + ]); + + $this->assertSame('heroicon-o-map-pin', $plugin->getIconComponent()); + } + + public function test_icon_component_falls_back_when_the_stored_name_is_unknown(): void + { + $plugin = Plugin::factory()->approved()->create([ + 'icon_gradient' => 'blue-cyan', + 'icon_name' => 'location', + ]); + + $this->assertSame('heroicon-o-cube', $plugin->getIconComponent()); + } + + public function test_developer_plugin_page_renders_with_an_unknown_stored_icon(): void + { + $user = $this->createUserWithGitHub(); + $plugin = Plugin::factory()->approved()->for($user)->create([ + 'icon_gradient' => 'blue-cyan', + 'icon_name' => 'location', + ]); + + [$vendor, $package] = explode('/', $plugin->name); + + Livewire::actingAs($user) + ->test(Show::class, ['vendor' => $vendor, 'package' => $package]) + ->assertStatus(200); + } + + public function test_plugin_directory_renders_with_an_unknown_stored_icon(): void + { + Plugin::factory()->approved()->create([ + 'icon_gradient' => 'blue-cyan', + 'icon_name' => 'image', + ]); + + Livewire::test(PluginDirectory::class) + ->assertStatus(200); + } + + public function test_public_plugin_listing_renders_with_an_unknown_stored_icon(): void + { + $plugin = Plugin::factory()->approved()->create([ + 'icon_gradient' => 'blue-cyan', + 'icon_name' => 'image', + ]); + + $this->get(route('plugins.show', $plugin->routeParams())) + ->assertOk(); + } + + public function test_updating_the_icon_rejects_a_name_that_is_not_a_heroicon(): void + { + $user = $this->createUserWithGitHub(); + $plugin = Plugin::factory()->draft()->for($user)->create([ + 'icon_gradient' => null, + 'icon_name' => null, + ]); + + [$vendor, $package] = explode('/', $plugin->name); + + Livewire::actingAs($user) + ->test(Show::class, ['vendor' => $vendor, 'package' => $package]) + ->set('iconGradient', 'blue-cyan') + ->set('iconName', 'location') + ->call('updateIcon') + ->assertHasErrors('iconName'); + + $plugin->refresh(); + $this->assertNull($plugin->icon_name); + $this->assertNull($plugin->icon_gradient); + } + + public function test_updating_the_icon_accepts_a_real_heroicon(): void + { + $user = $this->createUserWithGitHub(); + $plugin = Plugin::factory()->draft()->for($user)->create([ + 'icon_gradient' => null, + 'icon_name' => null, + ]); + + [$vendor, $package] = explode('/', $plugin->name); + + Livewire::actingAs($user) + ->test(Show::class, ['vendor' => $vendor, 'package' => $package]) + ->set('iconGradient', 'blue-cyan') + ->set('iconName', 'map-pin') + ->call('updateIcon') + ->assertHasNoErrors(); + + $plugin->refresh(); + $this->assertSame('map-pin', $plugin->icon_name); + $this->assertSame('blue-cyan', $plugin->icon_gradient); + } +} From 152e6cef61c35446cc27cfc24ed71b38b4217ca6 Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Tue, 25 Aug 2026 18:13:33 +0100 Subject: [PATCH 2/2] Make the plugin message thread order deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The messages() relation already sorts with oldest(), so the test's latest('id') landed as a secondary key: `order by created_at asc, id desc`. That returns the *earliest* message, not the newest. Laravel stores timestamps at second precision, so the test only passed when the admin message and the developer reply happened to land in the same second — it went red on CI the moment they straddled a second boundary. Order the relation by id, which is monotonic and unique, so the same chronological order holds with ties defined. Clear the inherited sort in the test with reorder(), and travel a second before replying so the cross-second case is always exercised rather than hidden by equal timestamps. Co-Authored-By: Claude Opus 5 (1M context) --- app/Models/Plugin.php | 2 +- tests/Feature/Livewire/Customer/PluginMessagesTest.php | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/Models/Plugin.php b/app/Models/Plugin.php index 12490fa3..3a0178f6 100644 --- a/app/Models/Plugin.php +++ b/app/Models/Plugin.php @@ -159,7 +159,7 @@ public function messages(): HasMany { return $this->hasMany(PluginActivity::class) ->messages() - ->oldest(); + ->oldest('id'); } /** diff --git a/tests/Feature/Livewire/Customer/PluginMessagesTest.php b/tests/Feature/Livewire/Customer/PluginMessagesTest.php index 5a7e42b2..66be084a 100644 --- a/tests/Feature/Livewire/Customer/PluginMessagesTest.php +++ b/tests/Feature/Livewire/Customer/PluginMessagesTest.php @@ -221,13 +221,19 @@ public function test_developer_can_reply_and_the_team_is_notified(): void Notification::fake(); + // Land the reply in a later second than the admin's message, so the + // thread ordering is exercised rather than hidden by equal timestamps. + $this->travel(1)->second(); + $this->testable($plugin) ->set('replyMessage', 'Sure — added in v1.2.0.') ->call('sendMessage') ->assertHasNoErrors() ->assertSet('replyMessage', ''); - $reply = $plugin->messages()->latest('id')->first(); + // reorder() clears the relation's own "oldest first" sort; without it + // this only sorts by id within a single second of created_at. + $reply = $plugin->messages()->reorder()->latest('id')->first(); $this->assertSame(PluginActivityType::MessageFromDeveloper, $reply->type); $this->assertSame('Sure — added in v1.2.0.', $reply->note);