Skip to content
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

管理画面へのログイン画面を作成する #65 #71

Merged
merged 11 commits into from
Oct 9, 2023

Conversation

Kazuya-Sakashita
Copy link
Owner

@Kazuya-Sakashita Kazuya-Sakashita commented Oct 2, 2023

管理画面へのログイン画面を作成する #65

概要

管理画面へのログイン画面を作成する
サービスのユーザーは管理画面にはログインできないようにしたいので、ユーザーテーブルは分ける
Admin ユーザーの登録は一旦コンソールから行うようにする

完了条件

Admin ユーザーが管理画面にログインできること
サービスのユーザーは管理画面にはログインできないこと

@Kazuya-Sakashita Kazuya-Sakashita changed the base branch from main to develop October 2, 2023 22:54
@Kazuya-Sakashita
Copy link
Owner Author

Admin ユーザーが管理画面にログインできることということですので、管理画面のhomeか何かを作成し、そこに遷移させという意味で問題ないでしょうか?

管理者のログインの際にヘッダーがユーザーとなる仕様なので、こちらは、URLのパスにadminが含まれるかで分岐させadmin用のヘッダーを表示させる方法を考えていますが、その方法で問題ないでしょか?
https://qiita.com/Hbk__17/items/2231f4fd6e41e6ea3736

@Kazuya-Sakashita Kazuya-Sakashita self-assigned this Oct 2, 2023
@arinko-ya
Copy link
Collaborator

Admin ユーザーが管理画面にログインできることということですので、管理画面のhomeか何かを作成し、そこに遷移させという意味で問題ないでしょうか?

合ってます!

管理者のログインの際にヘッダーがユーザーとなる仕様なので、こちらは、URLのパスにadminが含まれるかで分岐させadmin用のヘッダーを表示させる方法を考えていますが、その方法で問題ないでしょか?

admin と user は完全に分離させる想定でいましたが、その方法の方がメリットありそうであればそちらでも大丈夫ですよ。
ちなみに管理画面の主な役割としては今のところですが、ユーザー管理、ペット管理、ピックアップ管理を想定しています。

@Kazuya-Sakashita Kazuya-Sakashita marked this pull request as ready for review October 7, 2023 10:11
Copy link
Collaborator

@arinko-ya arinko-ya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一旦以下の対応をお願いします!

  1. 管理画面にアクセスする場合、ログイン前のユーザーであればログイン画面にリダイレクトし、ログイン済みのユーザーであれば管理画面のホームに遷移するようお願いします。
  2. 管理画面のユーザーを自由に登録できるのは不自然ですので、サインアップ機能は除いてしまってください

@Kazuya-Sakashita
Copy link
Owner Author

91e2087
オーバーライトを変更するだけでは、うまくログイン画面に遷移しませんでした。
いろいろ試しましたが、理由がわかりません。No route matches [DELETE] "/admins/sign_in"が発生しています。
https://dev.to/jnchito/how-to-customize-devise-for-rails-70-and-turbo-ih
heartcombo/devise#5439
このあたりも確認してみたのですが、解決できませんでした。

07:50:11 web.1 | Started DELETE "/admins/sign_in" for 172.25.0.1 at 2023-10-09 07:50:11 +0900
07:50:11 web.1 | Cannot render console from 172.25.0.1! Allowed networks: 127.0.0.0/127.255.255.255, ::1
07:50:11 web.1 |
07:50:11 web.1 | ActionController::RoutingError (No route matches [DELETE] "/admins/sign_in"):
07:50:11 web.1 |
07:50:11 web.1 | Started GET "/admins/home/index" for 172.25.0.1 at 2023-10-09 07:50:11 +0900
07:50:11 web.1 | Cannot render console from 172.25.0.1! Allowed networks: 127.0.0.0/127.255.255.255, ::1
07:50:11 web.1 | Processing by Admins::HomeController#index as HTML
07:50:11 web.1 | Completed 401 Unauthorized in 1ms (ActiveRecord: 0.0ms | Allocations: 288)

@Kazuya-Sakashita
Copy link
Owner Author

遷移がうまく行かないので、一旦、Admins::HomeControllerにbefore_action :authenticate_admin! を使いしています。なぜ、
オーバーライトさせてもうまく動作しないのでしょうか。

@arinko-ya
Copy link
Collaborator

arinko-ya commented Oct 8, 2023

Admins::HomeControllerにbefore_action :authenticate_admin! を使いしています。

すみません私が言葉足らずでした;
↑の対応で合っています!
遷移できなかった理由は上書きしたメソッドの挙動が、指定されたパスに delete でリクエストする、だからですね。
new は get なので404が発生していた感じです。

Copy link
Collaborator

@arinko-ya arinko-ya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

少しだけコメント入れましたがそちら修正いただけたらマージしてしまって OK です!

Comment on lines 4 to 5
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :validatable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/admins/sign_up でサインアップ画面に飛べてしまうので :registerable は外してしまってください

Comment on lines 7 to 24
<li class="nav-item active">
<%= link_to 'トップ', root_path, class: "nav-link" %>
</li>
<li class="nav-item active">
<%= link_to 'このサイトについて', root_path, class: "nav-link" %>
</li>
<% if admin_signed_in? %>
<li class="nav-item active">
<%= link_to '管理画面', '#', class: "nav-link" %>
</li>
<li class="nav-item active">
<%= link_to '管理者ログアウト', destroy_admin_session_path, class: "nav-link", data: { turbo_method: :delete } %>
</li>
<% else %>
<li class="nav-item active">
<%= link_to '管理者ログイン', new_admin_session_path, class: "nav-link" %>
</li>
<% end %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ログイン/ログアウト以外は不要で良いかなと思いました。

@Kazuya-Sakashita
Copy link
Owner Author

修正しました。

@Kazuya-Sakashita Kazuya-Sakashita merged commit e57c457 into develop Oct 9, 2023
@Kazuya-Sakashita Kazuya-Sakashita deleted the feature/65 branch October 9, 2023 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants