Dev signup #66
Merged
Dev signup #66
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
To test "sign in with developer strategy."
|
Oops, I need to describe about this patch. |
| @@ -5,7 +5,7 @@ class User < ActiveRecord::Base | |||
|
|
|||
| validates :provider, presence: true, inclusion: { in: Settings.providers } | |||
| validates :name, presence: true, uniqueness: true, length: { maximum: 40 } | |||
| validates :uid, presence: true, format: { with: /\A[0-9]+\Z/ } | |||
| validates :uid, presence: true, uniqueness: true, format: { with: /\A[0-9]+\Z/ } | |||
There was a problem hiding this comment.
Is it necessary? This uniqueness should be ensured by provider and uid.
There was a problem hiding this comment.
Oh, sorry, I have missed that the key for users table is composite.
I added scope instead of remove it because
messages from ActiveRecord validations are very helpful for end users.
|
The patch looks almost good. Some remains being fixed, I'll merge it. |
| @@ -5,7 +5,9 @@ class User < ActiveRecord::Base | |||
|
|
|||
| validates :provider, presence: true, inclusion: { in: Settings.providers } | |||
| validates :name, presence: true, uniqueness: true, length: { maximum: 40 } | |||
| validates :uid, presence: true, format: { with: /\A[0-9]+\Z/ } | |||
| validates :uid, presence: true, format: { with: /\A[0-9]+\Z/ }, | |||
| uniqueness: {scope: :provider, | |||
There was a problem hiding this comment.
Oh, I hadn't known that we could write like this!
kentaro
added a commit
that referenced
this pull request
Jan 21, 2013
|
Thanks!!1 |
|
Thanks reviewing and merging. |
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.
Commits for #53
I make a proposal on only functionality.
I know putting two long text links in
/caveatand global nav but I think it should be discussed and patched in another issue.I'm glad if you consider this patch.
Thanks.