fix(dashboard,report): timezone.localdate + off-by-one date windows
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) <noreply@anthropic.com>
This commit is contained in:
parent
2e6b78d28a
commit
e797a71b94
@ -2871,6 +2871,67 @@ class AbsenceWorkerDetailTests(TestCase):
|
|||||||
self.assertContains(resp, 'Annual Leave: 1')
|
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):
|
class AbsenceDashboardCardTests(TestCase):
|
||||||
"""Round D — Admin dashboard shows 'X absent in last 7 days' alert card
|
"""Round D — Admin dashboard shows 'X absent in last 7 days' alert card
|
||||||
when count > 0; renders nothing when count is 0."""
|
when count > 0; renders nothing when count is 0."""
|
||||||
|
|||||||
@ -101,7 +101,7 @@ def get_pay_period(team, reference_date=None):
|
|||||||
return (None, None)
|
return (None, None)
|
||||||
|
|
||||||
if reference_date is None:
|
if reference_date is None:
|
||||||
reference_date = timezone.now().date()
|
reference_date = timezone.localdate()
|
||||||
|
|
||||||
anchor = team.pay_start_date
|
anchor = team.pay_start_date
|
||||||
|
|
||||||
@ -422,7 +422,7 @@ def index(request):
|
|||||||
# The payroll dashboard has its own separate "Paid (60D)" card if
|
# The payroll dashboard has its own separate "Paid (60D)" card if
|
||||||
# the rolling-window view is wanted. Filtering by date__year +
|
# the rolling-window view is wanted. Filtering by date__year +
|
||||||
# date__month is unambiguous and matches the label exactly.
|
# date__month is unambiguous and matches the label exactly.
|
||||||
_today_dt = timezone.now().date()
|
_today_dt = timezone.localdate()
|
||||||
paid_this_month = PayrollRecord.objects.filter(
|
paid_this_month = PayrollRecord.objects.filter(
|
||||||
date__year=_today_dt.year,
|
date__year=_today_dt.year,
|
||||||
date__month=_today_dt.month,
|
date__month=_today_dt.month,
|
||||||
@ -436,8 +436,8 @@ def index(request):
|
|||||||
)['total'] or Decimal('0.00')
|
)['total'] or Decimal('0.00')
|
||||||
|
|
||||||
# This week summary
|
# This week summary
|
||||||
start_of_week = timezone.now().date() - timezone.timedelta(
|
start_of_week = timezone.localdate() - timezone.timedelta(
|
||||||
days=timezone.now().date().weekday()
|
days=timezone.localdate().weekday()
|
||||||
)
|
)
|
||||||
this_week_logs = WorkLog.objects.filter(date__gte=start_of_week).count()
|
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.
|
# 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
|
# Only shown on the dashboard when the count is non-zero (so the stat
|
||||||
# card disappears when everything is in good standing).
|
# card disappears when everything is in good standing).
|
||||||
today = datetime.date.today()
|
today = timezone.localdate()
|
||||||
thirty_days_out = today + datetime.timedelta(days=30)
|
thirty_days_out = today + datetime.timedelta(days=30)
|
||||||
certs_expired_count = WorkerCertificate.objects.filter(
|
certs_expired_count = WorkerCertificate.objects.filter(
|
||||||
valid_until__lt=today, worker__active=True,
|
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
|
# Conditional stat card — only renders when count > 0. Same look as
|
||||||
# the existing cert-expiry alert card. Admin-only (this branch only
|
# the existing cert-expiry alert card. Admin-only (this branch only
|
||||||
# runs for is_admin()).
|
# 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(
|
absences_recent_count = Absence.objects.filter(
|
||||||
date__gte=seven_days_ago,
|
date__gte=seven_days_ago,
|
||||||
).count()
|
).count()
|
||||||
@ -524,8 +528,8 @@ def index(request):
|
|||||||
).distinct().count()
|
).distinct().count()
|
||||||
|
|
||||||
# This week summary — only their own logs
|
# This week summary — only their own logs
|
||||||
start_of_week = timezone.now().date() - timezone.timedelta(
|
start_of_week = timezone.localdate() - timezone.timedelta(
|
||||||
days=timezone.now().date().weekday()
|
days=timezone.localdate().weekday()
|
||||||
)
|
)
|
||||||
this_week_logs = WorkLog.objects.filter(
|
this_week_logs = WorkLog.objects.filter(
|
||||||
date__gte=start_of_week, supervisor=user
|
date__gte=start_of_week, supervisor=user
|
||||||
@ -1037,7 +1041,7 @@ def work_history(request):
|
|||||||
|
|
||||||
# --- View mode: list or calendar ---
|
# --- View mode: list or calendar ---
|
||||||
view_mode = request.GET.get('view', 'list')
|
view_mode = request.GET.get('view', 'list')
|
||||||
today = timezone.now().date()
|
today = timezone.localdate()
|
||||||
|
|
||||||
# Build a query string that preserves all current filters
|
# Build a query string that preserves all current filters
|
||||||
# (used by the List/Calendar toggle links to keep filters when switching)
|
# (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,
|
'worker_absences': worker_absences,
|
||||||
'absence_total_count': absence_total_count,
|
'absence_total_count': absence_total_count,
|
||||||
'absence_reason_choices': Absence.REASON_CHOICES,
|
'absence_reason_choices': Absence.REASON_CHOICES,
|
||||||
'today': timezone.now().date(),
|
'today': timezone.localdate(),
|
||||||
}
|
}
|
||||||
return render(request, 'core/workers/detail.html', context)
|
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),
|
_active_warnings=Count('warnings', distinct=True),
|
||||||
).order_by('name')
|
).order_by('name')
|
||||||
|
|
||||||
today = datetime.date.today()
|
today = timezone.localdate()
|
||||||
thirty_days_out = today + datetime.timedelta(days=30)
|
thirty_days_out = today + datetime.timedelta(days=30)
|
||||||
|
|
||||||
rows = []
|
rows = []
|
||||||
@ -2006,7 +2010,7 @@ def team_detail(request, team_id):
|
|||||||
# --- Pay schedule preview: current + next 2 periods (3 total) ---
|
# --- Pay schedule preview: current + next 2 periods (3 total) ---
|
||||||
pay_periods = []
|
pay_periods = []
|
||||||
if team.pay_frequency and team.pay_start_date:
|
if team.pay_frequency and team.pay_start_date:
|
||||||
today = datetime.date.today()
|
today = timezone.localdate()
|
||||||
current = get_pay_period(team, today)
|
current = get_pay_period(team, today)
|
||||||
if current:
|
if current:
|
||||||
pay_periods.append(current)
|
pay_periods.append(current)
|
||||||
@ -3107,8 +3111,11 @@ def payroll_dashboard(request):
|
|||||||
'worker'
|
'worker'
|
||||||
).order_by('-date', '-id')
|
).order_by('-date', '-id')
|
||||||
|
|
||||||
# --- Recent payments total (last 60 days) ---
|
# --- Recent payments total (last 60 days, inclusive) ---
|
||||||
sixty_days_ago = timezone.now().date() - timezone.timedelta(days=60)
|
# 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(
|
recent_payments_total = PayrollRecord.objects.filter(
|
||||||
date__gte=sixty_days_ago
|
date__gte=sixty_days_ago
|
||||||
).aggregate(total=Sum('amount_paid'))['total'] or Decimal('0.00')
|
).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
|
# === CHART DATE-WINDOW SETUP (moved up so the batched queries below can
|
||||||
# also use it) ===
|
# also use it) ===
|
||||||
today = timezone.now().date()
|
today = timezone.localdate()
|
||||||
chart_months = []
|
chart_months = []
|
||||||
for i in range(5, -1, -1):
|
for i in range(5, -1, -1):
|
||||||
m = today.month - i
|
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(
|
payroll_record = PayrollRecord.objects.create(
|
||||||
worker=worker,
|
worker=worker,
|
||||||
amount_paid=total_amount,
|
amount_paid=total_amount,
|
||||||
date=timezone.now().date(),
|
date=timezone.localdate(),
|
||||||
)
|
)
|
||||||
|
|
||||||
# Link work logs to this payment
|
# Link work logs to this payment
|
||||||
@ -4175,9 +4182,9 @@ def add_adjustment(request):
|
|||||||
|
|
||||||
# Validate date
|
# Validate date
|
||||||
try:
|
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:
|
except ValueError:
|
||||||
adj_date = timezone.now().date()
|
adj_date = timezone.localdate()
|
||||||
|
|
||||||
# Validate project for types that require it
|
# Validate project for types that require it
|
||||||
project = None
|
project = None
|
||||||
@ -4815,7 +4822,7 @@ def worker_lookup_ajax(request, worker_id):
|
|||||||
return JsonResponse({'error': 'Not authorized'}, status=403)
|
return JsonResponse({'error': 'Not authorized'}, status=403)
|
||||||
|
|
||||||
worker = get_object_or_404(Worker, id=worker_id)
|
worker = get_object_or_404(Worker, id=worker_id)
|
||||||
today = timezone.now().date()
|
today = timezone.localdate()
|
||||||
|
|
||||||
# === AMOUNT PAYABLE ===
|
# === AMOUNT PAYABLE ===
|
||||||
# Same logic as preview_payslip: find unpaid work logs for this worker
|
# 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,
|
worker=worker,
|
||||||
type=repayment_type,
|
type=repayment_type,
|
||||||
amount=amount,
|
amount=amount,
|
||||||
date=timezone.now().date(),
|
date=timezone.localdate(),
|
||||||
description=description or f'{loan.get_loan_type_display()} repayment',
|
description=description or f'{loan.get_loan_type_display()} repayment',
|
||||||
loan=loan,
|
loan=loan,
|
||||||
)
|
)
|
||||||
@ -5198,7 +5205,7 @@ def create_receipt(request):
|
|||||||
|
|
||||||
else:
|
else:
|
||||||
# GET request — show a blank form with today's date
|
# 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()
|
items = ExpenseLineItemFormSet()
|
||||||
|
|
||||||
return render(request, 'core/create_receipt.html', {
|
return render(request, 'core/create_receipt.html', {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user