code review feedback

migrations/versions/0346_notify_number_sms_sender.py

@@ -9,6 +9,7 @@ import uuid
 
 from alembic import op
 from flask import current_app
+from sqlalchemy import text
 
 revision = '0346_notify_number_sms_sender'
 down_revision = '0345_move_broadcast_provider'
@@ -19,19 +20,33 @@ INBOUND_NUMBER = current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER'].strip('+'
 
 
 def upgrade():
-    op.execute("INSERT INTO service_sms_senders (id, sms_sender, service_id, is_default, created_at) "
+    conn = op.get_bind()
-               "VALUES ('{}', '{}', '{}',false, now())".format(
+    input_params = {
-        SMS_SENDER_ID,
+        "sms_sender_id": SMS_SENDER_ID,
-        INBOUND_NUMBER,
+        "inbound_number": INBOUND_NUMBER,
-        NOTIFY_SERVICE_ID
+        "notify_service_id": NOTIFY_SERVICE_ID
-    ))
+    }
+    conn.execute(text("INSERT INTO service_sms_senders (id, sms_sender, service_id, is_default, created_at) "
+               "VALUES (:sms_sender_id, :inbound_number, :notify_service_id,false, now())"), input_params)
+
 
     inbound_number_id = uuid.uuid4()
+    input_params = {
+        "inbound_number_id": inbound_number_id,
+        "inbound_number": INBOUND_NUMBER,
+    }
     # by adding a row in inbound_number we ensure the number isn't added to the table and assigned to a service.
-    op.execute("INSERT INTO INBOUND_NUMBERS (id, number, provider, active, created_at) VALUES('{}', "
+    conn.execute(text("INSERT INTO INBOUND_NUMBERS (id, number, provider, active, created_at) VALUES(:inbound_number_id, "
-               "'{}', '{}', false, now())".format(inbound_number_id, INBOUND_NUMBER, 'mmg'))
+               ":inbound_number, 'mmg', false, now())"), input_params)
 
 
 def downgrade():
-    op.execute("delete from service_sms_senders where id = '{}'".format(SMS_SENDER_ID))
+    conn = op.get_bind()
-    op.execute("delete from inbound_numbers where number = '{}'".format(INBOUND_NUMBER))
+    input_params = {
+        "sms_sender_id": SMS_SENDER_ID
+    }
+    conn.execute(text("delete from service_sms_senders where id = :sms_sender_id"), input_params)
+    input_params = {
+        "inbound_number": INBOUND_NUMBER
+    }
+    conn.execute(text("delete from inbound_numbers where number = :inbound_number"), input_params)

migrations/versions/0348_migrate_broadcast_settings_migrate_broadcast_settings.py

@@ -7,6 +7,7 @@ Create Date: 2021-02-18 15:25:30.667098
 """
 from alembic import op
 import sqlalchemy as sa
+from sqlalchemy import text
 from sqlalchemy.dialects import postgresql
 
 revision = '0348_migrate_broadcast_settings'
@@ -36,15 +37,20 @@ def upgrade():
   for service in services:
       input_params = {"service_id": service.id}
       setting = conn.execute(
-          "SELECT service_id, channel, provider FROM service_broadcast_settings WHERE service_id=:service_id;",
+          text("SELECT service_id, channel, provider FROM service_broadcast_settings WHERE service_id=:service_id;"),
           input_params).first()
       if setting:
         print(f"Service {service.id} already has service_broadcast_settings. No action required")
       else:
         channel = "severe" if service.restricted else "test"
         print(f"Service {service.id} does not have service_broadcast_settings. Will insert one with channel {channel}")
-        conn.execute("INSERT INTO service_broadcast_settings (service_id, channel, created_at) VALUES (%s, %s, now());",
+        input_params = {
-                     {service.id, channel})
+            "service_id": service.id,
+            "channel": channel
+        }
+        conn.execute(text("INSERT INTO service_broadcast_settings (service_id, channel, created_at) "
+                          "VALUES (:service_id, :channel, now());"),
+                     input_params)
 
 
 def downgrade():

migrations/versions/0352_broadcast_provider_types.py

@@ -7,6 +7,7 @@ Create Date: 2021-05-05 15:07:22.146657
 """
 from alembic import op
 import sqlalchemy as sa
+from sqlalchemy import text
 
 revision = '0352_broadcast_provider_types'
 down_revision = '0351_unique_key_annual_billing'
@@ -15,11 +16,15 @@ PROVIDER_TYPES = ('ee', 'three', 'vodafone', 'o2', 'all')
 
 
 def upgrade():
+    conn = op.get_bind()
     op.create_table('broadcast_provider_types',
                     sa.Column('name', sa.String(length=255), nullable=False),
                     sa.PrimaryKeyConstraint('name'))
     for provider in PROVIDER_TYPES:
-        op.execute("INSERT INTO broadcast_provider_types VALUES ('{}')".format(provider))
+        input_params = {
+            "provider": provider
+        }
+        conn.execute(text("INSERT INTO broadcast_provider_types VALUES (:provider)"), input_params)
     op.create_foreign_key('service_broadcast_settings_provider_fkey',
                           'service_broadcast_settings',
                           'broadcast_provider_types',

migrations/versions/0374_fix_reg_template_history.py

@@ -9,6 +9,8 @@ Create Date: 2022-08-22 11:04:15.888017
 # revision identifiers, used by Alembic.
 from datetime import datetime
 
+from sqlalchemy import text
+
 revision = '0374_fix_reg_template_history'
 down_revision = '0373_add_notifications_view'
 
@@ -18,32 +20,27 @@ import sqlalchemy as sa
 service_id = 'd6aa2c68-a2d9-4437-ab19-3ae8eb202553'
 user_id= '6af522d0-2915-4e52-83a3-3690455a5fe6'
 
+
 def upgrade():
-    op.get_bind()
+    conn = op.get_bind()
     
     # modify subject of verification email in templates
-    table_name = 'templates'
+    conn.execute("update templates set subject='Confirm US Notify registration' "
-    col = 'subject'
+                 "where name = 'Notify email verification code'")
-    val = 'Confirm US Notify registration'
-    select_by_col = 'name'
-    select_by_val = 'Notify email verification code'
-    op.execute(f"update {table_name} set {col}='{val}' where {select_by_col} = '{select_by_val}'")
     
     # modify subject of verification email in templates_history
-    table_name = 'templates_history'
+    conn.execute("update templates_history set subject='Confirm US Notify registration' "
-    op.execute(f"update {table_name} set {col}='{val}' where {select_by_col} = '{select_by_val}'")
+                 "where name = 'Notify email verification code'")
     
-    # modify content of verification email in templates
-    # table_name = 'templates'
-    # col = 'content'
     val = """Hi ((name)),\n\nTo complete your registration for US Notify please click the link below\n\n((url))"""
-    # select_by_col = 'name'
+    input_params = {
-    # select_by_val = 'Notify email verification code'
+        "val": val
-    op.execute("update templates set content='{}' where name = 'Notify email verification code'".format(val))
+    }
+    conn.execute(text("update templates set content=:val where name = 'Notify email verification code'"), input_params)
     
     # modify content of verification email in templates_history
     # table_name = 'templates_history'
-    op.execute("update templates_history set content='{}' where name = 'Notify email verification code'".format(val))
+    conn.execute(text("update templates_history set content=:val where name = 'Notify email verification code'"), input_params)
     
     # TODO: modify other templates as necessary and re-run this migration
     

migrations/versions/0375_fix_service_name.py

@@ -5,6 +5,7 @@ Revises: 0374_fix_reg_template_history
 Create Date: 2022-08-29 11:04:15.888017
 
 """
+from sqlalchemy import text
 
 revision = '0375_fix_service_name'
 down_revision = '0374_fix_reg_template_history'
@@ -16,7 +17,7 @@ service_id = current_app.config['NOTIFY_SERVICE_ID']
 
 
 def upgrade():
-    op.get_bind()
+    conn = op.get_bind()
     
     # modify name of default service user in services
     # table_name = 'services'
@@ -24,10 +25,13 @@ def upgrade():
     # val = 'US Notify'
     # select_by_col = 'id'
     # select_by_val = service_id
-    op.execute("update services set name='US Notify' where id = '{}'".format(service_id))
+    input_params = {
+        "service_id": service_id
+    }
+    conn.execute(text("update services set name='US Notify' where id =:service_id"), input_params)
     
     # table_name = 'services_history'
-    op.execute("update services_history set name='US Notify' where id = '{}'".format(service_id))
+    conn.execute(text("update services_history set name='US Notify' where id =:service_id"), input_params)
     
 
 def downgrade():

migrations/versions/0377_add_inbound_sms_number.py

@@ -9,7 +9,7 @@ import uuid
 
 from alembic import op
 from flask import current_app
-
+from sqlalchemy import text
 
 revision = '0377_add_inbound_sms_number'
 down_revision = '0376_add_provider_response'
@@ -20,36 +20,63 @@ DEFAULT_SERVICE_ID = current_app.config['NOTIFY_SERVICE_ID']
 
 
 def upgrade():
-    op.get_bind()
+    conn = op.get_bind()
 
     # delete the previous inbound_number with mmg as provider
     # table_name = 'inbound_numbers'
     # select_by_col = 'number'
     # select_by_val = INBOUND_NUMBER
-    op.execute("delete from inbound_numbers where number = '{}'".format(INBOUND_NUMBER))
+    input_params = {
+        "inbound_number": INBOUND_NUMBER
+    }
+    conn.execute(text("delete from inbound_numbers where number =:inbound_number"), input_params)
 
+    input_params = {
+        "inbound_number_id": INBOUND_NUMBER_ID,
+        "inbound_number": INBOUND_NUMBER,
+        "default_service_id": DEFAULT_SERVICE_ID
+    }
     # add the inbound number for the default service to inbound_numbers
-    op.execute("insert into inbound_numbers "
+    conn.execute(text("insert into inbound_numbers "
-               "(id, number, provider, service_id, active, created_at) "
+                      "(id, number, provider, service_id, active, created_at) "
-               "VALUES ('{}', '{}', 'sns', '{}', 'true', now())".format(INBOUND_NUMBER_ID,
+                      "VALUES (:inbound_number_id, :inbound_number, 'sns', :default_service_id, 'true', now())"),
-                                                                        INBOUND_NUMBER, DEFAULT_SERVICE_ID))
+                 input_params)
 
+    input_params = {
+        "inbound_number": INBOUND_NUMBER
+    }
     # add the inbound number for the default service to service_sms_senders
-    op.execute("update service_sms_senders set sms_sender='{}' "
+    conn.execute(text("update service_sms_senders set sms_sender=:inbound_number "
-               "where id = '286d6176-adbe-7ea7-ba26-b7606ee5e2a4'".format(INBOUND_NUMBER))
+               "where id = '286d6176-adbe-7ea7-ba26-b7606ee5e2a4'"), input_params)
 
     # add the inbound number for the default service to inbound_numbers
-    op.execute("insert into service_permissions (service_id, permission, created_at) "
+    input_params = {
-               "VALUES('{}', 'inbound_sms', now())".format(DEFAULT_SERVICE_ID))
+        "default_service_id": DEFAULT_SERVICE_ID
+    }
+    conn.execute(text("insert into service_permissions (service_id, permission, created_at) "
+                      "VALUES(:default_service_id, 'inbound_sms', now())"), input_params)
     # pass
 
 
 def downgrade():
-    op.execute("delete from service_sms_senders where inbound_number_id = '{}'".format(INBOUND_NUMBER_ID))
+    conn = op.get_bind()
-    op.execute("delete from inbound_numbers where number = '{}'".format(INBOUND_NUMBER))
+    input_params = {
-    op.execute("delete from service_permissions where service_id = '{}' and permission = 'inbound_sms'".format(
+        "inbound_number_id": INBOUND_NUMBER_ID
-        DEFAULT_SERVICE_ID))
+    }
-    op.execute("insert into inbound_numbers (id, number, provider, service_id, active, created_at) "
+    conn.execute(text("delete from service_sms_senders where inbound_number_id = :inbound_number_id"), input_params)
-               "VALUES('d7aea27f-340b-4428-9b20-4470dd978bda', '{}', 'mmg', 'null', 'false', 'now()')".format(
+    input_params = {
-        INBOUND_NUMBER))
+        "inbound_number": INBOUND_NUMBER
+    }
+    conn.execute(text("delete from inbound_numbers where number = :inbound_number"), input_params)
+    input_params = {
+        "default_service_id": DEFAULT_SERVICE_ID
+    }
+    conn.execute(text("delete from service_permissions "
+                      "where service_id = :default_service_id and permission = 'inbound_sms'"), input_params)
+    input_params = {
+        "inbound_number": INBOUND_NUMBER
+    }
+    conn.execute(text("insert into inbound_numbers (id, number, provider, service_id, active, created_at) "
+                      "VALUES('d7aea27f-340b-4428-9b20-4470dd978bda', :inbound_number, 'mmg', 'null', 'false', 'now()')"),
+                 input_params)
     # pass

migrations/versions/0391_update_sms_numbers.py

@@ -8,7 +8,7 @@ Create Date: 2023-03-01 12:36:38.226954
 from alembic import op
 from flask import current_app
 import sqlalchemy as sa
-
+from sqlalchemy import text
 
 revision = '0391_update_sms_numbers'
 down_revision = '0390_drop_dvla_provider.py'
@@ -17,18 +17,29 @@ NEW_SMS_NUMBER = current_app.config['NOTIFY_INTERNATIONAL_SMS_SENDER'].strip('+'
 
 
 def upgrade():
+    conn = op.get_bind()
     op.alter_column("service_sms_senders", "sms_sender", type_=sa.types.String(length=255))
     op.alter_column("inbound_numbers", "number", type_=sa.types.String(length=255))
-    op.execute("UPDATE service_sms_senders SET sms_sender = '+{}' "
+    input_params = {
-               "WHERE sms_sender IN ('{}', '{}')".format(NEW_SMS_NUMBER, OLD_SMS_NUMBER, NEW_SMS_NUMBER))
+        "new_sms_plus": f"+{NEW_SMS_NUMBER}",
-    op.execute("UPDATE inbound_numbers SET number = '+{}' "
+        "old_sms_number": OLD_SMS_NUMBER,
-               "WHERE number IN ('{}', '{}')".format(NEW_SMS_NUMBER, OLD_SMS_NUMBER, NEW_SMS_NUMBER))
+        "new_sms_number": NEW_SMS_NUMBER
+    }
+    conn.execute(text("UPDATE service_sms_senders SET sms_sender = :new_sms_plus "
+                      "WHERE sms_sender IN (:old_sms_number, :new_sms_number)"), input_params)
+    conn.execute(text("UPDATE inbound_numbers SET number = :new_sms_plus "
+                      "WHERE number IN (:old_sms_number, :new_sms_number)"), input_params)
 
 
 def downgrade():
-    op.execute("UPDATE service_sms_senders SET sms_sender = '{}' "
+    conn = op.get_bind()
-               "WHERE sms_sender = '+{}'".format(OLD_SMS_NUMBER, NEW_SMS_NUMBER))
+    input_params = {
-    op.execute("UPDATE inbound_numbers SET number = '{}' "
+        "old_sms_number": OLD_SMS_NUMBER,
-               "WHERE number = '+{}'".format(OLD_SMS_NUMBER, NEW_SMS_NUMBER))
+        "new_sms_plus": f"+{NEW_SMS_NUMBER}"
+    }
+    conn.execute(text("UPDATE service_sms_senders SET sms_sender = :old_sms_number "
+                      "WHERE sms_sender = :new_sms_plus"), input_params)
+    conn.execute(text("UPDATE inbound_numbers SET number = :old_sms_number "
+                      "WHERE number = :new_sms_plus"), input_params)
     op.alter_column("service_sms_senders", "sms_sender", type_=sa.types.String(length=11))
     op.alter_column("inbound_numbers", "number", type_=sa.types.String(length=11))