diff --git a/app/schemas.py b/app/schemas.py index 0ec518dc4..4c5fe961a 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -2,7 +2,7 @@ from flask_marshmallow.fields import fields from . import ma from . import models from app.dao.permissions_dao import permission_dao -from marshmallow import (post_load, ValidationError, validates) +from marshmallow import (post_load, ValidationError, validates, validates_schema) from marshmallow_sqlalchemy import field_for from utils.recipients import ( validate_email_address, InvalidEmailError, @@ -15,6 +15,7 @@ from utils.recipients import ( # Would be better to replace functionality in dao with the marshmallow supported # functionality. # http://marshmallow.readthedocs.org/en/latest/api_reference.html +# http://marshmallow.readthedocs.org/en/latest/extending.html class BaseSchema(ma.ModelSchema): @@ -72,6 +73,21 @@ class ServiceSchema(BaseSchema): model = models.Service exclude = ("updated_at", "created_at", "api_keys", "templates", "jobs", 'old_id') + @validates_schema + def validate_all(self, data): + # There are 2 instances where we want to check + # for duplicate service name. One when they updating + # an existing service and when they are creating a service + name = data.get('name', None) + service = models.Service.query.filter_by(name=name).first() + error_msg = "Duplicate service name '{}'".format(name) + if 'id' in data: + if service and str(service.id) != data['id']: + raise ValidationError(error_msg, 'name') + else: + if service: + raise ValidationError(error_msg, 'name') + class TemplateSchema(BaseSchema): class Meta: diff --git a/app/service/rest.py b/app/service/rest.py index 4ded0168c..43922ebdf 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -97,7 +97,6 @@ def update_service(service_id): current_data = dict(service_schema.dump(fetched_service).data.items()) current_data.update(request.get_json()) - print(current_data) update_dict, errors = service_schema.load(current_data) if errors: return jsonify(result="error", message=errors), 400 diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index fe9060949..e7296fbc4 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -813,7 +813,6 @@ def test_should_block_api_call_if_over_day_limit_regardless_of_type(notify_db, n data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) json_resp = json.loads(response.get_data(as_text=True)) - print(json_resp) assert response.status_code == 429 assert 'Exceeded send limits (1) for today' in json_resp['message'] diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index d6ca4b539..708d9bf45 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -5,6 +5,7 @@ from app.dao.users_dao import save_model_user from app.dao.services_dao import dao_remove_user_from_service from app.models import User from tests import create_authorization_header +from tests.app.conftest import sample_service as create_service def test_get_service_list(notify_api, service_factory): @@ -286,6 +287,35 @@ def test_should_not_create_service_if_missing_data(notify_api, sample_user): assert 'Missing data for required field.' in json_resp['message']['restricted'] +def test_should_not_create_service_with_duplicate_name(notify_api, + notify_db, + notify_db_session, + sample_user, + sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = { + 'name': sample_service.name, + 'user_id': sample_service.users[0].id, + 'limit': 1000, + 'restricted': False, + 'active': False} + auth_header = create_authorization_header( + path='/service', + method='POST', + request_body=json.dumps(data) + ) + headers = [('Content-Type', 'application/json'), auth_header] + resp = client.post( + '/service', + data=json.dumps(data), + headers=headers) + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 400 + assert "Duplicate service name '{}'".format( + sample_service.name) in json_resp['message']['name'] + + def test_update_service(notify_api, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -321,6 +351,40 @@ def test_update_service(notify_api, sample_service): assert result['data']['name'] == 'updated service name' +def test_should_not_update_service_with_duplicate_name(notify_api, + notify_db, + notify_db_session, + sample_user, + sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + service_name = "another name" + another_service = create_service( + notify_db, + notify_db_session, + service_name=service_name, + user=sample_user) + data = { + 'name': service_name + } + + auth_header = create_authorization_header( + path='/service/{}'.format(sample_service.id), + method='POST', + request_body=json.dumps(data) + ) + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 400 + assert "Duplicate service name '{}'".format( + service_name) in json_resp['message']['name'] + + def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db, notify_db_session): with notify_api.test_request_context(): with notify_api.test_client() as client: