Removed logic to persist workbook upon download from dissemination UI (…

…#4217)

backend/audit/views/pre_dissemination_download_view.py

@@ -1,3 +1,4 @@
+from django.http import HttpResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.views.generic import View
 
@@ -33,7 +34,11 @@ def get(self, request, report_id):
         )
         i2d_data = intake_to_dissem.load_all()
 
-        filename = generate_presubmission_report(i2d_data)
-        download_url = get_download_url(filename)
+        filename, workbook_bytes = generate_presubmission_report(i2d_data)
+        response = HttpResponse(
+            workbook_bytes,
+            content_type="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+        )
+        response["Content-Disposition"] = f"attachment; filename={filename}"
 
-        return redirect(download_url)
+        return response

backend/dissemination/summary_reports.py

@@ -5,11 +5,7 @@
 import time
 import openpyxl as pyxl
 
-from boto3 import client as boto3_client
-from botocore.client import ClientError, Config
-
 from django.conf import settings
-from fs.memoryfs import MemoryFS
 
 from openpyxl.workbook.defined_name import DefinedName
 from openpyxl.utils import quote_sheetname
@@ -606,40 +602,19 @@ def create_workbook(data, protect_sheets=False):
     return (workbook, t1 - t0)
 
 
-def persist_workbook(workbook):
+def prepare_workbook_for_download(workbook):
+
     t0 = time.time()
-    s3_client = boto3_client(
-        service_name="s3",
-        region_name=settings.AWS_S3_PRIVATE_REGION_NAME,
-        aws_access_key_id=settings.AWS_PRIVATE_ACCESS_KEY_ID,
-        aws_secret_access_key=settings.AWS_PRIVATE_SECRET_ACCESS_KEY,
-        endpoint_url=settings.AWS_S3_PRIVATE_INTERNAL_ENDPOINT,
-        config=Config(signature_version="s3v4"),
-    )
+    now = datetime.utcnow().strftime("%Y%m%d%H%M%S")
+    filename = f"fac-summary-report-{now}.xlsx"
+
+    # Save the workbook directly to a BytesIO object
+    workbook_bytes = io.BytesIO()
+    workbook.save(workbook_bytes)
+    workbook_bytes.seek(0)
 
-    with MemoryFS() as mem_fs:
-        now = datetime.utcnow().strftime("%Y%m%d%H%M%S")
-        filename = f"fac-summary-report-{now}.xlsx"
-        s3_dir = "temp"
-
-        workbook_write_fp = mem_fs.openbin(filename, mode="w")
-        workbook.save(workbook_write_fp)
-        workbook_read_fp = mem_fs.openbin(filename, mode="r")
-        workbook_read_fp.seek(0)
-        content = workbook_read_fp.read()
-        workbook_bytes = io.BytesIO(content)
-
-        try:
-            s3_client.put_object(
-                Body=workbook_bytes,
-                Bucket=settings.AWS_PRIVATE_STORAGE_BUCKET_NAME,
-                Key=f"{s3_dir}/{filename}",
-            )
-        except ClientError:
-            logger.warn(f"Unable to put summary report file {filename} in S3!")
-            raise
     t1 = time.time()
-    return (f"temp/{filename}", t1 - t0)
+    return (filename, workbook_bytes, t1 - t0)
 
 
 def generate_summary_report(report_ids, include_private=False):
@@ -651,12 +626,12 @@ def generate_summary_report(report_ids, include_private=False):
     data = separate_notes_single_fields_from_array_fields(data)
     (workbook, tcw) = create_workbook(data)
     insert_dissem_coversheet(workbook, bool(tribal_report_ids), include_private)
-    (filename, tpw) = persist_workbook(workbook)
+    (filename, workbook_bytes, tpw) = prepare_workbook_for_download(workbook)
     t1 = time.time()
     logger.info(
         f"SUMMARY_REPORTS generate_summary_report\n\ttotal: {t1-t0} ttri: {ttri} tgrdd: {tgrdd} tcw: {tcw} tpw: {tpw}"
     )
-    return filename
+    return (filename, workbook_bytes)
 
 
 # Ignore performance profiling for the presub.
@@ -667,9 +642,9 @@ def generate_presubmission_report(i2d_data):
     insert_precert_coversheet(workbook)
     workbook.security.workbookPassword = str(uuid.uuid4())
     workbook.security.lockStructure = True
-    (filename, _) = persist_workbook(workbook)
+    (filename, workbook_bytes, _) = prepare_workbook_for_download(workbook)
 
-    return filename
+    return (filename, workbook_bytes)
 
 
 def separate_notes_single_fields_from_array_fields(data):

backend/dissemination/test_summary_reports.py

@@ -25,7 +25,7 @@ def test_generate_summary_report_returns_filename(self):
             baker.make(FederalAward, report_id=g)
         self.refresh_materialized_view()
 
-        filename = generate_summary_report(report_ids)
+        filename, _ = generate_summary_report(report_ids)
 
         self.assertTrue(filename.startswith, "fac-summary-report-")
         self.assertTrue(filename.endswith, ".xlsx")

backend/dissemination/test_views.py

@@ -1,3 +1,4 @@
+import io
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.test import Client, TestCase
@@ -672,8 +673,8 @@ def _mock_filename(self):
     def _mock_download_url(self):
         return "http://example.com/gsa-fac-private-s3/temp/some-report-name.xlsx"
 
-    @patch("dissemination.summary_reports.persist_workbook")
-    def test_bad_search_returns_400(self, mock_persist_workbook):
+    @patch("dissemination.summary_reports.prepare_workbook_for_download")
+    def test_bad_search_returns_400(self, mock_prepare_workbook_for_download):
         """
         Submitting a form with bad parameters should throw a BadRequest.
         """
@@ -682,8 +683,8 @@ def test_bad_search_returns_400(self, mock_persist_workbook):
         )
         self.assertEquals(response.status_code, 400)
 
-    @patch("dissemination.summary_reports.persist_workbook")
-    def test_empty_results_returns_404(self, mock_persist_workbook):
+    @patch("dissemination.summary_reports.prepare_workbook_for_download")
+    def test_empty_results_returns_404(self, mock_prepare_workbook_for_download):
         """
         Searches with no results should return a 404, not an empty excel file.
         """
@@ -695,89 +696,162 @@ def test_empty_results_returns_404(self, mock_persist_workbook):
         )
         self.assertEquals(response.status_code, 404)
 
-    @patch("dissemination.views.get_download_url")
-    @patch("dissemination.summary_reports.persist_workbook")
-    def test_no_permissions_returns_404_on_private(
-        self, mock_persist_workbook, mock_get_download_url
+    @patch("dissemination.summary_reports.prepare_workbook_for_download")
+    def test_authorized_user_with_private_data(
+        self, mock_prepare_workbook_for_download
     ):
-        """
-        Non-permissioned users can access private audits through the summary report post.
-        """
-        mock_persist_workbook.return_value = self._mock_filename()
-        mock_get_download_url.return_value = self._mock_download_url()
+        """Test that an authorized user can access private data."""
+        mock_filename = "mocked-report.xlsx"
+        mock_workbook_bytes = io.BytesIO(b"fake file content")
+        mock_prepare_workbook_for_download.return_value = (
+            mock_filename,
+            mock_workbook_bytes,
+            1.0,
+        )
 
         general = self._make_general(is_public=False)
         baker.make(FederalAward, report_id=general)
+        baker.make(FindingText, report_id=general)
         self.refresh_materialized_view()
-        response = self.anon_client.post(self._summary_report_url(), {})
-        mock_persist_workbook.assert_called_once()
-        self.assertRedirects(
-            response,
-            self._mock_download_url(),
-            status_code=302,
-            target_status_code=200,
-            fetch_redirect_response=False,
-        )
-
-    @patch("dissemination.views.get_download_url")
-    @patch("dissemination.summary_reports.persist_workbook")
-    def test_permissions_returns_file_on_private(
-        self, mock_persist_workbook, mock_get_download_url
+
+        response = self.perm_client.post(self._summary_report_url(), {})
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response["Content-Type"],
+            "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+        )
+        self.assertIn(
+            f"attachment; filename={mock_filename}", response["Content-Disposition"]
+        )
+        self.assertEqual(response.content, b"fake file content")
+
+    @patch("dissemination.summary_reports.prepare_workbook_for_download")
+    def test_unauthorized_user_with_private_data(
+        self, mock_prepare_workbook_for_download
     ):
-        """
-        Permissioned users receive a file if there are private results.
-        """
-        mock_persist_workbook.return_value = self._mock_filename()
-        mock_get_download_url.return_value = self._mock_download_url()
+        """Test that an unauthorized user can still receive a file, but without private data."""
+        mock_filename = "mocked-report.xlsx"
+        mock_workbook_bytes = io.BytesIO(b"fake file content")
+        mock_prepare_workbook_for_download.return_value = (
+            mock_filename,
+            mock_workbook_bytes,
+            1.0,
+        )
 
         general = self._make_general(is_public=False)
         baker.make(FederalAward, report_id=general)
+        baker.make(FindingText, report_id=general)
         self.refresh_materialized_view()
+
+        response = self.anon_client.post(self._summary_report_url(), {})
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response["Content-Type"],
+            "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+        )
+        self.assertIn(
+            f"attachment; filename={mock_filename}", response["Content-Disposition"]
+        )
+        self.assertEqual(response.content, b"fake file content")
+
+    @patch("dissemination.summary_reports.prepare_workbook_for_download")
+    def test_authorized_user_with_public_data(self, mock_prepare_workbook_for_download):
+        """Test that an authorized user can access public data."""
+        mock_filename = "mocked-report.xlsx"
+        mock_workbook_bytes = io.BytesIO(b"fake file content")
+        mock_prepare_workbook_for_download.return_value = (
+            mock_filename,
+            mock_workbook_bytes,
+            1.0,
+        )
+
+        general = self._make_general(is_public=True)
+        baker.make(FederalAward, report_id=general)
+        baker.make(FindingText, report_id=general)
+        self.refresh_materialized_view()
+
         response = self.perm_client.post(self._summary_report_url(), {})
-        mock_persist_workbook.assert_called_once()
-        self.assertRedirects(
-            response,
-            self._mock_download_url(),
-            status_code=302,
-            target_status_code=200,
-            fetch_redirect_response=False,
-        )
-
-    @patch("dissemination.views.get_download_url")
-    @patch("dissemination.summary_reports.persist_workbook")
-    def test_empty_search_params_returns_file(
-        self, mock_persist_workbook, mock_get_download_url
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response["Content-Type"],
+            "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+        )
+        self.assertIn(
+            f"attachment; filename={mock_filename}", response["Content-Disposition"]
+        )
+        self.assertEqual(response.content, b"fake file content")
+
+    @patch("dissemination.summary_reports.prepare_workbook_for_download")
+    def test_unauthorized_user_with_public_data(
+        self, mock_prepare_workbook_for_download
     ):
+        """Test that an unauthorized user can access public data."""
+        mock_filename = "mocked-report.xlsx"
+        mock_workbook_bytes = io.BytesIO(b"fake file content")
+        mock_prepare_workbook_for_download.return_value = (
+            mock_filename,
+            mock_workbook_bytes,
+            1.0,
+        )
+
+        general = self._make_general(is_public=True)
+        baker.make(FederalAward, report_id=general)
+        baker.make(FindingText, report_id=general)
+        self.refresh_materialized_view()
+
+        response = self.anon_client.post(self._summary_report_url(), {})
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response["Content-Type"],
+            "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+        )
+        self.assertIn(
+            f"attachment; filename={mock_filename}", response["Content-Disposition"]
+        )
+        self.assertEqual(response.content, b"fake file content")
+
+    @patch("dissemination.summary_reports.prepare_workbook_for_download")
+    def test_empty_search_params_returns_file(self, mock_prepare_workbook_for_download):
         """
         File should be generated on empty search parameters ("search all").
         """
-        mock_persist_workbook.return_value = self._mock_filename()
-        mock_get_download_url.return_value = self._mock_download_url()
-
+        mock_filename = "mocked-report.xlsx"
+        mock_workbook_bytes = io.BytesIO(b"fake file content")
+        mock_prepare_workbook_for_download.return_value = (
+            mock_filename,
+            mock_workbook_bytes,
+            1.0,
+        )
         general = self._make_general(is_public=True)
         baker.make(FederalAward, report_id=general)
         self.refresh_materialized_view()
 
         response = self.anon_client.post(self._summary_report_url(), {})
-        mock_persist_workbook.assert_called_once()
-        self.assertRedirects(
-            response,
-            self._mock_download_url(),
-            status_code=302,
-            target_status_code=200,
-            fetch_redirect_response=False,
-        )
-
-    @patch("dissemination.views.get_download_url")
-    @patch("dissemination.summary_reports.persist_workbook")
-    def test_many_results_returns_file(
-        self, mock_persist_workbook, mock_get_download_url
-    ):
+
+        mock_prepare_workbook_for_download.assert_called_once()
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            response["Content-Type"],
+            "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+        )
+        self.assertIn(
+            f"attachment; filename={mock_filename}", response["Content-Disposition"]
+        )
+        self.assertEqual(response.content, b"fake file content")
+
+    @patch("dissemination.summary_reports.prepare_workbook_for_download")
+    def test_many_results_returns_file(self, mock_prepare_workbook_for_download):
         """
         File should still be generated if there are above SUMMARY_REPORT_DOWNLOAD_LIMIT total results.
         """
-        mock_persist_workbook.return_value = self._mock_filename()
-        mock_get_download_url.return_value = self._mock_download_url()
+        mock_filename = "mocked-report.xlsx"
+        mock_workbook_bytes = io.BytesIO(b"fake file content")
+        mock_prepare_workbook_for_download.return_value = (
+            mock_filename,
+            mock_workbook_bytes,
+            1.0,
+        )
 
         for i in range(4):
             general = self._make_general(
@@ -789,11 +863,15 @@ def test_many_results_returns_file(
 
         with self.settings(SUMMARY_REPORT_DOWNLOAD_LIMIT=2):
             response = self.anon_client.post(self._summary_report_url(), {})
-            mock_persist_workbook.assert_called_once()
-            self.assertRedirects(
-                response,
-                self._mock_download_url(),
-                status_code=302,
-                target_status_code=200,
-                fetch_redirect_response=False,
+            mock_prepare_workbook_for_download.assert_called_once()
+
+            self.assertEqual(response.status_code, 200)
+            self.assertEqual(
+                response["Content-Type"],
+                "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+            )
+            self.assertIn(
+                f"attachment; filename={mock_filename}", response["Content-Disposition"]
             )
+
+            self.assertEqual(response.content, b"fake file content")