Railsアプリに「コメント機能を追加する」課題を終えて

私は現在、フィヨルドブートキャンプにて未経験からプログラミング学習をしているものになります。

この記事は、自分自身が学習したことをまとめ、アウトプットすることを目的として書いていますので、間違いがある可能性がありますのでご注意ください。

今回の課題の概要

既存の本管理 Rails アプリに日報の投稿機能を追加し、それぞれにコメントが投稿できるようにする課題に取り組みました。なお、その際のコメントのモデルは日報にコメントする場合も、本にコメントする場合も共通のモデルを使用するという要件でした。

これ以上の詳細はネタバレになってしまうので割愛しますが、今回取り組んだ際にレビューで助言をもらってことをまとめ、次の課題に活かせるよう自分のためにまとめておきたいと思います。

課題に取り組んで感じた全体的な所感

Railsの課題に取り組むようになってから、今までの課題と違い、Railsは多くのディレクトリとファイルで構成されているため、多くのファイルをまたいでコードを書くようになりました。

そうすることで、あるファイルのコードを変更すると、別のファイルも変更するケースも多くなります。

初めてRailsを触っている状態で正直、まだどこにいて、何を修正して、修正したコードがどこまで影響があるのかをまだ想定ができないところがあり、凡ミスも多くなってしまったな〜と感じました😅

しかし、これも現場で働くエンジニアの方にレビューしていただくことで、この"事実"自体に気づかせてもらえますし、自分を客観的に見たときにどのように見えるのかの理解に繋がり、本当にありがたいと思っています！

かなり初歩的で、本来なら「ちゃんとせい！」で片付けられてしまう部分でも、丁寧に「答え」ではなくあくまで「助言」としての範囲で自走力UPと自らの気付きを担保したままで、補助してもらえるのがフィヨルドの良いところだなと改めて感じました😄

今回レビューを受けて修正したこと

要件の確認

要件の確認はしたつもりが、一部抜けてしまうことがありました。

国際化の部分に関して、ひとまず<h2>コメント投稿</h2>のようにベタ打ちしてあとで国際化しようと思っていて何箇所か取り残されていました😅

やはりファイルが多くなり抜けやすくなる、最終確認が甘いことで起きた問題だと思うので今一度、課題が終わってやったー！という勢いでpushせずに抜けはないかもう少し丁寧に確認していきたいと思います。

自分で確認すれば修正できるレベルのものであれば、それを怠ると無駄なことで相手の時間を奪ってしまうことになってしまうので、フィヨルドにいる間に極力減らしていきたいと思います😄もちろん、気づくのにも経験も必要だったりするとおもうので今のうちに！！

コメントを表示するrenderで無駄なコード記述があった

<%= render 'comments/comments', commentable: @book, comments: @comments %>

としてコントローラー側で表示するコメントを@commentsとしてコメントを取得していましたが（表示上は問題ありませんが）、

<%= render 'comments/comments', commentable: @book, comments: @book.comment %>

そもそもポリモーフィックな関係で紐づけてあるのでcommentableなモデルが共通して使えるメソッドとして.commentがあると気が付いていれば、わざわざコントローラーにコメントを取得するコードは必要ありません！（聞いているか？自分！）

コメントのフォームに必要のないデータを渡していた

app/views/comments/_form.html.erbに以下のように記述していました

<%= render 'comments/form', commentable: @book, comment: @comment %>

app/views/comments/_form.html.erb

<%= form_with model: [commentable, Comment.new] do |f| %>
  <div>
    <%= f.label :content, t('activerecord.attributes.report.content') %>
    <%= f.text_area :content, required: true %>
  </div>

  <div>
    <%= f.submit %>
  </div>
<% end %>

comment: @commentは使われていないので渡す必要はありません。

ログインユーザーがコメント・日報登録者本人でない場合、viewでは削除ボタンが表示されないようにしていたが、DELETEリクエストを飛ばされると受け入れてしまう状態になっていた

コントローラー側でも対処が必要なのを失念していました。deviseを使っているのでauthenticate_user!メソッドを使って修正をしました！

Github -devise-

2020/8/30追記

上記のように修正しましたが、これでは解決できていなかったため追加で修正しました。

以下追記↓

前回上記の助言を受けてコードを見直し、「authenticate_user!メソッドを使って解決〜」としていましたが、これでは解決できていませんでした😅

あらためてどんなヘルパーメソッドなのかを確認。

authenticate_user!メソッド ユーザがログインしているかどうかを確認し、ログインしていない場合はユーザをログインページにリダイレクトする

ということでログインしていれば作成していないユーザーでもDELETEできてしまうのは変わらず！

しかも、今回reports_controller.rbに記載しましたが、親コントローラーであるapplication_controller.rbにすでにbefore_action :authenticate_user!の記載があるので記述する意味もありませんでした😅

Github -devise-

実際にDELETEできてしまうのか確認してみた

レビューしてもらい確認する方法のヒントもらったので、本人でなくても削除できてしまうのか確認してみた。

1. デバッグのために一時的に 本 あるいは 日報 に対する「コメントを削除」ボタンを投稿者以外へ非表示にする制限を解除(誰でも押せるようにする)
2. 通常のブラウザでAさん、シークレットウィンドウでBさんとしてログインする。
3. Aさんとして書き込んだコメントを、Bさん側で消してみる。

結果として現状の実装だと消えてしまう。これがエラーとして処理されるのが望ましい。

ということで以下のように変更しました。

• 再度の修正 @report = current_user.reports.find_by(id: params[:id])を privateにメソッドとして定義してbefore_actionで事前に読み込ませることで本人であることを担保するように変更しました。

class ReportsController < ApplicationController
  before_action :set_report, only: %i[show]
  before_action :correct_user, only: %i[destroy edit update] # 追記

  # 省略

  # DELETE /reports/1 or /reports/1.json
  def destroy
    @report.destroy
    redirect_to reports_url, notice: t('controllers.common.notice_destroy', name: Report.model_name.human)
  end

  private
# 省略

  def correct_user　# 追記
    @report = current_user.reports.find_by(id: params[:id])
  end
end

DRYとなるコードを意識する

同じようなコードで重複している部分がありました！重複部分をメソッド化してDRYなコードになるように修正をしました。

今回は以下のコードが対象でした

<% if comment.user.name.present? %>
  <%= link_to comment.user.name, user_path(comment.user) %>
<% else %>
  <%= link_to comment.user.email, user_path(comment.user) %>
<% end %>

その日報やコメントを投稿したユーザーが 「名前を登録してたらそのまま名前を表示」 「名前の登録がない場合はメールアドレスを表示」 を確認するためのコードを書いていましたが、日報とコメントそれぞれ同じコードになっていたのでuserモデルにメソッドを記述して簡素化しました。

userモデル

  def display_name_priority
    name.presence || email
  end

修正後の記述

<%= link_to comment.user.display_name_priority, user_path(comment.user) %>

これで繰り返しもなくなり、すっきりとしたコードになりました！

インデントが揃っていなかった

これも初歩的なもので確認不足です！いろいろコードをごちゃごちゃ変更しているとついインデントがおかしいのに気が付かずpushしてしまいがちです😅

これもシンプルなミスでうが視認性の高いコードになるように確認をしっかりしていきたいと思います！これもファイルが多岐に渡るとどうしても気が付かないことがあるので、その都度確認する癖をつけたいと思います！

今後力を入れたいこと

コンソールなどを活用したデバッグがまだうまくできていないので意識して使っていきたいと思います。