From c743e52fe8f472fa05bdff49f41e4701f4028520 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 4 Jul 2017 17:02:28 +0100
Subject: [PATCH 01/14] Add organisations model and create dao

---
 ...ganisation_dao.py => organisations_dao.py} |  7 +++++
 app/models.py                                 | 13 +++++++++
 app/organisation/rest.py                      | 27 ++++++++++++++++---
 tests/app/dao/test_organisations_dao.py       | 14 ++++++++++
 tests/app/db.py                               | 14 ++++++++++
 5 files changed, 72 insertions(+), 3 deletions(-)
 rename app/dao/{organisation_dao.py => organisations_dao.py} (56%)
 create mode 100644 tests/app/dao/test_organisations_dao.py

diff --git a/app/dao/organisation_dao.py b/app/dao/organisations_dao.py
similarity index 56%
rename from app/dao/organisation_dao.py
rename to app/dao/organisations_dao.py
index 804fd8a1d..b9046816f 100644
--- a/app/dao/organisation_dao.py
+++ b/app/dao/organisations_dao.py
@@ -1,3 +1,5 @@
+from app import db
+from app.dao.dao_utils import transactional
 from app.models import Organisation
 
 
@@ -7,3 +9,8 @@ def dao_get_organisations():
 
 def dao_get_organisation_by_id(org_id):
     return Organisation.query.filter_by(id=org_id).one()
+
+
+@transactional
+def dao_create_organisation(organisation):
+    db.session.add(organisation)
diff --git a/app/models.py b/app/models.py
index 8fda806b8..f1eb075f5 100644
--- a/app/models.py
+++ b/app/models.py
@@ -137,6 +137,19 @@ class Organisation(db.Model):
     logo = db.Column(db.String(255), nullable=True)
     name = db.Column(db.String(255), nullable=True)
 
+    @classmethod
+    def from_json(cls, data):
+        """
+        Assumption: data has been validated appropriately.
+
+        Returns a Service object based on the provided data. Deserialises created_by to created_by_id as marshmallow
+        would.
+        """
+        # validate json with marshmallow
+        fields = data.copy()
+
+        return cls(**fields)
+
 
 DVLA_ORG_HM_GOVERNMENT = '001'
 DVLA_ORG_LAND_REGISTRY = '500'
diff --git a/app/organisation/rest.py b/app/organisation/rest.py
index 1bdfbff74..22b0d3968 100644
--- a/app/organisation/rest.py
+++ b/app/organisation/rest.py
@@ -1,8 +1,12 @@
-from flask import Blueprint, jsonify
+from flask import Blueprint, jsonify, request
 
-from app.dao.organisation_dao import dao_get_organisations, dao_get_organisation_by_id
+from app.dao.organisations_dao import dao_get_organisations, dao_get_organisation_by_id, dao_create_organisation
 from app.schemas import organisation_schema
-from app.errors import register_errors
+from app.errors import (
+    InvalidRequest,
+    register_errors
+)
+from app.models import Organisation
 
 organisation_blueprint = Blueprint('organisation', __name__)
 register_errors(organisation_blueprint)
@@ -18,3 +22,20 @@ def get_organisations():
 def get_organisation_by_id(org_id):
     data = organisation_schema.dump(dao_get_organisation_by_id(org_id)).data
     return jsonify(organisation=data)
+
+
+@organisation_blueprint.route('', methods=['POST'])
+def post_organisation():
+    data = request.get_json()
+    if not data.get('name', None):
+        errors = {'name': ['Missing data for required field.']}
+        raise InvalidRequest(errors, status_code=400)
+
+    # validate json with marshmallow
+    organisation_schema.load(request.get_json())
+
+    # unpack valid json into service object
+    valid_organisation = Organisation.from_json(data)
+
+    dao_create_organisation(valid_organisation)
+    return jsonify(data=organisation_schema.dump(valid_organisation).data), 201
diff --git a/tests/app/dao/test_organisations_dao.py b/tests/app/dao/test_organisations_dao.py
new file mode 100644
index 000000000..5192653c5
--- /dev/null
+++ b/tests/app/dao/test_organisations_dao.py
@@ -0,0 +1,14 @@
+import pytest
+
+from app.dao.organisations_dao import dao_create_organisation
+from app.models import Organisation
+
+from tests.app.db import create_organisation
+
+
+def test_create_organisation(notify_db, notify_db_session):
+    organisation = create_organisation()
+
+    assert Organisation.query.count() == 1
+    organisation_from_db = Organisation.query.first()
+    assert organisation == organisation_from_db
diff --git a/tests/app/db.py b/tests/app/db.py
index c212a6612..286ba943c 100644
--- a/tests/app/db.py
+++ b/tests/app/db.py
@@ -13,6 +13,7 @@ from app.models import (
     ServicePermission,
     Job,
     InboundSms,
+    Organisation,
     EMAIL_TYPE,
     SMS_TYPE,
     KEY_TYPE_NORMAL,
@@ -23,6 +24,7 @@ from app.dao.templates_dao import dao_create_template
 from app.dao.services_dao import dao_create_service
 from app.dao.service_permissions_dao import dao_add_service_permission
 from app.dao.inbound_sms_dao import dao_create_inbound_sms
+from app.dao.organisations_dao import dao_create_organisation
 
 
 def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-office.gov.uk", state='active'):
@@ -225,3 +227,15 @@ def create_service_inbound_api(
                                             )
     save_service_inbound_api(service_inbound_api)
     return service_inbound_api
+
+
+def create_organisation(colour='blue', logo='test_x2.png', name='test_logo'):
+    data = {
+        'colour': colour,
+        'logo': logo,
+        'name': name
+    }
+    organisation = Organisation(**data)
+    dao_create_organisation(organisation)
+
+    return organisation

From 37df051c32cdec171b250e3b1f8e82820103ceb6 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Wed, 5 Jul 2017 11:17:03 +0100
Subject: [PATCH 02/14] Updated Organisations DAO

---
 app/dao/organisations_dao.py            |  5 ++++
 tests/app/conftest.py                   |  6 ++++-
 tests/app/dao/test_organisations_dao.py | 36 ++++++++++++++++++++++++-
 tests/app/db.py                         |  2 +-
 tests/app/organisation/test_rest.py     | 18 +++++++++++++
 5 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/app/dao/organisations_dao.py b/app/dao/organisations_dao.py
index b9046816f..f8cb22090 100644
--- a/app/dao/organisations_dao.py
+++ b/app/dao/organisations_dao.py
@@ -14,3 +14,8 @@ def dao_get_organisation_by_id(org_id):
 @transactional
 def dao_create_organisation(organisation):
     db.session.add(organisation)
+
+
+@transactional
+def dao_update_organisation(organisation):
+    db.session.add(organisation)
diff --git a/tests/app/conftest.py b/tests/app/conftest.py
index 4fb05d422..290b74f6b 100644
--- a/tests/app/conftest.py
+++ b/tests/app/conftest.py
@@ -39,7 +39,7 @@ from app.dao.invited_user_dao import save_invited_user
 from app.dao.provider_rates_dao import create_provider_rates
 from app.clients.sms.firetext import FiretextClient
 from tests import create_authorization_header
-from tests.app.db import create_user, create_template, create_notification
+from tests.app.db import create_user, create_template, create_notification, create_organisation
 
 
 @pytest.yield_fixture
@@ -974,6 +974,10 @@ def sample_provider_rate(notify_db, notify_db_session, valid_from=None, rate=Non
     )
 
 
+def sample_organisation(notify_db, notify_db_session):
+    create_organisation()
+
+
 @pytest.fixture
 def restore_provider_details(notify_db, notify_db_session):
     """
diff --git a/tests/app/dao/test_organisations_dao.py b/tests/app/dao/test_organisations_dao.py
index 5192653c5..f5e9ecd4b 100644
--- a/tests/app/dao/test_organisations_dao.py
+++ b/tests/app/dao/test_organisations_dao.py
@@ -1,6 +1,8 @@
 import pytest
 
-from app.dao.organisations_dao import dao_create_organisation
+from app.dao.organisations_dao import (
+    dao_create_organisation, dao_get_organisations, dao_get_organisation_by_id, dao_update_organisation
+)
 from app.models import Organisation
 
 from tests.app.db import create_organisation
@@ -12,3 +14,35 @@ def test_create_organisation(notify_db, notify_db_session):
     assert Organisation.query.count() == 1
     organisation_from_db = Organisation.query.first()
     assert organisation == organisation_from_db
+
+
+def test_get_organisations_gets_all_organisations(notify_db, notify_db_session):
+    create_organisation(name='test_org_1')
+    create_organisation(name='test_org_2')
+
+    organisations = dao_get_organisations()
+
+    assert len(organisations) == 2
+
+
+def test_get_organisation_by_id_gets_correct_organisation(notify_db, notify_db_session):
+    organisation = create_organisation()
+
+    organisation_from_db = dao_get_organisation_by_id(organisation.id)
+
+    assert organisation_from_db == organisation
+
+
+def test_update_organisation(notify_db, notify_db_session):
+    updated_name = 'new name'
+    organisation = create_organisation()
+
+    organisation_from_db = Organisation.query.first()
+    assert organisation.name != updated_name
+
+    organisation.name = updated_name
+
+    dao_update_organisation(organisation)
+    organisation_from_db = Organisation.query.first()
+
+    assert organisation_from_db.name == updated_name
diff --git a/tests/app/db.py b/tests/app/db.py
index 286ba943c..ea9e4cbdd 100644
--- a/tests/app/db.py
+++ b/tests/app/db.py
@@ -229,7 +229,7 @@ def create_service_inbound_api(
     return service_inbound_api
 
 
-def create_organisation(colour='blue', logo='test_x2.png', name='test_logo'):
+def create_organisation(colour='blue', logo='test_x2.png', name='test_org_1'):
     data = {
         'colour': colour,
         'logo': logo,
diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py
index f024edd31..918be97cb 100644
--- a/tests/app/organisation/test_rest.py
+++ b/tests/app/organisation/test_rest.py
@@ -37,3 +37,21 @@ def test_get_organisation_by_id(notify_api, notify_db, notify_db_session):
     assert organisation['logo'] == '/path/image.png'
     assert organisation['name'] == 'My Org'
     assert organisation['id'] == str(org.id)
+
+
+def test_create_organisation(client, notify_db, notify_db_session):
+    data = {
+        'name': 'test organisation',
+        'colour': '#0000ff',
+        'logo': '/images/test_x2.png'
+    }
+    auth_header = create_authorization_header()
+
+    response = client.post(
+        '/organisation',
+        headers=[('Content-Type', 'application/json'), auth_header],
+        data=json.dumps(data)
+    )
+    assert response.status_code == 201
+    json_resp = json.loads(response.get_data(as_text=True))
+    assert data['name'] == json_resp['data']['name']

From f76962ad4d8616a34eed2ef11620aab57e77f978 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Wed, 5 Jul 2017 13:37:28 +0100
Subject: [PATCH 03/14] Add versioning to Organisations

---
 app/dao/organisations_dao.py            |  4 +++-
 app/models.py                           |  5 ++---
 tests/app/dao/test_organisations_dao.py | 20 +++++++++++++++++++-
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/app/dao/organisations_dao.py b/app/dao/organisations_dao.py
index f8cb22090..c4f265c7f 100644
--- a/app/dao/organisations_dao.py
+++ b/app/dao/organisations_dao.py
@@ -1,5 +1,5 @@
 from app import db
-from app.dao.dao_utils import transactional
+from app.dao.dao_utils import transactional, version_class
 from app.models import Organisation
 
 
@@ -12,10 +12,12 @@ def dao_get_organisation_by_id(org_id):
 
 
 @transactional
+@version_class(Organisation)
 def dao_create_organisation(organisation):
     db.session.add(organisation)
 
 
 @transactional
+@version_class(Organisation)
 def dao_update_organisation(organisation):
     db.session.add(organisation)
diff --git a/app/models.py b/app/models.py
index f1eb075f5..8dd56c96e 100644
--- a/app/models.py
+++ b/app/models.py
@@ -130,7 +130,7 @@ class BrandingTypes(db.Model):
     name = db.Column(db.String(255), primary_key=True)
 
 
-class Organisation(db.Model):
+class Organisation(db.Model, Versioned):
     __tablename__ = 'organisation'
     id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4)
     colour = db.Column(db.String(7), nullable=True)
@@ -142,8 +142,7 @@ class Organisation(db.Model):
         """
         Assumption: data has been validated appropriately.
 
-        Returns a Service object based on the provided data. Deserialises created_by to created_by_id as marshmallow
-        would.
+        Returns a Organisation object based on the provided data. 
         """
         # validate json with marshmallow
         fields = data.copy()
diff --git a/tests/app/dao/test_organisations_dao.py b/tests/app/dao/test_organisations_dao.py
index f5e9ecd4b..75e2f40b6 100644
--- a/tests/app/dao/test_organisations_dao.py
+++ b/tests/app/dao/test_organisations_dao.py
@@ -38,7 +38,7 @@ def test_update_organisation(notify_db, notify_db_session):
     organisation = create_organisation()
 
     organisation_from_db = Organisation.query.first()
-    assert organisation.name != updated_name
+    assert organisation_from_db.name != updated_name
 
     organisation.name = updated_name
 
@@ -46,3 +46,21 @@ def test_update_organisation(notify_db, notify_db_session):
     organisation_from_db = Organisation.query.first()
 
     assert organisation_from_db.name == updated_name
+
+
+def test_create_organisation_creates_a_history_record_with_current_data(sample_user):
+    assert Organisation.query.count() == 0
+    assert Organisation.get_history_model().query.count() == 0
+    organisation = create_organisation()
+    assert Organisation.query.count() == 1
+    assert Organisation.get_history_model().query.count() == 1
+
+    organisation_from_db = Organisation.query.first()
+    organisation_history = Organisation.get_history_model().query.first()
+
+    assert organisation_from_db.id == organisation_history.id
+    assert organisation_from_db.name == organisation_history.name
+    assert organisation_from_db.version == 1
+    assert organisation_from_db.version == organisation_history.version
+    assert sample_user.id == organisation_history.created_by_id
+    assert organisation_from_db.created_by.id == organisation_history.created_by_id

From a6df96213bbc949b23cb16bd010e2d1a383f7c02 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Thu, 6 Jul 2017 10:56:03 +0100
Subject: [PATCH 04/14] Update model and migration script

---
 app/models.py                                 |  6 ++---
 .../versions/0105_change_logo_not_nullable.py | 27 +++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)
 create mode 100644 migrations/versions/0105_change_logo_not_nullable.py

diff --git a/app/models.py b/app/models.py
index 8dd56c96e..43e8b7ebe 100644
--- a/app/models.py
+++ b/app/models.py
@@ -130,11 +130,11 @@ class BrandingTypes(db.Model):
     name = db.Column(db.String(255), primary_key=True)
 
 
-class Organisation(db.Model, Versioned):
+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)
+    logo = db.Column(db.String(255), nullable=False)
     name = db.Column(db.String(255), nullable=True)
 
     @classmethod
@@ -142,7 +142,7 @@ class Organisation(db.Model, Versioned):
         """
         Assumption: data has been validated appropriately.
 
-        Returns a Organisation object based on the provided data. 
+        Returns a Organisation object based on the provided data.
         """
         # validate json with marshmallow
         fields = data.copy()
diff --git a/migrations/versions/0105_change_logo_not_nullable.py b/migrations/versions/0105_change_logo_not_nullable.py
new file mode 100644
index 000000000..09efbcb41
--- /dev/null
+++ b/migrations/versions/0105_change_logo_not_nullable.py
@@ -0,0 +1,27 @@
+"""empty message
+
+Revision ID: 0105_change_logo_not_nullable
+Revises: 0104_more_letter_orgs
+Create Date: 2017-07-06 10:14:35.188404
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '0105_change_logo_not_nullable'
+down_revision = '0104_more_letter_orgs'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+
+def upgrade():
+    op.alter_column('organisation', 'logo',
+               existing_type=sa.VARCHAR(length=255),
+               nullable=False)
+
+
+def downgrade():
+    op.alter_column('organisation', 'logo',
+               existing_type=sa.VARCHAR(length=255),
+               nullable=True)

From 2f8bc0feaeb9a3352e325c10cba2522736ca0e31 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Thu, 6 Jul 2017 10:58:48 +0100
Subject: [PATCH 05/14] Add create and update fns for organisation dao

---
 app/dao/organisations_dao.py            |  2 --
 tests/app/dao/test_organisations_dao.py | 38 ++++++++++++-------------
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/app/dao/organisations_dao.py b/app/dao/organisations_dao.py
index c4f265c7f..53acf5394 100644
--- a/app/dao/organisations_dao.py
+++ b/app/dao/organisations_dao.py
@@ -12,12 +12,10 @@ def dao_get_organisation_by_id(org_id):
 
 
 @transactional
-@version_class(Organisation)
 def dao_create_organisation(organisation):
     db.session.add(organisation)
 
 
 @transactional
-@version_class(Organisation)
 def dao_update_organisation(organisation):
     db.session.add(organisation)
diff --git a/tests/app/dao/test_organisations_dao.py b/tests/app/dao/test_organisations_dao.py
index 75e2f40b6..ada346de1 100644
--- a/tests/app/dao/test_organisations_dao.py
+++ b/tests/app/dao/test_organisations_dao.py
@@ -1,7 +1,10 @@
 import pytest
+from sqlalchemy.exc import IntegrityError
 
 from app.dao.organisations_dao import (
-    dao_create_organisation, dao_get_organisations, dao_get_organisation_by_id, dao_update_organisation
+    dao_create_organisation,
+    dao_get_organisations,
+    dao_get_organisation_by_id, dao_update_organisation
 )
 from app.models import Organisation
 
@@ -16,6 +19,21 @@ def test_create_organisation(notify_db, notify_db_session):
     assert organisation == organisation_from_db
 
 
+def test_create_organisation_without_name_or_colour_is_valid(notify_db, notify_db_session):
+    organisation = create_organisation(name=None, colour=None)
+
+    assert Organisation.query.count() == 1
+    organisation_from_db = Organisation.query.first()
+    assert organisation == organisation_from_db
+
+
+def test_create_organisation_without_logo_raises_error(notify_db, notify_db_session):
+    with pytest.raises(IntegrityError) as excinfo:
+        create_organisation(logo=None)
+    assert 'column "logo" violates not-null constraint' in str(excinfo.value)
+    assert Organisation.query.count() == 0
+
+
 def test_get_organisations_gets_all_organisations(notify_db, notify_db_session):
     create_organisation(name='test_org_1')
     create_organisation(name='test_org_2')
@@ -46,21 +64,3 @@ def test_update_organisation(notify_db, notify_db_session):
     organisation_from_db = Organisation.query.first()
 
     assert organisation_from_db.name == updated_name
-
-
-def test_create_organisation_creates_a_history_record_with_current_data(sample_user):
-    assert Organisation.query.count() == 0
-    assert Organisation.get_history_model().query.count() == 0
-    organisation = create_organisation()
-    assert Organisation.query.count() == 1
-    assert Organisation.get_history_model().query.count() == 1
-
-    organisation_from_db = Organisation.query.first()
-    organisation_history = Organisation.get_history_model().query.first()
-
-    assert organisation_from_db.id == organisation_history.id
-    assert organisation_from_db.name == organisation_history.name
-    assert organisation_from_db.version == 1
-    assert organisation_from_db.version == organisation_history.version
-    assert sample_user.id == organisation_history.created_by_id
-    assert organisation_from_db.created_by.id == organisation_history.created_by_id

From 76493a209c1f72f375fc4627db6ed3cfc8ef70f7 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Thu, 6 Jul 2017 11:02:13 +0100
Subject: [PATCH 06/14] Added POST for organisation rest

---
 app/organisation/rest.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/organisation/rest.py b/app/organisation/rest.py
index 22b0d3968..8d7756f65 100644
--- a/app/organisation/rest.py
+++ b/app/organisation/rest.py
@@ -27,8 +27,8 @@ def get_organisation_by_id(org_id):
 @organisation_blueprint.route('', methods=['POST'])
 def post_organisation():
     data = request.get_json()
-    if not data.get('name', None):
-        errors = {'name': ['Missing data for required field.']}
+    if not data.get('logo', None):
+        errors = {'logo': ['Missing data for required field.']}
         raise InvalidRequest(errors, status_code=400)
 
     # validate json with marshmallow

From 12e6cd0a8f767b1920f15d461e5c218439d104fe Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Fri, 7 Jul 2017 08:47:37 +0100
Subject: [PATCH 07/14] Add test for missing data

---
 tests/app/organisation/test_rest.py | 31 +++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py
index 918be97cb..ec9d3de58 100644
--- a/tests/app/organisation/test_rest.py
+++ b/tests/app/organisation/test_rest.py
@@ -55,3 +55,34 @@ def test_create_organisation(client, notify_db, notify_db_session):
     assert response.status_code == 201
     json_resp = json.loads(response.get_data(as_text=True))
     assert data['name'] == json_resp['data']['name']
+
+
+def test_create_organisation_without_logo_raises_error(client, notify_db, notify_db_session):
+    data = {
+        'name': 'test organisation',
+        'colour': '#0000ff',
+    }
+    auth_header = create_authorization_header()
+
+    response = client.post(
+        '/organisation',
+        headers=[('Content-Type', 'application/json'), auth_header],
+        data=json.dumps(data)
+    )
+    assert response.status_code == 400
+
+
+def test_create_organisation_without_name_or_colour_is_valid(client, notify_db, notify_db_session):
+    data = {
+        'name': None,
+        'colour': None,
+        'logo': 'images/text_x2.png'
+    }
+    auth_header = create_authorization_header()
+
+    response = client.post(
+        '/organisation',
+        headers=[('Content-Type', 'application/json'), auth_header],
+        data=json.dumps(data)
+    )
+    assert response.status_code == 400

From b814c5ff26675e4a7446be4c4393990245847c5f Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Fri, 7 Jul 2017 10:00:38 +0100
Subject: [PATCH 08/14] Refactor organisation rest to remove marshmallow

---
 app/models.py                           | 18 +++++++--------
 app/organisation/organisation_schema.py | 11 +++++++++
 app/organisation/rest.py                | 30 ++++++++++++-------------
 app/schemas.py                          |  7 ------
 tests/app/organisation/test_rest.py     |  6 ++---
 5 files changed, 37 insertions(+), 35 deletions(-)
 create mode 100644 app/organisation/organisation_schema.py

diff --git a/app/models.py b/app/models.py
index 43e8b7ebe..0e3079c80 100644
--- a/app/models.py
+++ b/app/models.py
@@ -137,17 +137,15 @@ class Organisation(db.Model):
     logo = db.Column(db.String(255), nullable=False)
     name = db.Column(db.String(255), nullable=True)
 
-    @classmethod
-    def from_json(cls, data):
-        """
-        Assumption: data has been validated appropriately.
+    def serialize(self):
+        serialized = {
+            "id": str(self.id),
+            "colour": self.colour,
+            "logo": self.logo,
+            "name": self.name,
+        }
 
-        Returns a Organisation object based on the provided data.
-        """
-        # validate json with marshmallow
-        fields = data.copy()
-
-        return cls(**fields)
+        return serialized
 
 
 DVLA_ORG_HM_GOVERNMENT = '001'
diff --git a/app/organisation/organisation_schema.py b/app/organisation/organisation_schema.py
new file mode 100644
index 000000000..f04e55d9f
--- /dev/null
+++ b/app/organisation/organisation_schema.py
@@ -0,0 +1,11 @@
+post_organisation_schema = {
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "description": "POST schema for getting organisation",
+    "type": "object",
+    "properties": {
+        "colour": {"type": ["string", "null"], "format": "string"},
+        "name": {"type": ["string", "null"], "minimum": 1},
+        "logo": {"type": ["string", "null"], "minimum": 1}
+    },
+    "required": ["logo"]
+}
diff --git a/app/organisation/rest.py b/app/organisation/rest.py
index 8d7756f65..a4260377a 100644
--- a/app/organisation/rest.py
+++ b/app/organisation/rest.py
@@ -1,12 +1,17 @@
 from flask import Blueprint, jsonify, request
 
-from app.dao.organisations_dao import dao_get_organisations, dao_get_organisation_by_id, dao_create_organisation
-from app.schemas import organisation_schema
+from app.dao.organisations_dao import (
+    dao_get_organisations,
+    dao_get_organisation_by_id,
+    dao_create_organisation
+)
 from app.errors import (
     InvalidRequest,
     register_errors
 )
 from app.models import Organisation
+from app.organisation.organisation_schema import post_organisation_schema
+from app.schema_validation import validate
 
 organisation_blueprint = Blueprint('organisation', __name__)
 register_errors(organisation_blueprint)
@@ -14,28 +19,23 @@ 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)
+    organisations = [o.serialize() for o in dao_get_organisations()]
+    return jsonify(organisations=organisations)
 
 
 @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)
+    organisation = dao_get_organisation_by_id(org_id)
+    return jsonify(organisation=organisation.serialize())
 
 
 @organisation_blueprint.route('', methods=['POST'])
 def post_organisation():
     data = request.get_json()
-    if not data.get('logo', None):
-        errors = {'logo': ['Missing data for required field.']}
-        raise InvalidRequest(errors, status_code=400)
 
-    # validate json with marshmallow
-    organisation_schema.load(request.get_json())
+    validate(data, post_organisation_schema)
 
-    # unpack valid json into service object
-    valid_organisation = Organisation.from_json(data)
+    organisation = Organisation(**data)
 
-    dao_create_organisation(valid_organisation)
-    return jsonify(data=organisation_schema.dump(valid_organisation).data), 201
+    dao_create_organisation(organisation)
+    return jsonify(data=organisation.serialize()), 201
diff --git a/app/schemas.py b/app/schemas.py
index bfdda4f28..50eeffb93 100644
--- a/app/schemas.py
+++ b/app/schemas.py
@@ -600,12 +600,6 @@ class EventSchema(BaseSchema):
         strict = True
 
 
-class OrganisationSchema(BaseSchema):
-    class Meta:
-        model = models.Organisation
-        strict = True
-
-
 class DaySchema(ma.Schema):
 
     class Meta:
@@ -658,7 +652,6 @@ service_history_schema = ServiceHistorySchema()
 api_key_history_schema = ApiKeyHistorySchema()
 template_history_schema = TemplateHistorySchema()
 event_schema = EventSchema()
-organisation_schema = OrganisationSchema()
 provider_details_schema = ProviderDetailsSchema()
 provider_details_history_schema = ProviderDetailsHistorySchema()
 day_schema = DaySchema()
diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py
index ec9d3de58..6e5b0b763 100644
--- a/tests/app/organisation/test_rest.py
+++ b/tests/app/organisation/test_rest.py
@@ -70,12 +70,12 @@ def test_create_organisation_without_logo_raises_error(client, notify_db, notify
         data=json.dumps(data)
     )
     assert response.status_code == 400
+    json_resp = json.loads(response.get_data(as_text=True))
+    assert json_resp['errors'][0]['message'] == "logo is a required property"
 
 
 def test_create_organisation_without_name_or_colour_is_valid(client, notify_db, notify_db_session):
     data = {
-        'name': None,
-        'colour': None,
         'logo': 'images/text_x2.png'
     }
     auth_header = create_authorization_header()
@@ -85,4 +85,4 @@ def test_create_organisation_without_name_or_colour_is_valid(client, notify_db,
         headers=[('Content-Type', 'application/json'), auth_header],
         data=json.dumps(data)
     )
-    assert response.status_code == 400
+    assert response.status_code == 201

From 3b1f229384d1723991fe43fffe7e2aa73fd96181 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Fri, 7 Jul 2017 14:48:00 +0100
Subject: [PATCH 09/14] Renamed migration script and refactor code

---
 app/dao/organisations_dao.py                              | 2 +-
 app/organisation/rest.py                                  | 7 ++-----
 ...o_not_nullable.py => 0106_change_logo_not_nullable.py} | 8 ++++----
 tests/app/conftest.py                                     | 6 +-----
 tests/app/dao/test_organisations_dao.py                   | 6 ++++--
 5 files changed, 12 insertions(+), 17 deletions(-)
 rename migrations/versions/{0105_change_logo_not_nullable.py => 0106_change_logo_not_nullable.py} (75%)

diff --git a/app/dao/organisations_dao.py b/app/dao/organisations_dao.py
index 53acf5394..f8cb22090 100644
--- a/app/dao/organisations_dao.py
+++ b/app/dao/organisations_dao.py
@@ -1,5 +1,5 @@
 from app import db
-from app.dao.dao_utils import transactional, version_class
+from app.dao.dao_utils import transactional
 from app.models import Organisation
 
 
diff --git a/app/organisation/rest.py b/app/organisation/rest.py
index a4260377a..630a1f8ef 100644
--- a/app/organisation/rest.py
+++ b/app/organisation/rest.py
@@ -1,14 +1,11 @@
 from flask import Blueprint, jsonify, request
 
 from app.dao.organisations_dao import (
+    dao_create_organisation,
     dao_get_organisations,
     dao_get_organisation_by_id,
-    dao_create_organisation
-)
-from app.errors import (
-    InvalidRequest,
-    register_errors
 )
+from app.errors import register_errors
 from app.models import Organisation
 from app.organisation.organisation_schema import post_organisation_schema
 from app.schema_validation import validate
diff --git a/migrations/versions/0105_change_logo_not_nullable.py b/migrations/versions/0106_change_logo_not_nullable.py
similarity index 75%
rename from migrations/versions/0105_change_logo_not_nullable.py
rename to migrations/versions/0106_change_logo_not_nullable.py
index 09efbcb41..1031a9092 100644
--- a/migrations/versions/0105_change_logo_not_nullable.py
+++ b/migrations/versions/0106_change_logo_not_nullable.py
@@ -1,14 +1,14 @@
 """empty message
 
-Revision ID: 0105_change_logo_not_nullable
-Revises: 0104_more_letter_orgs
+Revision ID: 0106_change_logo_not_nullable
+Revises: 0105_opg_letter_org
 Create Date: 2017-07-06 10:14:35.188404
 
 """
 
 # revision identifiers, used by Alembic.
-revision = '0105_change_logo_not_nullable'
-down_revision = '0104_more_letter_orgs'
+revision = '0106_change_logo_not_nullable'
+down_revision = '0105_opg_letter_org'
 
 from alembic import op
 import sqlalchemy as sa
diff --git a/tests/app/conftest.py b/tests/app/conftest.py
index 290b74f6b..4fb05d422 100644
--- a/tests/app/conftest.py
+++ b/tests/app/conftest.py
@@ -39,7 +39,7 @@ from app.dao.invited_user_dao import save_invited_user
 from app.dao.provider_rates_dao import create_provider_rates
 from app.clients.sms.firetext import FiretextClient
 from tests import create_authorization_header
-from tests.app.db import create_user, create_template, create_notification, create_organisation
+from tests.app.db import create_user, create_template, create_notification
 
 
 @pytest.yield_fixture
@@ -974,10 +974,6 @@ def sample_provider_rate(notify_db, notify_db_session, valid_from=None, rate=Non
     )
 
 
-def sample_organisation(notify_db, notify_db_session):
-    create_organisation()
-
-
 @pytest.fixture
 def restore_provider_details(notify_db, notify_db_session):
     """
diff --git a/tests/app/dao/test_organisations_dao.py b/tests/app/dao/test_organisations_dao.py
index ada346de1..b2464a9e1 100644
--- a/tests/app/dao/test_organisations_dao.py
+++ b/tests/app/dao/test_organisations_dao.py
@@ -35,12 +35,14 @@ def test_create_organisation_without_logo_raises_error(notify_db, notify_db_sess
 
 
 def test_get_organisations_gets_all_organisations(notify_db, notify_db_session):
-    create_organisation(name='test_org_1')
-    create_organisation(name='test_org_2')
+    org_1 = create_organisation(name='test_org_1')
+    org_2 = create_organisation(name='test_org_2')
 
     organisations = dao_get_organisations()
 
     assert len(organisations) == 2
+    assert org_1 == organisations[0]
+    assert org_2 == organisations[1]
 
 
 def test_get_organisation_by_id_gets_correct_organisation(notify_db, notify_db_session):

From af9091f2075b70e1c188e3dab56a491846bf19bb Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Mon, 10 Jul 2017 13:45:48 +0100
Subject: [PATCH 10/14] Refactor tests to use admin_request

---
 app/organisation/organisation_schema.py |  20 ++++-
 app/organisation/rest.py                |  21 ++++-
 tests/app/organisation/test_rest.py     | 115 ++++++++++++++----------
 3 files changed, 104 insertions(+), 52 deletions(-)

diff --git a/app/organisation/organisation_schema.py b/app/organisation/organisation_schema.py
index f04e55d9f..60b523057 100644
--- a/app/organisation/organisation_schema.py
+++ b/app/organisation/organisation_schema.py
@@ -1,11 +1,23 @@
-post_organisation_schema = {
+post_create_organisation_schema = {
     "$schema": "http://json-schema.org/draft-04/schema#",
     "description": "POST schema for getting organisation",
     "type": "object",
     "properties": {
-        "colour": {"type": ["string", "null"], "format": "string"},
-        "name": {"type": ["string", "null"], "minimum": 1},
-        "logo": {"type": ["string", "null"], "minimum": 1}
+        "colour": {"type": ["string", "null"]},
+        "name": {"type": ["string", "null"]},
+        "logo": {"type": ["string", "null"]}
     },
     "required": ["logo"]
 }
+
+post_update_organisation_schema = {
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "description": "POST schema for getting organisation",
+    "type": "object",
+    "properties": {
+        "colour": {"type": ["string", "null"]},
+        "name": {"type": ["string", "null"]},
+        "logo": {"type": ["string", "null"]}
+    },
+    "required": []
+}
diff --git a/app/organisation/rest.py b/app/organisation/rest.py
index 630a1f8ef..66318a3d5 100644
--- a/app/organisation/rest.py
+++ b/app/organisation/rest.py
@@ -2,12 +2,13 @@ from flask import Blueprint, jsonify, request
 
 from app.dao.organisations_dao import (
     dao_create_organisation,
+    dao_update_organisation,
     dao_get_organisations,
     dao_get_organisation_by_id,
 )
 from app.errors import register_errors
 from app.models import Organisation
-from app.organisation.organisation_schema import post_organisation_schema
+from app.organisation.organisation_schema import post_create_organisation_schema, post_update_organisation_schema
 from app.schema_validation import validate
 
 organisation_blueprint = Blueprint('organisation', __name__)
@@ -27,12 +28,26 @@ def get_organisation_by_id(org_id):
 
 
 @organisation_blueprint.route('', methods=['POST'])
-def post_organisation():
+def create_organisation():
     data = request.get_json()
 
-    validate(data, post_organisation_schema)
+    validate(data, post_create_organisation_schema)
 
     organisation = Organisation(**data)
 
     dao_create_organisation(organisation)
     return jsonify(data=organisation.serialize()), 201
+
+
+@organisation_blueprint.route('/<uuid:organisation_id>', methods=['POST'])
+def update_organisation(organisation_id):
+    fetched_organisation = dao_get_organisation_by_id(organisation_id)
+
+    data = request.get_json()
+
+    validate(data, post_update_organisation_schema)
+
+    for key in data.keys():
+        setattr(fetched_organisation, key, data[key])
+
+    return jsonify(data=fetched_organisation.serialize()), 200
diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py
index 6e5b0b763..ecd566d76 100644
--- a/tests/app/organisation/test_rest.py
+++ b/tests/app/organisation/test_rest.py
@@ -1,88 +1,113 @@
 from flask import json
+import pytest
 
 from app.models import Organisation
 
 from tests import create_authorization_header
 
 
-def test_get_organisations(notify_api, notify_db, notify_db_session):
+def test_get_organisations(admin_request, 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])
+    organisations = admin_request.get(
+        'organisation.get_organisations'
+    )['organisations']
 
-    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):
+def test_get_organisation_by_id(admin_request, 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])
+    response = admin_request.get(
+        'organisation.get_organisation_by_id',
+        _expected_status=200,
+        org_id=org.id
+    )
 
-    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)
+    assert set(response['organisation'].keys()) == {'colour', 'logo', 'name', 'id'}
+    assert response['organisation']['colour'] == '#FFFFFF'
+    assert response['organisation']['logo'] == '/path/image.png'
+    assert response['organisation']['name'] == 'My Org'
+    assert response['organisation']['id'] == str(org.id)
 
 
-def test_create_organisation(client, notify_db, notify_db_session):
+def test_post_create_organisation(admin_request, notify_db_session):
     data = {
         'name': 'test organisation',
         'colour': '#0000ff',
         'logo': '/images/test_x2.png'
     }
-    auth_header = create_authorization_header()
-
-    response = client.post(
-        '/organisation',
-        headers=[('Content-Type', 'application/json'), auth_header],
-        data=json.dumps(data)
+    response = admin_request.post(
+        'organisation.create_organisation',
+        _data=data,
+        _expected_status=201
     )
-    assert response.status_code == 201
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert data['name'] == json_resp['data']['name']
+    assert data['name'] == response['data']['name']
+    assert data['colour'] == response['data']['colour']
+    assert data['logo'] == response['data']['logo']
 
 
-def test_create_organisation_without_logo_raises_error(client, notify_db, notify_db_session):
+def test_post_create_organisation_without_logo_raises_error(admin_request, notify_db_session):
     data = {
         'name': 'test organisation',
         'colour': '#0000ff',
     }
-    auth_header = create_authorization_header()
-
-    response = client.post(
-        '/organisation',
-        headers=[('Content-Type', 'application/json'), auth_header],
-        data=json.dumps(data)
+    response = admin_request.post(
+        'organisation.create_organisation',
+        _data=data,
+        _expected_status=400
     )
-    assert response.status_code == 400
-    json_resp = json.loads(response.get_data(as_text=True))
-    assert json_resp['errors'][0]['message'] == "logo is a required property"
+    assert response['errors'][0]['message'] == "logo is a required property"
 
 
-def test_create_organisation_without_name_or_colour_is_valid(client, notify_db, notify_db_session):
+def test_post_create_organisation_without_name_or_colour_is_valid(admin_request, notify_db_session):
     data = {
         'logo': 'images/text_x2.png'
     }
-    auth_header = create_authorization_header()
-
-    response = client.post(
-        '/organisation',
-        headers=[('Content-Type', 'application/json'), auth_header],
-        data=json.dumps(data)
+    response = admin_request.post(
+        'organisation.create_organisation',
+        _data=data,
+        _expected_status=201
     )
-    assert response.status_code == 201
+
+    assert response['data']['logo'] == data['logo']
+    assert response['data']['name'] is None
+    assert response['data']['colour'] is None
+
+
+@pytest.mark.parametrize('data_update', [
+    ({'name': 'test organisation 1'}),
+    ({'logo': 'images/text_x3.png', 'colour': '#ffffff'})
+])
+def test_post_update_organisation_updates_field(admin_request, notify_db_session, data_update):
+    data = {
+        'name': 'test organisation',
+        'logo': 'images/text_x2.png'
+    }
+    response = admin_request.post(
+        'organisation.create_organisation',
+        _data=data,
+        _expected_status=201
+    )
+
+    org_id = response['data']['id']
+
+    response = admin_request.post(
+        'organisation.update_organisation',
+        _data=data_update,
+        organisation_id=org_id
+    )
+
+    organisations = Organisation.query.all()
+
+    assert len(organisations) == 1
+    assert str(organisations[0].id) == org_id
+    for key in data_update.keys():
+        assert getattr(organisations[0], key) == data_update[key]

From 1ca59c4b445c89969eb375d45d6b147952f52ee3 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 11 Jul 2017 12:38:30 +0100
Subject: [PATCH 11/14] Renamed migration script

---
 ...o_not_nullable.py => 0107_change_logo_not_nullable.py} | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
 rename migrations/versions/{0106_change_logo_not_nullable.py => 0107_change_logo_not_nullable.py} (75%)

diff --git a/migrations/versions/0106_change_logo_not_nullable.py b/migrations/versions/0107_change_logo_not_nullable.py
similarity index 75%
rename from migrations/versions/0106_change_logo_not_nullable.py
rename to migrations/versions/0107_change_logo_not_nullable.py
index 1031a9092..2c7ffe12d 100644
--- a/migrations/versions/0106_change_logo_not_nullable.py
+++ b/migrations/versions/0107_change_logo_not_nullable.py
@@ -1,14 +1,14 @@
 """empty message
 
-Revision ID: 0106_change_logo_not_nullable
-Revises: 0105_opg_letter_org
+Revision ID: 0107_change_logo_not_nullable
+Revises: 0106_null_noti_status
 Create Date: 2017-07-06 10:14:35.188404
 
 """
 
 # revision identifiers, used by Alembic.
-revision = '0106_change_logo_not_nullable'
-down_revision = '0105_opg_letter_org'
+revision = '0107_change_logo_not_nullable'
+down_revision = '0106_null_noti_status'
 
 from alembic import op
 import sqlalchemy as sa

From 106d59006b41b41c8b437b8434231ae36b3ddb89 Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 11 Jul 2017 17:04:29 +0100
Subject: [PATCH 12/14] Removed dao_update_organisation

---
 app/dao/organisations_dao.py            | 5 -----
 app/organisation/rest.py                | 4 +---
 tests/app/dao/test_organisations_dao.py | 9 ++++-----
 tests/app/organisation/test_rest.py     | 2 +-
 4 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/app/dao/organisations_dao.py b/app/dao/organisations_dao.py
index f8cb22090..b9046816f 100644
--- a/app/dao/organisations_dao.py
+++ b/app/dao/organisations_dao.py
@@ -14,8 +14,3 @@ def dao_get_organisation_by_id(org_id):
 @transactional
 def dao_create_organisation(organisation):
     db.session.add(organisation)
-
-
-@transactional
-def dao_update_organisation(organisation):
-    db.session.add(organisation)
diff --git a/app/organisation/rest.py b/app/organisation/rest.py
index 66318a3d5..2262d2cb0 100644
--- a/app/organisation/rest.py
+++ b/app/organisation/rest.py
@@ -2,7 +2,6 @@ from flask import Blueprint, jsonify, request
 
 from app.dao.organisations_dao import (
     dao_create_organisation,
-    dao_update_organisation,
     dao_get_organisations,
     dao_get_organisation_by_id,
 )
@@ -41,12 +40,11 @@ def create_organisation():
 
 @organisation_blueprint.route('/<uuid:organisation_id>', methods=['POST'])
 def update_organisation(organisation_id):
-    fetched_organisation = dao_get_organisation_by_id(organisation_id)
-
     data = request.get_json()
 
     validate(data, post_update_organisation_schema)
 
+    fetched_organisation = dao_get_organisation_by_id(organisation_id)
     for key in data.keys():
         setattr(fetched_organisation, key, data[key])
 
diff --git a/tests/app/dao/test_organisations_dao.py b/tests/app/dao/test_organisations_dao.py
index b2464a9e1..cb5b1466f 100644
--- a/tests/app/dao/test_organisations_dao.py
+++ b/tests/app/dao/test_organisations_dao.py
@@ -4,7 +4,7 @@ from sqlalchemy.exc import IntegrityError
 from app.dao.organisations_dao import (
     dao_create_organisation,
     dao_get_organisations,
-    dao_get_organisation_by_id, dao_update_organisation
+    dao_get_organisation_by_id
 )
 from app.models import Organisation
 
@@ -60,9 +60,8 @@ def test_update_organisation(notify_db, notify_db_session):
     organisation_from_db = Organisation.query.first()
     assert organisation_from_db.name != updated_name
 
-    organisation.name = updated_name
+    setattr(organisation_from_db, 'name', updated_name)
 
-    dao_update_organisation(organisation)
-    organisation_from_db = Organisation.query.first()
+    organisation_from_db_again = Organisation.query.first()
 
-    assert organisation_from_db.name == updated_name
+    assert organisation_from_db_again.name == updated_name
diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py
index ecd566d76..ea5926f5d 100644
--- a/tests/app/organisation/test_rest.py
+++ b/tests/app/organisation/test_rest.py
@@ -84,7 +84,7 @@ def test_post_create_organisation_without_name_or_colour_is_valid(admin_request,
 
 @pytest.mark.parametrize('data_update', [
     ({'name': 'test organisation 1'}),
-    ({'logo': 'images/text_x3.png', 'colour': '#ffffff'})
+    ({'logo': 'images/text_x3.png', 'colour': '#ffffff'}),
 ])
 def test_post_update_organisation_updates_field(admin_request, notify_db_session, data_update):
     data = {

From 0c77f2d0107911930a61cadcb85dcdcff9f38f8f Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Tue, 11 Jul 2017 18:18:23 +0100
Subject: [PATCH 13/14] Refactored dao_update_organisation

---
 app/dao/organisations_dao.py            |  7 +++++++
 app/organisation/rest.py                |  4 ++--
 tests/app/dao/test_organisations_dao.py | 16 ++++++++++------
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/app/dao/organisations_dao.py b/app/dao/organisations_dao.py
index b9046816f..71b8bfa85 100644
--- a/app/dao/organisations_dao.py
+++ b/app/dao/organisations_dao.py
@@ -14,3 +14,10 @@ def dao_get_organisation_by_id(org_id):
 @transactional
 def dao_create_organisation(organisation):
     db.session.add(organisation)
+
+
+@transactional
+def dao_update_organisation(organisation, **kwargs):
+    for key, value in kwargs.items():
+        setattr(organisation, key, value)
+    db.session.add(organisation)
diff --git a/app/organisation/rest.py b/app/organisation/rest.py
index 2262d2cb0..250c0d205 100644
--- a/app/organisation/rest.py
+++ b/app/organisation/rest.py
@@ -4,6 +4,7 @@ from app.dao.organisations_dao import (
     dao_create_organisation,
     dao_get_organisations,
     dao_get_organisation_by_id,
+    dao_update_organisation
 )
 from app.errors import register_errors
 from app.models import Organisation
@@ -45,7 +46,6 @@ def update_organisation(organisation_id):
     validate(data, post_update_organisation_schema)
 
     fetched_organisation = dao_get_organisation_by_id(organisation_id)
-    for key in data.keys():
-        setattr(fetched_organisation, key, data[key])
+    dao_update_organisation(fetched_organisation, **data)
 
     return jsonify(data=fetched_organisation.serialize()), 200
diff --git a/tests/app/dao/test_organisations_dao.py b/tests/app/dao/test_organisations_dao.py
index cb5b1466f..bc5d8c0a6 100644
--- a/tests/app/dao/test_organisations_dao.py
+++ b/tests/app/dao/test_organisations_dao.py
@@ -4,7 +4,8 @@ from sqlalchemy.exc import IntegrityError
 from app.dao.organisations_dao import (
     dao_create_organisation,
     dao_get_organisations,
-    dao_get_organisation_by_id
+    dao_get_organisation_by_id,
+    dao_update_organisation,
 )
 from app.models import Organisation
 
@@ -57,11 +58,14 @@ def test_update_organisation(notify_db, notify_db_session):
     updated_name = 'new name'
     organisation = create_organisation()
 
-    organisation_from_db = Organisation.query.first()
-    assert organisation_from_db.name != updated_name
+    organisations_1 = Organisation.query.all()
 
-    setattr(organisation_from_db, 'name', updated_name)
+    assert len(organisations_1) == 1
+    assert organisations_1[0].name != updated_name
 
-    organisation_from_db_again = Organisation.query.first()
+    dao_update_organisation(organisations_1[0], name=updated_name)
 
-    assert organisation_from_db_again.name == updated_name
+    organisations_2 = Organisation.query.all()
+
+    assert len(organisations_2) == 1
+    assert organisations_2[0].name == updated_name

From f53c0cfb8a207b929f1ba0c96b464753813bbfec Mon Sep 17 00:00:00 2001
From: Ken Tsang <ken.tsang@digital.cabinet-office.gov.uk>
Date: Wed, 12 Jul 2017 12:05:03 +0100
Subject: [PATCH 14/14] Rename migration script

---
 ...o_not_nullable.py => 0108_change_logo_not_nullable.py} | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
 rename migrations/versions/{0107_change_logo_not_nullable.py => 0108_change_logo_not_nullable.py} (74%)

diff --git a/migrations/versions/0107_change_logo_not_nullable.py b/migrations/versions/0108_change_logo_not_nullable.py
similarity index 74%
rename from migrations/versions/0107_change_logo_not_nullable.py
rename to migrations/versions/0108_change_logo_not_nullable.py
index 2c7ffe12d..7167d097b 100644
--- a/migrations/versions/0107_change_logo_not_nullable.py
+++ b/migrations/versions/0108_change_logo_not_nullable.py
@@ -1,14 +1,14 @@
 """empty message
 
-Revision ID: 0107_change_logo_not_nullable
-Revises: 0106_null_noti_status
+Revision ID: 0108_change_logo_not_nullable
+Revises: 0107_drop_template_stats
 Create Date: 2017-07-06 10:14:35.188404
 
 """
 
 # revision identifiers, used by Alembic.
-revision = '0107_change_logo_not_nullable'
-down_revision = '0106_null_noti_status'
+revision = '0108_change_logo_not_nullable'
+down_revision = '0107_drop_template_stats'
 
 from alembic import op
 import sqlalchemy as sa