Fix overtime_needs_pricing flag + add regression tests

The helper used log.overtime (which doesn't exist on WorkLog); the
correct field is overtime_amount. Combined with a defensive
`getattr(..., None) or 0`, the bug made the flag permanently False,
which would have silently hidden the 'Price now' banner in Tasks 3
and 4. Now reads overtime_amount directly (it's non-nullable with a
0.00 default, so no defensive shim is needed).

Adds 4 regression tests:
- test_overtime_needs_pricing_flag: the bug that just got fixed
- test_query_count_is_bounded: N+1 guard (4 queries regardless of worker count)
- test_empty_log_returns_zero_totals: log with no workers attached
- test_log_without_team_has_no_pay_period: log whose team became NULL

Also removes unused `reverse` import from tests.py.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Konrad du Plessis 2026-04-22 14:21:34 +02:00
parent 385d654082
commit b0aa35661b
2 changed files with 51 additions and 2 deletions

View File

@ -7,7 +7,6 @@ from decimal import Decimal
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.test import TestCase 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
from core.views import _build_work_log_payroll_context from core.views import _build_work_log_payroll_context
@ -109,3 +108,51 @@ class WorkLogPayrollContextTests(TestCase):
self.assertIsNotNone(end) self.assertIsNotNone(end)
self.assertLessEqual(start, self.log.date) self.assertLessEqual(start, self.log.date)
self.assertGreaterEqual(end, self.log.date) self.assertGreaterEqual(end, self.log.date)
def test_overtime_needs_pricing_flag(self):
"""Flag fires when log has OT hours but no priced_workers yet."""
# Start: no OT, no priced workers -> flag False
self.assertFalse(_build_work_log_payroll_context(self.log)['overtime_needs_pricing'])
# Add OT hours but keep priced_workers empty -> flag True
self.log.overtime_amount = Decimal('0.50')
self.log.priced_workers.clear()
self.log.save()
self.assertTrue(_build_work_log_payroll_context(self.log)['overtime_needs_pricing'])
# Price the OT -> flag False again
self.log.priced_workers.add(self.worker_a)
self.assertFalse(_build_work_log_payroll_context(self.log)['overtime_needs_pricing'])
def test_query_count_is_bounded(self):
"""Helper should not issue per-worker queries — guards against N+1."""
# 4 queries: payroll_records, priced_workers, workers.all, adjustments
# (plus whichever assertNumQueries overhead Django's test client adds).
# We assert a tight upper bound; regressions that add per-worker queries
# will push this well above the bound and fail the test.
with self.assertNumQueries(4):
_build_work_log_payroll_context(self.log)
def test_empty_log_returns_zero_totals(self):
"""Log with no workers: helper returns empty rows and zero totals."""
empty_log = WorkLog.objects.create(
date=datetime.date(2026, 4, 11),
project=self.project,
team=self.team,
supervisor=self.admin,
)
ctx = _build_work_log_payroll_context(empty_log)
self.assertEqual(ctx['worker_rows'], [])
self.assertEqual(ctx['total_earned'], Decimal('0.00'))
self.assertEqual(ctx['total_paid'], Decimal('0.00'))
self.assertEqual(ctx['total_outstanding'], Decimal('0.00'))
self.assertEqual(ctx['adjustments'], [])
def test_log_without_team_has_no_pay_period(self):
"""Log whose team was later soft-deleted to NULL still works."""
self.log.team = None
self.log.save()
ctx = _build_work_log_payroll_context(self.log)
self.assertEqual(ctx['pay_period'], (None, None))
# The rest of the context should still populate correctly.
self.assertEqual(len(ctx['worker_rows']), 3)

View File

@ -796,7 +796,9 @@ def _build_work_log_payroll_context(log):
pay_period = get_pay_period(log.team, reference_date=log.date) if log.team else (None, None) pay_period = get_pay_period(log.team, reference_date=log.date) if log.team else (None, None)
# Overtime "needs pricing" flag: log has OT hours but no priced_workers yet. # Overtime "needs pricing" flag: log has OT hours but no priced_workers yet.
log_overtime = getattr(log, 'overtime', None) or 0 # log.overtime_amount is a Decimal with default=0.00 — always present on saved
# instances, so no defensive getattr needed. Compare via Decimal arithmetic.
log_overtime = log.overtime_amount or Decimal('0.00')
overtime_needs_pricing = log_overtime > 0 and not priced_worker_ids overtime_needs_pricing = log_overtime > 0 and not priced_worker_ids
return { return {