diff --git a/app/Actions/PublishRouteArticleAction.php b/app/Actions/PublishRouteArticleAction.php new file mode 100644 index 00000000..b6a2f204 --- /dev/null +++ b/app/Actions/PublishRouteArticleAction.php @@ -0,0 +1,107 @@ +article; + + $routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHING]); + + try { + $extractedData = $this->articleFetcher->fetchArticleData($article); + $outcome = $this->publishingService->publishRouteArticle($routeArticle, $extractedData); + } catch (Exception $e) { + $routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]); + + ActionPerformed::dispatch('Failed to publish article', LogLevelEnum::ERROR, [ + 'article_id' => $article->id, + 'error' => $e->getMessage(), + ]); + + $this->notificationService->send( + NotificationTypeEnum::PUBLISH_FAILED, + NotificationSeverityEnum::ERROR, + "Publish failed: {$article->title}", + $e->getMessage(), + $article, + ); + + throw $e; + } + + match (true) { + $outcome->succeeded() => $this->recordPublished($routeArticle), + $outcome->wasSkipped() => $this->recordSkipped($routeArticle, $outcome), + default => $this->recordFailed($routeArticle, $outcome), + }; + + return $outcome; + } + + private function recordPublished(RouteArticle $routeArticle): void + { + $routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHED]); + + ActionPerformed::dispatch('Published article', LogLevelEnum::INFO, [ + 'article_id' => $routeArticle->article->id, + 'title' => $routeArticle->article->title, + ]); + } + + private function recordSkipped(RouteArticle $routeArticle, PublishOutcome $outcome): void + { + $routeArticle->update(['publish_status' => PublishStatusEnum::SKIPPED]); + + ActionPerformed::dispatch('Skipped publishing article', LogLevelEnum::INFO, [ + 'article_id' => $routeArticle->article->id, + 'title' => $routeArticle->article->title, + 'reason' => $outcome->reason, + ]); + } + + private function recordFailed(RouteArticle $routeArticle, PublishOutcome $outcome): void + { + $article = $routeArticle->article; + + $routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]); + + ActionPerformed::dispatch('No publication created for article', LogLevelEnum::WARNING, [ + 'article_id' => $article->id, + 'title' => $article->title, + 'reason' => $outcome->reason, + ]); + + $this->notificationService->send( + NotificationTypeEnum::PUBLISH_FAILED, + NotificationSeverityEnum::WARNING, + "Publish failed: {$article->title}", + $outcome->reason ?? 'No publication was created for this article.', + $article, + ); + } +} diff --git a/app/Enums/PublishStatusEnum.php b/app/Enums/PublishStatusEnum.php index 03260e30..c4e49b2a 100644 --- a/app/Enums/PublishStatusEnum.php +++ b/app/Enums/PublishStatusEnum.php @@ -7,5 +7,6 @@ enum PublishStatusEnum: string case UNPUBLISHED = 'unpublished'; case PUBLISHING = 'publishing'; case PUBLISHED = 'published'; + case SKIPPED = 'skipped'; case ERROR = 'error'; } diff --git a/app/Jobs/PublishNextArticleJob.php b/app/Jobs/PublishNextArticleJob.php index fd9a8118..e4ce99c7 100644 --- a/app/Jobs/PublishNextArticleJob.php +++ b/app/Jobs/PublishNextArticleJob.php @@ -2,19 +2,14 @@ namespace App\Jobs; +use App\Actions\PublishRouteArticleAction; use App\Enums\ApprovalStatusEnum; use App\Enums\LogLevelEnum; -use App\Enums\NotificationSeverityEnum; -use App\Enums\NotificationTypeEnum; -use App\Enums\PublishStatusEnum; use App\Events\ActionPerformed; use App\Exceptions\PublishException; use App\Models\ArticlePublication; use App\Models\RouteArticle; use App\Models\Setting; -use App\Services\Article\ArticleFetcher; -use App\Services\Notification\NotificationService; -use App\Services\Publishing\ArticlePublishingService; use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; @@ -38,7 +33,7 @@ public function __construct() * * @throws PublishException */ - public function handle(ArticleFetcher $articleFetcher, ArticlePublishingService $publishingService, NotificationService $notificationService): void + public function handle(PublishRouteArticleAction $publishRouteArticle): void { $interval = Setting::getArticlePublishingInterval(); @@ -72,52 +67,6 @@ public function handle(ArticleFetcher $articleFetcher, ArticlePublishingService 'route' => $routeArticle->feed_id.'-'.$routeArticle->platform_channel_id, ]); - $routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHING]); - - try { - $extractedData = $articleFetcher->fetchArticleData($article); - $publication = $publishingService->publishRouteArticle($routeArticle, $extractedData); - - if ($publication) { - $routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHED]); - - ActionPerformed::dispatch('Successfully published article', LogLevelEnum::INFO, [ - 'article_id' => $article->id, - 'title' => $article->title, - ]); - } else { - $routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]); - - ActionPerformed::dispatch('No publication created for article', LogLevelEnum::WARNING, [ - 'article_id' => $article->id, - 'title' => $article->title, - ]); - - $notificationService->send( - NotificationTypeEnum::PUBLISH_FAILED, - NotificationSeverityEnum::WARNING, - "Publish failed: {$article->title}", - 'No publication was created for this article. Check channel routing configuration.', - $article, - ); - } - } catch (PublishException $e) { - $routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]); - - ActionPerformed::dispatch('Failed to publish article', LogLevelEnum::ERROR, [ - 'article_id' => $article->id, - 'error' => $e->getMessage(), - ]); - - $notificationService->send( - NotificationTypeEnum::PUBLISH_FAILED, - NotificationSeverityEnum::ERROR, - "Publish failed: {$article->title}", - $e->getMessage(), - $article, - ); - - throw $e; - } + $publishRouteArticle->execute($routeArticle); } } diff --git a/app/Jobs/SyncChannelPostsJob.php b/app/Jobs/SyncChannelPostsJob.php index d598733f..96043a2e 100644 --- a/app/Jobs/SyncChannelPostsJob.php +++ b/app/Jobs/SyncChannelPostsJob.php @@ -70,7 +70,7 @@ private function syncLemmyChannelPosts(LogSaver $logSaver): void $communityId = $api->resolveCommunityId($this->channel->channel_id, $token); - $api->syncChannelPosts($token, $communityId, $this->channel->name); + $api->syncChannelPosts($token, $this->channel, $communityId); $logSaver->info('Channel posts synced successfully', $this->channel); } catch (Exception $e) { diff --git a/app/Listeners/PublishApprovedArticleListener.php b/app/Listeners/PublishApprovedArticleListener.php index b08e3b7e..ae9524e7 100644 --- a/app/Listeners/PublishApprovedArticleListener.php +++ b/app/Listeners/PublishApprovedArticleListener.php @@ -2,15 +2,8 @@ namespace App\Listeners; -use App\Enums\LogLevelEnum; -use App\Enums\NotificationSeverityEnum; -use App\Enums\NotificationTypeEnum; -use App\Enums\PublishStatusEnum; -use App\Events\ActionPerformed; +use App\Actions\PublishRouteArticleAction; use App\Events\RouteArticleApproved; -use App\Services\Article\ArticleFetcher; -use App\Services\Notification\NotificationService; -use App\Services\Publishing\ArticlePublishingService; use Exception; use Illuminate\Contracts\Queue\ShouldQueue; @@ -19,9 +12,7 @@ class PublishApprovedArticleListener implements ShouldQueue public string $queue = 'publishing'; public function __construct( - private ArticleFetcher $articleFetcher, - private ArticlePublishingService $publishingService, - private NotificationService $notificationService, + private PublishRouteArticleAction $publishRouteArticle, ) {} public function handle(RouteArticleApproved $event): void @@ -29,7 +20,6 @@ public function handle(RouteArticleApproved $event): void $routeArticle = $event->routeArticle; $article = $routeArticle->article; - // Skip if already published to this channel if ($article->articlePublications() ->where('platform_channel_id', $routeArticle->platform_channel_id) ->exists() @@ -37,50 +27,10 @@ public function handle(RouteArticleApproved $event): void return; } - $routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHING]); - try { - $extractedData = $this->articleFetcher->fetchArticleData($article); - $publication = $this->publishingService->publishRouteArticle($routeArticle, $extractedData); - - if ($publication) { - $routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHED]); - - ActionPerformed::dispatch('Published approved article', LogLevelEnum::INFO, [ - 'article_id' => $article->id, - 'title' => $article->title, - ]); - } else { - $routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]); - - ActionPerformed::dispatch('No publication created for approved article', LogLevelEnum::WARNING, [ - 'article_id' => $article->id, - 'title' => $article->title, - ]); - - $this->notificationService->send( - NotificationTypeEnum::PUBLISH_FAILED, - NotificationSeverityEnum::WARNING, - "Publish failed: {$article->title}", - 'No publication was created for this article. Check channel routing configuration.', - $article, - ); - } - } catch (Exception $e) { - $routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]); - - ActionPerformed::dispatch('Failed to publish approved article', LogLevelEnum::ERROR, [ - 'article_id' => $article->id, - 'error' => $e->getMessage(), - ]); - - $this->notificationService->send( - NotificationTypeEnum::PUBLISH_FAILED, - NotificationSeverityEnum::ERROR, - "Publish failed: {$article->title}", - $e->getMessage(), - $article, - ); + $this->publishRouteArticle->execute($routeArticle); + } catch (Exception) { + // The action has already recorded the failure and notified. } } } diff --git a/app/Models/PlatformChannelPost.php b/app/Models/PlatformChannelPost.php index a411b241..9b94da42 100644 --- a/app/Models/PlatformChannelPost.php +++ b/app/Models/PlatformChannelPost.php @@ -2,13 +2,12 @@ namespace App\Models; -use App\Enums\PlatformEnum; use Illuminate\Database\Eloquent\Factories\Factory; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Relations\BelongsTo; /** - * @method static where(string $string, PlatformEnum $platform) * @method static updateOrCreate(array $array, array $array1) */ class PlatformChannelPost extends Model @@ -17,9 +16,7 @@ class PlatformChannelPost extends Model use HasFactory; protected $fillable = [ - 'platform', - 'channel_id', - 'channel_name', + 'platform_channel_id', 'post_id', 'url', 'title', @@ -33,26 +30,24 @@ protected function casts(): array { return [ 'posted_at' => 'datetime', - 'platform' => PlatformEnum::class, ]; } - public static function urlExists(PlatformEnum $platform, string $channelId, string $url): bool + /** + * @return BelongsTo + */ + public function platformChannel(): BelongsTo { - return self::where('platform', $platform) - ->where('channel_id', $channelId) - ->where('url', $url) - ->exists(); + return $this->belongsTo(PlatformChannel::class); } - public static function duplicateExists(PlatformEnum $platform, string $channelId, ?string $url, ?string $title): bool + public static function duplicateExists(PlatformChannel $channel, ?string $url, ?string $title): bool { if (! $url && ! $title) { return false; } - return self::where('platform', $platform) - ->where('channel_id', $channelId) + return self::where('platform_channel_id', $channel->id) ->where(function ($query) use ($url, $title) { if ($url) { $query->orWhere('url', $url); @@ -64,16 +59,14 @@ public static function duplicateExists(PlatformEnum $platform, string $channelId ->exists(); } - public static function storePost(PlatformEnum $platform, string $channelId, ?string $channelName, string $postId, ?string $url, ?string $title, ?\DateTime $postedAt = null): self + public static function storePost(PlatformChannel $channel, string $postId, ?string $url, ?string $title, ?\DateTime $postedAt = null): self { return self::updateOrCreate( [ - 'platform' => $platform, - 'channel_id' => $channelId, + 'platform_channel_id' => $channel->id, 'post_id' => $postId, ], [ - 'channel_name' => $channelName, 'url' => $url, 'title' => $title, 'posted_at' => $postedAt ?? now(), diff --git a/app/Modules/Lemmy/Services/LemmyApiService.php b/app/Modules/Lemmy/Services/LemmyApiService.php index 0ab58d21..c3d23c5c 100644 --- a/app/Modules/Lemmy/Services/LemmyApiService.php +++ b/app/Modules/Lemmy/Services/LemmyApiService.php @@ -2,7 +2,7 @@ namespace App\Modules\Lemmy\Services; -use App\Enums\PlatformEnum; +use App\Models\PlatformChannel; use App\Models\PlatformChannelPost; use App\Modules\Lemmy\LemmyRequest; use Exception; @@ -117,12 +117,12 @@ public function getCommunityId(string $communityName, string $token): int } } - public function syncChannelPosts(string $token, int $platformChannelId, string $communityName): void + public function syncChannelPosts(string $token, PlatformChannel $channel, int $communityId): void { try { $request = new LemmyRequest($this->instance, $token); $response = $request->get('post/list', [ - 'community_id' => $platformChannelId, + 'community_id' => $communityId, 'limit' => 50, 'sort' => 'New', ]); @@ -130,7 +130,7 @@ public function syncChannelPosts(string $token, int $platformChannelId, string $ if (! $response->successful()) { logger()->warning('Failed to sync channel posts', [ 'status' => $response->status(), - 'platform_channel_id' => $platformChannelId, + 'platform_channel_id' => $channel->id, ]); return; @@ -143,9 +143,7 @@ public function syncChannelPosts(string $token, int $platformChannelId, string $ $post = $postData['post']; PlatformChannelPost::storePost( - PlatformEnum::LEMMY, - (string) $platformChannelId, - $communityName, + $channel, (string) $post['id'], $post['url'] ?? null, $post['name'] ?? null, @@ -154,14 +152,14 @@ public function syncChannelPosts(string $token, int $platformChannelId, string $ } logger()->info('Synced channel posts', [ - 'platform_channel_id' => $platformChannelId, + 'platform_channel_id' => $channel->id, 'posts_count' => count($posts), ]); } catch (Exception $e) { logger()->error('Exception while syncing channel posts', [ 'error' => $e->getMessage(), - 'platform_channel_id' => $platformChannelId, + 'platform_channel_id' => $channel->id, ]); } } diff --git a/app/Services/Publishing/ArticlePublishingService.php b/app/Services/Publishing/ArticlePublishingService.php index 4d427fca..622e5cb7 100644 --- a/app/Services/Publishing/ArticlePublishingService.php +++ b/app/Services/Publishing/ArticlePublishingService.php @@ -39,7 +39,7 @@ protected function makePublisher(mixed $account): LemmyPublisher * * @throws PublishException */ - public function publishRouteArticle(RouteArticle $routeArticle, array $extractedData): ?ArticlePublication + public function publishRouteArticle(RouteArticle $routeArticle, array $extractedData): PublishOutcome { $article = $routeArticle->article; $channel = $routeArticle->platformChannel; @@ -60,7 +60,7 @@ public function publishRouteArticle(RouteArticle $routeArticle, array $extracted 'route_article_id' => $routeArticle->id, ]); - return null; + return PublishOutcome::failure('No active account for channel'); } return $this->publishToChannel($article, $extractedData, $channel, $account); @@ -69,7 +69,7 @@ public function publishRouteArticle(RouteArticle $routeArticle, array $extracted /** * @param array $extractedData */ - private function publishToChannel(Article $article, array $extractedData, PlatformChannel $channel, mixed $account): ?ArticlePublication + private function publishToChannel(Article $article, array $extractedData, PlatformChannel $channel, mixed $account): PublishOutcome { $lock = Cache::lock("publish:{$article->id}:{$channel->id}", self::LOCK_TTL_SECONDS); @@ -84,7 +84,7 @@ private function publishToChannel(Article $article, array $extractedData, Platfo 'article_id' => $article->id, ]); - return null; + return PublishOutcome::skipped('Already published to this channel'); } return $this->doPublishToChannel($article, $extractedData, $channel, $account); @@ -94,31 +94,26 @@ private function publishToChannel(Article $article, array $extractedData, Platfo 'article_id' => $article->id, ]); - return null; + return PublishOutcome::skipped('Another worker is publishing this article'); } } /** * @param array $extractedData */ - private function doPublishToChannel(Article $article, array $extractedData, PlatformChannel $channel, mixed $account): ?ArticlePublication + private function doPublishToChannel(Article $article, array $extractedData, PlatformChannel $channel, mixed $account): PublishOutcome { try { // Check if this URL or title was already posted to this channel $title = $extractedData['title'] ?? $article->title; - if (PlatformChannelPost::duplicateExists( - $channel->platformInstance->platform, - (string) $channel->channel_id, - $article->url, - $title - )) { + if (PlatformChannelPost::duplicateExists($channel, $article->url, $title)) { $this->logSaver->info('Skipping duplicate: URL or title already posted to channel', $channel, [ 'article_id' => $article->id, 'url' => $article->url, 'title' => $title, ]); - return null; + return PublishOutcome::skipped('URL or title already posted to this channel'); } $publisher = $this->makePublisher($account); @@ -138,14 +133,14 @@ private function doPublishToChannel(Article $article, array $extractedData, Plat 'article_id' => $article->id, ]); - return $publication; + return PublishOutcome::published($publication); } catch (Exception $e) { $this->logSaver->warning('Failed to publish to channel', $channel, [ 'article_id' => $article->id, 'error' => $e->getMessage(), ]); - return null; + return PublishOutcome::failure($e->getMessage()); } } } diff --git a/app/Services/Publishing/PublishOutcome.php b/app/Services/Publishing/PublishOutcome.php new file mode 100644 index 00000000..4584908e --- /dev/null +++ b/app/Services/Publishing/PublishOutcome.php @@ -0,0 +1,49 @@ +publication !== null; + } + + public function wasSkipped(): bool + { + return $this->skipped; + } + + public function failed(): bool + { + return ! $this->succeeded() && ! $this->skipped; + } +} diff --git a/database/migrations/2024_01_01_000013_key_platform_channel_posts_by_local_channel.php b/database/migrations/2024_01_01_000013_key_platform_channel_posts_by_local_channel.php new file mode 100644 index 00000000..7adc6548 --- /dev/null +++ b/database/migrations/2024_01_01_000013_key_platform_channel_posts_by_local_channel.php @@ -0,0 +1,83 @@ +dropUnique('channel_post_unique'); + $table->dropIndex(['platform', 'channel_id', 'url']); + $table->dropIndex(['platform', 'channel_id', 'title']); + $table->unsignedBigInteger('platform_channel_id')->nullable()->after('id'); + }); + + // A name shared by two instances is ambiguous; those rows stay unmapped. + DB::table('platform_channel_posts')->orderBy('id')->chunkById(200, function ($rows) { + foreach ($rows as $row) { + $matches = DB::table('platform_channels') + ->where('name', $row->channel_name) + ->orWhere('channel_id', $row->channel_name) + ->pluck('id'); + + if ($matches->count() !== 1) { + continue; + } + + DB::table('platform_channel_posts') + ->where('id', $row->id) + ->update(['platform_channel_id' => $matches->first()]); + } + }); + + // Unmappable rows are discarded rather than guessed: the mirror is a + // cache SyncChannelPostsJob rebuilds every ten minutes. + DB::table('platform_channel_posts')->whereNull('platform_channel_id')->delete(); + + Schema::table('platform_channel_posts', function (Blueprint $table) { + $table->unsignedBigInteger('platform_channel_id')->nullable(false)->change(); + $table->dropColumn(['platform', 'channel_id', 'channel_name']); + }); + + Schema::table('platform_channel_posts', function (Blueprint $table) { + $table->foreign('platform_channel_id')->references('id')->on('platform_channels')->onDelete('cascade'); + $table->unique(['platform_channel_id', 'post_id'], 'channel_post_unique'); + $table->index(['platform_channel_id', 'url']); + $table->index(['platform_channel_id', 'title']); + }); + } + + public function down(): void + { + Schema::table('platform_channel_posts', function (Blueprint $table) { + $table->dropForeign(['platform_channel_id']); + $table->dropUnique('channel_post_unique'); + $table->dropIndex(['platform_channel_id', 'url']); + $table->dropIndex(['platform_channel_id', 'title']); + $table->string('platform')->default('lemmy'); + $table->string('channel_id')->default(''); + $table->string('channel_name')->nullable(); + }); + + DB::table('platform_channel_posts')->update([ + 'channel_id' => DB::raw('platform_channel_id'), + ]); + + Schema::table('platform_channel_posts', function (Blueprint $table) { + $table->dropColumn('platform_channel_id'); + $table->unique(['platform', 'channel_id', 'post_id'], 'channel_post_unique'); + $table->index(['platform', 'channel_id', 'url']); + $table->index(['platform', 'channel_id', 'title']); + }); + } +}; diff --git a/database/migrations/2024_01_01_000014_add_skipped_to_route_articles_publish_status.php b/database/migrations/2024_01_01_000014_add_skipped_to_route_articles_publish_status.php new file mode 100644 index 00000000..ae4bd839 --- /dev/null +++ b/database/migrations/2024_01_01_000014_add_skipped_to_route_articles_publish_status.php @@ -0,0 +1,35 @@ +enum('publish_status', ['unpublished', 'publishing', 'published', 'skipped', 'error']) + ->default('unpublished') + ->change(); + }); + } + + public function down(): void + { + DB::table('route_articles') + ->where('publish_status', 'skipped') + ->update(['publish_status' => 'unpublished']); + + Schema::table('route_articles', function (Blueprint $table) { + $table->enum('publish_status', ['unpublished', 'publishing', 'published', 'error']) + ->default('unpublished') + ->change(); + }); + } +}; diff --git a/tests/Feature/DuplicatePublishTest.php b/tests/Feature/DuplicatePublishTest.php index a1d37646..892a3b69 100644 --- a/tests/Feature/DuplicatePublishTest.php +++ b/tests/Feature/DuplicatePublishTest.php @@ -2,6 +2,7 @@ namespace Tests\Feature; +use App\Actions\PublishRouteArticleAction; use App\Enums\PublishStatusEnum; use App\Events\RouteArticleApproved; use App\Listeners\PublishApprovedArticleListener; @@ -95,7 +96,7 @@ private function makeListener(): PublishApprovedArticleListener $fetcher = Mockery::mock(ArticleFetcher::class); $fetcher->shouldReceive('fetchArticleData')->andReturn(['title' => 'Test Article']); - return new PublishApprovedArticleListener($fetcher, $service, new NotificationService); + return new PublishApprovedArticleListener(new PublishRouteArticleAction($fetcher, $service, new NotificationService)); } public function test_clicking_approve_twice_creates_only_one_remote_post(): void @@ -142,7 +143,7 @@ public function test_two_queued_listeners_create_only_one_remote_post(): void $fetcher = Mockery::mock(ArticleFetcher::class); $fetcher->shouldReceive('fetchArticleData')->andReturn(['title' => 'Test Article']); - $listener = new PublishApprovedArticleListener($fetcher, $service, new NotificationService); + $listener = new PublishApprovedArticleListener(new PublishRouteArticleAction($fetcher, $service, new NotificationService)); $listener->handle(new RouteArticleApproved($routeArticle)); $this->assertSame(1, $this->remoteCalls, 'Two listeners must not both post to Lemmy.'); diff --git a/tests/Feature/KeyPlatformChannelPostsMigrationTest.php b/tests/Feature/KeyPlatformChannelPostsMigrationTest.php new file mode 100644 index 00000000..1febf897 --- /dev/null +++ b/tests/Feature/KeyPlatformChannelPostsMigrationTest.php @@ -0,0 +1,130 @@ +up(); + } + + private function restoreLegacyTable(): void + { + Schema::dropIfExists('platform_channel_posts'); + + Schema::create('platform_channel_posts', function (Blueprint $table) { + $table->id(); + $table->string('platform'); + $table->string('channel_id'); + $table->string('channel_name')->nullable(); + $table->string('post_id'); + $table->string('title')->nullable(); + $table->string('url')->nullable(); + $table->timestamp('posted_at')->nullable(); + $table->timestamps(); + + $table->unique(['platform', 'channel_id', 'post_id'], 'channel_post_unique'); + $table->index(['platform', 'channel_id', 'url']); + $table->index(['platform', 'channel_id', 'title']); + }); + } + + private function seedLegacyRow(string $channelId, ?string $channelName, string $postId): void + { + DB::table('platform_channel_posts')->insert([ + 'platform' => 'lemmy', + 'channel_id' => $channelId, + 'channel_name' => $channelName, + 'post_id' => $postId, + 'url' => "https://news.test/{$postId}", + 'title' => "Post {$postId}", + 'posted_at' => now(), + 'created_at' => now(), + 'updated_at' => now(), + ]); + } + + public function test_maps_rows_to_the_local_channel_by_name(): void + { + $this->restoreLegacyTable(); + + $channel = PlatformChannel::factory()->create(['name' => 'newsbottest', 'channel_id' => 'newsbottest']); + $this->seedLegacyRow('217', 'newsbottest', '1'); + $this->seedLegacyRow('217', 'newsbottest', '2'); + + $this->runMigration(); + + $this->assertSame(2, DB::table('platform_channel_posts') + ->where('platform_channel_id', $channel->id)->count()); + } + + public function test_drops_rows_that_match_no_channel(): void + { + $this->restoreLegacyTable(); + + PlatformChannel::factory()->create(['name' => 'newsbottest', 'channel_id' => 'newsbottest']); + $this->seedLegacyRow('999', 'a-community-that-no-longer-exists', '1'); + + $this->runMigration(); + + $this->assertSame(0, DB::table('platform_channel_posts')->count()); + } + + public function test_drops_rows_whose_channel_name_is_ambiguous_across_instances(): void + { + $this->restoreLegacyTable(); + + $first = PlatformInstance::factory()->create(['url' => 'https://one.test']); + $second = PlatformInstance::factory()->create(['url' => 'https://two.test']); + + PlatformChannel::factory()->create([ + 'platform_instance_id' => $first->id, + 'name' => 'news', + 'channel_id' => 'news', + ]); + PlatformChannel::factory()->create([ + 'platform_instance_id' => $second->id, + 'name' => 'news', + 'channel_id' => 'news-two', + ]); + + $this->seedLegacyRow('8', 'news', '1'); + + $this->runMigration(); + + $this->assertSame(0, DB::table('platform_channel_posts')->count()); + } + + public function test_leaves_a_schema_the_new_code_can_use(): void + { + $this->restoreLegacyTable(); + + $channel = PlatformChannel::factory()->create(['name' => 'newsbottest', 'channel_id' => 'newsbottest']); + $this->seedLegacyRow('217', 'newsbottest', '1'); + + $this->runMigration(); + + $this->assertTrue(Schema::hasColumn('platform_channel_posts', 'platform_channel_id')); + $this->assertFalse(Schema::hasColumn('platform_channel_posts', 'channel_id')); + $this->assertFalse(Schema::hasColumn('platform_channel_posts', 'channel_name')); + $this->assertFalse(Schema::hasColumn('platform_channel_posts', 'platform')); + + $this->assertTrue( + PlatformChannelPost::duplicateExists($channel, 'https://news.test/1', 'Post 1') + ); + } +} diff --git a/tests/Feature/Listeners/PublishApprovedArticleListenerTest.php b/tests/Feature/Listeners/PublishApprovedArticleListenerTest.php index fa0bb857..fcb09735 100644 --- a/tests/Feature/Listeners/PublishApprovedArticleListenerTest.php +++ b/tests/Feature/Listeners/PublishApprovedArticleListenerTest.php @@ -2,6 +2,7 @@ namespace Tests\Feature\Listeners; +use App\Actions\PublishRouteArticleAction; use App\Enums\NotificationSeverityEnum; use App\Enums\NotificationTypeEnum; use App\Events\RouteArticleApproved; @@ -15,6 +16,7 @@ use App\Services\Article\ArticleFetcher; use App\Services\Notification\NotificationService; use App\Services\Publishing\ArticlePublishingService; +use App\Services\Publishing\PublishOutcome; use Exception; use Illuminate\Foundation\Testing\RefreshDatabase; use Mockery; @@ -53,7 +55,7 @@ public function test_exception_during_publishing_creates_error_notification(): v $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); - $listener = new PublishApprovedArticleListener($articleFetcherMock, $publishingServiceMock, new NotificationService); + $listener = new PublishApprovedArticleListener(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, new NotificationService)); $listener->handle(new RouteArticleApproved($routeArticle)); $this->assertDatabaseHas('notifications', [ @@ -82,9 +84,9 @@ public function test_no_publication_created_creates_warning_notification(): void $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $publishingServiceMock->shouldReceive('publishRouteArticle') ->once() - ->andReturn(null); + ->andReturn(PublishOutcome::failure('No publication created')); - $listener = new PublishApprovedArticleListener($articleFetcherMock, $publishingServiceMock, new NotificationService); + $listener = new PublishApprovedArticleListener(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, new NotificationService)); $listener->handle(new RouteArticleApproved($routeArticle)); $this->assertDatabaseHas('notifications', [ @@ -112,9 +114,9 @@ public function test_successful_publish_does_not_create_notification(): void $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $publishingServiceMock->shouldReceive('publishRouteArticle') ->once() - ->andReturn(ArticlePublication::factory()->make()); + ->andReturn(PublishOutcome::published($this->makePublication())); - $listener = new PublishApprovedArticleListener($articleFetcherMock, $publishingServiceMock, new NotificationService); + $listener = new PublishApprovedArticleListener(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, new NotificationService)); $listener->handle(new RouteArticleApproved($routeArticle)); $this->assertDatabaseCount('notifications', 0); @@ -135,7 +137,7 @@ public function test_skips_already_published_to_channel(): void $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $publishingServiceMock->shouldNotReceive('publishRouteArticle'); - $listener = new PublishApprovedArticleListener($articleFetcherMock, $publishingServiceMock, new NotificationService); + $listener = new PublishApprovedArticleListener(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, new NotificationService)); $listener->handle(new RouteArticleApproved($routeArticle)); $this->assertTrue(true); @@ -146,4 +148,12 @@ protected function tearDown(): void Mockery::close(); parent::tearDown(); } + + private function makePublication(): ArticlePublication + { + /** @var ArticlePublication $publication */ + $publication = ArticlePublication::factory()->make(); + + return $publication; + } } diff --git a/tests/Feature/MirrorDuplicateDetectionTest.php b/tests/Feature/MirrorDuplicateDetectionTest.php new file mode 100644 index 00000000..73c84e11 --- /dev/null +++ b/tests/Feature/MirrorDuplicateDetectionTest.php @@ -0,0 +1,197 @@ +create(); + $instance = PlatformInstance::factory()->create(['url' => 'https://lemmy.test']); + $channel = PlatformChannel::factory()->create([ + 'platform_instance_id' => $instance->id, + 'channel_id' => 'news', + 'name' => 'news', + ]); + $account = PlatformAccount::factory()->create(['instance_url' => 'https://lemmy.test']); + + /** @var Route $route */ + $route = Route::factory()->active()->create([ + 'feed_id' => $feed->id, + 'platform_channel_id' => $channel->id, + ]); + + $channel->platformAccounts()->attach($account->id, ['is_active' => true, 'priority' => 50]); + + $article = Article::factory()->create([ + 'feed_id' => $feed->id, + 'url' => 'https://news.test/already-posted', + ]); + + /** @var RouteArticle $routeArticle */ + $routeArticle = RouteArticle::factory()->forRoute($route)->approved()->create([ + 'article_id' => $article->id, + ]); + + $this->fixture = [$routeArticle, $channel, $article]; + } + + protected function tearDown(): void + { + Mockery::close(); + parent::tearDown(); + } + + private function service(?LemmyPublisher $publisher = null): ArticlePublishingService + { + $service = Mockery::mock(ArticlePublishingService::class, [app(LogSaver::class)])->makePartial(); + $service->shouldAllowMockingProtectedMethods(); + $service->shouldReceive('makePublisher')->andReturn( + $publisher ?? Mockery::mock(LemmyPublisher::class) + ); + + return $service; + } + + public function test_sync_writes_the_key_the_duplicate_check_reads(): void + { + [$routeArticle, $channel, $article] = $this->fixture; + + // Populate the mirror through the real sync path rather than seeding a + // row by hand, so write and read cannot silently disagree. + Http::fake([ + '*/api/v3/post/list*' => Http::response([ + 'posts' => [[ + 'post' => [ + 'id' => 555, + 'url' => $article->url, + 'name' => 'Already Posted', + 'published' => '2026-08-02T10:00:00Z', + ], + ]], + ]), + ]); + + (new LemmyApiService('https://lemmy.test')) + ->syncChannelPosts('token', $channel, 8); + + $this->assertDatabaseCount('platform_channel_posts', 1); + + $publisher = Mockery::mock(LemmyPublisher::class); + $publisher->shouldNotReceive('publishToChannel'); + + $result = $this->service($publisher)->publishRouteArticle($routeArticle, ['title' => 'Already Posted']); + + $this->assertTrue($result->wasSkipped()); + $this->assertDatabaseCount('article_publications', 0); + } + + public function test_a_skipped_duplicate_is_not_reported_as_a_publish_failure(): void + { + [$routeArticle, $channel, $article] = $this->fixture; + + PlatformChannelPost::storePost($channel, '555', $article->url, 'Already Posted'); + + $fetcher = Mockery::mock(ArticleFetcher::class); + $fetcher->shouldReceive('fetchArticleData')->andReturn(['title' => 'Already Posted']); + + $publisher = Mockery::mock(LemmyPublisher::class); + $publisher->shouldNotReceive('publishToChannel'); + + (new PublishApprovedArticleListener(new PublishRouteArticleAction($fetcher, $this->service($publisher), new NotificationService))) + ->handle(new RouteArticleApproved($routeArticle)); + + $this->assertSame(PublishStatusEnum::SKIPPED, $routeArticle->fresh()->publish_status); + $this->assertDatabaseCount('notifications', 0); + } + + public function test_a_genuine_failure_is_still_reported(): void + { + [$routeArticle] = $this->fixture; + + $fetcher = Mockery::mock(ArticleFetcher::class); + $fetcher->shouldReceive('fetchArticleData')->andReturn(['title' => 'Some Title']); + + $publisher = Mockery::mock(LemmyPublisher::class); + $publisher->shouldReceive('publishToChannel')->andThrow(new \RuntimeException('Lemmy rejected the post')); + + (new PublishApprovedArticleListener(new PublishRouteArticleAction($fetcher, $this->service($publisher), new NotificationService))) + ->handle(new RouteArticleApproved($routeArticle)); + + $this->assertSame(PublishStatusEnum::ERROR, $routeArticle->fresh()->publish_status); + $this->assertDatabaseHas('notifications', [ + 'type' => NotificationTypeEnum::PUBLISH_FAILED->value, + ]); + } + + public function test_mirror_is_keyed_by_the_local_channel(): void + { + [, $channel, $article] = $this->fixture; + + PlatformChannelPost::storePost( + $channel, + '555', + $article->url, + 'Already Posted', + ); + + $this->assertDatabaseHas('platform_channel_posts', [ + 'platform_channel_id' => $channel->id, + 'post_id' => '555', + ]); + } + + public function test_a_second_instance_with_the_same_community_name_does_not_collide(): void + { + [, $channel, $article] = $this->fixture; + + $otherInstance = PlatformInstance::factory()->create(['url' => 'https://other.test']); + $otherChannel = PlatformChannel::factory()->create([ + 'platform_instance_id' => $otherInstance->id, + 'channel_id' => 'news', + 'name' => 'news', + ]); + + PlatformChannelPost::storePost($channel, '1', $article->url, 'Same Title'); + + // Same community name on a different instance is a different community. + $this->assertFalse( + PlatformChannelPost::duplicateExists($otherChannel, $article->url, 'Same Title') + ); + + $this->assertTrue( + PlatformChannelPost::duplicateExists($channel, $article->url, 'Same Title') + ); + } +} diff --git a/tests/Unit/Jobs/PublishNextArticleJobTest.php b/tests/Unit/Jobs/PublishNextArticleJobTest.php index 66145951..28046481 100644 --- a/tests/Unit/Jobs/PublishNextArticleJobTest.php +++ b/tests/Unit/Jobs/PublishNextArticleJobTest.php @@ -2,6 +2,7 @@ namespace Tests\Unit\Jobs; +use App\Actions\PublishRouteArticleAction; use App\Enums\NotificationSeverityEnum; use App\Enums\NotificationTypeEnum; use App\Exceptions\PublishException; @@ -16,6 +17,7 @@ use App\Services\Article\ArticleFetcher; use App\Services\Notification\NotificationService; use App\Services\Publishing\ArticlePublishingService; +use App\Services\Publishing\PublishOutcome; use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Queue\Queueable; @@ -95,7 +97,7 @@ public function test_handle_returns_early_when_no_approved_route_articles(): voi $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertTrue(true); } @@ -114,7 +116,7 @@ public function test_handle_returns_early_when_no_unpublished_approved_route_art $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertTrue(true); } @@ -132,7 +134,7 @@ public function test_handle_skips_non_approved_route_articles(): void $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertTrue(true); } @@ -170,10 +172,10 @@ public function test_handle_publishes_oldest_approved_route_article(): void Mockery::on(fn ($ra) => $ra->article_id === $olderArticle->id), $extractedData ) - ->andReturn(ArticlePublication::factory()->make()); + ->andReturn(PublishOutcome::published($this->makePublication())); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertTrue(true); } @@ -200,7 +202,7 @@ public function test_handle_throws_exception_on_publishing_failure(): void $this->expectException(PublishException::class); - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); } public function test_handle_skips_publishing_when_last_publication_within_interval(): void @@ -219,7 +221,7 @@ public function test_handle_skips_publishing_when_last_publication_within_interv $publishingServiceMock->shouldNotReceive('publishRouteArticle'); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertTrue(true); } @@ -243,10 +245,10 @@ public function test_handle_publishes_when_last_publication_beyond_interval(): v $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $publishingServiceMock->shouldReceive('publishRouteArticle') ->once() - ->andReturn(ArticlePublication::factory()->make()); + ->andReturn(PublishOutcome::published($this->makePublication())); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertTrue(true); } @@ -270,10 +272,10 @@ public function test_handle_publishes_when_interval_is_zero(): void $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $publishingServiceMock->shouldReceive('publishRouteArticle') ->once() - ->andReturn(ArticlePublication::factory()->make()); + ->andReturn(PublishOutcome::published($this->makePublication())); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertTrue(true); } @@ -297,10 +299,10 @@ public function test_handle_publishes_when_last_publication_exactly_at_interval( $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $publishingServiceMock->shouldReceive('publishRouteArticle') ->once() - ->andReturn(ArticlePublication::factory()->make()); + ->andReturn(PublishOutcome::published($this->makePublication())); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertTrue(true); } @@ -321,10 +323,10 @@ public function test_handle_publishes_when_no_previous_publications_exist(): voi $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $publishingServiceMock->shouldReceive('publishRouteArticle') ->once() - ->andReturn(ArticlePublication::factory()->make()); + ->andReturn(PublishOutcome::published($this->makePublication())); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertTrue(true); } @@ -343,10 +345,10 @@ public function test_handle_creates_warning_notification_when_no_publication_cre $publishingServiceMock = Mockery::mock(ArticlePublishingService::class); $publishingServiceMock->shouldReceive('publishRouteArticle') ->once() - ->andReturn(null); + ->andReturn(PublishOutcome::failure('No publication created')); $job = new PublishNextArticleJob; - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); $this->assertDatabaseHas('notifications', [ 'type' => NotificationTypeEnum::PUBLISH_FAILED->value, @@ -380,7 +382,7 @@ public function test_handle_creates_notification_on_publish_exception(): void $job = new PublishNextArticleJob; try { - $job->handle($articleFetcherMock, $publishingServiceMock, $this->notificationService); + $job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService)); } catch (PublishException) { // Expected } @@ -413,4 +415,12 @@ protected function tearDown(): void Mockery::close(); parent::tearDown(); } + + private function makePublication(): ArticlePublication + { + /** @var ArticlePublication $publication */ + $publication = ArticlePublication::factory()->make(); + + return $publication; + } } diff --git a/tests/Unit/Jobs/SyncChannelPostsJobTest.php b/tests/Unit/Jobs/SyncChannelPostsJobTest.php index 62fcc10e..946c48a9 100644 --- a/tests/Unit/Jobs/SyncChannelPostsJobTest.php +++ b/tests/Unit/Jobs/SyncChannelPostsJobTest.php @@ -148,7 +148,7 @@ public function test_sync_resolves_non_numeric_channel_id_via_get_community_id() ->andReturn(42); $apiMock->shouldReceive('syncChannelPosts') ->once() - ->with('token', 42, $channel->name); + ->with('token', Mockery::on(fn ($arg) => $arg->is($channel)), 42); $logSaverMock = Mockery::mock(LogSaver::class); $logSaverMock->shouldReceive('info')->zeroOrMoreTimes(); @@ -179,7 +179,7 @@ public function test_sync_passes_resolved_community_id_to_sync_channel_posts(): ->andReturn(42); $apiMock->shouldReceive('syncChannelPosts') ->once() - ->with('token', 42, $channel->name); + ->with('token', Mockery::on(fn ($arg) => $arg->is($channel)), 42); $logSaverMock = Mockery::mock(LogSaver::class); $logSaverMock->shouldReceive('info')->zeroOrMoreTimes(); diff --git a/tests/Unit/Modules/Lemmy/Services/LemmyApiServiceTest.php b/tests/Unit/Modules/Lemmy/Services/LemmyApiServiceTest.php index 9d50ead0..f807bba1 100644 --- a/tests/Unit/Modules/Lemmy/Services/LemmyApiServiceTest.php +++ b/tests/Unit/Modules/Lemmy/Services/LemmyApiServiceTest.php @@ -2,7 +2,7 @@ namespace Tests\Unit\Modules\Lemmy\Services; -use App\Enums\PlatformEnum; +use App\Models\PlatformChannel; use App\Modules\Lemmy\Services\LemmyApiService; use Exception; use Illuminate\Foundation\Testing\RefreshDatabase; @@ -13,6 +13,13 @@ class LemmyApiServiceTest extends TestCase { use RefreshDatabase; + private ?PlatformChannel $channel = null; + + private function syncChannel(): PlatformChannel + { + return $this->channel ??= PlatformChannel::factory()->create(); + } + public function test_constructor_sets_instance(): void { $service = new LemmyApiService('lemmy.world'); @@ -248,7 +255,7 @@ public function test_sync_channel_posts_success(): void ]); $service = new LemmyApiService('lemmy.world'); - $service->syncChannelPosts('token', 42, 'test-community'); + $service->syncChannelPosts('token', $this->syncChannel(), 42); Http::assertSent(function ($request) { return str_contains($request->url(), '/api/v3/post/list') @@ -259,18 +266,14 @@ public function test_sync_channel_posts_success(): void // Verify posts were stored in the database $this->assertDatabaseHas('platform_channel_posts', [ - 'platform' => PlatformEnum::LEMMY->value, - 'channel_id' => '42', - 'channel_name' => 'test-community', + 'platform_channel_id' => $this->syncChannel()->id, 'post_id' => '1', 'url' => 'https://example.com/1', 'title' => 'Post 1', ]); $this->assertDatabaseHas('platform_channel_posts', [ - 'platform' => PlatformEnum::LEMMY->value, - 'channel_id' => '42', - 'channel_name' => 'test-community', + 'platform_channel_id' => $this->syncChannel()->id, 'post_id' => '2', 'url' => 'https://example.com/2', 'title' => 'Post 2', @@ -284,7 +287,7 @@ public function test_sync_channel_posts_handles_unsuccessful_response(): void ]); $service = new LemmyApiService('lemmy.world'); - $service->syncChannelPosts('token', 42, 'test-community'); + $service->syncChannelPosts('token', $this->syncChannel(), 42); Http::assertSentCount(1); $this->assertDatabaseCount('platform_channel_posts', 0); @@ -297,7 +300,7 @@ public function test_sync_channel_posts_handles_exception(): void }); $service = new LemmyApiService('lemmy.world'); - $service->syncChannelPosts('token', 42, 'test-community'); + $service->syncChannelPosts('token', $this->syncChannel(), 42); // Assert that the method completes without throwing $this->assertTrue(true); diff --git a/tests/Unit/Services/Publishing/ArticlePublishingServiceTest.php b/tests/Unit/Services/Publishing/ArticlePublishingServiceTest.php index cfdfd9f4..0ea074b2 100644 --- a/tests/Unit/Services/Publishing/ArticlePublishingServiceTest.php +++ b/tests/Unit/Services/Publishing/ArticlePublishingServiceTest.php @@ -2,7 +2,6 @@ namespace Tests\Unit\Services\Publishing; -use App\Enums\PlatformEnum; use App\Models\Article; use App\Models\ArticlePublication; use App\Models\Feed; @@ -99,7 +98,7 @@ public function test_publish_route_article_returns_null_when_no_active_account() $result = $this->service->publishRouteArticle($routeArticle, ['title' => 'Test']); - $this->assertNull($result); + $this->assertTrue($result->failed()); $this->assertDatabaseCount('article_publications', 0); } @@ -118,7 +117,7 @@ public function test_publish_route_article_successfully_publishes(): void $result = $service->publishRouteArticle($routeArticle, ['title' => 'Hello']); - $this->assertNotNull($result); + $this->assertTrue($result->succeeded()); $this->assertDatabaseHas('article_publications', [ 'article_id' => $article->id, 'platform_channel_id' => $channel->id, @@ -190,7 +189,7 @@ public function test_losing_the_lock_race_skips_without_publishing(): void // caller's catch block and be recorded as a publish failure. $result = $service->publishRouteArticle($routeArticle, ['title' => 'Hello']); - $this->assertNull($result); + $this->assertTrue($result->wasSkipped()); $this->assertDatabaseCount('article_publications', 0); } @@ -209,7 +208,7 @@ public function test_publish_route_article_handles_publishing_failure_gracefully $result = $service->publishRouteArticle($routeArticle, ['title' => 'Hello']); - $this->assertNull($result); + $this->assertTrue($result->failed()); $this->assertDatabaseCount('article_publications', 0); } @@ -219,9 +218,7 @@ public function test_publish_skips_duplicate_when_url_already_posted_to_channel( // Simulate the URL already being posted to this channel PlatformChannelPost::storePost( - PlatformEnum::LEMMY, - (string) $channel->channel_id, - $channel->name, + $channel, '999', $article->url, 'Different Title', @@ -236,7 +233,7 @@ public function test_publish_skips_duplicate_when_url_already_posted_to_channel( $result = $service->publishRouteArticle($routeArticle, ['title' => 'Some Title']); - $this->assertNull($result); + $this->assertTrue($result->wasSkipped()); $this->assertDatabaseCount('article_publications', 0); } @@ -246,9 +243,7 @@ public function test_publish_skips_duplicate_when_title_already_posted_to_channe // Simulate the same title already posted with a different URL PlatformChannelPost::storePost( - PlatformEnum::LEMMY, - (string) $channel->channel_id, - $channel->name, + $channel, '888', 'https://example.com/different-url', 'Breaking News', @@ -263,7 +258,7 @@ public function test_publish_skips_duplicate_when_title_already_posted_to_channe $result = $service->publishRouteArticle($routeArticle, ['title' => 'Breaking News']); - $this->assertNull($result); + $this->assertTrue($result->wasSkipped()); $this->assertDatabaseCount('article_publications', 0); } @@ -273,9 +268,7 @@ public function test_publish_proceeds_when_no_duplicate_exists(): void // Existing post in the channel has a completely different URL and title PlatformChannelPost::storePost( - PlatformEnum::LEMMY, - (string) $channel->channel_id, - $channel->name, + $channel, '777', 'https://example.com/other-article', 'Totally Different Title', @@ -292,7 +285,7 @@ public function test_publish_proceeds_when_no_duplicate_exists(): void $result = $service->publishRouteArticle($routeArticle, ['title' => 'Unique Title']); - $this->assertNotNull($result); + $this->assertTrue($result->succeeded()); $this->assertDatabaseHas('article_publications', [ 'article_id' => $article->id, 'post_id' => 456,