Ver 30.04 Fix reports and add Supervisor

core/forms.py

@@ -372,23 +372,18 @@ WorkerWarningFormSet = inlineformset_factory(
 def _supervisor_user_queryset():
     """Users eligible to supervise a team or project.
 
-    Returns ALL active users. Any active user can be picked as a supervisor
-    — the picker doesn't need to pre-filter by group or staff flags because
-    the app's `is_supervisor(user)` helper (in views.py) already grants
-    supervisor permissions to anyone assigned to a team/project FK/M2M,
-    regardless of their group membership.
-
-    Previously this filter required `is_staff`/`is_superuser` OR membership
-    in a "Work Logger" group, which was strictly more restrictive than the
-    permission model and hid field supervisors from the picker. Fix
-    (2026-04-23): trust the admin making the assignment; any active user
-    can be chosen, and the act of assignment is what confers supervisor
-    powers downstream.
-
-    Inactive users (`is_active=False`) are still excluded — deactivated
-    accounts should never appear in dropdowns.
+    Matches the app's role model (see `is_supervisor` in views.py):
+    anyone who is a Django admin (is_staff/is_superuser) OR is a member
+    of the "Work Logger" group. Active accounts only — no deactivated
+    users in the picker.
     """
-    return User.objects.filter(is_active=True).order_by('username')
+    from django.db.models import Q
+    return (
+        User.objects.filter(is_active=True)
+        .filter(Q(is_staff=True) | Q(is_superuser=True) | Q(groups__name='Work Logger'))
+        .distinct()
+        .order_by('username')
+    )
 
 
 class TeamForm(forms.ModelForm):
@@ -413,11 +408,12 @@ class TeamForm(forms.ModelForm):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        # Supervisor dropdown — show any active user. The app's is_supervisor
-        # helper (views.py) grants supervisor powers to whoever is assigned
-        # here, regardless of group or staff flags, so the picker doesn't
-        # need to pre-filter by role. Only is_active=True users appear, so
-        # deactivated accounts are hidden from the dropdown.
+        # Supervisor dropdown — show anyone who is either admin (is_staff/
+        # is_superuser) OR a member of the "Work Logger" group. This matches
+        # the app's role model: team supervisors are typically Work Loggers,
+        # not admins, so filtering by is_staff alone hides the people who
+        # actually supervise teams day-to-day. We use `active=True` to drop
+        # deactivated accounts from the picker.
         self.fields['supervisor'].queryset = _supervisor_user_queryset()
         self.fields['supervisor'].required = False
         # Include inactive workers too — matches admin parity. The template

core/tests.py

@@ -10,7 +10,7 @@ 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
+from core.views import _build_work_log_payroll_context
 
 
 class WorkLogPayrollContextTests(TestCase):
@@ -312,167 +312,3 @@ class WorkLogPayrollDetailTests(TestCase):
         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())

core/views.py

@@ -1878,30 +1878,11 @@ def _build_report_context(start_date, end_date, project_id=None, team_id=None):
     date_filter = Q(date__gte=start_date, date__lte=end_date)
 
     # --- PayrollRecords in range ---
-    #
-    # IMPORTANT — avoid M2M double-JOIN inflation:
-    # Chaining `.filter(work_logs__project_id=X).distinct().filter(work_logs__team_id=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
-    # N × M (where N and M are the counts of matching logs per record).
-    # `.aggregate(Sum())` is safe because Django wraps it in a subquery when
-    # distinct() is in play, but `.values().annotate(Sum())` isn't — so we
-    # 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:
-        records = records.filter(
-            id__in=PayrollRecord.objects.filter(
-                work_logs__project_id=project_id
-            ).values('id')
-        )
+        records = records.filter(work_logs__project_id=project_id).distinct()
     if team_id:
-        records = records.filter(
-            id__in=PayrollRecord.objects.filter(
-                work_logs__team_id=team_id
-            ).values('id')
-        )
+        records = records.filter(work_logs__team_id=team_id).distinct()
 
     # --- Total Paid Out (sum of all payments made) ---
     total_paid_out = records.aggregate(total=Sum('amount_paid'))['total'] or Decimal('0.00')
@@ -1914,16 +1895,11 @@ 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
-    # 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:
-        adjustments = adjustments.filter(
-            worker__in=Worker.objects.filter(teams__id=team_id).values('id')
-        )
+        adjustments = adjustments.filter(worker__teams__id=team_id).distinct()
 
     # --- Work Logs in range (for calculating actual labour cost) ---
     work_logs_qs = WorkLog.objects.filter(date__gte=start_date, date__lte=end_date)