mirror of
https://github.com/GSA/notifications-api.git
synced 2026-02-03 18:01:08 -05:00
The admin app now sends the email from when creating a service and when updating the service name.
This PR removes the need for the email_safe function. The api does not create the email_from field for the service. Tests were updated to reflect this change.
This commit is contained in:
@@ -93,11 +93,5 @@ def init_app(app):
|
||||
return response
|
||||
|
||||
|
||||
def email_safe(string):
|
||||
return "".join([
|
||||
character.lower() if character.isalnum() or character == "." else "" for character in re.sub("\s+", ".", string.strip()) # noqa
|
||||
])
|
||||
|
||||
|
||||
def create_uuid():
|
||||
return str(uuid.uuid4())
|
||||
|
||||
@@ -7,7 +7,6 @@ from flask import (
|
||||
)
|
||||
from sqlalchemy.orm.exc import NoResultFound
|
||||
|
||||
from app import email_safe
|
||||
from app.dao.api_key_dao import (
|
||||
save_model_api_key,
|
||||
get_model_api_keys,
|
||||
@@ -72,8 +71,6 @@ def create_service():
|
||||
user = get_model_users(data['user_id'])
|
||||
|
||||
data.pop('user_id', None)
|
||||
if 'name' in data:
|
||||
data['email_from'] = email_safe(data.get('name', None))
|
||||
|
||||
valid_service, errors = service_schema.load(request.get_json())
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import pytest
|
||||
from datetime import datetime
|
||||
from app import (email_safe, db)
|
||||
from app import db
|
||||
from app.models import (
|
||||
User, Service, Template, ApiKey, Job, Notification, InvitedUser, Permission)
|
||||
from app.dao.users_dao import (save_model_user, create_user_code, create_secret_code)
|
||||
@@ -16,16 +16,16 @@ import uuid
|
||||
@pytest.fixture(scope='function')
|
||||
def service_factory(notify_db, notify_db_session):
|
||||
class ServiceFactory(object):
|
||||
def get(self, service_name, user=None, template_type=None):
|
||||
def get(self, service_name, user=None, template_type=None, email_from=None):
|
||||
if not user:
|
||||
user = sample_user(notify_db, notify_db_session)
|
||||
service = sample_service(notify_db, notify_db_session, service_name, user)
|
||||
service = sample_service(notify_db, notify_db_session, service_name, user, email_from=email_from)
|
||||
if template_type == 'email':
|
||||
sample_template(
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
template_type=template_type,
|
||||
subject_line=email_safe(service_name),
|
||||
subject_line=service.email_from,
|
||||
service=service
|
||||
)
|
||||
else:
|
||||
@@ -102,7 +102,8 @@ def sample_service(notify_db,
|
||||
service_name="Sample service",
|
||||
user=None,
|
||||
restricted=False,
|
||||
limit=1000):
|
||||
limit=1000,
|
||||
email_from="sample.service"):
|
||||
if user is None:
|
||||
user = sample_user(notify_db, notify_db_session)
|
||||
data = {
|
||||
@@ -110,7 +111,7 @@ def sample_service(notify_db,
|
||||
'limit': limit,
|
||||
'active': False,
|
||||
'restricted': restricted,
|
||||
'email_from': email_safe(service_name)
|
||||
'email_from': email_from
|
||||
}
|
||||
service = Service.query.filter_by(name=service_name).first()
|
||||
if not service:
|
||||
|
||||
@@ -49,7 +49,8 @@ def test_get_jobs_for_service(notify_db, notify_db_session, sample_template):
|
||||
one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template)
|
||||
|
||||
other_user = create_user(notify_db, notify_db_session, email="test@digital.cabinet-office.gov.uk")
|
||||
other_service = create_service(notify_db, notify_db_session, user=other_user, service_name="other service")
|
||||
other_service = create_service(notify_db, notify_db_session, user=other_user, service_name="other service",
|
||||
email_from='other.service')
|
||||
other_template = create_template(notify_db, notify_db_session, service=other_service)
|
||||
other_job = create_job(notify_db, notify_db_session, service=other_service, template=other_template)
|
||||
|
||||
|
||||
@@ -85,20 +85,20 @@ def test_should_remove_user_from_service(sample_user):
|
||||
|
||||
|
||||
def test_get_all_services(service_factory):
|
||||
service_factory.get('service 1')
|
||||
service_factory.get('service 1', email_from='service.1')
|
||||
assert len(dao_fetch_all_services()) == 1
|
||||
assert dao_fetch_all_services()[0].name == 'service 1'
|
||||
|
||||
service_factory.get('service 2')
|
||||
service_factory.get('service 2', email_from='service.2')
|
||||
assert len(dao_fetch_all_services()) == 2
|
||||
assert dao_fetch_all_services()[1].name == 'service 2'
|
||||
|
||||
|
||||
def test_get_all_services_should_return_in_created_order(service_factory):
|
||||
service_factory.get('service 1')
|
||||
service_factory.get('service 2')
|
||||
service_factory.get('service 3')
|
||||
service_factory.get('service 4')
|
||||
service_factory.get('service 1', email_from='service.1')
|
||||
service_factory.get('service 2', email_from='service.2')
|
||||
service_factory.get('service 3', email_from='service.3')
|
||||
service_factory.get('service 4', email_from='service.4')
|
||||
assert len(dao_fetch_all_services()) == 4
|
||||
assert dao_fetch_all_services()[0].name == 'service 1'
|
||||
assert dao_fetch_all_services()[1].name == 'service 2'
|
||||
@@ -111,9 +111,9 @@ def test_get_all_services_should_return_empty_list_if_no_services():
|
||||
|
||||
|
||||
def test_get_all_services_for_user(service_factory, sample_user):
|
||||
service_factory.get('service 1', sample_user)
|
||||
service_factory.get('service 2', sample_user)
|
||||
service_factory.get('service 3', sample_user)
|
||||
service_factory.get('service 1', sample_user, email_from='service.1')
|
||||
service_factory.get('service 2', sample_user, email_from='service.2')
|
||||
service_factory.get('service 3', sample_user, email_from='service.3')
|
||||
assert len(dao_fetch_all_services_by_user(sample_user.id)) == 3
|
||||
assert dao_fetch_all_services_by_user(sample_user.id)[0].name == 'service 1'
|
||||
assert dao_fetch_all_services_by_user(sample_user.id)[1].name == 'service 2'
|
||||
@@ -121,9 +121,9 @@ def test_get_all_services_for_user(service_factory, sample_user):
|
||||
|
||||
|
||||
def test_get_all_only_services_user_has_access_to(service_factory, sample_user):
|
||||
service_factory.get('service 1', sample_user)
|
||||
service_factory.get('service 2', sample_user)
|
||||
service_3 = service_factory.get('service 3', sample_user)
|
||||
service_factory.get('service 1', sample_user, email_from='service.1')
|
||||
service_factory.get('service 2', sample_user, email_from='service.2')
|
||||
service_3 = service_factory.get('service 3', sample_user, email_from='service.3')
|
||||
new_user = User(
|
||||
name='Test User',
|
||||
email_address='new_user@digital.cabinet-office.gov.uk',
|
||||
@@ -151,17 +151,17 @@ def test_get_service_by_id_returns_none_if_no_service(notify_db):
|
||||
|
||||
|
||||
def test_get_service_by_id_returns_service(service_factory):
|
||||
service = service_factory.get('testing')
|
||||
service = service_factory.get('testing', email_from='testing')
|
||||
assert dao_fetch_service_by_id(service.id).name == 'testing'
|
||||
|
||||
|
||||
def test_can_get_service_by_id_and_user(service_factory, sample_user):
|
||||
service = service_factory.get('service 1', sample_user)
|
||||
service = service_factory.get('service 1', sample_user, email_from='service.1')
|
||||
assert dao_fetch_service_by_id_and_user(service.id, sample_user.id).name == 'service 1'
|
||||
|
||||
|
||||
def test_cannot_get_service_by_id_and_owned_by_different_user(service_factory, sample_user):
|
||||
service1 = service_factory.get('service 1', sample_user)
|
||||
service1 = service_factory.get('service 1', sample_user, email_from='service.1')
|
||||
new_user = User(
|
||||
name='Test User',
|
||||
email_address='new_user@digital.cabinet-office.gov.uk',
|
||||
@@ -169,7 +169,7 @@ def test_cannot_get_service_by_id_and_owned_by_different_user(service_factory, s
|
||||
mobile_number='+447700900986'
|
||||
)
|
||||
save_model_user(new_user)
|
||||
service2 = service_factory.get('service 2', new_user)
|
||||
service2 = service_factory.get('service 2', new_user, email_from='service.2')
|
||||
assert dao_fetch_service_by_id_and_user(service1.id, sample_user.id).name == 'service 1'
|
||||
with pytest.raises(NoResultFound) as e:
|
||||
dao_fetch_service_by_id_and_user(service2.id, sample_user.id)
|
||||
|
||||
@@ -74,8 +74,8 @@ def test_update_template(sample_service):
|
||||
|
||||
|
||||
def test_get_all_templates_for_service(service_factory):
|
||||
service_1 = service_factory.get('service 1')
|
||||
service_2 = service_factory.get('service 2')
|
||||
service_1 = service_factory.get('service 1', email_from='service.1')
|
||||
service_2 = service_factory.get('service 2', email_from='service.2')
|
||||
|
||||
assert Template.query.count() == 2
|
||||
assert len(dao_get_all_templates_for_service(service_1.id)) == 1
|
||||
|
||||
@@ -115,8 +115,8 @@ def test_get_all_notifications_newest_first(notify_api, notify_db, notify_db_ses
|
||||
def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notify_db_session):
|
||||
with notify_api.test_request_context():
|
||||
with notify_api.test_client() as client:
|
||||
service_1 = create_sample_service(notify_db, notify_db_session, service_name="1")
|
||||
service_2 = create_sample_service(notify_db, notify_db_session, service_name="2")
|
||||
service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1')
|
||||
service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2')
|
||||
|
||||
create_sample_notification(notify_db, notify_db_session, service=service_2)
|
||||
|
||||
@@ -586,8 +586,8 @@ def test_should_not_allow_template_from_another_service(notify_api, service_fact
|
||||
with notify_api.test_client() as client:
|
||||
mocker.patch('app.celery.tasks.send_sms.apply_async')
|
||||
|
||||
service_1 = service_factory.get('service 1', user=sample_user)
|
||||
service_2 = service_factory.get('service 2', user=sample_user)
|
||||
service_1 = service_factory.get('service 1', user=sample_user, email_from='service.1')
|
||||
service_2 = service_factory.get('service 2', user=sample_user, email_from='service.2')
|
||||
|
||||
service_2_templates = dao_get_all_templates_for_service(service_id=service_2.id)
|
||||
data = {
|
||||
@@ -736,8 +736,10 @@ def test_should_not_allow_email_template_from_another_service(notify_api, servic
|
||||
with notify_api.test_client() as client:
|
||||
mocker.patch('app.celery.tasks.send_email.apply_async')
|
||||
|
||||
service_1 = service_factory.get('service 1', template_type='email', user=sample_user)
|
||||
service_2 = service_factory.get('service 2', template_type='email', user=sample_user)
|
||||
service_1 = service_factory.get('service 1', template_type='email', user=sample_user,
|
||||
email_from='service.1')
|
||||
service_2 = service_factory.get('service 2', template_type='email', user=sample_user,
|
||||
email_from='service.2')
|
||||
|
||||
service_2_templates = dao_get_all_templates_for_service(service_id=service_2.id)
|
||||
|
||||
|
||||
@@ -98,7 +98,7 @@ def test_get_api_keys_should_return_all_keys_for_service(notify_api, notify_db,
|
||||
with notify_api.test_client() as client:
|
||||
another_user = create_user(notify_db, notify_db_session, email='another@it.gov.uk')
|
||||
another_service = create_sample_service(notify_db, notify_db_session, service_name='another',
|
||||
user=another_user)
|
||||
user=another_user, email_from='another')
|
||||
create_sample_api_key(notify_db, notify_db_session, service=another_service)
|
||||
api_key2 = ApiKey(**{'service_id': sample_api_key.service_id, 'name': 'second_api_key'})
|
||||
api_key3 = ApiKey(**{'service_id': sample_api_key.service_id, 'name': 'third_api_key',
|
||||
|
||||
@@ -13,9 +13,9 @@ from tests.app.conftest import sample_user as create_sample_user
|
||||
def test_get_service_list(notify_api, service_factory):
|
||||
with notify_api.test_request_context():
|
||||
with notify_api.test_client() as client:
|
||||
service_factory.get('one')
|
||||
service_factory.get('two')
|
||||
service_factory.get('three')
|
||||
service_factory.get('one', email_from='one')
|
||||
service_factory.get('two', email_from='two')
|
||||
service_factory.get('three', email_from='three')
|
||||
auth_header = create_authorization_header(
|
||||
path='/service',
|
||||
method='GET'
|
||||
@@ -36,9 +36,9 @@ def test_get_service_list_by_user(notify_api, sample_user, service_factory):
|
||||
|
||||
with notify_api.test_request_context():
|
||||
with notify_api.test_client() as client:
|
||||
service_factory.get('one', sample_user)
|
||||
service_factory.get('two', sample_user)
|
||||
service_factory.get('three', sample_user)
|
||||
service_factory.get('one', sample_user, email_from='one')
|
||||
service_factory.get('two', sample_user, email_from='two')
|
||||
service_factory.get('three', sample_user, email_from='three')
|
||||
|
||||
auth_header = create_authorization_header(
|
||||
path='/service',
|
||||
@@ -67,9 +67,9 @@ def test_get_service_list_by_user_should_return_empty_list_if_no_services(notify
|
||||
)
|
||||
save_model_user(new_user)
|
||||
|
||||
service_factory.get('one', sample_user)
|
||||
service_factory.get('two', sample_user)
|
||||
service_factory.get('three', sample_user)
|
||||
service_factory.get('one', sample_user, email_from='one')
|
||||
service_factory.get('two', sample_user, email_from='two')
|
||||
service_factory.get('three', sample_user, email_from='three')
|
||||
|
||||
auth_header = create_authorization_header(
|
||||
path='/service',
|
||||
@@ -138,7 +138,7 @@ def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db):
|
||||
def test_get_service_by_id_and_user(notify_api, service_factory, sample_user):
|
||||
with notify_api.test_request_context():
|
||||
with notify_api.test_client() as client:
|
||||
service = service_factory.get('new service', sample_user)
|
||||
service = service_factory.get('new service', sample_user, email_from='new.service')
|
||||
auth_header = create_authorization_header(
|
||||
path='/service/{}'.format(service.id),
|
||||
method='GET'
|
||||
@@ -179,7 +179,8 @@ def test_create_service(notify_api, sample_user):
|
||||
'user_id': sample_user.id,
|
||||
'limit': 1000,
|
||||
'restricted': False,
|
||||
'active': False}
|
||||
'active': False,
|
||||
'email_from': 'created.service'}
|
||||
auth_header = create_authorization_header(
|
||||
path='/service',
|
||||
method='POST',
|
||||
@@ -333,7 +334,8 @@ def test_update_service(notify_api, sample_service):
|
||||
assert json_resp['data']['name'] == sample_service.name
|
||||
|
||||
data = {
|
||||
'name': 'updated service name'
|
||||
'name': 'updated service name',
|
||||
'email_from': 'updated.service.name'
|
||||
}
|
||||
|
||||
auth_header = create_authorization_header(
|
||||
@@ -350,6 +352,7 @@ def test_update_service(notify_api, sample_service):
|
||||
result = json.loads(resp.get_data(as_text=True))
|
||||
assert resp.status_code == 200
|
||||
assert result['data']['name'] == 'updated service name'
|
||||
assert result['data']['email_from'] == 'updated.service.name'
|
||||
|
||||
|
||||
def test_should_not_update_service_with_duplicate_name(notify_api,
|
||||
@@ -364,7 +367,8 @@ def test_should_not_update_service_with_duplicate_name(notify_api,
|
||||
notify_db,
|
||||
notify_db_session,
|
||||
service_name=service_name,
|
||||
user=sample_user)
|
||||
user=sample_user,
|
||||
email_from='another.name')
|
||||
data = {
|
||||
'name': service_name
|
||||
}
|
||||
@@ -483,7 +487,8 @@ def test_default_permissions_are_added_for_user_service(notify_api,
|
||||
'user_id': sample_user.id,
|
||||
'limit': 1000,
|
||||
'restricted': False,
|
||||
'active': False}
|
||||
'active': False,
|
||||
'email_from': 'created.service'}
|
||||
auth_header = create_authorization_header(
|
||||
path='/service',
|
||||
method='POST',
|
||||
|
||||
@@ -303,8 +303,8 @@ def test_should_get_only_templates_for_that_servcie(notify_api, service_factory)
|
||||
with notify_api.test_request_context():
|
||||
with notify_api.test_client() as client:
|
||||
|
||||
service_1 = service_factory.get('service 1')
|
||||
service_2 = service_factory.get('service 2')
|
||||
service_1 = service_factory.get('service 1', email_from='service.1')
|
||||
service_2 = service_factory.get('service 2', email_from='service.2')
|
||||
|
||||
auth_header_1 = create_authorization_header(
|
||||
path='/service/{}/template'.format(service_1.id),
|
||||
|
||||
Reference in New Issue
Block a user