From 7fc7d99dacbf148a0442043a25db00f2f92c4594 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 3 Oct 2019 14:58:49 +0100 Subject: [PATCH] Update the new endpoint to return a 404 if the job or service id are not found. All our endpoint should perform a check that the params are valid - this is an easy whay to check that and is standard for our endpoints. I reverted the query to just filter by job id. --- app/dao/notifications_dao.py | 4 ++-- app/job/rest.py | 3 ++- .../notification_dao/test_notification_dao.py | 19 ++++----------- tests/app/job/test_rest.py | 23 +++++++++++++++---- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 273bd5eb1..a756a1ca2 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -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(*, service_id, job_id): - return Notification.query.filter_by(service_id=service_id, job_id=job_id).count() +def dao_get_notification_count_for_job_id(*, job_id): + return Notification.query.filter_by(job_id=job_id).count() @statsd(namespace="dao") diff --git a/app/job/rest.py b/app/job/rest.py index 7de187698..e5ce149fe 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -110,7 +110,8 @@ def get_all_notifications_for_service_job(service_id, job_id): @job_blueprint.route('//notification_count', methods=['GET']) def get_notification_count_for_job_id(service_id, job_id): - count = dao_get_notification_count_for_job_id(service_id=service_id, job_id=job_id) + dao_get_job_by_service_id_and_job_id(service_id, job_id) + count = dao_get_notification_count_for_job_id(job_id=job_id) return jsonify( count=count ), 200 diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 97cdf38af..eaf35fcc5 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -555,7 +555,7 @@ def test_get_all_notifications_for_job_by_status(sample_job): assert len(notifications(filter_dict={'status': NOTIFICATION_STATUS_TYPES[:3]}).items) == 3 -def test_dao_get_notification_count_for_job_id(notify_db_session, notify_db): +def test_dao_get_notification_count_for_job_id(notify_db_session): service = create_service() template = create_template(service) job = create_job(template, notification_count=3) @@ -564,27 +564,16 @@ 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(service_id=service.id, job_id=job.id) == 3 + assert dao_get_notification_count_for_job_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): +def test_dao_get_notification_count_for_job_id_returns_zero_for_no_notifications_for_job(notify_db_session): service = create_service() template = create_template(service) job = create_job(template, notification_count=3) create_notification(template) - 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 + assert dao_get_notification_count_for_job_id(job_id=job.id) == 0 def test_update_notification_sets_status(sample_notification): diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 6a96c9adc..998c576d5 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -563,15 +563,28 @@ def test_get_all_notifications_for_job_returns_correct_format( assert resp['notifications'][0]['status'] == sample_notification_with_job.status -def test_get_notification_count_for_job_id(admin_request, mocker): +def test_get_notification_count_for_job_id(admin_request, mocker, sample_job): mock_dao = mocker.patch('app.job.rest.dao_get_notification_count_for_job_id', return_value=3) - job_id = uuid.uuid4() - 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)) + response = admin_request.get('job.get_notification_count_for_job_id', + service_id=sample_job.service_id, job_id=sample_job.id) + mock_dao.assert_called_once_with(job_id=str(sample_job.id)) assert response["count"] == 3 +def test_get_notification_count_for_job_id_for_wrong_service_id(admin_request, sample_job): + service_id = uuid.uuid4() + response = admin_request.get('job.get_notification_count_for_job_id', service_id=service_id, + job_id=sample_job.id, _expected_status=404) + assert response['message'] == 'No result found' + + +def test_get_notification_count_for_job_id_for_wrong_job_id(admin_request, sample_service): + job_id = uuid.uuid4() + response = admin_request.get('job.get_notification_count_for_job_id', service_id=sample_service.id, + job_id=job_id, _expected_status=404) + assert response['message'] == 'No result found' + + def test_get_job_by_id(admin_request, sample_job): job_id = str(sample_job.id) service_id = sample_job.service.id