diff --git a/core/forms.py b/core/forms.py index f7492e1..82e100e 100644 --- a/core/forms.py +++ b/core/forms.py @@ -610,12 +610,9 @@ class SiteReportForm(forms.ModelForm): # ==================================================================== # === ABSENCE FORMS ================================================== # ==================================================================== -# Three forms mirror the SiteReport / WorkerWarning patterns: +# Two forms mirror the SiteReport / WorkerWarning patterns: # - AbsenceLogForm: standalone /absences/log/ with date-range support, # team filter, worker checkbox list, conflict detection. -# - AbsenceQuickForm: minimal form for the "Mark Absent" modal on -# /attendance/log/ — worker + date come from URL/POST, form only -# asks for reason / paid / notes. # - AbsenceEditForm: edit one existing absence; can correct # worker/date as well as the other fields. # @@ -780,53 +777,46 @@ class AbsenceLogForm(forms.ModelForm): Conflicts are warnings, NOT errors — a worker might genuinely have partial-day work + partial-day absence (e.g. sick leave that started mid-shift). The view shows these on a confirm screen so the admin - can review before proceeding.""" + can review before proceeding. + + PERF: single query for all candidate WorkLogs, then Python-side + filter by the actual (worker_id, date) pair set. Previously fired + one SELECT per (worker, date) pair (N+1 — 25 queries on a typical + 5-worker × 5-day submission). Now: 2 queries total (WorkLog + its + workers prefetch) regardless of pair count. + """ + pairs = self.expanded_pairs() + if not pairs: + return [] + # Build sets used as the outer filter (broad SQL match) AND the + # post-filter pair check (narrow Python match). The outer filter + # may match WorkLogs that include OTHER workers on those dates, + # so we still verify each (worker_id, date) against pair_set. + workers = {w for w, _ in pairs} + dates = {d for _, d in pairs} + pair_set = {(w.id, d) for w, d in pairs} + + wls = ( + WorkLog.objects + .filter(date__in=dates, workers__in=workers) + .select_related('project') + .prefetch_related('workers') + .distinct() + ) rows = [] - for worker, d in self.expanded_pairs(): - for wl in WorkLog.objects.filter(date=d, workers=worker).select_related('project'): - rows.append({ - 'worker_id': worker.id, - 'worker_name': worker.name, - 'date': d, - 'work_log_id': wl.id, - 'project_name': wl.project.name if wl.project else '—', - }) + for wl in wls: + for w in wl.workers.all(): + if (w.id, wl.date) in pair_set: + rows.append({ + 'worker_id': w.id, + 'worker_name': w.name, + 'date': wl.date, + 'work_log_id': wl.id, + 'project_name': wl.project.name if wl.project else '—', + }) return rows -class AbsenceQuickForm(forms.ModelForm): - """Minimal form for the ✗ Mark Absent modal on /attendance/log/. - - Worker and date come from URL/POST context (the row the admin clicked - on), so this form only asks for the three fields still missing: - reason / is_paid / notes.""" - - class Meta: - model = Absence - # `project` is optional — the modal may be opened from a worker row - # that already has a current project context (e.g. quick-mark from - # /attendance/log/), in which case the view can pre-fill it. - fields = ['project', 'reason', 'is_paid', 'notes'] - widgets = { - 'project': forms.Select(attrs={'class': 'form-select'}), - 'reason': forms.Select(attrs={'class': 'form-select'}), - 'is_paid': forms.CheckboxInput(attrs={'class': 'form-check-input'}), - 'notes': forms.Textarea(attrs={'rows': 2, 'class': 'form-control'}), - } - - def __init__(self, *args, user=None, **kwargs): - super().__init__(*args, **kwargs) - self.fields['project'].required = False - # Supervisor scope: project dropdown only shows their assigned projects. - # Admin / staff sees every active project. - if user is not None and not (user.is_staff or user.is_superuser): - self.fields['project'].queryset = Project.objects.filter( - active=True, supervisors=user, - ) - else: - self.fields['project'].queryset = Project.objects.filter(active=True) - - class AbsenceEditForm(forms.ModelForm): """Edit one existing Absence. Lets admin correct worker/date as well as the other fields (in case the absence was logged against the wrong diff --git a/core/templates/core/site_report_detail.html b/core/templates/core/site_report_detail.html index 023d75f..53ec3b7 100644 --- a/core/templates/core/site_report_detail.html +++ b/core/templates/core/site_report_detail.html @@ -116,7 +116,7 @@ might want to scan multiple at once. {% for c in checks_display %}
  • {% if c.value %} - + {% else %} {% endif %} diff --git a/core/tests.py b/core/tests.py index 184f706..5ac8e9e 100644 --- a/core/tests.py +++ b/core/tests.py @@ -1900,9 +1900,8 @@ class AbsenceUserQuerysetTests(TestCase): # ============================================================================= # === ABSENCE FORM TESTS (Task 3) === -# Tests for the three form classes added in core/forms.py: +# Tests for the form classes added in core/forms.py: # - AbsenceLogForm: standalone /absences/log/ with date-range + multi-worker -# - AbsenceQuickForm: minimal modal form on /attendance/log/ # - AbsenceEditForm: edit one existing absence # ============================================================================= diff --git a/core/views.py b/core/views.py index b8010f3..12b4cdd 100644 --- a/core/views.py +++ b/core/views.py @@ -495,6 +495,35 @@ def index(request): return render(request, 'core/index.html', context) +# === TEAM → ACTIVE WORKERS MAP HELPER === +# Used by both attendance_log() and absence_log() to feed the team-filter +# JavaScript ({team_id: [active_worker_id, ...]}) — picking a team in the +# dropdown auto-checks (or filters to) that team's active workers. +# +# PERF: Prefetch with `to_attr=` is critical. The naive version did +# `prefetch_related('workers')` then `.filter(active=True)` inside the +# loop — but `.filter()` on a prefetched M2M bypasses the prefetch cache +# and re-queries the DB, giving an N+1 (one SELECT per team). The +# `Prefetch(..., queryset=Worker.objects.filter(active=True), +# to_attr='active_workers_cached')` pattern moves the active-filter into +# the prefetch query itself — one SELECT total for all teams' workers. +def _build_team_workers_map(user): + """Return {team_id: [active_worker_id, ...]} for the team-filter JS. + + Single query via Prefetch(to_attr=…). Admins see every active team; + supervisors see only the teams they themselves supervise.""" + teams_qs = Team.objects.filter(active=True).prefetch_related( + Prefetch( + 'workers', + queryset=Worker.objects.filter(active=True), + to_attr='active_workers_cached', + ) + ) + if not is_admin(user): + teams_qs = teams_qs.filter(supervisor=user) + return {t.id: [w.id for w in t.active_workers_cached] for t in teams_qs} + + # === ATTENDANCE LOGGING === # This is where supervisors log which workers showed up to work each day. # Supports logging a single day or a date range (e.g. a whole week). @@ -693,17 +722,9 @@ def attendance_log(request): worker_rates[str(w.id)] = str(w.daily_rate) # Build team→workers mapping so the JS can auto-check workers when a - # team is selected from the dropdown. Key = team ID, Value = list of worker IDs. - team_workers_map = {} - teams_qs = Team.objects.filter(active=True).prefetch_related('workers') - if not is_admin(user): - # Supervisors only see their own teams - teams_qs = teams_qs.filter(supervisor=user) - for team in teams_qs: - active_worker_ids = list( - team.workers.filter(active=True).values_list('id', flat=True) - ) - team_workers_map[team.id] = active_worker_ids + # team is selected from the dropdown. Key = team ID, Value = list of + # active worker IDs. Single query via the shared helper. + team_workers_map = _build_team_workers_map(user) return render(request, 'core/attendance_log.html', { 'form': form, @@ -5418,18 +5439,10 @@ def absence_log(request): # === TEAM → WORKERS MAP for the in-page team filter (Fix A1, May 2026) === # Mirrors the pattern in attendance_log(): build a dict of team_id → - # [worker_ids] and pass it as JSON so the template's JS can hide + # [active_worker_ids] and pass it as JSON so the template's JS can hide # worker rows whose worker_id is not in the selected team's list. - # Supervisors only see their own teams; admins see every active team. - team_workers_map = {} - teams_qs = Team.objects.filter(active=True).prefetch_related('workers') - if not is_admin(request.user): - teams_qs = teams_qs.filter(supervisor=request.user) - for team in teams_qs: - active_worker_ids = list( - team.workers.filter(active=True).values_list('id', flat=True) - ) - team_workers_map[team.id] = active_worker_ids + # Single query via the shared `_build_team_workers_map` helper. + team_workers_map = _build_team_workers_map(request.user) return render(request, 'core/absences/log.html', { 'form': form, @@ -5580,6 +5593,57 @@ def _create_absences_atomic(pairs, reason, is_paid, notes, user, worklog_removal _sync_absence_payroll_adjustment(a) +def _apply_absence_filters(qs, request): + """Apply the standard URL-param filters to an Absence queryset. + + URL params: worker, team, project, reason, date_from, date_to, paid. + Each filter is best-effort — bad input (non-numeric IDs, malformed + date strings, unknown reason keys) is silently ignored rather than + 500-ing. + + Used by both absence_list and absence_export_csv so filter parity is + structurally enforced — adding a new param here updates both views + in one place.""" + worker_id = request.GET.get('worker') + team_id = request.GET.get('team') + project_id = request.GET.get('project') + reasons = request.GET.getlist('reason') + date_from = request.GET.get('date_from') + date_to = request.GET.get('date_to') + paid = request.GET.get('paid') + + if worker_id and worker_id.isdigit(): + qs = qs.filter(worker_id=worker_id) + if team_id and team_id.isdigit(): + qs = qs.filter(worker__teams__id=team_id).distinct() + if project_id and project_id.isdigit(): + # Direct FK filter (since 0015_absence_project) — matches + # absences explicitly linked to this project. + qs = qs.filter(project_id=project_id) + # Whitelist reason keys against REASON_CHOICES so an attacker can't + # sneak an arbitrary string into the SQL filter. + valid_reason_keys = dict(Absence.REASON_CHOICES) + reasons = [r for r in reasons if r in valid_reason_keys] + if reasons: + qs = qs.filter(reason__in=reasons) + # parse_date() returns None for malformed input — filter is skipped. + # Without this guard, Django's date coercion raises ValidationError + # and the request 500s on URL fuzzing. + if date_from: + parsed = parse_date(date_from) + if parsed: + qs = qs.filter(date__gte=parsed) + if date_to: + parsed = parse_date(date_to) + if parsed: + qs = qs.filter(date__lte=parsed) + if paid == 'paid': + qs = qs.filter(is_paid=True) + elif paid == 'unpaid': + qs = qs.filter(is_paid=False) + return qs + + @login_required def absence_list(request): """Filtered list of absences with pagination + reason badges. @@ -5597,8 +5661,9 @@ def absence_list(request): # === ACCESS GATE === # Admins always pass. Supervisors pass if they supervise at least # one team. Everyone else gets a 403 instead of an empty list, so - # it's obvious the page wasn't meant for them. - if not (is_admin(user) or user.supervised_teams.exists()): + # it's obvious the page wasn't meant for them. DRY: same gate logic + # as /absences/log/ — use the shared helper. + if not _user_can_log_absences(user): return HttpResponseForbidden('Permission denied.') # Base queryset — scoped to what this user is allowed to see, with @@ -5610,17 +5675,20 @@ def absence_list(request): ) # === FILTERS === - # Each filter is best-effort: bad input (non-numeric IDs, bad - # date strings, unknown reason keys) is silently ignored rather - # than 500-ing. Empty values are skipped. + # All filter logic lives in the shared `_apply_absence_filters` helper + # (used here AND by absence_export_csv to guarantee filter parity). + # We still read the raw param values out here so the template context + # below can preselect the matching dropdown/checkbox options. + qs = _apply_absence_filters(qs, request) + + # Raw param read-back for template preselection (NOT for filtering — + # the helper handles that). Multi-value reason filter (Fix A2, May 2026): + # template renders reasons as a checkbox list sharing name="reason", + # so the querystring carries ?reason=sick&reason=annual on multi-select. + # getlist() pulls them ALL; whitelist against REASON_CHOICES. worker_id = request.GET.get('worker') team_id = request.GET.get('team') project_id = request.GET.get('project') - # Multi-value reason filter (Fix A2, May 2026): the template renders - # the reasons as a checkbox list, all sharing name="reason", so the - # querystring carries ?reason=sick&reason=annual on multi-select. - # getlist() pulls them ALL; we then whitelist against REASON_CHOICES - # so an attacker can't sneak an arbitrary string into the SQL filter. reasons = request.GET.getlist('reason') valid_reason_keys = dict(Absence.REASON_CHOICES) reasons = [r for r in reasons if r in valid_reason_keys] @@ -5628,39 +5696,6 @@ def absence_list(request): date_to = request.GET.get('date_to') paid = request.GET.get('paid') - if worker_id and worker_id.isdigit(): - qs = qs.filter(worker_id=worker_id) - if team_id and team_id.isdigit(): - qs = qs.filter(worker__teams__id=team_id).distinct() - if project_id and project_id.isdigit(): - # Direct FK filter — was previously a transitive join via - # worker__work_logs__project_id, which was a workaround for not - # having Absence.project. Now that the FK exists, filter on it - # directly: matches absences explicitly linked to this project. - # (An absence whose worker happens to have worked on the project - # before but with a NULL absence.project will no longer appear — - # which is the correct behaviour: filter by what the absence - # says, not by adjacent activity.) - qs = qs.filter(project_id=project_id) - if reasons: - qs = qs.filter(reason__in=reasons) - # parse_date() returns None for malformed input (e.g. "not-a-date") - # so the filter is simply skipped. Without this guard, Django's - # date coercion raises ValidationError (NOT ValueError/TypeError) - # and the request 500s — a tiny URL-fuzzing footgun. - if date_from: - parsed = parse_date(date_from) - if parsed: - qs = qs.filter(date__gte=parsed) - if date_to: - parsed = parse_date(date_to) - if parsed: - qs = qs.filter(date__lte=parsed) - if paid == 'paid': - qs = qs.filter(is_paid=True) - elif paid == 'unpaid': - qs = qs.filter(is_paid=False) - # === PAGINATION === # 25 per page — keeps the table snappy even with years of history. paginator = Paginator(qs, 25) @@ -5802,48 +5837,9 @@ def absence_export_csv(request): qs = _absence_user_queryset(request.user).select_related('worker', 'logged_by', 'project') - # =========================================================== - # FILTER BLOCK — DUPLICATED from absence_list above. - # Kept verbatim (same params, same order) so the CSV export - # honours the list page's filter URL exactly. If a future - # change adds a filter to the list view, mirror it here too. - # Follow-up TODO: factor into a `_apply_absence_filters(qs, request)` - # helper so the two views can't drift apart. - # =========================================================== - worker_id = request.GET.get('worker') - team_id = request.GET.get('team') - project_id = request.GET.get('project') - # Multi-value reason filter — kept in parity with absence_list (Fix A2). - reasons = request.GET.getlist('reason') - valid_reason_keys = dict(Absence.REASON_CHOICES) - reasons = [r for r in reasons if r in valid_reason_keys] - date_from = request.GET.get('date_from') - date_to = request.GET.get('date_to') - paid = request.GET.get('paid') - - if worker_id and worker_id.isdigit(): - qs = qs.filter(worker_id=worker_id) - if team_id and team_id.isdigit(): - qs = qs.filter(worker__teams__id=team_id).distinct() - if project_id and project_id.isdigit(): - # Direct FK filter — was previously worker__work_logs__project_id - # (a workaround for not having Absence.project). Now uses the - # direct FK. Mirrors absence_list above for filter parity. - qs = qs.filter(project_id=project_id) - if reasons: - qs = qs.filter(reason__in=reasons) - if date_from: - parsed = parse_date(date_from) - if parsed: - qs = qs.filter(date__gte=parsed) - if date_to: - parsed = parse_date(date_to) - if parsed: - qs = qs.filter(date__lte=parsed) - if paid == 'paid': - qs = qs.filter(is_paid=True) - elif paid == 'unpaid': - qs = qs.filter(is_paid=False) + # Filter parity with absence_list — both views call the same helper + # so adding a new filter param updates both in one place. + qs = _apply_absence_filters(qs, request) resp = HttpResponse(content_type='text/csv') resp['Content-Disposition'] = 'attachment; filename="absences.csv"' diff --git a/static/css/custom.css b/static/css/custom.css index a3f2873..16295bd 100644 --- a/static/css/custom.css +++ b/static/css/custom.css @@ -2195,8 +2195,10 @@ th.sortable.sorted .sort-arrow { opacity: 1; } .badge-absence-sick { background: var(--badge-bonus-bg); color: var(--badge-bonus-fg); } .badge-absence-family { background: var(--badge-bonus-bg); color: var(--badge-bonus-fg); } .badge-absence-annual { background: var(--badge-bonus-bg); color: var(--badge-bonus-fg); } -.badge-absence-unpaid { background: var(--badge-neutral-bg, #6c757d); color: #fff; } +/* unpaid/other use a neutral grey directly — no theme variation needed, */ +/* #6c757d has enough contrast on both dark and light backgrounds. */ +.badge-absence-unpaid { background: #6c757d; color: #fff; } .badge-absence-iod { background: var(--badge-overtime-bg, #ffc107); color: var(--badge-overtime-fg, #000); } .badge-absence-suspension { background: var(--badge-deduction-bg); color: var(--badge-deduction-fg); } .badge-absence-absconded { background: var(--badge-deduction-bg); color: var(--badge-deduction-fg); } -.badge-absence-other { background: var(--badge-neutral-bg, #6c757d); color: #fff; } +.badge-absence-other { background: #6c757d; color: #fff; }