Merge pull request #3688 from bcgov/hotfix/alex-fuel-code-expiry-251230

Hotfix: Fuel code expiry - 3682

Author: AlexZorkin

backend/lcfs/db/migrations/versions/2025-12-30-09-00_5897848af3c9.py

@@ -0,0 +1,34 @@
+"""Add expiry_notification_sent_at column to fuel_code table
+
+Revision ID: 5897848af3c9
+Revises: 7ca5289dff90
+Create Date: 2025-12-30 09:00:00.000000
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "5897848af3c9"
+down_revision = "7ca5289dff90"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    # Add column to track when expiry notification was sent
+    # This prevents duplicate notifications from being sent on subsequent scheduler runs
+    op.add_column(
+        "fuel_code",
+        sa.Column(
+            "expiry_notification_sent_at",
+            sa.DateTime(timezone=True),
+            nullable=True,
+            comment="Timestamp when expiry notification email was sent. Used to prevent duplicate notifications.",
+        ),
+    )
+
+
+def downgrade() -> None:
+    op.drop_column("fuel_code", "expiry_notification_sent_at")

backend/lcfs/db/models/fuel/FuelCode.py

@@ -15,6 +15,13 @@
 
 
 class FuelCode(BaseModel, Auditable, EffectiveDates, Versioning):
+    """
+    Contains a list of all fuel codes.
+
+    The expiry_notification_sent_at field tracks when an expiry notification
+    email was sent for this fuel code. This prevents duplicate notifications
+    from being sent on subsequent scheduler runs.
+    """
     __tablename__ = "fuel_code"
     __table_args__ = {"comment": "Contains a list of all of fuel codes"}
 
@@ -87,6 +94,11 @@ class FuelCode(BaseModel, Auditable, EffectiveDates, Versioning):
     )
     former_company = Column(String(500), nullable=True, comment="Former company")
     notes = Column(String(1000), nullable=True, comment="Notes")
+    expiry_notification_sent_at = Column(
+        DateTime(timezone=True),
+        nullable=True,
+        comment="Timestamp when expiry notification email was sent. Used to prevent duplicate notifications.",
+    )
 
     # Define the relationships
     fuel_code_status = relationship(

backend/lcfs/scripts/tasks/fuel_code_expiry.py

@@ -63,6 +63,8 @@ async def notify_expiring_fuel_code(db_session: AsyncSession):
         # Send emails to each contact
         success_count = 0
         total_emails = len(email_groups)
+        # Track fuel codes that were successfully notified
+        notified_fuel_code_ids: list[int] = []
 
         base_context = {
             "subject": "Important Notice from the Deputy Director: Upcoming Expiry of BC LCFS Fuel Codes",
@@ -93,6 +95,9 @@ async def notify_expiring_fuel_code(db_session: AsyncSession):
                     )
                     if sent:
                         success_count += 1
+                        # Collect fuel code IDs that were successfully notified
+                        for code in comp_data["codes"]:
+                            notified_fuel_code_ids.append(code.fuel_code_id)
                         logger.info(
                             f"Successfully sent to {contact_email} | {company_name}"
                         )
@@ -105,6 +110,14 @@ async def notify_expiring_fuel_code(db_session: AsyncSession):
                     logger.error(
                         f"Error sending to {contact_email} | {company_name}: {e}"
                     )
+
+        # Mark successfully notified fuel codes to prevent duplicate notifications
+        if notified_fuel_code_ids:
+            logger.info(
+                f"Marking {len(notified_fuel_code_ids)} fuel codes as notified"
+            )
+            await fuel_code_repo.mark_fuel_codes_notified(notified_fuel_code_ids)
+
         logger.info(
             f"Sent fuel code expiry notifications to {success_count}/{total_emails} contacts"
         )

backend/lcfs/scripts/tests/test_fuel_code_expiry.py

@@ -13,7 +13,7 @@
 
 # Mock fuel code data structure
 MockFuelCode = namedtuple(
-    "MockFuelCode", ["fuel_code", "contact_email", "expiry_date", "company"]
+    "MockFuelCode", ["fuel_code_id", "fuel_code", "contact_email", "expiry_date", "company"]
 )
 
 
@@ -28,18 +28,21 @@ def mock_fuel_codes():
     """Mock fuel code data"""
     return [
         MockFuelCode(
+            fuel_code_id=1,
             fuel_code="FC001",
             contact_email="user1@example.com",
             expiry_date=date.today() + timedelta(days=30),
             company="Company A",
         ),
         MockFuelCode(
+            fuel_code_id=2,
             fuel_code="FC002",
             contact_email="user2@example.com",
             expiry_date=date.today() + timedelta(days=60),
             company="Company B",
         ),
         MockFuelCode(
+            fuel_code_id=3,
             fuel_code="FC003",
             contact_email="user1@example.com",  # Same email as FC001
             expiry_date=date.today() + timedelta(days=45),
@@ -53,18 +56,21 @@ def mock_fuel_codes_invalid_emails():
     """Mock fuel code data with invalid emails"""
     return [
         MockFuelCode(
+            fuel_code_id=1,
             fuel_code="FC001",
             contact_email="invalid-email",
             expiry_date=date.today() + timedelta(days=30),
             company="Company A",
         ),
         MockFuelCode(
+            fuel_code_id=2,
             fuel_code="FC002",
             contact_email="user@valid.com",
             expiry_date=date.today() + timedelta(days=60),
             company="Company B",
         ),
         MockFuelCode(
+            fuel_code_id=3,
             fuel_code="FC003",
             contact_email="",
             expiry_date=date.today() + timedelta(days=45),
@@ -96,6 +102,7 @@ async def test_notify_expiring_fuel_code_success(
             # Setup mocks
             mock_repo = AsyncMock()
             mock_repo.get_expiring_fuel_codes.return_value = mock_fuel_codes
+            mock_repo.mark_fuel_codes_notified = AsyncMock()
             mock_repo_class.return_value = mock_repo
 
             mock_email_repo = AsyncMock()
@@ -117,6 +124,12 @@ async def test_notify_expiring_fuel_code_success(
                 mock_email_service.send_fuel_code_expiry_notifications.call_count == 2
             )
 
+            # Verify that fuel codes are marked as notified after successful email sending
+            mock_repo.mark_fuel_codes_notified.assert_called_once()
+            # All 3 fuel code IDs should be marked (1, 2, 3)
+            notified_ids = mock_repo.mark_fuel_codes_notified.call_args[0][0]
+            assert sorted(notified_ids) == [1, 2, 3]
+
     @pytest.mark.anyio
     @patch(
         "lcfs.scripts.tasks.fuel_code_expiry.settings.feature_fuel_code_expiry_email",
@@ -194,6 +207,7 @@ async def test_notify_expiring_fuel_code_email_service_failure(
 
             mock_repo = AsyncMock()
             mock_repo.get_expiring_fuel_codes.return_value = mock_fuel_codes
+            mock_repo.mark_fuel_codes_notified = AsyncMock()
             mock_repo_class.return_value = mock_repo
 
             mock_email_repo = AsyncMock()
@@ -206,6 +220,8 @@ async def test_notify_expiring_fuel_code_email_service_failure(
             result = await notify_expiring_fuel_code(mock_db_session)
 
             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()
 
     @pytest.mark.anyio
     async def test_notify_expiring_fuel_code_partial_success(
@@ -222,13 +238,14 @@ async def test_notify_expiring_fuel_code_partial_success(
 
             mock_repo = AsyncMock()
             mock_repo.get_expiring_fuel_codes.return_value = mock_fuel_codes
+            mock_repo.mark_fuel_codes_notified = AsyncMock()
             mock_repo_class.return_value = mock_repo
 
             mock_email_repo = AsyncMock()
             mock_email_repo_class.return_value = mock_email_repo
 
             mock_email_service = AsyncMock()
-            # First call succeeds, second fails
+            # First call succeeds (Company A with codes 1,3), second fails (Company B with code 2)
             mock_email_service.send_fuel_code_expiry_notifications.side_effect = [
                 True,
                 False,
@@ -239,6 +256,12 @@ async def test_notify_expiring_fuel_code_partial_success(
 
             assert result is True  # Should return True if at least one email succeeded
 
+            # Only fuel codes from successful email should be marked as notified
+            mock_repo.mark_fuel_codes_notified.assert_called_once()
+            notified_ids = mock_repo.mark_fuel_codes_notified.call_args[0][0]
+            # Only codes 1 and 3 (Company A) should be marked since that email succeeded
+            assert sorted(notified_ids) == [1, 3]
+
     @pytest.mark.anyio
     @patch(
         "lcfs.scripts.tasks.fuel_code_expiry.settings.feature_fuel_code_expiry_email",
@@ -304,10 +327,10 @@ def test_group_codes_invalid_emails_fallback(self, mock_fuel_codes_invalid_email
         # Should have 2 email addresses (valid one + fallback)
         assert len(result) == 2
         assert "user@valid.com" in result
-        assert "tfrs@gov.bc.ca" in result  # fallback email
+        assert "lcfs@gov.bc.ca" in result  # fallback email
 
         # Fallback should have 2 companies (invalid-email and empty email cases)
-        fallback_data = result["tfrs@gov.bc.ca"]
+        fallback_data = result["lcfs@gov.bc.ca"]
         assert len(fallback_data["companies"]) == 2
         assert "Company A" in fallback_data["companies"]
         assert "Company C" in fallback_data["companies"]
@@ -373,6 +396,7 @@ async def test_notify_expiring_fuel_code_invalid_emails(
             mock_repo.get_expiring_fuel_codes.return_value = (
                 mock_fuel_codes_invalid_emails
             )
+            mock_repo.mark_fuel_codes_notified = AsyncMock()
             mock_repo_class.return_value = mock_repo
 
             mock_email_repo = AsyncMock()
@@ -385,11 +409,16 @@ async def test_notify_expiring_fuel_code_invalid_emails(
             result = await notify_expiring_fuel_code(mock_db_session)
 
             assert result is True
-            # Should have 3 email groups: valid email + tfrs@gov.bc.ca for 2 invalid companies
+            # Should have 3 email groups: valid email + lcfs@gov.bc.ca for 2 invalid companies
             assert (
                 mock_email_service.send_fuel_code_expiry_notifications.call_count == 3
             )
 
+            # Verify all fuel codes are marked as notified
+            mock_repo.mark_fuel_codes_notified.assert_called_once()
+            notified_ids = mock_repo.mark_fuel_codes_notified.call_args[0][0]
+            assert sorted(notified_ids) == [1, 2, 3]
+
     def test_edge_cases(self):
         """Test edge cases for email validation"""
         edge_cases = [
@@ -435,9 +464,9 @@ async def test_end_to_end_workflow(self, mock_db_session):
         # Create test data
         test_codes = [
             MockFuelCode(
-                "FC001", "valid@example.com", date.today() + timedelta(days=30), "Company A"
+                1, "FC001", "valid@example.com", date.today() + timedelta(days=30), "Company A"
             ),
-            MockFuelCode("FC002", "invalid-email", date.today() + timedelta(days=60), "Company B"),
+            MockFuelCode(2, "FC002", "invalid-email", date.today() + timedelta(days=60), "Company B"),
         ]
 
         with patch(
@@ -451,6 +480,7 @@ async def test_end_to_end_workflow(self, mock_db_session):
             # Setup mocks
             mock_repo = AsyncMock()
             mock_repo.get_expiring_fuel_codes.return_value = test_codes
+            mock_repo.mark_fuel_codes_notified = AsyncMock()
             mock_repo_class.return_value = mock_repo
 
             mock_email_repo = AsyncMock()
@@ -488,6 +518,11 @@ async def test_end_to_end_workflow(self, mock_db_session):
                 assert "contact_email" in context
                 assert "expiry_count" in context
 
+            # Verify fuel codes are marked as notified
+            mock_repo.mark_fuel_codes_notified.assert_called_once()
+            notified_ids = mock_repo.mark_fuel_codes_notified.call_args[0][0]
+            assert sorted(notified_ids) == [1, 2]
+
     @pytest.mark.anyio
     @patch(
         "lcfs.scripts.tasks.fuel_code_expiry.settings.feature_fuel_code_expiry_email",
@@ -507,6 +542,7 @@ async def test_logging_behavior(self, mock_db_session, mock_fuel_codes):
 
             mock_repo = AsyncMock()
             mock_repo.get_expiring_fuel_codes.return_value = mock_fuel_codes
+            mock_repo.mark_fuel_codes_notified = AsyncMock()
             mock_repo_class.return_value = mock_repo
 
             mock_email_repo = AsyncMock()
@@ -520,5 +556,5 @@ async def test_logging_behavior(self, mock_db_session, mock_fuel_codes):
 
             # Verify that info logging occurred
             assert (
-                mock_logger.info.call_count >= 3
-            )  # At least start, found codes, and completion messages
+                mock_logger.info.call_count >= 4
+            )  # At least start, found codes, marking notified, and completion messages

backend/lcfs/tests/fuel_code/test_fuel_code_repo.py

@@ -887,3 +887,21 @@ async def test_get_fuel_code_history(fuel_code_repo, mock_db):
 
     result = await fuel_code_repo.get_fuel_code_history(1, version=0)
     assert result == history
+
+
+@pytest.mark.anyio
+async def test_mark_fuel_codes_notified(fuel_code_repo, mock_db):
+    """Test marking fuel codes as notified"""
+    fuel_code_ids = [1, 2, 3]
+
+    await fuel_code_repo.mark_fuel_codes_notified(fuel_code_ids)
+
+    mock_db.execute.assert_called_once()
+
+
+@pytest.mark.anyio
+async def test_mark_fuel_codes_notified_empty_list(fuel_code_repo, mock_db):
+    """Test marking fuel codes as notified with empty list does nothing"""
+    await fuel_code_repo.mark_fuel_codes_notified([])
+
+    mock_db.execute.assert_not_called()