Skip to content

Commit

Permalink
remove broadcast from template_type
Browse files Browse the repository at this point in the history
we can't _just_ do the same process as 0467 because there are check
constraints that depend on these. When you say "alter column
template_type set type = new_type using old_type::text::new_type", it
takes that "using" statement as a way of translating the prev enum into
the new enum, in our case just casting to a str as an intermediate step.

however, the template_type column has several check constraints against
it (to say only letters can have a letter_attachment for example).
Postgres isn't smart enough to take the `using` clause and apply it to
these constraints. There are two ways around this:

* You could create a comparator function that tells postgres "if you
  have to compare old_type with new_type, here's a custom function"[^1]
* You can drop the old constraints and recreate them after the migration
  is done

We've gone with the second option as it's a bit simpler and I could copy
and paste from our previous scripts

[^1]: https://stackoverflow.com/a/57952345 was very helpful for this
  • Loading branch information
leohemsted committed Oct 1, 2024
1 parent 98b2db4 commit 67a99d0
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 0 deletions.
133 changes: 133 additions & 0 deletions migrations/versions/0468_remove_broadcast_type.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
"""
Create Date: 2024-10-01 11:08:46.900469
"""

from alembic import op
import sqlalchemy as sa
from sqlalchemy.dialects import postgresql
import textwrap

revision = "0468_remove_broadcast_type"
down_revision = "0467_remove_broadcast_perms"


template_type_new_values = ["email", "sms", "letter"]
template_type_new = postgresql.ENUM(*template_type_new_values, name="template_type")


def drop_check_constraints():
op.drop_constraint("chk_templates_letter_languages", "templates")
op.drop_constraint("chk_templates_history_letter_languages", "templates_history")

op.drop_constraint("ck_templates_letter_attachments", "templates")
op.drop_constraint("ck_templates_history_letter_attachments", "templates_history")

op.drop_constraint("ck_templates_non_email_has_unsubscribe_false", "templates")
op.drop_constraint("ck_templates_history_non_email_has_unsubscribe_false", "templates_history")


def add_check_constraints():
op.create_check_constraint(
"chk_templates_letter_languages",
"templates",
"""
CASE WHEN template_type = 'letter' THEN
letter_languages is not null
ELSE
letter_languages is null
END
""",
postgresql_not_valid=True,
)
op.create_check_constraint(
"chk_templates_history_letter_languages",
"templates_history",
"""
CASE WHEN template_type = 'letter' THEN
letter_languages is not null
ELSE
letter_languages is null
END
""",
postgresql_not_valid=True,
)

op.create_check_constraint(
"ck_templates_letter_attachments",
"templates",
"template_type = 'letter' OR letter_attachment_id IS NULL",
postgresql_not_valid=True,
)
op.create_check_constraint(
"ck_templates_history_letter_attachments",
"templates_history",
"template_type = 'letter' OR letter_attachment_id IS NULL",
postgresql_not_valid=True,
)

op.create_check_constraint(
"ck_templates_non_email_has_unsubscribe_false",
"templates",
"template_type = 'email' OR has_unsubscribe_link IS false",
postgresql_not_valid=True,
)
op.create_check_constraint(
"ck_templates_history_non_email_has_unsubscribe_false",
"templates_history",
"template_type = 'email' OR has_unsubscribe_link IS false",
postgresql_not_valid=True,
)


def upgrade():
"""
there are three check constraints on the template_type column (across both templates and templates_history).
These check constraints say things like "if type == 'letter'" - this is an enum comparison, so if we change the
type of the column, we get an error
> (psycopg2.errors.UndefinedFunction) operator does not exist: template_type = template_type_old
> HINT: No operator matches the given name and argument types. You might need to add explicit type casts.
This appears confusing because we're passing in a `using` clause to tell postgres exactly how to do this, but
this using clause only applies to the column type, and not other usages of that column within the check constraints.
To get round this, we "simply" drop all the constraints and then recreate them afterwards, referring to the new
"""
conn = op.get_bind()
conn.execute("ALTER TYPE template_type RENAME TO template_type_old;")
template_type_new.create(conn)

drop_check_constraints()

op.alter_column(
table_name="templates",
column_name="template_type",
type_=template_type_new,
postgresql_using="template_type::text::template_type",
)
op.alter_column(
table_name="templates_history",
column_name="template_type",
type_=template_type_new,
postgresql_using="template_type::text::template_type",
)
op.alter_column(
table_name="service_contact_list",
column_name="template_type",
type_=template_type_new,
postgresql_using="template_type::text::template_type",
)

conn.execute("DROP TYPE template_type_old;")
# note: need to revalidate these constraints in a separate migration to avoid a lengthy access exclusive lock
add_check_constraints()


def downgrade():
# don't need to do the constraints dance, or the enum type dance, when adding a new value

# ALTER TYPE must be run outside of a transaction block (see link below for details)
# https://alembic.sqlalchemy.org/en/latest/api/runtime.html#alembic.runtime.migration.MigrationContext.autocommit_block
with op.get_context().autocommit_block():
op.execute("ALTER TYPE template_type ADD VALUE 'broadcast'")
26 changes: 26 additions & 0 deletions migrations/versions/0469_validate_template_ck.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"""
Create Date: 2024-10-01 11:08:46.900469
"""

revision = "0469_validate_template_ck"
down_revision = "0468_remove_broadcast_type"

from alembic import op
import sqlalchemy as sa
from sqlalchemy.dialects import postgresql


def upgrade():

op.execute("ALTER TABLE templates VALIDATE CONSTRAINT chk_templates_letter_languages")
op.execute("ALTER TABLE templates_history VALIDATE CONSTRAINT chk_templates_history_letter_languages")

op.execute("ALTER TABLE templates VALIDATE CONSTRAINT ck_templates_letter_attachments")
op.execute("ALTER TABLE templates_history VALIDATE CONSTRAINT ck_templates_history_letter_attachments")

op.execute("ALTER TABLE templates VALIDATE CONSTRAINT ck_templates_non_email_has_unsubscribe_false")
op.execute("ALTER TABLE templates_history VALIDATE CONSTRAINT ck_templates_history_non_email_has_unsubscribe_false")


def downgrade():
pass

0 comments on commit 67a99d0

Please sign in to comment.