remove the FF_BOUNCE_RATE_BACKEND feature flag (#1978)

cds-snc · Sep 13, 2023 · 3fb5cfe · 3fb5cfe
1 parent fef7bd5
commit 3fb5cfe
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 136 deletions.
diff --git a/app/celery/process_ses_receipts_tasks.py b/app/celery/process_ses_receipts_tasks.py
@@ -79,13 +79,11 @@ def process_ses_results(self, response):
 
         statsd_client.incr("callback.ses.{}".format(notification_status))
 
-        # Record bounces and notifications in Redis
-        if current_app.config["FF_BOUNCE_RATE_BACKEND"]:
-            if notification_status == NOTIFICATION_PERMANENT_FAILURE:
-                bounce_rate_client.set_sliding_hard_bounce(notification.service_id, str(notification.id))
-                current_app.logger.info(
-                    f"Setting total hard bounce notifications for service {notification.service.id} with notification {notification.id} in REDIS"
-                )
+        if notification_status == NOTIFICATION_PERMANENT_FAILURE:
+            bounce_rate_client.set_sliding_hard_bounce(notification.service_id, str(notification.id))
+            current_app.logger.info(
+                f"Setting total hard bounce notifications for service {notification.service.id} with notification {notification.id} in REDIS"
+            )
 
         if notification.sent_at:
             statsd_client.timing_with_dates("callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at)

diff --git a/app/config.py b/app/config.py
@@ -578,7 +578,6 @@ class Config(object):
     FF_SALESFORCE_CONTACT = env.bool("FF_SALESFORCE_CONTACT", False)
 
     # Feature flags for bounce rate
-    FF_BOUNCE_RATE_BACKEND = env.bool("FF_BOUNCE_RATE_BACKEND", False)
     # Timestamp in epoch milliseconds to seed the bounce rate. We will seed data for (24, the below config) included.
     FF_BOUNCE_RATE_SEED_EPOCH_MS = os.getenv("FF_BOUNCE_RATE_SEED_EPOCH_MS", False)
 
@@ -680,7 +679,6 @@ class Test(Development):
     API_HOST_NAME = "http://localhost:6011"
 
     TEMPLATE_PREVIEW_API_HOST = "http://localhost:9999"
-    FF_BOUNCE_RATE_BACKEND = True
     FF_EMAIL_DAILY_LIMIT = False
     CRM_GITHUB_PERSONAL_ACCESS_TOKEN = "test-token"
     CRM_ORG_LIST_URL = "https://test-url.com"

diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py
@@ -168,9 +168,6 @@ def check_for_malware_errors(document_download_response_code, notification):
 
 
 def check_service_over_bounce_rate(service_id: str):
-    if not current_app.config["FF_BOUNCE_RATE_BACKEND"]:
-        return
-
     bounce_rate = bounce_rate_client.get_bounce_rate(service_id)
     bounce_rate_status = bounce_rate_client.check_bounce_rate_status(service_id)
     debug_data = bounce_rate_client.get_debug_data(service_id)
@@ -283,10 +280,9 @@ def send_email_to_provider(notification: Notification):
                 reply_to_address=validate_and_format_email_address(email_reply_to) if email_reply_to else None,
                 attachments=attachments,
             )
-            if current_app.config["FF_BOUNCE_RATE_BACKEND"]:
-                check_service_over_bounce_rate(service.id)
-                bounce_rate_client.set_sliding_notifications(service.id, str(notification.id))
-                current_app.logger.info(f"Setting total notifications for service {service.id} in REDIS")
+            check_service_over_bounce_rate(service.id)
+            bounce_rate_client.set_sliding_notifications(service.id, str(notification.id))
+            current_app.logger.info(f"Setting total notifications for service {service.id} in REDIS")
             current_app.logger.info(f"Notification id {notification.id} HAS BEEN SENT")
             notification.reference = reference
             update_notification_to_sending(notification, provider)

diff --git a/app/models.py b/app/models.py
@@ -1751,76 +1751,40 @@ def subject(self):
 
     @property
     def formatted_status(self):
-        if current_app.config["FF_BOUNCE_RATE_BACKEND"]:
-
-            def _getStatusByBounceSubtype():
-                """Return the status of a notification based on the bounce sub type"""
-                if self.feedback_subtype:
-                    return {
-                        "suppressed": "Blocked",
-                        "on-account-suppression-list": "Blocked",
-                    }.get(self.feedback_subtype, "No such address")
-                else:
-                    return "No such address"
-
-            return {
-                "email": {
-                    "failed": "Failed",
-                    "technical-failure": "Tech issue",
-                    "temporary-failure": "Content or inbox issue",
-                    "permanent-failure": _getStatusByBounceSubtype(),
-                    "virus-scan-failed": "Attachment has virus",
-                    "delivered": "Delivered",
-                    "sending": "In transit",
-                    "created": "In transit",
-                    "sent": "Delivered",
-                    "pending": "In transit",
-                    "pending-virus-check": "In transit",
-                    "pii-check-failed": "Exceeds Protected A",
-                },
-                "sms": {
-                    "failed": "Failed",
-                    "technical-failure": "Tech issue",
-                    "temporary-failure": "Carrier issue",
-                    "permanent-failure": "No such number",
-                    "delivered": "Delivered",
-                    "sending": "In transit",
-                    "created": "In transit",
-                    "pending": "In transit",
-                    "sent": "Sent",
-                },
-                "letter": {
-                    "technical-failure": "Technical failure",
-                    "sending": "Accepted",
-                    "created": "Accepted",
-                    "delivered": "Received",
-                    "returned-letter": "Returned",
-                },
-            }[self.template.template_type].get(self.status, self.status)
-
-        # -----------------
-        # remove this code when FF_BOUNCE_RATE_BACKEND is removed
-        # -----------------
+        def _getStatusByBounceSubtype():
+            """Return the status of a notification based on the bounce sub type"""
+            if self.feedback_subtype:
+                return {
+                    "suppressed": "Blocked",
+                    "on-account-suppression-list": "Blocked",
+                }.get(self.feedback_subtype, "No such address")
+            else:
+                return "No such address"
+
         return {
             "email": {
                 "failed": "Failed",
-                "technical-failure": "Technical failure",
-                "temporary-failure": "Inbox not accepting messages right now",
-                "permanent-failure": "Email address doesn’t exist",
-                "virus-scan-failed": "Attached file may contain malware",
+                "technical-failure": "Tech issue",
+                "temporary-failure": "Content or inbox issue",
+                "permanent-failure": _getStatusByBounceSubtype(),
+                "virus-scan-failed": "Attachment has virus",
                 "delivered": "Delivered",
-                "sending": "Sending",
-                "created": "Sending",
+                "sending": "In transit",
+                "created": "In transit",
                 "sent": "Delivered",
+                "pending": "In transit",
+                "pending-virus-check": "In transit",
+                "pii-check-failed": "Exceeds Protected A",
             },
             "sms": {
                 "failed": "Failed",
-                "technical-failure": "Technical failure",
-                "temporary-failure": "Phone not accepting messages right now",
-                "permanent-failure": "Phone number doesn’t exist",
+                "technical-failure": "Tech issue",
+                "temporary-failure": "Carrier issue",
+                "permanent-failure": "No such number",
                 "delivered": "Delivered",
-                "sending": "Sending",
-                "created": "Sending",
+                "sending": "In transit",
+                "created": "In transit",
+                "pending": "In transit",
                 "sent": "Sent",
             },
             "letter": {
@@ -1831,9 +1795,6 @@ def _getStatusByBounceSubtype():
                 "returned-letter": "Returned",
             },
         }[self.template.template_type].get(self.status, self.status)
-        # -----------------
-        # end remove
-        # -----------------
 
     def get_letter_status(self):
         """

diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py
@@ -169,8 +169,7 @@ def post_bulk():
 
     max_rows = current_app.config["CSV_MAX_ROWS"]
     epoch_seeding_bounce = current_app.config["FF_BOUNCE_RATE_SEED_EPOCH_MS"]
-    bounce_rate_v1 = current_app.config["FF_BOUNCE_RATE_BACKEND"]
-    if bounce_rate_v1 and epoch_seeding_bounce:
+    if epoch_seeding_bounce:
         _seed_bounce_data(epoch_seeding_bounce, str(authenticated_service.id))
 
     form = validate(request_json, post_bulk_request(max_rows))
@@ -271,9 +270,8 @@ def post_notification(notification_type: NotificationType):
     else:
         abort(404)
 
-    bounce_rate_v1 = current_app.config["FF_BOUNCE_RATE_BACKEND"]
     epoch_seeding_bounce = current_app.config["FF_BOUNCE_RATE_SEED_EPOCH_MS"]
-    if bounce_rate_v1 and epoch_seeding_bounce:
+    if epoch_seeding_bounce:
         _seed_bounce_data(epoch_seeding_bounce, str(authenticated_service.id))
 
     check_service_has_permission(notification_type, authenticated_service.permissions)

diff --git a/tests/app/conftest.py b/tests/app/conftest.py
@@ -1444,7 +1444,6 @@ def app_statsd(mocker):
 
 @pytest.fixture(scope="function")
 def app_bounce_rate_client(mocker):
-    current_app.config["FF_BOUNCE_RATE_BACKEND"] = True
     current_app.bounce_rate_client = mocker.Mock()
     return current_app
 

diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py
@@ -1184,16 +1184,15 @@ def json():
 
 class TestBounceRate:
     def test_send_email_should_use_service_reply_to_email(self, sample_service, sample_email_template, mocker, notify_api):
-        with set_config_values(notify_api, {"FF_BOUNCE_RATE_BACKEND": True}):
-            mocker.patch("app.aws_ses_client.send_email", return_value="reference")
-            mocker.patch("app.bounce_rate_client.set_sliding_notifications")
-            db_notification = save_notification(create_notification(template=sample_email_template, reply_to_text="[email protected]"))
-            create_reply_to_email(service=sample_service, email_address="[email protected]")
-
-            send_to_providers.send_email_to_provider(
-                db_notification,
-            )
-            app.bounce_rate_client.set_sliding_notifications.assert_called_once_with(sample_service.id, str(db_notification.id))
+        mocker.patch("app.aws_ses_client.send_email", return_value="reference")
+        mocker.patch("app.bounce_rate_client.set_sliding_notifications")
+        db_notification = save_notification(create_notification(template=sample_email_template, reply_to_text="[email protected]"))
+        create_reply_to_email(service=sample_service, email_address="[email protected]")
+
+        send_to_providers.send_email_to_provider(
+            db_notification,
+        )
+        app.bounce_rate_client.set_sliding_notifications.assert_called_once_with(sample_service.id, str(db_notification.id))
 
     def test_check_service_over_bounce_rate_critical(self, mocker: MockFixture, notify_api, fake_uuid):
         with notify_api.app_context():

diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py
@@ -1578,11 +1578,7 @@ def test_get_all_notifications_for_service_formatted_for_csv(client, sample_temp
     assert not resp["notifications"][0]["row_number"]
     assert resp["notifications"][0]["template_name"] == sample_template.name
     assert resp["notifications"][0]["template_type"] == notification.notification_type
-
-    if current_app.config["FF_BOUNCE_RATE_BACKEND"]:
-        assert resp["notifications"][0]["status"] == "In transit"
-    else:
-        assert resp["notifications"][0]["status"] == "Sending"
+    assert resp["notifications"][0]["status"] == "In transit"
 
 
 def test_get_notification_for_service_without_uuid(client, notify_db, notify_db_session):

diff --git a/tests/app/test_model.py b/tests/app/test_model.py
@@ -1,5 +1,4 @@
 import pytest
-from flask import current_app
 from freezegun import freeze_time
 from sqlalchemy.exc import IntegrityError
 
@@ -126,33 +125,6 @@ def test_notification_for_csv_returns_correct_job_row_number(sample_job):
     assert serialized["row_number"] == 1
 
 
-# This test needs to be removed when FF_BOUNCE_RATE_BACKEND is removed
-@freeze_time("2016-01-30 12:39:58.321312")
-@pytest.mark.parametrize(
-    "template_type, status, expected_status",
-    [
-        ("email", "failed", "Failed"),
-        ("email", "technical-failure", "Technical failure"),
-        ("email", "temporary-failure", "Inbox not accepting messages right now"),
-        ("email", "permanent-failure", "Email address doesn’t exist"),
-        ("sms", "temporary-failure", "Phone not accepting messages right now"),
-        ("sms", "permanent-failure", "Phone number doesn’t exist"),
-        ("sms", "sent", "Sent"),
-        ("letter", "created", "Accepted"),
-        ("letter", "sending", "Accepted"),
-        ("letter", "technical-failure", "Technical failure"),
-        ("letter", "delivered", "Received"),
-    ],
-)
-def test_notification_for_csv_returns_formatted_status(sample_service, template_type, status, expected_status):
-    if not current_app.config["FF_BOUNCE_RATE_BACKEND"]:
-        template = create_template(sample_service, template_type=template_type)
-        notification = save_notification(create_notification(template, status=status))
-
-        serialized = notification.serialize_for_csv()
-        assert serialized["status"] == expected_status
-
-
 @freeze_time("2016-01-30 12:39:58.321312")
 @pytest.mark.parametrize(
     "template_type, status, feedback_subtype, expected_status",
@@ -175,14 +147,13 @@ def test_notification_for_csv_returns_formatted_status(sample_service, template_
 def test_notification_for_csv_returns_formatted_status_ff_bouncerate(
     sample_service, template_type, status, feedback_subtype, expected_status
 ):
-    if current_app.config["FF_BOUNCE_RATE_BACKEND"]:
-        template = create_template(sample_service, template_type=template_type)
-        notification = save_notification(create_notification(template, status=status))
-        if feedback_subtype:
-            notification.feedback_subtype = feedback_subtype
-
-        serialized = notification.serialize_for_csv()
-        assert serialized["status"] == expected_status
+    template = create_template(sample_service, template_type=template_type)
+    notification = save_notification(create_notification(template, status=status))
+    if feedback_subtype:
+        notification.feedback_subtype = feedback_subtype
+
+    serialized = notification.serialize_for_csv()
+    assert serialized["status"] == expected_status
 
 
 @freeze_time("2017-03-26 23:01:53.321312")