38686-vm/core/tests.py
Konrad du Plessis 0ceceebba4 Fix: supervisor picker hid regular active users (only admins showed)
Reported: when creating a new team or project from the friendly UI
(/teams/new/ or /projects/new/), the Supervisor dropdown only lists
is_staff / is_superuser accounts. Users who should be eligible to
supervise (e.g. eendman, supervisor_smoke) are invisible in the
picker even though they are active.

Root cause:
core.forms._supervisor_user_queryset filtered to
  is_active=True AND (is_staff OR is_superuser OR groups__name='Work Logger')
That was strictly more restrictive than the app's own permission
helper is_supervisor(user) in views.py, which grants supervisor
powers to ANYONE assigned to a team/project (via the team.supervisor
FK or project.supervisors M2M), regardless of group membership.
On Konrad's dev DB that excluded 2 of 6 active users from the picker
(one in a custom group, one in no group) even though both were valid
supervisor candidates by the permission model.

Fix:
Queryset now returns every active user. The act of assigning a user
to a team/project is what confers supervisor-ness downstream, so
the picker no longer needs a pre-registered allow-list. Inactive
users (is_active=False) remain excluded — the one hard guardrail.

Docstring rewritten to explain the new behavior and why. Stale comment
in TeamForm.__init__ updated to match (the old comment still described
the pre-fix Work-Logger-group requirement).

Tests: 4 new regression tests in SupervisorPickerQuerysetTests:
  - regular active user is selectable (the core bug)
  - user in an unrelated group is selectable
  - inactive user is still excluded (guardrail)
  - admin is still selectable (no regression for prior use case)
All 28 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 19:52:29 +02:00

479 lines
22 KiB
Python
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# === TESTS FOR WORK LOG PAYROLL CROSS-LINK ===
# Covers the _build_work_log_payroll_context helper — the core logic that
# determines, for each worker on a log, whether they were paid for it.
import datetime
from decimal import Decimal
from django.contrib.auth.models import User
from django.test import TestCase
from django.urls import reverse
from core.models import Project, Team, Worker, WorkLog, PayrollRecord, PayrollAdjustment
from core.views import _build_work_log_payroll_context, _build_report_context
class WorkLogPayrollContextTests(TestCase):
"""Tests for the helper that builds the payroll-status view of a work log."""
def setUp(self):
# Minimal scenario: 1 admin, 1 project, 1 team, 3 workers, 1 log.
# Worker A has been paid for the log; Worker B is priced-not-paid;
# Worker C is unpaid.
self.admin = User.objects.create_user(username='admin', is_staff=True)
self.project = Project.objects.create(name='Test Project')
self.team = Team.objects.create(name='Team X', supervisor=self.admin)
self.worker_a = Worker.objects.create(name='Alice', id_number='A1', monthly_salary=Decimal('4000'))
self.worker_b = Worker.objects.create(name='Bob', id_number='B1', monthly_salary=Decimal('4000'))
self.worker_c = Worker.objects.create(name='Carol', id_number='C1', monthly_salary=Decimal('4000'))
self.log = WorkLog.objects.create(
date=datetime.date(2026, 4, 10),
project=self.project,
team=self.team,
supervisor=self.admin,
)
self.log.workers.add(self.worker_a, self.worker_b, self.worker_c)
# Worker A has a PayrollRecord linking them and this log — "Paid".
self.record_a = PayrollRecord.objects.create(
worker=self.worker_a,
amount_paid=Decimal('200.00'),
date=datetime.date(2026, 4, 15),
)
self.record_a.work_logs.add(self.log)
# Worker B appears in priced_workers but has no PayrollRecord — "Priced, not paid".
self.log.priced_workers.add(self.worker_b)
# Worker C has neither — "Unpaid".
def test_returns_log_and_worker_rows(self):
ctx = _build_work_log_payroll_context(self.log)
self.assertEqual(ctx['log'], self.log)
self.assertEqual(len(ctx['worker_rows']), 3)
def test_paid_worker_has_payslip_link(self):
ctx = _build_work_log_payroll_context(self.log)
row = next(r for r in ctx['worker_rows'] if r['worker'].id == self.worker_a.id)
self.assertEqual(row['status'], 'Paid')
self.assertEqual(row['payroll_record'], self.record_a)
self.assertGreater(row['earned'], 0)
def test_priced_but_unpaid_worker(self):
ctx = _build_work_log_payroll_context(self.log)
row = next(r for r in ctx['worker_rows'] if r['worker'].id == self.worker_b.id)
self.assertEqual(row['status'], 'Priced, not paid')
self.assertIsNone(row['payroll_record'])
def test_totally_unpaid_worker(self):
ctx = _build_work_log_payroll_context(self.log)
row = next(r for r in ctx['worker_rows'] if r['worker'].id == self.worker_c.id)
self.assertEqual(row['status'], 'Unpaid')
self.assertIsNone(row['payroll_record'])
def test_totals(self):
ctx = _build_work_log_payroll_context(self.log)
# Paid = Alice's daily_rate (one record exists for this log+worker).
self.assertEqual(ctx['total_paid'], self.worker_a.daily_rate)
# Outstanding = Bob + Carol each at their daily_rate.
expected = self.worker_b.daily_rate + self.worker_c.daily_rate
self.assertEqual(ctx['total_outstanding'], expected)
def test_adjustments_linked_to_log(self):
adj = PayrollAdjustment.objects.create(
worker=self.worker_a,
project=self.project,
type='Overtime',
amount=Decimal('50.00'),
date=datetime.date(2026, 4, 10),
description='Extra hour',
work_log=self.log,
)
ctx = _build_work_log_payroll_context(self.log)
self.assertIn(adj, ctx['adjustments'])
def test_pay_period_absent_if_no_schedule(self):
ctx = _build_work_log_payroll_context(self.log)
self.assertEqual(ctx['pay_period'], (None, None))
def test_pay_period_present_when_schedule_configured(self):
self.team.pay_frequency = 'weekly'
self.team.pay_start_date = datetime.date(2026, 1, 5) # A Monday
self.team.save()
ctx = _build_work_log_payroll_context(self.log)
start, end = ctx['pay_period']
self.assertIsNotNone(start)
self.assertIsNotNone(end)
self.assertLessEqual(start, 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)
# === TESTS FOR THE WORK LOG PAYROLL AJAX ENDPOINT ===
# These cover the JSON endpoint that the Task-5 modal will consume.
# The endpoint must: return JSON to admins, forbid supervisors/anons,
# and 404 on unknown logs.
class WorkLogPayrollAjaxTests(TestCase):
"""Tests for the JSON AJAX endpoint that powers the modal."""
def setUp(self):
# One admin, one non-admin supervisor, and a simple log with one worker.
self.admin = User.objects.create_user(
username='admin', password='pass', is_staff=True
)
self.supervisor = User.objects.create_user(
username='sup', password='pass', is_staff=False
)
project = Project.objects.create(name='P')
team = Team.objects.create(name='T', supervisor=self.admin)
worker = Worker.objects.create(name='W', id_number='1', monthly_salary=Decimal('4000'))
self.log = WorkLog.objects.create(
date=datetime.date(2026, 4, 10),
project=project, team=team, supervisor=self.admin,
)
self.log.workers.add(worker)
def test_admin_sees_200_json(self):
# Admin hits the endpoint and gets a well-formed JSON body.
self.client.login(username='admin', password='pass')
url = reverse('work_log_payroll_ajax', args=[self.log.id])
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
data = resp.json()
self.assertEqual(data['log_id'], self.log.id)
self.assertEqual(len(data['worker_rows']), 1)
self.assertEqual(data['worker_rows'][0]['status'], 'Unpaid')
def test_supervisor_forbidden(self):
# A non-admin user (even if authenticated) gets 403 JSON.
self.client.login(username='sup', password='pass')
url = reverse('work_log_payroll_ajax', args=[self.log.id])
resp = self.client.get(url)
self.assertEqual(resp.status_code, 403)
def test_anonymous_redirected_to_login(self):
# @login_required intercepts before our view ever runs — 302 to /accounts/login/.
url = reverse('work_log_payroll_ajax', args=[self.log.id])
resp = self.client.get(url)
self.assertEqual(resp.status_code, 302)
def test_missing_log_is_404(self):
# get_object_or_404 returns a 404 if the log_id doesn't exist.
self.client.login(username='admin', password='pass')
resp = self.client.get('/history/99999/payroll/ajax/')
self.assertEqual(resp.status_code, 404)
def test_full_payload_with_paid_worker_null_team_and_ot(self):
"""One scenario exercising: paid worker, null team, OT flag, full URL."""
# Create a paid worker on a separate log with no team (null team edge case)
# and with overtime hours but no priced_workers (unpriced OT flag).
project = Project.objects.create(name='P-full')
paid_worker = Worker.objects.create(
name='Paula', id_number='P1', monthly_salary=Decimal('4000')
)
log = WorkLog.objects.create(
date=datetime.date(2026, 4, 11),
project=project,
team=None, # null team
supervisor=self.admin,
overtime_amount=Decimal('1.50'), # > 0 and no priced_workers -> flag fires
)
log.workers.add(paid_worker)
# Link a PayrollRecord so Paula shows as Paid
record = PayrollRecord.objects.create(
worker=paid_worker,
amount_paid=Decimal('250.00'),
date=datetime.date(2026, 4, 16),
)
record.work_logs.add(log)
# Link an adjustment to the same log
adj = PayrollAdjustment.objects.create(
worker=paid_worker,
project=project,
type='Overtime',
amount=Decimal('75.00'),
date=datetime.date(2026, 4, 11),
description='OT pricing',
work_log=log,
payroll_record=record,
)
self.client.login(username='admin', password='pass')
resp = self.client.get(reverse('work_log_payroll_ajax', args=[log.id]))
self.assertEqual(resp.status_code, 200)
data = resp.json()
# Null team branch
self.assertIsNone(data['team'])
# Project is still present
self.assertEqual(data['project']['name'], 'P-full')
# Paid worker serialization shape
paula_row = next(r for r in data['worker_rows'] if r['worker_name'] == 'Paula')
self.assertEqual(paula_row['status'], 'Paid')
self.assertEqual(paula_row['payroll_record_id'], record.pk)
self.assertEqual(paula_row['paid_date'], '2026-04-16')
# OT flag fires (overtime > 0, no priced_workers)
self.assertTrue(data['overtime_needs_pricing'])
# full_page_url is the reverse of work_log_payroll_detail
self.assertEqual(data['full_page_url'], f'/history/{log.id}/')
# Adjustment serialized with payslip link
self.assertEqual(len(data['adjustments']), 1)
self.assertEqual(data['adjustments'][0]['type'], 'Overtime')
self.assertEqual(data['adjustments'][0]['payroll_record_id'], record.pk)
# === TESTS FOR THE WORK LOG PAYROLL FULL-PAGE VIEW ===
# These cover the HTML page at /history/<id>/ that shares the same context
# builder as the AJAX endpoint. Admin sees a 200 HTML page; supervisor 403.
class WorkLogPayrollDetailTests(TestCase):
"""Tests for the full-page /history/<id>/ view."""
def setUp(self):
self.admin = User.objects.create_user(
username='admin', password='pass', is_staff=True
)
self.supervisor = User.objects.create_user(
username='sup', password='pass', is_staff=False
)
project = Project.objects.create(name='P2')
team = Team.objects.create(name='T2', supervisor=self.admin)
worker = Worker.objects.create(name='Wanda', id_number='X', monthly_salary=Decimal('4000'))
self.log = WorkLog.objects.create(
date=datetime.date(2026, 4, 10),
project=project, team=team, supervisor=self.admin,
)
self.log.workers.add(worker)
def test_admin_gets_full_page(self):
self.client.login(username='admin', password='pass')
url = reverse('work_log_payroll_detail', args=[self.log.id])
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
self.assertContains(resp, 'FoxFitt')
self.assertContains(resp, 'History')
self.assertContains(resp, 'Wanda')
def test_supervisor_forbidden(self):
self.client.login(username='sup', password='pass')
url = reverse('work_log_payroll_detail', args=[self.log.id])
resp = self.client.get(url)
self.assertEqual(resp.status_code, 403)
# =============================================================================
# === TESTS FOR PAYROLL REPORT FILTER INFLATION ===
# Regression tests for the M2M double-JOIN bug in _build_report_context.
#
# THE BUG (fixed 2026-04-23):
# Filtering a report by project AND team via chained `.filter(work_logs__field=X)`
# calls produced TWO separate JOIN aliases on the core_payrollrecord_work_logs
# M2M table. Any downstream `.values().annotate(Sum(...))` then aggregated
# across the cartesian product of matching rows, inflating every per-worker
# total_paid and every payments_by_date amount by N² (where N = number of
# matching work logs per record). Total_paid_out itself was correct because
# `.aggregate(Sum(...))` wraps distinct() in a subquery, but the per-row
# values/annotate pattern doesn't get that help.
#
# These tests lock down the fix: with filters applied, the worker-level and
# date-level totals must equal the real payment amount, not a multiplied one.
# =============================================================================
class ReportContextFilterInflationTests(TestCase):
"""Report aggregates must not inflate when project + team filters combine."""
def setUp(self):
self.admin = User.objects.create_user(username='admin-r', is_staff=True)
self.project = Project.objects.create(name='Solar Farm Gamma')
self.team = Team.objects.create(name='Team Gamma', supervisor=self.admin)
self.worker = Worker.objects.create(
name='Test Worker', id_number='TW1', monthly_salary=Decimal('4000')
)
# Worker must be in the team's M2M for the adjustment-summary test
# to find them via worker__teams__id=team_id. In real data this is
# the standard setup — workers belong to a team.
self.team.workers.add(self.worker)
# Three work logs in the range, all in the same project + team.
# This is the minimum setup that reproduces the N² inflation: with
# one payroll record linked to 3 logs, the double-JOIN produces 9 rows.
self.logs = []
for day in (5, 10, 15):
log = WorkLog.objects.create(
date=datetime.date(2026, 3, day),
project=self.project,
team=self.team,
supervisor=self.admin,
)
log.workers.add(self.worker)
self.logs.append(log)
# One payment covering all 3 logs.
self.record = PayrollRecord.objects.create(
worker=self.worker,
amount_paid=Decimal('600.00'),
date=datetime.date(2026, 3, 20),
)
self.record.work_logs.add(*self.logs)
def _ctx(self, project_id=None, team_id=None):
return _build_report_context(
datetime.date(2026, 3, 1),
datetime.date(2026, 3, 31),
project_id=project_id,
team_id=team_id,
)
def test_worker_breakdown_not_inflated_with_project_filter_only(self):
ctx = self._ctx(project_id=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)
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)
payments = list(ctx['payments_by_date'])
self.assertEqual(len(payments), 1)
self.assertEqual(payments[0]['total'], Decimal('600.00'))
def test_total_paid_out_stays_correct_with_both_filters(self):
"""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)
self.assertEqual(ctx['total_paid_out'], Decimal('600.00'))
def test_adjustment_summary_not_inflated_with_team_filter(self):
"""Adjustments filtered by team go through worker__teams (M2M) — same
bug class. values().annotate(Sum()) would inflate if the worker is in
multiple teams or if the JOIN is chained with other M2M filters."""
PayrollAdjustment.objects.create(
worker=self.worker,
project=self.project,
type='Bonus',
amount=Decimal('100.00'),
date=datetime.date(2026, 3, 10),
description='Test bonus',
)
ctx = self._ctx(project_id=self.project.id, team_id=self.team.id)
totals = {item['type']: item['total'] for item in ctx['adjustment_totals']}
self.assertEqual(totals.get('Bonus'), Decimal('100.00'))
# =============================================================================
# === TESTS FOR SUPERVISOR PICKER QUERYSET ===
# Regression tests for the TeamForm/ProjectForm supervisor dropdown.
#
# THE BUG (fixed 2026-04-23):
# The picker required either admin flags (is_staff/is_superuser) or "Work Logger"
# group membership. Any active user NOT pre-enrolled in that specific group
# was invisible, even though the app's downstream `is_supervisor()` helper
# grants supervisor powers to anyone assigned to a team/project FK/M2M —
# regardless of group. The picker was strictly more restrictive than the
# permission model, hiding field supervisors from the person trying to assign
# them. Fix: allow any active user; let the act of assignment confer
# supervisor-ness (matching how `is_supervisor` works).
# =============================================================================
class SupervisorPickerQuerysetTests(TestCase):
"""Supervisor picker must surface all active users, not just admins."""
def test_regular_active_user_is_selectable(self):
"""A plain active user (no is_staff, no is_superuser, no Work Logger
group) must appear. Pre-fix: excluded by the Q filter."""
from core.forms import _supervisor_user_queryset
regular = User.objects.create_user(
username='field_supervisor', password='pass',
is_staff=False, is_superuser=False,
)
self.assertIn(regular, _supervisor_user_queryset())
def test_user_in_unrelated_group_is_still_selectable(self):
"""A user in some random group (not 'Work Logger') is still an active
user and must show up. Pre-fix: excluded because the Q filter specifically
checked groups__name='Work Logger'."""
from django.contrib.auth.models import Group
from core.forms import _supervisor_user_queryset
user = User.objects.create_user(username='misc_group_user', password='pass')
group, _ = Group.objects.get_or_create(name='Some Other Group')
user.groups.add(group)
self.assertIn(user, _supervisor_user_queryset())
def test_inactive_user_still_excluded(self):
"""Inactive users must NEVER appear in the picker — even if they used
to be admins or Work Loggers. Active-only is the one hard guardrail."""
from core.forms import _supervisor_user_queryset
inactive = User.objects.create_user(
username='former_employee', password='pass',
is_active=False, is_staff=True,
)
self.assertNotIn(inactive, _supervisor_user_queryset())
def test_admin_still_selectable(self):
"""Defense-in-depth: admins continue to appear (no regression for the
existing use case)."""
from core.forms import _supervisor_user_queryset
admin = User.objects.create_user(
username='an_admin', password='pass', is_staff=True
)
self.assertIn(admin, _supervisor_user_queryset())