Refactored ExtraOptionConfigs to BaseConfiguration pattern with ActiveModel::Validations - fixes #986

Author: philayres

app/helpers/application_helper.rb

@@ -214,7 +214,9 @@ def show_caption_before(key, captions, mode: nil, no_sub: nil, ignore_missing: t
 
     mode ||= action_name == 'new' ? :new : :edit
     caption = captions[key]
-    caption = caption[:"#{mode}_caption"] || caption[:caption] || '' if caption.is_a?(Hash)
+    if caption.is_a?(Hash) || caption.is_a?(OptionConfigs::BaseNamedConfiguration)
+      caption = caption[:"#{mode}_caption"] || caption[:caption] || ''
+    end
     if @form_object_instance && !no_sub
       caption = Formatter::Substitution.substitute(caption, data: @form_object_instance, tag_subs: nil,
                                                             ignore_missing:)

app/models/concerns/options_handler.rb

@@ -172,6 +172,72 @@ def configure_hash(config_item_name, with:)
       ch.const_set(config_item_name.ns_camelize, c)
     end
 
+    #
+    # Class method for declaring that a class stores a single direct value of a given type.
+    # This provides a consistent mechanism for defining configuration classes that hold
+    # one typed value (e.g., a string, array, hash, or if_condition) rather than
+    # multiple named attributes.
+    #
+    # The type metadata allows future validation and coercion. Currently supported types:
+    # - :string  — stores a String value
+    # - :array   — stores an Array value
+    # - :hash    — stores an arbitrary Hash value
+    # - :if_condition — stores a conditional Hash (for access/validation conditions)
+    #
+    # Example:
+    #   class Label < SomeBase
+    #     configure_direct :label, type: :string
+    #   end
+    #
+    # @param [Symbol] config_item_name - the name of the configuration item
+    # @param [Symbol] type - the value type (:string, :array, :hash, :if_condition)
+    def configure_direct(config_item_name, type:)
+      attr_accessor(config_item_name) unless method_defined?(config_item_name)
+
+      add_option_type(:direct, config_item_name)
+
+      # Store type metadata for future validation/coercion
+      @direct_types ||= {}
+      @direct_types[config_item_name] = type
+    end
+
+    #
+    # Returns the registered direct types for this class.
+    # @return [Hash{Symbol => Symbol}] mapping of attribute name to type
+    def direct_types
+      @direct_types || {}
+    end
+
+    #
+    # Class method for declaring a typed attribute whose value is an instance of
+    # a class inheriting from BaseConfiguration.
+    #
+    # When the including class is initialized with a hash configuration, the
+    # attribute value is automatically set by passing the corresponding hash
+    # entry to the type class constructor.
+    #
+    # @param [Symbol] config_item_name - the attribute name
+    # @param [Class] type - a class inheriting from BaseConfiguration that
+    #   accepts a hash in its constructor
+    #
+    # @example
+    #   configure_typed_attribute :creatable_if, type: ExtraOptionConfigs::IfCondition
+    def configure_typed_attribute(config_item_name, type:)
+      attr_accessor(config_item_name) unless method_defined?(config_item_name)
+
+      add_option_type(:typed, config_item_name)
+
+      @typed_attribute_types ||= {}
+      @typed_attribute_types[config_item_name] = type
+    end
+
+    #
+    # Returns the registered typed attribute types for this class.
+    # @return [Hash{Symbol => Class}] mapping of attribute name to type class
+    def typed_attribute_types
+      @typed_attribute_types || {}
+    end
+
     #
     # List of configuration items having child options.
     # Each represents the name of an accessor attribute in this model
@@ -180,7 +246,9 @@ def option_types
       @option_types ||= {
         multi: [],
         simple: [],
-        hash: []
+        hash: [],
+        direct: [],
+        typed: []
       }
     end
 
@@ -303,6 +371,7 @@ def setup_from_hash_config
     setup_all_options_multi hash_configuration
     setup_all_options_simple hash_configuration
     setup_all_options_hash hash_configuration
+    setup_all_options_typed hash_configuration
 
     hash_configuration
   end
@@ -322,6 +391,14 @@ def setup_all_options_simple(hash_configuration)
     end
   end
 
+  def setup_all_options_typed(hash_configuration)
+    self.class.option_types[:typed]&.each do |option_type|
+      type_class = self.class.typed_attribute_types[option_type]
+      config_val = hash_configuration[option_type]
+      send("#{option_type}=", type_class.new(config_val))
+    end
+  end
+
   def setup_all_options_hash(hash_configuration)
     self.class.option_types[:hash].each do |option_type|
       setup_options_hash(hash_configuration, option_type)

app/models/option_configs/activity_log_options.rb

@@ -10,14 +10,21 @@ class ActivityLogOptions < ExtraOptions
     ValidNfsStoreCanPerformKeys = %i[download_if view_files_as_image_if view_files_as_html_if send_files_to_trash_if
                                      move_files_if user_file_actions_if].freeze
 
+    def self.config_class_registry
+      super.merge(
+        e_sign: ExtraOptionConfigs::ESignConfig,
+        nfs_store: ExtraOptionConfigs::NfsStoreConfig
+      )
+    end
+
     def self.add_key_attributes
-      %i[e_sign nfs_store]
+      []
     end
 
     attr_accessor(*key_attributes)
 
     def initialize(name, config, parent_activity_log)
-      super(name, config, parent_activity_log)
+      super
 
       if @config_obj.disabled
         Rails.logger.info "configuration for this activity log has not been enabled: #{@config_obj.table_name}"
@@ -26,57 +33,11 @@ def initialize(name, config, parent_activity_log)
       raise FphsException, 'extra log options name: property can not be blank' if self.name.blank?
 
       # Activity logs have some predefined captions. Set these up.
-      if caption_before && !caption_before.is_a?(Hash)
+      if caption_before && !caption_before.is_a?(Hash) && !caption_before.is_a?(ExtraOptionConfigs::BaseConfiguration)
         raise FphsException, 'extra log options caption_before: must be a hash of {field_name: caption, ...}'
       end
 
       init_caption_before
-
-      clean_e_sign_def
-      clean_nfs_store_def
-    end
-
-    def clean_nfs_store_def
-      return unless nfs_store
-
-      can_perform = nfs_store[:can]
-
-      unless valid_config_keys?(nfs_store, ValidNfsStoreKeys)
-        failed_config :nfs_store,
-                      "nfs_store contains invalid keys #{nfs_store.keys} - " \
-                      "expected only #{ValidNfsStoreKeys}"
-      end
-
-      unless can_perform.nil? || valid_config_keys?(can_perform, ValidNfsStoreCanPerformKeys)
-        failed_config :nfs_store__can,
-                      "nfs_store.can contains invalid keys #{can_perform.keys} - " \
-                      "expected only #{ValidNfsStoreCanPerformKeys}"
-      end
-
-      NfsStore::Config::ExtraOptions.clean_def nfs_store
-    end
-
-    def clean_e_sign_def
-      return unless e_sign
-
-      # Set up the structure so that we can use the standard reference methods to parse the configuration
-      e_sign[:document_reference] = { item: e_sign[:document_reference] } unless e_sign[:document_reference][:item]
-      e_sign[:document_reference].each_value do |refitem|
-        # Make all keys singular, to simplify configurations
-        refitem.transform_keys! do |k|
-          new_k = k.to_s.singularize.to_sym
-        end
-
-        refitem.each do |mn, conf|
-          to_class = ModelReference.to_record_class_for_type(mn)
-
-          refitem[mn][:to_record_label] = conf[:label] || to_class&.human_name
-          if to_class&.respond_to?(:no_master_association)
-            refitem[mn][:no_master_association] = to_class.no_master_association
-          end
-          refitem[mn][:to_model_name_us] = to_class&.to_s&.ns_underscore
-        end
-      end
     end
 
     # A list of all fields defined within all the individual activity definitions. This does not include
@@ -129,7 +90,7 @@ def init_caption_before
         protocol_id: {
           caption: "Select the protocol this #{curr_name} is related to. A tracker event will be recorded under this protocol."
         },
-        "set_related_#{item_type}_rank".to_sym => {
+        "set_related_#{item_type}_rank": {
           caption: "To change the rank of the related #{item_type.to_s.humanize}, select it:"
         }
       }

app/models/option_configs/base_named_configuration.rb

@@ -4,6 +4,61 @@ class BaseNamedConfiguration < OptionConfigs::BaseOptions
 
     attr_accessor :owner, :use_hash_config
 
+    #
+    # Hash-like access to configuration attributes by key name.
+    # Enables backward compatibility with code that expects Hash-like access
+    # on individual named configuration items, e.g. `named_config[:caption]`
+    # @param [Symbol | String] key - the attribute name
+    # @return [Object] the attribute value, or nil if not recognized
+    def [](key)
+      sym_key = key.to_sym
+      return nil unless respond_to?(sym_key)
+
+      send(sym_key)
+    end
+
+    #
+    # Convert all configured attributes to a plain Hash.
+    # Mirrors OptionsHandler::Configuration#to_h for named configurations.
+    # @return [Hash{Symbol => Object}]
+    def to_h
+      res = {}
+      self.class.option_types[:simple].each { |k| res[k] = send(k) }
+      res
+    end
+
+    alias to_hash to_h
+
+    # Hash-compatible dig for nested access on named configurations.
+    # @param keys [Array<Symbol>] nested key path
+    # @return [Object, nil]
+    def dig(*keys)
+      first = keys.shift
+      val = self[first]
+      return val if keys.empty? || val.nil?
+
+      val.respond_to?(:dig) ? val.dig(*keys) : nil
+    end
+
+    #
+    # Equality comparison: compare as plain Hash for backward compatibility
+    # with code that previously stored raw Hashes instead of NamedConfiguration objects.
+    # @param other [Object] value to compare against
+    # @return [Boolean]
+    def ==(other)
+      return to_h == other if other.is_a?(Hash)
+
+      super
+    end
+
+    #
+    # Return a Hash containing only non-nil attribute values.
+    # Useful for serialization where nil values should be omitted.
+    # @return [Hash{Symbol => Object}]
+    def filtered_hash
+      to_h.reject { |_k, v| v.nil? }
+    end
+
     def config_text
       return super unless owner