From 992f9d78f931a28554419fea5fff6cd654a102ad Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 16 May 2016 16:16:14 +0100 Subject: [PATCH] There is a problem where columns on the templates table were not set. It is also discovered that columns that have a default value and use the version mixin must set the value when creating the db object before the insert otherwise the history table will be missing the default value. Updated the templates_history.created_by_id with a value where missing, using the current version of the template for this value. Update templates_history.archived to false. This is okay as we do not yet have a way to set this value to true. Removed the versions attribute from the TemplateSchema, there is not a need for this column. --- README.md | 4 ++ app/dao/templates_dao.py | 1 + app/schemas.py | 5 -- migrations/versions/0015_fix_template_data.py | 54 +++++++++++++++++++ tests/app/template/test_rest.py | 11 ++-- 5 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 migrations/versions/0015_fix_template_data.py diff --git a/README.md b/README.md index de0819f21..2ab8131e6 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,10 @@ export TWILIO_AUTH_TOKEN=[contact team member for auth token] export VERIFY_CODE_FROM_EMAIL_ADDRESS='no-reply@notify.works' export MMG_API_KEY=mmg=secret-key export MMG_FROM_NUMBER="MMG +export STATSD_ENABLED=True +export STATSD_HOST="localhost" +export STATSD_PORT=1000 +export STATSD_PREFIX="stats-prefix" "> environment.sh ``` diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index 09b35eb11..dbd822672 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -13,6 +13,7 @@ from app.dao.dao_utils import ( @version_class(Template) def dao_create_template(template): template.id = uuid.uuid4() # must be set now so version history model can use same id + template.archived = False db.session.add(template) diff --git a/app/schemas.py b/app/schemas.py index 7e8bcfbb6..6496797e7 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -114,11 +114,6 @@ class BaseTemplateSchema(BaseSchema): class TemplateSchema(BaseTemplateSchema): created_by = field_for(models.Template, 'created_by', required=True) - versions = fields.Method("template_versions", dump_only=True) - - def template_versions(self, template): - return [x.version for x in models.Template.get_history_model().query.filter_by( - id=template.id).options(load_only("version"))] @validates_schema def validate_type(self, data): diff --git a/migrations/versions/0015_fix_template_data.py b/migrations/versions/0015_fix_template_data.py new file mode 100644 index 000000000..9ca17a71b --- /dev/null +++ b/migrations/versions/0015_fix_template_data.py @@ -0,0 +1,54 @@ +"""empty message + +Revision ID: 0015_fix_template_data +Revises: 0014_add_template_version +Create Date: 2016-05-16 13:55:27.179748 + +""" + +# revision identifiers, used by Alembic. +revision = '0015_fix_template_data' +down_revision = '0014_add_template_version' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.get_bind() + query = 'update templates_history set created_by_id = ' \ + '(select created_by_id from templates where templates.id = templates_history.id) ' \ + 'where created_by_id is null' + op.execute(query) + op.execute('update templates_history set archived = False') + op.alter_column('api_keys_history', 'created_at', + existing_type=postgresql.TIMESTAMP(), + nullable=False) + op.alter_column('api_keys_history', 'created_by_id', + existing_type=postgresql.UUID(), + nullable=False) + op.alter_column('templates_history', 'archived', + existing_type=sa.BOOLEAN(), + nullable=False) + op.alter_column('templates_history', 'created_by_id', + existing_type=postgresql.UUID(), + nullable=False) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.alter_column('templates_history', 'created_by_id', + existing_type=postgresql.UUID(), + nullable=True) + op.alter_column('templates_history', 'archived', + existing_type=sa.BOOLEAN(), + nullable=True) + op.alter_column('api_keys_history', 'created_by_id', + existing_type=postgresql.UUID(), + nullable=True) + op.alter_column('api_keys_history', 'created_at', + existing_type=postgresql.TIMESTAMP(), + nullable=True) + ### end Alembic commands ### diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 7b5865895..cd6389b10 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -30,7 +30,7 @@ def test_should_create_a_new_sms_template_for_a_service(notify_api, sample_user, assert json_resp['data']['content'] == 'template content' assert json_resp['data']['service'] == str(sample_service.id) assert json_resp['data']['id'] - assert json_resp['data']['versions'] == [1] + assert json_resp['data']['version'] == 1 assert not json_resp['data']['subject'] @@ -60,7 +60,7 @@ def test_should_create_a_new_email_template_for_a_service(notify_api, sample_use assert json_resp['data']['content'] == 'template content' assert json_resp['data']['service'] == str(sample_service.id) assert json_resp['data']['subject'] == 'subject' - assert json_resp['data']['versions'] == [1] + assert json_resp['data']['version'] == 1 assert json_resp['data']['id'] @@ -220,6 +220,7 @@ def test_should_be_able_to_update_a_template(notify_api, sample_user, sample_ser assert create_response.status_code == 201 json_resp = json.loads(create_response.get_data(as_text=True)) assert json_resp['data']['name'] == 'my template' + assert json_resp['data']['version'] == 1 data = { 'content': 'my template has new content ', 'created_by': str(sample_user.id) @@ -236,7 +237,7 @@ def test_should_be_able_to_update_a_template(notify_api, sample_user, sample_ser assert update_response.status_code == 200 update_json_resp = json.loads(update_response.get_data(as_text=True)) assert update_json_resp['data']['content'] == 'my template has new content alert("foo")' - assert update_json_resp['data']['versions'] == [1, 2] + assert update_json_resp['data']['version'] == 2 def test_should_be_able_to_archive_template(notify_api, sample_user, sample_service, sample_template): @@ -310,9 +311,9 @@ def test_should_be_able_to_get_all_templates_for_a_service(notify_api, sample_us assert response.status_code == 200 update_json_resp = json.loads(response.get_data(as_text=True)) assert update_json_resp['data'][0]['name'] == 'my template 1' - assert update_json_resp['data'][0]['versions'] == [1] + assert update_json_resp['data'][0]['version'] == 1 assert update_json_resp['data'][1]['name'] == 'my template 2' - assert update_json_resp['data'][1]['versions'] == [1] + assert update_json_resp['data'][1]['version'] == 1 def test_should_get_only_templates_for_that_service(notify_api, sample_user, service_factory):