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

Task/read from last used #2092

Merged
merged 3 commits into from
Jan 23, 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
7 changes: 7 additions & 0 deletions app/dao/api_key_dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ def expire_api_key(service_id, api_key_id):
db.session.add(api_key)


@transactional
def update_last_used_api_key(api_key_id, last_used=None) -> None:
api_key = ApiKey.query.filter_by(id=api_key_id).one()
api_key.last_used_timestamp = last_used if last_used else datetime.utcnow()
db.session.add(api_key)


@transactional
@version_class(ApiKey)
def update_compromised_api_key_info(service_id, api_key_id, compromised_info):
Expand Down
29 changes: 20 additions & 9 deletions app/dao/fact_notification_status_dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,24 +336,35 @@ def get_total_notifications_sent_for_api_key(api_key_id):

def get_last_send_for_api_key(api_key_id):
"""
SELECT last_used_timestamp as last_notification_created
FROM api_keys
WHERE id = 'api_key_id';

If last_used_timestamp is null, then check notifications table/ or notification_history.
SELECT max(created_at) as last_notification_created
FROM notifications
WHERE api_key_id = 'api_key_id'
GROUP BY api_key_id;
"""
notification_table = (
db.session.query(func.max(Notification.created_at).label("last_notification_created"))
.filter(Notification.api_key_id == api_key_id)
.all()
# Fetch last_used_timestamp from api_keys table
api_key_table = (
db.session.query(ApiKey.last_used_timestamp.label("last_notification_created")).filter(ApiKey.id == api_key_id).all()
)
if not notification_table[0][0]:
if not api_key_table[0][0]:
notification_table = (
db.session.query(func.max(NotificationHistory.created_at).label("last_notification_created"))
.filter(NotificationHistory.api_key_id == api_key_id)
db.session.query(func.max(Notification.created_at).label("last_notification_created"))
.filter(Notification.api_key_id == api_key_id)
.all()
)
notification_table = [] if notification_table[0][0] is None else notification_table
return notification_table
if not notification_table[0][0]:
notification_table = (
db.session.query(func.max(NotificationHistory.created_at).label("last_notification_created"))
.filter(NotificationHistory.api_key_id == api_key_id)
.all()
)
notification_table = [] if notification_table[0][0] is None else notification_table
return notification_table
return api_key_table


def get_api_key_ranked_by_notifications_created(n_days_back):
Expand Down
12 changes: 12 additions & 0 deletions app/notifications/process_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from app.celery import provider_tasks
from app.celery.letters_pdf_tasks import create_letters_pdf
from app.config import QueueNames
from app.dao.api_key_dao import update_last_used_api_key
from app.dao.notifications_dao import (
bulk_insert_notifications,
dao_create_notification,
Expand Down Expand Up @@ -133,6 +134,8 @@ def persist_notification(
if redis_store.get(redis.daily_limit_cache_key(service.id)):
redis_store.incr(redis.daily_limit_cache_key(service.id))
current_app.logger.info("{} {} created at {}".format(notification_type, notification_id, notification_created_at))
if api_key_id:
update_last_used_api_key(api_key_id, notification_created_at)
return notification


Expand Down Expand Up @@ -298,6 +301,7 @@ def persist_notifications(notifications: List[VerifiedNotification]) -> List[Not
"""

lofnotifications = []
api_key_last_used = None

for notification in notifications:
notification_created_at = notification.get("created_at") or datetime.utcnow()
Expand Down Expand Up @@ -357,7 +361,15 @@ def persist_notifications(notifications: List[VerifiedNotification]) -> List[Not
notification.get("notification_created_at"), # type: ignore
)
)
# If the bulk message is sent using an api key, we want to keep track of the last time the api key was used
# We will only update the api key once
api_key_id = notification.get("api_key_id")
if api_key_id:
api_key_last_used = datetime.utcnow()
if api_key_last_used:
update_last_used_api_key(api_key_id, api_key_last_used)
bulk_insert_notifications(lofnotifications)

return lofnotifications


Expand Down
12 changes: 12 additions & 0 deletions tests/app/dao/test_api_key_dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
resign_api_keys,
save_model_api_key,
update_compromised_api_key_info,
update_last_used_api_key,
)
from app.models import KEY_TYPE_NORMAL, ApiKey
from tests.app.db import create_api_key
Expand Down Expand Up @@ -61,6 +62,17 @@ def test_expire_api_key_should_update_the_api_key_and_create_history_record(noti
sorted_all_history[1].version = 2


def test_last_used_should_update_the_api_key_and_not_create_history_record(notify_api, sample_api_key):
last_used = datetime.utcnow()
update_last_used_api_key(api_key_id=sample_api_key.id, last_used=last_used)
all_api_keys = get_model_api_keys(service_id=sample_api_key.service_id)
assert len(all_api_keys) == 1
assert all_api_keys[0].last_used_timestamp == last_used

all_history = sample_api_key.get_history_model().query.all()
assert len(all_history) == 1


def test_update_compromised_api_key_info_and_create_history_record(notify_api, sample_api_key):
update_compromised_api_key_info(
service_id=sample_api_key.service_id, api_key_id=sample_api_key.id, compromised_info={"key": "value"}
Expand Down
8 changes: 8 additions & 0 deletions tests/app/dao/test_fact_notification_status_dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,14 @@ def test_get_total_notifications_sent_for_api_key(notify_db_session):
assert dict(api_key_stats_3) == dict([(EMAIL_TYPE, total_sends), (SMS_TYPE, total_sends)])


def test_get_last_send_for_api_key_check_last_used(notify_db_session):
service = create_service(service_name="First Service")
api_key = create_api_key(service, last_used=datetime.utcnow())

last_send = get_last_send_for_api_key(str(api_key.id))[0][0]
assert last_send == api_key.last_used_timestamp


def test_get_last_send_for_api_key(notify_db_session):
service = create_service(service_name="First Service")
api_key = create_api_key(service)
Expand Down
3 changes: 2 additions & 1 deletion tests/app/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ def create_letter_rate(
return rate


def create_api_key(service, key_type=KEY_TYPE_NORMAL, key_name=None):
def create_api_key(service, key_type=KEY_TYPE_NORMAL, key_name=None, last_used=None):
id_ = uuid.uuid4()

name = key_name if key_name else "{} api key {}".format(key_type, id_)
Expand All @@ -563,6 +563,7 @@ def create_api_key(service, key_type=KEY_TYPE_NORMAL, key_name=None):
key_type=key_type,
id=id_,
secret=uuid.uuid4(),
last_used_timestamp=last_used,
)
db.session.add(api_key)
db.session.commit()
Expand Down
7 changes: 7 additions & 0 deletions tests/app/notifications/test_process_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
LETTER_TYPE,
NORMAL,
PRIORITY,
ApiKey,
Notification,
NotificationHistory,
ScheduledNotification,
Expand Down Expand Up @@ -433,6 +434,10 @@ def test_persist_notifications_list(self, sample_job, sample_api_key, notify_db_
assert persisted_notification[1].to == "[email protected]"
assert persisted_notification[0].service == sample_job.service

# Test that the api key last_used_timestamp got updated
api_key = ApiKey.query.get(sample_api_key.id)
assert api_key.last_used_timestamp is not None

def test_persist_notifications_reply_to_text_is_original_value_if_sender_is_changed_later(
self, sample_template, sample_api_key, mocker
):
Expand Down Expand Up @@ -913,6 +918,8 @@ def test_transform_email_notification_stores_normalised_email(

assert persisted_notification.to == recipient
assert persisted_notification.normalised_to == expected_recipient_normalised
api_key = ApiKey.query.get(sample_api_key.id)
assert api_key.last_used_timestamp is not None


class TestDBSaveAndSendNotification:
Expand Down