diff --git a/core/tests.py b/core/tests.py index 6c021cd..fdb87a6 100644 --- a/core/tests.py +++ b/core/tests.py @@ -9,7 +9,7 @@ from django.contrib.auth.models import User from django.test import TestCase from django.urls import reverse -from core.models import Project, Team, Worker, WorkLog, PayrollRecord, PayrollAdjustment +from core.models import Project, Team, Worker, WorkLog, PayrollRecord, PayrollAdjustment, Loan from core.views import _build_work_log_payroll_context, _build_report_context @@ -1133,3 +1133,88 @@ class AdjustmentsTabTests(TestCase): self.assertEqual(resp.status_code, 403) # a1 still present self.assertTrue(PayrollAdjustment.objects.filter(id=self.a1.id).exists()) + + def test_bulk_delete_cascades_new_loan(self): + """Bulk-deleting a 'New Loan' adjustment must also delete its + linked Loan row AND any still-unpaid Loan Repayment adjustments + — same cascade as the single-row delete_adjustment view. Without + this, the bulk endpoint would orphan Loan rows and leave pending + repayments in place.""" + # Create a Loan + New Loan adjustment + unpaid repayment + loan = Loan.objects.create( + worker=self.w1, + principal_amount=Decimal('1000'), + remaining_balance=Decimal('1000'), + date=datetime.date(2026, 4, 1), + loan_type='loan', + ) + new_loan_adj = PayrollAdjustment.objects.create( + worker=self.w1, project=self.proj, type='New Loan', + amount=Decimal('1000'), date=datetime.date(2026, 4, 1), + description='Test loan', loan=loan, + ) + unpaid_repayment = PayrollAdjustment.objects.create( + worker=self.w1, project=self.proj, type='Loan Repayment', + amount=Decimal('500'), date=datetime.date(2026, 5, 1), + description='First repayment', loan=loan, + ) + + self._login_admin() + resp = self.client.post( + reverse('bulk_delete_adjustments'), + {'adjustment_ids': [new_loan_adj.id]}, + ) + self.assertEqual(resp.status_code, 200) + body = resp.json() + self.assertEqual(body['deleted'], 1) + + # New Loan adjustment gone + self.assertFalse(PayrollAdjustment.objects.filter(id=new_loan_adj.id).exists()) + # Linked Loan row gone (cascade) + self.assertFalse(Loan.objects.filter(id=loan.id).exists()) + # Unpaid repayment gone (cascade) + self.assertFalse(PayrollAdjustment.objects.filter(id=unpaid_repayment.id).exists()) + + def test_bulk_delete_skips_loan_with_paid_repayments(self): + """If a 'New Loan' has any paid repayments, bulk-delete must + refuse to delete it (would lose audit trail). Other rows in the + batch are unaffected.""" + loan = Loan.objects.create( + worker=self.w1, + principal_amount=Decimal('1000'), + remaining_balance=Decimal('500'), + date=datetime.date(2026, 4, 1), + loan_type='loan', + ) + new_loan_adj = PayrollAdjustment.objects.create( + worker=self.w1, project=self.proj, type='New Loan', + amount=Decimal('1000'), date=datetime.date(2026, 4, 1), + description='Test loan', loan=loan, + ) + # One PAID repayment + pr = PayrollRecord.objects.create( + worker=self.w1, date=datetime.date(2026, 5, 1), + amount_paid=Decimal('500'), + ) + PayrollAdjustment.objects.create( + worker=self.w1, project=self.proj, type='Loan Repayment', + amount=Decimal('500'), date=datetime.date(2026, 5, 1), + description='Paid', loan=loan, payroll_record=pr, + ) + + self._login_admin() + # Send the New Loan plus a2 (unpaid Bonus) — expect only a2 to delete + resp = self.client.post( + reverse('bulk_delete_adjustments'), + {'adjustment_ids': [new_loan_adj.id, self.a2.id]}, + ) + self.assertEqual(resp.status_code, 200) + body = resp.json() + self.assertEqual(body['deleted'], 1) # only a2 + self.assertEqual(body['requested'], 2) + self.assertEqual(body['skipped_reasons'], {'has_paid_repayments': 1}) + + # New Loan survives, Loan survives, a2 gone + self.assertTrue(PayrollAdjustment.objects.filter(id=new_loan_adj.id).exists()) + self.assertTrue(Loan.objects.filter(id=loan.id).exists()) + self.assertFalse(PayrollAdjustment.objects.filter(id=self.a2.id).exists()) diff --git a/core/views.py b/core/views.py index e0871f8..65c447c 100644 --- a/core/views.py +++ b/core/views.py @@ -3890,6 +3890,63 @@ def edit_adjustment(request, adj_id): # Removes an unpaid adjustment. Handles cascade logic for Loans and Overtime. # ============================================================================= +# ============================================================================= +# === ADJUSTMENT CASCADE DELETE HELPER === +# Shared by delete_adjustment (single row) and bulk_delete_adjustments (many +# rows) so both paths have identical semantics. "New Loan" and "Advance +# Payment" each own a linked Loan row that needs teardown; "Overtime" needs +# its worker un-priced from the WorkLog. Without this helper, bulk-delete +# would orphan Loan rows and leave priced_workers stale. +# ============================================================================= + + +def _delete_adjustment_with_cascade(adj): + """Delete one PayrollAdjustment, cascading through its linked objects. + + Returns a tuple `(ok: bool, reason: str or None)`: + - `(True, None)` — row deleted successfully (with any cascades done) + - `(False, 'paid')` — adjustment already paid; refuse + - `(False, 'has_paid_repayments')` — linked Loan has paid repayments; + deleting it would lose the repayment audit trail + + Cascade rules: + - New Loan / Advance Payment: delete the linked `Loan` row plus any + still-unpaid repayment adjustments. If ANY repayment has already + been paid, abort (otherwise we'd lose history of money that + already moved). + - Overtime: remove the worker from work_log.priced_workers so the + overtime can be re-priced cleanly later. + - Other types: plain delete, no cascade. + """ + if adj.payroll_record is not None: + return False, 'paid' + + adj_type = adj.type + + if adj_type in ('New Loan', 'Advance Payment') and adj.loan: + repayment_type = 'Advance Repayment' if adj_type == 'Advance Payment' else 'Loan Repayment' + paid_repayments = PayrollAdjustment.objects.filter( + loan=adj.loan, + type=repayment_type, + payroll_record__isnull=False, + ) + if paid_repayments.exists(): + return False, 'has_paid_repayments' + # Delete all still-unpaid repayments, then the Loan itself + PayrollAdjustment.objects.filter( + loan=adj.loan, + type=repayment_type, + payroll_record__isnull=True, + ).delete() + adj.loan.delete() + elif adj_type == 'Overtime' and adj.work_log: + # "Un-price" the overtime — worker can be re-priced cleanly later + adj.work_log.priced_workers.remove(adj.worker) + + adj.delete() + return True, None + + @login_required def delete_adjustment(request, adj_id): if request.method != 'POST': @@ -3898,48 +3955,21 @@ def delete_adjustment(request, adj_id): return HttpResponseForbidden("Not authorized.") adj = get_object_or_404(PayrollAdjustment, id=adj_id) - - # Can't delete adjustments that have been paid - if adj.payroll_record is not None: - messages.error(request, 'Cannot delete a paid adjustment.') - return redirect('payroll_dashboard') - adj_type = adj.type worker_name = adj.worker.name - # === CASCADE DELETE for New Loan and Advance Payment === - # Both create Loan records that need cleanup when deleted. - if adj_type in ('New Loan', 'Advance Payment') and adj.loan: - # Determine which repayment type to look for - repayment_type = 'Advance Repayment' if adj_type == 'Advance Payment' else 'Loan Repayment' - - # Check if any paid repayments exist for this loan/advance - paid_repayments = PayrollAdjustment.objects.filter( - loan=adj.loan, - type=repayment_type, - payroll_record__isnull=False, - ) - if paid_repayments.exists(): + ok, reason = _delete_adjustment_with_cascade(adj) + if not ok: + if reason == 'paid': + messages.error(request, 'Cannot delete a paid adjustment.') + elif reason == 'has_paid_repayments': label = 'advance' if adj_type == 'Advance Payment' else 'loan' messages.error( request, f'Cannot delete {label} for {worker_name} — it has paid repayments.' ) - return redirect('payroll_dashboard') + return redirect('payroll_dashboard') - # Delete all unpaid repayments for this loan/advance, then the loan itself - PayrollAdjustment.objects.filter( - loan=adj.loan, - type=repayment_type, - payroll_record__isnull=True, - ).delete() - adj.loan.delete() - - elif adj_type == 'Overtime' and adj.work_log: - # "Un-price" the overtime — remove worker from priced_workers M2M - adj.work_log.priced_workers.remove(adj.worker) - - adj.delete() messages.success(request, f'{adj_type} adjustment for {worker_name} deleted.') return redirect('payroll_dashboard') @@ -3956,31 +3986,49 @@ def delete_adjustment(request, adj_id): @login_required @require_POST def bulk_delete_adjustments(request): - """Delete multiple unpaid PayrollAdjustment rows in one DB call. + """Delete multiple unpaid PayrollAdjustment rows with full cascade. Body (form-encoded): `adjustment_ids` — repeated once per ID. - Returns JSON: `{"deleted": N, "requested": M}`. + Returns JSON: `{"deleted": N, "requested": M, "skipped_reasons": {...}}`. Admin-only; supervisors get 403. POST-only; anything else gets 405 from @require_POST. + + Cascade: each row is deleted via `_delete_adjustment_with_cascade` + (shared with the single-row `delete_adjustment` view) so bulk and + single-row have identical semantics. Rows that fail (already paid, + or a linked loan with paid repayments) are counted in `skipped_reasons` + but don't block the rest of the batch. """ if not is_admin(request.user): return JsonResponse({'error': 'Admin access required'}, status=403) - ids = request.POST.getlist('adjustment_ids') - # Defensive filter: only DELETE rows that are still unpaid. - # If a user's browser had stale state showing a paid row as - # unpaid, this quietly skips it instead of destroying payroll - # history. - to_delete = PayrollAdjustment.objects.filter( + # Int-coerce and drop non-digit values (defensive against garbled input — + # ids are client-generated so any non-digit would crash the queryset). + raw_ids = request.POST.getlist('adjustment_ids') + ids = [int(v) for v in raw_ids if v.strip().isdigit()] + + # Fetch each adjustment individually — we need the cascade helper to + # operate per-row (it deletes the linked Loan / unprices Overtime). + # Pre-filtering for .payroll_record__isnull=True is fine as an upfront + # short-circuit but the helper double-checks anyway. + adjustments = list(PayrollAdjustment.objects.filter( id__in=ids, payroll_record__isnull=True, - ) - deleted_count = to_delete.count() - to_delete.delete() + ).select_related('loan', 'work_log', 'worker')) + + deleted = 0 + skipped_reasons = {} + for adj in adjustments: + ok, reason = _delete_adjustment_with_cascade(adj) + if ok: + deleted += 1 + else: + skipped_reasons[reason] = skipped_reasons.get(reason, 0) + 1 return JsonResponse({ - 'deleted': deleted_count, + 'deleted': deleted, 'requested': len(ids), + 'skipped_reasons': skipped_reasons, })