Skip to content

Latest commit

 

History

History
332 lines (238 loc) · 8.3 KB

File metadata and controls

332 lines (238 loc) · 8.3 KB

Rules for AI

1. Code Quality

  • Follow Rubocop: Always consider our Rubocop rules when writing code. If uncertain about a style or convention, check .rubocop.yml or existing code patterns.

2. Techniques & Standards

Ruby Style

Use Hash Value Omission: For Ruby 3.1+ code, use hash value omission syntax when the key and value are the same.

  • BAD: { instance: instance, duration: duration }
  • GOOD: { instance:, duration: }
  • BAD: create(:account, instance: instance, status: status)
  • GOOD: create(:account, instance:, status:)

This applies to:

  • Method arguments
  • Hash literals
  • Factory definitions
  • Any hash where keys match variable names

Avoid Ugly Multi-line Formatting: When array or hash elements span multiple lines, put each element on its own line. Never create partial multi-line elements that result in awkward ,], or ,), patterns on the same line.

  • BAD - partial multi-line creates ugly ,],:

    rows: [
      ['Label', format_number(value),
       format_change(value, previous),],
    ]
  • GOOD - each element on its own line:

    rows: [
      [
        'Label',
        format_number(value),
        format_change(value, previous),
      ],
    ]
  • ALSO GOOD - single line if short enough:

    rows: [
      ['Label', format_number(value), format_change(value, previous)],
    ]

Note: Trailing commas are fine (and enforced by Rubocop). The issue is the formatting that creates visual clutter.

ActiveRecord Queries

Use Ruby Ranges: Prefer Ruby range syntax over string comparisons for date/time and numeric ranges.

  • BAD: .where('created_at > ?', 1.day.ago)
  • GOOD: .where(created_at: 1.day.ago..) (Use endless range for >=)
  • GOOD: .where(created_at: ..1.day.ago) (Use beginless range for <=)
  • GOOD: .where(created_at: ...1.day.ago) (Use range with 3 dots for <)

Exception Handling

Always specify the exception class in rescue clauses:

  • BAD: rescue => e
  • GOOD: rescue StandardError => e
  • BETTER: rescue SpecificError => e

Let errors propagate naturally unless you have a specific reason to catch them.

Database migrations

Always use rails generate migration to create new migrations. Avoid creating migration files manually to ensure proper timestamps and naming conventions.

3. Architecture

No Side Effects in Models: Avoid putting side-effects (like sending notifications or enqueuing jobs) in models. These belong in the Service Layer / Interactors.

4. Testing

Validation Testing

DO NOT write specs that test basic ActiveRecord validations like:

  • validates :field, presence: true
  • validates :field, uniqueness: true
  • validates :field, numericality: true
  • Other standard Rails validations

Why?

  • These validations are tested by Rails itself
  • They add maintenance overhead without adding value
  • They make specs brittle to simple model changes

What to test instead:

  • Complex custom validations with business logic
  • Validation behavior in integration/request specs
  • The actual user-facing behavior that relies on validations

Examples:

BAD - Testing simple validations:

describe Favorite do
  it { is_expected.to validate_presence_of(:user_id) }
  it { is_expected.to validate_uniqueness_of(:topic_id).scoped_to(:user_id) }
end

GOOD - Testing through behavior:

describe "POST /api/v1/favorites" do
  subject(:request) do
    post "/api/v1/favorites", params: { topic_id: }
  end

  let(:user) { create(:user) }
  let(:topic) { create(:topic) }
  let(:topic_id) { topic.id }

  it "creates a favorite" do
    expect { request }.to change(Favorite, :count).by(1)

    expect(response).to have_http_status(:created)
  end
end

GOOD - Testing complex custom validations:

describe 'custom validation' do
  subject(:request) { described_class.new(start_date:, end_date:) }

  context 'when both start_date and end_date are present' do
    let(:start_date) { Date.today }
    let(:end_date) { Date.yesterday }

    it 'requires end_date to be after start_date' do
      expect(request).not_to be_valid
      expect(request.errors[:end_date]).to include('must be after start date')
    end
  end
end

Mocks and Spies

Prefer using instance_spy and the expect(...).to receive style for verification over have_received. This mocking style is generally more readable and separates setup from verification effectively.

Use instance_spy primarily when you stub the initializer to return the spy. This allows you to avoid defining the interface of a double explicitly, as spies allow any message.

BAD:

let(:client) { instance_double(ClientClass, call: nil) }

before do
  allow(ClientClass).to receive(:new).and_return(client)
end

it 'calls the client' do
  subject.perform
  expect(client).to have_received(:call)
end

GOOD:

let(:client) { instance_spy(ClientClass) }

before do
  allow(ClientClass).to receive(:new).and_return(client)
end

it 'calls the client' do
  expect(client).to receive(:call)
  subject.perform
end

For Class Methods

For class methods, set expectations directly before the action - do not use allow + have_received.

BAD:

before do
  allow(UserActivityLog).to receive(:create!)
end

it "logs user activity" do
  action.call

  expect(UserActivityLog).to have_received(:create!).with(
    user:, topic:, action_type: :view,
  )
end

GOOD:

it "logs user activity" do
  expect(UserActivityLog).to receive(:create!).with(
    user:, topic:, action_type: :view,
  )

  action.call
end

Use Contexts for Conditional Scenarios

Always use context blocks to organize specs by conditions or states. This makes specs more readable and clearly separates different scenarios.

Use context for:

  • Different states: context 'when user is authenticated'
  • Different inputs: context 'with valid params'
  • Different configurations: context 'with feature flag enabled'
  • Error conditions: context 'when API fails'
  • Variations: context 'with force: true'

BAD:

describe "#call" do
  it "returns error for missing topic" do
    # ...
  end

  it "creates favorite for authenticated user" do
    user.update!(login_count: 1)
    # ...
  end

  it "logs activity when user is present" do
    # ...
  end
end

GOOD:

describe "#call" do
  context "when topic does not exist" do
    it "returns an error" do
      # ...
    end
  end

  context "with authenticated user" do
    let(:user) { create(:user, login_count: 1) }

    it "creates a favorite" do
      # ...
    end

    it "logs the favorite action" do
      # ...
    end
  end

  context "when database error occurs" do
    before do
      allow(Favorite).to receive(:create!).and_raise(ActiveRecord::RecordInvalid)
    end

    it "captures exception and returns error" do
      # ...
    end
  end
end

Benefits:

  • Clear separation of different scenarios
  • Easier to locate specific test cases
  • Better test organization and readability
  • let blocks can be scoped to specific contexts

Side-Effect Records in Before Blocks

When creating records that exist only for their side effects (not directly referenced in tests), use before blocks instead of let!. This keeps let blocks clean and focused on records that are actually referenced.

BAD - Creating side-effect records in let!:

let!(:topic_with_activity) do
  topic = create(:topic, content_provider:)
  create(:user_activity_log, topic:, user:, action_type: :view)
  topic
end

GOOD - Separate the side-effect into a before block:

let!(:topic_with_activity) do
  create(:topic, content_provider:)
end

before do
  create(:user_activity_log, topic: topic_with_activity, user:, action_type: :view)
end

Why?

  • let blocks should return the record they're named after
  • Side effects hidden inside let blocks are harder to understand
  • before blocks clearly signal "setup that happens before tests"
  • Makes test data setup more explicit and readable

5. Workflow

  • Run DB migrations: Make sure to always run any pending database migrations with rails db:migrate RAILS_ENV=test command before running tests.
  • Run Verification: Once a chunk of work is completed, always run both Rubocop checks and relevant tests to ensure no regressions or style violations.