Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into landline-validation-d…
Browse files Browse the repository at this point in the history
…issallow-premium
  • Loading branch information
rparke committed Aug 7, 2024
2 parents 3069ff3 + 585f3fb commit 91bfb60
Show file tree
Hide file tree
Showing 20 changed files with 803 additions and 159 deletions.
6 changes: 6 additions & 0 deletions app/celery/letters_pdf_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,12 @@ def process_sanitised_letter(self, sanitise_data):
"Processing invalid precompiled pdf with id %s (file %s)", notification_id, filename
)

# Log letters that fail with no fixed abode error so we can check for false positives
if letter_details["message"] == "no-fixed-abode-address":
current_app.logger.info(
"Precomiled PDF with id %s was invalid due to no fixed abode address", notification_id
)

_move_invalid_letter_and_update_status(
notification=notification,
filename=filename,
Expand Down
2 changes: 1 addition & 1 deletion app/celery/service_callback_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def _send_data_to_service_callback_api(self, data, service_callback_url, token,
try:
self.retry(queue=QueueNames.CALLBACKS_RETRY)
except self.MaxRetriesExceededError as e:
current_app.logger.error(
current_app.logger.warning(
"Retry: %s has retried the max num of times for callback url %s and id: %s",
function_name,
service_callback_url,
Expand Down
5 changes: 4 additions & 1 deletion app/clients/letter/dvla.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,10 @@ def _format_create_print_job_json(
}

if callback_url:
json_payload["callback"] = {"target": callback_url, "retry": {"enabled": True, "maxRetryWindow": 3600}}
json_payload["callbackParams"] = {
"target": callback_url,
"retryParams": {"enabled": True, "maxRetryWindow": 10800},
}

# `despatchMethod` should not be added for second class letters
if postage == FIRST_CLASS:
Expand Down
53 changes: 52 additions & 1 deletion app/dao/unsubscribe_request_dao.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from datetime import datetime

from sqlalchemy import desc, func

from app import db
from app.dao.dao_utils import autocommit
from app.models import UnsubscribeRequest, UnsubscribeRequestReport
from app.models import Job, Notification, NotificationHistory, Template, UnsubscribeRequest, UnsubscribeRequestReport
from app.utils import midnight_n_days_ago


Expand Down Expand Up @@ -57,6 +59,35 @@ def get_unsubscribe_request_reports_dao(service_id):
)


def get_unsubscribe_request_report_by_id_dao(batch_id):
return UnsubscribeRequestReport.query.filter_by(id=batch_id).one_or_none()


def get_unsubscribe_requests_data_for_download_dao(service_id, batch_id):
results = []
for table in [Notification, NotificationHistory]:
query = (
db.session.query(
UnsubscribeRequest.notification_id,
UnsubscribeRequest.email_address,
Template.name.label("template_name"),
table.template_id,
func.coalesce(Job.original_file_name, "N/A").label("original_file_name"),
table.sent_at.label("template_sent_at"),
)
.outerjoin(Job, table.job_id == Job.id)
.filter(
UnsubscribeRequest.service_id == service_id,
UnsubscribeRequest.unsubscribe_request_report_id == batch_id,
UnsubscribeRequest.notification_id == table.id,
table.template_id == Template.id,
)
.order_by(desc(Template.name), desc(Job.original_file_name), desc(table.sent_at))
)
results = results + query.all()
return results


def get_unbatched_unsubscribe_requests_dao(service_id):
return (
UnsubscribeRequest.query.filter_by(service_id=service_id, unsubscribe_request_report_id=None)
Expand All @@ -68,3 +99,23 @@ def get_unbatched_unsubscribe_requests_dao(service_id):
@autocommit
def create_unsubscribe_request_reports_dao(unsubscribe_request_report):
db.session.add(unsubscribe_request_report)


@autocommit
def update_unsubscribe_request_report_processed_by_date_dao(report, report_has_been_processed):
report.processed_by_service_at = datetime.utcnow() if report_has_been_processed else None
db.session.add(report)


@autocommit
def assign_unbatched_unsubscribe_requests_to_report_dao(report_id, service_id, earliest_timestamp, latest_timestamp):
"""
This method updates the unsubscribe_request_report_id of all un-batched unsubscribe requests that fall
within the earliest_timestamp and latest_timestamp values to report_id
"""
UnsubscribeRequest.query.filter(
UnsubscribeRequest.unsubscribe_request_report_id.is_(None),
UnsubscribeRequest.service_id == service_id,
UnsubscribeRequest.created_at >= earliest_timestamp,
UnsubscribeRequest.created_at <= latest_timestamp,
).update({"unsubscribe_request_report_id": report_id})
35 changes: 23 additions & 12 deletions app/delivery/send_to_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,16 @@ def send_sms_to_provider(notification):
statsd_client.timing("sms.live-key.total-time", delta_seconds)


def _get_email_headers(notification: Notification) -> list[dict[str, str]]:
headers = []

if notification.unsubscribe_link:
headers += [
{"Name": "List-Unsubscribe", "Value": f"<{notification.unsubscribe_link}>"},
def _get_email_headers(notification: Notification, template: SerialisedTemplate) -> list[dict[str, str]]:
if unsubscribe_link := notification.get_unsubscribe_link_for_headers(
template_has_unsubscribe_link=template.has_unsubscribe_link
):
return [
{"Name": "List-Unsubscribe", "Value": f"<{unsubscribe_link}>"},
{"Name": "List-Unsubscribe-Post", "Value": "List-Unsubscribe=One-Click"},
]

return headers
return []


def send_email_to_provider(notification):
Expand All @@ -121,15 +121,26 @@ def send_email_to_provider(notification):
if notification.status == "created":
provider = provider_to_use(EMAIL_TYPE)

template_dict = SerialisedTemplate.from_id_and_service_id(
template = SerialisedTemplate.from_id_and_service_id(
template_id=notification.template_id, service_id=service.id, version=notification.template_version
).__dict__
)

unsubscribe_link_for_body = notification.get_unsubscribe_link_for_body(
template_has_unsubscribe_link=template.has_unsubscribe_link
)

html_email = HTMLEmailTemplate(
template_dict, values=notification.personalisation, **get_html_email_options(service)
template.__dict__,
values=notification.personalisation,
unsubscribe_link=unsubscribe_link_for_body,
**get_html_email_options(service),
)

plain_text_email = PlainTextEmailTemplate(template_dict, values=notification.personalisation)
plain_text_email = PlainTextEmailTemplate(
template.__dict__,
values=notification.personalisation,
unsubscribe_link=unsubscribe_link_for_body,
)
created_at = notification.created_at
key_type = notification.key_type
if notification.key_type == KEY_TYPE_TEST:
Expand All @@ -149,7 +160,7 @@ def send_email_to_provider(notification):
body=str(plain_text_email),
html_body=str(html_email),
reply_to_address=notification.reply_to_text,
headers=_get_email_headers(notification),
headers=_get_email_headers(notification, template),
)
notification.reference = reference
update_notification_to_sending(notification, provider)
Expand Down
6 changes: 2 additions & 4 deletions app/functional_tests_fixtures/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
EMAIL_AUTH,
EXTRA_LETTER_FORMATTING,
INBOUND_SMS_TYPE,
LETTER_TYPE,
SECOND_CLASS,
SEND_EMAILS,
SEND_LETTERS,
Expand Down Expand Up @@ -280,6 +279,7 @@ def _create_service(org_id, user, service_name="Functional Tests"):
"sms_message_limit": 1000,
"letter_message_limit": 1000,
"email_message_limit": 1000,
"contact_link": current_app.config["ADMIN_BASE_URL"],
}
service = Service.from_json(data)
dao_create_service(service, user)
Expand Down Expand Up @@ -356,9 +356,6 @@ def _create_email_template(service, user_id):

new_template = template_schema.load(data)

if not new_template.postage and new_template.template_type == LETTER_TYPE:
new_template.postage = SECOND_CLASS

new_template.service = service

dao_create_template(new_template)
Expand Down Expand Up @@ -412,6 +409,7 @@ def _create_letter_template(service, user_id):
new_template = template_schema.load(data)

new_template.service = service
new_template.postage = SECOND_CLASS

dao_create_template(new_template)

Expand Down
52 changes: 51 additions & 1 deletion app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
DATETIME_FORMAT_NO_TIMEZONE,
get_dt_string_or_none,
get_uuid_string_or_none,
url_with_token,
)


Expand Down Expand Up @@ -1622,7 +1623,9 @@ def serialize(self):
"completed_at": self.completed_at(),
"scheduled_for": None,
"postage": self.postage,
"one_click_unsubscribe_url": self.unsubscribe_link,
"one_click_unsubscribe_url": self.get_unsubscribe_link_for_headers(
template_has_unsubscribe_link=self.template.has_unsubscribe_link
),
}

if self.notification_type == LETTER_TYPE:
Expand Down Expand Up @@ -1748,6 +1751,30 @@ def _get_letter_cost(self):

return letter_rate

def _generate_unsubscribe_link(self, base_url):
return url_with_token(
self.to,
url=f"/unsubscribe/{str(self.id)}/",
base_url=base_url,
)

def get_unsubscribe_link_for_headers(self, *, template_has_unsubscribe_link):
"""
Generates a URL on the API domain, which accepts a POST request from an email client
"""
if self.unsubscribe_link:
return self.unsubscribe_link
if template_has_unsubscribe_link:
return self._generate_unsubscribe_link(current_app.config["API_HOST_NAME"])

def get_unsubscribe_link_for_body(self, *, template_has_unsubscribe_link):
"""
Generates a URL on the admin domain, which serves a page telling the user they
have been unsubscribed
"""
if template_has_unsubscribe_link:
return self._generate_unsubscribe_link(current_app.config["ADMIN_BASE_URL"])


class NotificationHistory(db.Model):
__tablename__ = "notification_history"
Expand Down Expand Up @@ -2676,6 +2703,29 @@ class UnsubscribeRequestReport(db.Model):
processed_by_service_at = db.Column(db.DateTime, nullable=True)
count = db.Column(db.BigInteger, nullable=False)

def serialize(self):
return {
"batch_id": str(self.id),
"count": self.count,
"earliest_timestamp": self.earliest_timestamp.isoformat(),
"latest_timestamp": self.latest_timestamp.isoformat(),
"processed_by_service_at": (
self.processed_by_service_at.isoformat() if self.processed_by_service_at else None
),
"is_a_batched_report": True,
}

@staticmethod
def serialize_unbatched_requests(unbatched_unsubscribe_requests):
return {
"batch_id": None,
"count": len(unbatched_unsubscribe_requests),
"earliest_timestamp": unbatched_unsubscribe_requests[-1].created_at.isoformat(),
"latest_timestamp": unbatched_unsubscribe_requests[0].created_at.isoformat(),
"processed_by_service_at": None,
"is_a_batched_report": False,
}


class UnsubscribeRequest(db.Model):
__tablename__ = "unsubscribe_request"
Expand Down
4 changes: 3 additions & 1 deletion app/notifications/notifications_letter_callback.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,7 @@ def process_letter_callback():
notification_id = signing.decode(token)
except BadSignature:
current_app.logger.info("Letter callback with invalid token of %s received", token)
else:
current_app.logger.info("Letter callback for notification id %s received", notification_id)

current_app.logger.info("Letter callback for notification id %s received", notification_id)
return jsonify(result="success"), 200
6 changes: 0 additions & 6 deletions app/notifications/process_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
dao_delete_notifications_by_id,
)
from app.models import Notification
from app.utils import url_with_token
from app.v2.errors import BadRequestError, QrCodeTooLongError

REDIS_GET_AND_INCR_DAILY_LIMIT_DURATION_SECONDS = Histogram(
Expand Down Expand Up @@ -125,11 +124,6 @@ def persist_notification(
if not notification_id:
notification_id = uuid.uuid4()

if template_has_unsubscribe_link and not unsubscribe_link:
base_url = current_app.config["API_HOST_NAME"]
url = f"/unsubscribe/{str(notification_id)}/"
unsubscribe_link = url_with_token(recipient, url=url, base_url=base_url)

notification = Notification(
id=notification_id,
template_id=template_id,
Expand Down
16 changes: 15 additions & 1 deletion app/notifications/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
rate_limit_cache_key,
)
from notifications_utils.recipient_validation.email_address import validate_and_format_email_address
from notifications_utils.recipient_validation.errors import InvalidPhoneError
from notifications_utils.recipient_validation.phone_number import (
PhoneNumber,
get_international_phone_info,
validate_and_format_phone_number,
)
Expand All @@ -21,6 +23,7 @@
KEY_TYPE_TEAM,
KEY_TYPE_TEST,
LETTER_TYPE,
SMS_TO_UK_LANDLINES,
SMS_TYPE,
)
from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id
Expand Down Expand Up @@ -138,7 +141,18 @@ def validate_and_format_recipient(send_to, key_type, service, notification_type,
service_can_send_to_recipient(send_to, key_type, service, allow_guest_list_recipients)

if notification_type == SMS_TYPE:
international_phone_info = check_if_service_can_send_to_number(service, send_to)
if service.has_permission(SMS_TO_UK_LANDLINES):
try:
phone_number = PhoneNumber(send_to, allow_international=service.has_permission(INTERNATIONAL_SMS_TYPE))
return phone_number.get_normalised_format()
except InvalidPhoneError as e:
if e.code == InvalidPhoneError.Codes.NOT_A_UK_MOBILE:
raise BadRequestError(message="Cannot send to international mobile numbers") from e
else:
raise

else:
international_phone_info = check_if_service_can_send_to_number(service, send_to)

return validate_and_format_phone_number(number=send_to, international=international_phone_info.international)
elif notification_type == EMAIL_TYPE:
Expand Down
Loading

0 comments on commit 91bfb60

Please sign in to comment.