Preparation to drop email in User in Rust codes
#1911
Merged
Conversation
This file contains hidden or 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
|
r? @smarnach (rust_highfive has picked a reviewer for you, use r? to override) |
|
☔ The latest upstream changes (presumably #1901) made this pull request unmergeable. Please resolve the merge conflicts. |
hbina
changed the title
email from user structemail from Rust codes
hbina
changed the title
email from Rust codesemail in User in Rust codes
carols10cents
requested changes
Nov 22, 2019
src/controllers/krate/metadata.rs
Outdated
| @@ -220,18 +222,21 @@ pub fn reverse_dependencies(req: &mut dyn Request) -> CargoResult<Response> { | |||
|
|
|||
| let version_ids: Vec<i32> = rev_deps.iter().map(|dep| dep.version_id).collect(); | |||
|
|
|||
| // FIXME :: Another one of these silly transformation that is necessary to test | |||
| // whether or not this function uses the column email from the user | |||
| // table. Here, we simply set it to None. | |||
There was a problem hiding this comment.
Does this comment still apply, since the transformation it's talking about was removed in ea51376?
There was a problem hiding this comment.
You are right, I don't think this applies anymore. Do you want me to merge these commits into one?
|
☔ The latest upstream changes (presumably #1909) made this pull request unmergeable. Please resolve the merge conflicts. |
Every queries out of `User`'stable should now be converted into an intermediate type called `UserNoEmailType` (could be renamed, I guess). Conversion from this type to `User` is simply a copy except that `email` returned will always be `None`. I think its possible to remove `email` from User directly because its not actually the mirror of `User`'s table (the mirror struct is actually `NewUser`
carols10cents
approved these changes
Nov 25, 2019
|
Thank you so much! I introduced some conflicts by merging some other PRs, so I resolved those and squashed for you while I was in there. Great work!! @bors r+ |
|
📌 Commit 3af1934 has been approved by |
bors
added a commit
that referenced
this pull request
Nov 25, 2019
…cents Preparation to drop `email` in User in Rust codes Related: #1888 This commit(s) prepares the migration to dropping `email` column from the `User` table. Here, I introduce a new intermediate type called `UserNoEmailType`. Copied from my latest commit: Every queries out of `User`'stable should now be converted into an intermediate type called `UserNoEmailType`. This is done by selecting only the appropriate columns which is defined in `user::ALL_COLUMNS` (notice that it excludes `email`) This could be renamed, I guess. Conversion from this type to `User` is simply a copy except that `email` returned will always be `None`. I think its possible to remove `email` from User directly because its not actually the mirror of `User`'s table. The mirror struct is actually `NewUser`.
|
☀️ Test successful - checks-travis |
carols10cents
added a commit
to integer32llc/crates.io
that referenced
this pull request
Nov 25, 2019
bors
added a commit
that referenced
this pull request
Nov 25, 2019
Revert "Auto merge of #1911 - hbina:remove_email_from_user_struct, r=… …carols10cents" This reverts commit 92edac3, reversing changes made to 4df7001. I missed a spot when reviewing #1911. This reverts that merge so that master is deployable again; I'm going to self-r+ this and then open a new PR with #1911 and a fix on it and have someone else review that.
|
Thank you! |
bors
added a commit
that referenced
this pull request
Nov 26, 2019
Missed a conversion from UserNoEmailType to User When reviewing #1911, I missed a spot. I reverted the PR merge on master, so this PR reverts the revert and adds the fix. I tried to write a test that would have caught this, but I utterly failed at making a real-enough app/request that would have `.user()` but would let me set the `session()` :( Also a lot of this code is temporary and will be undone once I've rebased #1891, but this needs to be deployed separately from #1891 to ensure there aren't any uses of the email column. r? @jtgeibel
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.
Related: #1888
This commit(s) prepares the migration to dropping
emailcolumn from theUsertable.Here, I introduce a new intermediate type called
UserNoEmailType.Copied from my latest commit:
Every queries out of
User'stable should now be converted into an intermediate type calledUserNoEmailType.This is done by selecting only the appropriate columns which is defined in
user::ALL_COLUMNS(notice that it excludesemail)This could be renamed, I guess.
Conversion from this type to
Useris simply a copy except thatemailreturned will always beNone.I think its possible to remove
emailfrom User directly because its not actually the mirror ofUser's table.The mirror struct is actually
NewUser.