Update Sale.php - #4496
Conversation
comiting changes for the issue relating to the reward system
📝 WalkthroughWalkthroughA new public method Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Models/Sale.php`:
- Around line 441-452: In update($sale_id = null, $sale_data = null) add reward
reconciliation: within the same DB transaction that updates the sales row and
payments, load current sales_reward_points entries for the sale
(sales_reward_points), compute the existing reward amount total and compare to
the new payments payload (detect payment_type === 'Reward' and sum
payment_amount), calculate the delta, update or insert the sales_reward_points
row accordingly, and increment/decrement the customer's points balance
(customers.points) by that delta; ensure you also handle payment_type changes
to/from 'Reward' and rollback the transaction on any error so sale, payments,
reward ledger, and customer balance stay consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| public function update($sale_id = null, $sale_data = null): bool //Bug Identifyed: When a sale is edited, the reward points are not recalculated. | ||
| // This function needs a hook to the Rewards model to check if the payment_type | ||
| // has changed to or from 'Reward' to ensure the customer's balance is updated. | ||
| { | ||
| $builder = $this->db->table('sales'); | ||
| $builder->where('sale_id', $sale_id); | ||
| $update_data = $sale_data; | ||
| unset($update_data['payments']); | ||
| $success = $builder->update($update_data); | ||
|
|
||
| // DATA INTEGRITY ISSUE: If the payment_amount is edited here, the | ||
| // 'sales_reward_points' table does not receive an update, leading | ||
| // to a mismatch between the sale total and the customer's point balance. |
There was a problem hiding this comment.
The sale-edit path still does not reconcile rewards.
These comments describe the bug, but update() still only persists sales and sales_payments. Changing a payment to/from rewards, or changing the reward payment amount, leaves both customers.points and the sales_reward_points entry stale. Please load the existing reward ledger for the sale, compute the delta against the edited payments, and apply that inside the same transaction as the sale/payment update.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Models/Sale.php` around lines 441 - 452, In update($sale_id = null,
$sale_data = null) add reward reconciliation: within the same DB transaction
that updates the sales row and payments, load current sales_reward_points
entries for the sale (sales_reward_points), compute the existing reward amount
total and compare to the new payments payload (detect payment_type === 'Reward'
and sum payment_amount), calculate the delta, update or insert the
sales_reward_points row accordingly, and increment/decrement the customer's
points balance (customers.points) by that delta; ensure you also handle
payment_type changes to/from 'Reward' and rollback the transaction on any error
so sale, payments, reward ledger, and customer balance stay consistent.
| public function delete_sale(int $sale_id): bool | ||
| { | ||
| // CRITICAL BUG: Deleting a sale fails to restore 'used' points to the customer. | ||
| // We must query 'sales_reward_points' for this $sale_id and increment | ||
| // the 'points' column in the 'customers' table before completing the deletion. | ||
| return $this->db->table('sales')->where('sale_id', $sale_id)->delete(); | ||
| } |
There was a problem hiding this comment.
Avoid introducing a header-only hard delete for sales.
delete_sale() contradicts the existing delete() contract and removes only the sales row. That either leaves dependent sale rows / sales_reward_points behind or fails on FK constraints, and it never reverses the customer's net reward delta (used back, earned removed). The current refund path in app/Controllers/Sales.php:1531-1545 still calls delete(), so this helper also does not fix the live workflow. Put the reward reversal into the shared transactional delete path instead of adding a raw hard delete.
comiting changes for the issue relating to the reward system
updating rewards and
deleting reward in case of rewfunds
Summary by CodeRabbit
New Features
Documentation