Use service_id in the query to make it safer, also use named parameters

This commit is contained in:
Pea Tyczynska
2019-09-25 15:48:28 +01:00
parent 8cf8d24e37
commit c48aa77dd5
4 changed files with 19 additions and 7 deletions

View File

@@ -179,8 +179,8 @@ def get_notifications_for_job(service_id, job_id, filter_dict=None, page=1, page
@statsd(namespace="dao")
def dao_get_notification_count_for_job_id(job_id):
return Notification.query.filter_by(job_id=job_id).count()
def dao_get_notification_count_for_job_id(*, service_id, job_id):
return Notification.query.filter_by(service_id=service_id, job_id=job_id).count()
@statsd(namespace="dao")

View File

@@ -110,7 +110,7 @@ def get_all_notifications_for_service_job(service_id, job_id):
@job_blueprint.route('/<job_id>/notification_count', methods=['GET'])
def get_notification_count_for_job_id(service_id, job_id):
count = dao_get_notification_count_for_job_id(job_id)
count = dao_get_notification_count_for_job_id(service_id=service_id, job_id=job_id)
return jsonify(
count=count
), 200

View File

@@ -564,7 +564,7 @@ def test_dao_get_notification_count_for_job_id(notify_db_session, notify_db):
create_notification(template)
assert dao_get_notification_count_for_job_id(job.id) == 3
assert dao_get_notification_count_for_job_id(service_id=service.id, job_id=job.id) == 3
def test_dao_get_notification_count_for_job_id_only_finds_notification_already_in_db(notify_db_session, notify_db):
@@ -573,7 +573,18 @@ def test_dao_get_notification_count_for_job_id_only_finds_notification_already_i
job = create_job(template, notification_count=3)
create_notification(template)
assert dao_get_notification_count_for_job_id(job.id) == 0
assert dao_get_notification_count_for_job_id(service_id=service.id, job_id=job.id) == 0
def test_dao_get_notification_count_for_job_id_doesnt_work_with_non_existing_service_id(notify_db_session, notify_db):
service = create_service()
template = create_template(service)
job = create_job(template, notification_count=3)
for i in range(3):
create_notification(job=job)
fake_service_id = str(uuid.uuid4())
assert dao_get_notification_count_for_job_id(service_id=fake_service_id, job_id=job.id) == 0
def test_update_notification_sets_status(sample_notification):

View File

@@ -566,8 +566,9 @@ def test_get_all_notifications_for_job_returns_correct_format(
def test_get_notification_count_for_job_id(admin_request, mocker):
mock_dao = mocker.patch('app.job.rest.dao_get_notification_count_for_job_id', return_value=3)
job_id = uuid.uuid4()
response = admin_request.get('job.get_notification_count_for_job_id', service_id=uuid.uuid4(), job_id=job_id)
mock_dao.assert_called_once_with(str(job_id))
service_id = uuid.uuid4()
response = admin_request.get('job.get_notification_count_for_job_id', service_id=service_id, job_id=job_id)
mock_dao.assert_called_once_with(service_id=service_id, job_id=str(job_id))
assert response["count"] == 3