STORES 予約で Web エンジニアをしている osd です。実装漏れの問題はプロダクト開発の中でも根本的な解決が難しい問題の一つだと思っていて、その問題を解決する一例として「複製機能の実装漏れを防ぐ仕組み」についてお話しします。

予約ページとは

STORES 予約では、予約受付の設定が区切られる予約ページという概念が存在します。予約ページでは

価格の設定
支払い方法の設定
担当スタッフの設定
予約受付時間の設定

などの設定を行うことができ、よく機能追加や改修が行われる箇所の一つです。

複製機能とは

複雑な設定値を持つこともあり、設定漏れや再設定の作業を削減するために対象の予約ページを複製して新しい予約ページを作成する機能が存在しています。複製機能では設定値によっては仕様上設定が引き継がれないものも存在しますが、基本的な設定は引き継がれて新しい予約ページが作成されます。

複製機能の実装漏れに気付くことは、実装者のドメイン知識に依存しており、実装段階やレビュー段階での発見が困難なことが難点として挙げられます。

実装漏れを防ぐ仕組み

とある問い合わせを受けて、予約ページ複製時に新機能の設定値が複製されていないことが発覚しました。原因としては設計時の考慮漏れで、今後の対策として機能追加に気を付けることのドキュメントを整備するなど頑張りベースの対応が考えられましたが、仕組みベースで考慮もれに気付ける仕組みを導入することにしました。

複製処理の実装

複製処理では複製対象となる BookingPage から必要な複製対象情報を取得し、新しい BookingPage を作成します。複製対象情報は複製処理の中で複製するメソッドを呼び出すことで複製されます。

大まかな実装は以下のようになります。

# frozen_string_literal: true

class BookingPageDuplicationService
  def initialize(source_booking_page, merchant)
    @source_booking_page = source_booking_page
    @merchant = merchant
  end

  def duplicate
    ApplicationRecord.transaction do
      new_booking_page =
        merchant.booking_pages.create!(
          status: :completed,
          published_at: nil, # 非公開で複製
          ...
        )
      self.dest_booking_page = new_booking_page
      # 以下の duplicate_hogehoges methodは予約複製機能の考慮もれを防ぐために duplicate + association名 のmethod名にする
      duplicate_price_settings
      duplicate_image_settings
      duplicate_auto_mail_settings
      ...
      dest_booking_page.save!
    end
    true
  rescue
    # ...例外処理
  end

  private

  # 価格設定の複製処理
  def duplicate_price_settings
    dest_booking_page.price_settings.build!(
      price: source_booking_page.price_setting.price,
      ...
    )
  end

  # 画像設定の複製処理
  def duplicate_image_settings
    ...
  end

  # 自動メール設定の複製処理
  def duplicate_auto_mail_settings
    ...
  end

  ...
end

新機能を追加する際には、新しい複製処理をまとめた private method を定義して duplicate method 内でその処理を呼び出すことで複製対象に加えることが予想されます。つまり、複製対象の追加漏れを検知するために

複製対象となり得る情報の一覧を取得できるようにする
対象となり得る情報に対応する処理が実装されていない際に確認を促す

という二つのステップを考えました。予約ページに対して何らかの機能が追加される場合、高い確率で追加された機能を表現するモデルの関連が追加されると仮定し、対応する private method が実装されていなかった際に警告することで考慮漏れに気づける可能性が高いという算段です。

複製対象情報の一覧を取得できるようにする

このステップを実現するために、親となる model に対して関連を持つ model 名を取得する reflect_on_all_associations method を利用することにしました。このメソッドは Rails の Active Record の Reflection 関連メソッドであり、クラス内の全ての関連に対する AssociationReflection オブジェクトの配列を取得することができます。参考: ActiveRecord::Reflection::ClassMethods

このメソッドを利用して、対象となる BookingPage の関連情報を取得することで、複製対象となり得る情報を大枠を取得することができると考えました。

booking_page_association_names = BookingPage.reflect_on_all_associations.map { _1.plural_name }

※ BookingPage に対して 2 段階以上ネストしている場合など、複製対象となり得る情報を網羅出来ていないケースも考えられますが、今回はスコープを絞ってこの方法で対応することにしました。

対象となり得る情報に対応する処理が実装されていない際に確認を促す

次に、対象となる情報に差分が生まれた際に実装者に複製機能の確認を促したいです。そこで、取得した一覧情報を元に BookingPage の関連を見て、新しい関連が追加された際に落ちる test を書くことで確認を促すことができると考えました。

describe '予約ページ複製において、新しいbooking_pageへのassociationが正しく認識されていること' do
  # 以下のテストが落ちた際は、BookingPageに対して新しい関連が追加された可能性があります。
  # その場合は以下のテストを修正して明示的に対象外のmodelとしてください。
  # また、新しく複製対象のmodelが実装された際には `duplicate_' + association名(ex. duplicate_ + booking_page_images = `duplicate_booking_page_images`) のメソッドを実装してください。(例外的にメソッド名を変更しているものに関してはコメント共に `exceptionally_duplication_target_model_name` に追加してください)

  it do
    booking_page_model_association_names = BookingPage.reflect_on_all_associations.map { _1.plural_name }
    duplication_untarget_model_name = %w[
      merchants
      staff
      ...
    ]

    # ↓のモデルは例外的にメソッド名が異なるため、ここで除外している
    exceptionally_duplication_target_model_name = [
      # 二つまとめて複製するメソッドとしてduplicate_start_and_end_reservation_periodsで実装済み
      'start_reservation_periods',
      'end_reservation_periods',
      # 別modelの複製処理に含まれている
      'settings',
    ]

    defined_duplication_method_names =
      BookingPageDuplicationRequest
        .private_instance_methods
        .select { |method| method.to_s.start_with?('duplicate_') }
        .map { _1.to_s.sub('duplicate_', '') }

    expected_duplication_method_names =
      booking_page_model_association_names - duplication_untarget_model_name -
      exceptionally_duplication_target_model_name

    expect(defined_duplication_method_names).to include(*expected_duplication_method_names)
  end
end

つまり、実装者が BookingPage に対して関連を追加した際の流れとしては以下のようになります。

flowchart TD
    A[BookingPage に新しい関連を定義] -->|複製処理に対象メソッドの定義なし| B[testが落ちる]
    A[BookingPage に新しい関連を定義] -->|複製処理に対象メソッドの定義あり| complete[完了]
    B -->|複製処理が必要| hantei[test修正 or private method追加]
    hantei -->|関連名に沿ったメソッドを定義する| C[メソッドの追加<br/>複製処理実装のissueを追加]
    hantei -->|関連名に沿わないメソッドを定義する| E[exceptionally_duplication_target_model_name に追加<br/>複製処理実装のissueを追加]
    B -->|複製処理が不必要| D[対象外の関連名として<br/>定義を追加することでtestを修正]
    E --> complete
    C --> complete
    D --> complete