Aus Angeboten heraus, können Standard-, Aktonto- und Schlussrechnunge…#574
Conversation
…n erstellt werden
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughDer PR führt einen neuen Ablauf zur Rechnungserstellung aus Angeboten ein: Validierung per FormRequest, umgebaute Controller-Logik mit Rechnungstypen und Zugferd-Feldern, neue Route- und UI-Anbindung sowie eine Anpassung der PDF-Zeilen-Ausgabe. ChangesRechnung aus Angebot erstellen
Estimated code review effort: 4 (Complex) | ~60 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Http/Controllers/App/OfferController.php (1)
189-310: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFehlende Datenbanktransaktion beim Erstellen der Rechnung.
storeInvoiceerzeugt eineInvoiceund (je nach Zweig) mehrereInvoiceLine-Datensätze über separatesave()-Aufrufe/Schleifen. Schlägt ein Schritt mitten in der Verarbeitung fehl (z. B. DB-Fehler in derforeach-Schleife für Gegenbuchungen), bleibt eine unvollständige Rechnung (Invoice ohne/mit nur teilweisen Positionen) in der Datenbank zurück. Der gesamte Block sollte inDB::transaction()gekapselt werden.Ebenso fehlt der explizite Rückgabetyp (`RedirectResponse`) an der Methode, den die Guideline für PHP-Methoden verlangt. As per coding guidelines, "In PHP files, use explicit return type declarations and type hints for all method parameters."🛠️ Vorschlag (Skizze)
- public function storeInvoice(OfferInvoiceStoreRequest $request, Offer $offer) + public function storeInvoice(OfferInvoiceStoreRequest $request, Offer $offer): RedirectResponse { - $offer->loadSum('lines', 'amount'); - ... - return redirect()->route('app.invoice.details', ['invoice' => $invoice->id]); + return DB::transaction(function () use ($request, $offer) { + $offer->loadSum('lines', 'amount'); + // ... unveränderte Logik ... + return redirect()->route('app.invoice.details', ['invoice' => $invoice->id]); + }); }🤖 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/OfferController.php` around lines 189 - 310, Wrap the entire invoice creation flow in storeInvoice() in a DB::transaction() so the Invoice, its address update, and all InvoiceLine saves either all succeed or all roll back together, including the final/deposit/default branches and the foreach over $offer->invoices. Also update the method signature on storeInvoice() to use an explicit RedirectResponse return type, keeping the existing parameters and preserving the redirects to app.invoice.details.Source: Coding guidelines
🧹 Nitpick comments (1)
app/Http/Controllers/App/OfferController.php (1)
623-639: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMethodenname
getOfferLine_QBweicht von PHP-Namenskonventionen ab.Mischung aus camelCase und Unterstrich sowie das kryptische Suffix
_QBerschweren die Lesbarkeit. Ein sprechender camelCase-Name (z. B.copyOfferLinesToInvoice) wäre klarer.🤖 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/OfferController.php` around lines 623 - 639, The method name getOfferLine_QB mixes camelCase with an underscore and the _QB suffix, so rename it to a clear camelCase name that reflects its purpose, such as copyOfferLinesToInvoice, and update any internal references or callers so the OfferController method remains discoverable and consistent with PHP naming conventions.
🤖 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/OfferController.php`:
- Around line 234-249: Handle the nullable result from
$offer->lines()->whereIn('type_id', [1, 3])->first() before using $firstLine in
the invoice creation flow inside OfferController. Add a guard in the branch that
builds the invoice line so you either skip processing or fail gracefully when no
matching line exists, and only read $firstLine->text, $firstLine->tax_rate,
$firstLine->tax_rate_id, and $firstLine->rate->rate after confirming both
$firstLine and $firstLine->rate are present. Also remove the duplicate
invoice_id assignment while updating the same invoice line setup.
- Around line 163-187: The initial $offer->load('contact', 'lines.rate') call in
createInvoice is redundant and gets overridden by the later load chain, which
reloads lines without rate. Remove the duplicate eager-load pattern and make
sure the final load sequence includes lines.rate alongside the ordered lines
relation so OfferData::from($offer) still has rate available without triggering
extra queries.
In `@app/Http/Requests/OfferInvoiceStoreRequest.php`:
- Around line 24-31: In OfferInvoiceStoreRequest::messages(), the custom
validation messages still reference unrelated fields from a different request,
so they never apply to the rules() in this class. Update the message keys to
match the actual validated attributes in OfferInvoiceStoreRequest (the fields
defined in rules(), such as invoice_type_id, should_summarize, and deposit) and
replace the copied German texts with messages that fit those validations.
In `@resources/js/Pages/App/Offer/OfferCreateInvoice.tsx`:
- Around line 14-28: Rename the copied identifiers in the invoice page so they
reflect the actual screen: update OfferHistory and OfferTermsProps in
OfferCreateInvoice.tsx to invoice-specific names, and keep the component
signature aligned with the new props interface. Make sure the exported/used
React component name and its props type clearly match the invoice creation
context so DevTools, error boundaries, and future maintenance show the correct
component identity.
- Around line 96-102: The JSX in OfferCreateInvoice renders literal “(” and “)”
text nodes around the OfferDetailsSide sidebar, so remove those stray characters
from the returned markup and keep only the wrapping div structure. Use the
OfferCreateInvoice component and the OfferDetailsSide block as the place to
clean up the rendered JSX so the sidebar no longer shows extra parentheses in
the UI.
---
Outside diff comments:
In `@app/Http/Controllers/App/OfferController.php`:
- Around line 189-310: Wrap the entire invoice creation flow in storeInvoice()
in a DB::transaction() so the Invoice, its address update, and all InvoiceLine
saves either all succeed or all roll back together, including the
final/deposit/default branches and the foreach over $offer->invoices. Also
update the method signature on storeInvoice() to use an explicit
RedirectResponse return type, keeping the existing parameters and preserving the
redirects to app.invoice.details.
---
Nitpick comments:
In `@app/Http/Controllers/App/OfferController.php`:
- Around line 623-639: The method name getOfferLine_QB mixes camelCase with an
underscore and the _QB suffix, so rename it to a clear camelCase name that
reflects its purpose, such as copyOfferLinesToInvoice, and update any internal
references or callers so the OfferController method remains discoverable and
consistent with PHP naming conventions.
🪄 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: 8ab191ab-6d3e-4681-8a99-100e13938bb3
⛔ Files ignored due to path filters (3)
.DS_Storeis excluded by!**/.DS_Storeapp/.DS_Storeis excluded by!**/.DS_Storeapp/Http/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (7)
app/Http/Controllers/App/OfferController.phpapp/Http/Requests/OfferInvoiceStoreRequest.phpapp/Models/Offer.phpresources/js/Components/twc-ui/form-card.tsxresources/js/Pages/App/Offer/OfferCreateInvoice.tsxresources/js/Pages/App/Offer/OfferDetailsLayout.tsxroutes/tenant/offers.php
| $firstLine = $offer->lines()->whereIn('type_id', [1, 3])->first(); | ||
| $firstLineParts = explode("\n", $firstLine->text); | ||
| $text[] = $firstLineParts[0]; | ||
| $text[] = 'gemäß AG-'.$offer->formated_offer_number; | ||
| $invoiceLine->invoice_id = $invoice->id; | ||
| $invoiceLine->type_id = 3; | ||
| $invoiceLine->invoice_id = $invoice->id; | ||
| $invoiceLine->pos = 1; | ||
| $invoiceLine->quantity = 1; | ||
| $invoiceLine->unit = '*'; | ||
| $invoiceLine->price = $amount; | ||
| $invoiceLine->amount = $amount; | ||
| $invoiceLine->text = implode("\n", $text); | ||
| $invoiceLine->tax_rate = $firstLine->tax_rate; | ||
| $invoiceLine->tax_rate_id = $firstLine->tax_rate_id; | ||
| $invoiceLine->tax = $amount / 100 * $firstLine->rate->rate; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Mögliche Null-Pointer-Exception bei $firstLine.
$offer->lines()->whereIn('type_id', [1, 3])->first() kann null zurückgeben, falls das Angebot keine passende Zeile (Typ 1 oder 3) besitzt. Direkt danach wird $firstLine->text (bzw. $firstLine->rate->rate) ohne Null-Prüfung zugegriffen – das führt zu einem Fatal Error und würde die gesamte Rechnungserstellung abbrechen. Zusätzlich ist $firstLine->rate nicht garantiert geladen/gesetzt (z. B. wenn tax_rate_id null ist), was zu einer weiteren NPE führen kann.
🛠️ Vorschlag
$firstLine = $offer->lines()->whereIn('type_id', [1, 3])->first();
+ if (! $firstLine || ! $firstLine->rate) {
+ abort(422, 'Für die Rechnungserstellung wird mindestens eine gültige Angebotsposition mit Steuersatz benötigt.');
+ }
$firstLineParts = explode("\n", $firstLine->text);Also applies to: 281-299
🤖 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/OfferController.php` around lines 234 - 249, Handle
the nullable result from $offer->lines()->whereIn('type_id', [1, 3])->first()
before using $firstLine in the invoice creation flow inside OfferController. Add
a guard in the branch that builds the invoice line so you either skip processing
or fail gracefully when no matching line exists, and only read $firstLine->text,
$firstLine->tax_rate, $firstLine->tax_rate_id, and $firstLine->rate->rate after
confirming both $firstLine and $firstLine->rate are present. Also remove the
duplicate invoice_id assignment while updating the same invoice line setup.
| public function messages(): array | ||
| { | ||
| return [ | ||
| 'group.required' => 'Die Einstellungsgruppe ist erforderlich.', | ||
| 'group.in' => 'Die Einstellungsgruppe ist ungültig.', | ||
| 'key.required' => 'Der Einstellungsschlüssel ist erforderlich.', | ||
| ]; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
messages() referenziert nicht-existente Felder.
Die Schlüssel group.required, group.in und key.required gehören zu keinem der in rules() definierten Felder (invoice_type_id, should_summarize, deposit). Das sieht nach einem Copy-Paste-Rest aus einer anderen Request-Klasse aus – die deutschen Fehlermeldungen für diese Rechnungs-Request greifen dadurch nie.
🛠️ Vorschlag für passende Fehlermeldungen
public function messages(): array
{
return [
- 'group.required' => 'Die Einstellungsgruppe ist erforderlich.',
- 'group.in' => 'Die Einstellungsgruppe ist ungültig.',
- 'key.required' => 'Der Einstellungsschlüssel ist erforderlich.',
+ 'invoice_type_id.required' => 'Die Rechnungsart ist erforderlich.',
+ 'invoice_type_id.in' => 'Die Rechnungsart ist ungültig.',
+ 'should_summarize.required' => 'Bitte gib an, ob die Positionen zusammengefasst werden sollen.',
+ 'deposit.required' => 'Die Höhe der Anzahlung ist erforderlich.',
+ 'deposit.min' => 'Die Anzahlung muss mindestens :min € betragen.',
];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function messages(): array | |
| { | |
| return [ | |
| 'group.required' => 'Die Einstellungsgruppe ist erforderlich.', | |
| 'group.in' => 'Die Einstellungsgruppe ist ungültig.', | |
| 'key.required' => 'Der Einstellungsschlüssel ist erforderlich.', | |
| ]; | |
| } | |
| public function messages(): array | |
| { | |
| return [ | |
| 'invoice_type_id.required' => 'Die Rechnungsart ist erforderlich.', | |
| 'invoice_type_id.in' => 'Die Rechnungsart ist ungültig.', | |
| 'should_summarize.required' => 'Bitte gib an, ob die Positionen zusammengefasst werden sollen.', | |
| 'deposit.required' => 'Die Höhe der Anzahlung ist erforderlich.', | |
| 'deposit.min' => 'Die Anzahlung muss mindestens :min € betragen.', | |
| ]; | |
| } |
🤖 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/Requests/OfferInvoiceStoreRequest.php` around lines 24 - 31, In
OfferInvoiceStoreRequest::messages(), the custom validation messages still
reference unrelated fields from a different request, so they never apply to the
rules() in this class. Update the message keys to match the actual validated
attributes in OfferInvoiceStoreRequest (the fields defined in rules(), such as
invoice_type_id, should_summarize, and deposit) and replace the copied German
texts with messages that fit those validations.
|
@coderrabitai kannst du einen neuen review durchführen? |
|
@coderabbitai kannst du einen neuen review durchführen? |
|
✅ Action performedReview finished.
|
|
@coderabbitai kannst du einen neuen review durchführen? |
|
✅ Action performedReview finished.
|
1 participant
…n erstellt werden
Summary by CodeRabbit
Neue Funktionen
Verbesserungen