mirror of
https://github.com/GSA/notifications-api.git
synced 2026-05-17 15:31:08 -04:00
Need to remove priority logic
This commit is contained in:
@@ -11,7 +11,6 @@ from app.cloudfoundry_config import cloud_config
|
||||
|
||||
class QueueNames(object):
|
||||
PERIODIC = "periodic-tasks"
|
||||
PRIORITY = "priority-tasks"
|
||||
DATABASE = "database-tasks"
|
||||
SEND_SMS = "send-sms-tasks"
|
||||
CHECK_SMS = "check-sms_tasks"
|
||||
@@ -30,7 +29,6 @@ class QueueNames(object):
|
||||
@staticmethod
|
||||
def all_queues():
|
||||
return [
|
||||
QueueNames.PRIORITY,
|
||||
QueueNames.PERIODIC,
|
||||
QueueNames.DATABASE,
|
||||
QueueNames.SEND_SMS,
|
||||
|
||||
@@ -33,20 +33,6 @@ def dao_get_provider_versions(provider_id):
|
||||
)
|
||||
|
||||
|
||||
def _adjust_provider_priority(provider, new_priority):
|
||||
current_app.logger.info(
|
||||
f"Adjusting provider priority - {provider.identifier} going from {provider.priority} to {new_priority}"
|
||||
)
|
||||
provider.priority = new_priority
|
||||
|
||||
# Automatic update so set as notify user
|
||||
provider.created_by_id = current_app.config["NOTIFY_USER_ID"]
|
||||
|
||||
# update without commit so that both rows can be changed without ending the transaction
|
||||
# and releasing the for_update lock
|
||||
_update_provider_details_without_commit(provider)
|
||||
|
||||
|
||||
def _get_sms_providers_for_update(time_threshold):
|
||||
"""
|
||||
Returns a list of providers, while holding a for_update lock on the provider details table, guaranteeing that those
|
||||
@@ -86,11 +72,7 @@ def get_provider_details_by_notification_type(
|
||||
if supports_international:
|
||||
filters.append(ProviderDetails.supports_international == supports_international)
|
||||
|
||||
return (
|
||||
ProviderDetails.query.filter(*filters)
|
||||
.order_by(asc(ProviderDetails.priority))
|
||||
.all()
|
||||
)
|
||||
return ProviderDetails.query.filter(*filters).all()
|
||||
|
||||
|
||||
@autocommit
|
||||
@@ -135,7 +117,6 @@ def dao_get_provider_stats():
|
||||
ProviderDetails.id,
|
||||
ProviderDetails.display_name,
|
||||
ProviderDetails.identifier,
|
||||
ProviderDetails.priority,
|
||||
ProviderDetails.notification_type,
|
||||
ProviderDetails.active,
|
||||
ProviderDetails.updated_at,
|
||||
@@ -149,7 +130,6 @@ def dao_get_provider_stats():
|
||||
.outerjoin(User, ProviderDetails.created_by_id == User.id)
|
||||
.order_by(
|
||||
ProviderDetails.notification_type,
|
||||
ProviderDetails.priority,
|
||||
)
|
||||
.all()
|
||||
)
|
||||
|
||||
@@ -168,11 +168,7 @@ def send_notification(notification_type):
|
||||
reply_to_text=template.reply_to_text,
|
||||
)
|
||||
if not simulated:
|
||||
queue_name = (
|
||||
QueueNames.PRIORITY
|
||||
if template.process_type == TemplateProcessType.PRIORITY
|
||||
else None
|
||||
)
|
||||
queue_name = None
|
||||
send_notification_to_queue(notification=notification_model, queue=queue_name)
|
||||
|
||||
else:
|
||||
|
||||
@@ -23,7 +23,6 @@ def get_providers():
|
||||
"id": row.id,
|
||||
"display_name": row.display_name,
|
||||
"identifier": row.identifier,
|
||||
"priority": row.priority,
|
||||
"notification_type": row.notification_type,
|
||||
"active": row.active,
|
||||
"updated_at": row.updated_at,
|
||||
|
||||
@@ -80,11 +80,7 @@ def send_one_off_notification(service_id, post_data):
|
||||
client_reference=client_reference,
|
||||
)
|
||||
|
||||
queue_name = (
|
||||
QueueNames.PRIORITY
|
||||
if template.process_type == TemplateProcessType.PRIORITY
|
||||
else None
|
||||
)
|
||||
queue_name = None
|
||||
|
||||
send_notification_to_queue(
|
||||
notification=notification,
|
||||
|
||||
@@ -176,11 +176,7 @@ def process_sms_or_email_notification(
|
||||
)
|
||||
|
||||
if not simulated:
|
||||
queue_name = (
|
||||
QueueNames.PRIORITY
|
||||
if template_process_type == TemplateProcessType.PRIORITY
|
||||
else None
|
||||
)
|
||||
queue_name = None
|
||||
send_notification_to_queue_detached(
|
||||
key_type=api_user.key_type,
|
||||
notification_type=notification_type,
|
||||
|
||||
@@ -6,7 +6,6 @@ from sqlalchemy.sql import desc
|
||||
|
||||
from app import notification_provider_clients
|
||||
from app.dao.provider_details_dao import (
|
||||
_adjust_provider_priority,
|
||||
_get_sms_providers_for_update,
|
||||
dao_get_provider_stats,
|
||||
dao_update_provider_details,
|
||||
@@ -33,9 +32,6 @@ def set_primary_sms_provider(identifier):
|
||||
get_alternative_sms_provider(identifier)
|
||||
)
|
||||
|
||||
primary_provider.priority = 10
|
||||
secondary_provider.priority = 20
|
||||
|
||||
dao_update_provider_details(primary_provider)
|
||||
dao_update_provider_details(secondary_provider)
|
||||
|
||||
@@ -55,18 +51,6 @@ def test_can_get_sms_international_providers(notify_db_session):
|
||||
assert all(prov.supports_international for prov in sms_providers)
|
||||
|
||||
|
||||
def test_can_get_sms_providers_in_order_of_priority(notify_db_session):
|
||||
providers = get_provider_details_by_notification_type(NotificationType.SMS, False)
|
||||
priorities = [provider.priority for provider in providers]
|
||||
assert priorities == sorted(priorities)
|
||||
|
||||
|
||||
def test_can_get_email_providers_in_order_of_priority(notify_db_session):
|
||||
providers = get_provider_details_by_notification_type(NotificationType.EMAIL)
|
||||
|
||||
assert providers[0].identifier == "ses"
|
||||
|
||||
|
||||
def test_can_get_email_providers(notify_db_session):
|
||||
assert len(get_provider_details_by_notification_type(NotificationType.EMAIL)) == 1
|
||||
types = [
|
||||
@@ -146,61 +130,6 @@ def test_get_alternative_sms_provider_fails_if_unrecognised():
|
||||
get_alternative_sms_provider("ses")
|
||||
|
||||
|
||||
@freeze_time("2016-01-01 00:30")
|
||||
def test_adjust_provider_priority_sets_priority(
|
||||
restore_provider_details,
|
||||
notify_user,
|
||||
sns_provider,
|
||||
):
|
||||
# need to update these manually to avoid triggering the `onupdate` clause of the updated_at column
|
||||
ProviderDetails.query.filter(ProviderDetails.identifier == "sns").update(
|
||||
{"updated_at": datetime.min}
|
||||
)
|
||||
|
||||
_adjust_provider_priority(sns_provider, 50)
|
||||
|
||||
assert sns_provider.updated_at == utc_now()
|
||||
assert sns_provider.created_by.id == notify_user.id
|
||||
assert sns_provider.priority == 50
|
||||
|
||||
|
||||
@freeze_time("2016-01-01 00:30")
|
||||
def test_adjust_provider_priority_adds_history(
|
||||
restore_provider_details,
|
||||
notify_user,
|
||||
sns_provider,
|
||||
):
|
||||
# need to update these manually to avoid triggering the `onupdate` clause of the updated_at column
|
||||
ProviderDetails.query.filter(ProviderDetails.identifier == "sns").update(
|
||||
{"updated_at": datetime.min}
|
||||
)
|
||||
|
||||
old_provider_history_rows = (
|
||||
ProviderDetailsHistory.query.filter(
|
||||
ProviderDetailsHistory.id == sns_provider.id
|
||||
)
|
||||
.order_by(desc(ProviderDetailsHistory.version))
|
||||
.all()
|
||||
)
|
||||
|
||||
_adjust_provider_priority(sns_provider, 50)
|
||||
|
||||
updated_provider_history_rows = (
|
||||
ProviderDetailsHistory.query.filter(
|
||||
ProviderDetailsHistory.id == sns_provider.id
|
||||
)
|
||||
.order_by(desc(ProviderDetailsHistory.version))
|
||||
.all()
|
||||
)
|
||||
|
||||
assert len(updated_provider_history_rows) - len(old_provider_history_rows) == 1
|
||||
assert (
|
||||
updated_provider_history_rows[0].version - old_provider_history_rows[0].version
|
||||
== 1
|
||||
)
|
||||
assert updated_provider_history_rows[0].priority == 50
|
||||
|
||||
|
||||
@freeze_time("2016-01-01 01:00")
|
||||
def test_get_sms_providers_for_update_returns_providers(restore_provider_details):
|
||||
ProviderDetails.query.filter(ProviderDetails.identifier == "sns").update(
|
||||
|
||||
@@ -42,7 +42,6 @@ def test_get_provider_contains_correct_fields(client, sample_template):
|
||||
"created_by_name",
|
||||
"display_name",
|
||||
"identifier",
|
||||
"priority",
|
||||
"notification_type",
|
||||
"active",
|
||||
"updated_at",
|
||||
@@ -53,24 +52,6 @@ def test_get_provider_contains_correct_fields(client, sample_template):
|
||||
assert allowed_keys == set(json_resp[0].keys())
|
||||
|
||||
|
||||
def test_should_be_able_to_update_priority(client, restore_provider_details):
|
||||
provider = ProviderDetails.query.first()
|
||||
|
||||
update_resp = client.post(
|
||||
"/provider-details/{}".format(provider.id),
|
||||
headers=[
|
||||
("Content-Type", "application/json"),
|
||||
create_admin_authorization_header(),
|
||||
],
|
||||
data=json.dumps({"priority": 5}),
|
||||
)
|
||||
assert update_resp.status_code == 200
|
||||
update_json = json.loads(update_resp.get_data(as_text=True))["provider_details"]
|
||||
assert update_json["identifier"] == provider.identifier
|
||||
assert update_json["priority"] == 5
|
||||
assert provider.priority == 5
|
||||
|
||||
|
||||
def test_should_be_able_to_update_status(client, restore_provider_details):
|
||||
provider = ProviderDetails.query.first()
|
||||
|
||||
|
||||
@@ -1113,49 +1113,6 @@ def test_create_template_raises_invalid_request_when_content_too_large(
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"notification_type,send_to",
|
||||
[
|
||||
(NotificationType.SMS, "2028675309"),
|
||||
(
|
||||
NotificationType.EMAIL,
|
||||
"sample@email.com",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_send_notification_uses_priority_queue_when_template_is_marked_as_priority(
|
||||
client,
|
||||
sample_service,
|
||||
mocker,
|
||||
notification_type,
|
||||
send_to,
|
||||
):
|
||||
sample = create_template(
|
||||
sample_service,
|
||||
template_type=notification_type,
|
||||
process_type=TemplateProcessType.PRIORITY,
|
||||
)
|
||||
mocked = mocker.patch(
|
||||
f"app.celery.provider_tasks.deliver_{notification_type}.apply_async"
|
||||
)
|
||||
|
||||
data = {"to": send_to, "template": str(sample.id)}
|
||||
|
||||
auth_header = create_service_authorization_header(service_id=sample.service_id)
|
||||
|
||||
response = client.post(
|
||||
path=f"/notifications/{notification_type}",
|
||||
data=json.dumps(data),
|
||||
headers=[("Content-Type", "application/json"), auth_header],
|
||||
)
|
||||
|
||||
response_data = json.loads(response.data)["data"]
|
||||
notification_id = response_data["notification"]["id"]
|
||||
|
||||
assert response.status_code == 201
|
||||
mocked.assert_called_once_with([notification_id], queue="priority-tasks")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"notification_type, send_to",
|
||||
[
|
||||
|
||||
@@ -161,24 +161,6 @@ def test_send_one_off_notification_calls_persist_correctly_for_email(
|
||||
)
|
||||
|
||||
|
||||
def test_send_one_off_notification_honors_priority(
|
||||
notify_db_session, persist_mock, celery_mock
|
||||
):
|
||||
service = create_service()
|
||||
template = create_template(service=service)
|
||||
template.process_type = TemplateProcessType.PRIORITY
|
||||
|
||||
post_data = {
|
||||
"template_id": str(template.id),
|
||||
"to": "202-867-5309",
|
||||
"created_by": str(service.created_by_id),
|
||||
}
|
||||
|
||||
send_one_off_notification(service.id, post_data)
|
||||
|
||||
assert celery_mock.call_args[1]["queue"] == QueueNames.PRIORITY
|
||||
|
||||
|
||||
def test_send_one_off_notification_raises_if_invalid_recipient(notify_db_session):
|
||||
service = create_service()
|
||||
template = create_template(service=service)
|
||||
|
||||
@@ -4,10 +4,9 @@ from app.config import QueueNames
|
||||
def test_queue_names_all_queues_correct():
|
||||
# Need to ensure that all_queues() only returns queue names used in API
|
||||
queues = QueueNames.all_queues()
|
||||
assert len(queues) == 15
|
||||
assert len(queues) == 14
|
||||
assert set(
|
||||
[
|
||||
QueueNames.PRIORITY,
|
||||
QueueNames.PERIODIC,
|
||||
QueueNames.DATABASE,
|
||||
QueueNames.SEND_SMS,
|
||||
|
||||
Reference in New Issue
Block a user