Refactoring Offer-Status#573
Conversation
WalkthroughDer Backend-Controller und das Offer-Modell liefern jetzt Angebotsstatus-Listen über ChangesStatuslisten-Integration Backend und Frontend
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant OfferController
participant Offer
participant OfferDetailsFrontend
Client->>OfferController: show/terms/history Request
OfferController->>Offer: Angebotsdaten laden
OfferController-->>Client: Inertia Response inkl. statuses
Client->>OfferDetailsFrontend: statuses als Prop empfangen
OfferDetailsFrontend->>OfferDetailsFrontend: statuses.find() zur Statuslabel-Auflösung
Client->>OfferController: duplicate(offer)
OfferController->>Offer: neues Offer erstellen
OfferController->>Offer: status = PENDING
OfferController-->>Client: Redirect/Response
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/Http/Controllers/App/OfferController.php (1)
270-279: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFalsches Objekt erhält den neuen Status – Duplikat bleibt beim alten Status.
$offer->status = OfferStatusEnum::PENDING->value;ändert den Status des Original-Angebots, nicht des Duplikats, und es fehlt einsave(). Laut Änderungsbeschreibung soll aber der Status des Duplikats aufPENDINGgesetzt werden.Offer::duplicate()(sieheapp/Models/Offer.php) übernimmt den Status perreplicate()unverändert vom Original, sodass das neue Angebot ohne diese Korrektur weiterhin z. B.ACCEPTEDoderREJECTEDbleibt. Zusätzlich verpufft die aktuelle Zeile komplett wirkungslos, da$offernie gespeichert wird.🐛 Vorgeschlagener Fix
public function duplicate(Offer $offer) { $duplicatedOffer = Offer::duplicate($offer); - $offer->status = OfferStatusEnum::PENDING->value; + $duplicatedOffer->status = OfferStatusEnum::PENDING->value; + $duplicatedOffer->save(); $duplicatedOffer->addHistory('hat das Angebot erstellt.', 'created', auth()->user());🤖 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 270 - 279, The duplicate action in OfferController currently updates the original Offer instead of the cloned one, so the duplicated record keeps the old status. Change duplicate(Offer $offer) to set OfferStatusEnum::PENDING on the object returned by Offer::duplicate($offer), then persist that duplicated offer before returning the redirect; keep the history call on the duplicated record as it already is. Use the existing duplicate() workflow and Offer::duplicate() behavior in Offer.php to ensure the new Offer is the one saved with the pending status.app/Models/Offer.php (1)
252-286: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStatus beim Duplizieren direkt auf
PENDINGsetzen
Offer::duplicate()übernimmt den Originalstatus perreplicate(), und der Reset im Controller trifft nur das ursprüngliche$offer-Objekt ohnesave(). Dadurch bleibt das duplizierte Angebot auf dem alten Status.Den Reset direkt in
app/Models/Offer.phpvor demsave()setzen und die fehlerhafte Controller-Zeile 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/Offer.php` around lines 252 - 286, In Offer::duplicate(), the duplicated offer is currently keeping the original status because replicate() copies it and the controller reset only affects the source Offer instance. Set the duplicated model’s status to PENDING inside Offer::duplicate() before save(), alongside the other field resets on $duplicatedOffer, and remove the now-redundant controller-side status reset. Use the duplicate() method and $duplicatedOffer assignment block as the fix location.
🧹 Nitpick comments (1)
resources/js/Pages/App/Offer/OfferDetailsLayout.tsx (1)
150-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDebug-
console.logentfernen.
console.log(status)sieht nach einem vergessenen Debug-Artefakt aus und sollte vor dem Merge entfernt werden.🧹 Vorgeschlagener Fix
const handleStatusChange = async (status: string) => { - console.log(status) const statusName = statuses?.find(item => item.id === status)?.name🤖 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/Offer/OfferDetailsLayout.tsx` at line 150, Remove the մն leftover debug logging from OfferDetailsLayout by deleting the console.log(status) statement in the component logic; locate it via the OfferDetailsLayout function/component and ensure no console output remains in the merged code.
🤖 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/Http/Controllers/App/OfferController.php`:
- Around line 270-279: The duplicate action in OfferController currently updates
the original Offer instead of the cloned one, so the duplicated record keeps the
old status. Change duplicate(Offer $offer) to set OfferStatusEnum::PENDING on
the object returned by Offer::duplicate($offer), then persist that duplicated
offer before returning the redirect; keep the history call on the duplicated
record as it already is. Use the existing duplicate() workflow and
Offer::duplicate() behavior in Offer.php to ensure the new Offer is the one
saved with the pending status.
In `@app/Models/Offer.php`:
- Around line 252-286: In Offer::duplicate(), the duplicated offer is currently
keeping the original status because replicate() copies it and the controller
reset only affects the source Offer instance. Set the duplicated model’s status
to PENDING inside Offer::duplicate() before save(), alongside the other field
resets on $duplicatedOffer, and remove the now-redundant controller-side status
reset. Use the duplicate() method and $duplicatedOffer assignment block as the
fix location.
---
Nitpick comments:
In `@resources/js/Pages/App/Offer/OfferDetailsLayout.tsx`:
- Line 150: Remove the մն leftover debug logging from OfferDetailsLayout by
deleting the console.log(status) statement in the component logic; locate it via
the OfferDetailsLayout function/component and ensure no console output remains
in the merged code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37218543-3be3-4eee-96b1-53330b651fbd
📒 Files selected for processing (7)
app/Http/Controllers/App/OfferController.phpapp/Models/Offer.phpresources/js/Pages/App/Offer/OfferDetails.tsxresources/js/Pages/App/Offer/OfferDetailsLayout.tsxresources/js/Pages/App/Offer/OfferDetailsSide.tsxresources/js/Pages/App/Offer/OfferHistory.tsxresources/js/Pages/App/Offer/OfferTerms.tsx
1 participant
Summary by CodeRabbit
Neue Funktionen
Bug Fixes