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

Q&Aで、メンターと管理者が「質問作成者」を変更できるようにする #7817

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Q&Aで、メンターと管理者が「質問作成者」を変更できるようにする #7817

wants to merge 7 commits into from
+76 −11

Conversation

nishitatsu-dev
Copy link
Contributor

@nishitatsu-dev nishitatsu-dev commented May 30, 2024 •
edited
Loading

Issue

概要

  • Q&Aの「質問の編集画面」で、質問者を指定できる欄を追加した
    （メンターと管理者だけの機能。他のユーザーには表示されない）

変更確認方法

  1. feature/questioner-can-be-changed-by-mentor-or-admin をローカルに取り込む
  2. 機能の表示と動作の確認
    1. 「メンター」または「管理者」でログインする
    2. 「(development) 未解決のQ&A | FBC.
      （http://localhost:3000/questions?target=not_solved）」にアクセスする
    3. 「＋ 質問するボタン」または「既存の質問」をクリックし、編集画面に移動する
    4. 入力画面に「作成者」欄が在ることを確認する（位置は、下記「Screenshot」を参照）
    5. 「作成者」欄で、質問者名を「選択」または「入力」する
      （新規作成の場合は、「タイトル」と「質問文」の入力が必要）
    6. 登録・更新する。（ボタンは以下のどれかが表示される）
      • 登録する：新規作成の場合
      • 質問を公開：WIPボタンで途中保存した質問の場合
      • 更新する：既に公開されている質問を編集した場合
    7. 「個別の質問の画面」または「一覧の画面」で、指定した「質問者」が質問タイトル下に表示されていることを確認する
  3. 一般ユーザーで非表示になる確認
    1. 一般ユーザー（「メンター」「管理者」以外）でログインする
    2. 上記「ⅱ、ⅲ」を行う
    3. 入力画面に「作成者」欄が無いことを確認する

Screenshot

変更前（質問の編集画面）

image

変更後（質問の編集画面）

image

@nishitatsu-dev
Copy link
Contributor Author

nishitatsu-dev commented May 30, 2024 •
edited
Loading

@machida
Q&Aの「question作成者変更」の欄のデザインをお願いします🙏

  • app/views/questions/_form.html.slimの55~67行目が、今回追加した「question作成者変更」の欄です
  • 現状は、周辺の欄を適当にコピペしている状態です

@machida
Copy link
Member

machida commented Jun 3, 2024

@nishitatsu-dev お待たせしました！デザイン調整を行いました。

@machida machida removed their assignment Jun 3, 2024
@nishitatsu-dev
Copy link
Contributor Author

@machida
デザインありがとうございました〜

@nishitatsu-dev nishitatsu-dev force-pushed the feature/questioner-can-be-changed-by-mentor-or-admin branch from 3e09c8e to 8a2ef77 Compare June 7, 2024 06:36
@nishitatsu-dev
Copy link
Contributor Author

変更箇所の補足メモ

変更の意図が分かりにくい部分についてのメモです

  • app/javascript/choices-ui.js
    「１つのページ内に複数のインクリメンタルサーチを置くケース」に対応するコードを追加しました
  • app/views/questions/_form.html.slim
    前半の変更箇所は、元からあったインクリメンタルサーチ（プラクティスの選択）の部分で、
    「ページ内に複数のインクリメンタルサーチを置くケース」に対応するように変更しました
  • app/controllers/questions_controller.rb
    「質問を新規作成した時に、作成者がcurrent_userになってしまう」ことに対して、変更しました
  • test/system/questions_test.rb
    既存の２つのテストの修正は、「ページ内に複数のインクリメンタルサーチを置くケース」への対応です
    （書き方が少し違っていたので統一しました）

@nishitatsu-dev nishitatsu-dev marked this pull request as ready for review June 7, 2024 12:50
@nishitatsu-dev
Copy link
Contributor Author

@nakamu-kazu222
レビューをお願いしたいのですが、よろしいでしょうか？
全く急いでおりません。よろしくお願いいたします🙏

@nishitatsu-dev
Copy link
Contributor Author

@nakamu-kazu222
すみません、お忙しそうなので、他の方に依頼致します🙏

@nishitatsu-dev nishitatsu-dev requested review from masyuko0222 and removed request for nakamu-kazu222 June 11, 2024 07:38
@nishitatsu-dev
Copy link
Contributor Author

@masyuko0222
レビューをお願いしたいのですが、よろしいでしょうか？
全く急いでおりません。よろしくお願いいたします🙏

変更箇所に関する補足メモです→ #7817 (comment)
不明点あれば、気軽にご連絡ください〜

@masyuko0222
Copy link
Contributor

@nishitatsu-dev
承知いたしました〜。

@nishitatsu-dev
Copy link
Contributor Author

@masyuko0222
ありがとうございます！よろしくお願いします🙏

@nakamu-kazu222
Copy link
Contributor

@nishitatsu-dev

申し訳ございません
返信が遅れました

次の機会がありましたら、よろしくお願いいたします

masyuko0222
masyuko0222 reviewed Jun 17, 2024
View reviewed changes
Copy link
Contributor

@masyuko0222 masyuko0222 left a comment

Choose a reason for hiding this comment

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

動作確認大丈夫でした！

気になる点をコメントしましたので、ご確認よろしくお願いいたします。

app/views/questions/_form.html.slim Outdated
= f.label :user_id, '作成者', class: 'a-form-label'
.select-user
- if question.new_record?
= f.collection_select(:user_id, User.order(:login_name), :id, :login_name, { selected: current_user },
Copy link
Contributor

Choose a reason for hiding this comment

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

初期値としてcurrent_userを選択しておくことを期待されているのかと思いますが、今はなっていません。

selectedオプションにはUserオブジェクトではなく、Userのidを渡すべきかと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます。チェックしたつもりになっていました🙏
idを渡すように修正しました〜
（動作も確認しました〜）

app/views/questions/_form.html.slim
Copy link
Contributor

Choose a reason for hiding this comment

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

[NITS]
新規作成時、作成ユーザーを削除(×ボタンを押す)してから作成を試みた際のエラー文言が少し気になりました。

image

Copy link
Contributor Author

@nishitatsu-dev nishitatsu-dev Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

色々とチェックありがとうございます！！勉強になります🙏

  • 原因：以下２つのバリデーションが実行されていました〜
    1. belongs_to :user
      belongs_toは、バリデーションを実行するのがデフォルトで、optional: trueを付けると実行されないそうです。参考：Active Record の関連付け 4.1.2.11 :optional - Railsガイド v6.1。（ご存知でしたらすみません🙇🏻）
    2. validates :user, presence: true
  • 対応：ⅱの方を削除しました〜

@nishitatsu-dev nishitatsu-dev force-pushed the feature/questioner-can-be-changed-by-mentor-or-admin branch from 8a2ef77 to c3fd234 Compare June 17, 2024 13:01
@nishitatsu-dev
Copy link
Contributor Author

@masyuko0222
レビューありがとうございます！
修正しましたので、ご確認お願いします🙏

@masyuko0222 masyuko0222 self-requested a review June 19, 2024 11:00
masyuko0222
masyuko0222 approved these changes Jun 19, 2024
View reviewed changes
Copy link
Contributor

@masyuko0222 masyuko0222 left a comment

Choose a reason for hiding this comment

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

確認できました！OKです！

@nishitatsu-dev
Copy link
Contributor Author

@masyuko0222
ありがとうございます！！！

@nishitatsu-dev
Copy link
Contributor Author

@komagata
レビューをお願い致します🙏

komagata
komagata reviewed Jun 24, 2024
View reviewed changes
app/controllers/questions_controller.rb Outdated
@@ -52,7 +52,8 @@ def new
def edit; end

def create
@question = current_user.questions.new(question_params)
@question = Question.new(question_params)
@question.user = current_user unless admin_login? || current_user.mentor?
Copy link
Member

@komagata komagata Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@nishitatsu-dev

Suggested change
@question.user = current_user unless admin_login? || current_user.mentor?
@question.user = current_user if !admin_or_mentor_login?

unlessとorが両方あると理解しずらいので避けるのがいいと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます🙏提案いただいた書き方に修正しました。
また、app/views/questions/_form.html.slim の、「フォーム表示に関する条件分岐（55行目）」も同じメソッドを使うように修正しました。

@nishitatsu-dev nishitatsu-dev force-pushed the feature/questioner-can-be-changed-by-mentor-or-admin branch from c3fd234 to 624f219 Compare June 25, 2024 10:37
@nishitatsu-dev
Copy link
Contributor Author

@komagata
レビューありがとうございます。
修正しましたので、ご確認お願いします🙏

@komagata
Copy link
Member

komagata commented Jul 2, 2024

@nishitatsu-dev マージコミットが入っているようなので修正してみてください～

nishitatsu-dev and others added 7 commits July 3, 2024 10:51
@nishitatsu-dev nishitatsu-dev force-pushed the feature/questioner-can-be-changed-by-mentor-or-admin branch from 624f219 to 4fe16ef Compare July 3, 2024 02:07
@nishitatsu-dev
Copy link
Contributor Author

@komagata
マージコミットの件、対応しました。ご確認お願いします🙏

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

@komagata komagata komagata left review comments

@masyuko0222 masyuko0222 masyuko0222 approved these changes

Assignees

@nishitatsu-dev nishitatsu-dev

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nishitatsu-dev @machida @masyuko0222 @nakamu-kazu222 @komagata