コメント機能の実装
フィヨルドーブートキャンプでRailsのプラクティス(カリキュラム)をかれこれ数ヶ月やっておりましたが、ギリギリ1月のうちに完了させることができました！！やっと...
最後の課題である「コメントを付けられるようにする」を実装してコードレビューを受けてものすごい修正したので、同じあやまちを繰り返さないためにも振り返ります。
前回の記事↓
前提条件
- 本の管理、ユーザー登録、ユーザーフォローができるRailsアプリがある
- deviseを利用
今回の要件
- アプリに日報(report)をCRUDできる機能を追加する
- 日報を更新・削除できるのは投稿した本人のみ（本のCRUDは制限なし）
- 本と日報の各詳細画面からコメントを投稿できるようにする
- 投稿したコメントは本とレポートの各詳細画面から確認できる
- コメントは内容に加えて、投稿したユーザーの名前（未入力ならメアド）と投稿日時も表示する
- ポリモーフィック関連を使ってbookに付くコメントとreportに付くコメントを共通のコードで動くようにする
すっごい難しかった〜。そして要件には関係ないところもけっこう指摘された。。では修正したところを挙げていきますー。
あ、ポリモーフィックについては今回は書きません。難しかったけどレビューで指摘を受けたところにあんまり関係なかったから。。
本文に出てくるcommentableというのがポリモーフィックの面影(?)です。
修正したこと
勝手に変更・削除しない
indexに表示させる項目を勝手に削除したりしていました。もちろん勉強のために自分だけがさわっているアプリなので支障はないけど、チームで開発することを前提にこういうことまで指摘してもらえるのは喜ばしいことだと思います。勝手に仕様変更ダメ、ぜったい。
ちなみにフィヨルドのプラクティスでは、ゼロから自分で作ったアプリではなく、既存のアプリに修正を加える形で実装していきます。
コメントを備忘録にしない
つい忘れてしまいそうだからと自分のために書く必要のないコメントをメモのように書いてしまうんですよ...。しかしコメントは基本的にWHY?を書くところ。
不要なパラメーターを受け取っていた
コントローラーに、例えば
def person_params params.require(:person).permit(:name, :age) end
のように記述するところで、必要ない項目をpermitの中に書いていました。ちゃんと確認しよう。
indexの並び順を考慮しよう
allだと突然並び順がかわるので、orderを使うなどしましょう。
def index @reports = Report.all # @reports = Report.order(:id)に修正 end
findを多用しない
「そのコメントを書いたユーザー」をUser.find(xxxxx)と書いたりしていたけど、comment.userと書けば取得できる。なんてスッキリ。
findを使うとそのたびにDBにクエリが投げられる可能性があるそう。
長くなったらメソッドを定義しよう
要件の「ユーザーの名前（未入力ならメアド）を表示させる」を
User.find(report.user_id).name.empty?
のように書いていたけど、name_or_emailメソッドを定義するとreport.name_or_emailと書ける。report以外にもコメントにも使うし。
ちなみにユーザー名が存在するかはpresenceメソッドが使える。
モデル定義はbelongs_to→has_manyの順番
絶対というわけじゃないけど、この順番で記述するのが一般的だと知った。
時刻表示はstrftimeよりもlメソッド
時刻表示を調整するときstrftimeを使っていたけど、lメソッドのほうが修正などがラクです。
まさに今回のコードレビューをしてくださった伊藤さんの記事を読むとわかりやすい↓
【初心者向け・動画付き】Railsで日時をフォーマットするときはstrftimeよりも、lメソッドを使おう - Qiita)
パーシャルビューではインスタンス変数を直接参照しない
参考：partialではインスタンス変数を参照しない方がいい - Qiita
<%= render 'shared/form', item: @item %>
のように書こう。
HTML5の入力チェック機能を使おう
コメントの入力については、HTMLの入力必須のチェック機能を使って実装するようにした。
参考：input required－HTML5タグリファレンス
これで未入力で投稿された場合の考慮をする必要がなくなる。
コメント入力とインスタンス変数
これ、説明するのが難しいし自分だけがこんなに悩んだ気がするんだけど...
まず、日報のshowはこんな感じになってます。
コメント入力欄について、最初こんな風に書いていた。
<%= form_with(model: [@commentable, @comment], method: :post) do |form| %>
ここのmodel: [@commentable, @comment]についての話です。エラーが出て正しい書き方がわからなかったので質問したらスッキリしました。
これは、上に書いた「パーシャルビューではインスタンス変数を直接参照しない」の話も受けています。@commentじゃなくてcommentと書けるし、もっと言うとComment.newにしちゃえば、ということですが、それを理解するのに時間がかかりました...。
@commentのようにコントローラからインスタンス変数を受け取る場合、基本的に「編集するためにDBに保存されているデータを表示したいから」または「バリデーションエラーになったデータをもう一度表示したいから」という目的があってのことです。
この目的に当てはまらないのでインスタンス変数にする必要がない。コメントのバリデーションは「HTML5の入力チェック機能を使おう」に書いた通りなので今回は不要。
今回のコメント入力は「常に新規の入力フォームを表示する」ということになるので、コントローラからインスタンス変数を受け取る必要はありません。そのためにコントローラではcomment = Comment.newする代わりに、直接form_forの中でComment.newしてしまえばいいのではないかと考えることができます。
これを踏まえて、私はmodel: [@commentable, @comment]をmodel: [@commentable, Comment.new]に書きかえました。(@commentableはインスタンス変数のままになっており、「パーシャルビューではインスタンス変数を直接参照しない」を満たしてないけどここでは無視してネ)
そうすると、comments_controllerのcreateアクションに記述していた以下のコードのComments.newの行がいらなくなると思い込んでました。
def create @comment = Comments.new(comment_params) # この行が不要になるのでは? @comment.user_id = current_user.id if @comment.save (省略)
この行に該当するのがmodel: [@commentable, Comment.new]のComment.newだと思って、この1行が消せるんだと考えました。
でもsaveしなきゃいけないし、どうやるんだ...と思ってわからなくなったのが質問した経緯です。
答えは、createアクションのComments.newは消せません。saveしないといけないから。その前にuser_idも代入してるけど。
しかし、BooksControllerやReportsControllerのshowアクションにある@comment = Comment.newは消せます。これはコメント投稿したあとに、該当する日報や本のコメント一覧を表示させるために必要だから書いてたけど、form_withでmodel: [@commentable, Comment.new]とすることで不要になりました。
長々と説明してしまったけど、自分としてはスッキリしたのでヨシ！！
感想
これ以外にも、書いてないけど間違えていたこともあります。あとは質問するときにきちんと説明せず言葉足らずだったのも反省。質問って難しい。
しかしコードレビューは楽しい！少しずつコードがきれいになっていくのが気持ちいいし、新たな学びがたくさんある。レビュアーは大変でしょうけど...。独学で勉強していたら体験できないことなので、スクールの一番のメリットだと感じる。 しかもフィヨルドはレビューしてくれるメンター陣が超豪華！！
