From c3e56d5d2d36cb282936745f57c1554cd6e99faa Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 20 Apr 2017 18:21:56 +0100 Subject: [PATCH] Revert "add DVLA organisations to API" --- app/__init__.py | 4 - app/celery/tasks.py | 3 +- app/dao/dvla_organisation_dao.py | 5 - app/dvla_organisation/__init__.py | 0 app/dvla_organisation/rest.py | 14 -- app/models.py | 18 --- app/schemas.py | 1 - migrations/versions/0072_add_dvla_orgs.py | 60 --------- tests/app/celery/test_tasks.py | 1 - tests/app/dao/test_services_dao.py | 4 - tests/app/dvla_organisation/__init__.py | 0 tests/app/dvla_organisation/test_rest.py | 13 -- tests/app/service/test_rest.py | 152 +++++++++++----------- tests/conftest.py | 3 +- 14 files changed, 79 insertions(+), 199 deletions(-) delete mode 100644 app/dao/dvla_organisation_dao.py delete mode 100644 app/dvla_organisation/__init__.py delete mode 100644 app/dvla_organisation/rest.py delete mode 100644 migrations/versions/0072_add_dvla_orgs.py delete mode 100644 tests/app/dvla_organisation/__init__.py delete mode 100644 tests/app/dvla_organisation/test_rest.py diff --git a/app/__init__.py b/app/__init__.py index 82bce0425..8e774cccb 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -90,7 +90,6 @@ def register_blueprint(application): from app.provider_details.rest import provider_details as provider_details_blueprint from app.spec.rest import spec as spec_blueprint from app.organisation.rest import organisation_blueprint - from app.dvla_organisation.rest import dvla_organisation_blueprint from app.delivery.rest import delivery_blueprint from app.notifications.receive_notifications import receive_notifications_blueprint from app.notifications.notifications_ses_callback import ses_callback_blueprint @@ -149,9 +148,6 @@ def register_blueprint(application): organisation_blueprint.before_request(requires_admin_auth) application.register_blueprint(organisation_blueprint, url_prefix='/organisation') - dvla_organisation_blueprint.before_request(requires_admin_auth) - application.register_blueprint(dvla_organisation_blueprint, url_prefix='/dvla_organisations') - letter_job.before_request(requires_admin_auth) application.register_blueprint(letter_job) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 464e3e1be..a7ca9665c 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -321,8 +321,7 @@ def create_dvla_file_contents(job_id): notification.template.__dict__, notification.personalisation, notification_reference=notification.reference, - contact_block=notification.service.letter_contact_block, - org_id=notification.service.dvla_organisation.id, + contact_block=notification.service.letter_contact_block )) for notification in dao_get_all_notifications_for_job(job_id) ) diff --git a/app/dao/dvla_organisation_dao.py b/app/dao/dvla_organisation_dao.py deleted file mode 100644 index 879671947..000000000 --- a/app/dao/dvla_organisation_dao.py +++ /dev/null @@ -1,5 +0,0 @@ -from app.models import DVLAOrganisation - - -def dao_get_dvla_organisations(): - return DVLAOrganisation.query.all() diff --git a/app/dvla_organisation/__init__.py b/app/dvla_organisation/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/app/dvla_organisation/rest.py b/app/dvla_organisation/rest.py deleted file mode 100644 index acf9e09e0..000000000 --- a/app/dvla_organisation/rest.py +++ /dev/null @@ -1,14 +0,0 @@ -from flask import Blueprint, jsonify - -from app.dao.dvla_organisation_dao import dao_get_dvla_organisations -from app.errors import register_errors - -dvla_organisation_blueprint = Blueprint('dvla_organisation', __name__) -register_errors(dvla_organisation_blueprint) - - -@dvla_organisation_blueprint.route('', methods=['GET']) -def get_dvla_organisations(): - return jsonify({ - org.id: org.name for org in dao_get_dvla_organisations() - }) diff --git a/app/models.py b/app/models.py index a2e92093f..f829ca429 100644 --- a/app/models.py +++ b/app/models.py @@ -113,16 +113,6 @@ class Organisation(db.Model): name = db.Column(db.String(255), nullable=True) -DVLA_ORG_HM_GOVERNMENT = '001' -DVLA_ORG_LAND_REGISTRY = '500' - - -class DVLAOrganisation(db.Model): - __tablename__ = 'dvla_organisation' - id = db.Column(db.String, primary_key=True) - name = db.Column(db.String(255), nullable=True) - - class Service(db.Model, Versioned): __tablename__ = 'services' @@ -157,14 +147,6 @@ class Service(db.Model, Versioned): sms_sender = db.Column(db.String(11), nullable=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'), - index=True, - nullable=False, - default=DVLA_ORG_HM_GOVERNMENT - ) - dvla_organisation = db.relationship('DVLAOrganisation') branding = db.Column( db.String(255), db.ForeignKey('branding_type.name'), diff --git a/app/schemas.py b/app/schemas.py index 58dd1f29c..203ba0934 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -177,7 +177,6 @@ class ServiceSchema(BaseSchema): created_by = field_for(models.Service, 'created_by', required=True) organisation = field_for(models.Service, 'organisation') branding = field_for(models.Service, 'branding') - dvla_organisation = field_for(models.Service, 'dvla_organisation') class Meta: model = models.Service diff --git a/migrations/versions/0072_add_dvla_orgs.py b/migrations/versions/0072_add_dvla_orgs.py deleted file mode 100644 index 7360c34d6..000000000 --- a/migrations/versions/0072_add_dvla_orgs.py +++ /dev/null @@ -1,60 +0,0 @@ -"""empty message - -Revision ID: 0072_add_dvla_orgs -Revises: 0071_add_job_error_state -Create Date: 2017-04-19 15:25:45.155886 - -""" - -# revision identifiers, used by Alembic. -revision = '0072_add_dvla_orgs' -down_revision = '0071_add_job_error_state' - -from alembic import op -import sqlalchemy as sa - - -def upgrade(): - ### commands auto generated by Alembic - please adjust! ### - op.create_table('dvla_organisation', - sa.Column('id', sa.String(), nullable=False), - sa.Column('name', sa.String(length=255), nullable=True), - sa.PrimaryKeyConstraint('id') - ) - - # insert initial values - HMG and Land Reg - op.execute(""" - INSERT INTO dvla_organisation VALUES - ('001', 'HM Government'), - ('500', 'Land Registry') - """) - - op.add_column('services', sa.Column('dvla_organisation_id', sa.String(), nullable=True)) - op.add_column('services_history', sa.Column('dvla_organisation_id', sa.String(), nullable=True)) - - # set everything to be HMG for now - op.execute("UPDATE services SET dvla_organisation_id = '001'") - op.execute("UPDATE services_history SET dvla_organisation_id = '001'") - - op.alter_column('services', 'dvla_organisation_id', nullable=False) - op.alter_column('services_history', 'dvla_organisation_id', nullable=False) - - op.create_index( - op.f('ix_services_dvla_organisation_id'), - 'services', - ['dvla_organisation_id'], - unique=False - ) - op.create_index( - op.f('ix_services_history_dvla_organisation_id'), - 'services_history', - ['dvla_organisation_id'], - unique=False - ) - - op.create_foreign_key(None, 'services', 'dvla_organisation', ['dvla_organisation_id'], ['id']) - -def downgrade(): - op.drop_column('services_history', 'dvla_organisation_id') - op.drop_column('services', 'dvla_organisation_id') - op.drop_table('dvla_organisation') diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 15d7e571b..0a960329b 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1038,7 +1038,6 @@ def test_create_dvla_file_contents(sample_letter_template, mocker): assert calls[1][1]['contact_block'] == 'London,\nSW1A 1AA' assert calls[0][1]['notification_reference'] == '1' assert calls[1][1]['notification_reference'] == '2' - assert calls[1][1]['org_id'] == '001' @freeze_time("2017-03-23 11:09:00.061258") diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index d37f8341b..1b90a8539 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -43,7 +43,6 @@ from app.models import ( InvitedUser, Service, BRANDING_GOVUK, - DVLA_ORG_HM_GOVERNMENT, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST @@ -78,7 +77,6 @@ def test_create_service(sample_user): assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.branding == BRANDING_GOVUK - assert service_db.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT assert service_db.research_mode is False assert service.active is True assert sample_user in service_db.users @@ -265,9 +263,7 @@ def test_create_service_creates_a_history_record_with_current_data(sample_user): assert sample_user.id == service_history.created_by_id assert service_from_db.created_by.id == service_history.created_by_id assert service_from_db.branding == BRANDING_GOVUK - assert service_from_db.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT assert service_history.branding == BRANDING_GOVUK - assert service_history.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT def test_update_service_creates_a_history_record_with_current_data(sample_user): diff --git a/tests/app/dvla_organisation/__init__.py b/tests/app/dvla_organisation/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/tests/app/dvla_organisation/test_rest.py b/tests/app/dvla_organisation/test_rest.py deleted file mode 100644 index e40c843bf..000000000 --- a/tests/app/dvla_organisation/test_rest.py +++ /dev/null @@ -1,13 +0,0 @@ -from flask import json - -from tests import create_authorization_header - - -def test_get_dvla_organisations(client): - auth_header = create_authorization_header() - - response = client.get('/dvla_organisations', headers=[auth_header]) - - assert response.status_code == 200 - dvla_organisations = json.loads(response.get_data(as_text=True)) - assert dvla_organisations == {'001': 'HM Government', '500': 'Land Registry'} diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 43edbc040..846d461b9 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -10,7 +10,7 @@ from freezegun import freeze_time 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, Organisation, DVLA_ORG_LAND_REGISTRY +from app.models import User, Organisation from tests import create_authorization_header from tests.app.conftest import ( sample_service as create_service, @@ -129,20 +129,21 @@ def test_get_service_list_should_return_empty_list_if_no_services(notify_api, no assert len(json_resp['data']) == 0 -def test_get_service_by_id(client, sample_service): - auth_header = create_authorization_header() - resp = client.get( - '/service/{}'.format(sample_service.id), - headers=[auth_header] - ) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) - 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']['branding'] == 'govuk' - assert json_resp['data']['dvla_organisation'] == '001' +def test_get_service_by_id(notify_api, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth_header = create_authorization_header() + resp = client.get( + '/service/{}'.format(sample_service.id), + headers=[auth_header] + ) + assert resp.status_code == 200 + json_resp = json.loads(resp.get_data(as_text=True)) + 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']['branding'] == 'govuk' def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db): @@ -190,40 +191,41 @@ def test_get_service_by_id_should_404_if_no_service_for_user(notify_api, sample_ assert json_resp['message'] == 'No result found' -def test_create_service(client, sample_user): - data = { - 'name': 'created service', - 'user_id': str(sample_user.id), - 'message_limit': 1000, - 'restricted': False, - 'active': False, - 'email_from': 'created.service', - '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(data), - headers=headers) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 201 - assert json_resp['data']['id'] - assert json_resp['data']['name'] == 'created service' - assert json_resp['data']['email_from'] == 'created.service' - assert not json_resp['data']['research_mode'] - assert json_resp['data']['dvla_organisation'] == '001' +def test_create_service(notify_api, sample_user): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = { + 'name': 'created service', + 'user_id': str(sample_user.id), + 'message_limit': 1000, + 'restricted': False, + 'active': False, + 'email_from': 'created.service', + '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(data), + headers=headers) + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 201 + assert json_resp['data']['id'] + assert json_resp['data']['name'] == 'created service' + assert json_resp['data']['email_from'] == 'created.service' + assert not json_resp['data']['research_mode'] - auth_header_fetch = create_authorization_header() + auth_header_fetch = create_authorization_header() - resp = client.get( - '/service/{}?user_id={}'.format(json_resp['data']['id'], sample_user.id), - headers=[auth_header_fetch] - ) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['data']['name'] == 'created service' - assert not json_resp['data']['research_mode'] - assert not json_resp['data']['can_send_letters'] + resp = client.get( + '/service/{}?user_id={}'.format(json_resp['data']['id'], sample_user.id), + headers=[auth_header_fetch] + ) + assert resp.status_code == 200 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['data']['name'] == 'created service' + assert not json_resp['data']['research_mode'] + assert not json_resp['data']['can_send_letters'] def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_uuid): @@ -369,41 +371,41 @@ def test_create_service_should_throw_duplicate_key_constraint_for_existing_email assert "Duplicate service name '{}'".format(service_name) in json_resp['message']['name'] -def test_update_service(client, notify_db, sample_service): +def test_update_service(notify_api, 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']['name'] == sample_service.name + with notify_api.test_request_context(): + with notify_api.test_client() as client: + 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 - data = { - 'name': 'updated service name', - 'email_from': 'updated.service.name', - 'created_by': str(sample_service.created_by.id), - 'organisation': str(org.id), - 'dvla_organisation': DVLA_ORG_LAND_REGISTRY - } + data = { + 'name': 'updated service name', + 'email_from': 'updated.service.name', + 'created_by': str(sample_service.created_by.id), + 'organisation': str(org.id) + } - auth_header = create_authorization_header() + 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']['name'] == 'updated service name' - assert result['data']['email_from'] == 'updated.service.name' - assert result['data']['organisation'] == str(org.id) - assert result['data']['dvla_organisation'] == DVLA_ORG_LAND_REGISTRY + 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']['name'] == 'updated service name' + assert result['data']['email_from'] == 'updated.service.name' + assert result['data']['organisation'] == str(org.id) def test_update_service_flags(notify_api, sample_service): diff --git a/tests/conftest.py b/tests/conftest.py index b08059725..41fec2d91 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -75,8 +75,7 @@ def notify_db_session(notify_db): "branding_type", "job_status", "provider_details_history", - "template_process_type", - "dvla_organisation"]: + "template_process_type"]: notify_db.engine.execute(tbl.delete()) notify_db.session.commit()