Refactor of ExtraOptions, etc

[philayres]

Requirements

The current implementation of DynamicOptions, ActivityLogOptions and ExternalIdentifierOptions (all inheriting from ExtraOptions) doesn't follow OO principles and is difficult to extend.

ExtraOptions needs to be refactored so that each top-level configuration currently handled by methods named clean_... are represented by a class for configuration setup and validation. These may lead to deeper granularity for save_triggers, batch_triggers and config_triggers, so that each trigger type also has a configuration class.

The ReportOptions class might be a model to follow, since it relies on OptionsHandler. This provides class methods for configure, configure_attributes, configure_hash, allowing configuration structures to be quickly specified and captured from a Hash. These set up standard Rails attributes. The class may be extended to allow additional configure_... methods if needed, to allow type specification of attributes and reference to other configuration structures where necessary.

The aim is to both encapsulate the configuration logic of each major configuration type, and to allow better validation. The persistence of options as YAML must not be changed.

If possible, using regular Rails validations on each of the configuration classes would be ideal, even though the classes aren't stored as ActiveRecord instances in their own tables. The current ConfigErrors approach for reporting errors is reasonable, although may require modification to operate with regular Rails validations.

It is essential that there is a mechanism for retrieving the defined validations on each class (if there are some kind of reflection or similar mechanisms), so that the validations can eventually be used to drive configurations on the front end.

Important Note

The ExtraOptions clean_... methods provide essential reformatting of incoming configurations for use in the broader system. The logic of these must not be changed. It may be that they are just simply moved into their relevant configuration class and called after the main configuration setup.

To avoid breaking existing functionality, we need to ensure there is test coverage for the output of each of the clean_... methods as they stand. These tests should continue to pass (without changes) after each configuration class is implemented.

---

Detailed Context & Background

Current Architecture

The option configuration hierarchy is:

BaseOptions (includes ActiveModel::Validations, ConfigErrors)
  └── ExtraOptions (abstract base for dynamic definitions)
        ├── DynamicModelOptions
        ├── ActivityLogOptions (adds e_sign, nfs_store configs)
        └── ExternalIdentifierOptions
  └── ReportOptions (already uses OptionsHandler pattern)

ExtraOptions is initialized with (name, config, config_obj) where config is a parsed Hash from YAML options and config_obj is the dynamic definition record (e.g., a DynamicModel, ActivityLog, or ExternalIdentifier instance).

The constructor calls 21 clean_... methods sequentially to normalize, validate, and restructure configuration values into standard formats. These methods handle:

clean_ method	Configuration area	Complexity
clean_fields_def	Field list setup	Low
clean_field_configs	Per-field configuration merging	High
clean_label_def	Definition label	Low
clean_caption_before_def	Caption formatting (text-to-HTML)	Medium
clean_dialog_before_def	Dialog overlay definitions	Medium
clean_labels_def	Field labels	Low
clean_show_if_def	Conditional visibility (with branching logic parsing)	High
clean_save_action_def	Save action cascading (on_save → on_create/on_update)	Medium
clean_view_options_def	View options	Low
clean_db_configs_def	Database column configs	Low
clean_access_if_def	creatable_if, editable_if, showable_if	Low
clean_valid_if_def	Validation trigger conditions	Medium
clean_filestore_def	Filestore configuration	Low
clean_field_options_def	Field-level edit options	Medium
clean_embed_def	Embedded resource definitions	Medium
clean_references_def	Model reference configurations	High
clean_save_triggers	Save trigger cascading	Medium
clean_batch_triggers	Batch trigger setup	Low
clean_config_triggers	Config trigger setup	Low
clean_preset_fields	Preset field values	Low
clean_set_variables_def	Variable definitions	Medium

ActivityLogOptions adds 2 more: clean_e_sign_def and clean_nfs_store_def.

Model Pattern to Follow

ReportOptions uses the OptionsHandler concern which provides:

• configure :name, with: [...] — creates a Configuration sub-class with typed accessors
• configure_attributes [...] — creates simple accessor attributes
• configure_hash :name, with: [...] — creates keyed Configuration hashes

OptionsHandler already includes ActiveModel::Validations, automatically parses YAML/JSON config text, and provides setup_from_hash_config to populate configuration instances. These Configuration sub-classes live inside OptionsHandler::Configuration.

ConfigErrors Mechanism

Currently, errors are collected via failed_config(type, message, ...) into config_errors / config_warnings arrays on each ExtraOptions instance. The all_option_configs_errors and all_option_configs_notices class methods aggregate these across all option configurations. These are used for admin panel display rather than standard Rails validation.

Existing Test Coverage

Current specs are minimal for clean_... method outputs:

• spec/models/option_configs/extra_options_spec.rb (37 lines) — only tests multi-option-type defaults setup
• spec/models/option_configs/extra_options_anchor_spec.rb (226 lines) — tests YAML anchor/library handling
• spec/models/option_configs/activity_log_options_spec.rb (466 lines) — tests activity log-specific features
• spec/models/option_configs/dynamic_model_options_spec.rb (739 lines) — tests dynamic model option types
• spec/models/option_configs/valid_if_spec.rb (181 lines) — tests valid_if evaluations

None of these specs directly test the output of individual clean_... methods with known inputs.

---

Acceptance Criteria

Phase 1: Capture current behavior with tests (prerequisite)

• AC-1.1: For each clean_... method in ExtraOptions, write RSpec tests that:
  • Provide known YAML/Hash input configurations
  • Call the method (or initialize ExtraOptions with that config)
  • Assert the resulting attribute values match the current output format
  • Cover edge cases: nil inputs, empty hashes, invalid keys, nested structures
• AC-1.2: For ActivityLogOptions-specific clean_e_sign_def and clean_nfs_store_def, write equivalent tests
• AC-1.3: All existing specs in spec/models/option_configs/ continue to pass without modifications

Phase 2: Create configuration classes

• AC-2.1: Each top-level configuration area (from the clean_... methods table above) has a dedicated configuration class, using OptionsHandler's configure / configure_attributes patterns where applicable
• AC-2.2: Configuration classes live under OptionConfigs::ExtraOptionConfigs:: namespace
• AC-2.3: Each configuration class encapsulates:
  • Attribute accessors for its configuration options
  • The reformatting/normalization logic from the corresponding clean_... method
  • Any validation logic (using ActiveModel::Validations where possible)
• AC-2.4: ExtraOptions#initialize delegates to configuration class instances instead of calling clean_... methods directly
• AC-2.5: The public interface of ExtraOptions (attribute accessors like .save_trigger, .references, .caption_before, etc.) remains unchanged — existing consumer code must not need modification
• AC-2.6: YAML persistence format is unchanged — no migrations or data transformations required

Phase 3: Validation improvements

• AC-3.1: Configuration classes and embedded NamedConfiguration classes use ActiveModel::Validations for structural validation
• AC-3.2: A mechanism exists to reflect/introspect defined validations on each configuration class (e.g., ClassName.validators or a custom method returning validation metadata)
• AC-3.3: ConfigErrors integrates with ActiveModel::Validations so that validation errors from configuration classes appear in the existing admin error display
• AC-3.4: No regressions in admin panel error reporting for invalid configurations
• AC-3.5: Check for valid? should be performed immediately after adding a named configuration
• AC-3.6: A valid? check on configuration classes that contain a embedded NamedConfiguration class should run through all contained NamedConfiguration instances to check validity.

Phase 4: Trigger configuration granularity (if time permits)

• AC-4.1: save_trigger, batch_trigger, and config_trigger each have their own configuration class with trigger-type-specific sub-classes
• AC-4.2: Individual trigger types (e.g., on_create, on_save, on_update) are represented as configuration objects

---

Technical Considerations & Dependencies

Architecture Decisions (Confirmed)

1. Namespace: Configuration classes live under OptionConfigs::ExtraOptionConfigs::.
2. Delegation strategy: ExtraOptions uses delegate / method forwarding to maintain the current flat API. Consumer code continues to call extra_options.caption_before, etc., without knowledge of sub-configuration classes.
3. Order dependency: Several clean_... methods must run in a specific order (e.g., clean_fields_def before clean_field_configs, clean_field_configs before add_field_configs_from_standalone_defs). The refactoring should attempt to decouple these where possible, rather than just preserving the current ordering.
4. config_obj coupling: Many clean_... methods reference @config_obj (the parent definition record):
   • Where attributes from config_obj are required for setup and validation, these should be extracted ahead of time and passed to the configuration class.
   • Where attributes in config_obj are set directly (mutated), this logic should be moved out of the configuration class / clean_... method and performed at the end of ExtraOptions / ActivityLogOptions initialization.
5. raw_field_configs: The current Marshal.load(Marshal.dump(field_configs)) deep clone can be handled differently if needed, as long as the result (a fully independent deep copy) is the same.

Dependencies

• OptionsHandler concern (app/models/concerns/options_handler.rb) — core pattern to adopt
• ConfigErrors module (app/models/option_configs/config_errors.rb) — must remain compatible
• Formatter::Substitution.text_to_html — used in caption processing
• Redcap::DataDictionaries::BranchingLogic — used in clean_show_if_def
• ModelReference.to_record_class_for_type — used in clean_references_def and clean_embed_def
• Resources::Models.find_by — used in clean_embed_def
• NfsStore::Config::ExtraOptions.clean_def — used in clean_nfs_store_def
• ConditionalActions — used in calc_if, calc_valid_if, calc_save_action_if

Files Impacted

• app/models/option_configs/extra_options.rb (1055 lines) — primary refactoring target
• app/models/option_configs/activity_log_options.rb (153 lines) — subclass with additional configs
• app/models/option_configs/dynamic_model_options.rb (15 lines) — subclass
• app/models/option_configs/external_identifier_options.rb (23 lines) — subclass
• app/models/option_configs/config_errors.rb (121 lines) — may need adaptation for ActiveModel::Validations integration
• app/models/concerns/options_handler.rb (383 lines) — may need extensions for new configure_... method types
• New files: one per configuration class under app/models/option_configs/extra_option_configs/ (estimated ~15-20 new files)

---

Potential Edge Cases & Risks

1. Initialization order sensitivity: The clean_... methods are called in a specific sequence. Moving them to separate classes risks breaking order-dependent logic (e.g., field_configs depends on fields being set first, and show_if depends on show_if_condition_strings).
2. Cross-configuration references: Some clean_... methods modify shared state (e.g., add_field_configs_from_standalone_defs reads from multiple attributes). Encapsulating into separate classes may require a "post-initialization" phase.
3. Error accumulation: failed_config errors are accumulated on the ExtraOptions instance. Configuration sub-classes need a way to report errors back to the parent.
4. Dynamic class references: clean_references_def and clean_embed_def look up runtime classes that may not yet exist during app type imports. This "soft failure" behavior must be preserved.
5. Deep cloning of field_configs: The raw_field_configs deep clone (currently Marshal.load(Marshal.dump(...))) can use an alternative approach, but must produce a fully independent copy.
6. Backward compatibility: External code accesses configuration via extra_options.save_trigger, extra_options.references, etc. These must remain as direct attribute accessors, not requiring knowledge of sub-configuration classes.
7. ActivityLogOptions divergence: ActivityLogOptions overrides initialize with additional logic (e.g., init_caption_before, e_sign/nfs_store cleaning). The refactoring must account for subclass-specific initialization.
8. Performance: Creating many small configuration objects per ExtraOptions instance could impact startup time if there are hundreds of configurations. Should be benchmarked.

---

Suggested Effort Estimation

Phase	Effort	Notes
Phase 1: Test coverage for clean_... outputs	3-5 days	~21 methods to cover with multiple input scenarios each
Phase 2: Configuration classes	5-8 days	~15-20 new classes, refactoring ExtraOptions, maintaining backward compatibility
Phase 3: Validation improvements	2-3 days	ActiveModel::Validations integration + reflection mechanism
Phase 4: Trigger sub-classes	2-3 days	Optional, depends on Phase 2 patterns
Total estimated	12-19 days	Incremental delivery recommended per-configuration-class

Recommended Implementation Approach

1. Start with Phase 1 (test coverage) — this is a hard prerequisite
2. Pick one simple clean_... method (e.g., clean_labels_def or clean_view_options_def) as a proof-of-concept configuration class
3. Validate the pattern works with existing tests
4. Apply the pattern to remaining methods, starting with low-complexity ones
5. Tackle high-complexity methods (clean_references_def, clean_field_configs, clean_show_if_def) last
6. Phase 3 and Phase 4 can proceed in parallel once Phase 2 patterns are established

[philayres]

Architecture Decisions

• Namespace: Configuration classes should live under OptionConfigs::ExtraOptionConfigs::
• Delegation strategy: ExtraOptions should use delegate / method forwarding to maintain the current flat API
• Order dependency: Several clean_... methods must run in a specific order (e.g., clean_fields_def before clean_field_configs, clean_field_configs before add_field_configs_from_standalone_defs) - we should attempt to decouple these
• config_obj coupling: Many clean_... methods reference @config_obj (the parent definition record)
  • where attributes from config_obj are required for setup and validation, these should be extracted ahead of time
  • where attributes in config_obj are set directly, this should be moved out of the configuration class / clean_... method and be performed at the end of the ExtraOptions / ActivityLogOptions initialization
• raw_field_configs: - the self.raw_field_configs = Marshal.load(Marshal.dump(field_configs)) can be handled differently if needed, as long as the result is the same