-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add user signup with GitHub OAuth #125
Conversation
520e2d7
to
ab0d1b2
Compare
ab0d1b2
to
42a814c
Compare
app/models/profile.rb
Outdated
validates :name, presence: true | ||
validates :email, presence: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emailとnameはGithub側で空欄で登録できるので、必須にしないほうが親切かも?と思いました!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
必須にする場合は、profile作成失敗時に適切にエラーメッセージを表示してあげると良さそうです!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ちなみにアバター画像については、頑張って外そうと試みたのですが、gravatarの画像が勝手に設定されちゃってできませんでした...
docによると
アカウントにサインアップすると、GitHub はとりあえずランダムなアイデンティコンを生成します。
とあるので空になることはないのかもしれません。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emailはもしかしたら、と思っていたんですがnameは想定外だったので、チェックしていただいて助かりました。
ちょっと修正を入れます
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コメントはしましたが、後追い対応でも良さそうだと思いました!
b3422ca
to
0ffb99b
Compare
コメントの対応として、以下の対応を入れました
|
GitHub の OAuth を使用して、サインアップする機能の実装です。