diff --git a/.ds.baseline b/.ds.baseline index cec28396c..859f30b4d 100644 --- a/.ds.baseline +++ b/.ds.baseline @@ -675,7 +675,7 @@ "filename": "tests/conftest.py", "hashed_secret": "f8377c90fcfd699f0ddbdcb30c2c9183d2d933ea", "is_verified": false, - "line_number": 3289, + "line_number": 3266, "is_secret": false } ], @@ -710,5 +710,5 @@ } ] }, - "generated_at": "2024-05-20T16:03:05Z" + "generated_at": "2024-05-29T21:18:03Z" } diff --git a/app/s3_client/__init__.py b/app/s3_client/__init__.py index e0933b464..7de3509d2 100644 --- a/app/s3_client/__init__.py +++ b/app/s3_client/__init__.py @@ -1,3 +1,5 @@ +import os + import botocore from boto3 import Session from botocore.config import Config @@ -29,6 +31,17 @@ def get_s3_object( ) s3 = session.resource("s3", config=AWS_CLIENT_CONFIG) obj = s3.Object(bucket_name, filename) + # This 'proves' that use of moto in the relevant tests in test_send.py + # mocks everything related to S3. What you will see in the logs is: + # Exception: CREATED AT + # + # raise Exception(f"CREATED AT {_s3.Bucket(bucket_name).creation_date}") + if os.getenv("NOTIFY_ENVIRONMENT") == "test": + teststr = str(s3.Bucket(bucket_name).creation_date).lower() + if "magicmock" not in teststr: + raise Exception( + "Test is not mocked, use @mock_aws or the relevant mocker.patch to avoid accessing S3" + ) return obj diff --git a/app/s3_client/s3_csv_client.py b/app/s3_client/s3_csv_client.py index 21c329887..752f054a4 100644 --- a/app/s3_client/s3_csv_client.py +++ b/app/s3_client/s3_csv_client.py @@ -28,6 +28,7 @@ def get_csv_upload(service_id, upload_id): def s3upload(service_id, filedata): + upload_id = str(uuid.uuid4()) bucket_name, file_location, access_key, secret_key, region = get_csv_location( service_id, upload_id diff --git a/notifications_utils/s3.py b/notifications_utils/s3.py index cdcc70a5c..d33cbe25a 100644 --- a/notifications_utils/s3.py +++ b/notifications_utils/s3.py @@ -38,6 +38,17 @@ def s3upload( region_name=region, ) _s3 = session.resource("s3", config=AWS_CLIENT_CONFIG) + # This 'proves' that use of moto in the relevant tests in test_send.py + # mocks everything related to S3. What you will see in the logs is: + # Exception: CREATED AT + # + # raise Exception(f"CREATED AT {_s3.Bucket(bucket_name).creation_date}") + if os.getenv("NOTIFY_ENVIRONMENT") == "test": + teststr = str(_s3.Bucket(bucket_name).creation_date).lower() + if "magicmock" not in teststr: + raise Exception( + "Test is not mocked, use @mock_aws or the relevant mocker.patch to avoid accessing S3" + ) key = _s3.Object(bucket_name, file_location) @@ -82,6 +93,17 @@ def s3download( ) s3 = session.resource("s3", config=AWS_CLIENT_CONFIG) key = s3.Object(bucket_name, filename) + # This 'proves' that use of moto in the relevant tests in test_send.py + # mocks everything related to S3. What you will see in the logs is: + # Exception: CREATED AT + # + # raise Exception(f"CREATED AT {_s3.Bucket(bucket_name).creation_date}") + if os.getenv("NOTIFY_ENVIRONMENT") == "test": + teststr = str(s3.Bucket(bucket_name).creation_date).lower() + if "magicmock" not in teststr: + raise Exception( + "Test is not mocked, use @mock_aws or the relevant mocker.patch to avoid accessing S3" + ) return key.get()["Body"] except botocore.exceptions.ClientError as error: raise S3ObjectNotFound(error.response, error.operation_name) diff --git a/poetry.lock b/poetry.lock index 0add4deb3..78ba0826d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1535,6 +1535,50 @@ files = [ {file = "mistune-0.8.4.tar.gz", hash = "sha256:59a3429db53c50b5c6bcc8a07f8848cb00d7dc8bdb431a4ab41920d201d4756e"}, ] +[[package]] +name = "moto" +version = "5.0.8" +description = "" +optional = false +python-versions = ">=3.8" +files = [ + {file = "moto-5.0.8-py2.py3-none-any.whl", hash = "sha256:7d1035e366434bfa9fcc0621f07d5aa724b6846408071d540137a0554c46f214"}, + {file = "moto-5.0.8.tar.gz", hash = "sha256:517fb808dc718bcbdda54c6ffeaca0adc34cf6e10821bfb01216ce420a31765c"}, +] + +[package.dependencies] +boto3 = ">=1.9.201" +botocore = ">=1.14.0" +cryptography = ">=3.3.1" +Jinja2 = ">=2.10.1" +python-dateutil = ">=2.1,<3.0.0" +requests = ">=2.5" +responses = ">=0.15.0" +werkzeug = ">=0.5,<2.2.0 || >2.2.0,<2.2.1 || >2.2.1" +xmltodict = "*" + +[package.extras] +all = ["PyYAML (>=5.1)", "antlr4-python3-runtime", "aws-xray-sdk (>=0.93,!=0.96)", "cfn-lint (>=0.40.0)", "docker (>=3.0.0)", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "jsonpath-ng", "multipart", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)", "setuptools"] +apigateway = ["PyYAML (>=5.1)", "joserfc (>=0.9.0)", "openapi-spec-validator (>=0.5.0)"] +apigatewayv2 = ["PyYAML (>=5.1)", "openapi-spec-validator (>=0.5.0)"] +appsync = ["graphql-core"] +awslambda = ["docker (>=3.0.0)"] +batch = ["docker (>=3.0.0)"] +cloudformation = ["PyYAML (>=5.1)", "aws-xray-sdk (>=0.93,!=0.96)", "cfn-lint (>=0.40.0)", "docker (>=3.0.0)", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)", "setuptools"] +cognitoidp = ["joserfc (>=0.9.0)"] +dynamodb = ["docker (>=3.0.0)", "py-partiql-parser (==0.5.5)"] +dynamodbstreams = ["docker (>=3.0.0)", "py-partiql-parser (==0.5.5)"] +glue = ["pyparsing (>=3.0.7)"] +iotdata = ["jsondiff (>=1.1.2)"] +proxy = ["PyYAML (>=5.1)", "antlr4-python3-runtime", "aws-xray-sdk (>=0.93,!=0.96)", "cfn-lint (>=0.40.0)", "docker (>=2.5.1)", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "jsonpath-ng", "multipart", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)", "setuptools"] +resourcegroupstaggingapi = ["PyYAML (>=5.1)", "cfn-lint (>=0.40.0)", "docker (>=3.0.0)", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)"] +s3 = ["PyYAML (>=5.1)", "py-partiql-parser (==0.5.5)"] +s3crc32c = ["PyYAML (>=5.1)", "crc32c", "py-partiql-parser (==0.5.5)"] +server = ["PyYAML (>=5.1)", "antlr4-python3-runtime", "aws-xray-sdk (>=0.93,!=0.96)", "cfn-lint (>=0.40.0)", "docker (>=3.0.0)", "flask (!=2.2.0,!=2.2.1)", "flask-cors", "graphql-core", "joserfc (>=0.9.0)", "jsondiff (>=1.1.2)", "jsonpath-ng", "openapi-spec-validator (>=0.5.0)", "py-partiql-parser (==0.5.5)", "pyparsing (>=3.0.7)", "setuptools"] +ssm = ["PyYAML (>=5.1)"] +stepfunctions = ["antlr4-python3-runtime", "jsonpath-ng"] +xray = ["aws-xray-sdk (>=0.93,!=0.96)", "setuptools"] + [[package]] name = "msgpack" version = "1.0.8" @@ -2598,6 +2642,25 @@ requests = ">=2.22,<3" [package.extras] fixture = ["fixtures"] +[[package]] +name = "responses" +version = "0.25.0" +description = "A utility library for mocking out the `requests` Python library." +optional = false +python-versions = ">=3.8" +files = [ + {file = "responses-0.25.0-py3-none-any.whl", hash = "sha256:2f0b9c2b6437db4b528619a77e5d565e4ec2a9532162ac1a131a83529db7be1a"}, + {file = "responses-0.25.0.tar.gz", hash = "sha256:01ae6a02b4f34e39bffceb0fc6786b67a25eae919c6368d05eabc8d9576c2a66"}, +] + +[package.dependencies] +pyyaml = "*" +requests = ">=2.30.0,<3.0" +urllib3 = ">=1.25.10,<3.0" + +[package.extras] +tests = ["coverage (>=6.0.0)", "flake8", "mypy", "pytest (>=7.0.0)", "pytest-asyncio", "pytest-cov", "pytest-httpserver", "tomli", "tomli-w", "types-PyYAML", "types-requests"] + [[package]] name = "rich" version = "13.7.1" @@ -2925,7 +2988,18 @@ files = [ {file = "xlwt-1.3.0.tar.gz", hash = "sha256:c59912717a9b28f1a3c2a98fd60741014b06b043936dcecbc113eaaada156c88"}, ] +[[package]] +name = "xmltodict" +version = "0.13.0" +description = "Makes working with XML feel like you are working with JSON" +optional = false +python-versions = ">=3.4" +files = [ + {file = "xmltodict-0.13.0-py2.py3-none-any.whl", hash = "sha256:aa89e8fd76320154a40d19a0df04a4695fb9dc5ba977cbb68ab3e4eb225e7852"}, + {file = "xmltodict-0.13.0.tar.gz", hash = "sha256:341595a488e3e01a85a9d8911d8912fd922ede5fecc4dce437eb4b6c8d037e56"}, +] + [metadata] lock-version = "2.0" python-versions = "^3.12.2" -content-hash = "dbfc0e66e41f6da3845785157b95ef70193bc66c91b53fe0afb0eac0a1ea5490" +content-hash = "ebecc7a1869605e491b4cb168a621e9e8b64a69bca74189857782c51c9fd7b9c" diff --git a/pyproject.toml b/pyproject.toml index 9a1eb25ee..9762f7140 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,6 +82,7 @@ flake8-print = "^5.0.0" flake8-pytest-style = "^1.7.2" isort = "^5.13.2" jinja2-cli = {version = "==0.8.2", extras = ["yaml"]} +moto = "*" pip-audit = "*" pre-commit = "^3.7.1" pytest = "^8.2.1" diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 449259d8d..24403e895 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -6,6 +6,7 @@ from io import BytesIO from itertools import repeat from os import path from random import randbytes +from unittest.mock import ANY from uuid import uuid4 from zipfile import BadZipFile @@ -17,7 +18,11 @@ from xlrd.xldate import XLDateAmbiguous, XLDateError, XLDateNegative, XLDateTooL from notifications_utils.recipients import RecipientCSV from notifications_utils.template import SMSPreviewTemplate -from tests import validate_route_permission, validate_route_permission_with_client +from tests import ( + sample_uuid, + validate_route_permission, + validate_route_permission_with_client, +) from tests.conftest import ( SERVICE_ONE_ID, create_active_caseworking_user, @@ -434,10 +439,15 @@ def test_upload_files_in_different_formats( service_one, mocker, mock_get_service_template, - mock_s3_set_metadata, - mock_s3_upload, fake_uuid, ): + + mock_s3_set_metadata = mocker.patch( + "app.main.views.send.set_metadata_on_csv_upload" + ) + + mock_s3_upload = mocker.patch("app.main.views.send.s3upload") + with open(filename, "rb") as uploaded: page = client_request.post( "main.send_messages", @@ -456,7 +466,7 @@ def test_upload_files_in_different_formats( "202 205 8823,Still Not Pete,Crimson,Pear" ) mock_s3_set_metadata.assert_called_once_with( - SERVICE_ONE_ID, fake_uuid, original_file_name=filename + SERVICE_ONE_ID, ANY, original_file_name=filename ) else: assert not mock_s3_upload.called @@ -470,13 +480,16 @@ def test_send_messages_sanitises_and_truncates_file_name_for_metadata( service_one, mocker, mock_get_service_template_with_placeholders, - mock_s3_set_metadata, - mock_s3_get_metadata, - mock_s3_upload, - mock_s3_download, mock_get_job_doesnt_exist, fake_uuid, ): + + mock_s3_set_metadata = mocker.patch( + "app.main.views.send.set_metadata_on_csv_upload" + ) + + mocker.patch("app.main.views.send.s3upload") + filename = f"😁{'a' * 2000}.csv" client_request.post( @@ -577,15 +590,20 @@ def test_upload_csv_file_with_errors_shows_check_page_with_errors( service_one, mocker, mock_get_service_template_with_placeholders, - mock_s3_set_metadata, - mock_s3_get_metadata, - mock_s3_upload, mock_get_users_by_service, mock_get_service_statistics, mock_get_job_doesnt_exist, mock_get_jobs, fake_uuid, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) mocker.patch( "app.main.views.send.s3download", return_value=""" @@ -624,15 +642,21 @@ def test_upload_csv_file_with_empty_message_shows_check_page_with_errors( service_one, mocker, mock_get_empty_service_template_with_optional_placeholder, - mock_s3_set_metadata, - mock_s3_get_metadata, - mock_s3_upload, mock_get_users_by_service, mock_get_service_statistics, mock_get_job_doesnt_exist, mock_get_jobs, fake_uuid, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) + mocker.patch( "app.main.views.send.s3download", return_value=""" @@ -676,15 +700,20 @@ def test_upload_csv_file_with_very_long_placeholder_shows_check_page_with_errors service_one, mocker, mock_get_service_template_with_placeholders, - mock_s3_set_metadata, - mock_s3_get_metadata, - mock_s3_upload, mock_get_users_by_service, mock_get_service_statistics, mock_get_job_doesnt_exist, mock_get_jobs, fake_uuid, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) big_placeholder = " ".join(["not ok"] * 402) mocker.patch( "app.main.views.send.s3download", @@ -811,9 +840,6 @@ def test_upload_csv_file_with_missing_columns_shows_error( client_request, mocker, mock_get_service_template_with_placeholders, - mock_s3_set_metadata, - mock_s3_get_metadata, - mock_s3_upload, mock_get_users_by_service, mock_get_service_statistics, mock_get_job_doesnt_exist, @@ -823,6 +849,15 @@ def test_upload_csv_file_with_missing_columns_shows_error( file_contents, expected_error, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) + mocker.patch("app.main.views.send.s3download", return_value=file_contents) page = client_request.post( @@ -885,10 +920,13 @@ def test_upload_csv_size_too_big( def test_upload_valid_csv_redirects_to_check_page( client_request, mock_get_service_template_with_placeholders, - mock_s3_upload, - mock_s3_set_metadata, fake_uuid, + mocker, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) client_request.post( "main.send_messages", service_id=SERVICE_ONE_ID, @@ -937,13 +975,16 @@ def test_upload_valid_csv_shows_preview_and_table( mock_get_service_statistics, mock_get_job_doesnt_exist, mock_get_jobs, - mock_s3_get_metadata, - mock_s3_set_metadata, fake_uuid, extra_args, expected_link_in_first_row, expected_message, ): + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) with client_request.session_transaction() as session: session["file_uploads"] = {fake_uuid: {"template_id": fake_uuid}} @@ -1031,8 +1072,12 @@ def test_show_all_columns_if_there_are_duplicate_recipient_columns( mock_get_job_doesnt_exist, mock_get_jobs, fake_uuid, - mock_s3_get_metadata, ): + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) with client_request.session_transaction() as session: session["file_uploads"] = {fake_uuid: {"template_id": fake_uuid}} @@ -1080,12 +1125,17 @@ def test_404_for_previewing_a_row_out_of_range( mock_get_service_statistics, mock_get_job_doesnt_exist, mock_get_jobs, - mock_s3_get_metadata, - mock_s3_set_metadata, fake_uuid, row_index, expected_status, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) with client_request.session_transaction() as session: session["file_uploads"] = {fake_uuid: {"template_id": fake_uuid}} @@ -1523,7 +1573,6 @@ def test_send_one_off_redirects_to_start_if_you_skip_steps( client_request, service_one, fake_uuid, - mock_s3_upload, mock_get_users_by_service, mock_get_service_statistics, mock_has_no_jobs, @@ -1559,7 +1608,6 @@ def test_send_one_off_redirects_to_start_if_index_out_of_bounds_and_some_placeho service_one, fake_uuid, mock_get_service_email_template, - mock_s3_download, mock_get_users_by_service, mock_get_service_statistics, mock_has_no_jobs, @@ -1628,7 +1676,6 @@ def test_send_one_off_email_to_self_without_placeholders_redirects_to_check_page mocker, service_one, mock_get_service_email_template_without_placeholders, - mock_s3_upload, mock_get_users_by_service, mock_get_service_statistics, mock_has_no_jobs, @@ -1835,13 +1882,20 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( mock_get_live_service, mock_get_job_doesnt_exist, mock_get_jobs, - mock_s3_get_metadata, - mock_s3_set_metadata, service_one, fake_uuid, - mock_s3_upload, mocker, ): + + mock_s3_set_metadata = mocker.patch( + "app.main.views.send.set_metadata_on_csv_upload" + ) + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) mocker.patch( "app.main.views.send.s3download", return_value="\n".join( @@ -1895,9 +1949,6 @@ def test_upload_csvfile_with_international_validates( api_user_active, client_request, mock_get_service_template, - mock_s3_set_metadata, - mock_s3_get_metadata, - mock_s3_upload, mock_has_permissions, mock_get_users_by_service, mock_get_service_statistics, @@ -1908,6 +1959,14 @@ def test_upload_csvfile_with_international_validates( should_allow_international, service_one, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) if international_sms_permission: service_one["permissions"] += ("sms", "international_sms") mocker.patch( @@ -1942,15 +2001,20 @@ def test_test_message_can_only_be_sent_now( mocker, service_one, mock_get_service_template, - mock_s3_download, mock_get_users_by_service, mock_get_service_statistics, mock_get_job_doesnt_exist, mock_get_jobs, - mock_s3_get_metadata, - mock_s3_set_metadata, + mock_s3_download, fake_uuid, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) content = client_request.get( "main.check_messages", service_id=service_one["id"], @@ -1972,8 +2036,12 @@ def test_preview_button_is_correctly_labelled( mock_get_job_doesnt_exist, mock_get_jobs, fake_uuid, - mock_s3_get_metadata, ): + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) mocker.patch( "app.main.views.send.s3download", return_value="\n".join(["phone_number"] + (["2028670123"] * 1000)), @@ -2058,7 +2126,6 @@ def test_route_permissions( mock_get_jobs, mock_get_notifications, mock_create_job, - mock_s3_upload, fake_uuid, route, response_code, @@ -2092,8 +2159,8 @@ def test_route_permissions_send_check_notifications( response_code, method, mock_create_job, - mock_s3_upload, ): + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) with client_request.session_transaction() as session: session["recipient"] = "2028675301" session["placeholders"] = {"name": "a"} @@ -2173,8 +2240,6 @@ def test_check_messages_back_link( mock_get_job_doesnt_exist, mock_get_jobs, mock_s3_download, - mock_s3_get_metadata, - mock_s3_set_metadata, fake_uuid, mocker, template_type, @@ -2182,6 +2247,13 @@ def test_check_messages_back_link( extra_args, expected_url, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) content = "Hi there ((name))" if has_placeholders else "Hi there" template_data = create_template( template_id=fake_uuid, template_type=template_type, content=content @@ -2236,8 +2308,12 @@ def test_check_messages_shows_too_many_messages_errors( fake_uuid, num_requested, expected_msg, - mock_s3_get_metadata, ): + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) # csv with 100 phone numbers mocker.patch( "app.main.views.send.s3download", @@ -2281,7 +2357,6 @@ def test_check_messages_shows_too_many_messages_errors( def test_check_messages_shows_trial_mode_error( client_request, - mock_s3_get_metadata, mock_get_users_by_service, mock_get_service_template, mock_has_permissions, @@ -2291,6 +2366,11 @@ def test_check_messages_shows_trial_mode_error( fake_uuid, mocker, ): + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) mocker.patch( "app.main.views.send.s3download", return_value=("phone number,\n2028675209"), # Not in team @@ -2426,8 +2506,12 @@ def test_check_messages_column_error_doesnt_show_optional_columns( mock_get_service_statistics, mock_get_job_doesnt_exist, mock_get_jobs, - mock_s3_get_metadata, ): + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) mocker.patch( "app.main.views.send.s3download", return_value="\n".join( @@ -2468,10 +2552,17 @@ def test_check_messages_adds_sender_id_in_session_to_metadata( mock_get_service_statistics, mock_get_job_doesnt_exist, mock_get_jobs, - mock_s3_get_metadata, - mock_s3_set_metadata, fake_uuid, ): + + mock_s3_set_metadata = mocker.patch( + "app.main.views.send.set_metadata_on_csv_upload" + ) + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) mocker.patch( "app.main.views.send.s3download", return_value=("phone number,\n2028675209") ) @@ -2508,11 +2599,15 @@ def test_check_messages_shows_over_max_row_error( mock_get_service_statistics, mock_get_job_doesnt_exist, mock_get_jobs, - mock_s3_get_metadata, mock_s3_download, fake_uuid, mocker, ): + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) mock_recipients = mocker.patch("app.main.views.send.RecipientCSV").return_value mock_recipients.max_rows = 11111 mock_recipients.__len__.return_value = 99999 @@ -2662,8 +2757,8 @@ def test_send_notification_submits_data( expected_personalisation, mocker, mock_create_job, - mock_s3_upload, ): + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) with client_request.session_transaction() as session: session["recipient"] = recipient session["placeholders"] = placeholders @@ -2690,8 +2785,8 @@ def test_send_notification_clears_session( mock_get_service_template, mocker, mock_create_job, - mock_s3_upload, ): + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) with client_request.session_transaction() as session: session["recipient"] = "2028675301" session["placeholders"] = {"a": "b"} @@ -2752,8 +2847,8 @@ def test_send_notification_redirects_to_view_page( extra_redirect_args, mocker, mock_create_job, - mock_s3_upload, ): + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) with client_request.session_transaction() as session: session["recipient"] = "2028675301" session["placeholders"] = {"a": "b"} @@ -2812,8 +2907,9 @@ def test_send_notification_shows_error_if_400( exception_msg, expected_h1, expected_err_details, - mock_s3_upload, ): + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) + class MockHTTPError(HTTPError): message = exception_msg @@ -2851,8 +2947,9 @@ def test_send_notification_shows_email_error_in_trial_mode( mocker, mock_get_service_email_template, mock_create_job, - mock_s3_upload, ): + mocker.patch("app.main.views.send.s3upload", return_value=sample_uuid()) + class MockHTTPError(HTTPError): message = TRIAL_MODE_MSG status_code = 400 @@ -2897,9 +2994,6 @@ def test_reply_to_is_previewed_if_chosen( client_request, mocker, mock_get_service_email_template, - mock_s3_download, - mock_s3_get_metadata, - mock_s3_set_metadata, mock_get_users_by_service, mock_get_service_statistics, mock_get_job_doesnt_exist, @@ -2910,6 +3004,8 @@ def test_reply_to_is_previewed_if_chosen( extra_args, reply_to_address, ): + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + mocker.patch( "app.main.views.send.s3download", return_value=""" @@ -2952,9 +3048,6 @@ def test_sms_sender_is_previewed( client_request, mocker, mock_get_service_template, - mock_s3_download, - mock_s3_get_metadata, - mock_s3_set_metadata, mock_get_users_by_service, mock_get_service_statistics, mock_get_job_doesnt_exist, @@ -2965,6 +3058,13 @@ def test_sms_sender_is_previewed( extra_args, sms_sender, ): + + mocker.patch("app.main.views.send.set_metadata_on_csv_upload") + + mocker.patch( + "app.main.views.send.get_csv_metadata", + return_value={"original_file_name": "example.csv"}, + ) mocker.patch( "app.main.views.send.s3download", return_value=""" diff --git a/tests/conftest.py b/tests/conftest.py index 00451bdf7..592ec9403 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1843,14 +1843,6 @@ def mock_get_users_by_service(mocker): ) -@pytest.fixture() -def mock_s3_upload(mocker): - def _upload(service_id, filedata): - return sample_uuid() - - return mocker.patch("app.main.views.send.s3upload", side_effect=_upload) - - @pytest.fixture() def mock_s3_download(mocker): def _download(service_id, upload_id): @@ -1863,21 +1855,6 @@ def mock_s3_download(mocker): return mocker.patch("app.main.views.send.s3download", side_effect=_download) -@pytest.fixture() -def mock_s3_get_metadata(mocker): - def _get_metadata(service_id, upload_id): - return {"original_file_name": "example.csv"} - - return mocker.patch( - "app.main.views.send.get_csv_metadata", side_effect=_get_metadata - ) - - -@pytest.fixture() -def mock_s3_set_metadata(mocker): - return mocker.patch("app.main.views.send.set_metadata_on_csv_upload") - - @pytest.fixture() def sample_invite(mocker, service_one): id_ = USER_ONE_ID diff --git a/tests/notifications_utils/test_s3.py b/tests/notifications_utils/test_s3.py index 46b863c4f..d05fa8fdc 100644 --- a/tests/notifications_utils/test_s3.py +++ b/tests/notifications_utils/test_s3.py @@ -2,6 +2,7 @@ from urllib.parse import parse_qs import botocore import pytest +from moto import mock_aws from notifications_utils.s3 import S3ObjectNotFound, s3download, s3upload @@ -12,6 +13,7 @@ location = "some_file_location" content_type = "binary/octet-stream" +@mock_aws def test_s3upload_save_file_to_bucket(mocker): mocked = mocker.patch("notifications_utils.s3.Session.resource") s3upload(