Seeder für Rechnungsarten, kleinere Optimierungen#562
Conversation
WalkthroughDie PR ergänzt eine JSON-Antwort für Dokumente, stellt Rechnungsseitenleisten auf ChangesDokument- und Rechnungs-UI
InvoiceType-Daten und Seeding
Backend-, PDF- und ZUGFeRD-Anpassungen
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.2.2)PHPStan was skipped because the config uses disallowed Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Models/Contact.php (1)
319-328: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNull-Checks für
getInvoiceAddress()-Aufrufer ergänzen.Die Methode liefert
ContactAddress|null, aber mehrere Aufrufer dereferenzieren das Ergebnis weiter ohne Absicherung (app/Services/ZugferdService.php,app/Models/InvoiceReminder.php,app/Http/Controllers/App/OfferController.php). Bei fehlender Rechnungsadresse führt das zu einem Fatal Error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Models/Contact.php` around lines 319 - 328, `getInvoiceAddress()` can return null, but its callers currently dereference the result without checking. Update the usages in `ZugferdService`, `InvoiceReminder`, and `OfferController` to guard against a missing address before accessing fields or methods, and handle the null case gracefully (for example by skipping the operation or using a fallback). Use the `Contact::getInvoiceAddress()` return value safely wherever it is consumed.
🧹 Nitpick comments (7)
resources/js/Pages/App/Invoice/InvoiceDetailsSide.tsx (1)
16-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
showSecondarybleibt ungenutzt.Die Prop ist im Interface deklariert, wird aber weder destrukturiert noch verwendet. Entweder implementieren oder entfernen.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/Pages/App/Invoice/InvoiceDetailsSide.tsx` around lines 16 - 25, The InvoiceDetailsSideProps interface declares showSecondary, but InvoiceDetailsSide does not destructure or use it. Either remove showSecondary from InvoiceDetailsSideProps and the component props if it is not needed, or add it to the InvoiceDetailsSide parameter list and use it in the component logic so the prop is handled consistently.app/Http/Controllers/App/DocumentController.php (3)
66-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInline-Kommentare durch sprechende Logik ersetzen.
Die Kommentare beschreiben einfache Branches; z. B. kann
$showHiddenDocumentsdieis_hidden-Regel ohne Kommentar ausdrücken. As per coding guidelines, “Prefer PHPDoc blocks over inline comments, and avoid comments in code unless the logic is exceptionally complex.”Also applies to: 89-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Http/Controllers/App/DocumentController.php` at line 66, The inline comments in DocumentController should be removed and replaced with self-explanatory logic using the existing branching variables and conditions, especially around the is_hidden handling in the document listing flow. Update the relevant checks in the controller methods that control hidden-document visibility, using meaningful symbols like $showHiddenDocuments and the document filter logic, so the intent is clear from the code itself without comments.Source: Coding guidelines
89-91: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winJSON-Pfad lädt unnötig viele Inertia-Daten vorab.
Für
expectsJson()werden vorher Kontakte, Typen, Projekte, Filterlisten und Bookmarks geladen, obwohl nur$documentszurückgegeben wird. Verschiebe die Inertia-only Queries hinter den JSON-Return, damit der DocumentSelector nicht mehrere unnötige Queries pro Suche ausführt.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Http/Controllers/App/DocumentController.php` around lines 89 - 91, The expectsJson() branch in DocumentController::index is loading Inertia-only data too early, causing unnecessary queries before returning DocumentData for the DocumentSelector. Move the contacts, types, projects, filter lists, and bookmarks queries behind the JSON return so they only run for the Inertia path, keeping the JSON path limited to the $documents response.
91-91: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFür den Selector kein vollständiges
DocumentDataserialisieren.
DocumentDataenthält u. a.fulltext; das kann OCR-Volltexte unnötig in jede Selector-Antwort schreiben. Nutze für diesen JSON-Zweig besser ein schlankes DTO/Resource mit den tatsächlich benötigten Feldern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Http/Controllers/App/DocumentController.php` at line 91, The JSON selector response currently serializes full DocumentData objects, which can include unnecessary fields like fulltext. Update the selector branch in DocumentController to return a slimmer DTO/Resource instead of DocumentData::collect($documents)->toJson(), and keep only the fields actually needed by the selector response. Use the DocumentController selector logic and the DocumentData collection call as the main place to replace with a minimal response shape.app/Models/Contact.php (2)
319-319: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNullable Type-Hint-Stil.
ContactAddress | nullkönnte konsistenter als?ContactAddressgeschrieben werden, wie es üblicherweise in dieser Codebasis für nullable Types genutzt wird (z. B.?string,?int).♻️ Vorschlag
- public function getInvoiceAddress(?int $contactId = 0): ContactAddress | null + public function getInvoiceAddress(?int $contactId = 0): ?ContactAddress🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Models/Contact.php` at line 319, The nullable return type in getInvoiceAddress should use the codebase’s standard nullable syntax. Update the Contact::getInvoiceAddress signature from ContactAddress | null to the consistent ?ContactAddress form, matching the existing style used for other nullable types in this class and surrounding code.
319-328: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
$contactIdausgetInvoiceAddress()entfernen oder tatsächlich verwenden.
Der Parameter ist aktuell ungenutzt; die Methode holt immer die Rechnungsadresse des aktuellen Kontakts. Falls keine spätere Nutzung geplant ist, die Signatur und den überflüssigen Aufruf entfernen.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Models/Contact.php` around lines 319 - 328, The getInvoiceAddress() method currently accepts $contactId but never uses it, so either remove the parameter from the method signature and any callers, or update the implementation to actually look up the invoice address for the provided contact ID. Use the getInvoiceAddress() symbol in Contact and ensure the method behavior matches its public API without keeping an unused argument.Source: Linters/SAST tools
database/seeders/InvoiceTypeSeeder.php (1)
16-16: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueKeine Fehlerbehandlung, falls
invoice_types.jsonfehlt oder ungültig ist.
Storage::disk('json')->json('invoice_types.json')liefertnullzurück, wenn die Datei fehlt oder kein valides JSON enthält. Die anschließendeforeach-Schleife würde dann mit einer TypeError abbrechen (foreach() argument must be of type array|object, null given). Da Seeder häufig in CI/Deployment-Pipelines automatisiert laufen, könnte ein fehlendes Datenverzeichnis den gesamten Seeding-Vorgang unkontrolliert abbrechen lassen.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/seeders/InvoiceTypeSeeder.php` at line 16, The InvoiceTypeSeeder::run flow assumes Storage::disk('json')->json('invoice_types.json') always returns data, but it can be null when the file is missing or invalid. Add a guard in the seeder before the foreach loop to handle a null/empty $invoiceTypes value gracefully, and log or skip seeding instead of letting the loop throw a TypeError. Use the InvoiceTypeSeeder and its $invoiceTypes loading logic as the place to fix this.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Http/Controllers/App/DocumentController.php`:
- Line 46: The DocumentController::index method currently returns the result of
toJson() as a raw string, bypassing the normal response handling for the
DataCollection. Update index so the JSON branch returns an actual JsonResponse
instead of a string, and adjust the method’s return type accordingly while
keeping the existing behavior for the non-JSON branch.
In `@app/Models/Contact.php`:
- Line 301: The conditional in Contact should follow the coding guideline that
control structures always use curly braces. Update the early return around the
$address check in the Contact model so the if statement uses braces even though
it returns a single empty string, and keep the surrounding logic unchanged.
In `@database/json/invoice_types.json`:
- Around line 24-45: The invoice type entry for the cancel case in
invoice_types.json is using the wrong ZUGFeRD identifier. Update the object with
key "cancel" (the one with print_name/display_name Stornorechnung/Storno) so its
zugferd_id is 381 instead of 384, and keep the correction-related entry separate
so the invoice type mapping stays semantically correct.
In `@database/seeders/InvoiceTypeSeeder.php`:
- Around line 18-28: Das InvoiceType-Modell blockiert beim Seeder das Setzen von
id, zugferd_id und is_default, weil diese Felder nicht in InvoiceType::$fillable
enthalten sind. Passe app/Models/InvoiceType.php so an, dass firstOrCreate() im
InvoiceTypeSeeder diese Attribute übernehmen kann, indem du die fehlenden Felder
zu $fillable hinzufügst oder die Guard-Definition entsprechend lockerst.
Referenziere dabei die Symbole InvoiceType::$fillable und
InvoiceTypeSeeder::firstOrCreate() als Stellen zur Korrektur.
In `@resources/js/Pages/App/Invoice/InvoiceDetailsSide.tsx`:
- Around line 16-20: `InvoiceDetailsSide` now requires `zugferd_profiles`, but
existing callers like `InvoiceDetailsEditBaseData` still render it with only
`invoice`, causing breakage. Update the relevant call site to pass the
`zugferd_profiles` data through, or revert `InvoiceDetailsSideProps` so
`zugferd_profiles` remains optional if that matches the intended API. Make sure
the ZUGFeRD section in `InvoiceDetailsSide` still handles missing values safely.
In `@resources/js/Pages/App/Invoice/InvoiceSendByMail.tsx`:
- Line 73: The render in InvoiceSendByMail uses the wrong component name,
causing the undefined JSX error: replace the InvoiceDetailsSidex reference with
the imported InvoiceDetailsSide so it matches the existing import. Also update
this component’s Props/usePage wiring to pass the newly required
zugferd_profiles prop into InvoiceDetailsSide, following the pattern used in
InvoiceDetails and InvoiceHistory.
---
Outside diff comments:
In `@app/Models/Contact.php`:
- Around line 319-328: `getInvoiceAddress()` can return null, but its callers
currently dereference the result without checking. Update the usages in
`ZugferdService`, `InvoiceReminder`, and `OfferController` to guard against a
missing address before accessing fields or methods, and handle the null case
gracefully (for example by skipping the operation or using a fallback). Use the
`Contact::getInvoiceAddress()` return value safely wherever it is consumed.
---
Nitpick comments:
In `@app/Http/Controllers/App/DocumentController.php`:
- Line 66: The inline comments in DocumentController should be removed and
replaced with self-explanatory logic using the existing branching variables and
conditions, especially around the is_hidden handling in the document listing
flow. Update the relevant checks in the controller methods that control
hidden-document visibility, using meaningful symbols like $showHiddenDocuments
and the document filter logic, so the intent is clear from the code itself
without comments.
- Around line 89-91: The expectsJson() branch in DocumentController::index is
loading Inertia-only data too early, causing unnecessary queries before
returning DocumentData for the DocumentSelector. Move the contacts, types,
projects, filter lists, and bookmarks queries behind the JSON return so they
only run for the Inertia path, keeping the JSON path limited to the $documents
response.
- Line 91: The JSON selector response currently serializes full DocumentData
objects, which can include unnecessary fields like fulltext. Update the selector
branch in DocumentController to return a slimmer DTO/Resource instead of
DocumentData::collect($documents)->toJson(), and keep only the fields actually
needed by the selector response. Use the DocumentController selector logic and
the DocumentData collection call as the main place to replace with a minimal
response shape.
In `@app/Models/Contact.php`:
- Line 319: The nullable return type in getInvoiceAddress should use the
codebase’s standard nullable syntax. Update the Contact::getInvoiceAddress
signature from ContactAddress | null to the consistent ?ContactAddress form,
matching the existing style used for other nullable types in this class and
surrounding code.
- Around line 319-328: The getInvoiceAddress() method currently accepts
$contactId but never uses it, so either remove the parameter from the method
signature and any callers, or update the implementation to actually look up the
invoice address for the provided contact ID. Use the getInvoiceAddress() symbol
in Contact and ensure the method behavior matches its public API without keeping
an unused argument.
In `@database/seeders/InvoiceTypeSeeder.php`:
- Line 16: The InvoiceTypeSeeder::run flow assumes
Storage::disk('json')->json('invoice_types.json') always returns data, but it
can be null when the file is missing or invalid. Add a guard in the seeder
before the foreach loop to handle a null/empty $invoiceTypes value gracefully,
and log or skip seeding instead of letting the loop throw a TypeError. Use the
InvoiceTypeSeeder and its $invoiceTypes loading logic as the place to fix this.
In `@resources/js/Pages/App/Invoice/InvoiceDetailsSide.tsx`:
- Around line 16-25: The InvoiceDetailsSideProps interface declares
showSecondary, but InvoiceDetailsSide does not destructure or use it. Either
remove showSecondary from InvoiceDetailsSideProps and the component props if it
is not needed, or add it to the InvoiceDetailsSide parameter list and use it in
the component logic so the prop is handled consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06b302c0-f968-4b16-84dc-cc4d00ccc59e
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
app/Http/Controllers/App/ContactController.phpapp/Http/Controllers/App/DocumentController.phpapp/Models/Contact.phpapp/Services/PdfService.phpdatabase/json/invoice_types.jsondatabase/seeders/InvoiceTypeSeeder.phpdatabase/seeders/TenantSeeder.phpdatabase/seeders/UpdatePdfGlobalCssSeeder.phpresources/js/Pages/App/Document/DocumentSelector.tsxresources/js/Pages/App/Invoice/InvoiceDetails.tsxresources/js/Pages/App/Invoice/InvoiceDetailsSide.tsxresources/js/Pages/App/Invoice/InvoiceDetailsSideLight.tsxresources/js/Pages/App/Invoice/InvoiceHistory.tsxresources/js/Pages/App/Invoice/InvoiceSendByMail.tsxresources/js/Pages/App/Offer/OfferDetailsLayout.tsx
💤 Files with no reviewable changes (1)
- resources/js/Pages/App/Invoice/InvoiceDetailsSideLight.tsx
| InvoiceType::firstOrCreate([ | ||
| 'id' => $value['id'], | ||
| ], [ | ||
| 'print_name' => $value['print_name'], | ||
| 'display_name' => $value['display_name'], | ||
| 'abbreviation' => $value['abbreviation'], | ||
| 'zugferd_id' => $value['zugferd_id'], | ||
| 'key' => $value['key'], | ||
| 'is_default' => $value['is_default'], | ||
|
|
||
| ]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Mass-Assignment schlägt für id, zugferd_id und is_default fehl – InvoiceType::$fillable deckt diese Felder nicht ab.
Laut dem referenzierten Modell app/Models/InvoiceType.php:16-31 enthält $fillable nur print_name, display_name, abbreviation und key. Der Seeder versucht jedoch, zusätzlich id, zugferd_id und is_default per firstOrCreate() zu setzen. firstOrCreate() führt intern ein fill() mit allen übergebenen Attributen aus (aus Query- und Werte-Array) – nicht-fillable Attribute werden dabei standardmäßig stillschweigend verworfen (bzw. es kommt zur MassAssignmentException, falls preventSilentlyDiscardingAttributes() aktiv ist).
Das bedeutet konkret:
zugferd_idundis_defaultwerden bei neu erstellten Datensätzen nicht gesetzt (bleibennull/Default), obwohl die JSON-Datei diese Werte explizit liefert.idwird nicht wie in der JSON-Datei vorgegeben gesetzt, sondern per Auto-Increment vergeben – dadurch könnten IDs bei mehrfacher Ausführung in unterschiedlichen Umgebungen abweichen und referenzielle Konsistenz zu anderen Tabellen (falls per festerinvoice_type_idreferenziert wird) gefährden.
Bitte $fillable im InvoiceType-Modell um id, zugferd_id und is_default ergänzen (oder $guarded = [] nutzen), damit der Seeder wie beabsichtigt funktioniert.
🐛 Vorgeschlagene Korrektur im InvoiceType-Modell
protected $fillable = [
'print_name',
'display_name',
'abbreviation',
'key',
+ 'zugferd_id',
+ 'is_default',
];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@database/seeders/InvoiceTypeSeeder.php` around lines 18 - 28, Das
InvoiceType-Modell blockiert beim Seeder das Setzen von id, zugferd_id und
is_default, weil diese Felder nicht in InvoiceType::$fillable enthalten sind.
Passe app/Models/InvoiceType.php so an, dass firstOrCreate() im
InvoiceTypeSeeder diese Attribute übernehmen kann, indem du die fehlenden Felder
zu $fillable hinzufügst oder die Guard-Definition entsprechend lockerst.
Referenziere dabei die Symbole InvoiceType::$fillable und
InvoiceTypeSeeder::firstOrCreate() als Stellen zur Korrektur.
There was a problem hiding this comment.
Bei den anderen Seedern hattest Du gestern nichts dagegen, dass wir id im Seeder setzen..
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd InvoiceType.php app/Models -x cat -n {}
echo "---"
fd -e php . database/seeders -x echo {}
echo "---"
rg -n "firstOrCreate|fillable|guarded" database/seeders app/Models/InvoiceType.phpLength of output: 3030
🏁 Script executed:
#!/bin/bash
echo "=== TitleSeeder ==="
cat -n database/seeders/TitleSeeder.php
echo "=== Title model ==="
fd -i title.php app/Models -x cat -n {}
echo "=== TaxSeeder ==="
cat -n database/seeders/TaxSeeder.php
echo "=== Tax model ==="
fd -i '^tax.php' app/Models -x cat -n {}
echo "=== DocumentTypeSeeder ==="
cat -n database/seeders/DocumentTypeSeeder.php
echo "=== DocumentType model ==="
fd -i documenttype.php app/Models -x cat -n {}Length of output: 8107
@dspangenberg Fair point, and you're right — I checked TitleSeeder, TaxSeeder, and DocumentTypeSeeder, and all of them set id in firstOrCreate() even though id isn't in the respective models' $fillable (Title, Tax, DocumentType). So that's an established pattern in this codebase already, and I shouldn't have flagged it here as if it were new/wrong. Withdrawing that part of the comment.
That said, the zugferd_id and is_default part is a bit different: unlike Tax, which does include is_default in its $fillable, the InvoiceType model's $fillable only has print_name, display_name, abbreviation, key. So those two attributes specifically will be silently dropped on create via firstOrCreate(), meaning newly seeded invoice types would always get zugferd_id = null and is_default = false/0 regardless of what's in invoice_types.json. If that's intentional (e.g., set separately elsewhere), feel free to disregard — otherwise it'd be worth adding those two fields to $fillable.
🧠 Learnings used
Learnt from: dspangenberg
Repo: dspangenberg/opsc PR: 337
File: routes/tenant.php:148-152
Timestamp: 2026-03-05T19:36:23.252Z
Learning: Ensure POSTAL_PUBLIC_KEY in .env is a full PEM-formatted RSA public key (BEGIN PUBLIC KEY and END PUBLIC KEY with proper newlines). Since Laravel does not preserve literal newlines in env values, store the key with real newline characters in .env (quoted) or convert on read (e.g., in config/services.php use str_replace('\n', "\n", env('POSTAL_PUBLIC_KEY'))). The key is derived from the Postal DKIM key via the p= value from postal default-dkim-record. Apply this guidance wherever the key is read in PHP (notably in Laravel config) to ensure the PEM is valid.
Learnt from: dspangenberg
Repo: dspangenberg/opsc PR: 339
File: app/Enums/InboxEntryStatus.php:7-9
Timestamp: 2026-03-05T22:26:43.349Z
Learning: Enforce enum case naming in PHP projects: use UPPER_SNAKE_CASE for enum case keys (e.g., PENDING, ACCEPTED, REJECTED). TitleCase should not be used for enum cases. This applies globally to PHP files in the repository (e.g., app/Enums/InboxEntryStatus.php and similar).
|
@coderabbitai bitte führen einen neuen review durch |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Services/PdfService.php (1)
100-138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReihenfolge der Anhänge wird durch getrennte Schleifen verfälscht.
Die neue "nicht gefunden"-Seite wird sofort in der ersten Schleife hinzugefügt, während gültige Attachment-Seiten erst in der zweiten Schleife (
$files) angehängt werden. Bei gemischter Attachment-Reihenfolge (z. B. gültig → fehlend → gültig) erscheinen alle Fehlerhinweise vor den eigentlichen Anhang-Seiten statt an der erwarteten Position.Falls die Reihenfolge der Anhänge für Nutzer relevant ist (z. B. Rechnung + Belege in bestimmter Reihenfolge), sollte die Fehlerseite inline zur gleichen Zeit wie die zugehörigen Datei-Seiten verarbeitet werden, statt in zwei getrennten Durchläufen.
🐛 Vorschlag zur Beibehaltung der Reihenfolge
if ($attachments && count($attachments) > 0) { - $files = []; - - foreach ($attachments as $attachment) { - if (is_string($attachment)) { - $files[] = $attachment; - } else { - $document = Document::find($attachment); - if ($document) { - $media = $document->firstMedia('file'); - - if ($media && Storage::disk($media->disk)->exists($media->getDiskPath())) { - $attachmentFile = FileHelperService::createTemporaryFileFromDoc($media->filename, - $media->contents()); - if (file_exists($attachmentFile)) { - $files[] = $attachmentFile; - } - } else { - $mpdf->AddPage(); - $mpdf->WriteText(10, 20, $document->title.' nicht gefunden'); - } - } - } - } - - if (count($files) > 0) { - foreach ($files as $file) { + foreach ($attachments as $attachment) { + if (is_string($attachment)) { + $file = $attachment; + } else { + $document = Document::find($attachment); + if (! $document) { + continue; + } + $media = $document->firstMedia('file'); + if (! $media || ! Storage::disk($media->disk)->exists($media->getDiskPath())) { + $mpdf->AddPage(); + $mpdf->WriteText(10, 20, $document->title.' nicht gefunden'); + + continue; + } + $file = FileHelperService::createTemporaryFileFromDoc($media->filename, $media->contents()); + if (! file_exists($file)) { + continue; + } + } + + { $mpdf->AddPage(); $pagecount = $mpdf->setSourceFile($file); for ($i = 1; $i <= $pagecount; $i++) { $tplIdx = $mpdf->ImportPage($i); $mpdf->useTemplate($tplIdx); if ($i < $pagecount) { $mpdf->AddPage(); } } } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Services/PdfService.php` around lines 100 - 138, The attachment processing in PdfService::createPdf is split into two loops, which breaks the original attachment order when mixing found and missing documents. Refactor the attachment handling so each item is rendered in sequence inside the same foreach over $attachments, and handle the “nicht gefunden” page inline for missing Document/Media cases instead of deferring valid files to a later $files loop. Keep the ordering logic centered around the existing Document::find, firstMedia('file'), and FileHelperService::createTemporaryFileFromDoc flow.
♻️ Duplicate comments (1)
app/Http/Controllers/App/DocumentController.php (1)
91-95: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winJSON-Payload nicht doppelt serialisieren.
$data->toJson()erzeugt bereits einen JSON-String;response()->json(...)kodiert diesen String erneut. Dadurch bekommtDocumentSelectorkeinen Objekt-Payload mitdata, sondern einen String, und die Dokumentliste bleibt leer. Ähnlich zum früheren Hinweis isttoJson()hier weiterhin die Ursache.🐛 Minimaler Fix
- return response()->json($data->toJson()); + return response()->json($data);#!/bin/bash set -euo pipefail echo "== Backend JSON branch ==" sed -n '90,96p' app/Http/Controllers/App/DocumentController.php | cat -n echo "== Frontend JSON consumption ==" sed -n '22,31p' resources/js/Pages/App/Document/DocumentSelector.tsx | cat -n echo "== Double-encoding demonstration ==" python3 - <<'PY' import json payload = '{"data":[{"id":1}]}' wire_body = json.dumps(payload) client_value = json.loads(wire_body) print(type(client_value).__name__, client_value) print("has data key:", isinstance(client_value, dict) and "data" in client_value) PY🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Http/Controllers/App/DocumentController.php` around lines 91 - 95, The JSON branch in DocumentController::show is double-encoding the payload because DocumentData::collect($documents)->toJson() already returns a JSON string before response()->json() runs. Update the expectsJson() path to return the DocumentData collection/object directly through response()->json(...) so the frontend DocumentSelector receives a proper object with a data field instead of a string. Keep the fix localized to the JSON response branch and remove the extra toJson() call.
🧹 Nitpick comments (3)
app/Http/Controllers/App/DocumentController.php (1)
67-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInline-Kommentare hier vermeiden.
Die Hinweise beschreiben einfache, direkt aus dem Code erkennbare Logik; bitte entfernen oder die Absicht über Methodennamen/Extraktion ausdrücken. As per coding guidelines, “Prefer PHPDoc blocks over inline comments, and avoid comments in code unless the logic is exceptionally complex.”
Also applies to: 90-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Http/Controllers/App/DocumentController.php` at line 67, Remove the inline German comment in DocumentController and express the intent through clearer code structure instead; since the logic is straightforward, either delete the comment entirely or extract the filtering behavior into a well-named helper method in the controller so the purpose is obvious without an inline note. Keep the change localized to the conditional handling of is_hidden and preserve the existing behavior.Source: Coding guidelines
resources/js/Pages/App/Invoice/InvoiceDetailsSide.tsx (1)
136-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDoppel-Cast
as unknown as string[]deutet auf Typinkonsistenz hin.Der
value-Prop vonDataCardFieldscheintinvoice.invoice_addressnicht direkt alsstring[]zu akzeptieren. Statt an der Aufrufstelle überunknownzu casten, wäre eine Anpassung derDataCardField-Prop-Typen sauberer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/Pages/App/Invoice/InvoiceDetailsSide.tsx` around lines 136 - 142, The `InvoiceDetailsSide` usage of `DataCardField` is hiding a type mismatch with a double cast on `invoice.invoice_address`. Update the `DataCardField` prop typing so `value` accepts the actual address shape directly, and then remove the `as unknown as string[]` cast at the call site while keeping the `ArrayTextField` usage intact.app/Models/Contact.php (1)
320-329: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
$contactId-Parameter bleibt ungenutzt.
getInvoiceAddress()erhält weiterhin$contactId, verwendet ihn im Körper aber nicht – die Methode filtert die Adresse nur überAddressCategory. Aufrufer wiegetFormatedInvoiceAddress($contactId)übergeben den Wert, ohne dass er etwas bewirkt. Entweder Parameter entfernen oder tatsächlich zur Filterung nach Kontakt-Adresse verwenden.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Models/Contact.php` around lines 320 - 329, The $contactId parameter in getInvoiceAddress() is unused, so either remove it from the method signature and update callers like getFormatedInvoiceAddress(), or use it in the address lookup if contact-specific filtering is intended. Check Contact::getInvoiceAddress() and the related formatting method to keep the API consistent and avoid passing a value that has no effect.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/Services/PdfService.php`:
- Around line 100-138: The attachment processing in PdfService::createPdf is
split into two loops, which breaks the original attachment order when mixing
found and missing documents. Refactor the attachment handling so each item is
rendered in sequence inside the same foreach over $attachments, and handle the
“nicht gefunden” page inline for missing Document/Media cases instead of
deferring valid files to a later $files loop. Keep the ordering logic centered
around the existing Document::find, firstMedia('file'), and
FileHelperService::createTemporaryFileFromDoc flow.
---
Duplicate comments:
In `@app/Http/Controllers/App/DocumentController.php`:
- Around line 91-95: The JSON branch in DocumentController::show is
double-encoding the payload because DocumentData::collect($documents)->toJson()
already returns a JSON string before response()->json() runs. Update the
expectsJson() path to return the DocumentData collection/object directly through
response()->json(...) so the frontend DocumentSelector receives a proper object
with a data field instead of a string. Keep the fix localized to the JSON
response branch and remove the extra toJson() call.
---
Nitpick comments:
In `@app/Http/Controllers/App/DocumentController.php`:
- Line 67: Remove the inline German comment in DocumentController and express
the intent through clearer code structure instead; since the logic is
straightforward, either delete the comment entirely or extract the filtering
behavior into a well-named helper method in the controller so the purpose is
obvious without an inline note. Keep the change localized to the conditional
handling of is_hidden and preserve the existing behavior.
In `@app/Models/Contact.php`:
- Around line 320-329: The $contactId parameter in getInvoiceAddress() is
unused, so either remove it from the method signature and update callers like
getFormatedInvoiceAddress(), or use it in the address lookup if contact-specific
filtering is intended. Check Contact::getInvoiceAddress() and the related
formatting method to keep the API consistent and avoid passing a value that has
no effect.
In `@resources/js/Pages/App/Invoice/InvoiceDetailsSide.tsx`:
- Around line 136-142: The `InvoiceDetailsSide` usage of `DataCardField` is
hiding a type mismatch with a double cast on `invoice.invoice_address`. Update
the `DataCardField` prop typing so `value` accepts the actual address shape
directly, and then remove the `as unknown as string[]` cast at the call site
while keeping the `ArrayTextField` usage intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea230ebb-d85f-4aee-afce-4a80c8d9fc84
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
app/Http/Controllers/App/ContactController.phpapp/Http/Controllers/App/DocumentController.phpapp/Http/Controllers/App/InvoiceController.phpapp/Models/Contact.phpapp/Models/InvoiceType.phpapp/Services/PdfService.phpapp/Services/ZugferdService.phpdatabase/json/invoice_types.jsondatabase/seeders/InvoiceTypeSeeder.phpdatabase/seeders/TenantSeeder.phpdatabase/seeders/UpdatePdfGlobalCssSeeder.phpresources/js/Pages/App/Document/DocumentSelector.tsxresources/js/Pages/App/Invoice/InvoiceDetails.tsxresources/js/Pages/App/Invoice/InvoiceDetailsEditBaseData.tsxresources/js/Pages/App/Invoice/InvoiceDetailsSide.tsxresources/js/Pages/App/Invoice/InvoiceDetailsSideLight.tsxresources/js/Pages/App/Invoice/InvoiceHistory.tsxresources/js/Pages/App/Invoice/InvoiceSendByMail.tsxresources/js/Pages/App/Offer/OfferDetailsLayout.tsxresources/views/pdf/invoice/index.blade.php
💤 Files with no reviewable changes (1)
- resources/js/Pages/App/Invoice/InvoiceDetailsSideLight.tsx
1 participant
Summary by CodeRabbit
Neue Funktionen
Bug Fixes