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