From f26690143898e577345052cbda92f3e4a96bbae2 Mon Sep 17 00:00:00 2001
From: Bryan Rite <bryan@bryanrite.com>
Date: Mon, 2 Mar 2026 20:39:03 -0800
Subject: [PATCH] Decouple Contracts and Forms

This allows Forms to be used outside the Contracts and Operations which allows for easier usage and isolated testing.

Breaking change, moved from sync to on_sync as the default form postprocess method name. state[:contract] is no longer accessible in prepopulate or sync hooks, you should be using self anyways.
---
 lib/operational/error.rb                    |   1 +
 lib/operational/form.rb                     |  51 ++++
 lib/operational/operation/contract.rb       |  47 +--
 spec/operational/form_spec.rb               | 303 +++++++++++++++++++-
 spec/operational/operation/contract_spec.rb | 274 ------------------
 5 files changed, 369 insertions(+), 307 deletions(-)

diff --git a/lib/operational/error.rb b/lib/operational/error.rb
index b535347..c8d767b 100644
--- a/lib/operational/error.rb
+++ b/lib/operational/error.rb
@@ -3,4 +3,5 @@ class Error < StandardError; end
   class InvalidContractModel < Error; end
   class MethodNotImplemented < Error; end
   class UnknownStepType < Error; end
+  class MethodCollision < Error; end
 end
diff --git a/lib/operational/form.rb b/lib/operational/form.rb
index 2197b69..e404edb 100644
--- a/lib/operational/form.rb
+++ b/lib/operational/form.rb
@@ -4,6 +4,57 @@ class Form
     include ActiveModel::Attributes
     include ActiveModel::Dirty
 
+    def self.inherited(subclass)
+      super
+      subclass.instance_variable_set(:@_operational_sync_check_pending, true)
+    end
+
+    def self.method_added(method_name)
+      super
+      if method_name == :sync && @_operational_sync_check_pending
+        raise MethodCollision,
+          "#{self} defines #sync, which collides with Operational::Form#sync. " \
+          "Rename your method to #on_sync — it will be called automatically during sync."
+      end
+    end
+
+    def self.build(model: nil, model_persisted: nil, state: {}, prepopulate_method: :prepopulate)
+      form = new
+
+      if model
+        raise InvalidContractModel unless model.respond_to?(:attributes)
+        valid_form_attrs = form.attribute_names
+        valid_params = model.attributes.slice(*valid_form_attrs).compact
+        form.assign_attributes(valid_params)
+      end
+
+      form.instance_variable_set(:@_operational_model_persisted, (model_persisted.nil? ? model&.persisted? || false : !!model_persisted))
+      form.instance_variable_set(form.send(:_operational_state_variable), state.dup.freeze)
+      form.send(prepopulate_method, state) if form.respond_to?(prepopulate_method)
+      form.changes_applied if form.respond_to?(:changes_applied)
+      form
+    end
+
+    def validate(params = {})
+      params = params.to_unsafe_h if params.respond_to?(:to_unsafe_h)
+      params = params.with_indifferent_access if params.respond_to?(:with_indifferent_access)
+      valid_params = params.slice(*attribute_names)
+      assign_attributes(valid_params)
+      valid?
+    end
+
+    def sync(model: nil, state: {}, sync_method: :on_sync)
+      if model
+        raise InvalidContractModel unless model.respond_to?(:attributes)
+        valid_model_attrs = model.attribute_names
+        valid_params = attributes.slice(*valid_model_attrs)
+        model.assign_attributes(valid_params)
+      end
+
+      send(sync_method, state) if respond_to?(sync_method)
+      true
+    end
+
     def persisted?
       @_operational_model_persisted
     end
diff --git a/lib/operational/operation/contract.rb b/lib/operational/operation/contract.rb
index 7d29de8..9849e3e 100644
--- a/lib/operational/operation/contract.rb
+++ b/lib/operational/operation/contract.rb
@@ -3,20 +3,16 @@ class Operation
     module Contract
       def self.Build(contract:, name: :contract, model_key: nil, model_persisted: nil, prepopulate_method: :prepopulate)
         lambda do |state|
-          state[name] = contract.new
-
-          if model_key.present?
-            raise InvalidContractModel if !state[model_key]&.respond_to?(:attributes)
-            valid_form_attrs = state[name].attribute_names
-            valid_params = state[model_key].attributes.slice(*valid_form_attrs).compact
-            state[name].assign_attributes(valid_params)
-          end
-
-          state[name].instance_variable_set(:@_operational_model_persisted, (model_persisted.nil? ? state[model_key]&.persisted? || false : !!model_persisted))
-          state[name].instance_variable_set(state[name].send(:_operational_state_variable), state.dup.freeze)
-          state[name].send(prepopulate_method, state) if state[name].respond_to?(prepopulate_method)
-          state[name].changes_applied if state[name].respond_to?(:changes_applied)
-          return true
+          model = model_key.present? ? state[model_key] : nil
+          raise InvalidContractModel if model_key.present? && model.nil?
+
+          state[name] = contract.build(
+            model: model,
+            model_persisted: model_persisted,
+            state: state,
+            prepopulate_method: prepopulate_method
+          )
+          true
         end
       end
 
@@ -28,30 +24,17 @@ def self.Validate(name: :contract, params_path: nil)
             when params_path.is_a?(Array) then params_path
           end
 
-          valid_attrs = state[name].attribute_names
-
           raw_params = state.dig(*valid_path) || {}
-          raw_params = raw_params.to_unsafe_h if raw_params.respond_to?(:to_unsafe_h)
-          raw_params = raw_params.with_indifferent_access
-
-          valid_params = raw_params.slice(*valid_attrs)
-
-          state[name].assign_attributes(valid_params)
-          state[name].valid?
+          state[name].validate(raw_params)
         end
       end
 
-      def self.Sync(name: :contract, model_key: nil, sync_method: :sync)
+      def self.Sync(name: :contract, model_key: nil, sync_method: :on_sync)
         lambda do |state|
-          if model_key.present?
-            raise InvalidContractModel if !state[model_key]&.respond_to?(:attributes)
-            valid_model_attrs = state[model_key].attribute_names
-            valid_params = state[name].attributes.slice(*valid_model_attrs)
-            state[model_key].assign_attributes(valid_params)
-          end
+          model = model_key.present? ? state[model_key] : nil
+          raise InvalidContractModel if model_key.present? && model.nil?
 
-          state[name].send(sync_method, state) if state[name].respond_to?(sync_method)
-          return true
+          state[name].sync(model: model, state: state, sync_method: sync_method)
         end
       end
     end
diff --git a/spec/operational/form_spec.rb b/spec/operational/form_spec.rb
index 9fb12f1..4d2aed7 100644
--- a/spec/operational/form_spec.rb
+++ b/spec/operational/form_spec.rb
@@ -1,6 +1,16 @@
 require 'spec_helper'
 
-RSpec.describe Operational::Operation do
+RSpec.describe Operational::Form do
+  let(:model_class) do
+    Class.new do
+      include ActiveModel::Model
+      include ActiveModel::Attributes
+
+      attribute :name, :string
+      attribute :email, :string
+    end
+  end
+
   let(:form_class) do
     Class.new(Operational::Form) do
       attribute :name, :string
@@ -19,4 +29,295 @@
       expect(form).to respond_to(attr)
     end
   end
+
+  describe ".build" do
+    it "returns an instance of the form" do
+      form = form_class.build
+      expect(form).to be_a form_class
+    end
+
+    describe "model" do
+      let(:model) { model_class.new(name: "Test", email: "test@test.com") }
+
+      it "prepopulates matching attributes from the model" do
+        form = form_class.build(model: model)
+        expect(form.name).to eq "Test"
+      end
+
+      it "ignores model attributes that don't exist on the form" do
+        form = form_class.build(model: model)
+        expect(form.attributes).to_not have_key("email")
+      end
+
+      it "skips nil model attributes to preserve form defaults" do
+        form_with_default = Class.new(Operational::Form) do
+          attribute :name, :string, default: "Default"
+        end
+
+        model = model_class.new(name: nil)
+        form = form_with_default.build(model: model)
+        expect(form.name).to eq "Default"
+      end
+
+      it "raises InvalidContractModel when model doesn't quack like ActiveModel" do
+        expect {
+          form_class.build(model: "not a model")
+        }.to raise_error Operational::InvalidContractModel
+      end
+    end
+
+    describe "model_persisted" do
+      it "defaults to false when no model given" do
+        form = form_class.build
+        expect(form.persisted?).to eq false
+      end
+
+      it "detects persistence from the model" do
+        persisted_model_class = Class.new do
+          include ActiveModel::Model
+          include ActiveModel::Attributes
+          attribute :name, :string
+          def persisted? = true
+        end
+
+        form = form_class.build(model: persisted_model_class.new)
+        expect(form.persisted?).to eq true
+      end
+
+      it "allows explicit override" do
+        form = form_class.build(model_persisted: true)
+        expect(form.persisted?).to eq true
+      end
+
+      it "overrides model persistence when explicitly set" do
+        persisted_model_class = Class.new do
+          include ActiveModel::Model
+          include ActiveModel::Attributes
+          attribute :name, :string
+          def persisted? = true
+        end
+
+        form = form_class.build(model: persisted_model_class.new, model_persisted: false)
+        expect(form.persisted?).to eq false
+      end
+    end
+
+    describe "state" do
+      it "attaches frozen state" do
+        form = form_class.build(state: { key: "value" })
+        frozen_state = form.instance_variable_get(:@state)
+
+        expect(frozen_state).to be_frozen
+        expect(frozen_state[:key]).to eq "value"
+      end
+
+      it "defaults to empty frozen hash" do
+        form = form_class.build
+        frozen_state = form.instance_variable_get(:@state)
+
+        expect(frozen_state).to be_frozen
+        expect(frozen_state).to eq({})
+      end
+    end
+
+    describe "prepopulate" do
+      let(:form_class) do
+        Class.new(Operational::Form) do
+          attribute :name, :string
+
+          def prepopulate(state)
+            self.name = "prepopulated"
+          end
+        end
+      end
+
+      it "calls prepopulate if defined" do
+        form = form_class.build
+        expect(form.name).to eq "prepopulated"
+      end
+
+      it "allows overriding the prepopulate method name" do
+        form_class = Class.new(Operational::Form) do
+          attribute :name, :string
+
+          def custom_prepopulate(state)
+            self.name = "custom"
+          end
+        end
+
+        form = form_class.build(prepopulate_method: :custom_prepopulate)
+        expect(form.name).to eq "custom"
+      end
+    end
+
+    describe "dirty tracking" do
+      let(:model) { model_class.new(name: "Original") }
+
+      it "resets dirty tracking after build" do
+        form = form_class.build(model: model)
+
+        expect(form.changed?).to be false
+        expect(form.changes).to be_empty
+      end
+
+      it "tracks changes after build" do
+        form = form_class.build(model: model)
+        form.name = "Changed"
+
+        expect(form.changed?).to be true
+        expect(form.name_changed?).to be true
+        expect(form.name_was).to eq "Original"
+      end
+    end
+  end
+
+  describe "#validate" do
+    it "assigns matching params and runs validations" do
+      form = form_class.build
+      result = form.validate(name: "Test")
+
+      expect(result).to eq true
+      expect(form.name).to eq "Test"
+    end
+
+    it "returns false when invalid" do
+      form = form_class.build
+      result = form.validate(name: "")
+
+      expect(result).to eq false
+      expect(form.errors.added?(:name, :blank)).to eq true
+    end
+
+    it "filters params to valid attribute names" do
+      form = form_class.build
+      form.validate(name: "Test", unknown: "ignored")
+
+      expect(form.name).to eq "Test"
+      expect(form.attributes).to_not have_key("unknown")
+    end
+
+    it "handles ActionController::Parameters" do
+      params_class = Class.new(Hash) do
+        def to_unsafe_h = to_h
+      end
+
+      params = params_class[name: "Test"]
+      form = form_class.build
+      result = form.validate(params)
+
+      expect(result).to eq true
+      expect(form.name).to eq "Test"
+    end
+
+    it "works with state-dependent validators" do
+      form_class = Class.new(Operational::Form) do
+        attribute :name, :string
+        validate :must_have_context
+
+        def must_have_context
+          errors.add(:base, "no context") unless @state[:context]
+        end
+      end
+
+      form = form_class.build(state: { context: true })
+      expect(form.validate(name: "Test")).to eq true
+
+      form = form_class.build(state: {})
+      expect(form.validate(name: "Test")).to eq false
+    end
+  end
+
+  describe "#sync" do
+    let(:model) { model_class.new(name: "Original", email: "original@test.com") }
+
+    it "syncs matching attributes to the model" do
+      form = form_class.build(model: model)
+      form.validate(name: "Updated")
+      form.sync(model: model)
+
+      expect(model.name).to eq "Updated"
+    end
+
+    it "only syncs attributes that exist on the model" do
+      form = form_class.build(model: model)
+      form.validate(name: "Updated")
+      form.sync(model: model)
+
+      expect(model.email).to eq "original@test.com"
+    end
+
+    it "returns true" do
+      form = form_class.build
+      expect(form.sync).to eq true
+    end
+
+    it "raises InvalidContractModel when model doesn't quack like ActiveModel" do
+      form = form_class.build
+      expect {
+        form.sync(model: "not a model")
+      }.to raise_error Operational::InvalidContractModel
+    end
+
+    it "works without a model" do
+      form = form_class.build
+      expect(form.sync).to eq true
+    end
+
+    describe "on_sync hook" do
+      let(:form_class) do
+        Class.new(Operational::Form) do
+          attribute :name, :string
+
+          def on_sync(state)
+            state[:synced] = true
+          end
+        end
+      end
+
+      it "calls on_sync with state" do
+        state = {}
+        form = form_class.build
+        form.sync(state: state)
+
+        expect(state[:synced]).to eq true
+      end
+
+      it "calls on_sync with model and state" do
+        state = {}
+        form = form_class.build(model: model)
+        form.validate(name: "Updated")
+        form.sync(model: model, state: state)
+
+        expect(model.name).to eq "Updated"
+        expect(state[:synced]).to eq true
+      end
+
+      it "allows overriding the sync method name" do
+        form_class = Class.new(Operational::Form) do
+          attribute :name, :string
+
+          def custom_sync(state)
+            state[:custom] = true
+          end
+        end
+
+        state = {}
+        form = form_class.build
+        form.sync(state: state, sync_method: :custom_sync)
+
+        expect(state[:custom]).to eq true
+      end
+    end
+  end
+
+  describe "MethodCollision" do
+    it "raises when a subclass defines #sync" do
+      expect {
+        Class.new(Operational::Form) do
+          def sync(state)
+          end
+        end
+      }.to raise_error Operational::MethodCollision, /Rename your method to #on_sync/
+    end
+  end
 end
diff --git a/spec/operational/operation/contract_spec.rb b/spec/operational/operation/contract_spec.rb
index 50cc43a..b5b8be6 100644
--- a/spec/operational/operation/contract_spec.rb
+++ b/spec/operational/operation/contract_spec.rb
@@ -5,7 +5,6 @@
     Class.new do
       include ActiveModel::Model
       include ActiveModel::Attributes
-      include ActiveModel::Dirty
 
       attribute :name, :string
       attribute :email, :string
@@ -37,13 +36,6 @@
       expect(state[:contract]).to be_a CreateForm
     end
 
-    it "does not expose the state variable as an attribute" do
-      described_class::Build(contract: CreateForm).call(state)
-
-      expect(state[:contract].instance_variable_get(:@state)).to eq state
-      expect(state[:contract].attributes).to_not include :state
-    end
-
     describe "name" do
       it "allows overriding the state key" do
         described_class::Build(contract: CreateForm, name: :test).call(state)
@@ -71,164 +63,6 @@
           described_class::Build(contract: CreateForm, model_key: :invalid).call({ model: model })
         }.to raise_error Operational::InvalidContractModel
       end
-
-      it "applies the matching attributes from model to contract" do
-        state = { model: model }
-
-        described_class::Build(contract: CreateForm, model_key: :model).call(state)
-
-        expect(state[:contract].name).to eq "Test"
-      end
-
-      describe "with default values on the form" do
-        let(:form_class) do
-          Class.new(Operational::Form) do
-            attribute :name, :string, default: "Default Name"
-            validates :name, presence: true
-          end
-        end
-
-        let(:model) do
-          model_class.new(name: nil, email: "test@test.com")
-        end
-
-        it "does not apply nil attributes from model to contract" do
-          state = { model: model }
-
-          described_class::Build(contract: CreateForm, model_key: :model).call(state)
-
-          expect(state[:contract].name).to eq "Default Name"
-        end
-      end
-    end
-
-    describe "model_persisted" do
-      it "defaults to not persisted" do
-        described_class::Build(contract: CreateForm).call(state)
-
-        expect(state[:contract].persisted?).to eq false
-      end
-
-      it "allows setting the persisted value" do
-        described_class::Build(contract: CreateForm, model_persisted: true).call(state)
-
-        expect(state[:contract].persisted?).to eq true
-      end
-
-      context "with a persisted model being sent" do
-        let(:model_class) do
-          Class.new do
-            include ActiveModel::Model
-            include ActiveModel::Attributes
-
-            attribute :name, :string
-
-            def persisted?
-              true
-            end
-          end
-        end
-
-        let(:state) { { model: model_class.new } }
-
-        it "uses the model's persistence by default" do
-          described_class::Build(contract: CreateForm, model_key: :model).call(state)
-
-          expect(state[:contract].persisted?).to eq true
-        end
-
-        it "allows overriding the model's persistence" do
-          described_class::Build(contract: CreateForm, model_key: :model, model_persisted: false).call(state)
-
-          expect(state[:contract].persisted?).to eq false
-        end
-
-      end
-
-      context "with a non-persisted model being sent" do
-        let(:model_class) do
-          Class.new do
-            include ActiveModel::Model
-            include ActiveModel::Attributes
-
-            attribute :name, :string
-
-            def persisted?
-              false
-            end
-          end
-        end
-
-        let(:state) { { model: model_class.new } }
-
-        it "uses the model's persistence by default" do
-          described_class::Build(contract: CreateForm, model_key: :model).call(state)
-
-          expect(state[:contract].persisted?).to eq false
-        end
-
-        it "allows overriding the model's persistence" do
-          described_class::Build(contract: CreateForm, model_key: :model, model_persisted: true).call(state)
-
-          expect(state[:contract].persisted?).to eq true
-        end
-      end
-    end
-
-    describe "changes_applied" do
-      let(:model) do
-        model_class.new(name: "First", email: "first@test.com")
-      end
-
-      it "calls changes_applied after syncing from model" do
-        state = { model: model }
-
-        step = described_class.Build(contract: CreateForm, model_key: :model)
-        step.call(state)
-
-        contract = state[:contract]
-        expect(contract.name).to eq "First"
-        expect(contract.changed?).to be false
-        expect(contract.changes).to be_empty
-
-        contract.name = "Second"
-        expect(contract.name).to eq "Second"
-        expect(contract.changed?).to be true
-        expect(contract.name_changed?).to be true
-        expect(contract.name_was).to eq("First")
-        expect(contract.changes).to eq({ "name" => ["First", "Second"] })
-        expect(contract.name_change).to eq(["First", "Second"])
-      end
-    end
-
-    describe "prepopulate_method" do
-      let(:form_class) do
-        Class.new(Operational::Form) do
-          attribute :name, :string
-
-          def prepopulate(state)
-            state[:prepopulate] = "test"
-          end
-
-          def other_prepopulate(state)
-            state[:other_prepopulate] = "test"
-          end
-        end
-      end
-
-      it "runs the prepopulate_method if defined" do
-        described_class::Build(contract: CreateForm).call(state)
-
-        expect(state[:prepopulate]).to eq "test"
-        expect(state[:other_prepopulate]).to be_nil
-      end
-
-      it "allows overriding the method name" do
-        described_class::Build(contract: CreateForm, prepopulate_method: :other_prepopulate).call(state)
-
-        expect(state[:other_prepopulate]).to eq "test"
-        expect(state[:prepopulate]).to be_nil
-      end
     end
   end
 
@@ -243,63 +77,6 @@ def other_prepopulate(state)
       expect(described_class::Validate()).to respond_to :call
     end
 
-    it "applies the matching params to the contract" do
-      described_class::Validate().call(state)
-
-      expect(state[:contract].name).to eq "update"
-    end
-
-    context "when form is valid" do
-      it "returns true" do
-        expect(described_class::Validate().call(state)).to eq true
-      end
-
-      it "return empty error object" do
-        described_class::Validate().call(state)
-
-        expect(state[:contract].valid?).to eq true
-        expect(state[:contract].errors).to be_empty
-      end
-    end
-
-    context "when form is invalid" do
-      let(:state) { { params: { email: "update@test.com" }} }
-
-      it "returns false" do
-        validate_step = described_class::Validate()
-
-        expect(validate_step.call(state)).to eq false
-      end
-
-      it "returns error object" do
-        validate_step = described_class::Validate()
-        validate_step.call(state)
-
-        expect(state[:contract].valid?).to eq false
-        expect(state[:contract].errors).to be_present
-        expect(state[:contract].errors.full_messages).to eq ["Name can't be blank"]
-      end
-    end
-
-    context "requiring the state for validiations" do
-      let(:form_class) do
-        Class.new(Operational::Form) do
-          validate :state_is_present_validator
-
-          def state_is_present_validator
-            @state.present? &&
-            @state.frozen? &&
-            @state[:params] == { name: "update", email: "update@test.com" } &&
-            @state[:contract] == self
-          end
-        end
-      end
-
-      it "exposes the frozen state via the instance var" do
-        expect(described_class::Validate().call(state)).to eq true
-      end
-    end
-
     describe "name" do
       it "allows overriding the state key" do
         described_class::Build(contract: CreateForm, name: :test).call(state)
@@ -360,16 +137,6 @@ def state_is_present_validator
     end
 
     describe "model_key" do
-      let(:model_class) do
-        Class.new do
-          include ActiveModel::Model
-          include ActiveModel::Attributes
-
-          attribute :name, :string
-          attribute :email, :string
-        end
-      end
-
       let(:model) do
         model_class.new(name: "Test", email: "test@test.com")
       end
@@ -387,47 +154,6 @@ def state_is_present_validator
           described_class::Sync(model_key: :invalid).call(state)
         }.to raise_error Operational::InvalidContractModel
       end
-
-      it "applies the matching attributes from contract to model" do
-        described_class::Build(contract: CreateForm, model_key: :model).call(state)
-        described_class::Validate().call(state)
-        described_class::Sync(model_key: :model).call(state)
-
-        expect(state[:model].name).to eq state[:params][:name]
-        expect(state[:model].email).to eq model.email
-      end
-    end
-
-    describe "sync_method" do
-      let(:form_class) do
-        Class.new(Operational::Form) do
-          attribute :name, :string
-
-          def sync(state)
-            state[:sync] = "test"
-          end
-
-          def other_sync(state)
-            state[:other_sync] = "test"
-          end
-        end
-      end
-
-      it "runs the sync_method if defined" do
-        described_class::Build(contract: CreateForm).call(state)
-        described_class::Sync().call(state)
-
-        expect(state[:sync]).to eq "test"
-        expect(state[:other_sync]).to be_nil
-      end
-
-      it "allows overriding the method name" do
-        described_class::Build(contract: CreateForm).call(state)
-        described_class::Sync(sync_method: :other_sync).call(state)
-
-        expect(state[:other_sync]).to eq "test"
-        expect(state[:sync]).to be_nil
-      end
     end
   end
 end