Merge pull request #46 from state-machines/fix/preserve-custom-human-names

fix: preserve custom human_name for both states and events

Author: seuros

lib/state_machines/integrations/active_model.rb

@@ -8,7 +8,7 @@
 require 'state_machines/integrations/active_model/version'
 
 module StateMachines
-  module Integrations #:nodoc:
+  module Integrations # :nodoc:
     # Adds support for integrating state machines with ActiveModel classes.
     #
     # == Examples
@@ -345,14 +345,14 @@ def reset(object)
       end
 
       # Runs state events around the object's validation process
-      def around_validation(object)
-        object.class.state_machines.transitions(object, action, after: false).perform { yield }
+      def around_validation(object, &)
+        object.class.state_machines.transitions(object, action, after: false).perform(&)
       end
 
       protected
 
       def define_state_initializer
-        define_helper :instance, <<-end_eval, __FILE__, __LINE__ + 1
+        define_helper :instance, <<-END_EVAL, __FILE__, __LINE__ + 1
           def initialize(params = nil, **kwargs)
             # Support both positional hash and keyword arguments
             attrs = params.nil? ? kwargs : params
@@ -370,7 +370,7 @@ def initialize(params = nil, **kwargs)
               end
             end
           end
-        end_eval
+        END_EVAL
       end
 
       # Whether validations are supported in the integration.  Only true if
@@ -388,7 +388,7 @@ def runs_validations_on_action?
 
       # Gets the terminator to use for callbacks
       def callback_terminator
-        @terminator ||= ->(result) { result == false }
+        @callback_terminator ||= ->(result) { result == false }
       end
 
       # Determines the base scope to use when looking up translations
@@ -418,7 +418,7 @@ def translate(klass, key, value)
         # Generate all possible translation keys
         translations = ancestors.map { |ancestor| :"#{ancestor.model_name.to_s.underscore}.#{name}.#{group}.#{value}" }
         translations.concat(ancestors.map { |ancestor| :"#{ancestor.model_name.to_s.underscore}.#{group}.#{value}" })
-        translations.concat([:"#{name}.#{group}.#{value}", :"#{group}.#{value}", value.humanize.downcase])
+        translations.push(:"#{name}.#{group}.#{value}", :"#{group}.#{value}", value.humanize.downcase)
         I18n.translate(translations.shift, default: translations, scope: [i18n_scope(klass), :state_machines])
       end
 
@@ -432,10 +432,12 @@ def ancestors_for(klass)
       def define_state_accessor
         name = self.name
 
+        return unless supports_validations?
+
         owner_class.validates_each(attribute) do |object|
           machine = object.class.state_machine(name)
           machine.invalidate(object, :state, :invalid) unless machine.states.match(object)
-        end if supports_validations?
+        end
       end
 
       # Adds hooks into validation for automatically firing events
@@ -453,22 +455,32 @@ def define_validation_hook
       # Creates a new callback in the callback chain, always inserting it
       # before the default Observer callbacks that were created after
       # initialization.
-      def add_callback(type, options, &block)
+      def add_callback(type, options, &)
         options[:terminator] = callback_terminator
         super
       end
 
       # Configures new states with the built-in humanize scheme
       def add_states(*)
         super.each do |new_state|
-          new_state.human_name = ->(state, klass) { translate(klass, :state, state.name) }
+          # Only set the translation lambda if human_name is the default auto-generated value
+          # This preserves user-specified human names while still applying translations for defaults
+          default_human_name = new_state.name ? new_state.name.to_s.tr('_', ' ') : 'nil'
+          if new_state.human_name == default_human_name
+            new_state.human_name = ->(state, klass) { translate(klass, :state, state.name) }
+          end
         end
       end
 
       # Configures new event with the built-in humanize scheme
       def add_events(*)
         super.each do |new_event|
-          new_event.human_name = ->(event, klass) { translate(klass, :event, event.name) }
+          # Only set the translation lambda if human_name is the default auto-generated value
+          # This preserves user-specified human names while still applying translations for defaults
+          default_human_name = new_event.name ? new_event.name.to_s.tr('_', ' ') : 'nil'
+          if new_event.human_name == default_human_name
+            new_event.human_name = ->(event, klass) { translate(klass, :event, event.name) }
+          end
         end
       end
     end

test/event_human_name_test.rb

@@ -0,0 +1,176 @@
+# frozen_string_literal: true
+
+require_relative 'test_helper'
+
+class EventHumanNameTest < BaseTestCase
+  def setup
+    @model = new_model do
+      include ActiveModel::Validations
+      attr_accessor :status
+    end
+  end
+
+  def test_should_allow_custom_human_name_on_event
+    machine = StateMachines::Machine.new(@model, :status, initial: :parked) do
+      event :start, human_name: 'Start Engine' do
+        transition parked: :running
+      end
+
+      event :stop do
+        transition running: :parked
+      end
+
+      event :pause, human_name: 'Temporarily Pause' do
+        transition running: :paused
+      end
+    end
+
+    assert_equal 'Start Engine', machine.events[:start].human_name(@model)
+    assert_equal 'Temporarily Pause', machine.events[:pause].human_name(@model)
+  end
+
+  def test_should_not_override_custom_event_human_name_with_translation
+    # Set up I18n translations
+    I18n.backend.store_translations(:en, {
+                                      activemodel: {
+                                        state_machines: {
+                                          events: {
+                                            ignite: 'Translation for Ignite',
+                                            park: 'Translation for Park',
+                                            repair: 'Translation for Repair'
+                                          }
+                                        }
+                                      }
+                                    })
+
+    machine = StateMachines::Machine.new(@model, :status, initial: :parked) do
+      event :ignite, human_name: 'Custom Ignition' do
+        transition parked: :idling
+      end
+
+      event :park do
+        transition idling: :parked
+      end
+
+      event :repair, human_name: 'Custom Repair Process' do
+        transition any => :parked
+      end
+    end
+
+    # Custom human names should be preserved
+    assert_equal 'Custom Ignition', machine.events[:ignite].human_name(@model)
+    assert_equal 'Custom Repair Process', machine.events[:repair].human_name(@model)
+
+    # Event without custom human_name should use translation
+    assert_equal 'Translation for Park', machine.events[:park].human_name(@model)
+  end
+
+  def test_should_allow_custom_event_human_name_as_string
+    machine = StateMachines::Machine.new(@model, :status) do
+      event :activate, human_name: 'Turn On'
+    end
+
+    assert_equal 'Turn On', machine.events[:activate].human_name(@model)
+  end
+
+  def test_should_allow_custom_event_human_name_as_lambda
+    machine = StateMachines::Machine.new(@model, :status) do
+      event :process, human_name: ->(event, klass) { "#{klass.name}: #{event.name.to_s.capitalize} Action" }
+    end
+
+    assert_equal 'Foo: Process Action', machine.events[:process].human_name(@model)
+  end
+
+  def test_should_use_default_translation_when_no_custom_event_human_name
+    machine = StateMachines::Machine.new(@model, :status) do
+      event :idle
+    end
+
+    # Should fall back to humanized version when no translation exists
+    assert_equal 'idle', machine.events[:idle].human_name(@model)
+  end
+
+  def test_should_handle_nil_event_human_name
+    machine = StateMachines::Machine.new(@model, :status) do
+      event :wait
+    end
+
+    # Explicitly set to nil
+    machine.events[:wait].human_name = nil
+
+    # When human_name is nil, Event#human_name returns nil
+    assert_nil machine.events[:wait].human_name(@model)
+  end
+
+  def test_should_preserve_event_human_name_through_multiple_definitions
+    machine = StateMachines::Machine.new(@model, :status, initial: :draft)
+
+    # First define event with custom human name
+    machine.event :publish, human_name: 'Make Public' do
+      transition draft: :published
+    end
+
+    # Redefine the same event (this should not override the human_name)
+    machine.event :publish do
+      transition pending: :published
+    end
+
+    assert_equal 'Make Public', machine.events[:publish].human_name(@model)
+  end
+
+  def test_should_work_with_state_machine_helper_method
+    @model.class_eval do
+      state_machine :status, initial: :pending do
+        event :approve, human_name: 'Grant Approval' do
+          transition pending: :approved
+        end
+
+        event :reject do
+          transition pending: :rejected
+        end
+      end
+    end
+
+    machine = @model.state_machine(:status)
+    assert_equal 'Grant Approval', machine.events[:approve].human_name(@model)
+  end
+
+  def test_should_handle_complex_i18n_lookup_with_custom_event_human_name
+    # Set up complex I18n structure
+    I18n.backend.store_translations(:en, {
+                                      activemodel: {
+                                        state_machines: {
+                                          foo: {
+                                            status: {
+                                              events: {
+                                                submit: 'Model Specific Submit'
+                                              }
+                                            }
+                                          },
+                                          status: {
+                                            events: {
+                                              submit: 'Machine Specific Submit'
+                                            }
+                                          },
+                                          events: {
+                                            submit: 'Generic Submit'
+                                          }
+                                        }
+                                      }
+                                    })
+
+    machine = StateMachines::Machine.new(@model, :status) do
+      event :submit, human_name: 'Send for Review' do
+        transition draft: :pending
+      end
+    end
+
+    # Should use the custom human_name, not any of the I18n translations
+    assert_equal 'Send for Review', machine.events[:submit].human_name(@model)
+  end
+
+  def teardown
+    # Clear I18n translations after each test
+    I18n.backend.reload!
+  end
+end

test/human_name_preservation_test.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+require_relative 'test_helper'
+
+class HumanNamePreservationTest < BaseTestCase
+  def setup
+    @model = new_model do
+      include ActiveModel::Validations
+      attr_accessor :status
+    end
+  end
+
+  def test_should_preserve_custom_state_human_name_when_using_activemodel_integration
+    # This test specifically verifies that PR #38's fix works:
+    # Using ||= instead of = in add_states method
+
+    @model.class_eval do
+      state_machine :status, initial: :pending do
+        # Define a state with a custom human_name
+        state :pending, human_name: 'My Custom Pending'
+        state :approved
+      end
+    end
+
+    machine = @model.state_machine(:status)
+
+    # The custom human_name should be preserved, not overwritten by the integration
+    assert_equal 'My Custom Pending', machine.states[:pending].human_name(@model)
+  end
+
+  def test_should_preserve_custom_event_human_name_when_using_activemodel_integration
+    # This test verifies our additional fix for events:
+    # Using ||= instead of = in add_events method
+
+    @model.class_eval do
+      state_machine :status, initial: :pending do
+        event :approve, human_name: 'Grant Authorization' do
+          transition pending: :approved
+        end
+
+        event :reject do
+          transition pending: :rejected
+        end
+      end
+    end
+
+    machine = @model.state_machine(:status)
+
+    # The custom human_name should be preserved, not overwritten by the integration
+    assert_equal 'Grant Authorization', machine.events[:approve].human_name(@model)
+  end
+
+  def test_regression_issue_37_hard_coded_human_name_preserved
+    # This is the exact regression test for issue #37
+    # "Hard-coded human_name is being overwritten"
+
+    @model.class_eval do
+      state_machine :status do
+        state :pending, human_name: 'Pending Approval'
+        state :active, human_name: 'Active State'
+
+        event :activate, human_name: 'Activate Now' do
+          transition pending: :active
+        end
+      end
+    end
+
+    machine = @model.state_machine(:status)
+
+    # Both states and events should preserve their hard-coded human names
+    assert_equal 'Pending Approval', machine.states[:pending].human_name(@model)
+    assert_equal 'Active State', machine.states[:active].human_name(@model)
+    assert_equal 'Activate Now', machine.events[:activate].human_name(@model)
+  end
+end