refactor(report): drop dead year_projects context + SQL cost velocity
Two unrelated cleanups in `_build_report_context` and the helper next to it. - Removed `year_projects`, `year_teams`, and `current_year` from the report context dict. No template ever rendered them — they were added 2026-04 as part of an executive-report design that never shipped that section. Each render fired 2 extra GROUP BY queries for nothing. - `_company_cost_velocity` no longer loops every (work_log × worker) pair in Python. Single SQL aggregate (`Sum(monthly_salary / 20)`) instead — one round-trip regardless of dataset size. Old behaviour loaded the entire WorkLog table + M2M into memory for the hero KPI card. Regression test (`test_sql_aggregate_matches_python_loop`) uses the old Python loop as the expected oracle. Findings 14 + 16. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e797a71b94
commit
3ef6db71c9
@ -537,6 +537,68 @@ class CompanyCostVelocityTests(TestCase):
|
||||
self.assertEqual(result['working_days'], 1) # not 2
|
||||
|
||||
|
||||
class ReportContextDeadKeysTests(TestCase):
|
||||
"""Regression for Finding 14: the report context used to emit
|
||||
`year_projects`, `year_teams`, and `current_year` — none of which
|
||||
any template consumed. Removing them saved 2 GROUP BY queries per
|
||||
/report/ render. This test ensures they don't come back."""
|
||||
|
||||
def setUp(self):
|
||||
self.admin = User.objects.create_user(username='dead-keys', is_staff=True)
|
||||
|
||||
def test_year_projects_not_in_context(self):
|
||||
ctx = _build_report_context(
|
||||
datetime.date(2026, 1, 1), datetime.date(2026, 1, 31),
|
||||
)
|
||||
self.assertNotIn('year_projects', ctx)
|
||||
self.assertNotIn('year_teams', ctx)
|
||||
self.assertNotIn('current_year', ctx)
|
||||
|
||||
|
||||
class CompanyCostVelocitySQLAggregateTests(TestCase):
|
||||
"""Regression for Finding 16: _company_cost_velocity used to do a
|
||||
Python loop summing worker.daily_rate. Now uses a single SQL
|
||||
aggregate. Both implementations must produce the same result."""
|
||||
|
||||
def setUp(self):
|
||||
self.admin = User.objects.create_user(username='cv-sql', is_staff=True)
|
||||
self.project = Project.objects.create(name='SQL CV')
|
||||
# 3 workers at different salaries so the result is sensitive to
|
||||
# any miscalculation (multiplier, division order, etc).
|
||||
self.w1 = Worker.objects.create(name='A', id_number='C1', monthly_salary=Decimal('4000'))
|
||||
self.w2 = Worker.objects.create(name='B', id_number='C2', monthly_salary=Decimal('6000'))
|
||||
self.w3 = Worker.objects.create(name='C', id_number='C3', monthly_salary=Decimal('8000'))
|
||||
# Log on 3 dates, different worker combos
|
||||
log1 = WorkLog.objects.create(
|
||||
date=datetime.date(2026, 3, 1), project=self.project, supervisor=self.admin,
|
||||
)
|
||||
log1.workers.add(self.w1, self.w2, self.w3) # All three
|
||||
log2 = WorkLog.objects.create(
|
||||
date=datetime.date(2026, 3, 2), project=self.project, supervisor=self.admin,
|
||||
)
|
||||
log2.workers.add(self.w1, self.w2)
|
||||
log3 = WorkLog.objects.create(
|
||||
date=datetime.date(2026, 3, 3), project=self.project, supervisor=self.admin,
|
||||
)
|
||||
log3.workers.add(self.w3)
|
||||
|
||||
def test_sql_aggregate_matches_python_loop(self):
|
||||
"""SQL aggregate must match the result of summing daily_rate in Python."""
|
||||
from core.views import _company_cost_velocity
|
||||
# Expected via Python loop (the OLD implementation)
|
||||
total_cost_python = Decimal('0.00')
|
||||
for wl in WorkLog.objects.prefetch_related('workers').all():
|
||||
for worker in wl.workers.all():
|
||||
total_cost_python += worker.daily_rate
|
||||
# 3 distinct dates
|
||||
expected_days = 3
|
||||
expected_avg_daily = (total_cost_python / expected_days).quantize(Decimal('0.01'))
|
||||
|
||||
result = _company_cost_velocity()
|
||||
self.assertEqual(result['working_days'], expected_days)
|
||||
self.assertEqual(result['avg_daily'], expected_avg_daily)
|
||||
|
||||
|
||||
class CurrentOutstandingInScopeTests(TestCase):
|
||||
"""Hero card 2 — 'Outstanding NOW' with optional filter scope."""
|
||||
|
||||
|
||||
@ -279,13 +279,27 @@ def _compute_outstanding(project_ids=None, team_ids=None, include_inactive_worke
|
||||
# =============================================================================
|
||||
|
||||
def _company_cost_velocity():
|
||||
"""Return company-wide avg daily and monthly labour cost (lifetime)."""
|
||||
# Total lifetime labour cost: sum of (worker.daily_rate) over every
|
||||
# (log, worker) pair that has ever been logged.
|
||||
total_cost = Decimal('0.00')
|
||||
for wl in WorkLog.objects.prefetch_related('workers').all():
|
||||
for worker in wl.workers.all():
|
||||
total_cost += worker.daily_rate
|
||||
"""Return company-wide avg daily and monthly labour cost (lifetime).
|
||||
|
||||
PERF: this used to be a Python loop summing `worker.daily_rate` over
|
||||
every (log, worker) pair, which loaded the entire WorkLog table
|
||||
plus all M2M worker rows into memory. Now does it in a single SQL
|
||||
aggregate over the M2M through-table — one round-trip regardless
|
||||
of size.
|
||||
|
||||
Caveat: the SQL aggregate uses `monthly_salary / 20` directly,
|
||||
which is the same formula as `Worker.daily_rate`. If the property
|
||||
ever changes, this helper must be updated too. The
|
||||
DailyRateSemantics CLAUDE.md note flags this duality.
|
||||
"""
|
||||
# Single SQL aggregate: sum (workers__monthly_salary / 20) over every
|
||||
# (work_log × worker) pair in the M2M.
|
||||
total_cost = WorkLog.objects.aggregate(
|
||||
t=Sum(F('workers__monthly_salary') / Decimal('20'))
|
||||
)['t'] or Decimal('0.00')
|
||||
# Quantize once at the end (intermediate Decimal arithmetic keeps full
|
||||
# precision; final 2dp display goes through `money` filter anyway).
|
||||
total_cost = Decimal(total_cost).quantize(Decimal('0.01'))
|
||||
|
||||
# Distinct work-log dates = working days
|
||||
working_days = WorkLog.objects.values('date').distinct().count()
|
||||
@ -2535,19 +2549,10 @@ def _build_report_context(start_date, end_date, project_ids=None, team_ids=None)
|
||||
all_time_logs.filter(team__isnull=False), 'team__name', 'team'
|
||||
)
|
||||
|
||||
# --- THIS YEAR: project and team costs for the current calendar year ---
|
||||
current_year = timezone.now().year
|
||||
year_start = datetime.date(current_year, 1, 1)
|
||||
year_end = datetime.date(current_year, 12, 31)
|
||||
year_logs = WorkLog.objects.filter(date__gte=year_start, date__lte=year_end)
|
||||
if project_ids:
|
||||
year_logs = year_logs.filter(project_id__in=project_ids)
|
||||
if team_ids:
|
||||
year_logs = year_logs.filter(team_id__in=team_ids)
|
||||
year_projects = _get_labour_costs(year_logs, 'project__name', 'project')
|
||||
year_teams = _get_labour_costs(
|
||||
year_logs.filter(team__isnull=False), 'team__name', 'team'
|
||||
)
|
||||
# NOTE: "This year" project/team breakdowns used to be computed here
|
||||
# and emitted as `year_projects` / `year_teams` / `current_year`,
|
||||
# but no template ever consumed them — removed May 2026 as dead
|
||||
# context (saves 2 extra GROUP BY queries per /report/ render).
|
||||
|
||||
# --- Loans & Advances Outstanding (current balances) ---
|
||||
# team filter goes through worker__teams (M2M). Use the subquery pattern
|
||||
@ -2658,12 +2663,9 @@ def _build_report_context(start_date, end_date, project_ids=None, team_ids=None)
|
||||
'advances_outstanding': advances_outstanding,
|
||||
'loans_issued': loans_issued,
|
||||
'advances_issued': advances_issued,
|
||||
# --- All Time & Year context ---
|
||||
# --- All Time context ---
|
||||
'alltime_projects': alltime_projects,
|
||||
'alltime_teams': alltime_teams,
|
||||
'current_year': current_year,
|
||||
'year_projects': year_projects,
|
||||
'year_teams': year_teams,
|
||||
# --- Selected Period tables ---
|
||||
'payments_by_date': payments_by_date,
|
||||
'cost_per_project': cost_per_project,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user