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.

Sign up for GitHub

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

Jump to bottom

質問箱管理画面の追加 #16

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

質問箱管理画面の追加 #16

wants to merge 5 commits into from

Conversation

KishiKyousuke
Copy link

ref #11

目的

質問箱作成の際に設定したパスワードを使用して、投稿された質問一覧を確認できる質問箱管理画面にアクセスできるようにする。

変更点

  • admin/rooms_controller.rbにshowアクションを追加
  • showアクションに固有の認証を追加
  • 質問箱管理画面admin/rooms/show.html.slimを作成
  • 質問箱トップページrooms/show.html.slimに質問箱管理画面へのリンクとパスワード入力フォームを追加

@KishiKyousuke
Copy link
Author

@machida
rooms/show.html.slimに追加した質問箱管理画面へのリンクとパスワード入力フォームのデザインをお願いいたします！
加えて、質問箱管理画面admin/rooms/show.html.slimは質問箱一覧画面のデザインを流用しているため、こちらのデザインのご確認もよろしくお願いいたします！

@KishiKyousuke KishiKyousuke force-pushed the add-question-box-administration-page branch from 414198d to 65d60f9 Compare January 18, 2021 00:09
@KishiKyousuke
Copy link
Author

@MashioSano
もしお時間がよろしければレビューをお願いいたします❗
フォームの位置等のデザインは町田さんに調整していただく予定です。

@KishiKyousuke KishiKyousuke force-pushed the password-can-be-set-in-question-box branch 2 times, most recently from 3aa39d5 to af55a6f Compare January 18, 2021 04:38
sano11o1
sano11o1 suggested changes Jan 19, 2021
View reviewed changes
Copy link

@sano11o1 sano11o1 left a comment

Choose a reason for hiding this comment

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

お待たせしました~
一点気になる点をコメントしました。
ご確認のほどよろしくお願いします!

app/controllers/admin/rooms_controller.rb Outdated
def index
@rooms = Room.page(params[:page])
@topics = Room.page(params[:page])
Copy link

Choose a reason for hiding this comment

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

@roomsから@topicsに変更した理由が気になりました！👀

Copy link
Author

Choose a reason for hiding this comment

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

@MashioSano
ご指摘ありがとうございます！
完全な凡ミスを見逃してました！助かります😄

Copy link
Author

Choose a reason for hiding this comment

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

@MashioSano
ご指摘いただいた箇所を修正しました！
お手すきの際にご確認よろしくお願いいたします！😊

@KishiKyousuke KishiKyousuke force-pushed the add-question-box-administration-page branch 3 times, most recently from 6004752 to 0d07fc1 Compare January 19, 2021 06:07
sano11o1
sano11o1 approved these changes Jan 19, 2021
View reviewed changes
Copy link

@sano11o1 sano11o1 left a comment

Choose a reason for hiding this comment

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

確認しました!LGTMです!🎊

@sano11o1
Copy link

@KishiKyousuke
このプルリクエストのFile Changedの範囲外になりますが、ここのpassword_confirmationはなくても動くと思いました!

tallk/app/controllers/rooms_controller.rb

Lines 37 to 40 in af55a6f

def room_params
params.require(:room).permit(:name, :password, :password_confirmation)
end
end

(既に手元でこの部分を修正されていたらすいません)

@KishiKyousuke KishiKyousuke force-pushed the password-can-be-set-in-question-box branch from af55a6f to 6caeb98 Compare January 19, 2021 09:01
@KishiKyousuke
admin/rooms_controller.rbにshowアクションと質問箱管理画面の認証メソッドを追加
c05ff95 
- 入力されたパスワードが間違っている場合、質問箱のトップ画面にリダイレクトされます
@KishiKyousuke
routes.rbに質問箱管理画面へのルーティングを追加
47eda45
@KishiKyousuke KishiKyousuke force-pushed the add-question-box-administration-page branch from 0d07fc1 to 02e1490 Compare January 19, 2021 09:09
@KishiKyousuke
Copy link
Author

@MashioSano
こちらもご指摘ありがとうございます❗
派生元のブランチに修正を加えておきました😊

@KishiKyousuke
Copy link
Author

@komagata
お手すきの際に、レビューをお願いいたします❗🙇🏻‍♂️

@KishiKyousuke KishiKyousuke force-pushed the add-question-box-administration-page branch from 02e1490 to dd99e46 Compare January 20, 2021 05:35
@KishiKyousuke
Copy link
Author

@komagata
質問箱管理画面の「作成時間」の表示をlメソッドを使用した形式に修正しました。
また、以前マージされた #10 のPRで作成したviewにもlメソッドを使用せず時間表示をしている箇所があったため、Issueを作成しておきます。
お手すきの際にレビューをお願いいたします🙇🏻‍♂️

Base automatically changed from password-can-be-set-in-question-box to master January 20, 2021 05:49
komagata
komagata requested changes Jan 20, 2021
View reviewed changes
config/locales/ja.yml Outdated
@@ -1,4 +1,7 @@
ja:
time:
formats:
default: "%Y/%m/%d %H:%M"
Copy link
Member

Choose a reason for hiding this comment

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

rails-i18n gemとかにはこういうの含まれているのでそれを使うといいかもです。

@komagata
Copy link
Member

@KishiKyousuke レビュー依頼するときはDraftじゃない状態でお願いします〜（未完成でレビュー依頼という意味合いになるので）

@KishiKyousuke KishiKyousuke force-pushed the add-question-box-administration-page branch from dd99e46 to 41bce93 Compare January 20, 2021 07:45
@KishiKyousuke KishiKyousuke marked this pull request as ready for review January 20, 2021 07:46
@KishiKyousuke
Copy link
Author

@komagata
rails-i18nを導入し、時間の表示形式をrails-i18n内に定義されたフォーマットを使うように変更しました。
町田さんにデザインを依頼している状態なので、DraftPullRequestのままでレビューを依頼してしまいました💦

komagata
komagata approved these changes Jan 22, 2021
View reviewed changes
Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

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

確認しました、OKですー🙆‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@komagata komagata komagata approved these changes

@sano11o1 sano11o1 sano11o1 approved these changes

Assignees

@machida machida

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@KishiKyousuke @sano11o1 @komagata @machida