From 4b12f9df3090eda301092ad2dc36d7ed7beca2f1 Mon Sep 17 00:00:00 2001 From: myrmidex Date: Sat, 25 Jul 2026 20:08:19 +0200 Subject: [PATCH 1/4] 106 - Add feed creation modal and Add button to Feeds page --- app/Livewire/Feeds.php | 65 ++++++++++ resources/views/livewire/feeds.blade.php | 117 ++++++++++++++++- tests/Feature/Livewire/FeedsTest.php | 157 +++++++++++++++++++++++ 3 files changed, 338 insertions(+), 1 deletion(-) create mode 100644 tests/Feature/Livewire/FeedsTest.php diff --git a/app/Livewire/Feeds.php b/app/Livewire/Feeds.php index b1f86be7..1ac63001 100644 --- a/app/Livewire/Feeds.php +++ b/app/Livewire/Feeds.php @@ -2,12 +2,25 @@ namespace App\Livewire; +use App\Actions\CreateFeedAction; use App\Models\Feed; +use App\Models\Language; use Illuminate\Contracts\View\View; +use InvalidArgumentException; use Livewire\Component; class Feeds extends Component { + public bool $showCreateModal = false; + + public string $newName = ''; + + public string $newProvider = ''; + + public ?int $newLanguageId = null; + + public string $newDescription = ''; + public function toggle(int $feedId): void { $feed = Feed::findOrFail($feedId); @@ -15,12 +28,64 @@ public function toggle(int $feedId): void $feed->save(); } + public function openCreateModal(): void + { + $this->reset(['newName', 'newProvider', 'newLanguageId', 'newDescription']); + $this->resetErrorBag(); + $this->showCreateModal = true; + } + + public function closeCreateModal(): void + { + $this->showCreateModal = false; + } + + public function createFeed(CreateFeedAction $action): void + { + $providers = array_keys($this->activeProviders()); + + $this->validate([ + 'newName' => 'required|string|max:255', + 'newProvider' => ['required', 'string', 'in:'.implode(',', $providers)], + 'newLanguageId' => 'required|integer|exists:languages,id', + ]); + + try { + $action->execute( + $this->newName, + $this->newProvider, + $this->newLanguageId, + // Blade textarea binds an empty string when blank; the action expects null for "no description". + $this->newDescription !== '' ? $this->newDescription : null, + ); + } catch (InvalidArgumentException $e) { + $this->addError('newProvider', 'This provider is not available for the selected language.'); + + return; + } + + $this->closeCreateModal(); + } + + /** + * @return array> + */ + private function activeProviders(): array + { + /** @var array> $providers */ + $providers = config('feed.providers', []); + + return array_filter($providers, fn (array $provider): bool => ($provider['is_active'] ?? false) === true); + } + public function render(): View { $feeds = Feed::orderBy('name')->get(); return view('livewire.feeds', [ 'feeds' => $feeds, + 'providers' => $this->activeProviders(), + 'languages' => Language::where('is_active', true)->orderBy('name')->get(), ])->layout('layouts.app'); } } diff --git a/resources/views/livewire/feeds.blade.php b/resources/views/livewire/feeds.blade.php index 5f4fc72f..7ce870fc 100644 --- a/resources/views/livewire/feeds.blade.php +++ b/resources/views/livewire/feeds.blade.php @@ -1,5 +1,15 @@
- + + +
@forelse ($feeds as $feed) @@ -76,7 +86,112 @@ class="text-gray-400 hover:text-gray-600"

No feeds have been configured yet.

+
+ +
@endforelse
+ + + @if ($showCreateModal) + + @endif diff --git a/tests/Feature/Livewire/FeedsTest.php b/tests/Feature/Livewire/FeedsTest.php new file mode 100644 index 00000000..5e518173 --- /dev/null +++ b/tests/Feature/Livewire/FeedsTest.php @@ -0,0 +1,157 @@ +create(['name' => 'Existing Feed']); + + Livewire::test(Feeds::class) + ->assertSee('Add Feed') + ->assertSee('Existing Feed'); + } + + public function test_add_button_renders_in_empty_state(): void + { + Livewire::test(Feeds::class) + ->assertSee('No feeds') + ->assertSee('Add Feed'); + } + + public function test_open_create_modal_shows_modal(): void + { + Livewire::test(Feeds::class) + ->assertSet('showCreateModal', false) + ->call('openCreateModal') + ->assertSet('showCreateModal', true); + } + + public function test_create_feed_requires_name(): void + { + $language = Language::factory()->english()->create(); + + Livewire::test(Feeds::class) + ->call('openCreateModal') + ->set('newProvider', 'guardian') + ->set('newLanguageId', $language->id) + ->call('createFeed') + ->assertHasErrors(['newName' => 'required']); + } + + public function test_create_feed_requires_provider(): void + { + $language = Language::factory()->english()->create(); + + Livewire::test(Feeds::class) + ->call('openCreateModal') + ->set('newName', 'Tech News') + ->set('newLanguageId', $language->id) + ->call('createFeed') + ->assertHasErrors(['newProvider' => 'required']); + } + + public function test_create_feed_requires_language(): void + { + Livewire::test(Feeds::class) + ->call('openCreateModal') + ->set('newName', 'Tech News') + ->set('newProvider', 'guardian') + ->call('createFeed') + ->assertHasErrors(['newLanguageId' => 'required']); + } + + public function test_create_feed_rejects_unknown_provider(): void + { + $language = Language::factory()->english()->create(); + + Livewire::test(Feeds::class) + ->call('openCreateModal') + ->set('newName', 'Tech News') + ->set('newProvider', 'nonexistent') + ->set('newLanguageId', $language->id) + ->call('createFeed') + ->assertHasErrors('newProvider'); + + $this->assertDatabaseCount('feeds', 0); + } + + public function test_create_feed_succeeds_and_resolves_url(): void + { + $language = Language::factory()->english()->create(); + + Livewire::test(Feeds::class) + ->call('openCreateModal') + ->set('newName', 'Guardian News') + ->set('newProvider', 'guardian') + ->set('newLanguageId', $language->id) + ->set('newDescription', 'British daily') + ->call('createFeed') + ->assertHasNoErrors() + ->assertSet('showCreateModal', false); + + $this->assertDatabaseHas('feeds', [ + 'name' => 'Guardian News', + 'provider' => 'guardian', + 'language_id' => $language->id, + 'url' => 'https://www.theguardian.com/international/rss', + 'description' => 'British daily', + 'is_active' => true, + ]); + } + + public function test_create_feed_leaves_description_null_when_blank(): void + { + $language = Language::factory()->english()->create(); + + Livewire::test(Feeds::class) + ->call('openCreateModal') + ->set('newName', 'Guardian News') + ->set('newProvider', 'guardian') + ->set('newLanguageId', $language->id) + ->call('createFeed') + ->assertHasNoErrors(); + + $this->assertDatabaseHas('feeds', [ + 'name' => 'Guardian News', + 'description' => null, + ]); + } + + public function test_create_feed_surfaces_invalid_provider_language_combination(): void + { + // Belga only supports 'en'; a non-'en' language triggers InvalidArgumentException in the action. + $language = Language::factory()->create(['short_code' => 'nl', 'name' => 'Dutch', 'is_active' => true]); + + Livewire::test(Feeds::class) + ->call('openCreateModal') + ->set('newName', 'Belga NL') + ->set('newProvider', 'belga') + ->set('newLanguageId', $language->id) + ->call('createFeed') + ->assertHasErrors('newProvider') + ->assertSet('showCreateModal', true); + + $this->assertDatabaseCount('feeds', 0); + } + + public function test_toggle_flips_active_state(): void + { + $feed = Feed::factory()->create(['is_active' => true]); + + Livewire::test(Feeds::class) + ->call('toggle', $feed->id); + + $this->assertFalse($feed->fresh()->is_active); + } +} -- 2.45.2 From d676d7866bff6a1aa2358a3159672ad4f2cf0d05 Mon Sep 17 00:00:00 2001 From: myrmidex Date: Sat, 25 Jul 2026 20:15:52 +0200 Subject: [PATCH 2/4] 106 - Add channel creation modal and Add button to Channels page --- app/Livewire/Channels.php | 66 +++++++ resources/views/livewire/channels.blade.php | 117 +++++++++++- tests/Feature/Livewire/ChannelsTest.php | 199 ++++++++++++++++++++ 3 files changed, 381 insertions(+), 1 deletion(-) create mode 100644 tests/Feature/Livewire/ChannelsTest.php diff --git a/app/Livewire/Channels.php b/app/Livewire/Channels.php index 49ea79ca..2dab6276 100644 --- a/app/Livewire/Channels.php +++ b/app/Livewire/Channels.php @@ -2,15 +2,30 @@ namespace App\Livewire; +use App\Actions\CreateChannelAction; +use App\Models\Language; use App\Models\PlatformAccount; use App\Models\PlatformChannel; +use App\Models\PlatformInstance; use Illuminate\Contracts\View\View; +use Illuminate\Validation\Rule; use Livewire\Component; +use RuntimeException; class Channels extends Component { public ?int $managingChannelId = null; + public bool $showCreateModal = false; + + public string $newName = ''; + + public ?int $newPlatformInstanceId = null; + + public ?int $newLanguageId = null; + + public string $newDescription = ''; + public function toggle(int $channelId): void { $channel = PlatformChannel::findOrFail($channelId); @@ -18,6 +33,55 @@ public function toggle(int $channelId): void $channel->save(); } + public function openCreateModal(): void + { + $this->reset(['newName', 'newPlatformInstanceId', 'newLanguageId', 'newDescription']); + $this->resetErrorBag(); + $this->showCreateModal = true; + } + + public function closeCreateModal(): void + { + $this->showCreateModal = false; + } + + public function createChannel(CreateChannelAction $action): void + { + $this->validate([ + // name doubles as the Lemmy community slug (used verbatim as channel_id for + // community lookup at publish time), so it must be lowercase slug format. + 'newName' => [ + 'required', + 'string', + 'max:255', + 'regex:/^[a-z0-9_]+$/', + Rule::unique('platform_channels', 'name') + ->where('platform_instance_id', $this->newPlatformInstanceId), + ], + 'newPlatformInstanceId' => 'required|integer|exists:platform_instances,id', + 'newLanguageId' => 'nullable|integer|exists:languages,id', + ], [ + 'newName.regex' => 'The name must be a valid community slug (lowercase letters, numbers, and underscores only).', + 'newName.unique' => 'A channel with this name already exists for this instance.', + ]); + + try { + $action->execute( + $this->newName, + $this->newPlatformInstanceId, + $this->newLanguageId, + // Blade textarea binds an empty string when blank; the action expects null for "no description". + $this->newDescription !== '' ? $this->newDescription : null, + ); + } catch (RuntimeException $e) { + $this->addError('newPlatformInstanceId', $e->getMessage()); + + return; + } + + $this->closeCreateModal(); + } + public function openAccountModal(int $channelId): void { $this->managingChannelId = $channelId; @@ -69,6 +133,8 @@ public function render(): View 'channels' => $channels, 'managingChannel' => $managingChannel, 'availableAccounts' => $availableAccounts, + 'platformInstances' => PlatformInstance::where('is_active', true)->orderBy('name')->get(), + 'languages' => Language::where('is_active', true)->orderBy('name')->get(), ])->layout('layouts.app'); } } diff --git a/resources/views/livewire/channels.blade.php b/resources/views/livewire/channels.blade.php index 9bf0c75c..756a6e60 100644 --- a/resources/views/livewire/channels.blade.php +++ b/resources/views/livewire/channels.blade.php @@ -1,5 +1,15 @@
- + + +
@forelse ($channels as $channel) @@ -98,6 +108,17 @@ class="text-red-500 hover:text-red-700"

No platform channels have been configured yet.

+
+ +
@endforelse
@@ -155,4 +176,98 @@ class="w-full inline-flex justify-center rounded-md border border-gray-300 shado @endif + + + @if ($showCreateModal) + + @endif diff --git a/tests/Feature/Livewire/ChannelsTest.php b/tests/Feature/Livewire/ChannelsTest.php new file mode 100644 index 00000000..ff3d5188 --- /dev/null +++ b/tests/Feature/Livewire/ChannelsTest.php @@ -0,0 +1,199 @@ +create(['url' => 'https://lemmy.world']); + PlatformAccount::factory()->create([ + 'instance_url' => 'https://lemmy.world', + 'is_active' => true, + ]); + + return $instance; + } + + public function test_add_button_renders_when_channels_exist(): void + { + PlatformChannel::factory()->create(['name' => 'existing_channel']); + + Livewire::test(Channels::class) + ->assertSee('Add Channel') + ->assertSee('existing_channel'); + } + + public function test_add_button_renders_in_empty_state(): void + { + Livewire::test(Channels::class) + ->assertSee('No channels') + ->assertSee('Add Channel'); + } + + public function test_open_create_modal_shows_modal(): void + { + Livewire::test(Channels::class) + ->assertSet('showCreateModal', false) + ->call('openCreateModal') + ->assertSet('showCreateModal', true); + } + + public function test_create_channel_requires_name(): void + { + $instance = $this->instanceWithActiveAccount(); + + Livewire::test(Channels::class) + ->call('openCreateModal') + ->set('newPlatformInstanceId', $instance->id) + ->call('createChannel') + ->assertHasErrors(['newName' => 'required']); + } + + public function test_create_channel_requires_platform_instance(): void + { + Livewire::test(Channels::class) + ->call('openCreateModal') + ->set('newName', 'tech_community') + ->call('createChannel') + ->assertHasErrors(['newPlatformInstanceId' => 'required']); + } + + public function test_create_channel_succeeds_and_attaches_account(): void + { + $instance = $this->instanceWithActiveAccount(); + $language = Language::factory()->create(); + + Livewire::test(Channels::class) + ->call('openCreateModal') + ->set('newName', 'tech_community') + ->set('newPlatformInstanceId', $instance->id) + ->set('newLanguageId', $language->id) + ->set('newDescription', 'A tech community') + ->call('createChannel') + ->assertHasNoErrors() + ->assertSet('showCreateModal', false); + + $this->assertDatabaseHas('platform_channels', [ + 'name' => 'tech_community', + 'display_name' => 'Tech_community', + 'platform_instance_id' => $instance->id, + 'language_id' => $language->id, + 'description' => 'A tech community', + 'is_active' => true, + ]); + + $channel = PlatformChannel::where('name', 'tech_community')->firstOrFail(); + $this->assertCount(1, $channel->platformAccounts); + } + + public function test_create_channel_leaves_description_null_when_blank(): void + { + $instance = $this->instanceWithActiveAccount(); + + Livewire::test(Channels::class) + ->call('openCreateModal') + ->set('newName', 'tech_community') + ->set('newPlatformInstanceId', $instance->id) + ->call('createChannel') + ->assertHasNoErrors(); + + $this->assertDatabaseHas('platform_channels', [ + 'name' => 'tech_community', + 'description' => null, + ]); + } + + public function test_create_channel_rejects_non_slug_name(): void + { + $instance = $this->instanceWithActiveAccount(); + + Livewire::test(Channels::class) + ->call('openCreateModal') + ->set('newName', 'Tech News') + ->set('newPlatformInstanceId', $instance->id) + ->call('createChannel') + ->assertHasErrors(['newName' => 'regex']); + + $this->assertDatabaseCount('platform_channels', 0); + } + + public function test_create_channel_rejects_duplicate_name_on_same_instance(): void + { + $instance = $this->instanceWithActiveAccount(); + PlatformChannel::factory()->create([ + 'platform_instance_id' => $instance->id, + 'name' => 'tech_community', + ]); + + Livewire::test(Channels::class) + ->call('openCreateModal') + ->set('newName', 'tech_community') + ->set('newPlatformInstanceId', $instance->id) + ->call('createChannel') + ->assertHasErrors(['newName' => 'unique']) + ->assertSet('showCreateModal', true); + + $this->assertDatabaseCount('platform_channels', 1); + } + + public function test_create_channel_allows_same_name_on_different_instance(): void + { + $instanceA = $this->instanceWithActiveAccount(); + $instanceB = PlatformInstance::factory()->create(['url' => 'https://lemmy.other']); + PlatformAccount::factory()->create([ + 'instance_url' => 'https://lemmy.other', + 'is_active' => true, + ]); + PlatformChannel::factory()->create([ + 'platform_instance_id' => $instanceB->id, + 'name' => 'tech_community', + ]); + + Livewire::test(Channels::class) + ->call('openCreateModal') + ->set('newName', 'tech_community') + ->set('newPlatformInstanceId', $instanceA->id) + ->call('createChannel') + ->assertHasNoErrors(); + + $this->assertDatabaseCount('platform_channels', 2); + } + + public function test_create_channel_surfaces_no_active_accounts_error(): void + { + // Instance exists but has no active account for its url. + $instance = PlatformInstance::factory()->create(['url' => 'https://lemmy.world']); + + Livewire::test(Channels::class) + ->call('openCreateModal') + ->set('newName', 'tech_community') + ->set('newPlatformInstanceId', $instance->id) + ->call('createChannel') + ->assertHasErrors('newPlatformInstanceId') + ->assertSet('showCreateModal', true); + + $this->assertDatabaseCount('platform_channels', 0); + } + + public function test_toggle_flips_active_state(): void + { + $channel = PlatformChannel::factory()->create(['is_active' => true]); + + Livewire::test(Channels::class) + ->call('toggle', $channel->id); + + $this->assertFalse($channel->fresh()->is_active); + } +} -- 2.45.2 From 839f24700617b7c754277205d9c5103505a8b9dd Mon Sep 17 00:00:00 2001 From: myrmidex Date: Wed, 29 Jul 2026 20:15:43 +0200 Subject: [PATCH 3/4] 106 - Enforce channel_id integrity across create paths and sync job --- .gitignore | 6 +- .../Api/V1/PlatformChannelsController.php | 6 + .../Requests/StorePlatformChannelRequest.php | 26 ++++- app/Jobs/SyncChannelPostsJob.php | 23 +++- app/Livewire/Channels.php | 8 ++ app/Models/PlatformChannel.php | 2 +- ...channel_id_unique_to_platform_channels.php | 57 ++++++++++ .../Api/V1/PlatformChannelsControllerTest.php | 63 +++++++++++ tests/Unit/Jobs/SyncChannelPostsJobTest.php | 105 ++++++++++++++++++ tests/Unit/Models/PlatformChannelTest.php | 37 ++++++ 10 files changed, 319 insertions(+), 14 deletions(-) create mode 100644 database/migrations/2024_01_01_000011_add_channel_id_unique_to_platform_channels.php diff --git a/.gitignore b/.gitignore index 970182b9..58008dd4 100644 --- a/.gitignore +++ b/.gitignore @@ -20,11 +20,7 @@ yarn-error.log /package-lock.json /auth.json /composer.lock -/.fleet /.idea -/.nova -/.vscode -/.zed /coverage-report* /coverage.xml -/.claude +/.php-cs-fixer.dist.php diff --git a/app/Http/Controllers/Api/V1/PlatformChannelsController.php b/app/Http/Controllers/Api/V1/PlatformChannelsController.php index a72f5ed4..443ba540 100644 --- a/app/Http/Controllers/Api/V1/PlatformChannelsController.php +++ b/app/Http/Controllers/Api/V1/PlatformChannelsController.php @@ -8,6 +8,7 @@ use App\Models\PlatformAccount; use App\Models\PlatformChannel; use Exception; +use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; @@ -51,6 +52,11 @@ public function store(StorePlatformChannelRequest $request, CreateChannelAction 'Platform channel created successfully and linked to platform account!', 201 ); + } catch (UniqueConstraintViolationException $e) { + // The (platform_instance_id, channel_id) unique index is the last line of + // defence if a duplicate slips past request validation — surface it as a + // validation error rather than leaking the driver's SQL message in a 500. + return $this->sendError('A channel with this name already exists for this instance.', [], 422); } catch (RuntimeException $e) { return $this->sendError($e->getMessage(), [], 422); } catch (Exception $e) { diff --git a/app/Http/Requests/StorePlatformChannelRequest.php b/app/Http/Requests/StorePlatformChannelRequest.php index 0950699f..0a1af8c6 100644 --- a/app/Http/Requests/StorePlatformChannelRequest.php +++ b/app/Http/Requests/StorePlatformChannelRequest.php @@ -3,6 +3,7 @@ namespace App\Http\Requests; use Illuminate\Foundation\Http\FormRequest; +use Illuminate\Validation\Rule; class StorePlatformChannelRequest extends FormRequest { @@ -12,15 +13,36 @@ public function authorize(): bool } /** - * @return array + * @return array|string> */ public function rules(): array { return [ 'platform_instance_id' => 'required|exists:platform_instances,id', - 'name' => 'required|string|max:255', + // name doubles as the Lemmy community slug (CreateChannelAction copies it + // verbatim into channel_id for community lookup at publish time), so it must + // be slug format and unique per instance — matching the Livewire create form. + 'name' => [ + 'required', + 'string', + 'max:255', + 'regex:/^[a-z0-9_]+$/', + Rule::unique('platform_channels', 'name') + ->where('platform_instance_id', $this->input('platform_instance_id')), + ], 'language_id' => 'nullable|exists:languages,id', 'description' => 'nullable|string', ]; } + + /** + * @return array + */ + public function messages(): array + { + return [ + 'name.regex' => 'The name must be a valid community slug (lowercase letters, numbers, and underscores only).', + 'name.unique' => 'A channel with this name already exists for this instance.', + ]; + } } diff --git a/app/Jobs/SyncChannelPostsJob.php b/app/Jobs/SyncChannelPostsJob.php index 3ded0649..edf86824 100644 --- a/app/Jobs/SyncChannelPostsJob.php +++ b/app/Jobs/SyncChannelPostsJob.php @@ -65,17 +65,19 @@ private function syncLemmyChannelPosts(LogSaver $logSaver): void throw new PlatformAuthException(PlatformEnum::LEMMY, 'No active account found for channel'); } - $api = new LemmyApiService($this->channel->platformInstance->url); + $api = $this->makeApiService($this->channel->platformInstance->url); $token = $this->getAuthToken($api, $account); - $platformChannelId = $this->channel->channel_id - ? $this->channel->channel_id - : $api->getCommunityId($this->channel->name, $token); + // channel_id holds a Lemmy community slug (non-numeric) or a numeric + // community id; syncChannelPosts() needs the numeric id. Mirror the + // resolution used in LemmyPublisher::createPost(). + $communityId = is_numeric($this->channel->channel_id) + ? (int) $this->channel->channel_id + : $api->getCommunityId($this->channel->channel_id, $token); - $api->syncChannelPosts($token, $platformChannelId, $this->channel->name); + $api->syncChannelPosts($token, $communityId, $this->channel->name); $logSaver->info('Channel posts synced successfully', $this->channel); - } catch (Exception $e) { $logSaver->error('Failed to sync channel posts', $this->channel, [ 'error' => $e->getMessage(), @@ -85,6 +87,15 @@ private function syncLemmyChannelPosts(LogSaver $logSaver): void } } + /** + * Seam for testing — LemmyApiService needs a per-instance URL so it cannot be + * container-resolved. Override in tests to inject a mock. + */ + protected function makeApiService(string $instanceUrl): LemmyApiService + { + return new LemmyApiService($instanceUrl); + } + /** * @throws PlatformAuthException */ diff --git a/app/Livewire/Channels.php b/app/Livewire/Channels.php index 2dab6276..0b785186 100644 --- a/app/Livewire/Channels.php +++ b/app/Livewire/Channels.php @@ -8,6 +8,7 @@ use App\Models\PlatformChannel; use App\Models\PlatformInstance; use Illuminate\Contracts\View\View; +use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Validation\Rule; use Livewire\Component; use RuntimeException; @@ -73,6 +74,13 @@ public function createChannel(CreateChannelAction $action): void // Blade textarea binds an empty string when blank; the action expects null for "no description". $this->newDescription !== '' ? $this->newDescription : null, ); + } catch (UniqueConstraintViolationException $e) { + // Unreachable via this form (the unique rule above catches duplicates first), + // but the (platform_instance_id, channel_id) index can still fire if channel_id + // ever drifts from name. Surface it as a field error instead of a 500. + $this->addError('newName', 'A channel with this name already exists for this instance.'); + + return; } catch (RuntimeException $e) { $this->addError('newPlatformInstanceId', $e->getMessage()); diff --git a/app/Models/PlatformChannel.php b/app/Models/PlatformChannel.php index 9291a36e..055b5f60 100644 --- a/app/Models/PlatformChannel.php +++ b/app/Models/PlatformChannel.php @@ -15,7 +15,7 @@ * @property int $id * @property int $platform_instance_id * @property PlatformInstance $platformInstance - * @property int $channel_id + * @property string $channel_id * @property string $name * @property int $language_id * @property Language|null $language diff --git a/database/migrations/2024_01_01_000011_add_channel_id_unique_to_platform_channels.php b/database/migrations/2024_01_01_000011_add_channel_id_unique_to_platform_channels.php new file mode 100644 index 00000000..952f9b64 --- /dev/null +++ b/database/migrations/2024_01_01_000011_add_channel_id_unique_to_platform_channels.php @@ -0,0 +1,57 @@ + getCommunityId). It is kept equal to `name` on create, + * but only `name` had a uniqueness guarantee. Enforce the same scope on + * channel_id so duplicate/ambiguous community references cannot exist. + */ + public function up(): void + { + $this->guardAgainstDuplicates(); + + Schema::table('platform_channels', function (Blueprint $table) { + $table->unique(['platform_instance_id', 'channel_id'], 'platform_channels_channel_id_unique'); + }); + } + + /** + * Pre-flight check so an existing database with drifted data fails with an + * actionable message naming the offending rows, rather than a raw driver error. + */ + private function guardAgainstDuplicates(): void + { + $duplicates = DB::table('platform_channels') + ->select('platform_instance_id', 'channel_id', DB::raw('COUNT(*) as total')) + ->groupBy('platform_instance_id', 'channel_id') + ->havingRaw('COUNT(*) > 1') + ->get(); + + if ($duplicates->isEmpty()) { + return; + } + + $details = $duplicates + ->map(fn ($row) => "platform_instance_id={$row->platform_instance_id} channel_id='{$row->channel_id}' ({$row->total} rows)") + ->implode('; '); + + throw new RuntimeException( + 'Cannot add unique index on platform_channels (platform_instance_id, channel_id): '. + 'duplicate rows exist. Resolve these before migrating: '.$details + ); + } + + public function down(): void + { + Schema::table('platform_channels', function (Blueprint $table) { + $table->dropUnique('platform_channels_channel_id_unique'); + }); + } +}; diff --git a/tests/Feature/Http/Controllers/Api/V1/PlatformChannelsControllerTest.php b/tests/Feature/Http/Controllers/Api/V1/PlatformChannelsControllerTest.php index 84f06cbe..9ad57d9e 100644 --- a/tests/Feature/Http/Controllers/Api/V1/PlatformChannelsControllerTest.php +++ b/tests/Feature/Http/Controllers/Api/V1/PlatformChannelsControllerTest.php @@ -111,6 +111,69 @@ public function test_store_validates_platform_instance_exists(): void ->assertJsonValidationErrors(['platform_instance_id']); } + public function test_store_rejects_non_slug_name(): void + { + $instance = PlatformInstance::factory()->create(); + + // name is copied verbatim into channel_id and used as the Lemmy community + // reference, so non-slug values must be rejected at the API boundary too. + $response = $this->postJson('/api/v1/platform-channels', [ + 'platform_instance_id' => $instance->id, + 'name' => 'Tech News', + ]); + + $response->assertStatus(422) + ->assertJsonValidationErrors(['name']); + + $this->assertDatabaseCount('platform_channels', 0); + } + + public function test_store_rejects_duplicate_name_for_same_instance(): void + { + $instance = PlatformInstance::factory()->create(); + PlatformChannel::factory()->create([ + 'platform_instance_id' => $instance->id, + 'name' => 'tech_news', + 'channel_id' => 'tech_news', + ]); + + $response = $this->postJson('/api/v1/platform-channels', [ + 'platform_instance_id' => $instance->id, + 'name' => 'tech_news', + ]); + + $response->assertStatus(422) + ->assertJsonValidationErrors(['name']); + + $this->assertDatabaseCount('platform_channels', 1); + } + + public function test_store_allows_same_name_on_different_instance(): void + { + $instanceA = PlatformInstance::factory()->create(); + $instanceB = PlatformInstance::factory()->create(); + + PlatformAccount::factory()->create([ + 'instance_url' => $instanceB->url, + 'is_active' => true, + ]); + + PlatformChannel::factory()->create([ + 'platform_instance_id' => $instanceA->id, + 'name' => 'tech_news', + 'channel_id' => 'tech_news', + ]); + + $response = $this->postJson('/api/v1/platform-channels', [ + 'platform_instance_id' => $instanceB->id, + 'name' => 'tech_news', + ]); + + $response->assertStatus(201); + + $this->assertDatabaseCount('platform_channels', 2); + } + public function test_show_returns_platform_channel_successfully(): void { $instance = PlatformInstance::factory()->create(); diff --git a/tests/Unit/Jobs/SyncChannelPostsJobTest.php b/tests/Unit/Jobs/SyncChannelPostsJobTest.php index d439bff1..db31c90a 100644 --- a/tests/Unit/Jobs/SyncChannelPostsJobTest.php +++ b/tests/Unit/Jobs/SyncChannelPostsJobTest.php @@ -7,6 +7,7 @@ use App\Models\PlatformAccount; use App\Models\PlatformChannel; use App\Models\PlatformInstance; +use App\Modules\Lemmy\Services\LemmyApiService; use App\Services\Log\LogSaver; use Exception; use Illuminate\Contracts\Queue\ShouldBeUnique; @@ -132,6 +133,110 @@ public function test_handle_logs_start_message(): void $this->assertTrue(true); } + public function test_sync_resolves_non_numeric_channel_id_via_get_community_id(): void + { + [$channel, $account] = $this->makeSyncableChannel('tech_news'); + + $apiMock = Mockery::mock(LemmyApiService::class); + $apiMock->shouldReceive('login') + ->once() + ->with($account->username, $account->password) + ->andReturn('token'); + $apiMock->shouldReceive('getCommunityId') + ->once() + ->with('tech_news', 'token') + ->andReturn(42); + $apiMock->shouldReceive('syncChannelPosts') + ->once() + ->with('token', 42, $channel->name); + + $logSaverMock = Mockery::mock(LogSaver::class); + $logSaverMock->shouldReceive('info')->zeroOrMoreTimes(); + $logSaverMock->shouldReceive('error')->zeroOrMoreTimes(); + + $this->makeJobWithApi($channel, $apiMock)->handle($logSaverMock); + + // The behaviour under test is the mocked call sequence above; assert explicitly + // so PHPUnit does not flag the test as risky for performing no assertions. + $this->addToAssertionCount(1); + } + + public function test_sync_uses_numeric_channel_id_directly_without_lookup(): void + { + [$channel, $account] = $this->makeSyncableChannel('42'); + + $apiMock = Mockery::mock(LemmyApiService::class); + $apiMock->shouldReceive('login') + ->once() + ->with($account->username, $account->password) + ->andReturn('token'); + // The behaviour that matters here is that no community lookup happens — the + // numeric channel_id is used as-is. (The int-ness of the argument is not worth + // asserting: syncChannelPosts() declares `int $platformChannelId`, so PHP coerces + // '42' at the call boundary whether or not the job casts it first.) + $apiMock->shouldNotReceive('getCommunityId'); + $apiMock->shouldReceive('syncChannelPosts') + ->once() + ->with('token', 42, $channel->name); + + $logSaverMock = Mockery::mock(LogSaver::class); + $logSaverMock->shouldReceive('info')->zeroOrMoreTimes(); + $logSaverMock->shouldReceive('error')->zeroOrMoreTimes(); + + $this->makeJobWithApi($channel, $apiMock)->handle($logSaverMock); + + // As above: the shouldNotReceive('getCommunityId') expectation is the assertion. + $this->addToAssertionCount(1); + } + + /** + * @return array{0: PlatformChannel, 1: PlatformAccount} + */ + private function makeSyncableChannel(string $channelId): array + { + $platformInstance = PlatformInstance::factory()->create([ + 'platform' => PlatformEnum::LEMMY, + 'url' => 'https://lemmy.example.com', + ]); + + $account = PlatformAccount::factory()->create([ + 'instance_url' => $platformInstance->url, + 'is_active' => true, + ]); + + $channel = PlatformChannel::factory()->create([ + 'platform_instance_id' => $platformInstance->id, + 'name' => 'tech_news', + 'channel_id' => $channelId, + 'is_active' => true, + ]); + + $channel->platformAccounts()->attach($account->id, [ + 'is_active' => true, + 'priority' => 1, + 'created_at' => now(), + 'updated_at' => now(), + ]); + + return [$channel->fresh(['platformInstance']), $account]; + } + + private function makeJobWithApi(PlatformChannel $channel, LemmyApiService $api): SyncChannelPostsJob + { + return new class($channel, $api) extends SyncChannelPostsJob + { + public function __construct(PlatformChannel $channel, private readonly LemmyApiService $api) + { + parent::__construct($channel); + } + + protected function makeApiService(string $instanceUrl): LemmyApiService + { + return $this->api; + } + }; + } + public function test_job_can_be_serialized(): void { $platformInstance = PlatformInstance::factory()->create(); diff --git a/tests/Unit/Models/PlatformChannelTest.php b/tests/Unit/Models/PlatformChannelTest.php index f6e81bab..6e6bfeb4 100644 --- a/tests/Unit/Models/PlatformChannelTest.php +++ b/tests/Unit/Models/PlatformChannelTest.php @@ -9,6 +9,7 @@ use App\Models\PlatformInstance; use App\Models\Route; use Carbon\Carbon; +use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Foundation\Testing\RefreshDatabase; use Tests\TestCase; @@ -45,6 +46,42 @@ public function test_casts_is_active_to_boolean(): void $this->assertFalse($channel->is_active); } + public function test_channel_id_is_unique_per_platform_instance(): void + { + $instance = PlatformInstance::factory()->create(); + PlatformChannel::factory()->create([ + 'platform_instance_id' => $instance->id, + 'name' => 'tech', + 'channel_id' => 'tech', + ]); + + $this->expectException(UniqueConstraintViolationException::class); + + PlatformChannel::factory()->create([ + 'platform_instance_id' => $instance->id, + 'name' => 'tech_alias', + 'channel_id' => 'tech', + ]); + } + + public function test_same_channel_id_allowed_across_different_instances(): void + { + $instanceA = PlatformInstance::factory()->create(); + $instanceB = PlatformInstance::factory()->create(); + + PlatformChannel::factory()->create([ + 'platform_instance_id' => $instanceA->id, + 'channel_id' => 'tech', + ]); + $second = PlatformChannel::factory()->create([ + 'platform_instance_id' => $instanceB->id, + 'channel_id' => 'tech', + ]); + + $this->assertDatabaseCount('platform_channels', 2); + $this->assertEquals('tech', $second->channel_id); + } + public function test_belongs_to_platform_instance_relationship(): void { $instance = PlatformInstance::factory()->create(); -- 2.45.2 From 982428faa78b8e8084ecd1bceb149162d2f9c3bb Mon Sep 17 00:00:00 2001 From: myrmidex Date: Wed, 29 Jul 2026 20:27:40 +0200 Subject: [PATCH 4/4] 106 - Extract shared community id resolution into LemmyApiService --- app/Jobs/SyncChannelPostsJob.php | 7 +---- .../Lemmy/Services/LemmyApiService.php | 15 +++++++++ app/Modules/Lemmy/Services/LemmyPublisher.php | 4 +-- phpstan-baseline.neon | 6 ---- .../Api/V1/OnboardingControllerTest.php | 8 +++-- tests/Unit/Jobs/SyncChannelPostsJobTest.php | 19 +++++++----- .../Lemmy/Services/LemmyApiServiceTest.php | 31 +++++++++++++++++++ .../Lemmy/Services/LemmyPublisherTest.php | 22 +++++++++++-- 8 files changed, 84 insertions(+), 28 deletions(-) diff --git a/app/Jobs/SyncChannelPostsJob.php b/app/Jobs/SyncChannelPostsJob.php index edf86824..d598733f 100644 --- a/app/Jobs/SyncChannelPostsJob.php +++ b/app/Jobs/SyncChannelPostsJob.php @@ -68,12 +68,7 @@ private function syncLemmyChannelPosts(LogSaver $logSaver): void $api = $this->makeApiService($this->channel->platformInstance->url); $token = $this->getAuthToken($api, $account); - // channel_id holds a Lemmy community slug (non-numeric) or a numeric - // community id; syncChannelPosts() needs the numeric id. Mirror the - // resolution used in LemmyPublisher::createPost(). - $communityId = is_numeric($this->channel->channel_id) - ? (int) $this->channel->channel_id - : $api->getCommunityId($this->channel->channel_id, $token); + $communityId = $api->resolveCommunityId($this->channel->channel_id, $token); $api->syncChannelPosts($token, $communityId, $this->channel->name); diff --git a/app/Modules/Lemmy/Services/LemmyApiService.php b/app/Modules/Lemmy/Services/LemmyApiService.php index 448fa57e..0ab58d21 100644 --- a/app/Modules/Lemmy/Services/LemmyApiService.php +++ b/app/Modules/Lemmy/Services/LemmyApiService.php @@ -83,6 +83,21 @@ public function login(string $username, string $password): ?string return null; } + /** + * Resolve a PlatformChannel.channel_id to a numeric Lemmy community id. + * + * channel_id holds either a community slug (the usual case — CreateChannelAction + * copies `name` into it) or an already-numeric community id. Callers that need the + * numeric id should use this rather than reimplementing the check, so the two forms + * stay handled identically everywhere. + */ + public function resolveCommunityId(string $channelId, string $token): int + { + return is_numeric($channelId) + ? (int) $channelId + : $this->getCommunityId($channelId, $token); + } + public function getCommunityId(string $communityName, string $token): int { try { diff --git a/app/Modules/Lemmy/Services/LemmyPublisher.php b/app/Modules/Lemmy/Services/LemmyPublisher.php index 3d398eba..be7855a0 100644 --- a/app/Modules/Lemmy/Services/LemmyPublisher.php +++ b/app/Modules/Lemmy/Services/LemmyPublisher.php @@ -54,9 +54,7 @@ private function createPost(string $token, array $extractedData, PlatformChannel { $languageId = $extractedData['language_id'] ?? null; - $communityId = is_numeric($channel->channel_id) - ? (int) $channel->channel_id - : $this->api->getCommunityId($channel->channel_id, $token); + $communityId = $this->api->resolveCommunityId($channel->channel_id, $token); return $this->api->createPost( $token, diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 15c47150..8f8dd6ae 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -102,12 +102,6 @@ parameters: count: 6 path: tests/Unit/Models/PlatformChannelTest.php - - - message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertIsString\(\) with int will always evaluate to false\.$#' - identifier: method.impossibleType - count: 1 - path: tests/Unit/Models/PlatformChannelTest.php - - message: '#^Access to an undefined property App\\Models\\Language\:\:\$pivot\.$#' identifier: property.notFound diff --git a/tests/Feature/Http/Controllers/Api/V1/OnboardingControllerTest.php b/tests/Feature/Http/Controllers/Api/V1/OnboardingControllerTest.php index 9ee425de..df6356db 100644 --- a/tests/Feature/Http/Controllers/Api/V1/OnboardingControllerTest.php +++ b/tests/Feature/Http/Controllers/Api/V1/OnboardingControllerTest.php @@ -96,7 +96,10 @@ public function test_status_shows_route_step_when_platform_account_feed_and_chan $language = Language::first(); PlatformAccount::factory()->create(['is_active' => true]); Feed::factory()->language($language)->create(['is_active' => true]); - PlatformChannel::factory()->create(['is_active' => true]); + // Reuse the seeded language: PlatformChannelFactory otherwise creates one via + // faker->unique()->languageCode(), which can randomly draw 'en' and collide with + // the short_code created in setUp(). + PlatformChannel::factory()->create(['is_active' => true, 'language_id' => $language->id]); $response = $this->getJson('/api/v1/onboarding/status'); @@ -119,7 +122,8 @@ public function test_status_shows_no_onboarding_needed_when_all_components_exist $language = Language::first(); PlatformAccount::factory()->create(['is_active' => true]); Feed::factory()->language($language)->create(['is_active' => true]); - PlatformChannel::factory()->create(['is_active' => true]); + // Reuse the seeded language — see note above re: languageCode() collisions. + PlatformChannel::factory()->create(['is_active' => true, 'language_id' => $language->id]); Route::factory()->create(['is_active' => true]); $response = $this->getJson('/api/v1/onboarding/status'); diff --git a/tests/Unit/Jobs/SyncChannelPostsJobTest.php b/tests/Unit/Jobs/SyncChannelPostsJobTest.php index db31c90a..62fcc10e 100644 --- a/tests/Unit/Jobs/SyncChannelPostsJobTest.php +++ b/tests/Unit/Jobs/SyncChannelPostsJobTest.php @@ -142,7 +142,7 @@ public function test_sync_resolves_non_numeric_channel_id_via_get_community_id() ->once() ->with($account->username, $account->password) ->andReturn('token'); - $apiMock->shouldReceive('getCommunityId') + $apiMock->shouldReceive('resolveCommunityId') ->once() ->with('tech_news', 'token') ->andReturn(42); @@ -161,7 +161,7 @@ public function test_sync_resolves_non_numeric_channel_id_via_get_community_id() $this->addToAssertionCount(1); } - public function test_sync_uses_numeric_channel_id_directly_without_lookup(): void + public function test_sync_passes_resolved_community_id_to_sync_channel_posts(): void { [$channel, $account] = $this->makeSyncableChannel('42'); @@ -170,11 +170,13 @@ public function test_sync_uses_numeric_channel_id_directly_without_lookup(): voi ->once() ->with($account->username, $account->password) ->andReturn('token'); - // The behaviour that matters here is that no community lookup happens — the - // numeric channel_id is used as-is. (The int-ness of the argument is not worth - // asserting: syncChannelPosts() declares `int $platformChannelId`, so PHP coerces - // '42' at the call boundary whether or not the job casts it first.) - $apiMock->shouldNotReceive('getCommunityId'); + // The slug-vs-numeric branch itself now lives in LemmyApiService::resolveCommunityId + // and is covered by LemmyApiServiceTest; here we only assert the job forwards + // whatever that resolution returns. + $apiMock->shouldReceive('resolveCommunityId') + ->once() + ->with('42', 'token') + ->andReturn(42); $apiMock->shouldReceive('syncChannelPosts') ->once() ->with('token', 42, $channel->name); @@ -185,7 +187,8 @@ public function test_sync_uses_numeric_channel_id_directly_without_lookup(): voi $this->makeJobWithApi($channel, $apiMock)->handle($logSaverMock); - // As above: the shouldNotReceive('getCommunityId') expectation is the assertion. + // The mocked call sequence above is the assertion; assert explicitly so PHPUnit + // does not flag the test as risky. $this->addToAssertionCount(1); } diff --git a/tests/Unit/Modules/Lemmy/Services/LemmyApiServiceTest.php b/tests/Unit/Modules/Lemmy/Services/LemmyApiServiceTest.php index 2d8caf85..9d50ead0 100644 --- a/tests/Unit/Modules/Lemmy/Services/LemmyApiServiceTest.php +++ b/tests/Unit/Modules/Lemmy/Services/LemmyApiServiceTest.php @@ -163,6 +163,37 @@ public function test_get_community_id_success(): void }); } + public function test_resolve_community_id_looks_up_non_numeric_channel_id(): void + { + Http::fake([ + '*' => Http::response([ + 'community_view' => [ + 'community' => ['id' => 123], + ], + ], 200), + ]); + + $service = new LemmyApiService('lemmy.world'); + $id = $service->resolveCommunityId('test-community', 'token'); + + $this->assertSame(123, $id); + + Http::assertSent(fn ($request) => str_contains($request->url(), 'name=test-community')); + } + + public function test_resolve_community_id_uses_numeric_channel_id_without_lookup(): void + { + Http::fake(); + + $service = new LemmyApiService('lemmy.world'); + $id = $service->resolveCommunityId('42', 'token'); + + $this->assertSame(42, $id); + + // A numeric channel_id is already the community id — no lookup should happen. + Http::assertNothingSent(); + } + public function test_get_community_id_throws_on_unsuccessful_response(): void { Http::fake([ diff --git a/tests/Unit/Modules/Lemmy/Services/LemmyPublisherTest.php b/tests/Unit/Modules/Lemmy/Services/LemmyPublisherTest.php index 6e536767..e8459663 100644 --- a/tests/Unit/Modules/Lemmy/Services/LemmyPublisherTest.php +++ b/tests/Unit/Modules/Lemmy/Services/LemmyPublisherTest.php @@ -76,6 +76,10 @@ public function test_publish_to_channel_with_all_data(): void // Mock LemmyApiService $apiMock = Mockery::mock(LemmyApiService::class); + $apiMock->shouldReceive('resolveCommunityId') + ->once() + ->with('42', 'test-token') + ->andReturn(42); $apiMock->shouldReceive('createPost') ->once() ->with( @@ -129,6 +133,10 @@ public function test_publish_to_channel_with_minimal_data(): void // Mock LemmyApiService $apiMock = Mockery::mock(LemmyApiService::class); + $apiMock->shouldReceive('resolveCommunityId') + ->once() + ->with('24', 'minimal-token') + ->andReturn(24); $apiMock->shouldReceive('createPost') ->once() ->with( @@ -185,6 +193,10 @@ public function test_publish_to_channel_without_thumbnail(): void // Mock LemmyApiService $apiMock = Mockery::mock(LemmyApiService::class); + $apiMock->shouldReceive('resolveCommunityId') + ->once() + ->with('33', 'no-thumb-token') + ->andReturn(33); $apiMock->shouldReceive('createPost') ->once() ->with( @@ -266,6 +278,10 @@ public function test_publish_to_channel_throws_api_exception(): void // Mock LemmyApiService to throw exception $apiMock = Mockery::mock(LemmyApiService::class); + $apiMock->shouldReceive('resolveCommunityId') + ->once() + ->with('42', 'test-token') + ->andReturn(42); $apiMock->shouldReceive('createPost') ->once() ->andThrow(new Exception('API Error')); @@ -284,7 +300,7 @@ public function test_publish_to_channel_throws_api_exception(): void $publisher->publishToChannel($article, $extractedData, $channel); } - public function test_publish_to_channel_handles_string_channel_id(): void + public function test_publish_to_channel_forwards_resolved_community_id_to_create_post(): void { $account = PlatformAccount::factory()->make([ 'instance_url' => 'https://lemmy.world', @@ -309,9 +325,9 @@ public function test_publish_to_channel_handles_string_channel_id(): void ->once() ->andReturn('token'); - // Mock LemmyApiService - should call getCommunityId for non-numeric channel_id + // Mock LemmyApiService - should resolve non-numeric channel_id to a community id $apiMock = Mockery::mock(LemmyApiService::class); - $apiMock->shouldReceive('getCommunityId') + $apiMock->shouldReceive('resolveCommunityId') ->once() ->with('string-42', 'token') ->andReturn(42); -- 2.45.2