mirror of
https://github.com/GSA/notifications-api.git
synced 2026-09-11 10:28:55 -04:00
remove free_sms_fragment_limit from service
* remove from model * still required when calling POST /service - we just call through from dao_create_service to add a new annual billing entry. * removed from POST /service/<id> update_service - if you want to update/add a new one, use POST /service/<id>/free-sms-fragment-limit * made sure tests create services with default 250k limit.
This commit is contained in:
@@ -61,12 +61,12 @@ def dao_get_all_free_sms_fragment_limit(service_id):
|
||||
).order_by(AnnualBilling.financial_year_start).all()
|
||||
|
||||
|
||||
def dao_insert_annual_billing(service):
|
||||
def dao_insert_annual_billing_for_this_year(service, free_sms_fragment_limit):
|
||||
"""
|
||||
This method is called from create_service which is wrapped in a transaction.
|
||||
"""
|
||||
annual_billing = AnnualBilling(
|
||||
free_sms_fragment_limit=service.free_sms_fragment_limit,
|
||||
free_sms_fragment_limit=free_sms_fragment_limit,
|
||||
financial_year_start=get_current_financial_year_start_year(),
|
||||
service=service,
|
||||
)
|
||||
|
||||
@@ -41,7 +41,7 @@ from app.models import (
|
||||
)
|
||||
from app.statsd_decorators import statsd
|
||||
from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc
|
||||
from app.dao.annual_billing_dao import dao_insert_annual_billing
|
||||
from app.dao.annual_billing_dao import dao_insert_annual_billing_for_this_year
|
||||
|
||||
DEFAULT_SERVICE_PERMISSIONS = [
|
||||
SMS_TYPE,
|
||||
@@ -155,7 +155,7 @@ def dao_fetch_service_by_id_and_user(service_id, user_id):
|
||||
|
||||
@transactional
|
||||
@version_class(Service)
|
||||
def dao_create_service(service, user, service_id=None, service_permissions=None):
|
||||
def dao_create_service(service, user, free_sms_fragment_limit, service_id=None, service_permissions=None):
|
||||
# the default property does not appear to work when there is a difference between the sqlalchemy schema and the
|
||||
# db schema (ie: during a migration), so we have to set sms_sender manually here. After the GOVUK sms_sender
|
||||
# migration is completed, this code should be able to be removed.
|
||||
@@ -163,9 +163,6 @@ def dao_create_service(service, user, service_id=None, service_permissions=None)
|
||||
if service_permissions is None:
|
||||
service_permissions = DEFAULT_SERVICE_PERMISSIONS
|
||||
|
||||
if service.free_sms_fragment_limit is None:
|
||||
service.free_sms_fragment_limit = current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
|
||||
|
||||
from app.dao.permissions_dao import permission_dao
|
||||
service.users.append(user)
|
||||
permission_dao.add_default_service_permissions_for_user(user, service)
|
||||
@@ -180,7 +177,7 @@ def dao_create_service(service, user, service_id=None, service_permissions=None)
|
||||
|
||||
# do we just add the default - or will we get a value from FE?
|
||||
insert_service_sms_sender(service, current_app.config['FROM_NUMBER'])
|
||||
dao_insert_annual_billing(service)
|
||||
dao_insert_annual_billing_for_this_year(service, free_sms_fragment_limit)
|
||||
db.session.add(service)
|
||||
|
||||
|
||||
|
||||
@@ -228,7 +228,6 @@ class Service(db.Model, Versioned):
|
||||
created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False)
|
||||
prefix_sms = db.Column(db.Boolean, nullable=True, default=True)
|
||||
organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True)
|
||||
free_sms_fragment_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=True)
|
||||
organisation = db.relationship('Organisation')
|
||||
dvla_organisation_id = db.Column(
|
||||
db.String,
|
||||
|
||||
@@ -203,7 +203,6 @@ class ServiceSchema(BaseSchema):
|
||||
organisation_type = field_for(models.Service, 'organisation_type')
|
||||
branding = field_for(models.Service, 'branding')
|
||||
dvla_organisation = field_for(models.Service, 'dvla_organisation')
|
||||
free_sms_fragment_limit = field_for(models.Service, 'free_sms_fragment_limit')
|
||||
permissions = fields.Method("service_permissions")
|
||||
override_flag = False
|
||||
reply_to_email_address = fields.Method(method_name="get_reply_to_email_address")
|
||||
|
||||
+9
-9
@@ -87,7 +87,6 @@ from app.schemas import (
|
||||
detailed_service_schema
|
||||
)
|
||||
from app.utils import pagination_links
|
||||
from app.billing.rest import update_free_sms_fragment_limit_data
|
||||
|
||||
service_blueprint = Blueprint('service', __name__)
|
||||
|
||||
@@ -154,19 +153,24 @@ def get_service_by_id(service_id):
|
||||
@service_blueprint.route('', methods=['POST'])
|
||||
def create_service():
|
||||
data = request.get_json()
|
||||
if not data.get('user_id', None):
|
||||
errors = {'user_id': ['Missing data for required field.']}
|
||||
errors = {
|
||||
required_field: ['Missing data for required field.']
|
||||
for required_field in ['user_id', 'free_sms_fragment_limit']
|
||||
if not data.get(required_field, None)
|
||||
}
|
||||
if errors:
|
||||
raise InvalidRequest(errors, status_code=400)
|
||||
|
||||
# validate json with marshmallow
|
||||
service_schema.load(request.get_json())
|
||||
service_schema.load(data)
|
||||
|
||||
user = get_user_by_id(data.pop('user_id', None))
|
||||
free_sms_fragment_limit = data.pop('free_sms_fragment_limit')
|
||||
|
||||
# unpack valid json into service object
|
||||
valid_service = Service.from_json(data)
|
||||
|
||||
dao_create_service(valid_service, user)
|
||||
dao_create_service(valid_service, user, free_sms_fragment_limit)
|
||||
return jsonify(data=service_schema.dump(valid_service).data), 201
|
||||
|
||||
|
||||
@@ -185,10 +189,6 @@ def update_service(service_id):
|
||||
update_dict.crown = org_type == 'central'
|
||||
dao_update_service(update_dict)
|
||||
|
||||
# bridging code between frontend is deployed and data has not been migrated yet. Can only update current year
|
||||
if 'free_sms_fragment_limit' in req_json:
|
||||
update_free_sms_fragment_limit_data(fetched_service.id, req_json['free_sms_fragment_limit'])
|
||||
|
||||
if service_going_live:
|
||||
send_notification_to_service_users(
|
||||
service_id=service_id,
|
||||
|
||||
@@ -162,13 +162,12 @@ def sample_service(
|
||||
'message_limit': limit,
|
||||
'restricted': restricted,
|
||||
'email_from': email_from,
|
||||
'created_by': user,
|
||||
'free_sms_fragment_limit': free_sms_fragment_limit
|
||||
'created_by': user
|
||||
}
|
||||
service = Service.query.filter_by(name=service_name).first()
|
||||
if not service:
|
||||
service = Service(**data)
|
||||
dao_create_service(service, user, service_permissions=permissions)
|
||||
dao_create_service(service, user, free_sms_fragment_limit, service_permissions=permissions)
|
||||
|
||||
if research_mode:
|
||||
service.research_mode = research_mode
|
||||
@@ -1002,7 +1001,12 @@ def notify_service(notify_db, notify_db_session):
|
||||
created_by=user,
|
||||
prefix_sms=False,
|
||||
)
|
||||
dao_create_service(service=service, service_id=current_app.config['NOTIFY_SERVICE_ID'], user=user)
|
||||
dao_create_service(
|
||||
service=service,
|
||||
service_id=current_app.config['NOTIFY_SERVICE_ID'],
|
||||
user=user,
|
||||
free_sms_fragment_limit=250000 # live central gov service
|
||||
)
|
||||
|
||||
data = {
|
||||
'service': service,
|
||||
|
||||
+6
-1
@@ -79,7 +79,12 @@ def create_service(
|
||||
prefix_sms=prefix_sms,
|
||||
)
|
||||
|
||||
dao_create_service(service, service.created_by, service_id, service_permissions=service_permissions)
|
||||
dao_create_service(
|
||||
service,
|
||||
service.created_by,
|
||||
free_sms_fragment_limit=250000,
|
||||
service_id=service_id,
|
||||
service_permissions=service_permissions)
|
||||
|
||||
service.active = active
|
||||
service.research_mode = research_mode
|
||||
|
||||
+30
-101
@@ -38,7 +38,6 @@ from tests.app.db import (
|
||||
create_service_with_defined_sms_sender
|
||||
)
|
||||
from tests.app.db import create_user
|
||||
from app.dao.date_util import get_current_financial_year_start_year
|
||||
|
||||
|
||||
def test_get_service_list(client, service_factory):
|
||||
@@ -133,16 +132,8 @@ def test_get_service_by_id(admin_request, sample_service):
|
||||
assert json_resp['data']['dvla_organisation'] == '001'
|
||||
assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER']
|
||||
assert json_resp['data']['prefix_sms'] is True
|
||||
|
||||
|
||||
def test_get_service_by_id_returns_free_sms_limit(admin_request, sample_service):
|
||||
json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id)
|
||||
assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
|
||||
|
||||
|
||||
def test_get_detailed_service_by_id_returns_free_sms_limit(admin_request, sample_service):
|
||||
json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id, detailed=True)
|
||||
assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
|
||||
# deprecated field, no longer exists
|
||||
assert 'free_sms_fragment_limit' not in json_resp['data']
|
||||
|
||||
|
||||
@pytest.mark.parametrize('detailed', [True, False])
|
||||
@@ -228,7 +219,9 @@ def test_create_service(client, sample_user):
|
||||
'restricted': False,
|
||||
'active': False,
|
||||
'email_from': 'created.service',
|
||||
'created_by': str(sample_user.id)}
|
||||
'created_by': str(sample_user.id),
|
||||
'free_sms_fragment_limit': 250000
|
||||
}
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
resp = client.post(
|
||||
@@ -243,8 +236,6 @@ def test_create_service(client, sample_user):
|
||||
assert not json_resp['data']['research_mode']
|
||||
assert json_resp['data']['dvla_organisation'] == '001'
|
||||
assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER']
|
||||
# TODO: Remove this after the new data is used
|
||||
assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
|
||||
|
||||
service_db = Service.query.get(json_resp['data']['id'])
|
||||
assert service_db.name == 'created service'
|
||||
@@ -274,7 +265,8 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_u
|
||||
'message_limit': 1000,
|
||||
'restricted': False,
|
||||
'active': False,
|
||||
'created_by': str(fake_uuid)
|
||||
'created_by': str(fake_uuid),
|
||||
'free_sms_fragment_limit': 250000
|
||||
}
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
@@ -288,7 +280,7 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_u
|
||||
assert 'Missing data for required field.' in json_resp['message']['user_id']
|
||||
|
||||
|
||||
def test_create_service_free_sms_fragment_limit_is_optional(client, sample_user):
|
||||
def test_create_service_free_sms_fragment_limit_is_not_optional(admin_request, sample_user):
|
||||
data1 = {
|
||||
'name': 'service 1',
|
||||
'user_id': str(sample_user.id),
|
||||
@@ -296,56 +288,16 @@ def test_create_service_free_sms_fragment_limit_is_optional(client, sample_user)
|
||||
'restricted': False,
|
||||
'active': False,
|
||||
'email_from': 'sample_user.email1',
|
||||
'created_by': str(sample_user.id),
|
||||
'free_sms_fragment_limit': 9999
|
||||
'created_by': str(sample_user.id)
|
||||
}
|
||||
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
resp = client.post(
|
||||
'/service',
|
||||
data=json.dumps(data1),
|
||||
headers=headers)
|
||||
json_resp = json.loads(resp.get_data(as_text=True))
|
||||
assert resp.status_code == 201
|
||||
|
||||
# Test data from the new annual billing table
|
||||
service_id = json_resp['data']['id']
|
||||
annual_billing = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start={}'
|
||||
.format(service_id, get_current_financial_year_start_year()),
|
||||
headers=[('Content-Type', 'application/json'), create_authorization_header()])
|
||||
json_resp = json.loads(annual_billing.get_data(as_text=True))
|
||||
assert json_resp['free_sms_fragment_limit'] == 9999
|
||||
# TODO: Remove this after the new data is used
|
||||
assert json_resp['free_sms_fragment_limit'] == 9999
|
||||
|
||||
data2 = {
|
||||
'name': 'service 2',
|
||||
'user_id': str(sample_user.id),
|
||||
'message_limit': 1000,
|
||||
'restricted': False,
|
||||
'active': False,
|
||||
'email_from': 'sample_user.email2',
|
||||
'created_by': str(sample_user.id),
|
||||
}
|
||||
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
resp = client.post(
|
||||
'/service',
|
||||
data=json.dumps(data2),
|
||||
headers=headers)
|
||||
json_resp = json.loads(resp.get_data(as_text=True))
|
||||
assert resp.status_code == 201
|
||||
# Test data from the new annual billing table
|
||||
service_id = json_resp['data']['id']
|
||||
annual_billing = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start={}'
|
||||
.format(service_id, get_current_financial_year_start_year()),
|
||||
headers=[('Content-Type', 'application/json'), create_authorization_header()])
|
||||
json_resp = json.loads(annual_billing.get_data(as_text=True))
|
||||
assert json_resp['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
|
||||
# TODO: Remove this after the new data is used
|
||||
assert json_resp['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
|
||||
json_resp = admin_request.post(
|
||||
'service.create_service',
|
||||
_data=data1,
|
||||
_expected_status=400
|
||||
)
|
||||
assert json_resp['result'] == 'error'
|
||||
assert 'Missing data for required field.' in json_resp['message']['free_sms_fragment_limit']
|
||||
|
||||
|
||||
def test_should_error_if_created_by_missing(notify_api, sample_user):
|
||||
@@ -357,7 +309,8 @@ def test_should_error_if_created_by_missing(notify_api, sample_user):
|
||||
'message_limit': 1000,
|
||||
'restricted': False,
|
||||
'active': False,
|
||||
'user_id': str(sample_user.id)
|
||||
'user_id': str(sample_user.id),
|
||||
'free_sms_fragment_limit': 250000
|
||||
}
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
@@ -384,7 +337,8 @@ def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(no
|
||||
'message_limit': 1000,
|
||||
'restricted': False,
|
||||
'active': False,
|
||||
'created_by': str(fake_uuid)
|
||||
'created_by': str(fake_uuid),
|
||||
'free_sms_fragment_limit': 250000
|
||||
}
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
@@ -402,7 +356,8 @@ def test_should_not_create_service_if_missing_data(notify_api, sample_user):
|
||||
with notify_api.test_request_context():
|
||||
with notify_api.test_client() as client:
|
||||
data = {
|
||||
'user_id': str(sample_user.id)
|
||||
'user_id': str(sample_user.id),
|
||||
'free_sms_fragment_limit': 250000
|
||||
}
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
@@ -430,7 +385,9 @@ def test_should_not_create_service_with_duplicate_name(notify_api,
|
||||
'restricted': False,
|
||||
'active': False,
|
||||
'email_from': 'sample.service2',
|
||||
'created_by': str(sample_user.id)}
|
||||
'created_by': str(sample_user.id),
|
||||
'free_sms_fragment_limit': 250000
|
||||
}
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
resp = client.post(
|
||||
@@ -456,7 +413,9 @@ def test_create_service_should_throw_duplicate_key_constraint_for_existing_email
|
||||
'restricted': False,
|
||||
'active': False,
|
||||
'email_from': 'first.service',
|
||||
'created_by': str(sample_user.id)}
|
||||
'created_by': str(sample_user.id),
|
||||
'free_sms_fragment_limit': 250000
|
||||
}
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
resp = client.post(
|
||||
@@ -603,37 +562,6 @@ def test_update_service_flags_will_remove_service_permissions(client, notify_db,
|
||||
assert set([p.permission for p in permissions]) == set([SMS_TYPE, EMAIL_TYPE])
|
||||
|
||||
|
||||
# TODO: Remove after new table is created and verified
|
||||
def test_update_service_free_sms_fragment_limit(client, notify_db, sample_service):
|
||||
org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League')
|
||||
notify_db.session.add(org)
|
||||
notify_db.session.commit()
|
||||
|
||||
auth_header = create_authorization_header()
|
||||
resp = client.get(
|
||||
'/service/{}'.format(sample_service.id),
|
||||
headers=[auth_header]
|
||||
)
|
||||
json_resp = json.loads(resp.get_data(as_text=True))
|
||||
assert resp.status_code == 200
|
||||
assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT']
|
||||
|
||||
data = {
|
||||
'free_sms_fragment_limit': 9999
|
||||
}
|
||||
|
||||
auth_header = create_authorization_header()
|
||||
|
||||
resp = client.post(
|
||||
'/service/{}'.format(sample_service.id),
|
||||
data=json.dumps(data),
|
||||
headers=[('Content-Type', 'application/json'), auth_header]
|
||||
)
|
||||
result = json.loads(resp.get_data(as_text=True))
|
||||
assert resp.status_code == 200
|
||||
assert result['data']['free_sms_fragment_limit'] == 9999
|
||||
|
||||
|
||||
def test_update_permissions_will_override_permission_flags(client, service_with_no_permissions):
|
||||
auth_header = create_authorization_header()
|
||||
|
||||
@@ -916,7 +844,8 @@ def test_default_permissions_are_added_for_user_service(notify_api,
|
||||
'restricted': False,
|
||||
'active': False,
|
||||
'email_from': 'created.service',
|
||||
'created_by': str(sample_user.id)}
|
||||
'created_by': str(sample_user.id),
|
||||
'free_sms_fragment_limit': 250000}
|
||||
auth_header = create_authorization_header()
|
||||
headers = [('Content-Type', 'application/json'), auth_header]
|
||||
resp = client.post(
|
||||
|
||||
Reference in New Issue
Block a user