diff --git a/app/aws/s3.py b/app/aws/s3.py index 86676f3fc..c344279b1 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -124,6 +124,29 @@ def get_phone_number_from_s3(service_id, job_id, job_row_number): my_phone = re.sub(r"[\+\s\(\)\-\.]*", "", my_phone) return my_phone +def get_personalisation_from_s3(service_id, job_id, job_row_number): + job = JOBS.get(job_id) + if job is None: + job = get_job_from_s3(service_id, job_id) + JOBS[job_id] = job + incr_jobs_cache_misses() + else: + incr_jobs_cache_hits() + + job = job.split("\r\n") + first_row = job[0] + job.pop(0) + first_row = first_row.split(",") + correct_row = job[job_row_number] + correct_row = correct_row.split(",") + personalisation_dict = {} + index = 0 + for header in first_row: + personalisation_dict[header] = correct_row[index] + index = index + 1 + print(f"get personalisation returns {personalisation_dict}") + return personalisation_dict + def get_job_metadata_from_s3(service_id, job_id): obj = get_s3_object(*get_job_location(service_id, job_id)) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index ec9ea5053..5e83b60d1 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -73,6 +73,14 @@ def dao_create_notification(notification): if not notification.status: notification.status = NOTIFICATION_CREATED + # notify-api-749 do not write to db + # if we have a verify_code we know this is the authentication notification at login time + # and not csv (containing PII) provided by the user, so allow verify_code to continue to exist + print(f"PERSONALISATION = {notification.personalisation}") + if "verify_code" in str(notification.personalisation): + pass + else: + notification.personalisation="" db.session.add(notification) diff --git a/app/job/rest.py b/app/job/rest.py index 5852d7f63..419f33451 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -2,7 +2,7 @@ import dateutil import pytz from flask import Blueprint, current_app, jsonify, request -from app.aws.s3 import get_job_metadata_from_s3 +from app.aws.s3 import get_job_metadata_from_s3, get_personalisation_from_s3 from app.celery.tasks import process_job from app.config import QueueNames from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job @@ -87,6 +87,12 @@ def get_all_notifications_for_service_job(service_id, job_id): paginated_notifications.items, many=True ) + for notification in paginated_notifications.items: + if notification.job_id is not None: + notification.personalisation = get_personalisation_from_s3( + notification.service_id, notification.job_id, notification.job_row_number + ) + return ( jsonify( notifications=notifications, diff --git a/app/service/rest.py b/app/service/rest.py index 99d2b4c97..d58e865c4 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -2,6 +2,7 @@ import itertools from datetime import datetime from flask import Blueprint, current_app, jsonify, request +from app.aws.s3 import get_personalisation_from_s3 from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound from werkzeug.datastructures import MultiDict @@ -425,6 +426,12 @@ def get_all_notifications_for_service(service_id): include_one_off=include_one_off, ) + for notification in pagination.items: + if notification.job_id is not None: + notification.personalisation = get_personalisation_from_s3( + notification.service_id, notification.job_id, notification.job_row_number + ) + kwargs = request.args.to_dict() kwargs["service_id"] = service_id diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index c12a89f68..13e7ff7bb 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -1,6 +1,7 @@ from flask import current_app, jsonify, request, url_for from app import api_user, authenticated_service +from app.aws.s3 import get_personalisation_from_s3 from app.dao import notifications_dao from app.schema_validation import validate from app.v2.notifications import v2_notification_blueprint @@ -49,6 +50,12 @@ def get_notifications(): count_pages=False, ) + for notification in paginated_notifications.items: + if notification.job_id is not None: + notification.personalisation = get_personalisation_from_s3( + notification.service_id, notification.job_id, notification.job_row_number + ) + def _build_links(notifications): _links = { "current": url_for(".get_notifications", _external=True, **data), diff --git a/tests/app/aws/test_s3.py b/tests/app/aws/test_s3.py index 342f6d7df..879661a8e 100644 --- a/tests/app/aws/test_s3.py +++ b/tests/app/aws/test_s3.py @@ -7,6 +7,7 @@ from botocore.exceptions import ClientError from app.aws.s3 import ( file_exists, + get_personalisation_from_s3, get_phone_number_from_s3, get_s3_file, remove_csv_object, @@ -73,6 +74,35 @@ def test_get_phone_number_from_s3( assert phone_number == expected_phone_number +@pytest.mark.parametrize( + "job, job_id, job_row_number, expected_personalisation", + [ + ("phone number\r\n+15555555555", "aaa", 0, {"phone number": "+15555555555"}), + ( + "day of week,favorite color,phone number\r\nmonday,green,1.555.111.1111\r\ntuesday,red,+1 (555) 222-2222", + "bbb", + 1, + {"day of week": "tuesday", "favorite color": "red", "phone number": "+1 (555) 222-2222"}, + ), + ( + "day of week,favorite color,phone number\r\nmonday,green,1.555.111.1111\r\ntuesday,red,+1 (555) 222-2222", + "ccc", + 0, + {"day of week": "monday", "favorite color": "green", "phone number": "1.555.111.1111"}, + ), + ], +) +def test_get_personalisation_from_s3( + mocker, job, job_id, job_row_number, expected_personalisation +): + mocker.patch("app.aws.s3.redis_store") + get_job_mock = mocker.patch("app.aws.s3.get_job_from_s3") + get_job_mock.return_value = job + personalisation = get_personalisation_from_s3("service_id", job_id, job_row_number) + assert personalisation == expected_personalisation + + + def test_remove_csv_object(notify_api, mocker): get_s3_mock = mocker.patch("app.aws.s3.get_s3_object") remove_csv_object("mykey") diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 80ba897a5..662df8a29 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -428,7 +428,7 @@ def test_should_send_template_to_correct_sms_task_and_persist( assert not persisted_notification.sent_at assert not persisted_notification.sent_by assert not persisted_notification.job_id - assert persisted_notification.personalisation == {"name": "Jo"} + assert persisted_notification.personalisation == {} assert persisted_notification.notification_type == "sms" mocked_deliver_sms.assert_called_once_with( [str(persisted_notification.id)], queue="send-sms-tasks" @@ -644,7 +644,7 @@ def test_should_use_email_template_and_persist( assert persisted_notification.status == "created" assert not persisted_notification.sent_by assert persisted_notification.job_row_number == 1 - assert persisted_notification.personalisation == {"name": "Jo"} + assert persisted_notification.personalisation == {} assert persisted_notification.api_key_id is None assert persisted_notification.key_type == KEY_TYPE_NORMAL assert persisted_notification.notification_type == "email" @@ -714,7 +714,7 @@ def test_should_use_email_template_subject_placeholders( assert persisted_notification.status == "created" assert persisted_notification.created_at >= now assert not persisted_notification.sent_by - assert persisted_notification.personalisation == {"name": "Jo"} + assert persisted_notification.personalisation == {} assert not persisted_notification.reference assert persisted_notification.notification_type == "email" provider_tasks.deliver_email.apply_async.assert_called_once_with( diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index bd89b9fd8..dea7d6f94 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -481,8 +481,6 @@ def test_send_user_email_code( ) assert noti.to == sample_user.email_address assert str(noti.template_id) == current_app.config["EMAIL_2FA_TEMPLATE_ID"] - assert noti.personalisation["name"] == "Test User" - assert noti.personalisation["url"].startswith(expected_auth_url) deliver_email.assert_called_once_with([str(noti.id)], queue="notify-internal-tasks") diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index cb3957450..f23e53e51 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -986,6 +986,7 @@ def test_post_email_notification_with_archived_reply_to_id_returns_400( assert "BadRequestError" in resp_json["errors"][0]["error"] +@pytest.mark.skip(reason="We've removed personalization from db, needs refactor if we want to support this") @pytest.mark.parametrize( "csv_param", ( @@ -1041,6 +1042,7 @@ def test_post_notification_with_document_upload( notification = Notification.query.one() assert notification.status == NOTIFICATION_CREATED + assert notification.personalisation == { "first_link": "abababab-link", "second_link": "cdcdcdcd-link",