fix some issues uncovered during testing

This commit is contained in:
Kenneth Kehl
2023-05-01 13:26:19 -07:00
parent b16883e989
commit be04edd04a
7 changed files with 16 additions and 21 deletions

View File

@@ -42,6 +42,7 @@ from app.v2.errors import TooManyRequestsError
@notify_celery.task(name="process-job") @notify_celery.task(name="process-job")
def process_job(job_id, sender_id=None): def process_job(job_id, sender_id=None):
current_app.logger.warning("ENTER process_job")
start = datetime.utcnow() start = datetime.utcnow()
job = dao_get_job_by_id(job_id) job = dao_get_job_by_id(job_id)
current_app.logger.info("Starting process-job task for job id {} with status: {}".format(job_id, job.job_status)) current_app.logger.info("Starting process-job task for job id {} with status: {}".format(job_id, job.job_status))
@@ -166,10 +167,10 @@ def __total_sending_limits_for_job_exceeded(service, job, job_id):
else: else:
return False return False
except TooManyRequestsError: except TooManyRequestsError:
job.job_status = 'total sending limits exceeded' job.job_status = 'sending limits exceeded'
job.processing_finished = datetime.utcnow() job.processing_finished = datetime.utcnow()
dao_update_job(job) dao_update_job(job)
current_app.logger.info( current_app.logger.error(
"Job {} size {} error. Total sending limits {} exceeded".format( "Job {} size {} error. Total sending limits {} exceeded".format(
job_id, job.notification_count, service.message_limit) job_id, job.notification_count, service.message_limit)
) )

View File

@@ -436,7 +436,7 @@ class Service(db.Model, Versioned):
active = db.Column(db.Boolean, index=False, unique=False, nullable=False, default=True) active = db.Column(db.Boolean, index=False, unique=False, nullable=False, default=True)
message_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=False) message_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=False)
# TODO nullable if we are adding it late? # TODO nullable if we are adding it late?
total_message_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=250000) total_message_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=7)
restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False)
research_mode = db.Column(db.Boolean, index=False, unique=False, nullable=False, default=False) research_mode = db.Column(db.Boolean, index=False, unique=False, nullable=False, default=False)
email_from = db.Column(db.Text, index=False, unique=True, nullable=False) email_from = db.Column(db.Text, index=False, unique=True, nullable=False)

View File

@@ -142,7 +142,7 @@ def persist_notification(
if key_type != KEY_TYPE_TEST and current_app.config['REDIS_ENABLED']: if key_type != KEY_TYPE_TEST and current_app.config['REDIS_ENABLED']:
current_app.logger.info('Redis enabled, querying cache key for service id: {}'.format(service.id)) current_app.logger.info('Redis enabled, querying cache key for service id: {}'.format(service.id))
cache_key = redis.daily_limit_cache_key(service.id) cache_key = redis.daily_limit_cache_key(service.id)
total_key = redis.daily_total_cache_key() total_key = redis.total_limit_cache_key(service.id)
current_app.logger.info('Redis daily limit cache key: {}'.format(cache_key)) current_app.logger.info('Redis daily limit cache key: {}'.format(cache_key))
if redis_store.get(cache_key) is None: if redis_store.get(cache_key) is None:
current_app.logger.info('Redis daily limit cache key does not exist') current_app.logger.info('Redis daily limit cache key does not exist')
@@ -163,7 +163,8 @@ def persist_notification(
else: else:
current_app.logger.info('Redis total limit cache key does exist') current_app.logger.info('Redis total limit cache key does exist')
redis_store.incr(total_key) redis_store.incr(total_key)
current_app.logger.info('Redis total limit cache key has been incremented') current_app.logger.info(
f'Redis total limit cache key has been incremented to {redis_store.get(total_key)}')
current_app.logger.info( current_app.logger.info(
"{} {} created at {}".format(notification_type, notification_id, notification_created_at) "{} {} created at {}".format(notification_type, notification_id, notification_created_at)
) )

View File

@@ -87,7 +87,7 @@ def check_service_over_total_message_limit(key_type, service):
redis_store.set(cache_key, service_stats, ex=86400) redis_store.set(cache_key, service_stats, ex=86400)
return service_stats return service_stats
if int(service_stats) >= service.total_message_limit: if int(service_stats) >= service.total_message_limit:
current_app.logger.info( current_app.logger.warning(
"service {} has been rate limited for total use sent {} limit {}".format( "service {} has been rate limited for total use sent {} limit {}".format(
service.id, int(service_stats), service.total_message_limit) service.id, int(service_stats), service.total_message_limit)
) )

View File

@@ -1,3 +1,4 @@
from flask import current_app
from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.exc import NoResultFound
from app.config import QueueNames from app.config import QueueNames

View File

@@ -219,9 +219,11 @@ def test_persist_notification_increments_cache_for_trial_or_live_service(
reference="ref2") reference="ref2")
assert mock_incr.call_count == 2 assert mock_incr.call_count == 2
mock_incr.assert_has_calls([ mock_incr.assert_has_calls([
call(str(service.id) + "-2016-01-01-count", ), call(str(service.id) + "-2016-01-01-count", ),
call("2016-01-01-total", ) call(str(service.id) + "-2016-01-01-total-count", ),
]) ])
@@ -250,7 +252,8 @@ def test_persist_notification_sets_daily_limit_cache_if_one_does_not_exists(
assert mock_set.call_count == 2 assert mock_set.call_count == 2
mock_set.assert_has_calls([ mock_set.assert_has_calls([
call(str(service.id) + "-2016-01-01-count", 1, ex=86400), call(str(service.id) + "-2016-01-01-count", 1, ex=86400),
call("2016-01-01-total", 1, ex=86400) call(str(service.id) + "-2016-01-01-total-count", 1, ex=86400),
]) ])

View File

@@ -677,7 +677,6 @@ def test_update_service(client, notify_db_session, sample_service):
'created_by': str(sample_service.created_by.id), 'created_by': str(sample_service.created_by.id),
'email_branding': str(brand.id), 'email_branding': str(brand.id),
'organisation_type': 'federal', 'organisation_type': 'federal',
'total_message_limit': 250000,
} }
auth_header = create_admin_authorization_header() auth_header = create_admin_authorization_header()
@@ -717,7 +716,6 @@ def test_cant_update_service_org_type_to_random_value(client, sample_service):
def test_update_service_remove_email_branding(admin_request, notify_db_session, sample_service): def test_update_service_remove_email_branding(admin_request, notify_db_session, sample_service):
brand = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League') brand = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League')
sample_service.email_branding = brand sample_service.email_branding = brand
sample_service.total_message_limit = 250000
notify_db_session.commit() notify_db_session.commit()
resp = admin_request.post( resp = admin_request.post(
@@ -733,7 +731,6 @@ def test_update_service_change_email_branding(admin_request, notify_db_session,
brand2 = EmailBranding(colour='#111111', logo='avengers.png', name='Avengers') brand2 = EmailBranding(colour='#111111', logo='avengers.png', name='Avengers')
notify_db_session.add_all([brand1, brand2]) notify_db_session.add_all([brand1, brand2])
sample_service.email_branding = brand1 sample_service.email_branding = brand1
sample_service.total_message_limit = 250000
notify_db_session.commit() notify_db_session.commit()
resp = admin_request.post( resp = admin_request.post(
@@ -757,7 +754,6 @@ def test_update_service_flags(client, sample_service):
data = { data = {
'research_mode': True, 'research_mode': True,
'total_message_limit': 250000,
'permissions': [INTERNATIONAL_SMS_TYPE] 'permissions': [INTERNATIONAL_SMS_TYPE]
} }
@@ -796,7 +792,6 @@ def test_update_service_sets_volumes(
service_id=sample_service.id, service_id=sample_service.id,
_data={ _data={
field: value, field: value,
'total_message_limit': 250000,
}, },
_expected_status=expected_status, _expected_status=expected_status,
) )
@@ -821,7 +816,6 @@ def test_update_service_sets_research_consent(
service_id=sample_service.id, service_id=sample_service.id,
_data={ _data={
'consent_to_research': value, 'consent_to_research': value,
'total_message_limit': 250000,
}, },
_expected_status=expected_status, _expected_status=expected_status,
) )
@@ -898,7 +892,6 @@ def test_update_service_permissions_will_add_service_permissions(client, sample_
data = { data = {
'permissions': [EMAIL_TYPE, SMS_TYPE], 'permissions': [EMAIL_TYPE, SMS_TYPE],
'total_message_limit': 250000,
} }
resp = client.post( resp = client.post(
@@ -1023,7 +1016,6 @@ def test_should_not_update_service_with_duplicate_name(notify_api,
data = { data = {
'name': service_name, 'name': service_name,
'created_by': str(service.created_by.id), 'created_by': str(service.created_by.id),
'total_message_limit': 250000,
} }
auth_header = create_admin_authorization_header() auth_header = create_admin_authorization_header()
@@ -1055,7 +1047,6 @@ def test_should_not_update_service_with_duplicate_email_from(notify_api,
'name': service_name, 'name': service_name,
'email_from': email_from, 'email_from': email_from,
'created_by': str(service.created_by.id), 'created_by': str(service.created_by.id),
'total_message_limit': 250000,
} }
auth_header = create_admin_authorization_header() auth_header = create_admin_authorization_header()
@@ -1927,7 +1918,7 @@ def test_set_sms_prefixing_for_service(
result = admin_request.post( result = admin_request.post(
'service.update_service', 'service.update_service',
service_id=sample_service.id, service_id=sample_service.id,
_data={'prefix_sms': posted_value, 'total_message_limit': 250000}, _data={'prefix_sms': posted_value},
) )
assert result['data']['prefix_sms'] == stored_value assert result['data']['prefix_sms'] == stored_value
@@ -1939,7 +1930,7 @@ def test_set_sms_prefixing_for_service_cant_be_none(
resp = admin_request.post( resp = admin_request.post(
'service.update_service', 'service.update_service',
service_id=sample_service.id, service_id=sample_service.id,
_data={'prefix_sms': None, 'total_message_limit': 250000}, _data={'prefix_sms': None},
_expected_status=400, _expected_status=400,
) )
assert resp['message'] == {'prefix_sms': ['Field may not be null.']} assert resp['message'] == {'prefix_sms': ['Field may not be null.']}
@@ -2279,7 +2270,6 @@ def test_update_service_does_not_call_send_notification_for_live_service(sample_
data = { data = {
"restricted": True, "restricted": True,
'total_message_limit': 250000,
} }
auth_header = create_admin_authorization_header() auth_header = create_admin_authorization_header()
@@ -2299,7 +2289,6 @@ def test_update_service_does_not_call_send_notification_when_restricted_not_chan
data = { data = {
"name": 'Name of service', "name": 'Name of service',
'total_message_limit': 250000,
} }
auth_header = create_admin_authorization_header() auth_header = create_admin_authorization_header()