# Laravel Upgrade Log

Tracks the incremental Laravel 9 → 13 upgrade (see the audit discussion for
the full package-compatibility research). Recorded per hop: files touched,
why, and what was verified before moving on.

Target sequence: **9 → 10 → 11 → 12 → 13**, one commit per hop.

---

## Hop 1: Laravel 9 → 10

**Commit:** `2a6f3662` — "Upgrade Laravel 9 -> 10"

### Files touched

| File | Change | Why |
|---|---|---|
| `composer.json` | `laravel/framework` ^9→^10, `laravel/sanctum` ^2.7→^3.2, `laravel/ui` ^3→^4, `spatie/laravel-ignition` ^1→^2, `nunomaduro/collision` ^6→^7, `phpunit/phpunit` ^9→^10 | Laravel 10's own required dependency versions (per the official upgrade guide) |
| `composer.json` | Removed `doctrine/dbal` | Laravel no longer needs it by v11 anyway, and a full-repo grep found zero usage in this app (no `Doctrine` references, no `->change()` calls in migrations) — safe to drop now instead of carrying it forward |
| `composer.json` | Removed `fideloper/proxy`, `fruitcake/laravel-cors` | Both packages are abandoned; Laravel has shipped native equivalents (`TrustProxies`, `HandleCors`) since Laravel 7/9. Not required by the L10 upgrade itself, but bundled in since it touches the same `Kernel.php`/middleware files |
| `composer.json` | Swapped `fzaninotto/faker` → `fakerphp/faker` | `fzaninotto/faker` is abandoned (flagged by the IDE); `fakerphp/faker` is the maintained fork, same `Faker\...` namespace, no code changes needed in factories/seeders |
| `composer.lock` | Regenerated | Result of the above `composer update` |
| `app/Http/Middleware/TrustProxies.php` | `use Fideloper\Proxy\TrustProxies as Middleware;` → `use Illuminate\Http\Middleware\TrustProxies as Middleware;` | Points the custom `TrustProxies` middleware at Laravel's native class now that `fideloper/proxy` is removed |
| `app/Http/Kernel.php` | `\Fruitcake\Cors\HandleCors::class` → `\Illuminate\Http\Middleware\HandleCors::class` | Same reason — native CORS middleware now that `fruitcake/laravel-cors` is removed. `config/cors.php` needed no changes; Laravel's native middleware reads the same config file |
| `phpunit.xml` | Removed `processUncoveredFiles="true"` from `<coverage>` | Required for PHPUnit 10 (removed attribute) |

### Verified before committing
- `php artisan --version` → `10.50.3`
- `php artisan route:list` → all 152 routes resolve, no middleware pipeline errors
- `vendor/bin/phpunit` → 2/2 stub tests pass
- Booted `artisan serve`, confirmed `/api/version` and `/api/release` respond
- CORS preflight (`OPTIONS` with `Origin` header) returns correct `Access-Control-Allow-*` headers via the native `HandleCors` middleware
- `GET /sanctum/csrf-cookie` returns `XSRF-TOKEN` + session cookies correctly under Sanctum 3.3.3
- Fetched a real poll end-to-end through the `hashids` middleware (`GET /api/poll/{hashid}/{adminKey}`) against the local dev DB — full payload with nested choices/participants/comments/design, all IDs correctly hashid-encoded on the way out
- `composer audit` — flagged pre-existing `phpoffice/phpspreadsheet` 1.30.5 advisories (unrelated to this hop, untouched dependency; will resolve when it's bumped to ^5.3 during the maatwebsite/excel v4 migration) and two `laravel/framework` advisories affecting versions below the eventual 12.60/13.10 targets — tracked, not actioned yet

### Notable finding (not a regression)
While testing the poll-fetch endpoint, passing the poll's raw `key` DB
column as the `{key}` URL segment threw a `ConverterException` (it isn't a
valid hashids string). Passing `poll->getAdminkey()` instead works
correctly. This answers open question #5 in `Design.md` ("Ist das
API-Feld adminKey dasselbe wie das URL-Segment?") — no, the `{key}` route
segment is `getAdminkey()`, not the raw `key` column. Confirmed via code
reading, not caused by anything in this hop.

---

## Hop 2: Laravel 10 → 11

**Commit:** not yet committed.

### Files touched

| File | Change | Why |
|---|---|---|
| `composer.json` | `laravel/framework` ^10→^11, `laravel/sanctum` ^3.2→^4.0, `nunomaduro/collision` ^7→^8.1 | Laravel 11's required dependency versions (per the official upgrade guide) |
| `composer.lock` | Regenerated | Result of `composer update`; also pulled in Carbon 3 (nesbot/carbon 2.73→3.13) as a Laravel 11 transitive requirement |
| `config/sanctum.php` | Renamed `verify_csrf_token` → `validate_csrf_token`; added new `authenticate_session` key (`Laravel\Sanctum\Http\Middleware\AuthenticateSession::class`); kept `encrypt_cookies`/CSRF pointing at the app's own custom middleware classes (not swapped to generic Illuminate ones) | Sanctum 4 renamed/added these config keys (per the L11 upgrade guide); the app's own `App\Http\Middleware\{EncryptCookies,VerifyCsrfToken}` still work fine since they just extend Illuminate's base classes |
| `app/Http/Controllers/ParticipantController.php` | 5 call sites: `$now->diffInMinutes(...)` → `abs($now->diffInMinutes(...))` (lines 80, 99, 193, 402, 522) | **Real regression caught by testing, not a hypothetical.** Carbon 3 changed `diffIn*` to return signed floats instead of Carbon 2's always-positive default. Verified empirically: `$now->diffInMinutes($past)` now returns `-10` instead of `+10`. This value feeds `if ($isFirstAnswer \|\| $diffInMins > 5)`, which gates whether an admin gets notified of a participant's answer change — with the sign flip, `$diffInMins > 5` became permanently `false`, silently killing that notification path in all 5 places. `abs()` restores the original behavior |
| `app/Console/Commands/SyncMollieInvoices.php` | `$paymentTime->diffInHours($subStartTime) <= 24` → wrapped in `abs()` | Same class of bug: this gates whether a Mollie payment is treated as a subscription's "initial payment" (must be within 24h of `start_date`). Traced through the new signed semantics: payments *before* start_date still worked correctly by coincidence, but *any* payment after start_date — regardless of how much later — was incorrectly matching, since the unwrapped result was always negative and `negative <= 24` is always true |
| `app/Console/Commands/AutomatedReminder.php` | `$startTime->diffInSeconds(now())` → wrapped in `abs()` | Cosmetic only (feeds a "Duration: X seconds" log line, no branching logic depends on it) — fixed for correctness/clean output while already in the area |

### Verified before committing
- `php artisan --version` → `11.56.0`
- `vendor/bin/phpunit` → 2/2 stub tests pass
- `php artisan migrate:status` against the real dev DB — all migrations show `Ran`, confirms DB connectivity survived
- Booted `artisan serve`, re-ran the same live checks as hop 1: CORS headers correct, `/sanctum/csrf-cookie` issues `XSRF-TOKEN`/session cookies correctly under Sanctum 4.3.3, real poll fetch via the hashids middleware returns 200
- Empirically reproduced the Carbon 3 sign-flip via `tinker` before deciding on the fix, rather than assuming from the changelog wording

### Not done this hop (deferred, no action needed yet)
- Sanctum's `personal_access_tokens` migration was already present in `database/migrations/` from a prior manual publish — `vendor:publish --tag=sanctum-migrations` wasn't needed
- 4 migrations use `->change()` (`update_participant_attributes_table`, `update_participant_values_table`, `change_options_size_in_polls_table`, `update_description_column_in_polls_table`) — Laravel 11's stricter "must re-declare all modifiers" semantics only affects *new* `->change()` migrations going forward, not these already-applied ones. No action needed unless a new column-modifying migration is added

## Side quest: intervention/image v2 → v3

Done independently of the Laravel version hops (v3 dropped its Laravel
coupling entirely, so nothing gates this on which Laravel version is
running). Not yet committed.

### Files touched

| File | Change | Why |
|---|---|---|
| `composer.json`/`lock` | `intervention/image` ^2.7 → ^3.0 (resolved to 3.11.8, pulled in `intervention/gif` 4.2.4 as a new transitive dep) | v3 major upgrade |
| `config/app.php` | Removed `Intervention\Image\ImageServiceProvider::class` from `providers` and `'Image' => Intervention\Image\Facades\Image::class` from `aliases` | v3 dropped its own Laravel service provider/facade entirely (framework-agnostic now); without removing these, `composer install`'s `package:discover` step fails with `Class "Intervention\Image\ImageServiceProvider" not found`. Deliberately did **not** install `intervention/image-laravel` to get a replacement facade back — Laravel 13.20+ ships its own image integration that facade-name-collides with that package, so using `ImageManager` directly sidesteps it for good |
| `app/Http/Controllers/UserController.php` | Replaced `use Intervention\Image\Facades\Image;` with `use Intervention\Image\ImageManager;`; all 6 effective call sites (`imageUpload()` ×5, `logoUpload()` ×1) rewritten from `Image::make($file)->encode(...)->save($path, $quality)` to `ImageManager::gd()->read($file)->save($path, quality: $quality)` | v3 has no static facade by default; driver-based `ImageManager` is the v3-native API |

### A real finding, resolved with certainty (not left as an open question)
Previously flagged this as an ambiguity — v2's `Image::make($file)->encode('jpg', 5)->save($path, 25)` sets two different quality values (5, then 25), unclear which took effect. **Read v2's actual `Image::save()` source before porting**: it calls `$this->encode($format, $quality)` again internally using its own `$quality` argument, unconditionally overriding whatever `encode()` had set earlier. So the `save()` argument always won — the `encode('jpg', X)` calls were dead code the whole time. Effective qualities preserved in the v3 port: 25, 45, 25, 50, 25 (`imageUpload`), 50 (`logoUpload`) — all taken from the `save()` calls, matching actual prior behavior exactly, not a new decision.

### Verified before committing
- `php -l` on the touched file — clean
- `vendor/bin/phpunit` → 2/2 pass
- `composer dump-autoload` / `package:discover` succeeds cleanly after the provider/alias removal
- **Isolated logic test** (these endpoints require Sanctum auth, so a full HTTP multipart-upload test wasn't the efficient path): generated real JPEG/PNG test images and ran the *exact* ported `ImageManager::gd()->read($file)->save($path, quality: X)` calls against them via `tinker`, for all 3 quality values used in the app (25, 45, 50, plus 90 as a control). Output files are valid JPEGs at the correct dimensions, and file size scales monotonically with the quality argument (29.5KB → 44KB → 46KB → 102KB for quality 25/45/50/90) — confirms `quality:` is actually being honored, not silently ignored. (First attempt used a flat-color test image where all quality levels produced identical byte sizes — not a bug, just too little visual detail to compress differently; redid it with a high-entropy noise image to get a real signal.)
- Booted `artisan serve`, confirmed `/api/release` and the hashids-based poll fetch still return correctly (checks nothing else broke from the `config/app.php` provider/alias removal)

---

## Hop 3: Laravel 11 → 12

**Commit:** not yet committed.

### Files touched

| File | Change | Why |
|---|---|---|
| `composer.json` | `laravel/framework` ^11→^12, `phpunit/phpunit` ^10→^11, `nunomaduro/collision` ^8.1→^8.9 (auto-resolved; 8.5.0 turned out incompatible with any 12.x), `maatwebsite/excel` ^3.1→^4.0, `phpoffice/phpspreadsheet` ^1.21→^5.8 | Laravel 12's required versions, plus the mandatory excel/phpspreadsheet jump (3.1.x is simply uninstallable on Laravel 12) |
| `composer.json` | `config.platform.php` `8.2.28` → `8.3.11` | **Blocker, not cosmetic.** This stale pin (left over from the original project scaffold, never matched the real interpreter) made Composer's resolver believe PHP was 8.2 rather than the actually-installed 8.3.11, which caused `maatwebsite/excel ^4.0` (needs PHP ^8.3) to fail resolution entirely until fixed |
| `composer.lock` | Regenerated | Result of the above. Also fully cleared 3 pre-existing high-severity `phpoffice/phpspreadsheet` CVEs (memory-exhaustion DoS via crafted XLS/Gnumeric files, SSRF bypass) that were sitting on the old 1.30.5 the whole time — `composer audit` went from 3 findings to 0 |
| `app/Imports/{ContactsImport,ContactFromFileImport,UsersImport,UsersUpdate}.php` | `public function collection(Collection $rows)` → `public function collection(Collection $rows): void` | v4's `ToCollection` interface declares `collection(): void` explicitly (verified against the installed interface source, not assumed) — implementations must match |
| `app/Exports/UsersExport.php` | Deleted | Confirmed dead code in the original audit (never instantiated anywhere in the repo) — not worth porting through a major rewrite |

### The other real risk: PollExportService.php
This file never touches Maatwebsite's code — it's 450 lines of **raw PhpSpreadsheet** calls (`Spreadsheet`, `Xlsx` writer, `setCellValue`, `getCell()->setValueExplicit()`, `getStyle()->getFill()->setFillType()->getStartColor()->setRGB()`, `applyFromArray`, `DataType::TYPE_STRING`) — but it rode along on the forced 1.30→5.9 phpspreadsheet bump (4 major versions) since excel v4 pulls that in as a shared dependency. No code changes were needed here, but this got the most thorough testing in the hop (see below) since it's the actual paid poll-export feature, not just an internal admin tool.

### Verified before committing
- `php artisan --version` → `12.67.0`
- `composer audit` → 0 advisories (down from 3)
- `vendor/bin/phpunit` → 2/2 pass
- **Live end-to-end export test**: POSTed to the real `/api/download-excel/{id}` route (no auth required) against poll 118991 in the local dev DB. Got back a real file path, downloaded it, confirmed via `file` that it's a genuine "Microsoft Excel 2007+" archive (not corrupt), then **round-tripped it through `PhpOffice\PhpSpreadsheet\IOFactory::load()`** and read the cells back — title, poll admin URL, timezone, and "Name"/"Email" headers all came back exactly as expected. This validates the actual write+read cycle under PhpSpreadsheet 5.9, not just "did it not crash"
- **Live import test**: rather than fight Sanctum auth on `/contacts/upload`, ran `ContactsImport` through the real `Excel::import()` facade call (the same call path `ContactController` uses) against a real CSV, inside a DB transaction that was rolled back afterward — confirmed it created a `Contact` row with the right email/name, then discarded the change so the dev DB wasn't polluted
- Re-ran the standing regression checks from every prior hop: CORS headers, hashids-based poll fetch — still 200

### Deployment prerequisite — blocked and resolved on `65.21.184.46`
`maatwebsite/excel ^4.0` (and its `intervention/gif` dependency) require PHP
`>=8.3.0`. Local dev is on 8.3.11 so this was invisible there, but the VPS at
`65.21.184.46` was running PHP 8.2.28 and failed `composer install` on
Composer's `platform_check.php` gate. Server had PHP 8.4 already available
via `update-alternatives` (no 8.3) — switched to that for both the CLI
binary and the web-serving SAPI, which cleared the version gate.

Next: PHP 8.4 was a fresh install on that box and was missing `ext-curl`
(required by `mollie/mollie-api-php` — payments) and `ext-gd` (required by
`phpoffice/phpspreadsheet` and `intervention/image`'s GD driver — poll
export and image/logo upload). Fixed with `apt install php8.4-curl
php8.4-gd` + restart the PHP SAPI. Deliberately did **not** use Composer's
`--ignore-platform-req` escape hatch here — these are real runtime
dependencies of this hop's work, not resolver formalities; ignoring them
would have swapped a blocked deploy for fatal errors on first real use of
payments/export/image-upload in production.

Hops 1–2 (Laravel 10/11) only needed PHP 8.1/8.2, so those were deployable
independently of any of this.

**Follow-up:** `65.21.184.46` was on Ubuntu 20.04 (focal), which turned out
to be the real blocker underneath the PHP one — `focal`'s system libraries
are too old for `ondrej/php`'s `curl`/`gd` extension builds on PHP 8.3 *or*
8.4 (confirmed symmetrically missing for both versions before concluding
this). Fixed via `do-release-upgrade` to Ubuntu 22.04 (jammy), after which
the full extension set became available. Server ended up on **PHP 8.4**
(8.3 wasn't available via `update-alternatives` on this box) — satisfies
Laravel 12's `^8.2` floor and `maatwebsite/excel`'s `>=8.3.0` floor, and is
in fact the version Laravel's own installer defaults new Laravel 12 apps to.

**Testing gap to flag for QA**: every live verification in this hop (poll
export round-trip, CORS/Sanctum/hashids checks, image-upload quality math,
contact import) ran locally against PHP 8.3.11, not 8.4. Not expected to be
a compatibility problem, but it is a genuinely unverified delta — the team
should re-run the QA checklist against the real PHP 8.4 server rather than
assume the 8.3 results carry over unchanged, and watch `storage/logs/laravel.log`
for new deprecation noise (PHP 8.4 tightened enforcement of things like
implicitly-nullable parameters, which this codebase has plenty of).

### Checked and cleared, no action needed
- `image` validation rule SVG exclusion (Laravel 12 change) — only reference in the whole app is a commented-out, unexecuted line in `UserController.php`
- `config/filesystems.php` local-disk default root path change — the app already explicitly defines the `local` disk (`storage_path('app')`), so Laravel 12's "defaults to `storage/app/private` if undefined" change doesn't apply

## Hop 4: Laravel 12 → 13

**Commit:** `ba0d2a84` — "laravel-13" (composer.json/lock, phpunit.xml schema migration, TESTING-BACKLOG.md); test suite additions in `e16c1cc2` and CORS fix below on top.

### Files touched

| File | Change | Why |
|---|---|---|
| `composer.json` | `php` ^8.2→^8.3, `laravel/framework` ^12.0→^13.0, `laravel/tinker` ^2.0→^3.0 | L13's PHP floor is 8.3; tinker 2.x caps at Illuminate ^12, so it needed the major bump to 3.x to support L13 |
| `composer.lock` | Regenerated | Resolved cleanly to `laravel/framework v13.29.0` with Sanctum, Socialite, Maatwebsite Excel, Mollie, phpspreadsheet, Ignition all compatible — no conflicts, `composer audit` clean |
| `phpunit.xml` | Migrated to the current PHPUnit 11 schema (`--migrate-configuration`); uncommented `DB_CONNECTION=sqlite` / `DB_DATABASE=:memory:` | Schema warning was cosmetic, but the sqlite lines were a real gap — they'd been commented out, meaning any `RefreshDatabase` test would have run against the real MySQL dev DB (`DB_DATABASE=kulibri-backup` in `.env`) instead of an isolated in-memory DB |
| `config/cors.php` | `allowed_origins: ['*']` → `array_map('trim', explode(',', env('CORS_ALLOWED_ORIGINS', env('KULIBRI_FRONTEND_URL', 'http://localhost:8080'))))` | The `['*']` + `supports_credentials: true` combo is invalid per the CORS spec. With `fruitcake/php-cors` (the library backing Laravel's `HandleCors`), this combination is normally rejected, but the single-origin code path (`configureAllowedOrigin()`) unconditionally emits whatever's configured — the real risk was that a future switch to an origin-reflecting config would trust every origin with the user's session cookie. Defaults to the already-per-environment-configured `KULIBRI_FRONTEND_URL` (`config('app.frontend_url')`, used elsewhere in `config/app.php`/`routes/api.php`) rather than requiring a brand-new Ansible-managed var — `CORS_ALLOWED_ORIGINS` remains available as an explicit override for cases needing more than one trusted origin at once (e.g. a domain migration) |
| `.env.example` | Documented `CORS_ALLOWED_ORIGINS` as optional, defaulting to `KULIBRI_FRONTEND_URL` | No Ansible change needed per environment unless multiple origins are required |

### CSRF `Sec-Fetch-Site` — reviewed, no code change needed
L13 renames the CSRF middleware to `PreventRequestForgery`, which skips
token validation when the browser sends `Sec-Fetch-Site: same-origin`.
`laravel/framework#59431` reports this as a bypass, but it was closed
"not planned" by the maintainers — correctly: `Sec-Fetch-Site: same-origin`
is a browser-enforced guarantee the request isn't cross-site, which is
exactly what CSRF protection exists to prevent in the first place. Not
exploitable via a real cross-site attack against Kulibri's cookie-based
Sanctum SPA auth. No action taken.

### Verified before committing
- `php artisan --version` → `13.29.0`
- `php artisan route:list` → all routes resolve, no middleware pipeline errors
- `vendor/bin/phpunit` / `php artisan test` → all tests pass
- `config('cors.allowed_origins')` resolves to `['http://localhost:8080']` from `.env` after `config:clear`
- New regression coverage: [`tests/Feature/CorsTest.php`](tests/Feature/CorsTest.php) (preflight from the configured origin gets `Access-Control-Allow-Origin`/`Access-Control-Allow-Credentials`; an untrusted origin's own value is never reflected back; config can never regress to a bare `['*']` alongside `supports_credentials: true`)