Discord認証ができるようにする #7436

yocchan-git · 2024-02-25T12:25:29Z

Issue

Discord認証（GitHub認証みたいなやつ） #7409

概要

変更確認方法

feature/oauth-discord-to-fetch-idをローカルに取り込む
こちらを参考にenvファイルを設定(ファイルは./においてください)
(リダイレクトURLは http://localhost:3000/discord_authentications/newにしてください )
Gemfileにgem 'dotenv-rails'を追加して、bin/setupする
Discordアカウントがnilのユーザでログインする
(後から下記の方法でnilにしても大丈夫です)
http://localhost:3000/current_user/edit にアクセスする
下スクロールでSNSアカウントの「Discord アカウントを登録する」を押す
そのまま認証する
TOPにリダイレクトされたら、もう一度 http://localhost:3000/current_user/edit にアクセスする
「Discord アカウントは登録されています。」とaccount_nameが表示されているのを確認する
確認後はenvファイルなどは破棄してください！ありがとうございました🙏

Discordの`account_name`を`nil`にする方法

rails cを実行する
user = User.find_by(login_name: "kimura")など、ログイン名で検索をかける
user.discord_profile.account_name = nilを実行する
user.saveする

Screenshot

変更前

変更後

yocchan-git · 2024-02-26T04:38:50Z

@machida さん

下記の画像の２箇所の確認をお願いいたします🙏
主に「Discord アカウントを登録する」ボタンのデザインをしていただけますと嬉しいです

確認方法

feature/oauth-discord-to-fetch-idをローカルに取り込む
foreman start -f Procfile.devでサーバーを立ち上げ
ログインする
http://localhost:3000/current_user/edit にアクセスする

コンソールでDiscord アカウントの登録情報を変更する方法

rails cを実行する
user = User.find_by(login_name: "machida")など、ログイン名で検索をかける
user.discord_profile.account_name = nilなどを実行する
user.saveする

デザインの確認をしていただきたい箇所

Discordアカウントの登録箇所

アカウント登録されている時のフォーム

machida · 2024-02-26T04:42:15Z

@yocchan-git 了解ですー！実装ありがとうございます！！

yocchan-git · 2024-03-01T08:39:14Z

@machida さん

キャンセル時などの動作について対応しました！
もし、ローカルに取り込んでいましたらプルしていただけますと🙇‍♂️

machida · 2024-03-05T09:58:26Z

@yocchan-git お待たせしました！デザインを整えましたー

yocchan-git · 2024-03-05T23:35:04Z

@taco-nantai さん

レビュアーにアサインさせていただきました！
お手隙でレビューしていただけますと嬉しいです🙏

ghost · 2024-03-06T05:24:59Z

@yocchan-git
お疲れ様です。
レビュー依頼の件かしこまりました。
近日中に対応させていただきます。

ghost

正常に動作することを確認しました!
問題なさそうなのでApproveします👍

yocchan-git · 2024-03-10T10:41:13Z

tacoさん、設定めんどくさかったと思いますがありがとうございました！🙇‍♂️

@komagata さん
チームメンバーにapproveいただきました！
お手隙でレビューいただけますと嬉しいです🙏

komagata · 2024-03-19T08:02:35Z

@yocchan-git 独自実装ではなく、github認証と同じようにomniauthを使ってみてください～

yocchan-git · 2024-03-22T10:49:02Z

@komagata さん

ご指摘いただいた点修正しました！
お手隙で再度レビューいただけますと嬉しいです

また、こちらの設定もお手数ですがしていただけますと🙏
よろしくお願いいたします

komagata · 2024-03-24T01:15:35Z

app/controllers/discord_authentications_controller.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true


既存のgithubのコードをみて、それにあわせるか、必要であればそれも含めて統一的なコードになるように作る必要があります。

自分だけ独自のやり方で書くことを繰り返すとサービス全体としてはどんどんカオスなコードになってしまいます。

具体的に言えばgithub認証はuser_sessions_controller#callbackで処理しています。
それに対してdiscord認証はdiscord_authentications_controller#callbackで処理しています。
別のgoogle認証が入ったとしたらどちらに合わせればいいのかわからなくなりますし、
これは明らかに統一性を欠いています

自分の担当部分だけの最小限を目指すのではなく、サービスのコード全体の複雑性が最小限になるように考えてみてください。

例えばですが、user_sessions_controller#callback内のgithub認証の処理を別クラスに書き出し、user_sessions_controller#callbackからgithub認証用のクラスかdiscord認証用のクラスに飛ばすようにするとか、user_sessions_controllerからcallbackメソッドを削除してgithub用のクラスを作るなどです。

このあたりも対応できるようになれば一つレベルアップするとおもいます。

@komagata さん

丁寧にFBいただきありがとうございます！とても参考になります🙇‍♂️
ご指摘いただいた点修正しましたので、お手隙で再レビューお願いいたします

また気になる点ありましたら、ぜひお願いいたします🙏

komagata · 2024-03-29T16:10:32Z

app/models/discord_authentication.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+class DiscordAuthentication


いい感じだとおもいます。

app/models/authentication/discord.rb: Authentication::Discord みたいなまとめ方もよくあるのでこうした方が今後認証の種類が増えたときに自明になるかもです。

@komagata さん

承知しました！
修正しましたので、お手隙で再レビューいただけますと嬉しいです🙏

komagata · 2024-03-30T08:11:43Z

test/models/authentication/discord_test.rb

+class Authentication::DiscordTest < ActiveSupport::TestCase
+  include Rails.application.routes.url_helpers
+
+  test '引数が正常な値の時' do


テスト名は英語でおねがいします～。

@komagata さん

修正しました！ありがとうございます

app/views/users/form/_sns.html.slim

yocchan-git · 2024-04-02T01:16:02Z

test/system/authentication/discord_register_test.rb

+
+require 'application_system_test_case'
+
+class Authentication::DiscordRegisterTest < ApplicationSystemTestCase


Authentication::DiscordRegisterTestはAuthentication::DiscordTestにしたかったのですが、
それではモデルテストとクラス名が被ってしまうためテストが落ちてしまいます。

ですので、Authentication::DiscordRegisterTestというクラス名になりました。Githubも同様です。🙏

こういった場合は、authenticationフォルダにまとめなくて良いなど、
何かございましたら、ぜひ教えていただけますと嬉しいです🙇‍♂️

@yocchan-git なるほどです。integration_testでもおなじことがありました。そちらでもやったように、テストの種類がわかるようにAuthentication::DiscordSystemTestCaseとかの方がいいかもです。

@komagata さん

テストの種類がわかるようにAuthentication::DiscordSystemTestCaseとかの方がいいかもです。

ありがとうございます！修正しました
ご確認よろしくお願いいたします🙇‍♂️

yocchan-git · 2024-04-02T01:26:34Z

@komagata さん

指摘いただいた点修正しました。🙇‍♂️
また、テストコードを追加しました

申し訳ないのですが、再レビューいただけますと嬉しいです🙏
コメントも参考にいただけますと嬉しいです

komagata · 2024-04-03T06:40:12Z

test/system/authentication/discord_test.rb

+
+require 'application_system_test_case'
+
+class Authentication::DiscordSystemTestCase < ApplicationSystemTestCase


@yocchan-git すみません、僕が言っておいてなんですが、たしかクラスの末尾が‘Test‘ じゃないと実行されないとかあったようなきがします。

その場合 Authentication::DiscordSystemTest とかがいいのではないかとおもいます～

@komagata さん

そうなのですね！おっしゃってくださりありがとうございます
修正しましたので、ご確認いただけますと嬉しいです🙏

komagata

確認させていただきました。OKです～👌

いい感じでリファクタリングできていたとおもいます。
その前の状態でも大丈夫なところでは大丈夫なんですが、 @yocchan-git さんならできると思ったのでコメントさせていただきました。
一歩レベルアップした内容にできていたとおもいます～！

yocchan-git · 2024-04-03T09:10:31Z

komagataさん、何度もありがとうございました！
おかげさまでまた一つ掴んだ気がします🙇‍♂️

またぜひ教えていただけますと嬉しいです🙏

yocchan-git · 2024-04-03T09:53:52Z

@komagata さん

Githubの連携とログインがステージング環境で動作せずでして...😅
Githubの連携とログインはステージング環境でも確認できるのでしょうか？

テストコードは通っているので、確認させていただきました。🙏

ステージング環境での動作です↓

動作確認できない画面

komagata · 2024-04-03T11:59:15Z

@yocchan-git ちょっと考えてみてください～。

yocchan-git · 2024-04-04T04:43:38Z

@komagata さん

連絡ありがとうございます！🙏
結論、Github連携・ログインについてステージング環境では動作確認ができないと思ったのですがいかがでしょうか？

理由としては以下の通りです🙇‍♂️

ローカルでは正しく動作している
テストコードは通っている
(失礼で申し訳ないですが)ちょくちょく本番環境でバグが起きてる🤔
ステージング環境でのリダイレクト先が本番環境になっている→環境変数を使い回している？

ということから考えました。
本番環境でGithubについては要確認するつもりです。💪

ただ、ステージング環境で動作確認できないとかってあるのか？とも思っています。🤔
お手隙でご回答いただけますと嬉しいです

komagata · 2024-04-07T22:17:18Z

@yocchan-git Discord認証は確認できているが、GitHub認証は確認できないという意味でしょうか？
でしたらすみません、Discord関連がステージングで確認できていればOKです。

yocchan-git · 2024-04-07T22:57:02Z

@komagata さん
連絡ありがとうございます！

Discord認証は確認できているが、GitHub認証は確認できないという意味でしょうか？

おっしゃる通りです。説明がわかりづらくて申し訳ございませんでした。

machida · 2024-04-12T09:39:04Z

@yocchan-git @komagata
こちらって、間違った値が入力されていた場合ってどうなりますでしょうか？

この機能は4/12にリリースされたのですが、それ以前に手入力で間違った値やDiscordの仕様変更前の古いアカウントが登録されている状態で4/12のリリースを迎えた場合はどうなるか気になりました。

yocchan-git · 2024-04-12T09:42:20Z

@machida さん

今の所「連携解除」がないので
ユーザー側で設定し直しもできず、そのままになってしまいます。

確認せずで申し訳ございません！
これは僕の責任なので対応します笑

少しお待ちいただけますと嬉しいです！🙏

machida · 2024-04-12T09:50:58Z

@yocchan-git すいません、ありがとうございます！Issueを作ったのでポイント追加しますー

yocchan-git self-assigned this Feb 25, 2024

yocchan-git assigned machida Feb 26, 2024

machida removed their assignment Mar 5, 2024

yocchan-git force-pushed the feature/oauth-discord-to-fetch-id branch from 1e098a6 to 057f900 Compare March 5, 2024 10:50

yocchan-git marked this pull request as ready for review March 5, 2024 23:32

yocchan-git requested a review from a user March 5, 2024 23:33

ghost approved these changes Mar 10, 2024

View reviewed changes

yocchan-git requested a review from komagata March 10, 2024 10:41

yocchan-git force-pushed the feature/oauth-discord-to-fetch-id branch from 0cc8170 to bba378b Compare March 22, 2024 08:41

komagata reviewed Mar 24, 2024

View reviewed changes

yocchan-git requested a review from komagata March 24, 2024 08:29

komagata force-pushed the main branch from 033db0e to 39dc85f Compare March 27, 2024 07:19

yocchan-git force-pushed the feature/oauth-discord-to-fetch-id branch from 18b7e23 to 687e55e Compare March 28, 2024 12:11

komagata reviewed Mar 29, 2024

View reviewed changes

yocchan-git force-pushed the feature/oauth-discord-to-fetch-id branch from 687e55e to 855be2a Compare March 30, 2024 03:21

yocchan-git requested a review from komagata March 30, 2024 03:32

komagata reviewed Mar 30, 2024

View reviewed changes

yocchan-git requested a review from komagata March 30, 2024 09:17

komagata reviewed Apr 1, 2024

View reviewed changes

app/views/users/form/_sns.html.slim Outdated Show resolved Hide resolved

app/views/users/form/_sns.html.slim Outdated Show resolved Hide resolved

yocchan-git commented Apr 2, 2024

View reviewed changes

yocchan-git added 8 commits April 3, 2024 08:01

不要なコントローラーを削除する

c7ab26b

空白を追加や命名を変更する

92a88d2

テストが通るように修正する

0904812

returnの下に空白を入れる

53b252c

authenticationフォルダの中にクラスを作成する

8013648

テスト名は英語に修正する

3662c45

Slimの書き方を修正する

ef20341

テストコードの修正をする

43ea81c

yocchan-git force-pushed the feature/oauth-discord-to-fetch-id branch from 1bd20ff to c6b500b Compare April 2, 2024 23:02

テストのクラス名を修正する

e8cbd47

yocchan-git force-pushed the feature/oauth-discord-to-fetch-id branch from c6b500b to e8cbd47 Compare April 2, 2024 23:04

yocchan-git requested a review from komagata April 2, 2024 23:19

komagata reviewed Apr 3, 2024

View reviewed changes

テストのクラス名の末尾がTestで終わるようにする

ad8503e

yocchan-git requested a review from komagata April 3, 2024 07:01

komagata approved these changes Apr 3, 2024

View reviewed changes

komagata merged commit f56b7f2 into main Apr 3, 2024
4 checks passed

komagata deleted the feature/oauth-discord-to-fetch-id branch April 3, 2024 09:02

github-actions bot mentioned this pull request Apr 3, 2024

Release 2024-04-11 11:25:49 #7610

Merged

15 tasks

djkazunoko mentioned this pull request Apr 14, 2024

GitHubログイン機能を実装した djkazunoko/nijikai-go#67

Merged

8 tasks

hirano-vm4 mentioned this pull request Apr 15, 2024

Discord認証の解除ができるようにする #7666

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Discord認証ができるようにする #7436

Discord認証ができるようにする #7436

yocchan-git commented Feb 25, 2024 •

edited

Loading

yocchan-git commented Feb 26, 2024

machida commented Feb 26, 2024

yocchan-git commented Mar 1, 2024 •

edited

Loading

machida commented Mar 5, 2024

yocchan-git commented Mar 5, 2024

ghost commented Mar 6, 2024

ghost left a comment

yocchan-git commented Mar 10, 2024

komagata commented Mar 19, 2024

yocchan-git commented Mar 22, 2024

komagata Mar 24, 2024

yocchan-git Mar 24, 2024

komagata Mar 29, 2024

yocchan-git Mar 30, 2024

komagata Mar 30, 2024

yocchan-git Mar 30, 2024

yocchan-git Apr 2, 2024

komagata Apr 2, 2024

yocchan-git Apr 2, 2024

yocchan-git commented Apr 2, 2024

komagata Apr 3, 2024 •

edited

Loading

yocchan-git Apr 3, 2024

komagata left a comment

yocchan-git commented Apr 3, 2024

yocchan-git commented Apr 3, 2024 •

edited

Loading

komagata commented Apr 3, 2024

yocchan-git commented Apr 4, 2024

komagata commented Apr 7, 2024

yocchan-git commented Apr 7, 2024

machida commented Apr 12, 2024

yocchan-git commented Apr 12, 2024 •

edited

Loading

machida commented Apr 12, 2024

		@@ -0,0 +1,26 @@
		# frozen_string_literal: true

		class DiscordAuthentication


		require 'application_system_test_case'

		class Authentication::DiscordRegisterTest < ApplicationSystemTestCase


		require 'application_system_test_case'

		class Authentication::DiscordSystemTestCase < ApplicationSystemTestCase

Discord認証ができるようにする #7436

Discord認証ができるようにする #7436

Conversation

yocchan-git commented Feb 25, 2024 • edited Loading

Issue

概要

変更確認方法

Discordのaccount_nameをnilにする方法

Screenshot

変更前

変更後

yocchan-git commented Feb 26, 2024

確認方法

コンソールでDiscord アカウントの登録情報を変更する方法

デザインの確認をしていただきたい箇所

machida commented Feb 26, 2024

yocchan-git commented Mar 1, 2024 • edited Loading

machida commented Mar 5, 2024

yocchan-git commented Mar 5, 2024

ghost commented Mar 6, 2024

ghost left a comment

Choose a reason for hiding this comment

yocchan-git commented Mar 10, 2024

komagata commented Mar 19, 2024

yocchan-git commented Mar 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yocchan-git commented Apr 2, 2024

komagata Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

komagata left a comment

Choose a reason for hiding this comment

yocchan-git commented Apr 3, 2024

yocchan-git commented Apr 3, 2024 • edited Loading

komagata commented Apr 3, 2024

yocchan-git commented Apr 4, 2024

komagata commented Apr 7, 2024

yocchan-git commented Apr 7, 2024

machida commented Apr 12, 2024

yocchan-git commented Apr 12, 2024 • edited Loading

machida commented Apr 12, 2024

yocchan-git commented Feb 25, 2024 •

edited

Loading

Discordの`account_name`を`nil`にする方法

yocchan-git commented Mar 1, 2024 •

edited

Loading

komagata Apr 3, 2024 •

edited

Loading

yocchan-git commented Apr 3, 2024 •

edited

Loading

yocchan-git commented Apr 12, 2024 •

edited

Loading