Give routes a surrogate primary key #170

New issue

Open

opened 2026-09-26 16:07:32 +02:00 by myrmidex · 0 comments

myrmidex commented

2026-09-26 16:07:32 +02:00

Owner

Problem

routes has a composite primary key (feed_id, platform_channel_id) and no id. app/Models/Route.php:29 sets protected $primaryKey = null with $incrementing = false, and the comment concedes "Laravel doesn't handle composite primary keys well, so we'll use regular queries."

No polymorphic relation can target a route, which blocks the title rules in #155.

Scope — smaller than it appears

Investigated before filing:

All ~20 route lookups use where('feed_id', …)->where('platform_channel_id', …). Nothing uses find(), a route id, or route model binding. Adding id is additive — no call site changes.
route_articles' composite FK references (feed_id, platform_channel_id), which stays as a unique constraint. No route_id column, no backfill, no FK swap.
keywords FKs to feeds and platform_channels directly, not to routes. Unaffected.
RouteArticle::route() (app/Models/RouteArticle.php:56) is broken and dead. The relation is defined as:
```
return $this->belongsTo(Route::class, 'feed_id', 'feed_id')
    ->where('platform_channel_id', $this->platform_channel_id);
```
$this->platform_channel_id is evaluated when the relation is defined, not when the query runs, so it would misbehave under eager loading. Nothing calls it and no test covers it. Fix or delete.

The one real risk

Converting a composite PRIMARY KEY to a UNIQUE while adding an AUTO_INCREMENT id, with a foreign key depending on that index. MariaDB may refuse to drop the primary key while route_articles references those columns, forcing a drop-alter-re-add of the FK.

This is unverified — the dev database was unreachable when this ticket was written (see #169). It is why this ticket is sequenced after #169: on SQLite the migration would pass regardless of whether it works on MariaDB.

Acceptance criteria

routes.id exists and auto-increments; (feed_id, platform_channel_id) remains unique
Deleting a route still cascades to its route articles
Route no longer sets $primaryKey = null / $incrementing = false
Dead RouteArticle::route() relation resolved
The migration has a test, as FixBelgaFeedOffsetMigrationTest does for #158
Full suite green

#169 — MariaDB test suite, sequenced first
#155 — title rules, blocked on this

## Problem `routes` has a composite primary key `(feed_id, platform_channel_id)` and no `id`. `app/Models/Route.php:29` sets `protected $primaryKey = null` with `$incrementing = false`, and the comment concedes *"Laravel doesn't handle composite primary keys well, so we'll use regular queries."* No polymorphic relation can target a route, which blocks the title rules in #155. ## Scope — smaller than it appears Investigated before filing: - **All ~20 route lookups use `where('feed_id', …)->where('platform_channel_id', …)`.** Nothing uses `find()`, a route id, or route model binding. Adding `id` is **additive — no call site changes**. - **`route_articles`' composite FK references `(feed_id, platform_channel_id)`**, which stays as a unique constraint. **No `route_id` column, no backfill, no FK swap.** - **`keywords` FKs to `feeds` and `platform_channels` directly**, not to `routes`. Unaffected. - **`RouteArticle::route()` (`app/Models/RouteArticle.php:56`) is broken and dead.** The relation is defined as: ```php return $this->belongsTo(Route::class, 'feed_id', 'feed_id') ->where('platform_channel_id', $this->platform_channel_id); ``` `$this->platform_channel_id` is evaluated when the relation is *defined*, not when the query runs, so it would misbehave under eager loading. Nothing calls it and no test covers it. Fix or delete. ## The one real risk Converting a composite PRIMARY KEY to a UNIQUE while adding an AUTO_INCREMENT `id`, with a foreign key depending on that index. MariaDB may refuse to drop the primary key while `route_articles` references those columns, forcing a drop-alter-re-add of the FK. **This is unverified** — the dev database was unreachable when this ticket was written (see #169). It is why this ticket is sequenced after #169: on SQLite the migration would pass regardless of whether it works on MariaDB. ## Acceptance criteria - [ ] `routes.id` exists and auto-increments; `(feed_id, platform_channel_id)` remains unique - [ ] Deleting a route still cascades to its route articles - [ ] `Route` no longer sets `$primaryKey = null` / `$incrementing = false` - [ ] Dead `RouteArticle::route()` relation resolved - [ ] The migration has a test, as `FixBelgaFeedOffsetMigrationTest` does for #158 - [ ] Full suite green ## Related - #169 — MariaDB test suite, sequenced first - #155 — title rules, blocked on this