Skip to content

Commit

Permalink
Merge pull request #1455 from GSA/notify-api-1454
Browse files Browse the repository at this point in the history
change create notification to only insert once
  • Loading branch information
ccostino authored Dec 3, 2024
2 parents fe033b0 + 9b1eb70 commit cf6fe22
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 11 deletions.
27 changes: 17 additions & 10 deletions app/celery/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
from app.dao.templates_dao import dao_get_template_by_id
from app.enums import JobStatus, KeyType, NotificationType
from app.errors import TotalRequestsError
from app.notifications.process_notifications import persist_notification
from app.notifications.process_notifications import (
get_notification,
persist_notification,
)
from app.notifications.validators import check_service_over_total_message_limit
from app.serialised_models import SerialisedService, SerialisedTemplate
from app.service.utils import service_allowed_to_send_to
Expand Down Expand Up @@ -271,7 +274,7 @@ def save_email(
"Email {} failed as restricted service".format(notification_id)
)
return

original_notification = get_notification(notification_id)
try:
saved_notification = persist_notification(
template_id=notification["template"],
Expand All @@ -288,10 +291,11 @@ def save_email(
notification_id=notification_id,
reply_to_text=reply_to_text,
)

provider_tasks.deliver_email.apply_async(
[str(saved_notification.id)], queue=QueueNames.SEND_EMAIL
)
# we only want to send once
if original_notification is None:
provider_tasks.deliver_email.apply_async(
[str(saved_notification.id)], queue=QueueNames.SEND_EMAIL
)

current_app.logger.debug(
"Email {} created at {}".format(
Expand Down Expand Up @@ -329,6 +333,8 @@ def save_api_email_or_sms(self, encrypted_notification):
if notification["notification_type"] == NotificationType.EMAIL
else provider_tasks.deliver_sms
)

original_notification = get_notification(notification["id"])
try:
persist_notification(
notification_id=notification["id"],
Expand All @@ -347,10 +353,11 @@ def save_api_email_or_sms(self, encrypted_notification):
document_download_count=notification["document_download_count"],
)
# Only get here if save to the db was successful (i.e. first time)
provider_task.apply_async([notification["id"]], queue=q)
current_app.logger.debug(
f"{notification['notification_type']} {notification['id']} has been persisted and sent to delivery queue."
)
if original_notification is None:
provider_task.apply_async([notification["id"]], queue=q)
current_app.logger.debug(
f"{notification['id']} has been persisted and sent to delivery queue."
)

except IntegrityError:
current_app.logger.warning(
Expand Down
7 changes: 6 additions & 1 deletion app/dao/notifications_dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ def dao_create_notification(notification):
# notify-api-742 remove phone numbers from db
notification.to = "1"
notification.normalised_to = "1"
db.session.add(notification)

# notify-api-1454 insert only if it doesn't exist
stmt = select(Notification).where(Notification.id == notification.id)
result = db.session.execute(stmt).scalar()
if result is None:
db.session.add(notification)


def country_records_delivery(phone_prefix):
Expand Down
5 changes: 5 additions & 0 deletions app/notifications/process_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from app.dao.notifications_dao import (
dao_create_notification,
dao_delete_notifications_by_id,
get_notification_by_id,
)
from app.enums import KeyType, NotificationStatus, NotificationType
from app.errors import BadRequestError
Expand Down Expand Up @@ -53,6 +54,10 @@ def check_placeholders(template_object):
raise BadRequestError(fields=[{"template": message}], message=message)


def get_notification(notification_id):
return get_notification_by_id(notification_id)


def persist_notification(
*,
template_id,
Expand Down

0 comments on commit cf6fe22

Please sign in to comment.