Merge pull request #1152 from GSA/debug_wash

More debugging for issue where user cannot send all messages

app/commands.py

@@ -589,6 +589,17 @@ 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")
+def download_csv_file_by_name(csv_filename):
+
+    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))
+
+
 @notify_command(name="populate-annual-billing-with-the-previous-years-allowance")
 @click.option(
     "-y",
@@ -854,10 +865,11 @@ def promote_user_to_platform_admin(user_email_address):
 
 @notify_command(name="purge-csv-bucket")
 def purge_csv_bucket():
-    bucket_name = getenv("CSV_BUCKET_NAME")
-    access_key = getenv("CSV_AWS_ACCESS_KEY_ID")
-    secret = getenv("CSV_AWS_SECRET_ACCESS_KEY")
-    region = getenv("CSV_AWS_REGION")
+    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("ABOUT TO RUN PURGE CSV BUCKET")
     s3.purge_bucket(bucket_name, access_key, secret, region)
     print("RAN PURGE CSV BUCKET")

docs/all.md

@@ -60,6 +60,9 @@
 - [Data Storage Policies \& Procedures](#data-storage-policies--procedures)
   - [Potential PII Locations](#potential-pii-locations)
   - [Data Retention Policy](#data-retention-policy)
+- [Debug messages not being sent](#debug-messages-not-being-sent)
+  - [Getting the file location and tracing what happens](#getting-the-file-location-and-tracing-what-happens)
+  - [Viewing the csv file](#viewing-the-csv-file)
 
 
 # Infrastructure overview
@@ -1320,3 +1323,25 @@ Data Retention Policy
 Seven (7) days by default. Each service can be set with a custom policy via `ServiceDataRetention` by a Platform Admin. The `ServiceDataRetention` setting applies per-service and per-message type and controls both entries in the `notifications` table as well as `csv` contact files uploaded to s3
 
 Data cleanup is controlled by several tasks in the `nightly_tasks.py` file, kicked off by Celery Beat.
+
+
+# Debug messages not being sent
+
+
+## Getting the file location and tracing what happens
+
+
+Ask the user to provide the csv file name.  Either the csv file they uploaded, or the one that is autogenerated when they do a one-off send and is visible in the UI
+
+Starting with the admin logs, search for this file name.  When you find it, the log line should have the file name linked to the job_id and the csv file location.  Save both of these.
+
+In the api logs, search by job_id.  Either you will see evidence of the job failing and retrying over and over (in which case search for a stack trace using timestamp), or you will ultimately get to a log line that links the job_id to a message_id.  In this case, now search by message_id.  You should be able to find the actual result from AWS, either success or failure, with hopefully some helpful info.
+
+## Viewing the csv file
+
+If you need to view the questionable csv file, run the following command:
+
+
+```
+cf run-task notify-api "flask command download_csv_file_by_name -f <file location found in admin logs>"
+```

notifications_utils/recipients.py

@@ -602,6 +602,26 @@ def validate_us_phone_number(number):
         raise InvalidPhoneError(exc._msg) from exc
 
 
+def show_mangled_number_clues(number):
+
+    translator = {
+        "1": "X",
+        "2": "X",
+        "3": "X",
+        "4": "X",
+        "5": "X",
+        "6": "X",
+        "7": "X",
+        "8": "X",
+        "9": "X",
+        "0": "X",
+    }
+    for key in translator:
+        number = number.replace(key, translator[key])
+
+    return number
+
+
 def validate_phone_number(number, international=False):
     if (not international) or is_us_phone_number(number):
         return validate_us_phone_number(number)
@@ -619,7 +639,11 @@ def validate_phone_number(number, international=False):
     except NumberParseException as exc:
         if exc._msg == "Could not interpret numbers after plus-sign.":
             raise InvalidPhoneError("Not a valid country prefix") from exc
-        raise InvalidPhoneError(exc._msg) from exc
+        if not isinstance(number, str):
+            raise InvalidPhoneError(f"Number must be string, not type {type(number)}")
+        raise InvalidPhoneError(
+            f"Invalid phone number looks like {show_mangled_number_clues(number)} {exc._msg}"
+        )
 
 
 validate_and_format_phone_number = validate_phone_number

tests/app/service/send_notification/test_send_notification.py

@@ -79,7 +79,7 @@ def test_should_reject_bad_phone_numbers(notify_api, sample_template, mocker):
             assert json_resp["result"] == "error"
             assert len(json_resp["message"].keys()) == 1
             assert (
-                "Invalid phone number: The string supplied did not seem to be a phone number."
+                "Invalid phone number: Invalid phone number looks like invalid The string supplied did not seem to be a phone number."  # noqa
                 in json_resp["message"]["to"]
             )
             assert response.status_code == 400

tests/app/user/test_rest.py

@@ -226,7 +226,7 @@ def test_cannot_create_user_with_empty_strings(admin_request, notify_db_session)
     assert resp["message"] == {
         "email_address": ["Not a valid email address"],
         "mobile_number": [
-            "Invalid phone number: The string supplied did not seem to be a phone number."
+            "Invalid phone number: Invalid phone number looks like  The string supplied did not seem to be a phone number."  # noqa
         ],
         "name": ["Invalid name"],
     }
@@ -949,7 +949,7 @@ def test_cannot_update_user_with_mobile_number_as_empty_string(
         _expected_status=400,
     )
     assert resp["message"]["mobile_number"] == [
-        "Invalid phone number: The string supplied did not seem to be a phone number."
+        "Invalid phone number: Invalid phone number looks like  The string supplied did not seem to be a phone number."  # noqa
     ]
 
 

tests/notifications_utils/test_recipient_validation.py

@@ -9,6 +9,7 @@
     get_international_phone_info,
     international_phone_info,
     is_us_phone_number,
+    show_mangled_number_clues,
     try_validate_and_format_phone_number,
     validate_and_format_phone_number,
     validate_email_address,
@@ -324,6 +325,11 @@ def test_phone_number_rejects_invalid_international_values(phone_number, error_m
     assert error_message == str(e.value)
 
 
+def test_show_mangled_number_clues():
+    x = show_mangled_number_clues("848!!-202?-2020$$")
+    assert x == "XXX!!-XXX?-XXXX$$"
+
+
 @pytest.mark.parametrize("email_address", valid_email_addresses)
 def test_validate_email_address_accepts_valid(email_address):
     try: