Merge pull request #585 from alphagov/organisations

Organisations

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'

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
 

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()

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

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):

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('/<uuid:org_id>', 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)

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()

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')

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):

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)

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()