Remove the tokens table
#1090
Merged
Remove the tokens table
#1090
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sgrif
force-pushed
the
sg-rm-tokens-table
branch
2 times, most recently
from
462743a to
a1475db
Compare
|
@carols10cents FYI I've got a branch updating to Diesel master (likely to represent 1.0 API-wise) ready to go, just blocked on this. Stuff like #1108 (and this, which is why I branched off this PR) will conflict heavily with it since we deprecated our existing APIs both for insert and upsert. I'm fine with keeping it up to date as you review other stuff, but just wanted to let you know where I'm at |
|
Sorry sorry sorry. We need #1108 out before the release on thursday, so I'm going to merge and deploy that, then look at this. |
|
Lol it's completely fine. No rush. Like I said, just wanted to let you know what's up |
This table was always one-to-one with the emails table, and we should have always been updating it when the email was updated. This moves all of that logic into the database, and merges the two tables. The only reason that these tables were ever separate as far as I can tell was to represent when the confirmation email hadn't been sent. However, we can just do that with a nullable column. However, having the `token` column leads to some awkward `unwrap`s in the code where we return it, since we know that updating the `email` always generates a new token. For that reason I have made the `token` column `NOT NULL`, but left the timestamp nullable. Rows in the `emails` table that did not previously have an associated `token` row will have a `NULL` value for this column.
sgrif
force-pushed
the
sg-rm-tokens-table
branch
from
a1475db to
01bad8d
Compare
|
Phew, had a chance to look at this, looks good! Diesel upgrade away! bors: r+ |
bors-voyager bot
added a commit
that referenced
this pull request
Oct 10, 2017
1090: Remove the `tokens` table r=carols10cents This table was always one-to-one with the emails table, and we should have always been updating it when the email was updated. This moves all of that logic into the database, and merges the two tables. The only reason that these tables were ever separate as far as I can tell was to represent when the confirmation email hadn't been sent. However, we can just do that with a nullable column. However, having the `token` column leads to some awkward `unwrap`s in the code where we return it, since we know that updating the `email` always generates a new token. For that reason I have made the `token` column `NOT NULL`, but left the timestamp nullable. Rows in the `emails` table that did not previously have an associated `token` row will have a `NULL` value for this column.
|
<3
…On Mon, Oct 9, 2017, 7:16 PM Carol (Nichols || Goulding) < ***@***.***> wrote:
Phew, had a chance to look at this, looks good! Diesel upgrade away!
bors: r+
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1090 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABdWK8ekpOJMfysliC9QP_p4UrtIBsGOks5sqsVZgaJpZM4Pk9gk>
.
|
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This table was always one-to-one with the emails table, and we should
have always been updating it when the email was updated. This moves all
of that logic into the database, and merges the two tables.
The only reason that these tables were ever separate as far as I can
tell was to represent when the confirmation email hadn't been sent.
However, we can just do that with a nullable column.
However, having the
tokencolumn leads to some awkwardunwraps inthe code where we return it, since we know that updating the
emailalways generates a new token. For that reason I have made the
tokencolumn
NOT NULL, but left the timestamp nullable. Rows in theemailstable that did not previously have an associated
tokenrow will have aNULLvalue for this column.