Fix flaky chart date windows: use calendar months, not 30-day steps
_monthly and signup_split approximated "N months ago" as N*30 days, which drifts against real calendar months by several days a year. Late in some months that drift undershot a full month, so the "dense 13-point series" tests flaked depending on which day they ran (confirmed: every 28th+ of most months). Switched to dateutil.relativedelta for exact calendar-month arithmetic, which is stable on every day of every month.
This commit is contained in:
@@ -10,6 +10,7 @@ from datetime import timedelta
|
|||||||
from decimal import Decimal
|
from decimal import Decimal
|
||||||
|
|
||||||
from allauth.mfa.models import Authenticator
|
from allauth.mfa.models import Authenticator
|
||||||
|
from dateutil.relativedelta import relativedelta
|
||||||
from django.contrib.auth import get_user_model
|
from django.contrib.auth import get_user_model
|
||||||
from django.db.models import Count, DateField, DecimalField, Exists, F, IntegerField, OuterRef, Q, Subquery, Sum, Value
|
from django.db.models import Count, DateField, DecimalField, Exists, F, IntegerField, OuterRef, Q, Subquery, Sum, Value
|
||||||
from django.db.models.functions import Coalesce, TruncMonth
|
from django.db.models.functions import Coalesce, TruncMonth
|
||||||
@@ -212,7 +213,7 @@ def _dues_owed():
|
|||||||
def _monthly(queryset, field, value, months=MONTHS_OF_HISTORY):
|
def _monthly(queryset, field, value, months=MONTHS_OF_HISTORY):
|
||||||
"""A dense month-by-month series — zero-filled, because a chart that silently skips
|
"""A dense month-by-month series — zero-filled, because a chart that silently skips
|
||||||
empty months draws a smooth line over a month where nothing happened."""
|
empty months draws a smooth line over a month where nothing happened."""
|
||||||
start = (timezone.now() - timedelta(days=30 * months)).replace(day=1, hour=0, minute=0, second=0, microsecond=0)
|
start = (timezone.now() - relativedelta(months=months)).replace(day=1, hour=0, minute=0, second=0, microsecond=0)
|
||||||
|
|
||||||
rows = queryset.filter(**{f"{field}__gte": start}).annotate(month=TruncMonth(field)).values("month").annotate(value=value).order_by("month")
|
rows = queryset.filter(**{f"{field}__gte": start}).annotate(month=TruncMonth(field)).values("month").annotate(value=value).order_by("month")
|
||||||
found = {row["month"].strftime("%Y-%m"): row["value"] or 0 for row in rows if row["month"]}
|
found = {row["month"].strftime("%Y-%m"): row["value"] or 0 for row in rows if row["month"]}
|
||||||
@@ -297,7 +298,7 @@ def signup_split(club=None, months=MONTHS_OF_HISTORY):
|
|||||||
Each member's earliest season is resolved once up front rather than per row: the same
|
Each member's earliest season is resolved once up front rather than per row: the same
|
||||||
question asked inside a loop is one query per membership.
|
question asked inside a loop is one query per membership.
|
||||||
"""
|
"""
|
||||||
start = (timezone.now() - timedelta(days=30 * months)).replace(day=1, hour=0, minute=0, second=0, microsecond=0)
|
start = (timezone.now() - relativedelta(months=months)).replace(day=1, hour=0, minute=0, second=0, microsecond=0)
|
||||||
|
|
||||||
memberships = ClubMembership.objects.all() if club is None else ClubMembership.objects.filter(club=club)
|
memberships = ClubMembership.objects.all() if club is None else ClubMembership.objects.filter(club=club)
|
||||||
|
|
||||||
|
|||||||
@@ -779,6 +779,17 @@ class PlatformChartTests(TestCase):
|
|||||||
self.assertEqual(len(series), 13)
|
self.assertEqual(len(series), 13)
|
||||||
self.assertTrue(all(point["new"] == 0 and point["returning"] == 0 for point in series))
|
self.assertTrue(all(point["new"] == 0 and point["returning"] == 0 for point in series))
|
||||||
|
|
||||||
|
def test_the_series_stays_dense_on_a_late_day_of_the_month(self):
|
||||||
|
# A 30-days-per-month approximation of "12 months ago" drifts by a few days a
|
||||||
|
# year, and on the tail end of a month that drift used to fall short of a full
|
||||||
|
# calendar month, silently dropping the series to 12 points instead of 13.
|
||||||
|
late_month_day = timezone.now().replace(day=28)
|
||||||
|
|
||||||
|
with mock.patch.object(timezone, "now", return_value=late_month_day):
|
||||||
|
series = platform_charts()["signups"]
|
||||||
|
|
||||||
|
self.assertEqual(len(series), 13)
|
||||||
|
|
||||||
def test_signups_land_in_the_month_they_happened(self):
|
def test_signups_land_in_the_month_they_happened(self):
|
||||||
ClubMembership.objects.create(club=self.club, season=self.season, member=self.member, signed_up_at=timezone.localdate())
|
ClubMembership.objects.create(club=self.club, season=self.season, member=self.member, signed_up_at=timezone.localdate())
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user