diff --git a/plugins/compound-engineering/.claude-plugin/plugin.json b/plugins/compound-engineering/.claude-plugin/plugin.json index cf317be..e79cd6f 100644 --- a/plugins/compound-engineering/.claude-plugin/plugin.json +++ b/plugins/compound-engineering/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "compound-engineering", - "version": "2.15.2", + "version": "2.16.0", "description": "AI-powered development tools. 27 agents, 19 commands, 13 skills, 2 MCP servers for code review, research, design, and workflow automation.", "author": { "name": "Kieran Klaassen", diff --git a/plugins/compound-engineering/CHANGELOG.md b/plugins/compound-engineering/CHANGELOG.md index 55a6d0c..63cad2c 100644 --- a/plugins/compound-engineering/CHANGELOG.md +++ b/plugins/compound-engineering/CHANGELOG.md @@ -5,6 +5,26 @@ All notable changes to the compound-engineering plugin will be documented in thi The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.16.0] - 2025-12-21 + +### Enhanced + +- **`dhh-rails-style` skill** - Massively expanded reference documentation incorporating patterns from Marc Köhlbrugge's Unofficial 37signals Coding Style Guide: + - **controllers.md** - Added authorization patterns, rate limiting, Sec-Fetch-Site CSRF protection, request context concerns + - **models.md** - Added validation philosophy, let it crash philosophy (bang methods), default values with lambdas, Rails 7.1+ patterns (normalizes, delegated types, store accessor), concern guidelines with touch chains + - **frontend.md** - Added Turbo morphing best practices, Turbo frames patterns, 6 new Stimulus controllers (auto-submit, dialog, local-time, etc.), Stimulus best practices, view helpers, caching with personalization, broadcasting patterns + - **architecture.md** - Added path-based multi-tenancy, database patterns (UUIDs, state as records, hard deletes, counter caches), background job patterns (transaction safety, error handling, batch processing), email patterns, security patterns (XSS, SSRF, CSP), Active Storage patterns + - **gems.md** - Added expanded what-they-avoid section (service objects, form objects, decorators, CSS preprocessors, React/Vue), testing philosophy with Minitest/fixtures patterns + +- **`dhh-ruby-style` skill** - Expanded patterns.md with: + - Development philosophy (ship/validate/refine, fix root causes, vanilla Rails first) + - Rails 7.1+ idioms (params.expect, StringInquirer, positive naming conventions) + - Extraction guidelines (rule of three, start in controller extract when complex) + +### Credits + +- Reference patterns derived from [Marc Köhlbrugge's Unofficial 37signals Coding Style Guide](https://github.com/marckohlbrugge/unofficial-37signals-coding-style-guide) + ## [2.15.2] - 2025-12-21 ### Fixed diff --git a/plugins/compound-engineering/skills/dhh-rails-style/references/architecture.md b/plugins/compound-engineering/skills/dhh-rails-style/references/architecture.md index 29eb548..c68ee6a 100644 --- a/plugins/compound-engineering/skills/dhh-rails-style/references/architecture.md +++ b/plugins/compound-engineering/skills/dhh-rails-style/references/architecture.md @@ -19,21 +19,100 @@ Rails.application.routes.draw do end ``` -**Multi-tenancy** via URL (not subdomain): +**Verb-to-noun conversion:** +| Action | Resource | +|--------|----------| +| close a card | `card.closure` | +| watch a board | `board.watching` | +| mark as golden | `card.goldness` | +| archive a card | `card.archival` | + +**Shallow nesting** - avoid deep URLs: ```ruby -# /{account_id}/boards/... -scope "/:account_id" do - resources :boards +resources :boards do + resources :cards, shallow: true # /boards/:id/cards, but /cards/:id end ``` -Benefits: -- No subdomain DNS complexity -- Deep links work naturally -- Middleware extracts account_id, moves to SCRIPT_NAME -- `Current.account` available everywhere +**Singular resources** for one-per-parent: +```ruby +resource :closure # not resources +resource :goldness +``` + +**Resolve for URL generation:** +```ruby +# config/routes.rb +resolve("Comment") { |comment| [comment.card, anchor: dom_id(comment)] } + +# Now url_for(@comment) works correctly +``` + +## Multi-Tenancy (Path-Based) + +**Middleware extracts tenant** from URL prefix: + +```ruby +# lib/tenant_extractor.rb +class TenantExtractor + def initialize(app) + @app = app + end + + def call(env) + path = env["PATH_INFO"] + if match = path.match(%r{^/(\d+)(/.*)?$}) + env["SCRIPT_NAME"] = "/#{match[1]}" + env["PATH_INFO"] = match[2] || "/" + end + @app.call(env) + end +end +``` + +**Cookie scoping** per tenant: +```ruby +# Cookies scoped to tenant path +cookies.signed[:session_id] = { + value: session.id, + path: "/#{Current.account.id}" +} +``` + +**Background job context** - serialize tenant: +```ruby +class ApplicationJob < ActiveJob::Base + around_perform do |job, block| + Current.set(account: job.arguments.first.account) { block.call } + end +end +``` + +**Recurring jobs** must iterate all tenants: +```ruby +class DailyDigestJob < ApplicationJob + def perform + Account.find_each do |account| + Current.set(account: account) do + send_digest_for(account) + end + end + end +end +``` + +**Controller security** - always scope through tenant: +```ruby +# Good - scoped through user's accessible records +@card = Current.user.accessible_cards.find(params[:id]) + +# Avoid - direct lookup +@card = Card.find(params[:id]) +``` + + ## Authentication @@ -130,8 +209,98 @@ end - No Redis required - Same transactional guarantees as your data - Simpler infrastructure + +**Transaction safety:** +```ruby +# config/application.rb +config.active_job.enqueue_after_transaction_commit = true +``` + +**Error handling** by type: +```ruby +class DeliveryJob < ApplicationJob + # Transient errors - retry with backoff + retry_on Net::OpenTimeout, Net::ReadTimeout, + Resolv::ResolvError, + wait: :polynomially_longer + + # Permanent errors - log and discard + discard_on Net::SMTPSyntaxError do |job, error| + Sentry.capture_exception(error, level: :info) + end +end +``` + +**Batch processing** with continuable: +```ruby +class ProcessCardsJob < ApplicationJob + include ActiveJob::Continuable + + def perform + Card.in_batches.each_record do |card| + checkpoint! # Resume from here if interrupted + process(card) + end + end +end +``` + +## Database Patterns + +**UUIDs as primary keys** (time-sortable UUIDv7): +```ruby +# migration +create_table :cards, id: :uuid do |t| + t.references :board, type: :uuid, foreign_key: true +end +``` + +Benefits: No ID enumeration, distributed-friendly, client-side generation. + +**State as records** (not booleans): +```ruby +# Instead of closed: boolean +class Card::Closure < ApplicationRecord + belongs_to :card + belongs_to :creator, class_name: "User" +end + +# Queries become joins +Card.joins(:closure) # closed +Card.where.missing(:closure) # open +``` + +**Hard deletes** - no soft delete: +```ruby +# Just destroy +card.destroy! + +# Use events for history +card.record_event(:deleted, by: Current.user) +``` + +Simplifies queries, uses event logs for auditing. + +**Counter caches** for performance: +```ruby +class Comment < ApplicationRecord + belongs_to :card, counter_cache: true +end + +# card.comments_count available without query +``` + +**Account scoping** on every table: +```ruby +class Card < ApplicationRecord + belongs_to :account + default_scope { where(account: Current.account) } +end +``` + + ## Current Attributes @@ -339,3 +508,146 @@ end **Webhooks driven by events** - events are the canonical source. + + +## Email Patterns + +**Multi-tenant URL helpers:** +```ruby +class ApplicationMailer < ActionMailer::Base + def default_url_options + options = super + if Current.account + options[:script_name] = "/#{Current.account.id}" + end + options + end +end +``` + +**Timezone-aware delivery:** +```ruby +class NotificationMailer < ApplicationMailer + def daily_digest(user) + Time.use_zone(user.timezone) do + @user = user + @digest = user.digest_for_today + mail(to: user.email, subject: "Daily Digest") + end + end +end +``` + +**Batch delivery:** +```ruby +emails = users.map { |user| NotificationMailer.digest(user) } +ActiveJob.perform_all_later(emails.map(&:deliver_later)) +``` + +**One-click unsubscribe (RFC 8058):** +```ruby +class ApplicationMailer < ActionMailer::Base + after_action :set_unsubscribe_headers + + private + def set_unsubscribe_headers + headers["List-Unsubscribe-Post"] = "List-Unsubscribe=One-Click" + headers["List-Unsubscribe"] = "<#{unsubscribe_url}>" + end +end +``` + + + +## Security Patterns + +**XSS prevention** - escape in helpers: +```ruby +def formatted_content(text) + # Escape first, then mark safe + simple_format(h(text)).html_safe +end +``` + +**SSRF protection:** +```ruby +# Resolve DNS once, pin the IP +def fetch_safely(url) + uri = URI.parse(url) + ip = Resolv.getaddress(uri.host) + + # Block private networks + raise "Private IP" if private_ip?(ip) + + # Use pinned IP for request + Net::HTTP.start(uri.host, uri.port, ipaddr: ip) { |http| ... } +end + +def private_ip?(ip) + ip.start_with?("127.", "10.", "192.168.") || + ip.match?(/^172\.(1[6-9]|2[0-9]|3[0-1])\./) +end +``` + +**Content Security Policy:** +```ruby +# config/initializers/content_security_policy.rb +Rails.application.configure do + config.content_security_policy do |policy| + policy.default_src :self + policy.script_src :self + policy.style_src :self, :unsafe_inline + policy.base_uri :none + policy.form_action :self + policy.frame_ancestors :self + end +end +``` + +**ActionText sanitization:** +```ruby +# config/initializers/action_text.rb +Rails.application.config.after_initialize do + ActionText::ContentHelper.allowed_tags = %w[ + strong em a ul ol li p br h1 h2 h3 h4 blockquote + ] +end +``` + + + +## Active Storage Patterns + +**Variant preprocessing:** +```ruby +class User < ApplicationRecord + has_one_attached :avatar do |attachable| + attachable.variant :thumb, resize_to_limit: [100, 100], preprocessed: true + attachable.variant :medium, resize_to_limit: [300, 300], preprocessed: true + end +end +``` + +**Direct upload expiry** - extend for slow connections: +```ruby +# config/initializers/active_storage.rb +Rails.application.config.active_storage.service_urls_expire_in = 48.hours +``` + +**Avatar optimization** - redirect to blob: +```ruby +def show + expires_in 1.year, public: true + redirect_to @user.avatar.variant(:thumb).processed.url, allow_other_host: true +end +``` + +**Mirror service** for migrations: +```yaml +# config/storage.yml +production: + service: Mirror + primary: amazon + mirrors: [google] +``` + diff --git a/plugins/compound-engineering/skills/dhh-rails-style/references/controllers.md b/plugins/compound-engineering/skills/dhh-rails-style/references/controllers.md index 0f69560..1227238 100644 --- a/plugins/compound-engineering/skills/dhh-rails-style/references/controllers.md +++ b/plugins/compound-engineering/skills/dhh-rails-style/references/controllers.md @@ -62,8 +62,147 @@ end **FilterScoped** - handles complex filtering **TurboFlash** - flash messages via Turbo Stream **ViewTransitions** - disables on page refresh +**BlockSearchEngineIndexing** - sets X-Robots-Tag header +**RequestForgeryProtection** - Sec-Fetch-Site CSRF (modern browsers) + +## Authorization Patterns + +Controllers check permissions via before_action, models define what permissions mean: + +```ruby +# Controller concern +module Authorization + extend ActiveSupport::Concern + + private + def ensure_can_administer + head :forbidden unless Current.user.admin? + end + + def ensure_is_staff_member + head :forbidden unless Current.user.staff? + end +end + +# Usage +class BoardsController < ApplicationController + before_action :ensure_can_administer, only: [:destroy] +end +``` + +**Model-level authorization:** +```ruby +class Board < ApplicationRecord + def editable_by?(user) + user.admin? || user == creator + end + + def publishable_by?(user) + editable_by?(user) && !published? + end +end +``` + +Keep authorization simple, readable, colocated with domain. + + + +## Security Concerns + +**Sec-Fetch-Site CSRF Protection:** +Modern browsers send Sec-Fetch-Site header. Use it for defense in depth: + +```ruby +module RequestForgeryProtection + extend ActiveSupport::Concern + + included do + before_action :verify_request_origin + end + + private + def verify_request_origin + return if request.get? || request.head? + return if %w[same-origin same-site].include?( + request.headers["Sec-Fetch-Site"]&.downcase + ) + # Fall back to token verification for older browsers + verify_authenticity_token + end +end +``` + +**Rate Limiting (Rails 8+):** +```ruby +class MagicLinksController < ApplicationController + rate_limit to: 10, within: 15.minutes, only: :create +end +``` + +Apply to: auth endpoints, email sending, external API calls, resource creation. + + + +## Request Context Concerns + +**CurrentRequest** - populates Current with HTTP metadata: +```ruby +module CurrentRequest + extend ActiveSupport::Concern + + included do + before_action :set_current_request + end + + private + def set_current_request + Current.request_id = request.request_id + Current.user_agent = request.user_agent + Current.ip_address = request.remote_ip + Current.referrer = request.referrer + end +end +``` + +**CurrentTimezone** - wraps requests in user's timezone: +```ruby +module CurrentTimezone + extend ActiveSupport::Concern + + included do + around_action :set_timezone + helper_method :timezone_from_cookie + end + + private + def set_timezone + Time.use_zone(timezone_from_cookie) { yield } + end + + def timezone_from_cookie + cookies[:timezone] || "UTC" + end +end +``` + +**SetPlatform** - detects mobile/desktop: +```ruby +module SetPlatform + extend ActiveSupport::Concern + + included do + helper_method :platform + end + + def platform + @platform ||= request.user_agent&.match?(/Mobile|Android/) ? :mobile : :desktop + end +end +``` + + ## Turbo Stream Responses diff --git a/plugins/compound-engineering/skills/dhh-rails-style/references/frontend.md b/plugins/compound-engineering/skills/dhh-rails-style/references/frontend.md index ba92169..ba2fa65 100644 --- a/plugins/compound-engineering/skills/dhh-rails-style/references/frontend.md +++ b/plugins/compound-engineering/skills/dhh-rails-style/references/frontend.md @@ -14,6 +14,11 @@ render turbo_stream: turbo_stream.morph(@card) ``` +**Global morphing** - enable in layout: +```ruby +turbo_refreshes_with method: :morph, scroll: :preserve +``` + **Fragment caching** with `cached: true`: ```erb <%= render partial: "card", collection: @cards, cached: true %> @@ -22,6 +27,71 @@ render turbo_stream: turbo_stream.morph(@card) **No ViewComponents** - standard partials work fine. + +## Turbo Morphing Best Practices + +**Listen for morph events** to restore client state: +```javascript +document.addEventListener("turbo:morph-element", (event) => { + // Restore any client-side state after morph +}) +``` + +**Permanent elements** - skip morphing with data attribute: +```erb +
+ <%= @count %> +
+``` + +**Frame morphing** - add refresh attribute: +```erb +<%= turbo_frame_tag :assignment, src: path, refresh: :morph %> +``` + +**Common issues and solutions:** + +| Problem | Solution | +|---------|----------| +| Timers not updating | Clear/restart in morph event listener | +| Forms resetting | Wrap form sections in turbo frames | +| Pagination breaking | Use turbo frames with `refresh: :morph` | +| Flickering on replace | Switch to morph instead of replace | +| localStorage loss | Listen to `turbo:morph-element`, restore state | +
+ + +## Turbo Frames + +**Lazy loading** with spinner: +```erb +<%= turbo_frame_tag "menu", + src: menu_path, + loading: :lazy do %> +
Loading...
+<% end %> +``` + +**Inline editing** with edit/view toggle: +```erb +<%= turbo_frame_tag dom_id(card, :edit) do %> + <%= link_to "Edit", edit_card_path(card), + data: { turbo_frame: dom_id(card, :edit) } %> +<% end %> +``` + +**Target parent frame** without hardcoding: +```erb +<%= form_with model: @card, data: { turbo_frame: "_parent" } do |f| %> +``` + +**Real-time subscriptions:** +```erb +<%= turbo_stream_from @card %> +<%= turbo_stream_from @card, :activity %> +``` +
+ ## Stimulus Controllers @@ -85,12 +155,169 @@ export default class extends Controller { ``` ```javascript -// dialog (64 lines) - for modal dialogs -// local-save (59 lines) - localStorage persistence -// drag-and-drop (150 lines) - the largest, still reasonable +// auto-submit (28 lines) - debounced form submission +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static values = { delay: { type: Number, default: 300 } } + + connect() { + this.timeout = null + } + + submit() { + clearTimeout(this.timeout) + this.timeout = setTimeout(() => { + this.element.requestSubmit() + }, this.delayValue) + } + + disconnect() { + clearTimeout(this.timeout) + } +} +``` + +```javascript +// dialog (45 lines) - native HTML dialog management +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + open() { + this.element.showModal() + } + + close() { + this.element.close() + this.dispatch("closed") + } + + clickOutside(event) { + if (event.target === this.element) this.close() + } +} +``` + +```javascript +// local-time (40 lines) - relative time display +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static values = { datetime: String } + + connect() { + this.#updateTime() + } + + #updateTime() { + const date = new Date(this.datetimeValue) + const now = new Date() + const diffMinutes = Math.floor((now - date) / 60000) + + if (diffMinutes < 60) { + this.element.textContent = `${diffMinutes}m ago` + } else if (diffMinutes < 1440) { + this.element.textContent = `${Math.floor(diffMinutes / 60)}h ago` + } else { + this.element.textContent = `${Math.floor(diffMinutes / 1440)}d ago` + } + } +} ``` + +## Stimulus Best Practices + +**Values API** over getAttribute: +```javascript +// Good +static values = { delay: { type: Number, default: 300 } } + +// Avoid +this.element.getAttribute("data-delay") +``` + +**Cleanup in disconnect:** +```javascript +disconnect() { + clearTimeout(this.timeout) + this.observer?.disconnect() + document.removeEventListener("keydown", this.boundHandler) +} +``` + +**Action filters** - `:self` prevents bubbling: +```erb +
+``` + +**Helper extraction** - shared utilities in separate modules: +```javascript +// app/javascript/helpers/timing.js +export function debounce(fn, delay) { + let timeout + return (...args) => { + clearTimeout(timeout) + timeout = setTimeout(() => fn(...args), delay) + } +} +``` + +**Event dispatching** for loose coupling: +```javascript +this.dispatch("selected", { detail: { id: this.idValue } }) +``` + + + +## View Helpers (Stimulus-Integrated) + +**Dialog helper:** +```ruby +def dialog_tag(id, &block) + tag.dialog( + id: id, + data: { + controller: "dialog", + action: "click->dialog#clickOutside keydown.esc->dialog#close" + }, + &block + ) +end +``` + +**Auto-submit form helper:** +```ruby +def auto_submit_form_with(model:, delay: 300, **options, &block) + form_with( + model: model, + data: { + controller: "auto-submit", + auto_submit_delay_value: delay, + action: "input->auto-submit#submit" + }, + **options, + &block + ) +end +``` + +**Copy button helper:** +```ruby +def copy_button(content:, label: "Copy") + tag.button( + label, + data: { + controller: "copy", + copy_content_value: content, + action: "click->copy#copy" + } + ) +end +``` + + ## CSS Architecture @@ -205,3 +432,79 @@ Vanilla CSS with modern features, no preprocessors. .card.closed { } ``` + + +## User-Specific Content in Caches + +Move personalization to client-side JavaScript to preserve caching: + +```erb +<%# Cacheable fragment %> +<% cache card do %> +
+ +
+<% end %> +``` + +```javascript +// Reveal user-specific elements after cache hit +export default class extends Controller { + static values = { currentUser: Number } + static targets = ["ownerOnly"] + + connect() { + const creatorId = parseInt(this.element.dataset.creatorId) + if (creatorId === this.currentUserValue) { + this.ownerOnlyTargets.forEach(el => el.classList.remove("hidden")) + } + } +} +``` + +**Extract dynamic content** to separate frames: +```erb +<% cache [card, board] do %> +
+ <%= turbo_frame_tag card, :assignment, + src: card_assignment_path(card), + refresh: :morph %> +
+<% end %> +``` + +Assignment dropdown updates independently without invalidating parent cache. +
+ + +## Broadcasting with Turbo Streams + +**Model callbacks** for real-time updates: +```ruby +class Card < ApplicationRecord + include Broadcastable + + after_create_commit :broadcast_created + after_update_commit :broadcast_updated + after_destroy_commit :broadcast_removed + + private + def broadcast_created + broadcast_append_to [Current.account, board], :cards + end + + def broadcast_updated + broadcast_replace_to [Current.account, board], :cards + end + + def broadcast_removed + broadcast_remove_to [Current.account, board], :cards + end +end +``` + +**Scope by tenant** using `[Current.account, resource]` pattern. + diff --git a/plugins/compound-engineering/skills/dhh-rails-style/references/gems.md b/plugins/compound-engineering/skills/dhh-rails-style/references/gems.md index 86b6576..00933b9 100644 --- a/plugins/compound-engineering/skills/dhh-rails-style/references/gems.md +++ b/plugins/compound-engineering/skills/dhh-rails-style/references/gems.md @@ -89,8 +89,107 @@ Why: REST is sufficient when you control both ends. GraphQL complexity not justi factory_bot → Fixtures ``` Why: Fixtures are simpler, faster, and encourage thinking about data relationships upfront. + +**Service Objects:** +``` +Interactor, Trailblazer → Fat models +``` +Why: Business logic stays in models. Methods like `card.close` instead of `CardCloser.call(card)`. + +**Form Objects:** +``` +Reform, dry-validation → params.expect + model validations +``` +Why: Rails 7.1's `params.expect` is clean enough. Contextual validations on model. + +**Decorators:** +``` +Draper → View helpers + partials +``` +Why: Helpers and partials are simpler. No decorator indirection. + +**CSS:** +``` +Tailwind, Sass → Native CSS +``` +Why: Modern CSS has nesting, variables, layers. No build step needed. + +**Frontend:** +``` +React, Vue, SPAs → Turbo + Stimulus +``` +Why: Server-rendered HTML with sprinkles of JS. SPA complexity not justified. + +**Testing:** +``` +RSpec → Minitest +``` +Why: Simpler, faster boot, less DSL magic, ships with Rails. + +## Testing Philosophy + +**Minitest** - simpler, faster: +```ruby +class CardTest < ActiveSupport::TestCase + test "closing creates closure" do + card = cards(:one) + assert_difference -> { Card::Closure.count } do + card.close + end + assert card.closed? + end +end +``` + +**Fixtures** - loaded once, deterministic: +```yaml +# test/fixtures/cards.yml +open_card: + title: Open Card + board: main + creator: alice + +closed_card: + title: Closed Card + board: main + creator: bob +``` + +**Dynamic timestamps** with ERB: +```yaml +recent: + title: Recent + created_at: <%= 1.hour.ago %> + +old: + title: Old + created_at: <%= 1.month.ago %> +``` + +**Time travel** for time-dependent tests: +```ruby +test "expires after 15 minutes" do + magic_link = MagicLink.create!(user: users(:alice)) + + travel 16.minutes + + assert magic_link.expired? +end +``` + +**VCR** for external APIs: +```ruby +VCR.use_cassette("stripe/charge") do + charge = Stripe::Charge.create(amount: 1000) + assert charge.paid +end +``` + +**Tests ship with features** - same commit, not before or after. + + ## Decision Framework diff --git a/plugins/compound-engineering/skills/dhh-rails-style/references/models.md b/plugins/compound-engineering/skills/dhh-rails-style/references/models.md index 6df1bb5..4a8a15d 100644 --- a/plugins/compound-engineering/skills/dhh-rails-style/references/models.md +++ b/plugins/compound-engineering/skills/dhh-rails-style/references/models.md @@ -212,3 +212,148 @@ card.close card.ungild ``` + + +## Validation Philosophy + +Minimal validations on models. Use contextual validations on form/operation objects: + +```ruby +# Model - minimal +class User < ApplicationRecord + validates :email, presence: true, format: { with: URI::MailTo::EMAIL_REGEXP } +end + +# Form object - contextual +class Signup + include ActiveModel::Model + + attr_accessor :email, :name, :terms_accepted + + validates :email, :name, presence: true + validates :terms_accepted, acceptance: true + + def save + return false unless valid? + User.create!(email: email, name: name) + end +end +``` + +**Prefer database constraints** over model validations for data integrity: +```ruby +# migration +add_index :users, :email, unique: true +add_foreign_key :cards, :boards +``` + + + +## Let It Crash Philosophy + +Use bang methods that raise exceptions on failure: + +```ruby +# Preferred - raises on failure +@card = Card.create!(card_params) +@card.update!(title: new_title) +@comment.destroy! + +# Avoid - silent failures +@card = Card.create(card_params) # returns false on failure +if @card.save + # ... +end +``` + +Let errors propagate naturally. Rails handles ActiveRecord::RecordInvalid with 422 responses. + + + +## Default Values with Lambdas + +Use lambda defaults for associations with Current: + +```ruby +class Card < ApplicationRecord + belongs_to :creator, class_name: "User", default: -> { Current.user } + belongs_to :account, default: -> { Current.account } +end + +class Comment < ApplicationRecord + belongs_to :commenter, class_name: "User", default: -> { Current.user } +end +``` + +Lambdas ensure dynamic resolution at creation time. + + + +## Rails 7.1+ Model Patterns + +**Normalizes** - clean data before validation: +```ruby +class User < ApplicationRecord + normalizes :email, with: ->(email) { email.strip.downcase } + normalizes :phone, with: ->(phone) { phone.gsub(/\D/, "") } +end +``` + +**Delegated Types** - replace polymorphic associations: +```ruby +class Message < ApplicationRecord + delegated_type :messageable, types: %w[Comment Reply Announcement] +end + +# Now you get: +message.comment? # true if Comment +message.comment # returns the Comment +Message.comments # scope for Comment messages +``` + +**Store Accessor** - structured JSON storage: +```ruby +class User < ApplicationRecord + store :settings, accessors: [:theme, :notifications_enabled], coder: JSON +end + +user.theme = "dark" +user.notifications_enabled = true +``` + + + +## Concern Guidelines + +- **50-150 lines** per concern (most are ~100) +- **Cohesive** - related functionality only +- **Named for capabilities** - `Closeable`, `Watchable`, not `CardHelpers` +- **Self-contained** - associations, scopes, methods together +- **Not for mere organization** - create when genuine reuse needed + +**Touch chains** for cache invalidation: +```ruby +class Comment < ApplicationRecord + belongs_to :card, touch: true +end + +class Card < ApplicationRecord + belongs_to :board, touch: true +end +``` + +When comment updates, card's `updated_at` changes, which cascades to board. + +**Transaction wrapping** for related updates: +```ruby +class Card < ApplicationRecord + def close(creator: Current.user) + transaction do + create_closure!(creator: creator) + record_event(:closed) + notify_watchers_later + end + end +end +``` + diff --git a/plugins/compound-engineering/skills/dhh-ruby-style/references/patterns.md b/plugins/compound-engineering/skills/dhh-ruby-style/references/patterns.md index 5e3ea62..be6b238 100644 --- a/plugins/compound-engineering/skills/dhh-ruby-style/references/patterns.md +++ b/plugins/compound-engineering/skills/dhh-ruby-style/references/patterns.md @@ -619,6 +619,137 @@ EXPOSE 80 443 CMD ["./bin/rails", "server", "-b", "0.0.0.0"] ``` +## Development Philosophy + +### Ship, Validate, Refine + +```ruby +# 1. Merge prototype-quality code to test real usage +# 2. Iterate based on real feedback +# 3. Polish what works, remove what doesn't +``` + +DHH merges features early to validate in production. Perfect code that no one uses is worse than rough code that gets feedback. + +### Fix Root Causes + +```ruby +# ✅ Prevent race conditions at the source +config.active_job.enqueue_after_transaction_commit = true + +# ❌ Bandaid fix with retries +retry_on ActiveRecord::RecordNotFound, wait: 1.second +``` + +Address underlying issues rather than symptoms. + +### Vanilla Rails Over Abstractions + +```ruby +# ✅ Direct ActiveRecord +@card.comments.create!(comment_params) + +# ❌ Service layer indirection +CreateCommentService.call(@card, comment_params) +``` + +Use Rails conventions. Only abstract when genuine pain emerges. + +## Rails 7.1+ Idioms + +### params.expect (PR #120) + +```ruby +# ✅ Rails 7.1+ style +def card_params + params.expect(card: [:title, :description, tags: []]) +end + +# Returns 400 Bad Request if structure invalid + +# Old style +def card_params + params.require(:card).permit(:title, :description, tags: []) +end +``` + +### StringInquirer (PR #425) + +```ruby +# ✅ Readable predicates +event.action.inquiry.completed? +event.action.inquiry.pending? + +# Usage +case +when event.action.inquiry.completed? + send_notification +when event.action.inquiry.failed? + send_alert +end + +# Old style +event.action == "completed" +``` + +### Positive Naming + +```ruby +# ✅ Positive names +scope :active, -> { where(active: true) } +scope :visible, -> { where(visible: true) } +scope :published, -> { where.not(published_at: nil) } + +# ❌ Negative names +scope :not_deleted, -> { ... } # Use :active +scope :non_hidden, -> { ... } # Use :visible +scope :is_not_draft, -> { ... } # Use :published +``` + +## Extraction Guidelines + +### Rule of Three + +```ruby +# First time: Just do it inline +def process + # inline logic +end + +# Second time: Still inline, note the duplication +def process_again + # same logic +end + +# Third time: NOW extract +module Processing + def shared_logic + # extracted + end +end +``` + +Wait for genuine pain before extracting. + +### Start in Controller, Extract When Complex + +```ruby +# Phase 1: Logic in controller +def index + @cards = @board.cards.where(status: params[:status]) +end + +# Phase 2: Move to model scope +def index + @cards = @board.cards.by_status(params[:status]) +end + +# Phase 3: Extract concern if reused +def index + @cards = @board.cards.filtered(params) +end +``` + ## Anti-Patterns to Avoid ### Don't Add Service Objects for Simple Cases