Extract Rails integration from lib/comma.rb

[eitoball]

What

Moves the Rails-specific ActiveSupport.on_load hooks and the render csv: renderer out of lib/comma.rb into dedicated files under lib/comma/rails/.

Why

lib/comma.rb mixed gem boot/requires with ~30 lines of ActionController::Renderers.add :csv implementation and lazy-load hooks for ActiveRecord/Mongoid/ActionController, making the entry point harder to read and the Rails integration impossible to review in isolation. Closes #164.

Changes

• lib/comma.rb: slimmed down to gem boot, Comma::DEFAULT_OPTIONS, and requires only; now requires comma/rails.
• New lib/comma/rails.rb: holds the three ActiveSupport.on_load hooks (:active_record, :mongoid, :action_controller); the action_controller hook now requires comma/rails/renderer lazily instead of registering the renderer inline.
• New lib/comma/rails/renderer.rb: the ActionController::Renderers.add :csv implementation (filename, extension, mime type, BOM, option slicing, send_data), moved verbatim — no logic changes.
• No public API change; lazy-loading behavior and renderer options are unchanged.

Verified via bundle exec rspec against the default (non-Rails), active7.1.6 (ActiveRecord-only), and rails7.1.6 gemfiles (46 / 57 / 70 examples, all passing) plus rubocop (clean).

[Copilot]

🟡 Changes recommended

The extracted renderer contains confirmed correctness issues (Rails version string comparison and write_headers coercion turning true into nil) that can change behavior in real usage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Refactors the gem’s Rails integration by moving the ActiveSupport.on_load hooks and the render csv: renderer implementation out of lib/comma.rb into dedicated lib/comma/rails/* files, keeping the main entrypoint focused on core boot/requires.

Changes:

• Extract Rails lazy-load hooks into lib/comma/rails.rb.
• Extract the ActionController :csv renderer into lib/comma/rails/renderer.rb.
• Slim down lib/comma.rb to core setup plus requiring the Rails integration loader.

File summaries

File	Description
lib/comma.rb	Removes Rails-specific hooks/renderer code and delegates Rails integration loading to comma/rails.
lib/comma/rails.rb	Centralizes ActiveSupport.on_load hooks for ActiveRecord, Mongoid, and ActionController.
lib/comma/rails/renderer.rb	Holds the extracted ActionController::Renderers.add :csv implementation for controller render csv:.

Review details

Suppressed comments (1)

lib/comma/rails/renderer.rb:22

• The write_headers coercion sets the value to nil whenever it isn't a String, so write_headers: true becomes nil and is treated as false downstream (Generator omits headers when write_headers is present and falsy). Preserve non-String booleans and only coerce String values.

    disposition = "attachment; filename=\"#{filename}.#{extension}\""
    send_data data, type: mime_type, disposition: disposition
  end
end

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The changes are a straightforward refactor consistent with the stated acceptance criteria, with only a minor robustness improvement suggested.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The change is a straightforward refactor that preserves behavior while improving separation of concerns, and the prior version-comparison issue is addressed.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The refactor cleanly extracts Rails integration without changing the overall integration structure, and the only noted concern is a minor mismatch between the PR description and an included version-comparison fix.

Review details

Suppressed comments (1)

lib/comma/rails/renderer.rb:11

• The PR description says the renderer was moved “verbatim — no logic changes”, but this file includes a behavior change: the Rails version gate is now based on Rails.gem_version (Gem::Version) rather than a raw string comparison. Please update the PR description (or add a short note in the code) to reflect that this is also a correctness fix, so reviewers/users aren’t misled about the scope.

    mime_type = if Rails.gem_version >= Gem::Version.new('5.0.0')
                  options[:mime_type] || Mime[:csv]
                else
                  options[:mime_type] || Mime::CSV
                end

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite