diff --git a/app/Actions/PublishRouteArticleAction.php b/app/Actions/PublishRouteArticleAction.php index b6a2f204..deef6d52 100644 --- a/app/Actions/PublishRouteArticleAction.php +++ b/app/Actions/PublishRouteArticleAction.php @@ -8,6 +8,7 @@ use App\Enums\PublishStatusEnum; use App\Events\ActionPerformed; use App\Exceptions\PublishException; +use App\Models\Article; use App\Models\RouteArticle; use App\Services\Article\ArticleFetcher; use App\Services\Notification\NotificationService; @@ -33,7 +34,7 @@ public function execute(RouteArticle $routeArticle): PublishOutcome $routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHING]); try { - $extractedData = $this->articleFetcher->fetchArticleData($article); + $extractedData = $this->resolvePublishData($article); $outcome = $this->publishingService->publishRouteArticle($routeArticle, $extractedData); } catch (Exception $e) { $routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]); @@ -63,6 +64,22 @@ public function execute(RouteArticle $routeArticle): PublishOutcome return $outcome; } + /** + * @return array + */ + private function resolvePublishData(Article $article): array + { + if (empty($article->description) && empty($article->image_url)) { + return $this->articleFetcher->fetchArticleData($article); + } + + return [ + 'title' => $article->title, + 'description' => $article->description, + 'thumbnail' => $article->image_url, + ]; + } + private function recordPublished(RouteArticle $routeArticle): void { $routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHED]); diff --git a/database/factories/ArticleFactory.php b/database/factories/ArticleFactory.php index 57c212c5..f111c7b9 100644 --- a/database/factories/ArticleFactory.php +++ b/database/factories/ArticleFactory.php @@ -24,9 +24,22 @@ public function definition(): array 'title' => $this->faker->sentence(), 'description' => $this->faker->paragraph(), 'content' => $this->faker->paragraphs(3, true), - 'image_url' => $this->faker->optional()->imageUrl(), + 'image_url' => $this->faker->imageUrl(), 'published_at' => $this->faker->optional()->dateTimeBetween('-1 month', 'now'), 'author' => $this->faker->optional()->name(), ]; } + + /** + * An article discovered but never validated, so publishing must fetch its data live. + */ + public function unvalidated(): static + { + return $this->state(fn (array $attributes): array => [ + 'description' => null, + 'content' => null, + 'image_url' => null, + 'validated_at' => null, + ]); + } } diff --git a/tests/Feature/DuplicatePublishTest.php b/tests/Feature/DuplicatePublishTest.php index f4b65645..1d589a27 100644 --- a/tests/Feature/DuplicatePublishTest.php +++ b/tests/Feature/DuplicatePublishTest.php @@ -94,7 +94,7 @@ private function makeListener(): PublishApprovedArticleListener $service->shouldReceive('makePublisher')->andReturn($publisher); $fetcher = Mockery::mock(ArticleFetcher::class); - $fetcher->shouldReceive('fetchArticleData')->andReturn(['title' => 'Test Article']); + $fetcher->shouldNotReceive('fetchArticleData'); return new PublishApprovedArticleListener(new PublishRouteArticleAction($fetcher, $service, new NotificationService)); } @@ -160,7 +160,7 @@ public function test_two_queued_listeners_create_only_one_remote_post(): void $service->shouldReceive('makePublisher')->andReturn($publisher); $fetcher = Mockery::mock(ArticleFetcher::class); - $fetcher->shouldReceive('fetchArticleData')->andReturn(['title' => 'Test Article']); + $fetcher->shouldNotReceive('fetchArticleData'); $listener = new PublishApprovedArticleListener(new PublishRouteArticleAction($fetcher, $service, new NotificationService)); $listener->handle(new RouteArticleApproved($routeArticle)); diff --git a/tests/Feature/Listeners/PublishApprovedArticleListenerTest.php b/tests/Feature/Listeners/PublishApprovedArticleListenerTest.php index fcb09735..4f1cbe3c 100644 --- a/tests/Feature/Listeners/PublishApprovedArticleListenerTest.php +++ b/tests/Feature/Listeners/PublishApprovedArticleListenerTest.php @@ -31,7 +31,7 @@ private function createApprovedRouteArticle(string $title = 'Test Article'): Rou $feed = Feed::factory()->create(); /** @var Route $route */ $route = Route::factory()->active()->create(['feed_id' => $feed->id]); - $article = Article::factory()->create([ + $article = Article::factory()->unvalidated()->create([ 'feed_id' => $feed->id, 'title' => $title, ]); diff --git a/tests/Feature/Livewire/ArticlesTest.php b/tests/Feature/Livewire/ArticlesTest.php index 6d209c8e..87219c16 100644 --- a/tests/Feature/Livewire/ArticlesTest.php +++ b/tests/Feature/Livewire/ArticlesTest.php @@ -342,12 +342,16 @@ public function test_expanding_a_feed_shows_all_its_articles_beyond_one_page(): { $vrt = Feed::factory()->create(['name' => 'VRT News']); + $base = now()->startOfSecond(); + for ($i = 0; $i < 20; $i++) { - $this->travelTo(now()->addMinutes($i), function () use ($vrt, $i) { + $this->travelTo($base->copy()->addMinutes($i), function () use ($vrt, $i) { $this->createRouteArticleForFeed($vrt, "VRT Article {$i}"); }); } + $this->travelBack(); + $component = Livewire::test(Articles::class)->call('toggleFeed', $vrt->id); for ($i = 0; $i < 20; $i++) { diff --git a/tests/Feature/MirrorDuplicateDetectionTest.php b/tests/Feature/MirrorDuplicateDetectionTest.php index 73c84e11..ccd6b2ad 100644 --- a/tests/Feature/MirrorDuplicateDetectionTest.php +++ b/tests/Feature/MirrorDuplicateDetectionTest.php @@ -57,6 +57,7 @@ protected function setUp(): void $article = Article::factory()->create([ 'feed_id' => $feed->id, 'url' => 'https://news.test/already-posted', + 'title' => 'Already Posted', ]); /** @var RouteArticle $routeArticle */ @@ -124,7 +125,7 @@ public function test_a_skipped_duplicate_is_not_reported_as_a_publish_failure(): PlatformChannelPost::storePost($channel, '555', $article->url, 'Already Posted'); $fetcher = Mockery::mock(ArticleFetcher::class); - $fetcher->shouldReceive('fetchArticleData')->andReturn(['title' => 'Already Posted']); + $fetcher->shouldNotReceive('fetchArticleData'); $publisher = Mockery::mock(LemmyPublisher::class); $publisher->shouldNotReceive('publishToChannel'); @@ -141,7 +142,7 @@ 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']); + $fetcher->shouldNotReceive('fetchArticleData'); $publisher = Mockery::mock(LemmyPublisher::class); $publisher->shouldReceive('publishToChannel')->andThrow(new \RuntimeException('Lemmy rejected the post')); diff --git a/tests/Unit/Actions/PublishRouteArticleActionTest.php b/tests/Unit/Actions/PublishRouteArticleActionTest.php new file mode 100644 index 00000000..d3798776 --- /dev/null +++ b/tests/Unit/Actions/PublishRouteArticleActionTest.php @@ -0,0 +1,184 @@ + $articleAttributes + */ + private function createRouteArticle(array $articleAttributes, bool $unvalidated = false): RouteArticle + { + $feed = Feed::factory()->create(); + /** @var Route $route */ + $route = Route::factory()->active()->create(['feed_id' => $feed->id]); + + $factory = Article::factory(); + if ($unvalidated) { + $factory = $factory->unvalidated(); + } + $article = $factory->create(array_merge(['feed_id' => $feed->id], $articleAttributes)); + + /** @var RouteArticle $routeArticle */ + $routeArticle = RouteArticle::factory()->forRoute($route)->approved()->create([ + 'article_id' => $article->id, + ]); + + return $routeArticle; + } + + private function makePublication(): ArticlePublication + { + /** @var ArticlePublication $publication */ + $publication = ArticlePublication::factory()->create(); + + return $publication; + } + + public function test_publish_uses_stored_article_data_without_fetching(): void + { + $routeArticle = $this->createRouteArticle([ + 'title' => 'Stored Title', + 'description' => 'Stored description', + 'image_url' => 'https://cdn.test/stored.jpg', + ]); + + $fetcher = Mockery::mock(ArticleFetcher::class); + $fetcher->shouldNotReceive('fetchArticleData'); + + $publishingService = Mockery::mock(ArticlePublishingService::class); + $publishingService->shouldReceive('publishRouteArticle') + ->once() + ->with(Mockery::any(), [ + 'title' => 'Stored Title', + 'description' => 'Stored description', + 'thumbnail' => 'https://cdn.test/stored.jpg', + ]) + ->andReturn(PublishOutcome::published($this->makePublication())); + + (new PublishRouteArticleAction($fetcher, $publishingService, new NotificationService)) + ->execute($routeArticle); + + $this->assertSame(PublishStatusEnum::PUBLISHED, $routeArticle->fresh()->publish_status); + } + + public function test_publish_body_comes_from_description_not_content(): void + { + $routeArticle = $this->createRouteArticle([ + 'description' => 'The short description', + 'content' => 'The very much longer full article body text', + ]); + + $fetcher = Mockery::mock(ArticleFetcher::class); + $fetcher->shouldNotReceive('fetchArticleData'); + + $captured = null; + $publishingService = Mockery::mock(ArticlePublishingService::class); + $publishingService->shouldReceive('publishRouteArticle') + ->once() + ->andReturnUsing(function ($ra, $data) use (&$captured) { + $captured = $data; + + return PublishOutcome::published($this->makePublication()); + }); + + (new PublishRouteArticleAction($fetcher, $publishingService, new NotificationService)) + ->execute($routeArticle); + + $this->assertSame('The short description', $captured['description']); + $this->assertArrayNotHasKey('content', $captured); + } + + public function test_publish_falls_back_to_fetching_when_description_is_blank(): void + { + $routeArticle = $this->createRouteArticle([ + 'description' => '', + 'image_url' => null, + ]); + + $fetched = ['title' => 'Fetched Title', 'description' => 'Fetched description']; + + $fetcher = Mockery::mock(ArticleFetcher::class); + $fetcher->shouldReceive('fetchArticleData')->once()->andReturn($fetched); + + $publishingService = Mockery::mock(ArticlePublishingService::class); + $publishingService->shouldReceive('publishRouteArticle') + ->once() + ->with(Mockery::any(), $fetched) + ->andReturn(PublishOutcome::published($this->makePublication())); + + (new PublishRouteArticleAction($fetcher, $publishingService, new NotificationService)) + ->execute($routeArticle); + + $this->assertSame(PublishStatusEnum::PUBLISHED, $routeArticle->fresh()->publish_status); + } + + public function test_publish_falls_back_to_fetching_when_article_has_no_stored_data(): void + { + $routeArticle = $this->createRouteArticle([], unvalidated: true); + + $fetched = [ + 'title' => 'Fetched Title', + 'description' => 'Fetched description', + 'thumbnail' => 'https://cdn.test/fetched.jpg', + ]; + + $fetcher = Mockery::mock(ArticleFetcher::class); + $fetcher->shouldReceive('fetchArticleData')->once()->andReturn($fetched); + + $publishingService = Mockery::mock(ArticlePublishingService::class); + $publishingService->shouldReceive('publishRouteArticle') + ->once() + ->with(Mockery::any(), $fetched) + ->andReturn(PublishOutcome::published($this->makePublication())); + + (new PublishRouteArticleAction($fetcher, $publishingService, new NotificationService)) + ->execute($routeArticle); + + $this->assertSame(PublishStatusEnum::PUBLISHED, $routeArticle->fresh()->publish_status); + } + + public function test_publish_does_not_refetch_for_articles_stored_before_image_url_existed(): void + { + $routeArticle = $this->createRouteArticle([ + 'title' => 'Legacy Title', + 'description' => 'Legacy description', + 'image_url' => null, + ]); + + $fetcher = Mockery::mock(ArticleFetcher::class); + $fetcher->shouldNotReceive('fetchArticleData'); + + $publishingService = Mockery::mock(ArticlePublishingService::class); + $publishingService->shouldReceive('publishRouteArticle') + ->once() + ->with(Mockery::any(), [ + 'title' => 'Legacy Title', + 'description' => 'Legacy description', + 'thumbnail' => null, + ]) + ->andReturn(PublishOutcome::published($this->makePublication())); + + (new PublishRouteArticleAction($fetcher, $publishingService, new NotificationService)) + ->execute($routeArticle); + + $this->assertSame(PublishStatusEnum::PUBLISHED, $routeArticle->fresh()->publish_status); + } +} diff --git a/tests/Unit/Jobs/PublishNextArticleJobTest.php b/tests/Unit/Jobs/PublishNextArticleJobTest.php index 28046481..43913a39 100644 --- a/tests/Unit/Jobs/PublishNextArticleJobTest.php +++ b/tests/Unit/Jobs/PublishNextArticleJobTest.php @@ -46,7 +46,7 @@ private function createApprovedRouteArticle(array $articleOverrides = [], array $feed = Feed::factory()->create(); /** @var Route $route */ $route = Route::factory()->active()->create(array_merge(['feed_id' => $feed->id], $routeOverrides)); - $article = Article::factory()->create(array_merge(['feed_id' => $feed->id], $articleOverrides)); + $article = Article::factory()->unvalidated()->create(array_merge(['feed_id' => $feed->id], $articleOverrides)); /** @var RouteArticle $routeArticle */ $routeArticle = RouteArticle::factory()->forRoute($route)->approved()->create([ @@ -145,8 +145,8 @@ public function test_handle_publishes_oldest_approved_route_article(): void /** @var Route $route */ $route = Route::factory()->active()->create(['feed_id' => $feed->id]); - $olderArticle = Article::factory()->create(['feed_id' => $feed->id]); - $newerArticle = Article::factory()->create(['feed_id' => $feed->id]); + $olderArticle = Article::factory()->unvalidated()->create(['feed_id' => $feed->id]); + $newerArticle = Article::factory()->unvalidated()->create(['feed_id' => $feed->id]); RouteArticle::factory()->forRoute($route)->approved()->create([ 'article_id' => $olderArticle->id,