From a6dae2c81bbb99d240ec652828711de7296bda2c Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 14 Feb 2018 14:47:03 +0000 Subject: [PATCH] Remove unnecessary code to do with testing service name uniqueness notifications-admin has now been changed to always pass the service_id to the 'service/unique' endpoint. This means we don't need to cover the case of there being no service_id and the tests can also be updated. --- app/service/rest.py | 13 ++- tests/app/service/test_rest.py | 149 ++++++++++++++++++--------------- 2 files changed, 87 insertions(+), 75 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 9d8098d95..8d74124a2 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -677,13 +677,10 @@ def is_service_name_unique(): name_exists = Service.query.filter_by(name=name).first() - if service_id: - email_from_exists = Service.query.filter( - Service.email_from == email_from, - Service.id != service_id - ).first() - else: - email_from_exists = Service.query.filter_by(email_from=email_from).first() + email_from_exists = Service.query.filter( + Service.email_from == email_from, + Service.id != service_id + ).first() result = not (name_exists or email_from_exists) return jsonify(result=result), 200 @@ -694,6 +691,8 @@ def check_request_args(request): name = request.args.get('name', None) email_from = request.args.get('email_from', None) errors = [] + if not service_id: + errors.append({'service_id': ["Can't be empty"]}) if not name: errors.append({'name': ["Can't be empty"]}) if not email_from: diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index d4185a239..b1930a511 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2195,84 +2195,97 @@ def test_search_for_notification_by_to_field_returns_personlisation( assert notifications[0]['personalisation']['name'] == 'Foo' -def test_is_service_name_unique_returns_200_if_unique(client): - response = client.get('/service/unique?name=something&email_from=something', - headers=[create_authorization_header()]) - assert response.status_code == 200 - assert json.loads(response.get_data(as_text=True)) == {"result": True} - - -def test_is_service_name_unique_returns_200_if_unique_and_service_id_given( - client, - notify_db, - notify_db_session -): +def test_is_service_name_unique_returns_200_if_unique(admin_request, notify_db, notify_db_session): service = create_service(service_name='unique', email_from='unique') - service_id = str(service.id) - response = client.get( - '/service/unique?service_id={}&name=something&email_from=something'.format(service_id), - headers=[create_authorization_header()] + response = admin_request.get( + 'service.is_service_name_unique', + _expected_status=200, + service_id=service.id, + name='something', + email_from='something' ) - assert response.status_code == 200 - assert json.loads(response.get_data(as_text=True)) == {"result": True} - - -def test_is_service_name_unique_returns_200_when_capitalized( - client, - notify_db, - notify_db_session -): - service = create_service(service_name='unique', email_from='unique') - service_id = str(service.id) - - response = client.get( - '/service/unique?service_id={}&name={}&email_from={}'.format(service_id, 'UNIQUE', 'unique'), - headers=[create_authorization_header()] - ) - - assert response.status_code == 200 - assert json.loads(response.get_data(as_text=True)) == {"result": True} - - -def test_is_service_name_unique_returns_false_if_checking_capitalization_of_different_service( - client, - notify_db, - notify_db_session -): - create_service(service_name='unique', email_from='unique') - different_service_id = '111aa111-2222-bbbb-aaaa-111111111111' - - response = client.get( - '/service/unique?service_id={}&name={}&email_from={}'.format( - different_service_id, 'UNIQUE', 'unique'), - headers=[create_authorization_header()] - ) - - assert response.status_code == 200 - assert json.loads(response.get_data(as_text=True)) == {"result": False} + assert response == {"result": True} @pytest.mark.parametrize('name, email_from', - [("something unique", "something"), - ("unique", "something.unique"), - ("something unique", "something.unique") + [("UNIQUE", "unique"), + ("Unique.", "unique"), + ("**uniQUE**", "unique") ]) -def test_is_service_name_unique_returns_200_and_false(client, notify_db, notify_db_session, name, email_from): - create_service(service_name='something unique', email_from='something.unique') - response = client.get('/service/unique?name={}&email_from={}'.format(name, email_from), - headers=[create_authorization_header()]) - assert response.status_code == 200 - assert json.loads(response.get_data(as_text=True)) == {"result": False} +def test_is_service_name_unique_returns_200_with_name_capitalized_or_punctuation_added( + admin_request, + notify_db, + notify_db_session, + name, + email_from +): + service = create_service(service_name='unique', email_from='unique') + + response = admin_request.get( + 'service.is_service_name_unique', + _expected_status=200, + service_id=service.id, + name=name, + email_from=email_from + ) + + assert response == {"result": True} -def test_is_service_name_unique_returns_400_when_name_does_not_exist(client): - response = client.get('/service/unique', headers=[create_authorization_header()]) - assert response.status_code == 400 - json_resp = json.loads(response.get_data(as_text=True)) - assert json_resp["message"][0]["name"] == ["Can't be empty"] - assert json_resp["message"][1]["email_from"] == ["Can't be empty"] +@pytest.mark.parametrize('name, email_from', [ + ("existing name", "email.from"), + ("name", "existing.name") + ]) +def test_is_service_name_unique_returns_200_and_false_if_name_or_email_from_exist_for_a_different_service( + admin_request, + notify_db, + notify_db_session, + name, + email_from +): + create_service(service_name='existing name', email_from='existing.name') + different_service_id = '111aa111-2222-bbbb-aaaa-111111111111' + + response = admin_request.get( + 'service.is_service_name_unique', + _expected_status=200, + service_id=different_service_id, + name=name, + email_from=email_from + ) + + assert response == {"result": False} + + +def test_is_service_name_unique_returns_200_and_false_if_name_exists_for_the_same_service( + admin_request, + notify_db, + notify_db_session +): + service = create_service(service_name='unique', email_from='unique') + + response = admin_request.get( + 'service.is_service_name_unique', + _expected_status=200, + service_id=service.id, + name='unique', + email_from='unique2' + ) + + assert response == {"result": False} + + +def test_is_service_name_unique_returns_400_when_name_does_not_exist(admin_request): + response = admin_request.get( + 'service.is_service_name_unique', + _expected_status=400 + ) + + assert response["message"][0]["service_id"] == ["Can't be empty"] + assert response["message"][1]["name"] == ["Can't be empty"] + assert response["message"][2]["email_from"] == ["Can't be empty"] def test_get_email_reply_to_addresses_when_there_are_no_reply_to_email_addresses(client, sample_service):