Railsにて、コントローラのアクションが長くて見通しが悪くなったため、Formオブジェクトとフィルタを使うようにした

Railsを書いている中で、コントローラのアクションが長くなってしまい、コードの見通しが悪くなったことがありました。

そのような時はFormクラスとフィルタを使うのが良いと同僚に教わったため、リファクタリングした時のメモを残します。

環境
仕様について
修正したいアクションについて
Step1. フォームに関するバリデーションをFormオブジェクトへ移動
Step2. コントローラのバリデーションをフィルタへ移動
Step3. begin ~ rescueの begin を削除
ソースコード

環境

Rails 6.1.3.2

仕様について

以下のフォームがあるとします。

　
このフォームでは以下のことを行います。

入力したCodeのバリデーションを行う
- バリデーションをパスした場合は、外部APIを使ってデータを保存する
- バリデーションをバスできなかった場合は、入力フォームへ戻る

　
リファクタリング前は、このコミットの状態だとします。
https://github.com/thinkAmi-sandbox/controller_filter_app/pull/1/commits/bf683d47e826a523a2aa3d9fd569312993ad6dd3

　
以降、ビューやコントローラの詳細を見ていきます。

ビュー

データの保存は外部APIで行うため、今回のフォームはModelとは紐付いていません。

そのため、 form_with を使った時はこんな感じになります。

<h1>登録フォーム</h1>
<%= form_with(url: home_create_path, method: :post, local: true) do |f| %>
  <% flash[:messages]&.each do |message| %>
    <div><%= message %></div>
  <% end %>

  <div class="field">
    <%= f.label :code, 'Code' %>
    <%= f.text_field :code, autofocus: true %>
  </div>

  <div class="actions">
    <%= f.submit '送信' %>
  </div>
<% end %>

コントローラ

2つのアクションメソッドがあります。各機能は以下のとおりです。

new
- 上記ビューを表示
create
- 各種バリデーションと保存
  - バリデーションエラーの場合は、 new アクションへリダイレクト
  - バリデーションをパスしたら、外部APIを使ってデータを保存し、 new アクションへリダイレクト

　
コードとしてはこんな感じです。なお、バリデーション用・保存用の外部APIについては今回省略し、常に成功する ( true を返す) ものとします。

class HomeController < ApplicationController
  def new
  end

  def create
    code = create_params[:code]

    unless code =~ /[0-9]{2}/
      flash[:messages] = ['フォーマットが異なります']
      return redirect_to home_new_path
    end

    unless valid_by_api?(code)
      flash[:messages] = ['バリデーションエラーとなりました']
      return redirect_to home_new_path
    end

    begin
      create_by_api!(code)
      flash[:messages] = ['登録できました']
      redirect_to home_new_path
    rescue StandardError
      flash[:messages] = ['登録に失敗しました']
      redirect_to home_new_path
    end
  end

  private

  def create_params
    params.permit(:code)
  end

  def valid_by_api?(code)
    # 外部APIを使った確認：常にTrueを返す
    true
  end

  def create_by_api!(code)
    # 外部APIを使った登録：常にTrueを返す
    true
  end
end

テストコード (request spec)

上記のビューとコントローラーの動作を確認するための request spec も用意しました。

require 'rails_helper'

RSpec.describe 'Homes', type: :request do
  describe 'GET /new' do
    before { get home_new_path }

    it 'テンプレートが表示されること' do
      expect(response).to have_http_status '200'
      expect(response.body).to include('登録フォーム')
    end
  end

  describe 'POST /create' do
    let(:valid_code) { '12' }

    context '正常系' do
      before do
        params = { code: valid_code }
        post home_create_path(format: :json), params: params
      end

      it '入力画面にリダイレクトすること' do
        expect(response).to have_http_status '302'
      end

      it 'リダイレクト先でエラーが表示されていること' do
        follow_redirect!
        expect(response.body).to include '登録できました'
      end
    end

    context '異常系' do
      context 'code のフォーマットが不正な場合' do
        before do
          params = { code: 'abc' }
          post home_create_path(format: :json), params: params
        end

        it '入力画面にリダイレクトすること' do
          expect(response).to have_http_status '302'
        end

        it 'リダイレクト先でエラーが表示されていること' do
          follow_redirect!
          expect(response.body).to include 'フォーマットが異なります'
        end
      end

      context '外部APIのバリデーションでエラーになる場合' do
        before do
          expect_any_instance_of(HomeController).to receive(:valid_by_api?).once.with(valid_code).and_return(false)

          params = { code: valid_code }
          post home_create_path(format: :json), params: params
        end

        it '入力画面にリダイレクトすること' do
          expect(response).to have_http_status '302'
        end

        it 'リダイレクト先でエラーが表示されていること' do
          follow_redirect!
          expect(response.body).to include 'バリデーションエラーとなりました'
        end
      end

      context '外部APIの登録でエラーになる場合' do
        before do
          expect_any_instance_of(HomeController).to receive(:create_by_api!).once.with(valid_code).and_raise(StandardError)

          params = { code: valid_code }
          post home_create_path(format: :json), params: params
        end

        it '入力画面にリダイレクトすること' do
          expect(response).to have_http_status '302'
        end

        it 'リダイレクト先でエラーが表示されていること' do
          follow_redirect!
          expect(response.body).to include '登録に失敗しました'
        end
      end
    end
  end
end

　
以上が元のコードとなります。

修正したいアクションについて

先ほどの create アクションを再掲します。

アクションの中にバリデーションと保存処理が混在しており、見通しが悪くなっています。

これを修正していきます。

class HomeController < ApplicationController
  def create
    code = create_params[:code]

    unless code =~ /[0-9]{2}/
      flash[:messages] = ['フォーマットが異なります']
      return redirect_to home_new_path
    end

    unless valid_by_api?(code)
      flash[:messages] = ['バリデーションエラーとなりました']
      return redirect_to home_new_path
    end

    begin
      create_by_api!(code)
      flash[:messages] = ['登録できました']
      redirect_to home_new_path
    rescue StandardError
      flash[:messages] = ['登録に失敗しました']
      redirect_to home_new_path
    end
  end
# ...

Step1. フォームに関するバリデーションをFormオブジェクトへ移動

今回のバリデーションは

フォームで入力可能な値ではない
フォームで入力可能な値だが、外部APIを使った時に不正な値

の2つがあります。

1.はフォームに関係するバリデーションです。DjangoではFormを使いますが *1 、RailsではFormオブジェクトを使えば良いと教わりました。

そこで、1.のバリデーションをFormオブジェクトへ移していきます。

Formオブジェクトの作成

まずは app/forms/create_form.rb としてファイルを作成します。

Formオブジェクトでは

include ActiveModel::Model をinclude
attr_accessor として、入力項目 code を用意
Model同様、 validates を使ってバリデーションを実装

となります。

class CreateForm
  include ActiveModel::Model

  attr_accessor :code

  validates :code, format: { with: /[0-9]{2}/, message: 'フォーマットが異なります' }
end

コントローラの修正

ビューでFormオブジェクトを使えるように準備します。

ビューでFormオブジェクトを使えるようにするため、アクション new にてフォームオブジェクトのインスタンス変数をセット
アクション create にて入力値のバリデーションをFormオブジェクトで行う

とすると

def create
  code = create_params[:code]

  unless code =~ /[0-9]{2}/
    flash[:messages] = ['フォーマットが異なります']
    return redirect_to home_new_path
  end

が

def create
   if @create_form.invalid?
     flash[:messages] = @create_form.errors.map(&:full_message)
     return redirect_to home_new_path
   end

   code = @create_form.code

となります。

ビューの修正

フォームオブジェクトをビューに紐付けるため、 form_with の model としてFormオブジェクトの変数を渡します。

<%= form_with(model: @create_form, url: home_create_path, method: :post, local: true) do |f| %>

コントローラの再修正

modelを追加したため、create_params が変わります。

params.permit(:code)

から

params.require(:create_form).permit(:code)

と、require にFormオブジェクトの追加が必要になります。

テストコードの修正

create_paramに create_form を追加したため、

context '正常系' do
  before do
    params = { code: valid_code }
...

が

context '正常系' do
  before do
    params = { create_form: { code: valid_code }}
...

になります。

Step1の全体は、こちらのコミットになります。
https://github.com/thinkAmi-sandbox/controller_filter_app/pull/1/commits/07b48411721ee631ae806c58daed23123d7504d8

Step2. コントローラのバリデーションをフィルタへ移動

Formのバリデーションは移動できたものの、アクションにまだバリデーションが含まれてしまっています。

Djangoの login_required デコレータ *2 のような、「バリデーションエラーとなった時はリダイレクトする」のような機能がほしいなと思っていたところ、 before_action フィルタを使えばよいと教わりました。

　
まず、Formオブジェクトにセットするところやバリデーション部分をそれぞれ

def set_form
  @create_form = CreateForm.new(create_params)
end

def validate_form
  return if @create_form.valid?

  flash[:messages] = @create_form.errors.map(&:full_message)
  redirect_to home_new_path
end

def validate_api
  return if valid_by_api?(@create_form.code)

  flash[:messages] = ['バリデーションエラーとなりました']
  redirect_to home_new_path
end

というプライベートメソッドに移動します。

　
次に、 before_action フィルタで、それらのプライベートメソッドを使うよう

class HomeController < ApplicationController
  before_action :set_form, only: :create
  before_action :validate_form, only: :create
  before_action :validate_api, only: :create

とします。

　
その結果 create メソッドは

def create
  begin
    create_by_api!(code)
    flash[:messages] = ['登録できました']
    redirect_to home_new_path
  rescue StandardError
    flash[:messages] = ['登録に失敗しました']
    redirect_to home_new_path
  end
end

と、外部APIで登録する処理だけになりました。

　
なお、

before_action フィルタの数が増加
インスタンス変数にセットしている set_*** 系が増加

などが発生すると、これでも見通しが悪くなるかもしれません。

ただ、今回はそこまでの量ではないため、この形で終えようと思います。

　
Step2の全体は以下のコミットとなります。
https://github.com/thinkAmi-sandbox/controller_filter_app/pull/1/commits/ac415af8b913d732b74f20e32a8b1cc4ff85ed26

Step3. begin ~ rescueの begin を削除

Railsとは関係ないですが、リファクタリングついでです。

Rubyでは begin なしで良いと教わったため、

def create
  create_by_api!(@create_form.code)
  flash[:messages] = ['登録できました']
  redirect_to home_new_path
rescue StandardError
  flash[:messages] = ['登録に失敗しました']
  redirect_to home_new_path
end