admin/38686-vm
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor _build_report_context signature to multi-value filters

Browse Source 
project_id/team_id become project_ids/team_ids (list[int] or None).
Every internal filter uses the __in lookup; M2M filters use the
id__in subquery pattern documented in CLAUDE.md's Django ORM gotcha.
generate_report and generate_report_pdf switch to request.GET.getlist.
Old URL ?project=1 still works - getlist returns a single-element list.

Return dict gains six hero-KPI keys: current_outstanding, current_as_of,
company_avg_daily, company_avg_monthly, company_working_days,
team_project_activity - ready for the template restructure in Tasks 9-12.

Tests: 3 new multi-filter tests; existing inflation tests updated to the
new kwarg names. 42 total, all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Konrad du Plessis 2026-04-22 22:53:16 +02:00
parent ea1e4bdbcb
commit 16d192d5fc
2 changed files with 151 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
core/tests.py
View File

@ -368,28 +368,28 @@ class ReportContextFilterInflationTests(TestCase):
)
self.record.work_logs.add(*self.logs)
def _ctx(self, project_id=None, team_id=None):
def _ctx(self, project_ids=None, team_ids=None):
return _build_report_context(
datetime.date(2026, 3, 1),
datetime.date(2026, 3, 31),
project_id=project_id,
team_id=team_id,
project_ids=project_ids,
team_ids=team_ids,
)
def test_worker_breakdown_not_inflated_with_project_filter_only(self):
ctx = self._ctx(project_id=self.project.id)
ctx = self._ctx(project_ids=[self.project.id])
self.assertEqual(len(ctx['worker_breakdown']), 1)
# Pre-fix: this was 600 × 3 = 1800 (one JOIN, 3-way inflation).
self.assertEqual(ctx['worker_breakdown'][0]['total_paid'], Decimal('600.00'))
def test_worker_breakdown_not_inflated_with_both_filters(self):
ctx = self._ctx(project_id=self.project.id, team_id=self.team.id)
ctx = self._ctx(project_ids=[self.project.id], team_ids=[self.team.id])
self.assertEqual(len(ctx['worker_breakdown']), 1)
# Pre-fix: this was 600 × 3 × 3 = 5400 (two JOINs, 9-way inflation).
self.assertEqual(ctx['worker_breakdown'][0]['total_paid'], Decimal('600.00'))
def test_payments_by_date_not_inflated_with_both_filters(self):
ctx = self._ctx(project_id=self.project.id, team_id=self.team.id)
ctx = self._ctx(project_ids=[self.project.id], team_ids=[self.team.id])
payments = list(ctx['payments_by_date'])
self.assertEqual(len(payments), 1)
self.assertEqual(payments[0]['total'], Decimal('600.00'))
@ -398,7 +398,7 @@ class ReportContextFilterInflationTests(TestCase):
"""Regression guard: total_paid_out was ALREADY correct pre-fix
because .aggregate() handles distinct() via a subquery. Lock it in
so a future refactor doesn't accidentally reintroduce inflation here."""
ctx = self._ctx(project_id=self.project.id, team_id=self.team.id)
ctx = self._ctx(project_ids=[self.project.id], team_ids=[self.team.id])
self.assertEqual(ctx['total_paid_out'], Decimal('600.00'))
def test_adjustment_summary_not_inflated_with_team_filter(self):
@ -413,7 +413,7 @@ class ReportContextFilterInflationTests(TestCase):
date=datetime.date(2026, 3, 10),
description='Test bonus',
)
ctx = self._ctx(project_id=self.project.id, team_id=self.team.id)
ctx = self._ctx(project_ids=[self.project.id], team_ids=[self.team.id])
totals = {item['type']: item['total'] for item in ctx['adjustment_totals']}
self.assertEqual(totals.get('Bonus'), Decimal('100.00'))
@ -692,3 +692,60 @@ class ChapterOneEnrichmentTests(TestCase):
self.assertEqual(by_name['C1']['working_days'], 4)
self.assertEqual(by_name['C1']['avg_per_working_day'], Decimal('200.00'))
self.assertEqual(by_name['C1']['start_date'], datetime.date(2026, 1, 1))
# =============================================================================
# === TESTS FOR MULTI-VALUE FILTER SUPPORT (Task 6) ===
# _build_report_context now accepts project_ids and team_ids lists.
# =============================================================================
class ReportMultiFilterTests(TestCase):
"""Task 6 — multi-value project_ids / team_ids filters."""
def setUp(self):
self.admin = User.objects.create_user(username='mf', is_staff=True)
self.p1 = Project.objects.create(name='P1')
self.p2 = Project.objects.create(name='P2')
self.p3 = Project.objects.create(name='P3')
self.team = Team.objects.create(name='T', supervisor=self.admin)
self.w = Worker.objects.create(
name='W', id_number='W1', monthly_salary=Decimal('4000')
)
self.team.workers.add(self.w)
# One log + one paid record per project
for proj in (self.p1, self.p2, self.p3):
log = WorkLog.objects.create(
date=datetime.date(2026, 3, 1),
project=proj, team=self.team, supervisor=self.admin,
)
log.workers.add(self.w)
rec = PayrollRecord.objects.create(
worker=self.w, amount_paid=Decimal('100.00'),
date=datetime.date(2026, 3, 5),
)
rec.work_logs.add(log)
def _ctx(self, project_ids=None, team_ids=None):
from core.views import _build_report_context
return _build_report_context(
datetime.date(2026, 3, 1), datetime.date(2026, 3, 31),
project_ids=project_ids, team_ids=team_ids,
)
def test_multi_project_union(self):
ctx = self._ctx(project_ids=[self.p1.id, self.p2.id])
# Two projects paid R 100 each = R 200; third excluded
self.assertEqual(ctx['total_paid_out'], Decimal('200.00'))
def test_empty_list_equals_none(self):
ctx_none = self._ctx(project_ids=None)
ctx_empty = self._ctx(project_ids=[])
self.assertEqual(ctx_none['total_paid_out'], ctx_empty['total_paid_out'])
def test_no_inflation_with_multi_project(self):
"""Worker breakdown must not inflate when multiple projects are selected."""
ctx = self._ctx(project_ids=[self.p1.id, self.p2.id, self.p3.id])
self.assertEqual(len(ctx['worker_breakdown']), 1)
# All three records are for the same worker, R 100 each = R 300
self.assertEqual(ctx['worker_breakdown'][0]['total_paid'], Decimal('300.00'))

123
core/views.py
View File

@ -2027,11 +2027,16 @@ def _get_labour_costs(work_logs_qs, group_by_field, name_key):
]
def _build_report_context(start_date, end_date, project_id=None, team_id=None):
def _build_report_context(start_date, end_date, project_ids=None, team_ids=None):
"""
Compute all report data for the given date range and filters.
Returns a dictionary of totals, breakdowns, and worker-level data.
project_ids / team_ids are lists of ints (from request.GET.getlist).
None or [] are treated as "no filter" — returning data for every project
or every team respectively. A single-element list like [3] reproduces
the old single-id behaviour (so old URLs like ?project=3 still work).
Key design decision: "Worker-Days" counts total worker×log entries
(not distinct calendar dates). This correlates correctly with cost —
if 5 workers work 22 days, that's 110 worker-days, and
@ -2043,7 +2048,7 @@ def _build_report_context(start_date, end_date, project_id=None, team_id=None):
# --- PayrollRecords in range ---
#
# IMPORTANT — avoid M2M double-JOIN inflation:
# Chaining `.filter(work_logs__project_id=X).distinct().filter(work_logs__team_id=Y)`
# Chaining `.filter(work_logs__project_id__in=X).distinct().filter(work_logs__team_id__in=Y)`
# creates TWO separate JOIN aliases on core_payrollrecord_work_logs. Any
# later `.values().annotate(Sum())` then aggregates across the cartesian
# product of matching rows, inflating per-worker and per-date totals by
@ -2053,16 +2058,16 @@ def _build_report_context(start_date, end_date, project_id=None, team_id=None):
# use id__in subqueries to keep the outer queryset JOIN-free.
# See ReportContextFilterInflationTests for regression coverage.
records = PayrollRecord.objects.filter(date_filter)
if project_id:
if project_ids:
records = records.filter(
id__in=PayrollRecord.objects.filter(
work_logs__project_id=project_id
work_logs__project_id__in=project_ids
).values('id')
)
if team_id:
if team_ids:
records = records.filter(
id__in=PayrollRecord.objects.filter(
work_logs__team_id=team_id
work_logs__team_id__in=team_ids
).values('id')
)
@ -2077,23 +2082,23 @@ def _build_report_context(start_date, end_date, project_id=None, team_id=None):
)
# --- Adjustments in range ---
# project_id filters via an FK column (no JOIN inflation risk), but
# team_id goes through worker__teams M2M — apply the same subquery
# project_ids filters via an FK column (no JOIN inflation risk), but
# team_ids goes through worker__teams M2M — apply the same subquery
# pattern as above to keep adj_by_type's values().annotate(Sum()) safe.
adjustments = PayrollAdjustment.objects.filter(date_filter)
if project_id:
adjustments = adjustments.filter(project_id=project_id)
if team_id:
if project_ids:
adjustments = adjustments.filter(project_id__in=project_ids)
if team_ids:
adjustments = adjustments.filter(
worker__in=Worker.objects.filter(teams__id=team_id).values('id')
worker__in=Worker.objects.filter(teams__id__in=team_ids).values('id')
)
# --- Work Logs in range (for calculating actual labour cost) ---
work_logs_qs = WorkLog.objects.filter(date__gte=start_date, date__lte=end_date)
if project_id:
work_logs_qs = work_logs_qs.filter(project_id=project_id)
if team_id:
work_logs_qs = work_logs_qs.filter(team_id=team_id)
if project_ids:
work_logs_qs = work_logs_qs.filter(project_id__in=project_ids)
if team_ids:
work_logs_qs = work_logs_qs.filter(team_id__in=team_ids)
# Total worker-days across all work logs (counts M2M worker entries)
total_worker_days = work_logs_qs.aggregate(
@ -2110,10 +2115,10 @@ def _build_report_context(start_date, end_date, project_id=None, team_id=None):
# --- ALL TIME: project and team costs since the very first work log ---
all_time_logs = WorkLog.objects.all()
if project_id:
all_time_logs = all_time_logs.filter(project_id=project_id)
if team_id:
all_time_logs = all_time_logs.filter(team_id=team_id)
if project_ids:
all_time_logs = all_time_logs.filter(project_id__in=project_ids)
if team_ids:
all_time_logs = all_time_logs.filter(team_id__in=team_ids)
# === CHAPTER I — All Time Projects (enriched) ===
# Adds working_days and avg_per_working_day (the 2026-04-23 design).
# Can't just extend _get_labour_costs because that helper is used by
@ -2153,19 +2158,24 @@ def _build_report_context(start_date, end_date, project_id=None, team_id=None):
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_id:
year_logs = year_logs.filter(project_id=project_id)
if team_id:
year_logs = year_logs.filter(team_id=team_id)
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'
)
# --- Loans & Advances Outstanding (current balances) ---
# team filter goes through worker__teams (M2M). Use the subquery pattern
# (CLAUDE.md Django ORM gotcha) so we don't pick up JOIN inflation on the
# aggregate.
active_loans = Loan.objects.filter(active=True, date__lte=end_date)
if team_id:
active_loans = active_loans.filter(worker__teams__id=team_id)
if team_ids:
active_loans = active_loans.filter(
worker__in=Worker.objects.filter(teams__id__in=team_ids).values('id')
)
loans_outstanding = active_loans.filter(loan_type='loan').aggregate(
total=Sum('remaining_balance'))['total'] or Decimal('0.00')
advances_outstanding = active_loans.filter(loan_type='advance').aggregate(
@ -2174,9 +2184,10 @@ def _build_report_context(start_date, end_date, project_id=None, team_id=None):
# --- Loans & Advances Issued This Period ---
loans_issued_qs = Loan.objects.filter(date_filter, loan_type='loan')
advances_issued_qs = Loan.objects.filter(date_filter, loan_type='advance')
if team_id:
loans_issued_qs = loans_issued_qs.filter(worker__teams__id=team_id)
advances_issued_qs = advances_issued_qs.filter(worker__teams__id=team_id)
if team_ids:
team_worker_ids = Worker.objects.filter(teams__id__in=team_ids).values('id')
loans_issued_qs = loans_issued_qs.filter(worker__in=team_worker_ids)
advances_issued_qs = advances_issued_qs.filter(worker__in=team_worker_ids)
loans_issued = loans_issued_qs.aggregate(
total=Sum('principal_amount'))['total'] or Decimal('0.00')
advances_issued = advances_issued_qs.aggregate(
@ -2238,11 +2249,26 @@ def _build_report_context(start_date, end_date, project_id=None, team_id=None):
'adj_values': adj_values,
})
# === Hero KPI band data (executive report v2) ===
# Small helpers that power the new hero band at the top of the report.
# Kept separate so the big return dict stays easy to scan.
_cv = _company_cost_velocity()
return {
'start_date': start_date,
'end_date': end_date,
'project_name': Project.objects.get(id=project_id).name if project_id else 'All Projects',
'team_name': Team.objects.get(id=team_id).name if team_id else 'All Teams',
'project_name': (
', '.join(
Project.objects.filter(id__in=project_ids).values_list('name', flat=True)
)
if project_ids else 'All Projects'
),
'team_name': (
', '.join(
Team.objects.filter(id__in=team_ids).values_list('name', flat=True)
)
if team_ids else 'All Teams'
),
# --- Summary ---
'total_paid_out': total_paid_out,
'total_worker_days': total_worker_days,
@ -2264,6 +2290,15 @@ def _build_report_context(start_date, end_date, project_id=None, team_id=None):
'active_adj_types': active_adj_types,
'active_adj_labels': active_adj_labels,
'worker_breakdown': worker_breakdown,
# --- Hero KPI band (executive report v2) ---
'current_outstanding': _current_outstanding_in_scope(
project_ids=project_ids, team_ids=team_ids
),
'current_as_of': timezone.now(),
'company_avg_daily': _cv['avg_daily'],
'company_avg_monthly': _cv['avg_monthly'],
'company_working_days': _cv['working_days'],
'team_project_activity': _team_project_activity(work_logs_qs),
}
@ -2326,15 +2361,22 @@ def generate_report(request):
# Parse dates — supports both "month" and "start_date/end_date" params
start_date, end_date = _parse_report_dates(request)
project_id = request.GET.get('project') or None
team_id = request.GET.get('team') or None
# Multi-value: ?project=1&project=2 comes in as getlist ['1','2'].
# Cast to ints; drop empties. None if list is empty (= "no filter").
def _ids(name):
return [int(v) for v in request.GET.getlist(name) if v.strip().isdigit()]
project_ids = _ids('project') or None
team_ids = _ids('team') or None
if not start_date or not end_date:
messages.error(request, "Please select a month or provide start and end dates.")
return redirect('home')
# Build report data using shared helper
context = _build_report_context(start_date, end_date, project_id, team_id)
context = _build_report_context(
start_date, end_date,
project_ids=project_ids, team_ids=team_ids,
)
# Pass the raw query params so the "Download PDF" button can use them
context['query_string'] = request.GET.urlencode()
# Pass projects and teams so the "New Report" modal's dropdowns can
@ -2355,14 +2397,21 @@ def generate_report_pdf(request):
# Parse dates — same logic as the HTML view
start_date, end_date = _parse_report_dates(request)
project_id = request.GET.get('project') or None
team_id = request.GET.get('team') or None
# Multi-value: ?project=1&project=2 comes in as getlist ['1','2'].
# Cast to ints; drop empties. None if list is empty (= "no filter").
def _ids(name):
return [int(v) for v in request.GET.getlist(name) if v.strip().isdigit()]
project_ids = _ids('project') or None
team_ids = _ids('team') or None
if not start_date or not end_date:
messages.error(request, "Please select a month or provide start and end dates.")
return redirect('home')
context = _build_report_context(start_date, end_date, project_id, team_id)
context = _build_report_context(
start_date, end_date,
project_ids=project_ids, team_ids=team_ids,
)
context['now'] = timezone.now()
pdf = render_to_pdf('core/pdf/report_pdf.html', context)