fix personalisation

This commit is contained in:
Kenneth Kehl
2024-01-18 10:03:35 -08:00
parent 40f8718a61
commit 567dd390b4
9 changed files with 87 additions and 6 deletions

View File

@@ -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) my_phone = re.sub(r"[\+\s\(\)\-\.]*", "", my_phone)
return 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): def get_job_metadata_from_s3(service_id, job_id):
obj = get_s3_object(*get_job_location(service_id, job_id)) obj = get_s3_object(*get_job_location(service_id, job_id))

View File

@@ -73,6 +73,14 @@ def dao_create_notification(notification):
if not notification.status: if not notification.status:
notification.status = NOTIFICATION_CREATED 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) db.session.add(notification)

View File

@@ -2,7 +2,7 @@ import dateutil
import pytz import pytz
from flask import Blueprint, current_app, jsonify, request 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.celery.tasks import process_job
from app.config import QueueNames from app.config import QueueNames
from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job 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 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 ( return (
jsonify( jsonify(
notifications=notifications, notifications=notifications,

View File

@@ -2,6 +2,7 @@ import itertools
from datetime import datetime from datetime import datetime
from flask import Blueprint, current_app, jsonify, request from flask import Blueprint, current_app, jsonify, request
from app.aws.s3 import get_personalisation_from_s3
from sqlalchemy.exc import IntegrityError from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.exc import NoResultFound
from werkzeug.datastructures import MultiDict from werkzeug.datastructures import MultiDict
@@ -425,6 +426,12 @@ def get_all_notifications_for_service(service_id):
include_one_off=include_one_off, 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 = request.args.to_dict()
kwargs["service_id"] = service_id kwargs["service_id"] = service_id

View File

@@ -1,6 +1,7 @@
from flask import current_app, jsonify, request, url_for from flask import current_app, jsonify, request, url_for
from app import api_user, authenticated_service from app import api_user, authenticated_service
from app.aws.s3 import get_personalisation_from_s3
from app.dao import notifications_dao from app.dao import notifications_dao
from app.schema_validation import validate from app.schema_validation import validate
from app.v2.notifications import v2_notification_blueprint from app.v2.notifications import v2_notification_blueprint
@@ -49,6 +50,12 @@ def get_notifications():
count_pages=False, 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): def _build_links(notifications):
_links = { _links = {
"current": url_for(".get_notifications", _external=True, **data), "current": url_for(".get_notifications", _external=True, **data),

View File

@@ -7,6 +7,7 @@ from botocore.exceptions import ClientError
from app.aws.s3 import ( from app.aws.s3 import (
file_exists, file_exists,
get_personalisation_from_s3,
get_phone_number_from_s3, get_phone_number_from_s3,
get_s3_file, get_s3_file,
remove_csv_object, remove_csv_object,
@@ -73,6 +74,35 @@ def test_get_phone_number_from_s3(
assert phone_number == expected_phone_number 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): def test_remove_csv_object(notify_api, mocker):
get_s3_mock = mocker.patch("app.aws.s3.get_s3_object") get_s3_mock = mocker.patch("app.aws.s3.get_s3_object")
remove_csv_object("mykey") remove_csv_object("mykey")

View File

@@ -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_at
assert not persisted_notification.sent_by assert not persisted_notification.sent_by
assert not persisted_notification.job_id assert not persisted_notification.job_id
assert persisted_notification.personalisation == {"name": "Jo"} assert persisted_notification.personalisation == {}
assert persisted_notification.notification_type == "sms" assert persisted_notification.notification_type == "sms"
mocked_deliver_sms.assert_called_once_with( mocked_deliver_sms.assert_called_once_with(
[str(persisted_notification.id)], queue="send-sms-tasks" [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 persisted_notification.status == "created"
assert not persisted_notification.sent_by assert not persisted_notification.sent_by
assert persisted_notification.job_row_number == 1 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.api_key_id is None
assert persisted_notification.key_type == KEY_TYPE_NORMAL assert persisted_notification.key_type == KEY_TYPE_NORMAL
assert persisted_notification.notification_type == "email" 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.status == "created"
assert persisted_notification.created_at >= now assert persisted_notification.created_at >= now
assert not persisted_notification.sent_by assert not persisted_notification.sent_by
assert persisted_notification.personalisation == {"name": "Jo"} assert persisted_notification.personalisation == {}
assert not persisted_notification.reference assert not persisted_notification.reference
assert persisted_notification.notification_type == "email" assert persisted_notification.notification_type == "email"
provider_tasks.deliver_email.apply_async.assert_called_once_with( provider_tasks.deliver_email.apply_async.assert_called_once_with(

View File

@@ -481,8 +481,6 @@ def test_send_user_email_code(
) )
assert noti.to == sample_user.email_address assert noti.to == sample_user.email_address
assert str(noti.template_id) == current_app.config["EMAIL_2FA_TEMPLATE_ID"] 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") deliver_email.assert_called_once_with([str(noti.id)], queue="notify-internal-tasks")

View File

@@ -986,6 +986,7 @@ def test_post_email_notification_with_archived_reply_to_id_returns_400(
assert "BadRequestError" in resp_json["errors"][0]["error"] 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( @pytest.mark.parametrize(
"csv_param", "csv_param",
( (
@@ -1041,6 +1042,7 @@ def test_post_notification_with_document_upload(
notification = Notification.query.one() notification = Notification.query.one()
assert notification.status == NOTIFICATION_CREATED assert notification.status == NOTIFICATION_CREATED
assert notification.personalisation == { assert notification.personalisation == {
"first_link": "abababab-link", "first_link": "abababab-link",
"second_link": "cdcdcdcd-link", "second_link": "cdcdcdcd-link",