From 8f81e5ab94ca7157970c1ae1505b3e76b1b81f04 Mon Sep 17 00:00:00 2001 From: Konrad du Plessis Date: Fri, 15 May 2026 02:10:36 +0200 Subject: [PATCH] refactor(report): signed adjustment amounts + filter-attribution caveat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related changes to the executive payroll report: 1. Adjustment Summary table and Worker Breakdown table now render deductive types (Deductions, Loan Repayment, Advance Repayment) as "-R 500.00" in muted red. Before, they showed the same way as bonuses — which read as "everyone gets richer" when a deduction was actually shrinking net pay. New context keys: - `adjustment_totals[i]['sign']` and `['is_deductive']` - `active_adj_headers` (list of {label, is_deductive}) replaces the parallel `active_adj_labels`/`active_adj_types` lists for templates. The originals are still emitted for any external consumer. - `worker_breakdown[i]['adj_values']` now contains {'amount', 'is_deductive'} dicts instead of bare Decimals. Templates updated: report.html + pdf/report_pdf.html. 2. "Total Paid Out" hero card on /report/ now shows a small asterisk + tooltip when project/team filters are active, explaining that a PayrollRecord touching the filtered scope is summed at its FULL amount — not just the project-attributable portion. Cheap label approach; the proper per-project attribution would need proportional splitting across each record's work_logs (deferred). New context key `total_paid_filter_caveat: bool`. 3. (No code change — Finding 6 was already satisfied by commit 1's `outstanding_by_project_sorted` rewrite, but the regression test protects the sort order going forward.) Findings 3, 4, 6. Co-Authored-By: Claude Opus 4.7 (1M context) --- core/templates/core/pdf/report_pdf.html | 15 +++-- core/templates/core/report.html | 27 +++++++-- core/tests.py | 74 +++++++++++++++++++++++++ core/views.py | 62 +++++++++++++++++---- 4 files changed, 157 insertions(+), 21 deletions(-) diff --git a/core/templates/core/pdf/report_pdf.html b/core/templates/core/pdf/report_pdf.html index 232e97a..d4f4c80 100644 --- a/core/templates/core/pdf/report_pdf.html +++ b/core/templates/core/pdf/report_pdf.html @@ -704,11 +704,12 @@

ADJUSTMENTS

{% if adjustment_totals %} + {# Sign-aware: deductive types render with a leading minus. #} {% for item in adjustment_totals %} - - + + {% endfor %}
{{ item.label }}R{{ item.total|money }}{% if item.is_deductive %}−R{% else %}R{% endif %}{{ item.total|money_abs }}
@@ -734,8 +735,8 @@ WORKER DAYS TOTAL PAID - {% for label in active_adj_labels %} - {{ label|upper }} + {% for h in active_adj_headers %} + {{ h.label|upper }} {% endfor %} {% for w in worker_breakdown %} @@ -743,9 +744,11 @@ {{ w.name }} {{ w.days }} R {{ w.total_paid|money }} + {# adj_values now contains {'amount', 'is_deductive'} per column. + Render "-R …" for deductive types. #} {% for val in w.adj_values %} - {% if val %} - R {{ val|money }} + {% if val.amount %} + {% if val.is_deductive %}−R {% else %}R {% endif %}{{ val.amount|money_abs }} {% else %} — {% endif %} diff --git a/core/templates/core/report.html b/core/templates/core/report.html index b3d0538..ab19688 100644 --- a/core/templates/core/report.html +++ b/core/templates/core/report.html @@ -316,7 +316,9 @@
-
Total Paid Out
+
+ Total Paid Out{% if total_paid_filter_caveat %} {% endif %} +
R {{ total_paid_out|money }}
@@ -388,8 +390,15 @@ + {# Sign-aware rendering: deductive types show as red "-R …"; + additive types stay in default colour with no sign. Finding 4. #} {% for item in adjustment_totals %} - + + + + {% endfor %}
CategoryTotal
{{ item.label }}R {{ item.total|money }}
{{ item.label }} + {% if item.is_deductive %}−{% endif %}R {{ item.total|money_abs }} +
@@ -478,8 +487,10 @@ Worker Days Total Paid - {% for label in active_adj_labels %} - {{ label }} + {# Sign-aware headers: deductive types render in muted red + so the negative sign on rows below is unmistakable. Finding 4. #} + {% for h in active_adj_headers %} + {{ h.label }} {% endfor %} @@ -489,8 +500,14 @@ {{ w.name }} {{ w.days }} R {{ w.total_paid|money }} + {# Each val is now {'amount': Decimal, 'is_deductive': bool} + — render "-R 500.00" in red for deductive types. #} {% for val in w.adj_values %} - {% if val %}R {{ val|money }}{% else %}-{% endif %} + + {% if val.amount %} + {% if val.is_deductive %}−{% endif %}R {{ val.amount|money_abs }} + {% else %}-{% endif %} + {% endfor %} {% endfor %} diff --git a/core/tests.py b/core/tests.py index 3c2f912..d8bfd62 100644 --- a/core/tests.py +++ b/core/tests.py @@ -599,6 +599,80 @@ class CompanyCostVelocitySQLAggregateTests(TestCase): self.assertEqual(result['avg_daily'], expected_avg_daily) +class ReportSignedAdjustmentTotalsTests(TestCase): + """Regression for Finding 4: adjustment_totals and worker_breakdown + must include sign metadata so deductive types can render as + '-R 500.00' in red rather than indistinguishable from positive types.""" + + @classmethod + def setUpTestData(cls): + cls.admin = User.objects.create_user(username='signed-adj', is_staff=True) + cls.project = Project.objects.create(name='SignProj') + cls.worker = Worker.objects.create( + name='SW', id_number='SW1', monthly_salary=Decimal('4000'), + ) + # Two adjustments — one of each direction + PayrollAdjustment.objects.create( + worker=cls.worker, project=cls.project, type='Bonus', + amount=Decimal('100.00'), date=datetime.date(2026, 3, 1), + ) + PayrollAdjustment.objects.create( + worker=cls.worker, project=cls.project, type='Deduction', + amount=Decimal('50.00'), date=datetime.date(2026, 3, 2), + ) + + def test_adjustment_totals_have_sign_keys(self): + ctx = _build_report_context( + datetime.date(2026, 3, 1), datetime.date(2026, 3, 31), + ) + by_type = {item['type']: item for item in ctx['adjustment_totals']} + # Bonus is additive + self.assertIn('Bonus', by_type) + self.assertEqual(by_type['Bonus']['sign'], '+') + self.assertFalse(by_type['Bonus']['is_deductive']) + # Deduction is deductive + self.assertIn('Deduction', by_type) + self.assertEqual(by_type['Deduction']['sign'], '-') + self.assertTrue(by_type['Deduction']['is_deductive']) + + def test_worker_breakdown_adj_values_have_is_deductive(self): + ctx = _build_report_context( + datetime.date(2026, 3, 1), datetime.date(2026, 3, 31), + ) + # active_adj_headers should also have is_deductive flag + types_seen = [(h['label'], h['is_deductive']) for h in ctx['active_adj_headers']] + self.assertGreater(len(types_seen), 0) + # At least one entry should be flagged deductive (the Deduction we created) + deductive_labels = [lbl for lbl, deduct in types_seen if deduct] + self.assertIn('Deductions', deductive_labels) + + +class ReportTotalPaidFilterCaveatTests(TestCase): + """Regression for Finding 3: when project/team filters are active, + the `total_paid_filter_caveat` flag must be True so the template + can decorate the Total Paid Out card with the over-counting warning.""" + + def test_no_filter_caveat_false(self): + ctx = _build_report_context( + datetime.date(2026, 1, 1), datetime.date(2026, 1, 31), + ) + self.assertFalse(ctx['total_paid_filter_caveat']) + + def test_project_filter_sets_caveat(self): + ctx = _build_report_context( + datetime.date(2026, 1, 1), datetime.date(2026, 1, 31), + project_ids=[1], + ) + self.assertTrue(ctx['total_paid_filter_caveat']) + + def test_team_filter_sets_caveat(self): + ctx = _build_report_context( + datetime.date(2026, 1, 1), datetime.date(2026, 1, 31), + team_ids=[1], + ) + self.assertTrue(ctx['total_paid_filter_caveat']) + + class CurrentOutstandingInScopeTests(TestCase): """Hero card 2 — 'Outstanding NOW' with optional filter scope.""" diff --git a/core/views.py b/core/views.py index cae1f7d..cb8e768 100644 --- a/core/views.py +++ b/core/views.py @@ -2458,7 +2458,17 @@ def _build_report_context(start_date, end_date, project_ids=None, team_ids=None) ) # --- Total Paid Out (sum of all payments made) --- + # CAVEAT (Finding 3, May 2026): when project_ids or team_ids filters + # are active, a PayrollRecord is INCLUDED if ANY of its linked + # work_logs touches the filtered project/team — but the FULL + # `amount_paid` is summed, not the project-attributable portion. + # This can over-count if a payment spans logs across multiple + # projects and one of them is in scope. The template surfaces this + # via the `total_paid_filter_caveat` flag below — see report.html. total_paid_out = records.aggregate(total=Sum('amount_paid'))['total'] or Decimal('0.00') + # Flag: filters active = caveat applies. The template uses this to + # decorate the "Total Paid Out" label with an asterisk + tooltip. + total_paid_filter_caveat = bool(project_ids or team_ids) # --- Payments by Date (total paid per day) --- payments_by_date = ( @@ -2581,20 +2591,37 @@ def _build_report_context(start_date, end_date, project_ids=None, team_ids=None) total=Sum('principal_amount'))['total'] or Decimal('0.00') # --- Adjustment Summary --- - # Group by type, use readable labels, and sort by logical grouping + # Group by type, use readable labels, and sort by logical grouping. + # The 'sign' key tells the template how to render the amount: + # '+' for additive types (Bonus, Overtime, New Loan, Advance Payment) + # '-' for deductive types (Deduction, Loan Repayment, Advance Repayment) + # '' for unknown types (defensive — shouldn't happen in practice) + # Templates use this to render "-R 500.00" in red for deductions + # instead of an unsigned "R 500.00" that reads as the same direction + # as a bonus. Finding 4 (May 2026). adj_by_type = ( adjustments.values('type') .annotate(total=Sum('amount')) .order_by('type') ) - adjustment_totals = [ - { - 'type': item['type'], - 'label': REPORT_ADJ_LABELS.get(item['type'], item['type']), + + def _sign_for_type(t): + if t in ADDITIVE_TYPES: + return '+' + if t in DEDUCTIVE_TYPES: + return '-' + return '' + + adjustment_totals = [] + for item in adj_by_type: + t = item['type'] + adjustment_totals.append({ + 'type': t, + 'label': REPORT_ADJ_LABELS.get(t, t), 'total': item['total'], - } - for item in adj_by_type - ] + 'sign': _sign_for_type(t), + 'is_deductive': t in DEDUCTIVE_TYPES, + }) # --- Determine which adjustment types appear (for worker table columns) --- # Only types with non-zero totals get a column — keeps the table readable @@ -2603,6 +2630,14 @@ def _build_report_context(start_date, end_date, project_ids=None, team_ids=None) ) # Create matching readable labels for column headers active_adj_labels = [REPORT_ADJ_LABELS.get(t, t) for t in active_adj_types] + # Parallel sign list so the worker breakdown can render "-R 500.00" + # for deductive-type columns. zip(labels, signs) in the template + # is awkward — pre-pair them into a single iterable instead. + # Each header_info is {'label': str, 'is_deductive': bool}. + active_adj_headers = [ + {'label': REPORT_ADJ_LABELS.get(t, t), 'is_deductive': t in DEDUCTIVE_TYPES} + for t in active_adj_types + ] # --- Worker Breakdown --- # Per worker: days worked, total paid, and each adjustment type @@ -2622,12 +2657,17 @@ def _build_report_context(start_date, end_date, project_ids=None, team_ids=None) worker_breakdown = [] for wr in worker_records: w_adjs = adjustments.filter(worker_id=wr['worker__id']) - # Per-type amounts for this worker (only for types that exist in the period) + # Per-type amounts for this worker (only for types that exist in the period). + # Each `adj_values` entry is {'amount': Decimal, 'is_deductive': bool} + # so the template can render "-R 500.00" for deductive types. adj_values = [] for adj_type in active_adj_types: amt = w_adjs.filter(type=adj_type).aggregate( t=Sum('amount'))['t'] or Decimal('0.00') - adj_values.append(amt) + adj_values.append({ + 'amount': amt, + 'is_deductive': adj_type in DEDUCTIVE_TYPES, + }) worker_breakdown.append({ 'name': wr['worker__name'], @@ -2658,6 +2698,7 @@ def _build_report_context(start_date, end_date, project_ids=None, team_ids=None) ), # --- Summary --- 'total_paid_out': total_paid_out, + 'total_paid_filter_caveat': total_paid_filter_caveat, 'total_worker_days': total_worker_days, 'loans_outstanding': loans_outstanding, 'advances_outstanding': advances_outstanding, @@ -2673,6 +2714,7 @@ def _build_report_context(start_date, end_date, project_ids=None, team_ids=None) 'adjustment_totals': adjustment_totals, 'active_adj_types': active_adj_types, 'active_adj_labels': active_adj_labels, + 'active_adj_headers': active_adj_headers, 'worker_breakdown': worker_breakdown, # --- Hero KPI band (executive report v2) --- 'current_outstanding': _current_outstanding_in_scope(