Skip to content

Commit

Permalink
Fixed db_transition_safety bug and ensure load paths are correct depe…
Browse files Browse the repository at this point in the history
…nding on whether ffcrm is loaded as an engine or a rails app.
  • Loading branch information
steveyken committed Oct 18, 2012
1 parent 3911f04 commit f9de0d6
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 50 deletions.
10 changes: 5 additions & 5 deletions app/models/fields/custom_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ class CustomField < Field
before_create :add_column

SAFE_DB_TRANSITIONS = {
:any => [[:date, :time, :timestamp], [:integer, :float]],
:one => {:string => :text}
:any => [['date', 'time', 'timestamp'], ['integer', 'float']],
:one => {'string' => 'text'}
}

def available_as
Expand Down Expand Up @@ -93,13 +93,13 @@ def custom_validator(obj)
# :unsafe => transition is unsafe
#------------------------------------------------------------------------------
def db_transition_safety(old_type, new_type = self.as)
old_col, new_col = [old_type, new_type].map{|t| column_type(t) }
old_col, new_col = [old_type, new_type].map{|t| column_type(t).to_s }
return :null if old_col == new_col # no transition needed
return :safe if SAFE_DB_TRANSITIONS[:one].any? do |start, final|
old_col == start && new_col == final # one-to-one
old_col == start.to_s && new_col == final.to_s # one-to-one
end
return :safe if SAFE_DB_TRANSITIONS[:any].any? do |col_set|
[old_col, new_col].all?{|c| col_set.include?(c)} # any-to-any
[old_col, new_col].all?{|c| col_set.include?(c.to_s)} # any-to-any
end
:unsafe # Else, unsafe.
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/fields/custom_field_datetime_pair.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class CustomFieldDatetimePair < CustomFieldDatePair
# Register this CustomField with the application
#------------------------------------------------------------------------------
register(:as => 'datetime_pair', :klass => 'CustomFieldDatetimePair', :type => 'timestamp')

def render(value)
value && value.strftime(I18n.t("time.formats.mmddhhss"))
end
Expand Down
36 changes: 3 additions & 33 deletions app/models/fields/field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ class Field < ActiveRecord::Base
scope :without_pairs, where(:pair_id => nil)

delegate :klass, :klass_name, :klass_name=, :to => :field_group

# Subclasses are allowed to have their own list of settings
# They must define 'self.settings_keys' in order to access them
#------------------------------------------------------------------------------
class_attribute :settings_keys
self.settings_keys = []

BASE_FIELD_TYPES = {
'string' => {:klass => 'CustomField', :type => 'string'},
Expand Down Expand Up @@ -116,29 +110,7 @@ def render(value)
value.to_s
end
end

# Define 'settings' accessors/mutators methods for convenience
#------------------------------------------------------------------------------
def method_missing(method, *args, &block)
if settings_keys.include?(method.to_s)
settings[method]
elsif settings_keys.map{|s| "#{s}="}.include?(method.to_s)
settings[method.to_s - '='] = args.first
else
super
end
end

# Ensure class tells the truth about what methods it responds to
#------------------------------------------------------------------------------
def respond_to?(method, include_private=false)
if (settings_keys + settings_keys.map{|s| "#{s}="}).include?(method.to_s)
true
else
super
end
end


protected

class << self
Expand All @@ -158,14 +130,12 @@ def register(options)
(@@field_types ||= BASE_FIELD_TYPES).merge!(as => options)
end

# Returns class name given the key 'as'
# Returns class name given a key
#------------------------------------------------------------------------------
def lookup_class(as)
(@@field_types ||= BASE_FIELD_TYPES)[as][:klass]
end

#~ Field.descendants.each{|klass| klass.register}

end

end
4 changes: 2 additions & 2 deletions app/views/admin/fields/_form.html.haml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
- @field ||= Field.new
- @field ||= Field.new(:as => 'string')
- field_group_id ||= @field.field_group_id
- new_field = @field.id.nil?
- url = new_field ? admin_fields_path : admin_field_path
Expand All @@ -21,7 +21,7 @@
%td= spacer
%td
.label.top.req Field type:
= select :field, :as, field_edit_as_options, :include_blank => true, :id => nil
= select :field, :as, field_edit_as_options(@field), :include_blank => true, :id => nil

.subform
= render(:partial => 'admin/fields/subform') unless new_field
Expand Down
7 changes: 5 additions & 2 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ class Application < Rails::Application
# -- all .rb files in that directory are automatically loaded.

# Models are organized in sub-directories
config.autoload_paths += Dir["app/models/**"] +
Dir["app/controllers/**"]
config.autoload_paths += Dir[Rails.root.join("app/models/**")] +
Dir[Rails.root.join("app/controllers/entities")]

# Prevent Field class from being reloading more than once as this clears registered customfields
config.autoload_once_paths += [File.expand_path("../app/models/fields/field.rb", __FILE__)]

# Activate observers that should always be running.
unless ARGV.join.include?('assets:precompile')
Expand Down
4 changes: 1 addition & 3 deletions config/initializers/custom_fields.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#------------------------------------------------------------------------------



# Custom fields need to be loaded so they register their availability
#------------------------------------------------------------------------------
custom_field_path = File.join(Rails.root, 'app', 'models', 'fields', 'custom_field_*')
custom_field_path = File.join(File.dirname(__FILE__), '..', '..', 'app', 'models', 'fields', 'custom_field_*')
Dir[custom_field_path].each {|f| require f}
6 changes: 3 additions & 3 deletions spec/models/fields/custom_field_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

it "should add a column to the database" do
CustomField.connection.should_receive(:add_column).
with("contacts", "cf_test_field", :string, {})
with("contacts", "cf_test_field", 'string', {})
Contact.should_receive(:reset_column_information)
Contact.should_receive(:serialize_custom_fields!)

Expand Down Expand Up @@ -71,9 +71,9 @@

it "should change a column's type for safe transitions" do
CustomField.connection.should_receive(:add_column).
with("contacts", "cf_test_field", :string, {})
with("contacts", "cf_test_field", 'string', {})
CustomField.connection.should_receive(:change_column).
with("contacts", "cf_test_field", :text, {})
with("contacts", "cf_test_field", 'text', {})
Contact.should_receive(:reset_column_information).twice
Contact.should_receive(:serialize_custom_fields!).twice

Expand Down
2 changes: 1 addition & 1 deletion spec/models/fields/field_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@


it "should return a list of field types" do
Field.field_types['string'].should == {:type => :string, :options => nil}
Field.field_types['string'].should == {'klass' => 'CustomField', 'type' => 'string'}
end

it "should return a hash of input options" do
Expand Down

0 comments on commit f9de0d6

Please sign in to comment.