Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change create notification to only insert once #1455

Merged
merged 4 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading