refactor(report): signed adjustment amounts + filter-attribution caveat
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) <noreply@anthropic.com>
This commit is contained in:
Konrad du Plessis
parent
4186603bcb
commit
8f81e5ab94
@ -704,11 +704,12 @@
|
||||
<h3 class="sub-title">ADJUSTMENTS</h3>
|
||||
{% if adjustment_totals %}
|
||||
<table class="ledger">
|
||||
{# Sign-aware: deductive types render with a leading minus. #}
|
||||
{% for item in adjustment_totals %}
|
||||
<tr>
|
||||
<td class="lbl">{{ item.label }}</td>
|
||||
<td class="rsym">R</td>
|
||||
<td class="rnum">{{ item.total|money }}</td>
|
||||
<td class="rsym">{% if item.is_deductive %}−R{% else %}R{% endif %}</td>
|
||||
<td class="rnum">{{ item.total|money_abs }}</td>
|
||||
</tr>
|
||||
{% endfor %}
|
||||
</table>
|
||||
@ -734,8 +735,8 @@
|
||||
<th>WORKER</th>
|
||||
<th class="r">DAYS</th>
|
||||
<th class="r">TOTAL PAID</th>
|
||||
{% for label in active_adj_labels %}
|
||||
<th class="r">{{ label|upper }}</th>
|
||||
{% for h in active_adj_headers %}
|
||||
<th class="r">{{ h.label|upper }}</th>
|
||||
{% endfor %}
|
||||
</tr>
|
||||
{% for w in worker_breakdown %}
|
||||
@ -743,9 +744,11 @@
|
||||
<td class="name">{{ w.name }}</td>
|
||||
<td class="r">{{ w.days }}</td>
|
||||
<td class="total">R {{ w.total_paid|money }}</td>
|
||||
{# adj_values now contains {'amount', 'is_deductive'} per column.
|
||||
Render "-R …" for deductive types. #}
|
||||
{% for val in w.adj_values %}
|
||||
{% if val %}
|
||||
<td class="r">R {{ val|money }}</td>
|
||||
{% if val.amount %}
|
||||
<td class="r">{% if val.is_deductive %}−R {% else %}R {% endif %}{{ val.amount|money_abs }}</td>
|
||||
{% else %}
|
||||
<td class="dim">—</td>
|
||||
{% endif %}
|
||||
|
||||
@ -316,7 +316,9 @@
|
||||
<div class="row g-3 mb-4">
|
||||
<div class="col-lg-2 col-md-4 col-6">
|
||||
<div class="stat-card stat-card--danger h-100 p-3">
|
||||
<div class="stat-label">Total Paid Out</div>
|
||||
<div class="stat-label">
|
||||
Total Paid Out{% if total_paid_filter_caveat %}<span data-bs-toggle="tooltip" title="Includes the FULL amount of every payment whose work-log touches the selected projects/teams. Payments that span both in-scope and out-of-scope projects are over-counted here. The Labour Cost by Project table below is the right place for strict per-project totals."> <i class="fas fa-asterisk" style="font-size: 0.55rem; color: var(--accent);"></i></span>{% endif %}
|
||||
</div>
|
||||
<div class="stat-value" style="font-size: 1.1rem;">R {{ total_paid_out|money }}</div>
|
||||
</div>
|
||||
</div>
|
||||
@ -388,8 +390,15 @@
|
||||
<table class="table table-sm mb-0">
|
||||
<thead><tr><th>Category</th><th class="text-end">Total</th></tr></thead>
|
||||
<tbody>
|
||||
{# 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 %}
|
||||
<tr><td>{{ item.label }}</td><td class="text-end fw-semibold">R {{ item.total|money }}</td></tr>
|
||||
<tr>
|
||||
<td>{{ item.label }}</td>
|
||||
<td class="text-end fw-semibold {% if item.is_deductive %}text-danger{% endif %}">
|
||||
{% if item.is_deductive %}−{% endif %}R {{ item.total|money_abs }}
|
||||
</td>
|
||||
</tr>
|
||||
{% endfor %}
|
||||
</tbody>
|
||||
</table>
|
||||
@ -478,8 +487,10 @@
|
||||
<th>Worker</th>
|
||||
<th class="text-end">Days</th>
|
||||
<th class="text-end">Total Paid</th>
|
||||
{% for label in active_adj_labels %}
|
||||
<th class="text-end d-none d-md-table-cell" style="font-size: 0.75rem;">{{ label }}</th>
|
||||
{# 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 %}
|
||||
<th class="text-end d-none d-md-table-cell {% if h.is_deductive %}text-danger{% endif %}" style="font-size: 0.75rem;">{{ h.label }}</th>
|
||||
{% endfor %}
|
||||
</tr>
|
||||
</thead>
|
||||
@ -489,8 +500,14 @@
|
||||
<td class="fw-medium">{{ w.name }}</td>
|
||||
<td class="text-end">{{ w.days }}</td>
|
||||
<td class="text-end fw-semibold">R {{ w.total_paid|money }}</td>
|
||||
{# Each val is now {'amount': Decimal, 'is_deductive': bool}
|
||||
— render "-R 500.00" in red for deductive types. #}
|
||||
{% for val in w.adj_values %}
|
||||
<td class="text-end d-none d-md-table-cell" style="font-size: 0.8rem;">{% if val %}R {{ val|money }}{% else %}-{% endif %}</td>
|
||||
<td class="text-end d-none d-md-table-cell {% if val.is_deductive and val.amount %}text-danger{% endif %}" style="font-size: 0.8rem;">
|
||||
{% if val.amount %}
|
||||
{% if val.is_deductive %}−{% endif %}R {{ val.amount|money_abs }}
|
||||
{% else %}-{% endif %}
|
||||
</td>
|
||||
{% endfor %}
|
||||
</tr>
|
||||
{% endfor %}
|
||||
|
||||
@ -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."""
|
||||
|
||||
|
||||
@ -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(
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user