Email verification DB fields #258
Conversation
|
This pull request has been linked to Shortcut Story #14153: Email Token Database Migration. |
| ALTER TABLE users ADD COLUMN email_verification_expires TEXT DEFAULT NULL; | ||
|
|
||
| -- Token provided by the user to verify their email address. | ||
| ALTER TABLE users ADD COLUMN email_verification_token TEXT DEFAULT NULL; |
There was a problem hiding this comment.
I tried to add a UNIQUE constraint here because we will need to look up the user record by the token. However, it seems that it's not allowed for the ALTER TABLE statement. Realistically it shouldn't matter since it's unlikely that two hashes would be the same but I was wondering if we wanted the constraint anyway.
There was a problem hiding this comment.
The UNIQUE constraint also makes lookups faster since there is an index, so we probably would want the constraint. When you say it's not allowed is the problem that you can't do
ALTER TABLE users ADD COLUMN email_verification_token TEXT UNIQUE DEFAULT NULL?
Is it possible to do
CREATE UNIQUE INDEX unique_email_verification_token ON users (email_verification_token);?
(note that SQLite does allow nullable unique fields and multiple NULL values do not count towards uniqueness -- but this is a SQLite thing not a SQL thing -- https://stackoverflow.com/questions/22699409/sqlite-null-and-unique).
There was a problem hiding this comment.
Thanks, I wasn't able to apply the constraint on the token column but I was able to add a unique index.
| ALTER TABLE users ADD COLUMN email_verification_expires TEXT DEFAULT NULL; | ||
|
|
||
| -- Token provided by the user to verify their email address. | ||
| ALTER TABLE users ADD COLUMN email_verification_token TEXT DEFAULT NULL; |
There was a problem hiding this comment.
The UNIQUE constraint also makes lookups faster since there is an index, so we probably would want the constraint. When you say it's not allowed is the problem that you can't do
ALTER TABLE users ADD COLUMN email_verification_token TEXT UNIQUE DEFAULT NULL?
Is it possible to do
CREATE UNIQUE INDEX unique_email_verification_token ON users (email_verification_token);?
(note that SQLite does allow nullable unique fields and multiple NULL values do not count towards uniqueness -- but this is a SQLite thing not a SQL thing -- https://stackoverflow.com/questions/22699409/sqlite-null-and-unique).
Scope of changes
This adds a migration to add the email verification fields to the user table in the DB and updates the SQL handling code to create/read/update the new fields.
Fixes SC-14153
Type of change
Acceptance criteria
Describe how reviewers can test this change to be sure that it works correctly. Add a checklist if possible.
Author checklist
Reviewer(s) checklist