メンターはデフォルトでQ&AをWatch中にする #3120

yamadaab · 2021-08-11T09:24:42Z

#2874

機能
どのユーザーが質問を保存しても、メンターさんのみ自動的に「Watch中」になります。

修正点

questions_controllerを修正しました。
createメソッド内に「watch中」にする「change_to_watching_for_mentor_only」メソッドを追加しました。
質問が保存された後に、メンターさんをUserテーブルから検索します。
検索したメンターさんと質問を紐付けます。

yamadaab · 2021-08-12T02:17:49Z

@udaikue
お手隙の時にレビューをお願い致します。🙇‍♂️
オブジェクト指向など、懸念点はあるのですが駒形さんの意見に合わせて修正しようと思っています。
そのため、機能についてレビューして頂けると助かります🙇‍♂️

udaikue

確認しました！いいと思います〜🙂

yamadaab · 2021-08-12T23:25:26Z

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

@komagata
レビューお願い致します。🙇‍♂️

komagata · 2021-08-13T07:38:41Z

app/controllers/questions_controller.rb

@@ -95,4 +96,14 @@ def questions_property
      QuestionsProperty.new('未解決の質問一覧', '未解決の質問はまだありません。')
    end
  end
+
+  def change_to_watching_for_mentor_only
+    mentors = User.where(mentor: true)


mentorsってsocpeがあるかも？（なかったら作っちゃって大丈夫です）

Userクラスに「mentor」というscopeがありました。User.mentorで同じことができました！
ありがとうございます。

komagata · 2021-08-13T07:39:07Z

app/controllers/questions_controller.rb

@@ -95,4 +96,14 @@ def questions_property
      QuestionsProperty.new('未解決の質問一覧', '未解決の質問はまだありません。')
    end
  end
+
+  def change_to_watching_for_mentor_only


英語として不自然かも？watchを動詞として使っちゃっていいと思います～

「メンターのみ視聴中に変更する」でdeeplで翻訳するとこの訳になりました(^^;
Watchingから始めた場合でも考えたのですが、うまいメソッド名が思いつきませんでした。すみません🙇‍♂️

直訳は要注意です。
しっかり意味を理解した上で、なるべく端的に表現する言葉を最後まで考えてみてください。
Rubyでは「名前重要」が作者の座右の銘になるほど重要視されているので、Ruby業界の人はみんな結構このあたりにうるさい感じです🤭

komagata · 2021-08-26T06:51:50Z

app/controllers/questions_controller.rb

@@ -95,4 +96,13 @@ def questions_property
      QuestionsProperty.new('未解決の質問一覧', '未解決の質問はまだありません。')
    end
  end
+
+  def watching_mentors_only


メソッド名は動詞が原則なので英語の文法的にちょっとおかしいかもです。

sinsoku · 2021-09-01T14:53:24Z

app/controllers/questions_controller.rb

+        watchable: @question
+      )
+    end
+  end


1. メンターの数だけSQLが発行される（N+1）

メンターが10人いると10回INSERT文が発行されて遅くなってしまいます。

insert_all! を使って一回のSQLでレコードを追加するように直す方が良さそうです。

2. Watchの登録が途中で終わる可能性がある

メンターが10人いて、5人目のWatch登録でエラーになると残りはWatchにならずにエラー画面が表示されてしまいそうです。
対応方針は @komagata さんに伺った方が良いかも。

現状のまま途中でエラーが出たら、そのままエラーにする

Watchの登録に失敗することは稀だと思うので、エラー出てから考えるでも良い気はします

エラーを記録しつつ、残りの登録を継続して、最後にエラーを発生させる

エラーになったメンターの人を記録するので、コードが少し複雑になる

@sinsoku
アドバイスありがとうございます！
insert_allを使用する形に修正してみました(^^
但し、rubocopにisert_allは使わないようにメッセージが出てしまいましたので、駒形さんに相談して更に対応したい思います。🙇‍♂️
rubocopのメッセージ：Rails/SkipsModelValidations: Avoid using insert_all because it skips validations.

@komagata
遅くなりすみません。「質問を投稿するとメンターのみ"Watch中"にする」機能を修正しました。
メソッド名の変更しました。すみませんが、以下の確認をお願い致します。

メソッド名：create_mentors_watchは妥当でしょうか。

insert_allは使用しない方がよろしいでしょうか。

sinsokuさんのご指摘のように「メンターが10人いて、5人目のWatch登録でエラーになると残りはWatchにならずにエラー画面が表示されてしまいそう」のような想定をした方がよろしいでしょうか。それとも、別の想定がよろしいでしょうか。

@sinsoku
ご連絡遅くなってすみません。
駒形さんにも伺って、今回はrubocopの指摘を優先することとなりました。
とは言え、アドバイスとても勉強になりました。ありがとうございました。🙇‍♂️

@yamadaab 大変申し訳ありません、改めて考えてみたところメンター12人ぐらいいるので速度的にinsert_allを使った方がいいように思いました。

お手数ですがinsert_allにしてrubocopの指摘はコメントを使って無効化していただければありがたいです。

@komagata
時間がかかりすみません。rubocopに手間取りましたが、無事に修正できたと思います。
ご確認よろしくお願い致します。

yamadaab · 2021-09-16T07:58:39Z

@komagata
insert_allの修正と、「mentorをWatch中にする」メソッド名を「create_mentor_watch」としてみました。
event_callbacks.rbの「create_author_watch」というメソッドと揃えてみました。
ご確認よろしくお願い致します🙇‍♂️

komagata · 2021-09-21T05:47:48Z

app/controllers/questions_controller.rb

+    User.mentor.each do |mentor|
+      watch = Watch.new(
+        user: mentor,
+        watchable: @question
+      )
+      watch.save!


Suggested change

User.mentor.each do |mentor|

watch = Watch.new(

user: mentor,

watchable: @question

)

watch.save!

User.mentor.each do |mentor|

Watch.create!(

user: mentor,

watchable: @question

)

こういう感じでいいかもです〜

yamadaab · 2021-09-21T09:08:42Z

@komagata
修正が完了しました。
ただし、「提出物にブックマーク機能がほしい。提出物個別ページにブックマークボタンを設置。 #3232」のプルリクで追加されたテストのみ通らない状態のようです。(^^;

このような場合はどのようにすると良いでしょうか。
レビューを継続して頂いてもよろしいのでしょうか(^^;
すみませんが、ご確認よろしくお願い致しますm(__)m

komagata · 2021-09-23T04:57:15Z

@yamadaab Merge Commitができちゃってるかも？

yamadaab · 2021-09-27T09:42:13Z

@komagata
指摘頂いた箇所の修正が終わり、マージコミットも修正が終わりました。
先ほどはお騒がせしました。
確認のほどよろしくお願い致します。🙇‍♂️

komagata

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

machida assigned yamadaab Aug 11, 2021

yamadaab changed the title ~~メンターのみQ&AをWatch中にする機能~~ メンターはデフォルトでQ&AをWatch中にする Aug 12, 2021

yamadaab requested a review from udaikue August 12, 2021 02:15

yamadaab marked this pull request as ready for review August 12, 2021 02:15

udaikue approved these changes Aug 12, 2021

View reviewed changes

komagata requested changes Aug 13, 2021

View reviewed changes

yamadaab force-pushed the feature/Mentors_will_be_on_Watch_for_QandA_by_default branch from a9b94da to f8647af Compare August 20, 2021 06:20

komagata requested changes Aug 26, 2021

View reviewed changes

sinsoku reviewed Sep 1, 2021

View reviewed changes

yamadaab force-pushed the feature/Mentors_will_be_on_Watch_for_QandA_by_default branch from 3cf4814 to 2b69468 Compare September 3, 2021 07:24

yamadaab force-pushed the feature/Mentors_will_be_on_Watch_for_QandA_by_default branch 3 times, most recently from eb3ed64 to 6c7e7c6 Compare September 16, 2021 07:36

komagata requested changes Sep 21, 2021

View reviewed changes

yamadaab force-pushed the feature/Mentors_will_be_on_Watch_for_QandA_by_default branch from 44af325 to 14ebae6 Compare September 27, 2021 09:11

メンターのみQ&AをWatch中にする機能

fd214e8

yamadaab force-pushed the feature/Mentors_will_be_on_Watch_for_QandA_by_default branch from 14ebae6 to fd214e8 Compare October 2, 2021 12:11

rubocop対応

33183b2

yamadaab force-pushed the feature/Mentors_will_be_on_Watch_for_QandA_by_default branch from ee5898e to 33183b2 Compare October 4, 2021 00:39

komagata approved these changes Oct 4, 2021

View reviewed changes

komagata merged commit bb90f2d into main Oct 4, 2021

komagata deleted the feature/Mentors_will_be_on_Watch_for_QandA_by_default branch October 4, 2021 06:58

github-actions bot mentioned this pull request Oct 4, 2021

Release 2021-10-12 05:41:07 #3339

Merged

14 tasks

Paru871 mentioned this pull request Mar 29, 2022

[Q&A]質問の本公開時にメンター側のWatchを自動的にonにするように修正 #4526

Closed

Paru871 mentioned this pull request Mar 30, 2022

[Q&A]質問の本公開時にメンター側のWatchを自動的にonにするように修正 #4530

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

メンターはデフォルトでQ&AをWatch中にする #3120

メンターはデフォルトでQ&AをWatch中にする #3120

yamadaab commented Aug 11, 2021 •

edited

Loading

yamadaab commented Aug 12, 2021

udaikue left a comment

yamadaab commented Aug 12, 2021

komagata Aug 13, 2021

yamadaab Aug 20, 2021

komagata Aug 13, 2021

yamadaab Aug 20, 2021

komagata Aug 22, 2021

komagata Aug 26, 2021

sinsoku Sep 1, 2021

yamadaab Sep 3, 2021

yamadaab Sep 16, 2021

komagata Sep 28, 2021

yamadaab Oct 4, 2021

yamadaab commented Sep 16, 2021

komagata Sep 21, 2021

yamadaab commented Sep 21, 2021

komagata commented Sep 23, 2021

yamadaab commented Sep 27, 2021 •

edited

Loading

komagata left a comment

メンターはデフォルトでQ&AをWatch中にする #3120

メンターはデフォルトでQ&AをWatch中にする #3120

Conversation

yamadaab commented Aug 11, 2021 • edited Loading

yamadaab commented Aug 12, 2021

udaikue left a comment

Choose a reason for hiding this comment

yamadaab commented Aug 12, 2021

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

1. メンターの数だけSQLが発行される（N+1）

2. Watchの登録が途中で終わる可能性がある

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

yamadaab commented Sep 16, 2021

Choose a reason for hiding this comment

yamadaab commented Sep 21, 2021

komagata commented Sep 23, 2021

yamadaab commented Sep 27, 2021 • edited Loading

komagata left a comment

Choose a reason for hiding this comment

yamadaab commented Aug 11, 2021 •

edited

Loading

yamadaab commented Sep 27, 2021 •

edited

Loading