Make SMS prefix setting non-nullable

We have now:
- defaulted new services to start with this column set to `true`
- migrated all preexisting services[1] to have either `true` or `false` set
  for this column

There is no way for a service to switch back from `true`/`false` to
`null`.

This means that we can safely enforce a non-nullable constraint on this
column now.

1. There is a little gotcha: the GOV.UK Notify service still relies on
   the `sms_sender` column. It doesn’t have a row in the
   `service_sms_senders` table. This means the previous migration
   never changed this service’s value for `prefix_sms` from `null`. So
   this commit also changes its value to `False`, so that the rest of
   the migration, of the whole column, is possible.
This commit is contained in:
Chris Hill-Scott
2017-11-03 16:44:33 +00:00
parent bf5e6db666
commit e8a94422e5
4 changed files with 60 additions and 3 deletions

View File

@@ -0,0 +1,46 @@
"""
Revision ID: 0140_sms_prefix_non_nullable
Revises: 0139_migrate_sms_allowance_data
Create Date: 2017-11-07 13:04:04.077142
"""
from alembic import op
from flask import current_app
import sqlalchemy as sa
from sqlalchemy.dialects import postgresql
revision = '0140_sms_prefix_non_nullable'
down_revision = '0139_migrate_sms_allowance_data'
def upgrade():
op.execute("""
update services
set prefix_sms = false
where id = '{}'
""".format(current_app.config['NOTIFY_SERVICE_ID']))
op.alter_column(
'services',
'prefix_sms',
existing_type=sa.BOOLEAN(),
nullable=False,
)
def downgrade():
op.alter_column(
'services',
'prefix_sms',
existing_type=sa.BOOLEAN(),
nullable=True,
)
op.execute("""
update services
set prefix_sms = null
where id = '{}'
""".format(current_app.config['NOTIFY_SERVICE_ID']))

View File

@@ -992,7 +992,7 @@ def notify_service(notify_db, notify_db_session):
'active': True, 'active': True,
'restricted': False, 'restricted': False,
'email_from': 'notify.service', 'email_from': 'notify.service',
'created_by': user 'created_by': user,
} }
service = Service(**data) service = Service(**data)
db.session.add(service) db.session.add(service)

View File

@@ -68,7 +68,7 @@ def create_service(
research_mode=False, research_mode=False,
active=True, active=True,
email_from=None, email_from=None,
prefix_sms=None, prefix_sms=True,
): ):
service = Service( service = Service(
name=service_name, name=service_name,

View File

@@ -1551,7 +1551,6 @@ def test_prefixing_messages_based_on_prefix_sms(
@pytest.mark.parametrize('posted_value, stored_value, returned_value', [ @pytest.mark.parametrize('posted_value, stored_value, returned_value', [
(True, True, True), (True, True, True),
(False, False, False), (False, False, False),
(None, None, True),
]) ])
def test_set_sms_prefixing_for_service( def test_set_sms_prefixing_for_service(
client, client,
@@ -1581,6 +1580,18 @@ def test_set_sms_prefixing_for_service(
assert result['data']['sms_sender'] == current_app.config['FROM_NUMBER'] assert result['data']['sms_sender'] == current_app.config['FROM_NUMBER']
def test_set_sms_prefixing_for_service_cant_be_none(
admin_request,
sample_service,
):
admin_request.post(
'service.update_service',
service_id=sample_service.id,
_data={'prefix_sms': None},
_expected_status=500,
)
@pytest.mark.parametrize('today_only,stats', [ @pytest.mark.parametrize('today_only,stats', [
('False', {'requested': 2, 'delivered': 1, 'failed': 0}), ('False', {'requested': 2, 'delivered': 1, 'failed': 0}),
('True', {'requested': 1, 'delivered': 0, 'failed': 0}) ('True', {'requested': 1, 'delivered': 0, 'failed': 0})