Fix/url history defaults

[gweiying]

Problem

What problem are you trying to solve? What issue does this close?

Lambda functions for migrating links were not running successfully on health-staging-db as the description column in url_histories tables had a constraint that enforced non-null values but did not set a default value.

Related to this, the two SQL migration functions did not include description as an indexed column from url table when inserting the link into the url_histories table. Hence, when a new migration attempted to occur, the description in the url table was not being copied into the new row in url_histories table, so the column defaulted to null, violating the non-null constraint.

Solution

How did you solve the problem?

• Updated SQL functions to include description as an indexed column on url table when inserting the link into the url_histories table
• Added empty string as a default on description column in url_histories table in database models

Features:

• Details ...

Improvements:

• Details ...

Bug Fixes:

• Details ...

Before & After Screenshots

BEFORE:
[insert screenshot here]

AFTER:
[insert screenshot here]

Tests

What tests should be run to confirm functionality?

• Test migrate-short-link on health-staging
• Test migrate-user-links on health-staging

Deploy Notes

Notes regarding deployment of the contained body of work. These should note any
new dependencies, new scripts, etc.

Related fixes

Code changes

• Fix migration functions to add description when migrating url
• Update Sequelize models to add default empty string “” for description column in url_histories table

Update current databases

Health

• Run migration to fix this in health-staging
  • ALTER TABLE url_histories ADD "description" text NOT NULL DEFAULT '';
  • Update migration function migrate-user-links
  • Update migration function migrate-shortUrl-to-user
• Verify that this does not occur in health-prod since the model should be fixed

Edu

• Run migration to fix this in edu-staging
  • ALTER TABLE url_histories ADD "description" text NOT NULL DEFAULT '';
  • Add migration function migrate-user-links
  • Add migration function migrate-shortUrl-to-user
• Run migration to fix this in edu-prod
  • ALTER TABLE url_histories ADD "description" text NOT NULL DEFAULT '';
  • Add migration function migrate-user-links
  • Add migration function migrate-shortUrl-to-user

GoGov

• Run migration to fix this in gogov-staging
  • ALTER TABLE url_histories ADD "description" text NOT NULL DEFAULT ''; (Not necessary in gogov-staging and gogov-prod as previous migrations have added a default)
  • Update migration function migrate-user-links
  • Update migration function migrate-shortUrl-to-user
• Run migration to fix this in gogov-prod
  • ALTER TABLE url_histories ADD "description" text NOT NULL DEFAULT ''; (Not necessary in gogov-staging and gogov-prod as previous migrations have added a default)
  • Update migration function migrate-user-links
  • Update migration function migrate-shortUrl-to-user

---

• Look into debugging count for number of rows affected

[yong-jie]

Verified the following info about the description columns of each DB:

gosg-staging has '' as default
gosg-production has '' as default
edu-staging doesnt
edu-production doesnt

Did a bit of reading up as to how this happened, I suspect it's due to the differences between the migration file and the sequelize model definition.

Given that the gov variant was spun up way earlier, we had to run migration scripts to add the description column. This migration script declared a '' as default. However, subsequent variants (edu, health) directly make use of the sequelize models, which did not have a default value.

edit: managed to find the offending PR which holds a bit of context as to how this happened, for enrichment purposes 😂

[yong-jie]

Both the code and strategy looks good to me! i'd just like to propose one more addition to this PR which would be to explicitly commit a migration sql file into the codebase, similar to what has been done in this directory.

Even though the migration script is probably gonna just be a one-liner wrapped in a transaction, there's value in tracking the list of migrations we've done in production, and reduce the risk of executing incorrect commands on the database

[gweiying]

Both the code and strategy looks good to me! i'd just like to propose one more addition to this PR which would be to explicitly commit a migration sql file into the codebase, similar to what has been done in this directory.

Even though the migration script is probably gonna just be a one-liner wrapped in a transaction, there's value in tracking the list of migrations we've done in production, and reduce the risk of executing incorrect commands on the database

Fixed in 20f898d

[yong-jie]

lgtm!