From 19df677315c96af90c2c4a9f6a0beaac64f055cb Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 2 Mar 2017 17:59:02 +0000 Subject: [PATCH 1/5] Add created_by to provider and provider history table --- app/models.py | 4 +++ .../0068_add_created_by_to_provider.py | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 migrations/versions/0068_add_created_by_to_provider.py diff --git a/app/models.py b/app/models.py index 69833084c..dc6f4d2a4 100644 --- a/app/models.py +++ b/app/models.py @@ -391,6 +391,8 @@ class ProviderDetails(db.Model): active = db.Column(db.Boolean, default=False, nullable=False) version = db.Column(db.Integer, default=1, nullable=False) updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) + created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=True) + created_by = db.relationship('User') class ProviderDetailsHistory(db.Model, HistoryModel): @@ -404,6 +406,8 @@ class ProviderDetailsHistory(db.Model, HistoryModel): active = db.Column(db.Boolean, nullable=False) version = db.Column(db.Integer, primary_key=True, nullable=False) updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) + created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=True) + created_by = db.relationship('User') JOB_STATUS_PENDING = 'pending' diff --git a/migrations/versions/0068_add_created_by_to_provider.py b/migrations/versions/0068_add_created_by_to_provider.py new file mode 100644 index 000000000..a296b2309 --- /dev/null +++ b/migrations/versions/0068_add_created_by_to_provider.py @@ -0,0 +1,36 @@ +"""empty message + +Revision ID: 0068_add_created_by_to_provider +Revises: 0067_service_contact_block +Create Date: 2017-03-02 17:47:17.586815 + +""" + +# revision identifiers, used by Alembic. +revision = '0068_add_created_by_to_provider' +down_revision = '0067_service_contact_block' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('provider_details', sa.Column('created_by_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.create_index(op.f('ix_provider_details_created_by_id'), 'provider_details', ['created_by_id'], unique=False) + op.create_foreign_key(None, 'provider_details', 'users', ['created_by_id'], ['id']) + op.add_column('provider_details_history', sa.Column('created_by_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.create_index(op.f('ix_provider_details_history_created_by_id'), 'provider_details_history', ['created_by_id'], unique=False) + op.create_foreign_key(None, 'provider_details_history', 'users', ['created_by_id'], ['id']) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint(None, 'provider_details_history', type_='foreignkey') + op.drop_index(op.f('ix_provider_details_history_created_by_id'), table_name='provider_details_history') + op.drop_column('provider_details_history', 'created_by_id') + op.drop_constraint(None, 'provider_details', type_='foreignkey') + op.drop_index(op.f('ix_provider_details_created_by_id'), table_name='provider_details') + op.drop_column('provider_details', 'created_by_id') + # ### end Alembic commands ### From 37341e7a6224a9917d3a8a1d0a16c32cc820f09c Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 2 Mar 2017 18:03:53 +0000 Subject: [PATCH 2/5] Updates: * Add notify user id in config * Add dao method to get provider history versions along with tests * BUG: Provider switching did not handle case where priorities were equal. This * adds a fix to properly cover this case along with tests --- app/config.py | 1 + app/dao/provider_details_dao.py | 52 ++++++++++--- app/provider_details/switch_providers.py | 29 ++++--- tests/app/celery/test_scheduled_tasks.py | 17 ++++- tests/app/dao/test_provider_details_dao.py | 88 ++++++++++++++++++++-- 5 files changed, 155 insertions(+), 32 deletions(-) diff --git a/app/config.py b/app/config.py index 07c1c419e..5cb6cf2a4 100644 --- a/app/config.py +++ b/app/config.py @@ -81,6 +81,7 @@ class Config(object): MAX_VERIFY_CODE_COUNT = 10 NOTIFY_SERVICE_ID = 'd6aa2c68-a2d9-4437-ab19-3ae8eb202553' + NOTIFY_USER_ID = '6af522d0-2915-4e52-83a3-3690455a5fe6' INVITATION_EMAIL_TEMPLATE_ID = '4f46df42-f795-4cc4-83bb-65ca312f49cc' SMS_CODE_TEMPLATE_ID = '36fb0730-6259-4da1-8a80-c8de22ad4246' EMAIL_VERIFY_CODE_TEMPLATE_ID = 'ece42649-22a8-4d06-b87f-d52d5d3f0a27' diff --git a/app/dao/provider_details_dao.py b/app/dao/provider_details_dao.py index edbcef54a..2992a0107 100644 --- a/app/dao/provider_details_dao.py +++ b/app/dao/provider_details_dao.py @@ -1,11 +1,12 @@ from datetime import datetime -from sqlalchemy import asc +from sqlalchemy import asc, desc from app.dao.dao_utils import transactional from app.provider_details.switch_providers import ( - provider_is_already_primary_or_inactive, - update_provider_priorities + provider_is_inactive, + provider_is_primary, + switch_providers ) from app.models import ProviderDetails, ProviderDetailsHistory from app import db @@ -41,6 +42,14 @@ def get_current_provider(notification_type): ).first() +def dao_get_provider_versions(provider_id): + return ProviderDetailsHistory.query.filter_by( + id=provider_id + ).order_by( + desc(ProviderDetailsHistory.version) + ).all() + + @transactional def dao_toggle_sms_provider(identifier): alternate_provider = get_alternative_sms_provider(identifier) @@ -49,18 +58,24 @@ def dao_toggle_sms_provider(identifier): @transactional def dao_switch_sms_provider_to_provider_with_identifier(identifier): - current_provider = get_current_provider('sms') new_provider = get_provider_details_by_identifier(identifier) + if provider_is_inactive(new_provider): + return - if current_provider.priority == new_provider.priority: - # Since both priorities are equal, set the current provider - # to the one that we want to switch from - current_provider = get_alternative_sms_provider(identifier) + # Check first to see if there is another provider with the same priority + # as this needs to be updated differently + conflicting_provider = dao_get_sms_provider_with_equal_priority(new_provider.identifier, new_provider.priority) + providers_to_update = [] - if not provider_is_already_primary_or_inactive(current_provider, new_provider, identifier): - update_provider_priorities(current_provider, new_provider) - dao_update_provider_details(current_provider) - dao_update_provider_details(new_provider) + if conflicting_provider: + providers_to_update = switch_providers(conflicting_provider, new_provider) + else: + current_provider = get_current_provider('sms') + if not provider_is_primary(current_provider, new_provider, identifier): + providers_to_update = switch_providers(current_provider, new_provider) + + for provider in providers_to_update: + dao_update_provider_details(provider) def get_provider_details_by_notification_type(notification_type): @@ -76,3 +91,16 @@ def dao_update_provider_details(provider_details): history = ProviderDetailsHistory.from_original(provider_details) db.session.add(provider_details) db.session.add(history) + + +def dao_get_sms_provider_with_equal_priority(identifier, priority): + provider = db.session.query(ProviderDetails).filter( + ProviderDetails.identifier != identifier, + ProviderDetails.notification_type == 'sms', + ProviderDetails.priority == priority, + ProviderDetails.active + ).order_by( + asc(ProviderDetails.priority) + ).first() + + return provider diff --git a/app/provider_details/switch_providers.py b/app/provider_details/switch_providers.py index 6d2a992cb..4ed279de3 100644 --- a/app/provider_details/switch_providers.py +++ b/app/provider_details/switch_providers.py @@ -1,32 +1,39 @@ from flask import current_app +from app.dao.users_dao import get_user_by_id -def provider_is_already_primary_or_inactive(current_provider, new_provider, identifier): - if current_provider.identifier == identifier: - current_app.logger.warning('Provider {} is already activated'.format(current_provider.display_name)) + +def provider_is_inactive(new_provider): + if not new_provider.active: + current_app.logger.warning('Cancelling switch to {} as they are inactive'.format( + new_provider.identifier, + )) return True - elif not new_provider.active: - current_app.logger.warning('Cancelling switch from {} to {} as {} is inactive'.format( - current_provider.identifier, - new_provider.identifier, - new_provider.identifier - )) + +def provider_is_primary(current_provider, new_provider, identifier): + if current_provider.identifier == identifier: + current_app.logger.warning('Provider {} is already activated'.format(current_provider.display_name)) return True return False -def update_provider_priorities(current_provider, new_provider): +def switch_providers(current_provider, new_provider): + # Automatic update so set as notify user + notify_user = get_user_by_id(current_app.config['NOTIFY_USER_ID']) + current_provider.created_by_id = new_provider.created_by_id = notify_user.id + # Swap priority to change primary provider if new_provider.priority > current_provider.priority: new_provider.priority, current_provider.priority = current_provider.priority, new_provider.priority - # Incease other provider priority if equal + # Increase other provider priority if equal elif new_provider.priority == current_provider.priority: current_provider.priority += 10 _print_provider_switch_logs(current_provider, new_provider) + return current_provider, new_provider def _print_provider_switch_logs(current_provider, new_provider): diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 06ab343fa..bc7113c7f 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -316,8 +316,11 @@ def test_switch_providers_on_slow_delivery_runs_if_config_set( def test_switch_providers_triggers_on_slow_notification_delivery( notify_api, - prepare_current_provider + mocker, + prepare_current_provider, + sample_user ): + mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) starting_provider = get_current_provider('sms') with set_config_values(notify_api, { @@ -335,8 +338,11 @@ def test_switch_providers_triggers_on_slow_notification_delivery( def test_switch_providers_on_slow_delivery_does_not_switch_if_already_switched( notify_api, - prepare_current_provider + mocker, + prepare_current_provider, + sample_user ): + mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) starting_provider = get_current_provider('sms') with set_config_values(notify_api, { @@ -356,7 +362,10 @@ def test_switch_providers_on_slow_delivery_does_not_switch_if_already_switched( def test_switch_providers_on_slow_delivery_does_not_switch_based_on_older_notifications( notify_api, - prepare_current_provider + mocker, + prepare_current_provider, + sample_user, + ): """ Assume we have three slow delivery notifications for the current provider x. This triggers @@ -367,7 +376,9 @@ def test_switch_providers_on_slow_delivery_does_not_switch_based_on_older_notifi based on these as they are old. We only want to look for slow notifications after the point at which we switched back to provider x. """ + mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) starting_provider = get_current_provider('sms') + with set_config_values(notify_api, { 'FUNCTIONAL_TEST_PROVIDER_SERVICE_ID': '7954469d-8c6d-43dc-b8f7-86be2d69f5f3', 'FUNCTIONAL_TEST_PROVIDER_SMS_TEMPLATE_ID': '331a63e6-f1aa-4588-ad3f-96c268788ae7' diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index 02344832b..51339370c 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/tests/app/dao/test_provider_details_dao.py @@ -1,9 +1,8 @@ import pytest from datetime import datetime - from freezegun import freeze_time -from sqlalchemy import desc +from sqlalchemy import asc, desc from app.models import ProviderDetails, ProviderDetailsHistory from app import clients @@ -15,7 +14,9 @@ from app.dao.provider_details_dao import ( get_provider_details_by_notification_type, dao_switch_sms_provider_to_provider_with_identifier, dao_toggle_sms_provider, - dao_update_provider_details + dao_update_provider_details, + dao_get_provider_versions, + dao_get_sms_provider_with_equal_priority ) @@ -148,9 +149,13 @@ def test_switch_sms_provider_to_inactive_provider_does_not_switch( def test_toggle_sms_provider_switches_provider( + mocker, restore_provider_details, - current_sms_provider + current_sms_provider, + sample_user + ): + mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) dao_toggle_sms_provider(current_sms_provider.identifier) new_provider = get_current_provider('sms') @@ -161,8 +166,11 @@ def test_toggle_sms_provider_switches_provider( def test_toggle_sms_provider_switches_when_provider_priorities_are_equal( - restore_provider_details + mocker, + restore_provider_details, + sample_user ): + mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) current_provider = get_current_provider('sms') new_provider = get_alternative_sms_provider(current_provider.identifier) @@ -178,9 +186,12 @@ def test_toggle_sms_provider_switches_when_provider_priorities_are_equal( def test_toggle_sms_provider_updates_provider_history( + mocker, restore_provider_details, - current_sms_provider + current_sms_provider, + sample_user ): + mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) provider_history_rows = ProviderDetailsHistory.query.filter( ProviderDetailsHistory.id == current_sms_provider.id ).order_by( @@ -197,3 +208,68 @@ def test_toggle_sms_provider_updates_provider_history( assert len(updated_provider_history_rows) - len(provider_history_rows) == 1 assert updated_provider_history_rows[0].version - provider_history_rows[0].version == 1 + + +def test_toggle_sms_provider_switches_provider_stores_notify_user_id( + restore_provider_details, + sample_user, + mocker +): + mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) + + current_provider = get_current_provider('sms') + dao_toggle_sms_provider(current_provider.identifier) + new_provider = get_current_provider('sms') + + assert current_provider.identifier != new_provider.identifier + assert new_provider.created_by.id == sample_user.id + assert new_provider.created_by_id == sample_user.id + + +def test_toggle_sms_provider_switches_provider_stores_notify_user_id_in_history( + restore_provider_details, + sample_user, + mocker +): + mocker.patch('app.provider_details.switch_providers.get_user_by_id', return_value=sample_user) + + old_provider = get_current_provider('sms') + dao_toggle_sms_provider(old_provider.identifier) + new_provider = get_current_provider('sms') + + old_provider_from_history = ProviderDetailsHistory.query.filter_by( + identifier=old_provider.identifier, + version=old_provider.version + ).order_by( + asc(ProviderDetailsHistory.priority) + ).first() + new_provider_from_history = ProviderDetailsHistory.query.filter_by( + identifier=new_provider.identifier, + version=new_provider.version + ).order_by( + asc(ProviderDetailsHistory.priority) + ).first() + + assert old_provider.version == old_provider_from_history.version + assert new_provider.version == new_provider_from_history.version + assert new_provider_from_history.created_by_id == sample_user.id + assert old_provider_from_history.created_by_id == sample_user.id + + +def test_can_get_all_provider_history(current_sms_provider): + assert len(dao_get_provider_versions(current_sms_provider.id)) == 1 + + +def test_get_sms_provider_with_equal_priority_returns_provider( + restore_provider_details +): + current_provider = get_current_provider('sms') + new_provider = get_alternative_sms_provider(current_provider.identifier) + + current_provider.priority = new_provider.priority + dao_update_provider_details(current_provider) + + conflicting_provider = \ + dao_get_sms_provider_with_equal_priority(current_provider.identifier, current_provider.priority) + + assert conflicting_provider From fe2ccb2222cbc3e8d74ea475a7e97b0ef3c5572e Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 2 Mar 2017 18:09:55 +0000 Subject: [PATCH 3/5] Remove unused import and ensure provider details are restored after test --- tests/app/conftest.py | 1 - tests/app/delivery/test_send_to_providers.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 23e7bcede..d48ba9069 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -9,7 +9,6 @@ from flask import current_app from app import db from app.models import ( - User, Service, Template, ApiKey, diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index ee7948292..2894d75e9 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -23,7 +23,7 @@ from app.models import ( from tests.app.db import create_service, create_template, create_notification -def test_should_return_highest_priority_active_provider(notify_db_session): +def test_should_return_highest_priority_active_provider(restore_provider_details): providers = provider_details_dao.get_provider_details_by_notification_type('sms') first = providers[0] From f6dbc6a06cf1dd66f447971fb8bc3082a7d777fa Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Thu, 2 Mar 2017 18:10:33 +0000 Subject: [PATCH 4/5] Updates: * Add endpoint to retrieve provider history * Remove marshmallow schemas when updating a provider * Include created by user when updating a provider --- app/provider_details/rest.py | 42 ++++++++++++++------ app/schemas.py | 14 +++++++ tests/app/provider_details/test_rest.py | 51 +++++++++++++++++++++---- 3 files changed, 87 insertions(+), 20 deletions(-) diff --git a/app/provider_details/rest.py b/app/provider_details/rest.py index 35b84c077..bdd093e22 100644 --- a/app/provider_details/rest.py +++ b/app/provider_details/rest.py @@ -1,13 +1,13 @@ from flask import Blueprint, jsonify, request -from app.schemas import provider_details_schema - +from app.schemas import provider_details_schema, provider_details_history_schema from app.dao.provider_details_dao import ( get_provider_details, get_provider_details_by_id, - dao_update_provider_details + dao_update_provider_details, + dao_get_provider_versions ) - +from app.dao.users_dao import get_user_by_id from app.errors import ( register_errors, InvalidRequest @@ -29,19 +29,37 @@ def get_provider_by_id(provider_details_id): return jsonify(provider_details=data) +@provider_details.route('//versions', methods=['GET']) +def get_provider_versions(provider_details_id): + versions = dao_get_provider_versions(provider_details_id) + data = provider_details_history_schema.dump( + versions, + many=True + ).data + return jsonify(data=data) + + @provider_details.route('/', methods=['POST']) def update_provider_details(provider_details_id): - fetched_provider_details = get_provider_details_by_id(provider_details_id) + valid_keys = {'priority', 'created_by', 'active'} + invalid_keys = request.get_json().keys() - valid_keys - current_data = dict(provider_details_schema.dump(fetched_provider_details).data.items()) - current_data.update(request.get_json()) - update_dict = provider_details_schema.load(current_data).data - - invalid_keys = {'identifier', 'version', 'updated_at'} & set(key for key in request.get_json().keys()) if invalid_keys: message = "Not permitted to be updated" errors = {key: [message] for key in invalid_keys} raise InvalidRequest(errors, status_code=400) - dao_update_provider_details(update_dict) - return jsonify(provider_details=provider_details_schema.dump(fetched_provider_details).data), 200 + provider = get_provider_details_by_id(provider_details_id) + req_json = request.get_json() + + # Handle created_by differently due to how history entry is created + if 'created_by' in req_json: + user = get_user_by_id(req_json['created_by']) + provider.created_by_id = user.id + req_json.pop('created_by') + + for key in req_json: + setattr(provider, key, req_json[key]) + dao_update_provider_details(provider) + + return jsonify(provider_details=provider_details_schema.dump(provider).data), 200 diff --git a/app/schemas.py b/app/schemas.py index ee3b65330..fa1523f9e 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -155,12 +155,25 @@ class UserUpdatePasswordSchema(BaseSchema): class ProviderDetailsSchema(BaseSchema): + created_by_user = fields.Nested( + UserSchema, + attribute='created_by', + dump_to='created_by', + only=['id', 'name', 'email_address'], + dump_only=True + ) + class Meta: model = models.ProviderDetails exclude = ("provider_rates", "provider_stats") strict = True +class ProviderDetailsHistorySchema(ProviderDetailsSchema): + class Meta: + model = models.ProviderDetailsHistory + + class ServiceSchema(BaseSchema): created_by = field_for(models.Service, 'created_by', required=True) @@ -605,5 +618,6 @@ template_history_schema = TemplateHistorySchema() event_schema = EventSchema() organisation_schema = OrganisationSchema() provider_details_schema = ProviderDetailsSchema() +provider_details_history_schema = ProviderDetailsHistorySchema() day_schema = DaySchema() unarchived_template_schema = UnarchivedTemplateSchema() diff --git a/tests/app/provider_details/test_rest.py b/tests/app/provider_details/test_rest.py index 63ffd0012..15e2381be 100644 --- a/tests/app/provider_details/test_rest.py +++ b/tests/app/provider_details/test_rest.py @@ -1,7 +1,7 @@ import pytest from flask import json -from app.models import ProviderDetails +from app.models import ProviderDetails, ProviderDetailsHistory from tests import create_authorization_header @@ -45,7 +45,9 @@ def test_get_provider_details_contains_correct_fields(client, notify_db): ) json_resp = json.loads(response.get_data(as_text=True))['provider_details'] allowed_keys = { - "id", "display_name", "identifier", "priority", 'notification_type', "active", "version", "updated_at" + "id", "created_by", "display_name", + "identifier", "priority", 'notification_type', + "active", "version", "updated_at" } assert allowed_keys == set(json_resp[0].keys()) @@ -92,13 +94,46 @@ def test_should_be_able_to_update_status(client, restore_provider_details): def test_should_not_be_able_to_update_disallowed_fields(client, restore_provider_details, field, value): provider = ProviderDetails.query.first() - update_resp = client.post( + resp = client.post( '/provider-details/{}'.format(provider.id), headers=[('Content-Type', 'application/json'), create_authorization_header()], data=json.dumps({field: value}) ) - assert update_resp.status_code == 400 - update_resp = json.loads(update_resp.get_data(as_text=True)) - print(update_resp) - assert update_resp['message'][field][0] == 'Not permitted to be updated' - assert update_resp['result'] == 'error' + resp_json = json.loads(resp.get_data(as_text=True)) + + assert resp_json['message'][field][0] == 'Not permitted to be updated' + assert resp_json['result'] == 'error' + assert resp.status_code == 400 + + +def test_get_provider_versions_contains_correct_fields(client, notify_db): + provider = ProviderDetailsHistory.query.first() + response = client.get( + '/provider-details/{}/versions'.format(provider.id), + headers=[create_authorization_header()] + ) + json_resp = json.loads(response.get_data(as_text=True))['data'] + allowed_keys = { + "id", "created_by", "display_name", + "identifier", "priority", 'notification_type', + "active", "version", "updated_at" + } + assert allowed_keys == set(json_resp[0].keys()) + + +def test_update_provider_should_store_user_id(client, restore_provider_details, sample_user): + provider = ProviderDetails.query.first() + + update_resp_1 = client.post( + '/provider-details/{}'.format(provider.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()], + data=json.dumps({ + 'created_by': sample_user.id, + 'active': False + }) + ) + assert update_resp_1.status_code == 200 + update_resp_1 = json.loads(update_resp_1.get_data(as_text=True))['provider_details'] + assert update_resp_1['identifier'] == provider.identifier + assert not update_resp_1['active'] + assert not provider.active From 1ccaf08003a91fa9d52276516fb51f1b936a1299 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Wed, 8 Mar 2017 09:49:47 +0000 Subject: [PATCH 5/5] Fix migration and small refactor --- app/provider_details/rest.py | 4 ++-- .../0068_add_created_by_to_provider.py | 24 ++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/app/provider_details/rest.py b/app/provider_details/rest.py index bdd093e22..a86860b6e 100644 --- a/app/provider_details/rest.py +++ b/app/provider_details/rest.py @@ -42,15 +42,15 @@ def get_provider_versions(provider_details_id): @provider_details.route('/', methods=['POST']) def update_provider_details(provider_details_id): valid_keys = {'priority', 'created_by', 'active'} - invalid_keys = request.get_json().keys() - valid_keys + req_json = request.get_json() + invalid_keys = req_json.keys() - valid_keys if invalid_keys: message = "Not permitted to be updated" errors = {key: [message] for key in invalid_keys} raise InvalidRequest(errors, status_code=400) provider = get_provider_details_by_id(provider_details_id) - req_json = request.get_json() # Handle created_by differently due to how history entry is created if 'created_by' in req_json: diff --git a/migrations/versions/0068_add_created_by_to_provider.py b/migrations/versions/0068_add_created_by_to_provider.py index a296b2309..04c4c815c 100644 --- a/migrations/versions/0068_add_created_by_to_provider.py +++ b/migrations/versions/0068_add_created_by_to_provider.py @@ -2,7 +2,7 @@ Revision ID: 0068_add_created_by_to_provider Revises: 0067_service_contact_block -Create Date: 2017-03-02 17:47:17.586815 +Create Date: 2017-03-06 17:19:28.492005 """ @@ -14,23 +14,35 @@ from alembic import op import sqlalchemy as sa from sqlalchemy.dialects import postgresql + def upgrade(): # ### commands auto generated by Alembic - please adjust! ### op.add_column('provider_details', sa.Column('created_by_id', postgresql.UUID(as_uuid=True), nullable=True)) op.create_index(op.f('ix_provider_details_created_by_id'), 'provider_details', ['created_by_id'], unique=False) - op.create_foreign_key(None, 'provider_details', 'users', ['created_by_id'], ['id']) + op.create_foreign_key('provider_details_created_by_id_fkey', 'provider_details', 'users', ['created_by_id'], ['id']) op.add_column('provider_details_history', sa.Column('created_by_id', postgresql.UUID(as_uuid=True), nullable=True)) - op.create_index(op.f('ix_provider_details_history_created_by_id'), 'provider_details_history', ['created_by_id'], unique=False) - op.create_foreign_key(None, 'provider_details_history', 'users', ['created_by_id'], ['id']) + op.create_index( + op.f('ix_provider_details_history_created_by_id'), + 'provider_details_history', + ['created_by_id'], + unique=False + ) + op.create_foreign_key( + 'provider_details_history_created_by_id_fkey', + 'provider_details_history', + 'users', + ['created_by_id'], + ['id'] + ) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_constraint(None, 'provider_details_history', type_='foreignkey') + op.drop_constraint('provider_details_history_created_by_id_fkey', 'provider_details_history', type_='foreignkey') op.drop_index(op.f('ix_provider_details_history_created_by_id'), table_name='provider_details_history') op.drop_column('provider_details_history', 'created_by_id') - op.drop_constraint(None, 'provider_details', type_='foreignkey') + op.drop_constraint('provider_details_created_by_id_fkey', 'provider_details', type_='foreignkey') op.drop_index(op.f('ix_provider_details_created_by_id'), table_name='provider_details') op.drop_column('provider_details', 'created_by_id') # ### end Alembic commands ###