From 8a45ccd4e4c54d08f0bec1bdd553ff0a188571c2 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Tue, 31 Mar 2026 16:26:57 +0100 Subject: [PATCH] monitoring: fix timezone bug in funders daily changes This commit fixes https://github.com/ThreeSixtyGiving/datastore/issues/328 which was caused by UTC/BST differences meaning that funder records were not found. Pipeline runs happen just after midnight local time, but on a UTC->BST transition those runs then happen before midnight in UTC. Because the server uses UTC as default, and there's no user timezone information ingested by the API, the BST differences are not picked up so funder records are not found and the monitoring API thinks the funders have been removed. This fix sets the timezone in the API call to Europe/London so that BST differences are correctly handled and the records found. This commit also adds a couple of new test cases that check UTC->BST and also BST->UTC. --- datastore/api/monitoring/api.py | 7 + datastore/tests/test_monitoring_metrics.py | 194 ++++++++++++++++++++- 2 files changed, 200 insertions(+), 1 deletion(-) diff --git a/datastore/api/monitoring/api.py b/datastore/api/monitoring/api.py index 1a8a6ae..49a5389 100644 --- a/datastore/api/monitoring/api.py +++ b/datastore/api/monitoring/api.py @@ -138,6 +138,13 @@ def get_serializer_class(self): return ChangedFunderMetricsRecordJSONSerializer def get_queryset(self): + + # The default server timezone is UTC (as set in settings) so the following + # calculations will all be done in UTC by default. This causes problems + # during BST. To fix this manually set the timezone to Europe/London so + # that calculations will pick up GMT/BST differences. + timezone.activate("Europe/London") + start_date = date.fromisoformat(self.kwargs.get("start_date")) end_date = date.fromisoformat(self.kwargs.get("end_date")) diff --git a/datastore/tests/test_monitoring_metrics.py b/datastore/tests/test_monitoring_metrics.py index bc0df23..88f8a04 100644 --- a/datastore/tests/test_monitoring_metrics.py +++ b/datastore/tests/test_monitoring_metrics.py @@ -1,7 +1,7 @@ import csv import copy import io -from datetime import timedelta, datetime, date +from datetime import date, datetime, timedelta, timezone, UTC import datetime as dt from typing import Dict, Any, List, Optional from django.urls import reverse @@ -44,6 +44,8 @@ fake_grant_org, ) +TZ_BST_OFFSET = timezone(timedelta(hours=1)) + class TestMonitoringMetricsQueries(APITestCase): # Things to test: @@ -557,6 +559,196 @@ def test_funder_change_detection(self): self.assertFalse(funder_change_records[0]["record_is_new"]) self.assertFalse(funder_change_records[0]["record_was_removed"]) + def test_funder_change_detection_into_bst(self): + # This test simulates four days of pipeline runs from UTC to BST. + + # Day 1: UTC: creates the base case: 2 funders with 1 grant each + # Day 2: UTC: identical, should have no changes. + # Day 3: BST: add a second grant to funder 2, this chould generate a change to total_grants and total_gbp + # Day 4: BST: identical, should have no changes. + + fake = faker.Faker() + funder_a = fake_grant_org(fake) + funder_b = fake_grant_org(fake) + recipient = fake_grant_org(fake) + test_publisher = fake_publisher_info(fake) + + # Create two getter runs with two funders and in UTC. + with fake_getter_run( + fake, timestamp=datetime(2026, 3, 28, 0, 10, 0, tzinfo=UTC) + ) as getter_run_1: + sourcefile_a = fake_sourcefile( + fake, getter_run_1, publisher_info=test_publisher + ) + sourcefile_b = fake_sourcefile( + fake, getter_run_1, publisher_info=test_publisher + ) + fake_grant( + fake, sourcefile=sourcefile_a, funder=funder_a, recipient=recipient + ) + fake_grant( + fake, sourcefile=sourcefile_b, funder=funder_b, recipient=recipient + ) + + with fake_getter_run( + fake, timestamp=datetime(2026, 3, 29, 0, 10, 0, tzinfo=UTC) + ) as getter_run_2: + copy_sourcefile(fake, sourcefile_a, getter_run_2, copy_grants=True) + copy_sourcefile(fake, sourcefile_b, getter_run_2, copy_grants=True) + + # Test that no change records are detected when nothing has changed + self.assertEqual( + len( + self.get_funder_change_records( + from_=date(2026, 3, 28), to_=date(2026, 3, 29) + ) + ), + 0, + ) + + # Getter run, now in BST, add a second grant. + with fake_getter_run( + fake, timestamp=datetime(2026, 3, 30, 0, 10, 0, tzinfo=TZ_BST_OFFSET) + ) as getter_run_3: + copy_sourcefile(fake, sourcefile_a, getter_run_3, copy_grants=True) + + # # Add second grant to funder b. + sourcefile_b_3 = copy_sourcefile( + fake, sourcefile_b, getter_run_3, copy_grants=True + ) + fake_grant(fake, sourcefile_b_3, funder_b, recipient) + + # Test that there is only one change record and no removal record. + funder_change_records = self.get_funder_change_records( + from_=date(2026, 3, 29), to_=date(2026, 3, 30) + ) + self.assertEqual(len(funder_change_records), 1) + self.assertEqual( + funder_change_records[0]["start_record"]["funder_org_id"], funder_b["id"] + ) + self.assertEqual( + funder_change_records[0]["start_record"]["metrics"]["total_grants"], 1 + ) + self.assertEqual( + funder_change_records[0]["end_record"]["metrics"]["total_grants"], 2 + ) + self.assertIn("total_grants", funder_change_records[0]["changed_metrics"]) + self.assertIn("total_gbp", funder_change_records[0]["changed_metrics"]) + self.assertFalse(funder_change_records[0]["record_is_new"]) + self.assertFalse(funder_change_records[0]["record_was_removed"]) + + # Test final step, in BST. + with fake_getter_run( + fake, timestamp=datetime(2026, 3, 31, 0, 10, 0, tzinfo=TZ_BST_OFFSET) + ) as getter_run_4: + copy_sourcefile(fake, sourcefile_a, getter_run_4, copy_grants=True) + copy_sourcefile(fake, sourcefile_b_3, getter_run_4, copy_grants=True) + + ## Test that no change records are detected when nothing has changed + self.assertEqual( + len( + self.get_funder_change_records( + from_=date(2026, 3, 30), to_=date(2026, 3, 31) + ) + ), + 0, + ) + + def test_funder_change_detection_from_bst(self): + # This test simulates four days of pipeline runs from BST to UTC. to BST. + + # Day 1: BST: creates the base case: 2 funders with 1 grant each + # Day 2: BST: identical, should have no changes. + # Day 3: UTC: add a second grant to funder 2, this chould generate a change to total_grants and total_gbp + # Day 4: UTC: identical, should have no changes. + + fake = faker.Faker() + funder_a = fake_grant_org(fake) + funder_b = fake_grant_org(fake) + recipient = fake_grant_org(fake) + test_publisher = fake_publisher_info(fake) + + # Create two getter runs with two funders and in BST. + with fake_getter_run( + fake, timestamp=datetime(2026, 10, 24, 0, 10, 0, tzinfo=TZ_BST_OFFSET) + ) as getter_run_1: + sourcefile_a = fake_sourcefile( + fake, getter_run_1, publisher_info=test_publisher + ) + sourcefile_b = fake_sourcefile( + fake, getter_run_1, publisher_info=test_publisher + ) + fake_grant( + fake, sourcefile=sourcefile_a, funder=funder_a, recipient=recipient + ) + fake_grant( + fake, sourcefile=sourcefile_b, funder=funder_b, recipient=recipient + ) + + with fake_getter_run( + fake, timestamp=datetime(2026, 10, 25, 0, 10, 0, tzinfo=TZ_BST_OFFSET) + ) as getter_run_2: + copy_sourcefile(fake, sourcefile_a, getter_run_2, copy_grants=True) + copy_sourcefile(fake, sourcefile_b, getter_run_2, copy_grants=True) + + # Test that no change records are detected when nothing has changed + self.assertEqual( + len( + self.get_funder_change_records( + from_=date(2026, 10, 24), to_=date(2026, 10, 25) + ) + ), + 0, + ) + + # Getter run, now in BST, add a second grant. + with fake_getter_run( + fake, timestamp=datetime(2026, 10, 26, 0, 10, 0, tzinfo=UTC) + ) as getter_run_3: + copy_sourcefile(fake, sourcefile_a, getter_run_3, copy_grants=True) + + # # Add second grant to funder b. + sourcefile_b_3 = copy_sourcefile( + fake, sourcefile_b, getter_run_3, copy_grants=True + ) + fake_grant(fake, sourcefile_b_3, funder_b, recipient) + + # Test that there is only one change record and no removal record. + funder_change_records = self.get_funder_change_records( + from_=date(2026, 10, 25), to_=date(2026, 10, 26) + ) + self.assertEqual(len(funder_change_records), 1) + self.assertEqual( + funder_change_records[0]["start_record"]["funder_org_id"], funder_b["id"] + ) + self.assertEqual( + funder_change_records[0]["start_record"]["metrics"]["total_grants"], 1 + ) + self.assertEqual( + funder_change_records[0]["end_record"]["metrics"]["total_grants"], 2 + ) + self.assertIn("total_grants", funder_change_records[0]["changed_metrics"]) + self.assertIn("total_gbp", funder_change_records[0]["changed_metrics"]) + self.assertFalse(funder_change_records[0]["record_is_new"]) + self.assertFalse(funder_change_records[0]["record_was_removed"]) + + # Test final step, in BST. + with fake_getter_run( + fake, timestamp=datetime(2026, 10, 27, 0, 10, 0, tzinfo=UTC) + ) as getter_run_4: + copy_sourcefile(fake, sourcefile_a, getter_run_4, copy_grants=True) + copy_sourcefile(fake, sourcefile_b_3, getter_run_4, copy_grants=True) + + ## Test that no change records are detected when nothing has changed + self.assertEqual( + len( + self.get_funder_change_records( + from_=date(2026, 10, 26), to_=date(2026, 10, 27) + ) + ), + 0, + ) + def test_multiple_getterruns_in_one_day(self): """ When there are multiple getter runs in one day, the snapshot api should show only the most recent.