From 4cc7d64a35eddcc0d05dc2f081a1ed32d0dd3c47 Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Tue, 25 Aug 2026 09:39:15 +0100 Subject: [PATCH 1/2] Sync paid plugins to Satis automatically, remove them when they go free Automatic Satis ingestion was switched off in 7d0e59f2, leaving the manual "Sync to Satis" admin action as the only reliable route in. A newly submitted paid plugin already has its releases tagged on GitHub, so no webhook ever fires and it never reaches Satis until someone remembers to click the button. Enforce the invariant at the model instead: a paid plugin belongs in Satis, a free one does not. - Add syncToSatis()/removeFromSatis() to Plugin, and drive them from the updated hook when type changes, so the admin form, the Convert to Paid action and the developer's own draft edit are all covered. - Queue a build from submit() and approve(). Ingesting from submission is deliberate: PluginAccessController already grants admins access to pending paid plugins for review, which only works if they're in Satis. - Clear satis_synced_at when a plugin goes free, and remove the package. Composer gives a custom repository precedence over Packagist, so a stale entry keeps shadowing the public package metadata. - Add RemovePluginFromSatis, a queued job taking the package name so it can still run once the row is gone, and use it for deletion too. - Stamp satis_synced_at in buildForPlugin(), so a successful satis:build no longer leaves the admin table's Satis column showing not-synced. Co-Authored-By: Claude Opus 5 (1M context) --- .../PluginResource/Pages/EditPlugin.php | 5 +- app/Jobs/RemovePluginFromSatis.php | 28 ++++ app/Models/Plugin.php | 53 ++++++- app/Services/SatisService.php | 8 +- tests/Feature/SatisSync/SatisSyncTest.php | 136 +++++++++++++++++- 5 files changed, 219 insertions(+), 11 deletions(-) create mode 100644 app/Jobs/RemovePluginFromSatis.php diff --git a/app/Filament/Resources/PluginResource/Pages/EditPlugin.php b/app/Filament/Resources/PluginResource/Pages/EditPlugin.php index 7558b5047..40cf339e9 100644 --- a/app/Filament/Resources/PluginResource/Pages/EditPlugin.php +++ b/app/Filament/Resources/PluginResource/Pages/EditPlugin.php @@ -9,7 +9,6 @@ use App\Jobs\GeneratePluginOgImage; use App\Jobs\ReviewPluginRepository; use App\Jobs\SyncPlugin; -use App\Jobs\SyncPluginReleases; use App\Models\PluginLicense; use App\Models\User; use App\Notifications\PluginGranted; @@ -129,8 +128,6 @@ protected function getHeaderActions(): array 'tier' => $data['tier'], ]); - SyncPluginReleases::dispatch($this->record); - Notification::make() ->title("Converted '{$this->record->name}' to paid") ->body('Plugin type updated, prices synced, and Satis ingestion queued.') @@ -152,7 +149,7 @@ protected function getHeaderActions(): array ? "Last synced: {$this->record->satis_synced_at->diffForHumans()}. This will trigger a new Satis build for '{$this->record->name}'." : "This will trigger a Satis build for '{$this->record->name}' so it's available via Composer.") ->action(function (): void { - SyncPluginReleases::dispatch($this->record); + $this->record->syncToSatis(); Notification::make() ->title('Satis sync queued') diff --git a/app/Jobs/RemovePluginFromSatis.php b/app/Jobs/RemovePluginFromSatis.php new file mode 100644 index 000000000..14141fcd1 --- /dev/null +++ b/app/Jobs/RemovePluginFromSatis.php @@ -0,0 +1,28 @@ +removePackage($this->packageName); + } +} diff --git a/app/Models/Plugin.php b/app/Models/Plugin.php index d81629b1f..4ea7381ea 100644 --- a/app/Models/Plugin.php +++ b/app/Models/Plugin.php @@ -7,14 +7,15 @@ use App\Enums\PluginTier; use App\Enums\PluginType; use App\Enums\PriceTier; +use App\Jobs\RemovePluginFromSatis; use App\Jobs\SendNewPluginNotifications; +use App\Jobs\SyncPluginReleases; use App\Notifications\PluginApproved; use App\Notifications\PluginDeveloperReplied; use App\Notifications\PluginMessageReceived; use App\Notifications\PluginRejected; use App\Services\OgImageService; use App\Services\PluginSyncService; -use App\Services\SatisService; use App\Support\PluginReadme; use Illuminate\Database\Eloquent\Attributes\Scope; use Illuminate\Database\Eloquent\Builder; @@ -81,13 +82,20 @@ protected static function booted(): void if ($plugin->wasChanged('tier') && $plugin->tier !== null) { $plugin->syncPricesFromTier(); } + + // Satis membership follows the plugin's type, whichever route changed it + if ($plugin->wasChanged('type')) { + if ($plugin->isPaid()) { + $plugin->syncToSatis(); + } else { + $plugin->removeFromSatis(); + $plugin->updateQuietly(['satis_synced_at' => null]); + } + } }); static::deleting(function (Plugin $plugin): void { - // Remove from Satis when plugin is deleted - if ($plugin->name) { - resolve(SatisService::class)->removePackage($plugin->name); - } + $plugin->removeFromSatis(); resolve(OgImageService::class)->deleteForPlugin($plugin); }); @@ -322,6 +330,37 @@ public function isSatisSynced(): bool return $this->satis_synced_at !== null; } + /** + * Queue a satis build so the plugin is installable via Composer. + * + * Paid plugins are ingested from submission onwards, not from approval, so + * that reviewers can install and test them while the plugin is pending. + */ + public function syncToSatis(): void + { + if (! $this->isPaid()) { + return; + } + + SyncPluginReleases::dispatch($this); + } + + /** + * Queue the plugin's removal from satis. + * + * Composer gives a custom repository precedence over Packagist, so a plugin + * left in satis after it stops being paid would keep shadowing the public + * package metadata. + */ + public function removeFromSatis(): void + { + if (! $this->name) { + return; + } + + RemovePluginFromSatis::dispatch($this->name); + } + /** * Check if all required review checks have passed. * A plugin cannot be approved until these checks pass. @@ -622,6 +661,8 @@ public function approve(int $approvedById): void } resolve(PluginSyncService::class)->sync($this); + + $this->syncToSatis(); } public function reject(string $reason, int $rejectedById): void @@ -692,6 +733,8 @@ public function submit(): void null, $this->user_id ); + + $this->syncToSatis(); } /** diff --git a/app/Services/SatisService.php b/app/Services/SatisService.php index 29a9c64e2..6422916af 100644 --- a/app/Services/SatisService.php +++ b/app/Services/SatisService.php @@ -77,7 +77,13 @@ public function buildAll(): array */ public function buildForPlugin(Plugin $plugin): array { - return $this->build([$plugin], $this->resolveGitHubTokenFor($plugin)); + $result = $this->build([$plugin], $this->resolveGitHubTokenFor($plugin)); + + if ($result['success'] ?? false) { + $plugin->update(['satis_synced_at' => now()]); + } + + return $result; } /** diff --git a/tests/Feature/SatisSync/SatisSyncTest.php b/tests/Feature/SatisSync/SatisSyncTest.php index 44fd1b368..8de6e5362 100644 --- a/tests/Feature/SatisSync/SatisSyncTest.php +++ b/tests/Feature/SatisSync/SatisSyncTest.php @@ -2,7 +2,9 @@ namespace Tests\Feature\SatisSync; +use App\Enums\PluginType; use App\Filament\Resources\PluginResource\Pages\EditPlugin; +use App\Jobs\RemovePluginFromSatis; use App\Jobs\SyncPluginReleases; use App\Models\Plugin; use App\Models\User; @@ -27,17 +29,123 @@ protected function setUp(): void config(['filament.users' => ['admin@test.com']]); } - public function test_approval_does_not_dispatch_sync_plugin_releases(): void + public function test_submitting_a_paid_plugin_queues_a_satis_build(): void { Bus::fake([SyncPluginReleases::class]); + $plugin = Plugin::factory()->paid()->draft()->create(); + + $plugin->submit(); + + Bus::assertDispatched(SyncPluginReleases::class, function ($job) use ($plugin) { + return $job->plugin->is($plugin); + }); + } + + public function test_submitting_a_free_plugin_does_not_queue_a_satis_build(): void + { + Bus::fake([SyncPluginReleases::class]); + + $plugin = Plugin::factory()->free()->draft()->create(); + + $plugin->submit(); + + Bus::assertNotDispatched(SyncPluginReleases::class); + } + + public function test_approval_queues_a_satis_build_for_paid_plugins(): void + { + Http::fake(); + Bus::fake([SyncPluginReleases::class]); + $plugin = Plugin::factory()->paid()->pending()->create(); $plugin->approve($this->admin->id); + Bus::assertDispatched(SyncPluginReleases::class, function ($job) use ($plugin) { + return $job->plugin->is($plugin); + }); + } + + public function test_approval_does_not_queue_a_satis_build_for_free_plugins(): void + { + Http::fake(); + Bus::fake([SyncPluginReleases::class]); + + $plugin = Plugin::factory()->free()->pending()->create(); + + $plugin->approve($this->admin->id); + Bus::assertNotDispatched(SyncPluginReleases::class); } + public function test_switching_a_plugin_to_paid_queues_a_satis_build(): void + { + Bus::fake([SyncPluginReleases::class]); + + $plugin = Plugin::factory()->free()->approved()->create(); + + $plugin->update(['type' => PluginType::Paid]); + + Bus::assertDispatched(SyncPluginReleases::class, function ($job) use ($plugin) { + return $job->plugin->is($plugin); + }); + } + + public function test_switching_a_plugin_to_free_removes_it_from_satis(): void + { + Bus::fake([RemovePluginFromSatis::class]); + + $plugin = Plugin::factory()->paid()->approved()->create([ + 'satis_synced_at' => now(), + ]); + + $plugin->update(['type' => PluginType::Free]); + + Bus::assertDispatched(RemovePluginFromSatis::class, function ($job) use ($plugin) { + return $job->packageName === $plugin->name; + }); + + $this->assertNull($plugin->fresh()->satis_synced_at); + } + + public function test_editing_a_plugin_without_changing_its_type_leaves_satis_alone(): void + { + Bus::fake([SyncPluginReleases::class, RemovePluginFromSatis::class]); + + $plugin = Plugin::factory()->paid()->approved()->create(); + + $plugin->update(['description' => 'A freshly worded description.']); + + Bus::assertNotDispatched(SyncPluginReleases::class); + Bus::assertNotDispatched(RemovePluginFromSatis::class); + } + + public function test_deleting_a_plugin_removes_it_from_satis(): void + { + Bus::fake([RemovePluginFromSatis::class]); + + $plugin = Plugin::factory()->paid()->approved()->create(); + $packageName = $plugin->name; + + $plugin->delete(); + + Bus::assertDispatched(RemovePluginFromSatis::class, function ($job) use ($packageName) { + return $job->packageName === $packageName; + }); + } + + public function test_remove_plugin_from_satis_job_calls_the_satis_api(): void + { + $satisService = $this->mock(SatisService::class); + $satisService->shouldReceive('removePackage') + ->once() + ->with('acme/widget') + ->andReturn(['success' => true]); + + (new RemovePluginFromSatis('acme/widget'))->handle($satisService); + } + public function test_filament_sync_to_satis_action_dispatches_job(): void { Bus::fake([SyncPluginReleases::class]); @@ -126,6 +234,32 @@ public function test_is_satis_synced_returns_true_when_synced(): void $this->assertTrue($plugin->isSatisSynced()); } + public function test_building_a_single_plugin_stamps_satis_synced_at(): void + { + Http::fake(['*' => Http::response(['job_id' => 'test-123', 'message' => 'Build started'], 200)]); + + config(['services.satis.url' => 'https://satis.test', 'services.satis.api_key' => 'test-key']); + + $plugin = Plugin::factory()->paid()->approved()->create(); + + (new SatisService)->buildForPlugin($plugin); + + $this->assertNotNull($plugin->fresh()->satis_synced_at); + } + + public function test_building_a_single_plugin_does_not_stamp_satis_synced_at_on_failure(): void + { + Http::fake(['*' => Http::response(['error' => 'Boom'], 500)]); + + config(['services.satis.url' => 'https://satis.test', 'services.satis.api_key' => 'test-key']); + + $plugin = Plugin::factory()->paid()->approved()->create(); + + (new SatisService)->buildForPlugin($plugin); + + $this->assertNull($plugin->fresh()->satis_synced_at); + } + public function test_build_all_only_includes_paid_plugins(): void { Http::fake(['*' => Http::response(['job_id' => 'test-123', 'message' => 'Build started'], 200)]); From 276618af21a293b679d37b0ce90203b12552ace9 Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Tue, 25 Aug 2026 09:54:44 +0100 Subject: [PATCH 2/2] Let SatisService tolerate missing Satis configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SATIS_API_KEY has no default in config/services.php, so it is null whenever the env var is unset — as it is in CI. SatisService typed both properties as non-nullable string, so the service could not be constructed at all and fatalled in its own constructor, which made the "Satis API not configured" guards in removePackage() and triggerBuild() unreachable. This was latent before: SatisService was only ever built in the deleting hook. Now that SyncPluginReleases and RemovePluginFromSatis resolve it via method injection, any free/paid transition tripped it under the sync queue. - Make $apiUrl/$apiKey nullable so the existing degradation path works. - Return [] from fetchReleases() when the GitHub response has no JSON body; a 200 with a non-JSON body otherwise fatals against the array return type. - Cover both with regression tests that null the config explicitly, rather than depending on whether the developer happens to have SATIS_* in .env. Co-Authored-By: Claude Opus 5 (1M context) --- app/Jobs/SyncPluginReleases.php | 2 +- app/Services/SatisService.php | 8 ++++++-- tests/Feature/SatisSync/SatisSyncTest.php | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/app/Jobs/SyncPluginReleases.php b/app/Jobs/SyncPluginReleases.php index 513ef8710..e6003c008 100644 --- a/app/Jobs/SyncPluginReleases.php +++ b/app/Jobs/SyncPluginReleases.php @@ -159,7 +159,7 @@ protected function fetchReleases(string $owner, string $repo, ?string $token): a 'rate_limit_remaining' => $response->header('X-RateLimit-Remaining'), ]); - return $response->json(); + return $response->json() ?? []; } protected function processRelease(array $release): bool diff --git a/app/Services/SatisService.php b/app/Services/SatisService.php index 6422916af..b93bd84c4 100644 --- a/app/Services/SatisService.php +++ b/app/Services/SatisService.php @@ -13,9 +13,13 @@ class SatisService { use ResolvesGitHubToken; - protected string $apiUrl; + /** + * Nullable because SATIS_API_KEY has no default: every caller already + * degrades to a "Satis API not configured" result rather than failing. + */ + protected ?string $apiUrl; - protected string $apiKey; + protected ?string $apiKey; public function __construct() { diff --git a/tests/Feature/SatisSync/SatisSyncTest.php b/tests/Feature/SatisSync/SatisSyncTest.php index 8de6e5362..52d81be7f 100644 --- a/tests/Feature/SatisSync/SatisSyncTest.php +++ b/tests/Feature/SatisSync/SatisSyncTest.php @@ -135,6 +135,29 @@ public function test_deleting_a_plugin_removes_it_from_satis(): void }); } + public function test_type_changes_survive_satis_being_unconfigured(): void + { + Http::fake(); + config(['services.satis.url' => null, 'services.satis.api_key' => null]); + + $plugin = Plugin::factory()->free()->approved()->create(); + + $plugin->update(['type' => PluginType::Paid]); + $plugin->update(['type' => PluginType::Free]); + + $this->assertTrue($plugin->fresh()->isFree()); + } + + public function test_satis_service_reports_missing_configuration_rather_than_failing(): void + { + config(['services.satis.url' => null, 'services.satis.api_key' => null]); + + $service = new SatisService; + + $this->assertFalse($service->removePackage('acme/widget')['success']); + $this->assertFalse($service->build([Plugin::factory()->paid()->approved()->create()])['success']); + } + public function test_remove_plugin_from_satis_job_calls_the_satis_api(): void { $satisService = $this->mock(SatisService::class);