diff --git a/app/__init__.py b/app/__init__.py index d952d67b0..51c98808d 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -161,8 +161,6 @@ def register_blueprint(application): email_branding_blueprint.before_request(requires_admin_auth) application.register_blueprint(email_branding_blueprint, url_prefix='/email-branding') - # TODO: remove this route after admin is updated to refer to email branding - application.register_blueprint(email_branding_blueprint, url_prefix='/organisation') dvla_organisation_blueprint.before_request(requires_admin_auth) application.register_blueprint(dvla_organisation_blueprint, url_prefix='/dvla_organisations') diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 76d7c7e7a..ff819e30e 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -110,7 +110,7 @@ def process_job(job_id): TemplateClass = get_template_class(db_template.template_type) template = TemplateClass(db_template.__dict__) - current_app.logger.info("Starting job {} processing {} notifications".format(job_id, job.notification_count)) + current_app.logger.debug("Starting job {} processing {} notifications".format(job_id, job.notification_count)) for row_number, recipient, personalisation in RecipientCSV( s3.get_job_from_s3(str(service.id), str(job_id)), @@ -131,7 +131,7 @@ def job_complete(job, service, template_type, resumed=False, start=None): update_job_to_sent_to_dvla.apply_async([str(job.id)], queue=QueueNames.RESEARCH_MODE) else: build_dvla_file.apply_async([str(job.id)], queue=QueueNames.JOBS) - current_app.logger.info("send job {} to build-dvla-file in the {} queue".format(job.id, QueueNames.JOBS)) + current_app.logger.debug("send job {} to build-dvla-file in the {} queue".format(job.id, QueueNames.JOBS)) else: job.job_status = JOB_STATUS_FINISHED @@ -206,7 +206,7 @@ def save_sms(self, template = dao_get_template_by_id(notification['template'], version=notification['template_version']) if not service_allowed_to_send_to(notification['to'], service, key_type): - current_app.logger.info( + current_app.logger.debug( "SMS {} failed as restricted service".format(notification_id) ) return @@ -233,7 +233,7 @@ def save_sms(self, queue=QueueNames.SEND_SMS if not service.research_mode else QueueNames.RESEARCH_MODE ) - current_app.logger.info( + current_app.logger.debug( "SMS {} created at {} for job {}".format( saved_notification.id, saved_notification.created_at, @@ -283,7 +283,7 @@ def save_email(self, queue=QueueNames.SEND_EMAIL if not service.research_mode else QueueNames.RESEARCH_MODE ) - current_app.logger.info("Email {} created at {}".format(saved_notification.id, saved_notification.created_at)) + current_app.logger.debug("Email {} created at {}".format(saved_notification.id, saved_notification.created_at)) except SQLAlchemyError as e: handle_exception(self, notification, notification_id, e) @@ -340,7 +340,7 @@ def save_letter( else: update_notification_status_by_reference(saved_notification.reference, 'delivered') - current_app.logger.info("Letter {} created at {}".format(saved_notification.id, saved_notification.created_at)) + current_app.logger.debug("Letter {} created at {}".format(saved_notification.id, saved_notification.created_at)) except SQLAlchemyError as e: handle_exception(self, notification, notification_id, e) @@ -421,7 +421,7 @@ def update_letter_notifications_to_error(self, notification_references): } ) - current_app.logger.info("Updated {} letter notifications to technical-failure".format(updated_count)) + current_app.logger.debug("Updated {} letter notifications to technical-failure".format(updated_count)) def create_dvla_file_contents_for_job(job_id): @@ -553,7 +553,7 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id): }, timeout=60 ) - current_app.logger.info('send_inbound_sms_to_service sending {} to {}, response {}'.format( + current_app.logger.debug('send_inbound_sms_to_service sending {} to {}, response {}'.format( inbound_sms_id, inbound_api.url, response.status_code diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index a98be73a8..b272f116b 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -43,7 +43,7 @@ def send_sms_to_provider(notification): if notification.status == 'created': provider = provider_to_use(SMS_TYPE, notification.id, notification.international) - current_app.logger.info( + current_app.logger.debug( "Starting sending SMS {} to provider at {}".format(notification.id, datetime.utcnow()) ) template_model = dao_get_template_by_id(notification.template_id, notification.template_version) @@ -85,7 +85,7 @@ def send_sms_to_provider(notification): create_initial_notification_statistic_tasks(notification) - current_app.logger.info( + current_app.logger.debug( "SMS {} sent to provider {} at {}".format(notification.id, provider.get_name(), notification.sent_at) ) delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 @@ -99,7 +99,7 @@ def send_email_to_provider(notification): return if notification.status == 'created': provider = provider_to_use(EMAIL_TYPE, notification.id) - current_app.logger.info( + current_app.logger.debug( "Starting sending EMAIL {} to provider at {}".format(notification.id, datetime.utcnow()) ) template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__ @@ -140,7 +140,7 @@ def send_email_to_provider(notification): create_initial_notification_statistic_tasks(notification) - current_app.logger.info( + current_app.logger.debug( "Email {} sent to provider at {}".format(notification.id, notification.sent_at) ) delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 diff --git a/app/email_branding/rest.py b/app/email_branding/rest.py index 9ff339158..277d27714 100644 --- a/app/email_branding/rest.py +++ b/app/email_branding/rest.py @@ -21,17 +21,13 @@ register_errors(email_branding_blueprint) @email_branding_blueprint.route('', methods=['GET']) def get_email_branding_options(): email_branding_options = [o.serialize() for o in dao_get_email_branding_options()] - key = 'organisations' if request.path.startswith('/organisation') else 'email_branding' - return jsonify(**{key: email_branding_options}) return jsonify(email_branding=email_branding_options) @email_branding_blueprint.route('/', methods=['GET']) def get_email_branding_by_id(email_branding_id): email_branding = dao_get_email_branding_by_id(email_branding_id) - # TODO: remove this switch after admin is updated to refer to email branding - key = 'organisation' if request.path.startswith('/organisation') else 'email_branding' - return jsonify(**{key: email_branding.serialize()}) + return jsonify(email_branding=email_branding.serialize()) @email_branding_blueprint.route('', methods=['POST']) diff --git a/app/models.py b/app/models.py index 5a1682aff..580626b4b 100644 --- a/app/models.py +++ b/app/models.py @@ -148,25 +148,6 @@ class BrandingTypes(db.Model): name = db.Column(db.String(255), primary_key=True) -# TODO: remove this model after admin is updated to refer to email branding -class Organisation(db.Model): - __tablename__ = 'organisation' - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - colour = db.Column(db.String(7), nullable=True) - logo = db.Column(db.String(255), nullable=True) - name = db.Column(db.String(255), nullable=True) - - def serialize(self): - serialized = { - "id": str(self.id), - "colour": self.colour, - "logo": self.logo, - "name": self.name, - } - - return serialized - - class EmailBranding(db.Model): __tablename__ = 'email_branding' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) @@ -257,8 +238,6 @@ class Service(db.Model, Versioned): created_by = db.relationship('User') created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) prefix_sms = db.Column(db.Boolean, nullable=False, default=True) - organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True) - organisation = db.relationship('Organisation') dvla_organisation_id = db.Column( db.String, db.ForeignKey('dvla_organisation.id'), diff --git a/app/notifications/notifications_letter_callback.py b/app/notifications/notifications_letter_callback.py index ae722e017..97a6bebf2 100644 --- a/app/notifications/notifications_letter_callback.py +++ b/app/notifications/notifications_letter_callback.py @@ -47,7 +47,7 @@ def validate_schema(schema): @validate_schema(dvla_sns_callback_schema) def process_letter_response(): req_json = request.get_json(force=True) - current_app.logger.info('Received SNS callback: {}'.format(req_json)) + current_app.logger.debug('Received SNS callback: {}'.format(req_json)) if not autoconfirm_subscription(req_json): # The callback should have one record for an S3 Put Event. message = json.loads(req_json['Message']) diff --git a/app/notifications/notifications_sms_callback.py b/app/notifications/notifications_sms_callback.py index 4cf954840..950e68b7b 100644 --- a/app/notifications/notifications_sms_callback.py +++ b/app/notifications/notifications_sms_callback.py @@ -26,7 +26,7 @@ def process_mmg_response(): safe_to_log = data.copy() safe_to_log.pop("MSISDN") - current_app.logger.info( + current_app.logger.debug( "Full delivery response from {} for notification: {}\n{}".format(client_name, request.form.get('CID'), safe_to_log)) if errors: @@ -45,7 +45,7 @@ def process_firetext_response(): raise InvalidRequest(errors, status_code=400) safe_to_log = dict(request.form).copy() safe_to_log.pop('mobile') - current_app.logger.info( + current_app.logger.debug( "Full delivery response from {} for notification: {}\n{}".format(client_name, request.form.get('reference'), safe_to_log)) success, errors = process_sms_client_response(status=request.form.get('status'), diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 965fa7e6b..41378f8da 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -133,7 +133,7 @@ def send_notification_to_queue(notification, research_mode, queue=None): dao_delete_notifications_and_history_by_id(notification.id) raise - current_app.logger.info( + current_app.logger.debug( "{} {} sent to the {} queue for delivery".format(notification.notification_type, notification.id, queue)) diff --git a/app/notifications/receive_notifications.py b/app/notifications/receive_notifications.py index cd6eda598..b73730ece 100644 --- a/app/notifications/receive_notifications.py +++ b/app/notifications/receive_notifications.py @@ -59,7 +59,7 @@ def receive_mmg_sms(): tasks.send_inbound_sms_to_service.apply_async([str(inbound.id), str(service.id)], queue=QueueNames.NOTIFY) - current_app.logger.info( + current_app.logger.debug( '{} received inbound SMS with reference {} from MMG'.format(service.id, inbound.provider_reference)) return jsonify({ "status": "ok" @@ -96,7 +96,7 @@ def receive_firetext_sms(): statsd_client.incr('inbound.firetext.successful') tasks.send_inbound_sms_to_service.apply_async([str(inbound.id), str(service.id)], queue=QueueNames.NOTIFY) - current_app.logger.info( + current_app.logger.debug( '{} received inbound SMS with reference {} from Firetext'.format(service.id, inbound.provider_reference)) return jsonify({ "status": "ok" diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 750505834..ff0a9db3e 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -138,7 +138,7 @@ def send_notification(notification_type): research_mode=authenticated_service.research_mode, queue=queue_name) else: - current_app.logger.info("POST simulated notification for id: {}".format(notification_model.id)) + current_app.logger.debug("POST simulated notification for id: {}".format(notification_model.id)) notification_form.update({"template_version": template.version}) return jsonify( diff --git a/app/notifications/utils.py b/app/notifications/utils.py index 5f1443f09..bca551749 100644 --- a/app/notifications/utils.py +++ b/app/notifications/utils.py @@ -20,6 +20,6 @@ def confirm_subscription(confirmation_request): def autoconfirm_subscription(req_json): if req_json.get('Type') == 'SubscriptionConfirmation': - current_app.logger.info("SNS subscription confirmation url: {}".format(req_json['SubscribeURL'])) + current_app.logger.debug("SNS subscription confirmation url: {}".format(req_json['SubscribeURL'])) subscribed_topic = confirm_subscription(req_json) return subscribed_topic diff --git a/app/schemas.py b/app/schemas.py index a79eddf57..49c4eb7b9 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -203,8 +203,6 @@ class ServiceSchema(BaseSchema): branding = field_for(models.Service, 'branding') dvla_organisation = field_for(models.Service, 'dvla_organisation') permissions = fields.Method("service_permissions") - # TODO: remove this variable after admin is updated to refer to email branding - organisation = field_for(models.Service, 'organisation') email_branding = field_for(models.Service, 'email_branding') override_flag = False reply_to_email_address = fields.Method(method_name="get_reply_to_email_address") @@ -285,8 +283,6 @@ class DetailedServiceSchema(BaseSchema): 'template_statistics', 'service_provider_stats', 'service_notification_stats', - # TODO: remove this field after admin is updated to refer to email branding - 'organisation', 'email_branding', 'service_sms_senders', 'monthly_billing', diff --git a/app/service/rest.py b/app/service/rest.py index f3d4d8ac3..cab1cb645 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -184,11 +184,6 @@ def update_service(service_id): if org_type: service.crown = org_type == 'central' - # TODO: remove this block after admin is updated to refer to email branding - if 'organisation' in req_json: - org_id = req_json['organisation'] - service.email_branding = None if not org_id else EmailBranding.query.get(org_id) - if 'email_branding' in req_json: email_branding_id = req_json['email_branding'] service.email_branding = None if not email_branding_id else EmailBranding.query.get(email_branding_id) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index aa543e6c1..a619c0dc3 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -161,7 +161,7 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ queue=queue_name ) else: - current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) + current_app.logger.debug("POST simulated notification for id: {}".format(notification.id)) return notification diff --git a/migrations/versions/0162_remove_org.py b/migrations/versions/0162_remove_org.py new file mode 100644 index 000000000..be0c2e4c4 --- /dev/null +++ b/migrations/versions/0162_remove_org.py @@ -0,0 +1,45 @@ +""" + +Revision ID: 0162_remove_org +Revises: 0161_email_branding +Create Date: 2018-02-06 17:08:11.879844 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0162_remove_org' +down_revision = '0161_email_branding' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('services', 'organisation_id') + op.drop_column('services_history', 'organisation_id') + + op.drop_table('organisation') + + op.alter_column('service_email_branding', 'email_branding_id', nullable=False) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('services_history', sa.Column('organisation_id', postgresql.UUID(), autoincrement=False, nullable=True)) # noqa + op.add_column('services', sa.Column('organisation_id', postgresql.UUID(), autoincrement=False, nullable=True)) + + op.create_table( + 'organisation', + sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('colour', sa.VARCHAR(length=7), autoincrement=False, nullable=True), + sa.Column('logo', sa.VARCHAR(length=255), autoincrement=False, nullable=True), + sa.Column('name', sa.VARCHAR(length=255), autoincrement=False, nullable=True), + sa.PrimaryKeyConstraint('id', name='organisation_pkey') + ) + + op.create_index('ix_services_history_organisation_id', 'services_history', ['organisation_id'], unique=False) + op.create_foreign_key('services_organisation_id_fkey', 'services', 'organisation', ['organisation_id'], ['id']) + op.create_index('ix_services_organisation_id', 'services', ['organisation_id'], unique=False) + + op.alter_column('service_email_branding', 'email_branding_id', nullable=True) diff --git a/tests/app/email_branding/test_rest.py b/tests/app/email_branding/test_rest.py index d624dd2dd..19294e5ab 100644 --- a/tests/app/email_branding/test_rest.py +++ b/tests/app/email_branding/test_rest.py @@ -1,11 +1,7 @@ -import json - import pytest from app.models import EmailBranding -from tests import create_authorization_header - def test_get_email_branding_options(admin_request, notify_db, notify_db_session): email_branding1 = EmailBranding(colour='#FFFFFF', logo='/path/image.png', name='Org1') @@ -25,29 +21,6 @@ def test_get_email_branding_options(admin_request, notify_db, notify_db_session) } -def test_get_email_branding_options_from_old_endpoint(client, notify_db, notify_db_session): - email_branding1 = EmailBranding(colour='#FFFFFF', logo='/path/image.png', name='Org1') - email_branding2 = EmailBranding(colour='#000000', logo='/path/other.png', name='Org2') - notify_db.session.add_all([email_branding1, email_branding2]) - notify_db.session.commit() - - response = client.get( - '/organisation', - headers=[create_authorization_header()] - ) - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - - email_branding = json_resp['organisations'] - - assert len(email_branding) == 2 - assert { - email_branding['id'] for email_branding in email_branding - } == { - str(email_branding1.id), str(email_branding2.id) - } - - def test_get_email_branding_by_id(admin_request, notify_db, notify_db_session): email_branding = EmailBranding(colour='#FFFFFF', logo='/path/image.png', name='My Org') notify_db.session.add(email_branding) @@ -66,21 +39,6 @@ def test_get_email_branding_by_id(admin_request, notify_db, notify_db_session): assert response['email_branding']['id'] == str(email_branding.id) -def test_get_email_branding_by_id_from_old_endpoint(client, notify_db, notify_db_session): - email_branding = EmailBranding(colour='#FFFFFF', logo='/path/image.png', name='My Org') - notify_db.session.add(email_branding) - notify_db.session.commit() - - response = client.get( - '/organisation/{}'.format(email_branding.id), - headers=[create_authorization_header()] - ) - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - - assert json_resp['organisation']['id'] == str(email_branding.id) - - def test_post_create_email_branding(admin_request, notify_db_session): data = { 'name': 'test email_branding', diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 9a30d7e17..7a0b81031 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -16,7 +16,6 @@ from app.models import ( EmailBranding, InboundNumber, Notification, - Organisation, Service, ServiceEmailReplyTo, ServiceLetterContact, @@ -134,7 +133,7 @@ def test_get_service_by_id(admin_request, sample_service): assert json_resp['data']['name'] == sample_service.name assert json_resp['data']['id'] == str(sample_service.id) assert not json_resp['data']['research_mode'] - assert json_resp['data']['organisation'] is None + assert json_resp['data']['email_branding'] is None assert json_resp['data']['branding'] == 'govuk' assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] @@ -407,30 +406,17 @@ def test_create_service_should_throw_duplicate_key_constraint_for_existing_email def test_update_service(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() - # Need to set this up manually until org->email_branding migration is complete :( - brand = EmailBranding(id=org.id, colour='#000000', logo='justice-league.png', name='Justice League') + brand = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League') notify_db.session.add(brand) 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']['name'] == sample_service.name - assert json_resp['data']['organisation'] is None - assert json_resp['data']['email_branding'] is None + assert sample_service.email_branding is None data = { 'name': 'updated service name', 'email_from': 'updated.service.name', 'created_by': str(sample_service.created_by.id), - 'organisation': str(org.id), + 'email_branding': str(brand.id), 'dvla_organisation': DVLA_ORG_LAND_REGISTRY, 'organisation_type': 'foo', } @@ -446,46 +432,36 @@ def test_update_service(client, notify_db, sample_service): assert resp.status_code == 200 assert result['data']['name'] == 'updated service name' assert result['data']['email_from'] == 'updated.service.name' - assert result['data']['organisation'] == str(org.id) - assert result['data']['email_branding'] == str(org.id) + assert result['data']['email_branding'] == str(brand.id) assert result['data']['dvla_organisation'] == DVLA_ORG_LAND_REGISTRY assert result['data']['organisation_type'] == 'foo' -def test_update_service_remove_org(admin_request, notify_db, sample_service): - org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') - notify_db.session.add(org) - notify_db.session.commit() - # Need to set this up manually until org->email_branding migration is complete :( - brand = EmailBranding(id=org.id, colour='#000000', logo='justice-league.png', name='Justice League') - sample_service.organisation = org +def test_update_service_remove_email_branding(admin_request, notify_db, sample_service): + brand = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League') sample_service.email_branding = brand notify_db.session.commit() - resp = admin_request.post('service.update_service', service_id=sample_service.id, _data={'organisation': None}) - assert resp['data']['organisation'] is None + resp = admin_request.post( + 'service.update_service', + service_id=sample_service.id, + _data={'email_branding': None} + ) assert resp['data']['email_branding'] is None -def test_update_service_change_org(admin_request, notify_db, sample_service): - org1 = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') - org2 = Organisation(colour='#111111', logo='avengers.png', name='Avengers') - notify_db.session.add_all([org1, org2]) - notify_db.session.commit() - # Need to set this up manually until org->email_branding migration is complete :( - brand1 = EmailBranding(id=org1.id, colour='#000000', logo='justice-league.png', name='Justice League') - brand2 = EmailBranding(id=org2.id, colour='#111111', logo='avengers.png', name='Avengers') +def test_update_service_change_email_branding(admin_request, notify_db, sample_service): + brand1 = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League') + brand2 = EmailBranding(colour='#111111', logo='avengers.png', name='Avengers') notify_db.session.add_all([brand1, brand2]) - sample_service.organisation = org1 sample_service.email_branding = brand1 notify_db.session.commit() resp = admin_request.post( 'service.update_service', service_id=sample_service.id, - _data={'organisation': str(org2.id)} + _data={'email_branding': str(brand2.id)} ) - assert resp['data']['organisation'] == str(org2.id) assert resp['data']['email_branding'] == str(brand2.id)