# 09 – Qualitätsreview: Bisherige Umsetzung

**Datum:** 2026-04-27
**Reviewer:** Claude Code (KI-gestütztes Code-Review)
**Geprüfte Phasen:** P0, P1, P2, P3 (laut `08-PROGRESS.md`)

> **Status-Hinweis 2026-04-29:** Dieses Review ist ein historischer Befund vom 2026-04-27. Die kritischen Punkte aus §2 und mehrere Punkte aus §3 wurden laut `08-PROGRESS.md` und Code-Abgleich inzwischen behoben oder teilweise umgesetzt (u. a. `EnsureUserIsAdmin`, `SetCurrentPortal`/`PortalScope`, Magic-Link-Härtung, Factories, Billing-Address-Bugfix, `contact_user`-Import). Für den aktuellen operativen Status siehe `03-MIGRATION-PLAN.md` und `08-PROGRESS.md`; dieses Dokument bleibt als Review-Quelle erhalten.

Dieses Dokument bewertet die bisherige Implementierung nach vier Dimensionen:
1. Code-Qualität & Laravel-Konventionen
2. Datenmigrations-Bereitschaft (Legacy-Import)
3. Admin-UI/UX
4. Offene Risiken & Prioritäten

**Legende:** 🔴 kritisch · 🟡 sollte korrigiert werden · 🟢 gut / verbesserbar · ✅ keine Beanstandung

---

## 1. Was gut gemacht wurde

Bevor die Probleme beschrieben werden, eine ehrliche Bestandsaufnahme der Stärken:

- **Saubere Enum-Nutzung** (`Portal`, `PressReleaseStatus`, etc.) – konsequent in Migrations, Casts und Validation eingesetzt.
- **Migrations sind gut strukturiert** – jede Tabelle hat `deleted_at`, `portal`, `legacy_portal/id` wo nötig; `down()` ist vollständig implementiert.
- **MagicLink-Implementierung** – korrekt: Token-Hash in DB, Single-Use-Flag, TTL, Anti-Enumeration bei unbekannter E-Mail.
- **Idempotente Seeder** – `firstOrCreate` / `updateOrCreate` durchgängig; `PaymentOptionSeeder` mit DE/EN-Translations.
- **`UserRolePermissionSyncService`** – gute Vorbereitung, damit der spätere Legacy-Import denselben Rollenmechanismus nutzt.
- **Validation in User-Edit** – vollständig, typsicher, nutzt `Rule::exists()`/`Rule::unique()`.
- **Live-Suche mit Limits** – Firmenlookup und Kontaktlookup ab 2 Zeichen, max. 40 Treffer, serverseitig – performant und korrekt.
- **PHPDoc `@var` Array-Shape-Typen** – z. B. `contactForms` im User-Edit korrekt annotiert.
- **Tests vorhanden** für alle P3-Features (User-CRUD, Company-CRUD, Kontakt-CRUD, Filter-Presets, etc.).

---

## 2. Kritische Probleme (🔴)

### 2.1 Admin-Middleware fehlt: Jeder eingeloggte User kann Admin-Bereich nutzen

**Datei:** `routes/admin.php`

```php
Route::middleware(['auth'])->group(function () {
    Volt::route('admin/users', ...);
    Volt::route('admin/roles', ...);   // Rollenverwaltung!
    // ...
});
```

Alle Admin-Routen sind nur mit `auth` geschützt – **nicht** mit einer Rolle-/Permission-Prüfung. Ein frisch registrierter Customer-User kann `/admin/roles` aufrufen und Rollen bearbeiten.

**Fix:** Middleware oder Gate-Checks auf der Routengruppe.

```php
Route::middleware(['auth', 'verified', function ($request, $next) {
    abort_unless($request->user()?->canAccessAdmin(), 403);
    return $next($request);
}])->group(function () {
    // ...
});
```

Alternativ ein dediziertes `EnsureUserIsAdmin`-Middleware.

---

### 2.2 Kein Portal-Scoping: Mandantentrennung existiert nicht

**Geplant:** Phase 2.5 – `SetCurrentPortal`-Middleware + `CurrentPortalScope` (Global Scope)
**Status:** ⬜ Nicht implementiert

Alle Admin-Queries liefern aktuell Daten beider Portale unkontrolliert. Ein `admin`-User von `presseecho` sieht alle Firmen, Kontakte, PMs von `businessportal24` – und umgekehrt. Das ist für ein Produktivsystem inakzeptabel.

Der Global Scope müsste bei allen Multi-Portal-Modellen (Company, Contact, PressRelease, NewsletterSubscription, etc.) automatisch nach dem aktiven Portal filtern.

**Auswirkung auf bestehenden Code:** Nach Einführung des Scopes werden Tests und Queries, die aktuell portalübergreifend arbeiten, Ergebnisse verlieren. Das ist ein Breaking Change mit breiter Wirkung – deshalb **muss P2.5 vor weiteren P3-Implementierungen** erfolgen.

---

### 2.3 `persistUserLinks()` ohne Authorization-Check: N+1 bei jedem Klick

**Datei:** `resources/views/livewire/admin/users/edit.blade.php`, Zeile ~540

`persistUserLinks()` wird bei jedem Klick auf „Zuordnen"/„Entfernen" ausgeführt. Darin:
1. `User::query()->find($this->id)` – neuer DB-Query
2. `$user->companies()->sync(...)` – Pivot-Tabelle schreiben
3. `$user->contacts()->sync(...)` – Pivot-Tabelle schreiben

Es gibt **keinen Check**, ob der aktuell eingeloggte Admin tatsächlich diesen User bearbeiten darf. In einem Multi-Portal-Setup könnten Portal-A-Admins damit User von Portal B manipulieren.

Zusätzlich: `sync()` auf Kontakten schreibt **alle** `linkedContactIds` inklusive IDs, die nicht zu den verlinkten Firmen gehören – es gibt keinen serverseitigen Konsistenz-Check.

---

### 2.4 `MagicLinkConsumeController`: N+1 + Lazy-Load auf ungeprüftem Objekt

**Datei:** `app/Http/Controllers/Auth/MagicLinkConsumeController.php`

```php
$magicLink = MagicLink::query()->where(...)->first();

// Zeile 28: $magicLink->user->is_active  ← eager load fehlt
// Zeile 37: $magicLink->user->update(...) ← zweite Abfrage
// Zeile 42: Auth::guard('web')->login($magicLink->user) ← dritte Abfrage
```

Der User wird dreimal separat geladen. Mit `->with('user')->first()` wäre es eine Abfrage.

Außerdem: Wenn der User gelöscht ist (SoftDelete), gibt `$magicLink->user` `null` zurück, aber es gibt keinen `null`-Check – das führt zu einem `TypeError` statt einem sicheren Redirect.

**Fix:**
```php
$magicLink = MagicLink::query()
    ->with('user')
    ->where('token_hash', hash('sha256', $token))
    ->where('purpose', 'login')
    ->first();

if (! $magicLink || ! $magicLink->user || ! $magicLink->user->is_active) { ... }
```

---

### 2.5 Doppelte Permission-Zuweisung ist ein Anti-Pattern

**Datei:** `app/Services/Auth/UserRolePermissionSyncService.php`

```php
$user->assignRole($role);
$user->syncPermissions(
    $user->getPermissionsViaRoles()->pluck('name')->all()
);
```

Spatie Permissions ist so konzipiert, dass Rollen die Permissions tragen – `$user->can('...')` prüft automatisch über Rollen. Die **direkte Zuweisung derselben Permissions an den User** (zusätzlich zur Rolle) schafft Inkonsistenzen:

- Bei Änderung der Permissions einer Rolle bleiben die direkt zugewiesenen User-Permissions unverändert.
- `$user->getAllPermissions()` enthält Duplikate.
- Der Legacy-Import-Command muss diesen Doppelmechanismus kennen – fehleranfällig.

**Empfehlung:** Direkte Permission-Zuweisung an User komplett entfernen. Ausschließlich über Rollen arbeiten. Das entspricht dem Spatie-Standardmodell.

---

## 3. Sollte korrigiert werden (🟡)

### 3.1 Billing-Address-Update kann Felder nicht leeren

**Datei:** `resources/views/livewire/admin/users/edit.blade.php`, Zeile ~398–414

```php
$mergedBillingPayload = array_merge(
    $billingAddress->only([...]),
    collect($billingPayload)
        ->filter(static fn ($value) => filled($value))  // ← Problem
        ->all()
);
```

`filter(fn ($v) => filled($v))` entfernt alle leeren/null-Werte aus dem Payload. Das bedeutet: Hat ein Admin-User eine `address2` gesetzt und will sie wieder löschen (Feld leer lassen), bleibt der alte Wert stehen. Felder können einmal gesetzt nicht mehr entfernt werden.

**Fix:** Statt `filled()` nur `null !== $value` prüfen, oder den Payload direkt ohne Merge verwenden.

---

### 3.2 Drei Migrations für eine Feature-Idee am selben Tag

**Dateien:**
- `2026_04_24_120000_create_user_filter_presets_table.php`
- `2026_04_24_121500_add_is_default_to_user_filter_presets_table.php`
- `2026_04_24_123000_add_last_used_at_to_user_filter_presets_table.php`

Drei Migrations innerhalb von 90 Minuten für dasselbe Feature sind ein Zeichen dafür, dass die Tabelle nicht ausreichend vorab modelliert wurde. Solange die Tabelle noch nicht in Produktion ist, sollten diese zu **einer einzigen Migration** zusammengeführt werden.

Dies ist vor dem Go-Live wichtig, da viele kleine Patch-Migrations die `php artisan migrate` Ausführungszeit und Fehlerdiagnose erschweren.

---

### 3.3 Fehlende Factories für 12+ Modelle

Aktuell haben nur 8 Modelle Factories (`User`, `BillingAddress`, `InvoiceBillingAddress`, `Invoice`, `PaymentOption`, `PaymentOptionTranslation`, `UserPaymentOption`, `UserPayment`).

Folgende Modelle haben **keine Factory** und sind im Testcode mit rohen `Model::query()->create()` Calls aufgerufen:
- `Company`, `Contact`, `Profile`, `Category`, `CategoryTranslation`
- `PressRelease`, `PressReleaseImage`, `NewsletterSubscription`
- `MagicLink`, `UserFilterPreset`

In `UserManagementTest.php` sieht man z.B.:
```php
$companyA = Company::query()->create([
    'name' => 'Alpha GmbH',
    'slug' => 'alpha-gmbh',
]);
```

Das ist fragil: Wenn die `companies`-Tabelle ein NOT-NULL-Feld ohne Default bekommt, brechen alle Tests. Mit Factories werden die Defaults vom Framework verwaltet.

---

### 3.4 `contact_user` Pivot-Tabelle ist nicht im Datenmodell (04-DATA-MODEL.md)

Die Tabelle `contact_user` (viele-zu-viele User ↔ Kontakte) wurde in P3 eingeführt, ist aber in der Datenmodell-Dokumentation nicht beschrieben. Das schafft zwei Probleme:

1. **Legacy-Import (Phase 6) weiß nicht, dass diese Tabelle befüllt werden muss.** Der Import füllt Kontakte über `company_id` – die expliziten User-Kontakt-Verknüpfungen in `contact_user` müssen separat abgeleitet werden (welcher User hatte welche Kontakte?).

2. **Dual-Relationship-Komplexität:** Kontakte sind jetzt sowohl über `company_user` (indirekt via Firma) als auch direkt über `contact_user` mit Usern verknüpft. Der Fallback-Code in `mount()` (`if ($this->linkedContactIds === [])`) zeigt, dass die Logik bereits jetzt nicht trivial ist.

**Empfehlung:** `04-DATA-MODEL.md` um `contact_user` ergänzen und den Import-Ansatz für diese Tabelle klären.

---

### 3.5 `UserManagementTest.php` ist eine 820-Zeilen-Monodatei

Alle Admin-Tests (User-CRUD, Company-CRUD, Kontakt-CRUD, Filter-Presets, Company-Contacts-Tab, etc.) liegen in einer Datei. Das macht:
- Debugging schwerer (ein Fehler lässt 19+ Tests rot werden)
- Test-Isolation schwerer verständlich
- CI-Runs langsamer (kein paralleles Splitting)

**Empfehlung:** Aufteilen in:
- `tests/Feature/Admin/UserCrudTest.php`
- `tests/Feature/Admin/CompanyCrudTest.php`
- `tests/Feature/Admin/ContactCrudTest.php`
- `tests/Feature/Admin/FilterPresetTest.php`

---

### 3.6 `session()->flash()` in Livewire-Komponenten ist unzuverlässig

In vielen Volt-Komponenten wird `session()->flash('success', ...)` genutzt (z.B. in `addLinkedCompany()`, `removeLinkedContact()`). In Livewire 4 sind Flash-Messages über Session nach einem Livewire-Request (kein Full-Page-Reload) oft nicht sichtbar oder kommen mit Delay.

**Der Livewire-4-Standard** ist `$this->dispatch('notify', message: '...')` mit einem Alpine.js-Event-Listener, oder Flux-UI-eigene Notification-Komponenten. Session-Flash ist für Redirects nach `$this->redirect()` korrekt – aber nicht für inline Actions ohne Redirect.

---

### 3.7 ENUM-Spalten erschweren spätere Schema-Änderungen

Viele Tabellen nutzen `$table->enum(...)` mit PHP-Enum-Werten (z.B. `portal`, `registration_type`, `status`). Das ist funktional korrekt, hat aber einen Nachteil:

MySQL-ENUMs können nur via `ALTER TABLE` geändert werden – das sperrt die Tabelle kurz. Für `companies` und `contacts` (potentiell Hunderttausende Zeilen) kann das beim Go-Live-Rehearsal ein Problem werden.

**Alternative:** `VARCHAR` mit Laravel-seitiger Validation und Enum-Casts. Der Enum wird auf PHP-Ebene erzwungen, die DB ist flexibler.

Dies ist ein Low-Priority-Thema für den aktuellen Entwicklungsstand, aber beim Go-Live-Rehearsal (Phase 6.10) beachten.

---

## 4. Verbesserungsideen (🟢)

### 4.1 PressRelease-CRUD: Das Kern-Feature ist noch Dummy-Data

**Betroffene Dateien:**
- `resources/views/livewire/admin/press-releases/index.blade.php`
- `resources/views/livewire/admin/press-releases/create.blade.php`
- `resources/views/livewire/admin/press-releases/edit.blade.php`
- `resources/views/livewire/admin/press-releases/show.blade.php`

Alle vier Pressemitteilungs-Views enthalten noch Dummy-Daten (`// TODO: Implementierung mit echten Models nach Migration`). Pressemitteilungen sind **das Kernprodukt** beider Portale. Ohne diese Implementierung kann der Migrationserfolg nicht validiert werden.

Ebenso: `categories/index.blade.php`, `invoices/index.blade.php`, `payments/index.blade.php` sind alle noch nicht angebunden.

---

### 4.2 Kein `PressRelease`-Factory → keine Tests möglich

Das `PressRelease`-Model hat keine Factory und `HasFactory` fehlt. Für Phase 5 (`PressReleaseService` mit Tests) und Phase 6 (Import-Verification) wird ein vollständiger Test-Datensatz benötigt.

---

### 4.3 `UserFilterPreset`-Feature: zu früh für P3

Filter-Presets mit Default-Flag und Last-Used-Timestamp sind ein UX-Komfort-Feature. Zum aktuellen Zeitpunkt (die Admin-Hauptfunktionen sind noch nicht alle angebunden) binden sie Entwicklungszeit und Migration-Komplexität. Das Feature ist gut implementiert – aber die Priorisierung in P3 war verfrüht.

---

### 4.4 Keine Policies für Datenzugriff

Aktuell gibt es keine `Policy`-Klassen. Ein `admin`-User kann jede Company, jeden Kontakt, jeden User bearbeiten – egal ob er zu seinem Portal gehört. Mit der Einführung des Portal-Scopes (P2.5) wird das teilweise gelöst, aber spezifischere Zugriffsregeln (z.B. „Editor darf nur PMs im Status `draft` bearbeiten") benötigen Policies.

---

### 4.5 `MigratePresseData`-Command: Legacy-Import-Ansatz fehlt

**Datei:** `app/Console/Commands/MigratePresseData.php` (laut git status geändert)

Der Import-Command ist eine der kritischsten Anforderungen (D-18: wiederholbares Skript). Aus dem Progress-Log ist nicht ersichtlich, ob dieser Command bereits eine funktionierende Grundstruktur hat oder ob er noch ein Stub ist. Phase 6 (Import) ist der kritische Pfad für Go-Live.

---

### 4.6 Keine `gdpr_consent_at`-Erfassung beim Registrierungs-Flow

Das Feld `gdpr_consent_at` ist in der `users`-Tabelle vorhanden und im `fillable`-Array. Es wird aber nirgends im Registrierungs- oder User-Create-Flow befüllt. Für DSGVO-Konformität (besonders bei `presseecho.de` mit deutschen Nutzern) ist eine dokumentierte Einwilligung Pflicht.

---

## 5. Daten-Migration: Bereitschaft

| Aspekt | Status | Bewertung |
|---|---|---|
| Legacy-Tabellen-Mapping dokumentiert | ✅ `04-DATA-MODEL.md` vollständig | ✅ gut |
| `legacy_import_map` Tabelle | ✅ Migration + Model vorhanden | ✅ gut |
| Legacy-DB-Connections in `.env` | ⬜ `0.3` noch offen | 🔴 blockiert P6 |
| Legacy-Models unter `app/Models/Legacy/` | ⬜ nicht vorhanden | 🔴 P6 nicht startbar |
| Import-Commands (`legacy:import-*`) | ⬜ nicht vorhanden | 🔴 P6 nicht startbar |
| `legacy_invoices` Archiv-Migration | ✅ P6.5d umgesetzt | ✅ Vollimport-Command inkl. Status/User-Zuordnung/PDF-Payload; Rehearsal gegen Produktiv-Snapshot bleibt Pflicht |
| Portal-Scoping für Imports | ⬜ P2.5 fehlt | 🟡 Import ohne Scope unsicher |
| Idempotenz-Mechanismus | ✅ `legacy_import_map` Unique-Index | ✅ gut |
| Rehearsal-Plan dokumentiert | ✅ `05-DATABASE-MERGE.md` | ✅ gut |
| Factories für Import-Testdaten | ⬜ 12+ Modelle ohne Factory | 🟡 Tests schwer schreibbar |

---

## 6. Admin-UI/UX: Bewertung

### Was gut ist
- **Company-Detailseite mit Tabs** (Übersicht / Kontakte) ist ein gutes UX-Pattern.
- **Live-Suche** für Firma- und Kontaktzuordnung ist intuitiv und performant.
- **Soft-Delete mit Warn-Modal** für Companies und Kontakte verhindert versehentliches Löschen.
- **Pagination** in allen Index-Views vorhanden.

### Was fehlt / verbessert werden kann
- **Dashboard zeigt keine echten Daten** – die Widget-Queries (Anzahl PMs, neue Firmen, Newsletter-Abos) sind Stubs.
- **Breadcrumb-Navigation fehlt** – bei tief verschachtelten Views (Company → Kontakt → Edit) gibt es keine Orientierungshilfe.
- **Kein Portal-Switcher** (P2.6 offen) – Admins mit `portal = both` können nicht zwischen den Portal-Ansichten wechseln.
- **Pressemitteilungs-Workflow** (Draft → Review → Published) ist die Hauptfunktion des Systems und noch nicht implementiert.
- **Keine Suche in der Rollenübersicht** – bei vielen Custom-Rollen wird die Tabelle unübersichtlich.
- **User-Edit**: Rollenänderung und Firmenzuordnungsrolle werden nicht sofort persistiert (nur bei Save), während Firmen- und Kontaktzuordnung sofort persistiert werden – inkonsistentes UX-Verhalten.

---

## 7. Priorisierte To-Do-Liste

### Sofort (vor weiteren P3-Features)

| # | Maßnahme | Warum |
|---|---|---|
| A1 | Admin-Middleware: `canAccessAdmin()` auf Routengruppe erzwingen | Sicherheitslücke |
| A2 | `MagicLinkConsumeController`: `->with('user')` + null-Check | Laufzeitfehler bei gelöschten Usern |
| A3 | `UserRolePermissionSyncService`: Direkte Permission-Zuweisung entfernen | Anti-Pattern, bricht spätere Sync-Logik |
| A4 | P2.5 umsetzen: `SetCurrentPortal`-Middleware + Portal Global Scopes | Mandantentrennung ist Grundvoraussetzung |

### Kurzfristig (vor Go-Live-Rehearsal)

| # | Maßnahme | Warum |
|---|---|---|
| B1 | Drei `user_filter_presets`-Migrations zu einer zusammenführen | Migrations-Hygiene |
| B2 | Factories für Company, Contact, PressRelease, Category anlegen | Testbarkeit |
| B3 | `contact_user` in `04-DATA-MODEL.md` dokumentieren + Import-Strategie klären | Import-Korrektheit |
| B4 | Billing-Address-Update: Felder leerbar machen | Funktionsbug |
| B5 | `session()->flash()` durch Livewire-Events ersetzen | UX-Zuverlässigkeit |
| B6 | `gdpr_consent_at` im Registrierungsflow befüllen | DSGVO |

### Mittelfristig (P3-Abschluss)

| # | Maßnahme | Warum |
|---|---|---|
| C1 | PressRelease-CRUD auf echte Daten umstellen | Kernfeature |
| C2 | Categories-CRUD auf echte Daten umstellen | Abhängigkeit von PressRelease |
| C3 | Dashboard-Widgets mit echten Queries | Monitoring |
| C4 | `UserManagementTest.php` aufteilen | Testbarkeit |
| C5 | Policies für feinere Zugriffskontrolle | Sicherheit |

### P6-Vorbereitung (kritischer Pfad)

| # | Maßnahme | Warum |
|---|---|---|
| D1 | `.env` mit Legacy-DB-Connections konfigurieren (0.3) | Phase 6 blockiert |
| D2 | `app/Models/Legacy/`-Models anlegen | Phase 6 nicht startbar |
| D3 | `legacy:import-users` als ersten Command implementieren | Kritischer Pfad |
| D4 | Import-Command mit `--dry-run` und `--since` | D-18 |

---

## 8. Gesamtbewertung

**Umsetzungsqualität (bisherige Phasen):** Solide. Die Kern-Architektur ist gut durchdacht, die Entscheidungen (D-01 bis D-18) sind konsequent umgesetzt, das DB-Schema entspricht dem dokumentierten Zielmodell. Die Implementierungen in P3 (Company-CRUD, Kontakt-CRUD, User-CRUD) zeigen gute Livewire/Volt-Praxis.

**Größtes Risiko:** Phase P2.5 (Portal-Scoping) wurde übersprungen, aber in P3 weitergebaut. Wenn der Global Scope nachgezogen wird, müssen alle bestehenden Queries und Tests auf Korrektheit geprüft werden. Je länger das aufgeschoben wird, desto teurer wird die Nacharbeit.

**Kern-Gap:** Das Hauptfeature des Systems – Pressemitteilungen erstellen, einreichen, prüfen, veröffentlichen – ist vollständig als Dummy-Data implementiert. Das sollte die nächste Priorität nach der Security-Middleware sein.

**Import-Bereitschaft:** Die Tabellenstruktur ist bereit. Die Import-Commands, Legacy-Models und `.env`-Konfiguration fehlen noch vollständig. Phase 6 ist 3–4 Manntage Arbeit und der kritische Pfad zum Go-Live.