Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behavior concerning 'default-settings' in class hierarchies (STI) #102

Open
mayinx opened this issue Jan 19, 2020 · 0 comments
Open

Comments

@mayinx
Copy link

mayinx commented Jan 19, 2020

The consecutive use of the has_settings-macro in a class hierarchy along with the persistent: true-option produces (more or less) unexpected behavior concerning the default_settings-hash. Given the following setup you would expect (at least I do) that the User-subclass Student would inherit and redefine/extend its base_class' default_settings - but of course without affecting its base_class' default_settings-hash:

class User < ActiveRecord::Base
  has_settings persistent: true do |s|
    s.key :theme, defaults: { name: 'default', base_size: '14' }
    s.key :calendar, defaults: { scope: 'school' }    
  end
end

class Student < User
  has_settings persistent: true do |s|
    s.key :theme, defaults: { name: "student", base_size: "18"  }  
    s.key :profile, defaults: { scope: 'school_class' }
  end
end

class Teacher < User; end

Problem: As of now, calling default_settings on the subclass Student changes the base-class' default_settings as well - and even the settings of other subclasses (like Teacher in this example):

# alrighty:
User.default_settings:
=> {:theme=>{"name"=>"default", "base_size"=>"14"}, :calendar=>{"scope"=>"school"}}

# all good - no suprises here:
Teacher.default_settings:
=> {:theme=>{"name"=>"default", "base_size"=>"14"}, :calendar=>{"scope"=>"school"}}

# still good? At least looks like it:
Student.default_settings:
=> {:theme=>{"name"=>"student", "base_size"=>"18"}, :calendar=>{"scope"=>"school"},  :profile=>{"scope"=>"school_class"}}

# but: uh - oh - no good:
User.default_settings:
=> {:theme=>{"name"=>"student", "base_size"=>"18"}, :calendar=>{"scope"=>"school"},  :profile=>{"scope"=>"school_class"}}

# oh dear:
Teacher.default_settings:
=> {:theme=>{"name"=>"student", "base_size"=>"18"}, :calendar=>{"scope"=>"school"},  :profile=>{"scope"=>"school_class"}}

This is due to the internal implementation of the default_settings-hash as class_attribute and a (at least in an inheritance-setting) 'suboptimal' value assignment (via @klass.default_settings ||= {}) on this mutable structure - see RailsSettings::Configuration#initialize:

@klass.default_settings ||= {}

Are there any plans on addressing STI-related issues like this in future releases?

Anyway, for the meantime I came up with the following workaround - at least for my use case it seems to do the job just fine - hopefully, this helps others with the same issue:

# Inheritance-Patch (applied via mixin prepending, see down below) to handle
# consecutive uses of the 'has_settings'-macro along with the 'persistent: true'-option  
# in a class hierarchy.
#
# Ensures correct initializations of the 'default_settings'-class_attribute
# in subclasses by extending/redefining RailsSettings::Base.included; adds
# an 'inherited'-hook to the included behavior which clones the parent's mutable
# class_attribute 'default_settings' when subclassed.
#
# FYI: Necessary when using mutable structures likes hashes or arrays as
# class_attribute - otherwise setting the subclasses 'default_settings' would
# effect the base class' 'default_settings' as well (operation on the same
# element etc.). See here for more details:
#
#   https://apidock.com/rails/Class/class_attribute
#
module RailsSettingsBaseStiExtension
  def included(base)
    super(base) # call the default implementation first

    # Add 'inherited'-hook to clone 'default_settings' from parent to child class
    base.define_singleton_method(:inherited) do |subClass|
      if (self.methods.include?(:default_settings) &&
          subClass.methods.include?(:default_settings))
        subClass.default_settings = self.default_settings.clone
      end

      super(subClass) # necessary to avoid trouble / to silence warnings
    end
  end
end

# Use 'prepend' to be able to call 'super' in the extension to keep
# the default behavior and just add our extension to it
RailsSettings::Base.singleton_class.send(:prepend, RailsSettingsBaseStiExtension)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant