diff --git a/README.md b/README.md index d2dbc9892..8d31a6be3 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Create a local environment.sh file containing the following: ``` echo " -export NOTIFY_API_ENVIRONMENT='config.Development' +export NOTIFY_ENVIRONMENT='development' export ADMIN_BASE_URL='http://localhost:6012' export ADMIN_CLIENT_SECRET='dev-notify-secret-key' export ADMIN_CLIENT_USER_NAME='dev-notify-admin' diff --git a/app/__init__.py b/app/__init__.py index be9f8aa95..af3f15c74 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -71,6 +71,7 @@ def create_app(app_name=None): from app.events.rest import events as events_blueprint 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 application.register_blueprint(service_blueprint, url_prefix='/service') application.register_blueprint(user_blueprint, url_prefix='/user') @@ -85,6 +86,7 @@ def create_app(app_name=None): application.register_blueprint(events_blueprint) application.register_blueprint(provider_details_blueprint, url_prefix='/provider-details') application.register_blueprint(spec_blueprint, url_prefix='/spec') + application.register_blueprint(organisation_blueprint, url_prefix='/organisation') return application diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py new file mode 100644 index 000000000..804fd8a1d --- /dev/null +++ b/app/dao/organisation_dao.py @@ -0,0 +1,9 @@ +from app.models import Organisation + + +def dao_get_organisations(): + return Organisation.query.all() + + +def dao_get_organisation_by_id(org_id): + return Organisation.query.filter_by(id=org_id).one() diff --git a/app/history_meta.py b/app/history_meta.py index be0c184b5..9cbbe07f1 100644 --- a/app/history_meta.py +++ b/app/history_meta.py @@ -53,7 +53,12 @@ def _history_mapper(local_mapper): col = col.copy() orig.info['history_copy'] = col col.unique = False - col.default = col.server_default = None + + # if the column is nullable, we could end up overwriting an on-purpose null value with a default. + # if it's not nullable, however, the default may be relied upon to correctly set values within the database, + # so we should preserve it + if col.nullable: + col.default = col.server_default = None return col properties = util.OrderedDict() @@ -201,7 +206,9 @@ def create_history(obj): elif isinstance(prop, RelationshipProperty): if hasattr(history, prop.key+'_id'): - data[prop.key+'_id'] = getattr(obj, prop.key).id + foreign_obj = getattr(obj, prop.key) + # if it's a nullable relationship, foreign_obj will be None, and we actually want to record that + data[prop.key+'_id'] = getattr(foreign_obj, 'id', None) if not obj.version: obj.version = 1 diff --git a/app/models.py b/app/models.py index f236841ac..e3aeb3d13 100644 --- a/app/models.py +++ b/app/models.py @@ -5,7 +5,7 @@ from sqlalchemy.dialects.postgresql import ( UUID, JSON ) -from sqlalchemy import UniqueConstraint +from sqlalchemy import UniqueConstraint, text from app.encryption import ( hashpw, @@ -74,6 +74,24 @@ user_to_service = db.Table( ) +BRANDING_GOVUK = 'govuk' +BRANDING_ORG = 'org' +BRANDING_BOTH = 'both' + + +class BrandingTypes(db.Model): + __tablename__ = 'branding_type' + name = db.Column(db.String(255), primary_key=True) + + +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) + + class Service(db.Model, Versioned): __tablename__ = 'services' @@ -104,6 +122,15 @@ class Service(db.Model, Versioned): created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) reply_to_email_address = db.Column(db.Text, index=False, unique=False, nullable=True) 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') + branding = db.Column( + db.String(255), + db.ForeignKey('branding_type.name'), + index=True, + nullable=False, + default=BRANDING_GOVUK + ) class ApiKey(db.Model, Versioned): diff --git a/app/organisation/__init__.py b/app/organisation/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/organisation/rest.py b/app/organisation/rest.py new file mode 100644 index 000000000..1bdfbff74 --- /dev/null +++ b/app/organisation/rest.py @@ -0,0 +1,20 @@ +from flask import Blueprint, jsonify + +from app.dao.organisation_dao import dao_get_organisations, dao_get_organisation_by_id +from app.schemas import organisation_schema +from app.errors import register_errors + +organisation_blueprint = Blueprint('organisation', __name__) +register_errors(organisation_blueprint) + + +@organisation_blueprint.route('', methods=['GET']) +def get_organisations(): + data = organisation_schema.dump(dao_get_organisations(), many=True).data + return jsonify(organisations=data) + + +@organisation_blueprint.route('/', methods=['GET']) +def get_organisation_by_id(org_id): + data = organisation_schema.dump(dao_get_organisation_by_id(org_id)).data + return jsonify(organisation=data) diff --git a/app/schemas.py b/app/schemas.py index 659ea996f..168faa02c 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -114,12 +114,13 @@ class ServiceSchema(BaseSchema): 'old_id', 'template_statistics', 'service_provider_stats', - 'service_notification_stats') + 'service_notification_stats', + 'organisation') strict = True @validates('sms_sender') def validate_sms_sender(self, value): - if value and not re.match('^[a-zA-Z0-9\s]+$', value): + if value and not re.match(r'^[a-zA-Z0-9\s]+$', value): raise ValidationError('Only alphanumeric characters allowed') @@ -136,7 +137,8 @@ class DetailedServiceSchema(BaseSchema): 'jobs', 'template_statistics', 'service_provider_stats', - 'service_notification_stats' + 'service_notification_stats', + 'organisation' ) @@ -438,6 +440,12 @@ class EventSchema(BaseSchema): strict = True +class OrganisationSchema(BaseSchema): + class Meta: + model = models.Organisation + strict = True + + class FromToDateSchema(ma.Schema): class Meta: @@ -531,6 +539,7 @@ service_history_schema = ServiceHistorySchema() api_key_history_schema = ApiKeyHistorySchema() template_history_schema = TemplateHistorySchema() event_schema = EventSchema() +organisation_schema = OrganisationSchema() from_to_date_schema = FromToDateSchema() provider_details_schema = ProviderDetailsSchema() week_aggregate_notification_statistics_schema = WeekAggregateNotificationStatisticsSchema() diff --git a/migrations/versions/0046_organisations_and_branding.py b/migrations/versions/0046_organisations_and_branding.py new file mode 100644 index 000000000..c03faf2ce --- /dev/null +++ b/migrations/versions/0046_organisations_and_branding.py @@ -0,0 +1,64 @@ +"""empty message + +Revision ID: 0046_organisations_and_branding +Revises: 0045_billable_units +Create Date: 2016-08-04 12:00:43.682610 + +""" + +# revision identifiers, used by Alembic. +revision = '0046_organisations_and_branding' +down_revision = '0045_billable_units' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + op.create_table('branding_type', + sa.Column('name', sa.String(length=255), nullable=False), + sa.PrimaryKeyConstraint('name') + ) + op.create_table('organisation', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('colour', sa.String(length=7), nullable=True), + sa.Column('logo', sa.String(length=255), nullable=True), + sa.Column('name', sa.String(length=255), nullable=True), + sa.PrimaryKeyConstraint('id') + ) + + op.add_column('services', sa.Column('branding', sa.String(length=255))) + op.add_column('services', sa.Column('organisation_id', postgresql.UUID(as_uuid=True))) + op.add_column('services_history', sa.Column('branding', sa.String(length=255))) + op.add_column('services_history', sa.Column('organisation_id', postgresql.UUID(as_uuid=True))) + + op.execute("INSERT INTO branding_type VALUES ('govuk'), ('org'), ('both')") + # insert UKVI data as initial test data. hex and crest pulled from alphagov/whitehall + op.execute("""INSERT INTO organisation VALUES ( + '9d25d02d-2915-4e98-874b-974e123e8536', + '#9325b2', + 'ho_crest_27px_x2.png', + 'UK Visas and Immigration' + )""") + op.execute("UPDATE services SET branding='govuk'") + op.execute("UPDATE services_history SET branding='govuk'") + + op.alter_column('services', 'branding', nullable=False) + op.alter_column('services_history', 'branding', nullable=False) + + op.create_index(op.f('ix_services_branding'), 'services', ['branding'], unique=False) + op.create_index(op.f('ix_services_organisation_id'), 'services', ['organisation_id'], unique=False) + op.create_index(op.f('ix_services_history_branding'), 'services_history', ['branding'], unique=False) + op.create_index(op.f('ix_services_history_organisation_id'), 'services_history', ['organisation_id'], unique=False) + + op.create_foreign_key(None, 'services', 'branding_type', ['branding'], ['name']) + op.create_foreign_key(None, 'services', 'organisation', ['organisation_id'], ['id']) + + +def downgrade(): + op.drop_column('services_history', 'organisation_id') + op.drop_column('services_history', 'branding') + op.drop_column('services', 'organisation_id') + op.drop_column('services', 'branding') + op.drop_table('organisation') + op.drop_table('branding_type') diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 943343b61..1af238b4d 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -35,7 +35,8 @@ from app.models import ( Permission, User, InvitedUser, - Service + Service, + BRANDING_GOVUK ) from tests.app.conftest import ( @@ -60,10 +61,13 @@ def test_create_service(sample_user): created_by=sample_user) dao_create_service(service, sample_user) assert Service.query.count() == 1 - assert Service.query.first().name == "service_name" - assert Service.query.first().id == service.id - assert not Service.query.first().research_mode - assert sample_user in Service.query.first().users + + service_db = Service.query.first() + assert service_db.name == "service_name" + assert service_db.id == service.id + assert service_db.branding == BRANDING_GOVUK + assert not service_db.research_mode + assert sample_user in service_db.users def test_cannot_create_two_services_with_same_name(sample_user): @@ -254,6 +258,8 @@ def test_create_service_creates_a_history_record_with_current_data(sample_user): assert service_from_db.version == service_history.version 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_history.branding == BRANDING_GOVUK def test_update_service_creates_a_history_record_with_current_data(sample_user): diff --git a/tests/app/organisation/__init__.py b/tests/app/organisation/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py new file mode 100644 index 000000000..f024edd31 --- /dev/null +++ b/tests/app/organisation/test_rest.py @@ -0,0 +1,39 @@ +from flask import json + +from app.models import Organisation + +from tests import create_authorization_header + + +def test_get_organisations(notify_api, notify_db, notify_db_session): + org1 = Organisation(colour='#FFFFFF', logo='/path/image.png', name='Org1') + org2 = Organisation(colour='#000000', logo='/path/other.png', name='Org2') + notify_db.session.add_all([org1, org2]) + notify_db.session.commit() + + with notify_api.test_request_context(), notify_api.test_client() as client: + auth_header = create_authorization_header() + response = client.get('/organisation', headers=[auth_header]) + + assert response.status_code == 200 + organisations = json.loads(response.get_data(as_text=True))['organisations'] + assert len(organisations) == 2 + assert {org['id'] for org in organisations} == {str(org1.id), str(org2.id)} + + +def test_get_organisation_by_id(notify_api, notify_db, notify_db_session): + org = Organisation(colour='#FFFFFF', logo='/path/image.png', name='My Org') + notify_db.session.add(org) + notify_db.session.commit() + + with notify_api.test_request_context(), notify_api.test_client() as client: + auth_header = create_authorization_header() + response = client.get('/organisation/{}'.format(org.id), headers=[auth_header]) + + assert response.status_code == 200 + organisation = json.loads(response.get_data(as_text=True))['organisation'] + assert set(organisation.keys()) == {'colour', 'logo', 'name', 'id'} + assert organisation['colour'] == '#FFFFFF' + assert organisation['logo'] == '/path/image.png' + assert organisation['name'] == 'My Org' + assert organisation['id'] == str(org.id) diff --git a/tests/conftest.py b/tests/conftest.py index 0f88fe667..cf1789d6e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -49,7 +49,7 @@ def notify_db_session(request, notify_db): def teardown(): notify_db.session.remove() for tbl in reversed(notify_db.metadata.sorted_tables): - if tbl.name not in ["provider_details", "key_types"]: + if tbl.name not in ["provider_details", "key_types", "branding_type"]: notify_db.engine.execute(tbl.delete()) notify_db.session.commit()