merge from main

.ds.baseline

@@ -305,7 +305,7 @@
         "filename": "tests/app/service/test_rest.py",
         "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
         "is_verified": false,
-        "line_number": 1274,
+        "line_number": 1275,
         "is_secret": false
       }
     ],
@@ -384,5 +384,5 @@
       }
     ]
   },
-  "generated_at": "2024-07-22T21:27:35Z"
+  "generated_at": "2024-08-01T17:38:39Z"
 }

app/aws/s3.py

@@ -95,6 +95,32 @@ def get_s3_file(bucket_name, file_location, access_key, secret_key, region):
     return s3_file.get()["Body"].read().decode("utf-8")
 
 
+def download_from_s3(
+    bucket_name, s3_key, local_filename, access_key, secret_key, region
+):
+    session = Session(
+        aws_access_key_id=access_key,
+        aws_secret_access_key=secret_key,
+        region_name=region,
+    )
+    s3 = session.client("s3", config=AWS_CLIENT_CONFIG)
+    result = None
+    try:
+        result = s3.download_file(bucket_name, s3_key, local_filename)
+        print(f"File downloaded successfully to {local_filename}")
+    except botocore.exceptions.NoCredentialsError as nce:
+        print("Credentials not found")
+        raise Exception(nce)
+    except botocore.exceptions.PartialCredentialsError as pce:
+        print("Incomplete credentials provided")
+        raise Exception(pce)
+    except Exception as e:
+        print(f"An error occurred {e}")
+        text = f"EXCEPTION {e} local_filename {local_filename}"
+        raise Exception(text)
+    return result
+
+
 def get_s3_object(bucket_name, file_location, access_key, secret_key, region):
     session = Session(
         aws_access_key_id=access_key,

app/commands.py

@@ -592,14 +592,18 @@ def process_row_from_job(job_id, job_row_number):
 
 
 @notify_command(name="download-csv-file-by-name")
-@click.option("-f", "--csv_filename", required=True, help="csv file name")
+@click.option("-f", "--csv_filename", required=True, help="S3 file location")
 def download_csv_file_by_name(csv_filename):
-
+    # poetry run flask command download-csv-file-by-name -f <s3 file location>
+    # cf run-task notify-api-production --command "flask command download-csv-file-by-name -f <s3 location>"
     bucket_name = current_app.config["CSV_UPLOAD_BUCKET"]["bucket"]
     access_key = current_app.config["CSV_UPLOAD_BUCKET"]["access_key_id"]
     secret = current_app.config["CSV_UPLOAD_BUCKET"]["secret_access_key"]
     region = current_app.config["CSV_UPLOAD_BUCKET"]["region"]
-    print(s3.get_s3_file(bucket_name, csv_filename, access_key, secret, region))
+
+    s3.download_from_s3(
+        bucket_name, csv_filename, "download.csv", access_key, secret, region
+    )
 
 
 @notify_command(name="dump-sms-senders")
@@ -607,7 +611,7 @@ def download_csv_file_by_name(csv_filename):
 def dump_sms_senders(service_name):
 
     # poetry run flask command dump-sms-senders MyServiceName
-    # cf run-task notify-api-production --command "flask command dump-sms-senders MyServiceName"
+    # cf run-task notify-api-production --command "flask command dump-sms-senders <MyServiceName>"
     services = get_services_by_partial_name(service_name)
     if len(services) > 1:
         raise ValueError(

docs/all.md

@@ -1368,7 +1368,7 @@ cf run-task notify-api-production --command "flask command download-csv-file-by-
 locally, just do:
 
 ```
-poetry run flask command download-csv-file-by-name -f <file location in admin logs>
+poetry run flask command download-csv-file-by-name <file location in admin logs>
 ```
 
 ### Debug steps

notifications_utils/request_helper.py

@@ -84,6 +84,8 @@ class ResponseHeaderMiddleware(object):
                 current_app.logger.error(be)
             elif "AttributeError" in str(be):  # notify-api-1394
                 current_app.logger.error(be)
+            elif "MethodNotAllowed" in str(be):  # notify-admin-1392
+                current_app.logger.error(be)
             else:
                 raise be
 

poetry.lock

@@ -204,17 +204,17 @@ tests-no-zope = ["attrs[tests-mypy]", "cloudpickle", "hypothesis", "pympler", "p
 
 [[package]]
 name = "awscli"
-version = "1.33.32"
+version = "1.33.36"
 description = "Universal Command Line Environment for AWS."
 optional = false
 python-versions = ">=3.8"
 files = [
-    {file = "awscli-1.33.32-py3-none-any.whl", hash = "sha256:94e0e348b4e98d949d01f3c9bba817ef9c0da4aabea53a5d966445618339c7dd"},
+    {file = "awscli-1.33.36-py3-none-any.whl", hash = "sha256:b4d79946aff15f83a653d41573610c55a2f2311f7387e409074dea1d2f432037"},
-    {file = "awscli-1.33.32.tar.gz", hash = "sha256:90137622dcd7695939d64f57172901376d8ca5eb995c12e5fce50f55316c5b62"},
+    {file = "awscli-1.33.36.tar.gz", hash = "sha256:52de9dd8fb3600d8d99db7ce400d058413ce69634a476582af8e2301936f0af7"},
 ]
 
 [package.dependencies]
-botocore = "1.34.150"
+botocore = "1.34.154"
 colorama = ">=0.2.5,<0.4.7"
 docutils = ">=0.10,<0.17"
 PyYAML = ">=3.10,<6.1"
@@ -422,13 +422,13 @@ crt = ["botocore[crt] (>=1.21.0,<2.0a0)"]
 
 [[package]]
 name = "botocore"
-version = "1.34.150"
+version = "1.34.154"
 description = "Low-level, data-driven core of boto 3."
 optional = false
 python-versions = ">=3.8"
 files = [
-    {file = "botocore-1.34.150-py3-none-any.whl", hash = "sha256:b988d47f4d502df85befce11a48002421e4e6ea4289997b5e0261bac5fa76ce6"},
+    {file = "botocore-1.34.154-py3-none-any.whl", hash = "sha256:4eef4b1bb809b382ba9dc9c88f5fcc4a133f221a1acb693ee6bee4de9f325979"},
-    {file = "botocore-1.34.150.tar.gz", hash = "sha256:4d23387e0f076d87b637a2a35c0ff2b8daca16eace36b63ce27f65630c6b375a"},
+    {file = "botocore-1.34.154.tar.gz", hash = "sha256:64d9b4c85a504d77cb56dabb2ad717cd8e1717424a88edb458b01d1e5797262a"},
 ]
 
 [package.dependencies]
@@ -2818,13 +2818,13 @@ ptyprocess = ">=0.5"
 
 [[package]]
 name = "phonenumbers"
-version = "8.13.40"
+version = "8.13.42"
 description = "Python version of Google's common library for parsing, formatting, storing and validating international phone numbers."
 optional = false
 python-versions = "*"
 files = [
-    {file = "phonenumbers-8.13.40-py2.py3-none-any.whl", hash = "sha256:9582752c20a1da5ec4449f7f97542bf8a793c8e2fec0ab57f767177bb8fc0b1d"},
+    {file = "phonenumbers-8.13.42-py2.py3-none-any.whl", hash = "sha256:18acc22ee03116d27b26e990f53806a1770a3e05f05e1620bc09ad187f889456"},
-    {file = "phonenumbers-8.13.40.tar.gz", hash = "sha256:f137c2848b8e83dd064b71881b65680584417efa202177fd330e2f7ff6c68113"},
+    {file = "phonenumbers-8.13.42.tar.gz", hash = "sha256:7137904f2db3b991701e853174ce8e1cb8f540b8bfdf27617540de04c0b7bed5"},
 ]
 
 [[package]]
@@ -3626,17 +3626,17 @@ full = ["numpy"]
 
 [[package]]
 name = "redis"
-version = "5.0.7"
+version = "5.0.8"
 description = "Python client for Redis database and key-value store"
 optional = false
 python-versions = ">=3.7"
 files = [
-    {file = "redis-5.0.7-py3-none-any.whl", hash = "sha256:0e479e24da960c690be5d9b96d21f7b918a98c0cf49af3b6fafaa0753f93a0db"},
+    {file = "redis-5.0.8-py3-none-any.whl", hash = "sha256:56134ee08ea909106090934adc36f65c9bcbbaecea5b21ba704ba6fb561f8eb4"},
-    {file = "redis-5.0.7.tar.gz", hash = "sha256:8f611490b93c8109b50adc317b31bfd84fff31def3475b92e7e80bf39f48175b"},
+    {file = "redis-5.0.8.tar.gz", hash = "sha256:0c5b10d387568dfe0698c6fad6615750c24170e548ca2deac10c649d463e9870"},
 ]
 
 [package.extras]
-hiredis = ["hiredis (>=1.0.0)"]
+hiredis = ["hiredis (>1.0.0)"]
 ocsp = ["cryptography (>=36.0.1)", "pyopenssl (==20.0.1)", "requests (>=2.26.0)"]
 
 [[package]]
@@ -4739,4 +4739,4 @@ multidict = ">=4.0"
 [metadata]
 lock-version = "2.0"
 python-versions = "^3.12.2"
-content-hash = "4ef9a352a137fe091b1084b06959b9b674c30635771abc23010d2ba560f6bfd6"
+content-hash = "89b99841ebd4bd735104048c8f8f0a35bfedb700a7a5648ecabe713816e25579"

pyproject.toml

@@ -12,7 +12,7 @@ alembic = "==1.13.2"
 amqp = "==5.2.0"
 beautifulsoup4 = "==4.12.3"
 boto3 = "^1.34.150"
-botocore = "^1.34.150"
+botocore = "^1.34.154"
 cachetools = "==5.4.0"
 celery = {version = "==5.4.0", extras = ["redis"]}
 certifi = ">=2022.12.7"
@@ -55,7 +55,7 @@ geojson = "^3.1.0"
 govuk-bank-holidays = "^0.14"
 numpy = "^1.26.4"
 ordered-set = "^4.1.0"
-phonenumbers = "^8.13.40"
+phonenumbers = "^8.13.42"
 python-json-logger = "^2.0.7"
 pytz = "^2024.1"
 regex = "^2024.7.24"
@@ -76,7 +76,7 @@ urllib3 = "^2.2.2"
 webencodings = "^0.5.1"
 itsdangerous = "^2.2.0"
 jinja2 = "^3.1.4"
-redis = "^5.0.7"
+redis = "^5.0.8"
 requests = "^2.32.3"
 
 

tests/app/service/test_rest.py

@@ -1770,9 +1770,6 @@ def test_get_all_notifications_for_service_in_order_with_post_request(
     assert response.status_code == 200
 
 
-@pytest.mark.skip(
-    reason="We can't search on recipient if recipient is not kept in the db"
-)
 def test_get_all_notifications_for_service_filters_notifications_when_using_post_request(
     client, notify_db_session
 ):
@@ -1808,7 +1805,6 @@ def test_get_all_notifications_for_service_filters_notifications_when_using_post
         "page": 1,
         "template_type": [TemplateType.SMS],
         "status": [NotificationStatus.CREATED, NotificationStatus.SENDING],
-        "to": "0855",
     }
 
     response = client.post(
@@ -1818,7 +1814,7 @@ def test_get_all_notifications_for_service_filters_notifications_when_using_post
     )
 
     resp = json.loads(response.get_data(as_text=True))
-    assert len(resp["notifications"]) == 1
+    assert len(resp["notifications"]) == 2
     assert resp["notifications"][0]["to"] == "1"
     assert resp["notifications"][0]["status"] == returned_notification.status
     assert response.status_code == 200

tests/app/test_commands.py

@@ -8,6 +8,7 @@ from app.commands import (
     bulk_invite_user_to_service,
     create_new_service,
     create_test_user,
+    download_csv_file_by_name,
     fix_billable_units,
     insert_inbound_numbers_from_file,
     populate_annual_billing_with_defaults,
@@ -427,8 +428,22 @@ def test_promote_user_to_platform_admin(
     assert user.platform_admin is True
 
 
+def test_download_csv_file_by_name(notify_api, mocker):
+    mock_download = mocker.patch("app.commands.s3.download_from_s3")
+    notify_api.test_cli_runner().invoke(
+        download_csv_file_by_name,
+        [
+            "-f",
+            "NonExistentName",
+        ],
+    )
+    mock_download.assert_called_once()
+
+
 def test_promote_user_to_platform_admin_no_result_found(
-    notify_db_session, notify_api, sample_user
+    notify_db_session,
+    notify_api,
+    sample_user,
 ):
     assert sample_user.platform_admin is False