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/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);