Merge pull request #791 from NHSDigital/DTOSS-remove-email-functionality

Remove email functionality for reporting

Author: dnimmo

manage_breast_screening/config/.env.tpl

@@ -31,11 +31,6 @@ BASIC_AUTH_PASSWORD=changeme
 # Notifications specific env vars
 NOTIFICATIONS_BATCH_RETRY_LIMIT=5
 
-[email protected]
-NOTIFICATIONS_SMTP_PASSWORD=changeme
-[email protected],[email protected]
-NOTIFICATIONS_SMTP_IS_ENABLED=False
-
 API_OAUTH_API_KEY=""
 API_OAUTH_API_KID=""
 API_OAUTH_PRIVATE_KEY=""

manage_breast_screening/notifications/management/commands/create_reports.py

@@ -11,7 +11,6 @@
 from manage_breast_screening.notifications.models import ZONE_INFO
 from manage_breast_screening.notifications.queries.helper import Helper
 from manage_breast_screening.notifications.services.blob_storage import BlobStorage
-from manage_breast_screening.notifications.services.nhs_mail import NhsMail
 
 logger = getLogger(__name__)
 INSIGHTS_JOB_NAME = "CreateReports"
@@ -24,12 +23,10 @@ def __init__(
         self,
         name: str,
         params: list,
-        send_email: bool = False,
         query: str | None = None,
     ):
         self.name = name
         self.params = params
-        self.send_email = send_email
         self.query = query or name
 
 
@@ -44,12 +41,12 @@ class Command(BaseCommand):
     BSO_CODES = ["MBD"]
     REPORTS = [
         ReportConfig(
-            "invites-not-sent", [datetime.now(tz=ZONE_INFO).date()], True, "failures"
+            "invites-not-sent", [datetime.now(tz=ZONE_INFO).date()], "failures"
         ),
-        ReportConfig("reconciliation", ["3 months"], True),
+        ReportConfig("reconciliation", ["3 months"]),
     ]
     SMOKE_TEST_BSO_CODE = "SM0K3"
-    SMOKE_TEST_CONFIG = ReportConfig("reconciliation", ["1 week"], False)
+    SMOKE_TEST_CONFIG = ReportConfig("reconciliation", ["1 week"])
 
     def add_arguments(self, parser):
         parser.add_argument("--smoke-test", action="store_true")
@@ -59,7 +56,6 @@ def handle(self, *args, **options):
             logger.info("Create Report Command started")
 
             bso_codes, report_configs = self.configuration(options)
-            external_reports = {}
 
             for bso_code in bso_codes:
                 for report_config in report_configs:
@@ -74,15 +70,8 @@ def handle(self, *args, **options):
 
                     BlobStorage().add(filename, csv, content_type="text/csv")
 
-                    if report_config.send_email:
-                        external_reports[filename] = csv
-
                     logger.info("Report %s created", report_config.name)
 
-            if bool(external_reports):
-                NhsMail().send_reports_email(external_reports)
-                logger.info(f"Reports sent: {', '.join(external_reports.keys())}")
-
     def configuration(self, options: dict) -> tuple[list[str], list[ReportConfig]]:
         if options.get("smoke_test", False):
             bso_codes = [self.SMOKE_TEST_BSO_CODE]

manage_breast_screening/notifications/services/nhs_mail.py

@@ -8,7 +8,6 @@
 
 from manage_breast_screening.notifications.management.commands.create_reports import (
     Command,
-    ReportConfig,
 )
 from manage_breast_screening.notifications.queries.helper import Helper
 
@@ -38,11 +37,7 @@ def mocked_dependencies(self, dataframe, csv_data, now):
                 with patch(f"{Command.__module__}.datetime") as mock_datetime:
                     mock_datetime.today.return_value = now
 
-                    with patch(f"{Command.__module__}.NhsMail") as mock_email:
-                        mock_email_service = MagicMock()
-                        mock_email.return_value = mock_email_service
-
-                        yield (mock_read_sql, mock_blob_storage, mock_email_service)
+                    yield (mock_read_sql, mock_blob_storage)
 
     @pytest.fixture
     def dataframe(self, csv_data):
@@ -54,11 +49,10 @@ def test_handle_creates_all_reports(self, dataframe, csv_data, now):
         with self.mocked_dependencies(dataframe, csv_data, now) as md:
             Command().handle()
 
-        mock_read_sql, mock_blob_storage, mock_email_service = md
+        mock_read_sql, mock_blob_storage = md
 
         assert mock_read_sql.call_count == 2
         assert mock_blob_storage.add.call_count == 2
-        assert mock_email_service.send_reports_email.call_count == 1
 
         for bso_code in Command.BSO_CODES:
             mock_read_sql.assert_any_call(
@@ -82,13 +76,6 @@ def test_handle_creates_all_reports(self, dataframe, csv_data, now):
                 content_type="text/csv",
             )
 
-            mock_email_service.send_reports_email.assert_called_once_with(
-                {
-                    failures_filename: csv_data,
-                    reconciliation_filename: csv_data,
-                }
-            )
-
     def test_handle_raises_command_error(self, mock_insights_logger):
         with patch(f"{Helper.__module__}.Helper") as mock_query:
             mock_query.sql.side_effect = Exception("err")
@@ -127,7 +114,7 @@ def test_smoke_test_argument_uses_correct_configuration(
         with self.mocked_dependencies(dataframe, csv_data, now) as md:
             Command().handle(**{"smoke_test": True})
 
-        mock_read_sql, mock_blob_storage, mock_email_service = md
+        mock_read_sql, mock_blob_storage = md
 
         mock_read_sql.assert_called_once_with(
             Helper.sql("reconciliation"), connection, params=["1 week", "SM0K3"]
@@ -137,71 +124,3 @@ def test_smoke_test_argument_uses_correct_configuration(
             csv_data,
             content_type="text/csv",
         )
-        mock_email_service.assert_not_called()
-
-    def test_handle_does_not_email_reports_with_send_email_false(
-        self, dataframe, csv_data, now, monkeypatch
-    ):
-        """
-        Test that internal reports are not emailed but still stored.
-        """
-        test_reports = [
-            ReportConfig("external-report", [now.date()], True),
-            ReportConfig("internal-report", [now.date()], False),
-        ]
-        monkeypatch.setattr(Command, "REPORTS", test_reports)
-
-        with patch(f"{Helper.__module__}.Helper.sql") as mock_helper_sql:
-            mock_helper_sql.return_value = "SELECT 1"
-
-            with self.mocked_dependencies(dataframe, csv_data, now) as md:
-                Command().handle()
-
-            mock_read_sql, mock_blob_storage, mock_email_service = md
-
-            assert mock_read_sql.call_count == 2
-            assert mock_blob_storage.add.call_count == 2
-            assert mock_email_service.send_reports_email.call_count == 1
-
-            internal_filename = (
-                f"{now.strftime('%Y-%m-%dT%H:%M:%S')}-MBD-internal-report-report.csv"
-            )
-            external_filename = (
-                f"{now.strftime('%Y-%m-%dT%H:%M:%S')}-MBD-external-report-report.csv"
-            )
-            mock_blob_storage.add.assert_any_call(
-                internal_filename,
-                csv_data,
-                content_type="text/csv",
-            )
-            mock_blob_storage.add.assert_any_call(
-                external_filename,
-                csv_data,
-                content_type="text/csv",
-            )
-
-            mock_email_service.send_reports_email.assert_called_once_with(
-                {external_filename: csv_data}
-            )
-
-    def test_handle_with_internal_reports_does_not_call_email_service(
-        self, dataframe, csv_data, now, monkeypatch
-    ):
-        """
-        Test that internal reports are not emailed but still stored.
-        """
-        monkeypatch.setattr(
-            Command, "REPORTS", [ReportConfig("internal-report", [1], False)]
-        )
-
-        with patch(f"{Helper.__module__}.Helper.sql") as mock_helper_sql:
-            mock_helper_sql.return_value = "SELECT 1"
-
-            with self.mocked_dependencies(dataframe, csv_data, now) as md:
-                Command().handle()
-
-            mock_read_sql, mock_blob_storage, mock_email_service = md
-
-            assert mock_read_sql.call_count == 1
-            assert mock_blob_storage.add.call_count == 1
-            assert mock_email_service.send_reports_email.call_count == 0