コードレビューと3R — チームで品質を上げる

# コードレビューチェックリスト（.github/PULL_REQUEST_TEMPLATE.md）
 
## 変更の概要
<!-- この PR は何をしているか 1〜2文で -->
 
## レビュワー向けチェックリスト
 
### R1: Readability（可読性）
- [ ] 変数名・メソッド名を読むだけで意図が伝わるか
- [ ] 1つのメソッドが1つのことだけをしているか（目安: 15行以内）
- [ ] 「なぜ」が不明な箇所にコメントがあるか
- [ ] マジックナンバーが定数化されているか
- [ ] Railsの規約に従っているか（scope, callback の使い方）
 
### R2: Reusability（再利用性）
- [ ] 同じロジックが複数箇所にコピーされていないか
- [ ] 共通化できる処理は既存の抽象に乗れているか
- [ ] 新しいService/Concern/Gemとして切り出す価値があるか
 
### R3: Refactorability（リファクタリング可能性）
- [ ] 新しいユニットテストが書かれているか
- [ ] 変更の影響範囲が限定されているか（疎結合か）
- [ ] 将来の変更を予測した過度な設計になっていないか（YAGNI）
- [ ] テストが依存する外部サービスはモックされているか
 
### テスト
- [ ] テストがグリーンになっているか（CIの確認）
- [ ] カバレッジが下がっていないか
- [ ] エッジケース（空の配列、nil、0など）がテストされているか

# ケンタのコード（PRの内容）
def process_refund(order_id, reason)
  o = Order.find(order_id)
  if o.status == 'completed' && o.created_at > 30.days.ago
    o.status = 'refunded'
    o.refund_reason = reason
    o.save
    r = Refund.create(order_id: o.id, amount: o.total, reason: reason)
    UserMailer.refund_notification(o.user.email, o.id, r.id).deliver_now
    PaymentGateway.refund(o.payment_id, o.total)
    true
  else
    false
  end
end

# 現状: true/false
return false
 
# 提案: ServiceResult でエラー理由も伝える
ServiceResult.failure('返金期限が過ぎています')

# After: ユイのレビューを受けて改善したケンタのコード
class RefundService
  REFUNDABLE_PERIOD = 30.days
 
  def initialize(order)
    @order = order
  end
 
  def call(reason:)
    return ServiceResult.failure('注文が完了状態ではありません') unless @order.completed?
    return ServiceResult.failure('返金期限（30日）が過ぎています') unless within_refundable_period?
 
    ActiveRecord::Base.transaction do
      create_refund_record(reason)
      update_order_status(reason)
      notify_customer
      process_gateway_refund
    end
 
    ServiceResult.success(@refund)
  rescue PaymentGateway::Error => e
    ServiceResult.failure("決済ゲートウェイエラー: #{e.message}")
  end
 
  private
 
  def within_refundable_period?
    @order.created_at > REFUNDABLE_PERIOD.ago
  end
 
  def create_refund_record(reason)
    @refund = Refund.create!(
      order: @order,
      amount: @order.total,
      reason: reason
    )
  end
 
  def update_order_status(reason)
    @order.update!(status: :refunded, refund_reason: reason, refunded_at: Time.current)
  end
 
  def notify_customer
    RefundMailer.notification(@order.user, @order, @refund).deliver_later
  end
 
  def process_gateway_refund
    PaymentGateway.refund(@order.payment_id, @order.total)
  end
end

# テストも追加した
RSpec.describe RefundService do
  let(:order) { create(:order, :completed, created_at: 10.days.ago, total: 5_000) }
  subject(:service) { described_class.new(order) }
 
  describe '#call' do
    context '返金可能な注文の場合' do
      it '返金を成功させる' do
        result = service.call(reason: '商品の不具合')
        expect(result).to be_success
      end
 
      it '注文のステータスをrefundedに更新する' do
        service.call(reason: '商品の不具合')
        expect(order.reload.status).to eq('refunded')
      end
 
      it 'Refundレコードを作成する' do
        expect { service.call(reason: '商品の不具合') }.to change(Refund, :count).by(1)
      end
 
      it '顧客へ通知メールをキューに追加する' do
        expect { service.call(reason: '商品の不具合') }.to have_enqueued_mail(RefundMailer, :notification)
      end
    end
 
    context '30日を超えた注文の場合' do
      let(:order) { create(:order, :completed, created_at: 31.days.ago, total: 5_000) }
 
      it '失敗を返す' do
        result = service.call(reason: '返品希望')
        expect(result).not_to be_success
        expect(result.error).to include('返金期限')
      end
 
      it 'Refundレコードを作成しない' do
        expect { service.call(reason: '返品希望') }.not_to change(Refund, :count)
      end
    end
 
    context '完了していない注文の場合' do
      let(:order) { create(:order, :pending, total: 5_000) }
 
      it '失敗を返す' do
        result = service.call(reason: 'キャンセル希望')
        expect(result).not_to be_success
      end
    end
  end
end

# .github/workflows/code_quality.yml
name: Code Quality Check
 
on:
  pull_request:
    branches: [main, develop]
 
jobs:
  rubocop:
    name: RuboCop（静的解析）
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: ruby/setup-ruby@v1
        with:
          bundler-cache: true
      - name: Run RuboCop
        run: bundle exec rubocop --format github
        # PRに直接インラインコメントが付く（--format github）
 
  test_and_coverage:
    name: テストとカバレッジチェック
    runs-on: ubuntu-latest
    services:
      postgres:
        image: postgres:16
        env:
          POSTGRES_PASSWORD: password
        options: >-
          --health-cmd pg_isready
          --health-interval 10s
    steps:
      - uses: actions/checkout@v4
      - uses: ruby/setup-ruby@v1
        with:
          bundler-cache: true
      - name: Setup database
        run: bundle exec rails db:setup RAILS_ENV=test
      - name: Run RSpec
        run: bundle exec rspec --format progress
        env:
          COVERAGE: 'true'
      - name: カバレッジ閾値チェック
        run: |
          COVERAGE=$(cat coverage/.last_run.json | jq '.result.line')
          echo "Current coverage: ${COVERAGE}%"
          if (( $(echo "$COVERAGE < 80" | bc -l) )); then
            echo "❌ Coverage ${COVERAGE}% is below threshold 80%"
            exit 1
          fi
          echo "✅ Coverage ${COVERAGE}% meets threshold"

# .rubocop.yml — チームの合意事項を設定ファイルで管理
AllCops:
  NewCops: enable
  TargetRubyVersion: 3.2
  Exclude:
    - 'db/schema.rb'
    - 'db/migrate/**/*'
    - 'bin/**/*'
    - 'vendor/**/*'
 
# === Readability 関連 ===
Metrics/MethodLength:
  Max: 15
  Description: 'R1: メソッドは15行以内（単一責任の原則）'
 
Metrics/CyclomaticComplexity:
  Max: 8
  Description: 'R3: 条件分岐は8以下（テストケース数の目安）'
 
Metrics/AbcSize:
  Max: 20
 
# 命名規則
Naming/VariableNumber:
  EnforcedStyle: snake_case
 
# === Reusability 関連 ===
Rails/HasAndBelongsToMany:
  Enabled: true
  Description: 'has_many :through の使用を推奨'
 
Rails/InverseOf:
  Enabled: true
 
# === 安全性 ===
Rails/DynamicFindBy:
  Enabled: true
 
Security/YAMLLoad:
  Enabled: true
 
Rails/SkipsModelValidations:
  Enabled: true
  Description: 'update_column等はバリデーションをスキップするため要注意'

# Dangerfile — PRルールの自動チェック
# https://danger.systems/ruby/
 
# PRサイズチェック（大きすぎるPRを警告）
if git.lines_of_code > 500
  warn("このPRは#{git.lines_of_code}行の変更があります。分割を検討してください")
end
 
# テストファイルなしの変更を警告
has_app_changes = !git.modified_files.grep(%r{app/}).empty?
has_test_changes = !git.modified_files.grep(%r{spec/}).empty?
if has_app_changes && !has_test_changes
  warn("app/以下の変更がありますが、spec/に対応するテストがありません")
end
 
# PRテンプレートの3Rチェックリストが記入されているかチェック
if github.pr_body.length < 100
  warn("PRの説明が短すぎます。何を変更したか、なぜ変更したかを記載してください")
end
 
# CHANGELOGの更新チェック（外部公開Gemの場合）
if git.modified_files.include?('lib/discount_engine/version.rb')
  warn("version.rbが更新されましたが、CHANGELOG.mdが更新されていません") unless git.modified_files.include?('CHANGELOG.md')
end