diff --git a/app/Services/Parsers/BelgaHomepageParserAdapter.php b/app/Services/Parsers/BelgaHomepageParserAdapter.php index 0adb6750..e8b49a66 100644 --- a/app/Services/Parsers/BelgaHomepageParserAdapter.php +++ b/app/Services/Parsers/BelgaHomepageParserAdapter.php @@ -7,15 +7,13 @@ class BelgaHomepageParserAdapter implements HomepageParserInterface { /** - * Must stay in sync with config('feed.providers.belga.languages.en.url'). - * Nothing calls getHomepageUrl() at runtime — the fetcher uses $feed->url, - * seeded from that config value — so drift here would go unnoticed. + * Belga is English-only for now (#124). The unused $language argument keeps + * the constructor compatible with HomepageParserFactory, which passes one + * positionally to every homepage parser. + * + * @phpstan-ignore constructor.unusedParameter */ - private const API_URL = 'https://capi.belga.press/belgapress/api/public/pressreleases?offset=1&count=6&search=&start=&end=&newsroomId=70&language=EN'; - - public function __construct( - private string $language = 'en', - ) {} + public function __construct(string $language = 'en') {} public function canParse(string $url): bool { @@ -29,7 +27,9 @@ public function extractArticleUrls(string $html): array public function getHomepageUrl(): string { - return str_replace('language=EN', 'language='.strtoupper($this->language), self::API_URL); + $url = config('feed.providers.belga.languages.en.url'); + + return is_string($url) ? $url : ''; } public function getSourceName(): string diff --git a/config/feed.php b/config/feed.php index 0d82739e..78d23199 100644 --- a/config/feed.php +++ b/config/feed.php @@ -45,7 +45,7 @@ 'type' => 'website', 'is_active' => true, 'languages' => [ - 'en' => ['url' => 'https://capi.belga.press/belgapress/api/public/pressreleases?offset=1&count=6&search=&start=&end=&newsroomId=70&language=EN'], + 'en' => ['url' => 'https://capi.belga.press/belgapress/api/public/pressreleases?offset=1&count=50&search=&start=&end=&newsroomId=70&language=EN'], ], 'parsers' => [ 'homepage' => BelgaHomepageParserAdapter::class, diff --git a/database/migrations/2024_01_01_000012_sync_belga_feed_to_website_discovery.php b/database/migrations/2024_01_01_000012_sync_belga_feed_to_website_discovery.php new file mode 100644 index 00000000..275afc74 --- /dev/null +++ b/database/migrations/2024_01_01_000012_sync_belga_feed_to_website_discovery.php @@ -0,0 +1,65 @@ +where('provider', 'belga')->where('url', $url)->min('id') + ?? DB::table('feeds')->where('provider', 'belga')->min('id'); + + if ($keepId === null) { + return; + } + + DB::table('feeds') + ->where('provider', 'belga') + ->where('id', '!=', $keepId) + ->update([ + 'is_active' => false, + 'updated_at' => now(), + ]); + + DB::table('feeds') + ->where('id', $keepId) + ->update([ + 'type' => 'website', + 'url' => $url, + 'updated_at' => now(), + ]); + } + + public function down(): void + { + // No rollback: the previous RSS endpoint 404s, so restoring it would + // only reinstate a feed that silently yields no articles. + } +}; diff --git a/tests/Feature/SyncBelgaFeedMigrationTest.php b/tests/Feature/SyncBelgaFeedMigrationTest.php new file mode 100644 index 00000000..31c3d93b --- /dev/null +++ b/tests/Feature/SyncBelgaFeedMigrationTest.php @@ -0,0 +1,179 @@ +up(); + } + + private function findFeed(int $id): stdClass + { + $feed = DB::table('feeds')->find($id); + + $this->assertInstanceOf(stdClass::class, $feed); + + return $feed; + } + + private function targetUrl(): string + { + $url = config('feed.providers.belga.languages.en.url'); + + $this->assertIsString($url); + + return $url; + } + + private function seedFeed(string $url, string $type, bool $isActive = true, string $provider = 'belga'): int + { + return DB::table('feeds')->insertGetId([ + 'name' => 'Belga', + 'provider' => $provider, + 'type' => $type, + 'url' => $url, + 'is_active' => $isActive, + 'created_at' => now(), + 'updated_at' => now(), + ]); + } + + public function test_converts_a_feed_still_on_the_dead_rss_url(): void + { + DB::table('feeds')->delete(); + $id = $this->seedFeed(self::DEAD_RSS_URL, 'rss'); + + $this->runMigration(); + + $feed = $this->findFeed($id); + + $this->assertSame('website', $feed->type); + $this->assertSame($this->targetUrl(), $feed->url); + $this->assertTrue((bool) $feed->is_active); + } + + public function test_is_idempotent_when_the_feed_is_already_migrated(): void + { + DB::table('feeds')->delete(); + $id = $this->seedFeed($this->targetUrl(), 'website'); + + $this->runMigration(); + $this->runMigration(); + + $this->assertSame(1, DB::table('feeds')->where('provider', 'belga')->count()); + + $feed = $this->findFeed($id); + $this->assertSame('website', $feed->type); + $this->assertSame($this->targetUrl(), $feed->url); + } + + public function test_deactivates_superseded_rows_instead_of_colliding_on_the_unique_url(): void + { + DB::table('feeds')->delete(); + $staleId = $this->seedFeed(self::DEAD_RSS_URL, 'rss'); + $currentId = $this->seedFeed($this->targetUrl(), 'website'); + + $this->runMigration(); + + // Both rows survive: routes.feed_id cascades on delete, so a superseded + // feed is deactivated rather than removed. + $this->assertSame(2, DB::table('feeds')->where('provider', 'belga')->count()); + + $current = $this->findFeed($currentId); + $this->assertTrue((bool) $current->is_active); + $this->assertSame($this->targetUrl(), $current->url); + $this->assertSame('website', $current->type); + + $stale = $this->findFeed($staleId); + $this->assertFalse((bool) $stale->is_active); + } + + public function test_adopts_the_oldest_row_when_none_is_on_the_target_url(): void + { + DB::table('feeds')->delete(); + $oldestId = $this->seedFeed(self::DEAD_RSS_URL, 'rss'); + $newerId = $this->seedFeed('https://capi.belga.press/belgapress/api/public/pressreleases?count=6', 'website'); + + $this->runMigration(); + + // Neither row matches config, so the migration falls back to the lowest + // id — the oldest row, which is the one routes are most likely tied to. + $oldest = $this->findFeed($oldestId); + $this->assertSame($this->targetUrl(), $oldest->url); + $this->assertSame('website', $oldest->type); + $this->assertTrue((bool) $oldest->is_active); + + $this->assertFalse((bool) $this->findFeed($newerId)->is_active); + } + + public function test_leaves_other_providers_untouched(): void + { + DB::table('feeds')->delete(); + $vrtId = $this->seedFeed('https://www.vrt.be/vrtnws/nl/', 'website', true, 'vrt'); + // Seed a belga row too, so the migration actually runs its updates + // rather than bailing out at the "no belga feed" guard. + $this->seedFeed(self::DEAD_RSS_URL, 'rss'); + + $this->runMigration(); + + $vrt = $this->findFeed($vrtId); + + $this->assertSame('https://www.vrt.be/vrtnws/nl/', $vrt->url); + $this->assertSame('website', $vrt->type); + $this->assertTrue((bool) $vrt->is_active); + } + + public function test_down_is_a_deliberate_no_op(): void + { + DB::table('feeds')->delete(); + $id = $this->seedFeed($this->targetUrl(), 'website'); + + $migration = require database_path('migrations/2024_01_01_000012_sync_belga_feed_to_website_discovery.php'); + $migration->down(); + + // Rolling back must not restore the retired RSS url. + $feed = $this->findFeed($id); + $this->assertSame($this->targetUrl(), $feed->url); + $this->assertSame('website', $feed->type); + } + + public function test_does_nothing_when_no_belga_feed_exists(): void + { + DB::table('feeds')->delete(); + + $this->runMigration(); + + $this->assertSame(0, DB::table('feeds')->count()); + } + + public function test_does_nothing_when_the_configured_url_is_missing(): void + { + DB::table('feeds')->delete(); + config(['feed.providers.belga.languages.en.url' => '']); + $id = $this->seedFeed(self::DEAD_RSS_URL, 'rss'); + + $this->runMigration(); + + $feed = $this->findFeed($id); + + $this->assertSame('rss', $feed->type); + $this->assertSame(self::DEAD_RSS_URL, $feed->url); + } +} diff --git a/tests/Unit/Services/Parsers/BelgaHomepageParserTest.php b/tests/Unit/Services/Parsers/BelgaHomepageParserTest.php index 7fd3b933..15b74342 100644 --- a/tests/Unit/Services/Parsers/BelgaHomepageParserTest.php +++ b/tests/Unit/Services/Parsers/BelgaHomepageParserTest.php @@ -4,7 +4,7 @@ use App\Services\Parsers\BelgaHomepageParser; use App\Services\Parsers\BelgaHomepageParserAdapter; -use PHPUnit\Framework\TestCase; +use Tests\TestCase; class BelgaHomepageParserTest extends TestCase { @@ -98,10 +98,14 @@ public function test_adapter_delegates_and_reports_source(): void $this->assertSame('Belga News Agency', $adapter->getSourceName()); } - public function test_adapter_uppercases_language_in_api_url(): void + public function test_adapter_url_comes_from_config(): void { - $this->assertStringContainsString('language=EN', (new BelgaHomepageParserAdapter('en'))->getHomepageUrl()); - $this->assertStringContainsString('language=NL', (new BelgaHomepageParserAdapter('nl'))->getHomepageUrl()); + config(['feed.providers.belga.languages.en.url' => 'https://example.test/pressreleases?count=7']); + + $this->assertSame( + 'https://example.test/pressreleases?count=7', + (new BelgaHomepageParserAdapter('en'))->getHomepageUrl() + ); } public function test_adapter_api_url_pins_load_bearing_query_params(): void @@ -111,6 +115,7 @@ public function test_adapter_api_url_pins_load_bearing_query_params(): void $this->assertStringStartsWith('https://capi.belga.press/belgapress/api/public/pressreleases?', $url); $this->assertStringContainsString('newsroomId=70', $url); $this->assertStringContainsString('offset=1', $url); - $this->assertStringContainsString('count=6', $url); + $this->assertStringContainsString('count=50', $url); + $this->assertStringContainsString('language=EN', $url); } }