gitmedia/presseportale
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
presseportale/dev/migration 2026/09-REVIEW-QUALITAET.md
Kevin Adametz 5b8bdf4182
Some checks are pending
linter / quality (push) Waiting to run
Details
tests / ci (push) Waiting to run
Details
12-05-2026 Frontend dev
2026-05-12 18:32:33 +02:00

19 KiB
Raw Permalink Blame History

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

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.

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

$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:

$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

$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

$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.:

$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.