🎉

ポートフォリオ改善集

に公開

はじめに

昨日はgitの草をはやす作業でかなり時間を使ってしまいできていなかったポートフォリオの修正を行った際の内容をまとめました。

他ユーザーの投稿やコメントが削除できてしまう

ポートフォリオを作成し、学習段階で身につけたスキルだったはずですが
うっかり漏れていてメンターから指摘を受けたので再発防止、備忘録の為、記載します。
削除リンクを検証ツールから書き換えることで可能なので注意が必要です。

修正前はデベロッパーツールでidを変更すると該当する投稿やコメントを削除できる状況でした。
下記が該当するコードになります。

posts_controller.rb
def destroy
  @post = Post.find(params[:id])
  @post.destroy
  redirect_to posts_path
  flash[:notice] = "投稿を削除しました"
end
comments_controller.rb
def destroy
  @comment = Comment.find(params[:id])
  @comment.destroy if @comment
end

このままだとログインユーザーのデータなのかチェックをしていないため、違う会員でも削除ができてしまいます。
なので、下記のように修正します。

posts_controller.rb
def destroy
  @post = Post.find(params[:id])
  if @post.customer_id == current_customer.id
    @post.destroy
    redirect_to posts_path
    flash[:notice] = "投稿を削除しました"
  else
    redirect_to posts_path
    flash[:alert] = "他人の投稿は削除できません"
  end
end
comments_controller.rb
def destroy
  @comment = Comment.find_by(id: params[:id])
  if @comment.user_id == current_user.id
    @comment.destroy if @comment
  else
    redirect_to post_records_path
    flash[:alert] = "他人のコメントは削除できません。"
  end
end

これでログインユーザーのデータなのかチェックを行い違う場合は投稿一覧ページに遷移するようにできました。

before_actionで定義した内容の活用

各アクションの実行前にbefore_actionによってチェックを実行したのですが、そこで@post_recordを定義しているのに再度、各アクション内で@post_recordを定義してしまっていて、処理が重複してしまっていました。

post_records.controller.rb
before_action :is_matching_login_user, only: [:edit, :update, :destroy]
.
.
.

private
    def is_matching_login_user
      @post_record = PostRecord.find(params[:id])
      user = @post_record.user
      unless user.id == current_user.id
        redirect_to homes_calendar_path
      end
    end
 ...
 

def is_matching_login_user内で@post_recordを定義していたのですが下記のように各アクションに再定義してしまっていました。

post_records.controller.rb
 def edit
    @post_record = PostRecord.find(params[:id])"削除"
 end
 
 def update
    @post_record = PostRecord.find(params[:id])"削除"
    if @post_record.update(post_record_params)
      flash[:notice] = "編集が完了しました。"
      redirect_to post_record_path(params[:id])
    else
      flash.now[:alert] = "編集に失敗しました。"
      render :edit
    end
  end
  
  def destroy
    @post_record = PostRecord.find(params[:id])"削除"
    if @post_record.user_id == current_user.id
      @post_record.destroy
      redirect_to post_records_path
      flash[:notice] = "削除が完了しました。"
    else
      redirect_to post_records_path
      flash[:alert] = "他人の投稿は削除できません。"
    end
  end

処理を重複させないのと記述を簡易的にするためにそれぞれの@post_record = PostRecord.find(params[:id])を削除しました。
削除も同じように動作したので問題なかったです!
ここでbefore_actionについて理解が足りないと思ったので簡単にまとめておきます。

before_actonとは?

railsのコントローラーのアクションを実行する前に処理を行いたいときや、同じ記述の処理をまとめたい時に使います。
before_actonはauthenticate_user!などのuser関連で使うものだと思い込んでいました。
思い込みって怖いです...。

基本的な書き方は以下の通りです。

before_action :処理させたいメソッド名

before_action :処理させたいメソッド名, only: [:アクション1,:アクション2]

2つ目の記述の場合、アクション1と2が実行される前にのみ、指定した「処理したいメソッド」が実行されます。

簡単な使い方としてはcontroller内に重複している記述がある場合にリファクタリングする使い方があります。
先ほどの記述の場合は@post_record = PostRecord.find(params[:id])の部分が該当します。

リファクタリングとは?

リファクタリング(refactoring)という語の本来の意味は「再設計」を意味します。
リファクタリングを行う目的は「プログラマーなど作業者の負担を減らすため」「ソースコードを読みやすくするため」「問題発生時に早く処理するため」「システムを安定して長く使うため」といった理由が挙げられます。

まとめ

ポートフォリオでメンターさんに指摘された改善点の修正点をまとめました!!
これ以外にもチャレンジ機能についても指摘を頂いたので、時間の余裕を見て実装していきたいと思います。
そして今日はついにキャリアサポートの方と面談をしました!
希望条件や就活の進め方についてのすり合わせをおこなったのですが、ついに始まったなという感覚です。
準備でやることだらけなので学習時間を減らさないようにしていきます!!

Discussion