From a30b57a9df71e67dbf5f3ce1209d8a700524de2b Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Thu, 26 Mar 2026 15:47:50 -0700 Subject: [PATCH] fix: prevent duplicate fuel code expiry notification emails (#4131) The scheduler runs outside the FastAPI request lifecycle, so its db session has no auto-commit. mark_fuel_codes_notified() updated expiry_notification_sent_at but the change was never committed, causing every daily run to re-select and re-email the same codes. - Add explicit db_session.commit() after marking codes as notified - Fix get_expiring_fuel_codes query to use explicit join on FuelCodeStatus instead of joinedload for the WHERE filter - Update tests to verify commit behavior --- backend/lcfs/scripts/tasks/fuel_code_expiry.py | 4 ++++ backend/lcfs/scripts/tests/test_fuel_code_expiry.py | 8 ++++++++ backend/lcfs/web/api/fuel_code/repo.py | 3 ++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/backend/lcfs/scripts/tasks/fuel_code_expiry.py b/backend/lcfs/scripts/tasks/fuel_code_expiry.py index d19abc4c1..ce14a0f84 100644 --- a/backend/lcfs/scripts/tasks/fuel_code_expiry.py +++ b/backend/lcfs/scripts/tasks/fuel_code_expiry.py @@ -117,6 +117,10 @@ async def notify_expiring_fuel_code(db_session: AsyncSession): f"Marking {len(notified_fuel_code_ids)} fuel codes as notified" ) await fuel_code_repo.mark_fuel_codes_notified(notified_fuel_code_ids) + # Commit so the notification timestamps are persisted — without this, + # the session closes uncommitted and the same codes are re-selected + # on the next scheduler run, causing duplicate emails. + await db_session.commit() logger.info( f"Sent fuel code expiry notifications to {success_count}/{total_emails} contacts" diff --git a/backend/lcfs/scripts/tests/test_fuel_code_expiry.py b/backend/lcfs/scripts/tests/test_fuel_code_expiry.py index 59af0318a..39a6a2907 100644 --- a/backend/lcfs/scripts/tests/test_fuel_code_expiry.py +++ b/backend/lcfs/scripts/tests/test_fuel_code_expiry.py @@ -130,6 +130,9 @@ async def test_notify_expiring_fuel_code_success( notified_ids = mock_repo.mark_fuel_codes_notified.call_args[0][0] assert sorted(notified_ids) == [1, 2, 3] + # Verify the session is committed so notification timestamps persist + mock_db_session.commit.assert_called_once() + @pytest.mark.anyio @patch( "lcfs.scripts.tasks.fuel_code_expiry.settings.feature_fuel_code_expiry_email", @@ -222,6 +225,8 @@ async def test_notify_expiring_fuel_code_email_service_failure( assert result is False # No fuel codes should be marked as notified when all emails fail mock_repo.mark_fuel_codes_notified.assert_not_called() + # No commit should happen when no emails were sent + mock_db_session.commit.assert_not_called() @pytest.mark.anyio async def test_notify_expiring_fuel_code_partial_success( @@ -262,6 +267,9 @@ async def test_notify_expiring_fuel_code_partial_success( # Only codes 1 and 3 (Company A) should be marked since that email succeeded assert sorted(notified_ids) == [1, 3] + # Verify commit is called to persist the notification timestamps + mock_db_session.commit.assert_called_once() + @pytest.mark.anyio @patch( "lcfs.scripts.tasks.fuel_code_expiry.settings.feature_fuel_code_expiry_email", diff --git a/backend/lcfs/web/api/fuel_code/repo.py b/backend/lcfs/web/api/fuel_code/repo.py index 3a045376f..c44713971 100644 --- a/backend/lcfs/web/api/fuel_code/repo.py +++ b/backend/lcfs/web/api/fuel_code/repo.py @@ -1209,8 +1209,9 @@ async def get_expiring_fuel_codes(self) -> List[FuelCode]: """ query = ( select(FuelCode) + .join(FuelCode.fuel_code_status) .options( - joinedload(FuelCode.fuel_code_status), + contains_eager(FuelCode.fuel_code_status), joinedload(FuelCode.fuel_code_prefix), ) .where(