From e797a71b9438f8cca3a71da497e65e89991f30a1 Mon Sep 17 00:00:00 2001 From: Konrad du Plessis Date: Fri, 15 May 2026 01:59:12 +0200 Subject: [PATCH] fix(dashboard,report): timezone.localdate + off-by-one date windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related foot-guns on the admin dashboard and payroll dashboard: 1. Every `timezone.now().date()` call returned the date in UTC, not in Africa/Johannesburg. Between 22:00 and midnight SAST that's the NEXT calendar day — so the "today" the dashboard thought it was could be ahead of what the user sees on the clock. Now uses `timezone.localdate()` which respects `settings.TIME_ZONE`. Same fix for `datetime.date.today()` calls — those used the server's system clock, which on the production VM is set to UTC. 2. "Absences (last 7 days)" and "Paid (Last 60 Days)" both subtracted the FULL window length and combined it with `>=`, producing N+1 inclusive days. E.g. `today - timedelta(days=7)` with `date__gte` spans 8 calendar days, not 7. Now subtract N-1 so the windows are exactly N days. Regression test: DateWindowOffByOneTests. Findings 12 + 13. Co-Authored-By: Claude Opus 4.7 (1M context) --- core/tests.py | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++ core/views.py | 49 +++++++++++++++++++++++------------------ 2 files changed, 89 insertions(+), 21 deletions(-) diff --git a/core/tests.py b/core/tests.py index cf74707..6189af2 100644 --- a/core/tests.py +++ b/core/tests.py @@ -2871,6 +2871,67 @@ class AbsenceWorkerDetailTests(TestCase): self.assertContains(resp, 'Annual Leave: 1') +class DateWindowOffByOneTests(TestCase): + """Regression for Finding 13: 'last N days' windows on the dashboard + used to subtract N (not N-1), producing N+1 inclusive days. Now they + span exactly N days.""" + + @classmethod + def setUpTestData(cls): + cls.admin = User.objects.create_user( + username='offbyone', password='pw', is_staff=True, is_superuser=True, + ) + cls.worker = Worker.objects.create( + name='W', id_number='OBO1', monthly_salary=Decimal('6000'), + ) + + def setUp(self): + self.client.force_login(self.admin) + + def test_absences_last_7_days_window_is_exactly_7(self): + """Today + 6 days prior should be the 7-day window. Day 7 prior + (the 8th day inclusive) MUST be excluded.""" + from datetime import timedelta as _td + from django.utils import timezone as _tz + today = _tz.localdate() + # Six absences at 0,1,2,3,4,5,6 days back → all should count (7 total) + for n in range(7): + Absence.objects.create( + worker=self.worker, date=today - _td(days=n), + reason='sick' if n % 2 == 0 else 'other', + ) + # An absence 7 days back must NOT count (would make the window span 8 days) + Absence.objects.create( + worker=self.worker, date=today - _td(days=7), reason='other', + ) + resp = self.client.get('/') + self.assertEqual( + resp.context['absences_recent_count'], 7, + '"Last 7 days" must span exactly 7 calendar days inclusive.', + ) + + def test_recent_payments_last_60_days_window_is_exactly_60(self): + """Payroll-dashboard 'Paid (Last 60 Days)' must span exactly 60 days.""" + from datetime import timedelta as _td + from django.utils import timezone as _tz + today = _tz.localdate() + # Payment 59 days back → counts + PayrollRecord.objects.create( + worker=self.worker, amount_paid=Decimal('100.00'), + date=today - _td(days=59), + ) + # Payment 60 days back → MUST NOT count (would span 61 days) + PayrollRecord.objects.create( + worker=self.worker, amount_paid=Decimal('9999.00'), + date=today - _td(days=60), + ) + resp = self.client.get('/payroll/?status=history') + self.assertEqual( + resp.context['recent_payments_total'], Decimal('100.00'), + '"Last 60 days" must span exactly 60 calendar days inclusive.', + ) + + class AbsenceDashboardCardTests(TestCase): """Round D — Admin dashboard shows 'X absent in last 7 days' alert card when count > 0; renders nothing when count is 0.""" diff --git a/core/views.py b/core/views.py index 3dfc9a5..f584a1b 100644 --- a/core/views.py +++ b/core/views.py @@ -101,7 +101,7 @@ def get_pay_period(team, reference_date=None): return (None, None) if reference_date is None: - reference_date = timezone.now().date() + reference_date = timezone.localdate() anchor = team.pay_start_date @@ -422,7 +422,7 @@ def index(request): # The payroll dashboard has its own separate "Paid (60D)" card if # the rolling-window view is wanted. Filtering by date__year + # date__month is unambiguous and matches the label exactly. - _today_dt = timezone.now().date() + _today_dt = timezone.localdate() paid_this_month = PayrollRecord.objects.filter( date__year=_today_dt.year, date__month=_today_dt.month, @@ -436,8 +436,8 @@ def index(request): )['total'] or Decimal('0.00') # This week summary - start_of_week = timezone.now().date() - timezone.timedelta( - days=timezone.now().date().weekday() + start_of_week = timezone.localdate() - timezone.timedelta( + days=timezone.localdate().weekday() ) this_week_logs = WorkLog.objects.filter(date__gte=start_of_week).count() @@ -457,7 +457,7 @@ def index(request): # Count certificates that are expired or expire within the next 30 days. # Only shown on the dashboard when the count is non-zero (so the stat # card disappears when everything is in good standing). - today = datetime.date.today() + today = timezone.localdate() thirty_days_out = today + datetime.timedelta(days=30) certs_expired_count = WorkerCertificate.objects.filter( valid_until__lt=today, worker__active=True, @@ -472,7 +472,11 @@ def index(request): # Conditional stat card — only renders when count > 0. Same look as # the existing cert-expiry alert card. Admin-only (this branch only # runs for is_admin()). - seven_days_ago = today - datetime.timedelta(days=7) + # + # 7-day window math: subtract 6 days, not 7. With `>=` that produces + # exactly 7 inclusive days (today + 6 prior). Subtracting 7 would + # span 8 days (off-by-one — bit us pre-May 2026). + seven_days_ago = today - datetime.timedelta(days=6) absences_recent_count = Absence.objects.filter( date__gte=seven_days_ago, ).count() @@ -524,8 +528,8 @@ def index(request): ).distinct().count() # This week summary — only their own logs - start_of_week = timezone.now().date() - timezone.timedelta( - days=timezone.now().date().weekday() + start_of_week = timezone.localdate() - timezone.timedelta( + days=timezone.localdate().weekday() ) this_week_logs = WorkLog.objects.filter( date__gte=start_of_week, supervisor=user @@ -1037,7 +1041,7 @@ def work_history(request): # --- View mode: list or calendar --- view_mode = request.GET.get('view', 'list') - today = timezone.now().date() + today = timezone.localdate() # Build a query string that preserves all current filters # (used by the List/Calendar toggle links to keep filters when switching) @@ -1708,7 +1712,7 @@ def worker_detail(request, worker_id): 'worker_absences': worker_absences, 'absence_total_count': absence_total_count, 'absence_reason_choices': Absence.REASON_CHOICES, - 'today': timezone.now().date(), + 'today': timezone.localdate(), } return render(request, 'core/workers/detail.html', context) @@ -1804,7 +1808,7 @@ def _build_worker_report_context(status=None, project_id=None, team_id=None): _active_warnings=Count('warnings', distinct=True), ).order_by('name') - today = datetime.date.today() + today = timezone.localdate() thirty_days_out = today + datetime.timedelta(days=30) rows = [] @@ -2006,7 +2010,7 @@ def team_detail(request, team_id): # --- Pay schedule preview: current + next 2 periods (3 total) --- pay_periods = [] if team.pay_frequency and team.pay_start_date: - today = datetime.date.today() + today = timezone.localdate() current = get_pay_period(team, today) if current: pay_periods.append(current) @@ -3107,8 +3111,11 @@ def payroll_dashboard(request): 'worker' ).order_by('-date', '-id') - # --- Recent payments total (last 60 days) --- - sixty_days_ago = timezone.now().date() - timezone.timedelta(days=60) + # --- Recent payments total (last 60 days, inclusive) --- + # 60-day window math: subtract 59 days, not 60. With `>=` that yields + # exactly 60 inclusive days (today + 59 prior). The previous + # `days=60` produced a 61-day span (off-by-one). + sixty_days_ago = timezone.localdate() - timezone.timedelta(days=59) recent_payments_total = PayrollRecord.objects.filter( date__gte=sixty_days_ago ).aggregate(total=Sum('amount_paid'))['total'] or Decimal('0.00') @@ -3126,7 +3133,7 @@ def payroll_dashboard(request): # === CHART DATE-WINDOW SETUP (moved up so the batched queries below can # also use it) === - today = timezone.now().date() + today = timezone.localdate() chart_months = [] for i in range(5, -1, -1): m = today.month - i @@ -3686,7 +3693,7 @@ def _process_single_payment(worker_id, selected_log_ids=None, selected_adj_ids=N payroll_record = PayrollRecord.objects.create( worker=worker, amount_paid=total_amount, - date=timezone.now().date(), + date=timezone.localdate(), ) # Link work logs to this payment @@ -4175,9 +4182,9 @@ def add_adjustment(request): # Validate date try: - adj_date = datetime.datetime.strptime(date_str, '%Y-%m-%d').date() if date_str else timezone.now().date() + adj_date = datetime.datetime.strptime(date_str, '%Y-%m-%d').date() if date_str else timezone.localdate() except ValueError: - adj_date = timezone.now().date() + adj_date = timezone.localdate() # Validate project for types that require it project = None @@ -4815,7 +4822,7 @@ def worker_lookup_ajax(request, worker_id): return JsonResponse({'error': 'Not authorized'}, status=403) worker = get_object_or_404(Worker, id=worker_id) - today = timezone.now().date() + today = timezone.localdate() # === AMOUNT PAYABLE === # Same logic as preview_payslip: find unpaid work logs for this worker @@ -5008,7 +5015,7 @@ def add_repayment_ajax(request, worker_id): worker=worker, type=repayment_type, amount=amount, - date=timezone.now().date(), + date=timezone.localdate(), description=description or f'{loan.get_loan_type_display()} repayment', loan=loan, ) @@ -5198,7 +5205,7 @@ def create_receipt(request): else: # GET request — show a blank form with today's date - form = ExpenseReceiptForm(initial={'date': timezone.now().date()}) + form = ExpenseReceiptForm(initial={'date': timezone.localdate()}) items = ExpenseLineItemFormSet() return render(request, 'core/create_receipt.html', {