release/v1.4.2 #159
4 changed files with 245 additions and 2 deletions
|
|
@ -45,7 +45,8 @@
|
||||||
'type' => 'website',
|
'type' => 'website',
|
||||||
'is_active' => true,
|
'is_active' => true,
|
||||||
'languages' => [
|
'languages' => [
|
||||||
'en' => ['url' => 'https://capi.belga.press/belgapress/api/public/pressreleases?offset=1&count=50&search=&start=&end=&newsroomId=70&language=EN'],
|
// offset is a 0-based item index; offset=1 skips the newest release (#158).
|
||||||
|
'en' => ['url' => 'https://capi.belga.press/belgapress/api/public/pressreleases?offset=0&count=50&search=&start=&end=&newsroomId=70&language=EN'],
|
||||||
],
|
],
|
||||||
'parsers' => [
|
'parsers' => [
|
||||||
'homepage' => BelgaHomepageParserAdapter::class,
|
'homepage' => BelgaHomepageParserAdapter::class,
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,65 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
use Illuminate\Database\Migrations\Migration;
|
||||||
|
use Illuminate\Support\Facades\DB;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The Belga API offset parameter is 0-based, so the previous url's offset=1
|
||||||
|
* silently skipped the newest press release on every fetch (#158). The url in
|
||||||
|
* config/feed.php moved to offset=0; this migration updates the stored feed
|
||||||
|
* row to match.
|
||||||
|
*
|
||||||
|
* System feeds are maintained by the platform, not the end user, so their
|
||||||
|
* stored url is updated through migrations rather than the UI.
|
||||||
|
*
|
||||||
|
* Keyed on `provider`, never on the previous url: CreateFeedAction looks feeds
|
||||||
|
* up by url via firstOrCreate, so leaving the row on the old url while config
|
||||||
|
* points elsewhere would insert a second belga row on the next seed instead of
|
||||||
|
* updating the existing one.
|
||||||
|
*
|
||||||
|
* `feeds.url` is unique while `feeds.provider` is not, so a blanket update
|
||||||
|
* across several belga rows would collide. Adopt the row already on the target
|
||||||
|
* url (or the oldest belga row when none is) and deactivate the rest.
|
||||||
|
*
|
||||||
|
* Superseded rows are deactivated rather than deleted — routes.feed_id
|
||||||
|
* cascades on delete, so removing a feed would take its routing rules with it.
|
||||||
|
*/
|
||||||
|
return new class extends Migration
|
||||||
|
{
|
||||||
|
public function up(): void
|
||||||
|
{
|
||||||
|
$url = config('feed.providers.belga.languages.en.url');
|
||||||
|
|
||||||
|
if (! is_string($url) || $url === '') {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
$keepId = DB::table('feeds')->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([
|
||||||
|
'url' => $url,
|
||||||
|
'updated_at' => now(),
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function down(): void
|
||||||
|
{
|
||||||
|
// No rollback: restoring offset=1 would reinstate a feed that silently
|
||||||
|
// drops the newest press release.
|
||||||
|
}
|
||||||
|
};
|
||||||
176
tests/Feature/FixBelgaFeedOffsetMigrationTest.php
Normal file
176
tests/Feature/FixBelgaFeedOffsetMigrationTest.php
Normal file
|
|
@ -0,0 +1,176 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
namespace Tests\Feature;
|
||||||
|
|
||||||
|
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||||
|
use Illuminate\Support\Facades\DB;
|
||||||
|
use stdClass;
|
||||||
|
use Tests\TestCase;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The migration has already run by the time RefreshDatabase hands over, so
|
||||||
|
* these tests seed a pre-migration state and invoke up() against it directly.
|
||||||
|
*/
|
||||||
|
class FixBelgaFeedOffsetMigrationTest extends TestCase
|
||||||
|
{
|
||||||
|
use RefreshDatabase;
|
||||||
|
|
||||||
|
private const OFFSET_ONE_URL = 'https://capi.belga.press/belgapress/api/public/pressreleases?offset=1&count=50&search=&start=&end=&newsroomId=70&language=EN';
|
||||||
|
|
||||||
|
private function runMigration(): void
|
||||||
|
{
|
||||||
|
$migration = require database_path('migrations/2024_01_01_000024_fix_belga_feed_offset.php');
|
||||||
|
|
||||||
|
$migration->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, bool $isActive = true, string $provider = 'belga', string $type = 'website'): 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_moves_a_feed_still_on_the_offset_one_url(): void
|
||||||
|
{
|
||||||
|
DB::table('feeds')->delete();
|
||||||
|
$id = $this->seedFeed(self::OFFSET_ONE_URL);
|
||||||
|
|
||||||
|
$this->runMigration();
|
||||||
|
|
||||||
|
$feed = $this->findFeed($id);
|
||||||
|
|
||||||
|
$this->assertSame($this->targetUrl(), $feed->url);
|
||||||
|
$this->assertSame('website', $feed->type);
|
||||||
|
$this->assertTrue((bool) $feed->is_active);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_is_idempotent_when_the_feed_is_already_on_the_target_url(): void
|
||||||
|
{
|
||||||
|
DB::table('feeds')->delete();
|
||||||
|
$id = $this->seedFeed($this->targetUrl());
|
||||||
|
|
||||||
|
$this->runMigration();
|
||||||
|
$this->runMigration();
|
||||||
|
|
||||||
|
$this->assertSame(1, DB::table('feeds')->where('provider', 'belga')->count());
|
||||||
|
|
||||||
|
$feed = $this->findFeed($id);
|
||||||
|
$this->assertSame($this->targetUrl(), $feed->url);
|
||||||
|
$this->assertTrue((bool) $feed->is_active);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_deactivates_superseded_rows_instead_of_colliding_on_the_unique_url(): void
|
||||||
|
{
|
||||||
|
DB::table('feeds')->delete();
|
||||||
|
$staleId = $this->seedFeed(self::OFFSET_ONE_URL);
|
||||||
|
$currentId = $this->seedFeed($this->targetUrl());
|
||||||
|
|
||||||
|
$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);
|
||||||
|
|
||||||
|
$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::OFFSET_ONE_URL);
|
||||||
|
$newerId = $this->seedFeed('https://capi.belga.press/belgapress/api/public/pressreleases?offset=1&count=6');
|
||||||
|
|
||||||
|
$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->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/', 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::OFFSET_ONE_URL);
|
||||||
|
|
||||||
|
$this->runMigration();
|
||||||
|
|
||||||
|
$vrt = $this->findFeed($vrtId);
|
||||||
|
|
||||||
|
$this->assertSame('https://www.vrt.be/vrtnws/nl/', $vrt->url);
|
||||||
|
$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());
|
||||||
|
|
||||||
|
$migration = require database_path('migrations/2024_01_01_000024_fix_belga_feed_offset.php');
|
||||||
|
$migration->down();
|
||||||
|
|
||||||
|
// Rolling back must not restore the offset=1 url that drops the newest
|
||||||
|
// press release.
|
||||||
|
$feed = $this->findFeed($id);
|
||||||
|
$this->assertSame($this->targetUrl(), $feed->url);
|
||||||
|
$this->assertTrue((bool) $feed->is_active);
|
||||||
|
}
|
||||||
|
|
||||||
|
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::OFFSET_ONE_URL);
|
||||||
|
|
||||||
|
$this->runMigration();
|
||||||
|
|
||||||
|
$feed = $this->findFeed($id);
|
||||||
|
|
||||||
|
$this->assertSame(self::OFFSET_ONE_URL, $feed->url);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -114,7 +114,8 @@ public function test_adapter_api_url_pins_load_bearing_query_params(): void
|
||||||
|
|
||||||
$this->assertStringStartsWith('https://capi.belga.press/belgapress/api/public/pressreleases?', $url);
|
$this->assertStringStartsWith('https://capi.belga.press/belgapress/api/public/pressreleases?', $url);
|
||||||
$this->assertStringContainsString('newsroomId=70', $url);
|
$this->assertStringContainsString('newsroomId=70', $url);
|
||||||
$this->assertStringContainsString('offset=1', $url);
|
// 0-based item index: offset=1 skipped the newest release entirely (#158).
|
||||||
|
$this->assertStringContainsString('offset=0&', $url);
|
||||||
$this->assertStringContainsString('count=50', $url);
|
$this->assertStringContainsString('count=50', $url);
|
||||||
$this->assertStringContainsString('language=EN', $url);
|
$this->assertStringContainsString('language=EN', $url);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue