Merge bf23c08 into 07c760d

app/models/concerns/setter_override_mixin.rb

@@ -0,0 +1,29 @@
+module SetterOverrideMixin
+
+  #this allows multiple overrides of the setter method
+  #to be safely combined by calling the new implementation
+  #and passing the result to the old one
+  #
+  #pass in the column to override, a unique name for your override,
+  #and a block containing the normalization logic for the value
+  def safe_override_setter(col, mixin_name, &block)
+    setter_name = "#{col}="
+
+    if method_defined?(setter_name.to_sym)
+      prev_setter_name = "#{mixin_name}_#{col}="
+      alias_method prev_setter_name, setter_name
+
+      define_method setter_name do |val|
+        normalized_val = yield val
+
+        self.send(prev_setter_name, normalized_val)
+      end
+
+    else
+      define_method setter_name do |val|
+        normalized_val = yield val
+        super(normalized_val)
+      end
+    end
+  end
+end

app/models/concerns/with_nulled_blanks.rb

@@ -0,0 +1,18 @@
+module WithNulledBlanks
+  extend ActiveSupport::Concern
+
+  class_methods do
+    include SetterOverrideMixin
+    def columns_with_nulled_blanks(*cols)
+      cols.each do |col|
+        safe_override_setter(col, 'nulled_blanks') do |val|
+          if val.blank?
+            nil
+          else
+            val
+          end
+        end
+      end
+    end
+  end
+end

app/models/concerns/with_string_to_int_columns.rb

@@ -2,13 +2,14 @@ module WithStringToIntColumns
   extend ActiveSupport::Concern
 
   class_methods do
+    include SetterOverrideMixin
     def string_to_int_columns(*cols)
       cols.each do |col|
-        define_method "#{col}=" do |val|
+        safe_override_setter(col, 'string_to_int') do |val|
           if val.is_a?(String)
-            super(val.delete(','))
+            val.delete(',')
           else
-            super(val)
+            val
           end
         end
       end

app/models/concerns/with_stripped_whitespace.rb

@@ -1,11 +1,11 @@
 module WithStrippedWhitespace
   extend ActiveSupport::Concern
-
   class_methods do
+    include SetterOverrideMixin
     def columns_with_stripped_whitespace(*cols)
       cols.each do |col|
-        define_method "#{col}=" do |val|
-          super(val.squish)
+        safe_override_setter(col, 'squished') do |val|
+          val.squish
         end
       end
     end

app/models/variant.rb

@@ -9,6 +9,7 @@ class Variant < ActiveRecord::Base
   include Commentable
   include WithStringToIntColumns
   include WithStrippedWhitespace
+  include WithNulledBlanks
 
   belongs_to :gene
   belongs_to :secondary_gene, class_name: 'Gene'
@@ -29,6 +30,12 @@ class Variant < ActiveRecord::Base
 
   columns_with_stripped_whitespace :description
 
+  columns_with_nulled_blanks :reference_bases, :variant_bases,
+    :chromosome, :chromosome2,
+    :start, :start2,
+    :stop, :stop2,
+    :representative_transcript, :representative_transcript2
+
   string_to_int_columns :start, :start2, :stop, :stop2
 
   def init

spec/models/chained_setter_overrides_spec.rb

@@ -0,0 +1,52 @@
+require 'rails_helper'
+
+#Ex: Variant stop should both remove commas before casting
+#and save empty values as null
+describe 'SetterOverrideMixin' do
+
+  it 'should make whitespace or empty string NULL before saving to the db' do
+    variant = Fabricate(:variant)
+
+    variant.start = ""
+    variant.save
+    expect(variant.start).to be_nil
+
+    variant.start = "    "
+    variant.save
+    expect(variant.start).to be_nil
+
+  end
+
+  it 'should also remove commas before casting to an int' do
+    variant = Fabricate(:variant)
+    variant.start = "1,000"
+    variant.save
+    expect(variant.start).to eq(1000)
+  end
+
+  it 'should preserve the value otherwise' do
+    variant = Fabricate(:variant)
+
+    variant.start = 1000
+    variant.save
+    expect(variant.start).to eq(1000)
+
+    variant.start = "200"
+    variant.save
+    expect(variant.start).to eq(200)
+  end
+
+  it 'should work with suggested changes' do
+    variant = Fabricate(:variant, reference_bases: "ABCD")
+    user = Fabricate(:user)
+    applying_user = Fabricate(:user)
+    org = Fabricate(:organization)
+
+    variant.start = "1,000"
+    change = variant.suggest_change!(user, {}, {})
+    change.apply(applying_user, org, false)
+    variant.reload
+
+    expect(variant.start).to eq(1000)
+  end
+end

spec/models/with_nulled_blanks_spec.rb

@@ -0,0 +1,42 @@
+require 'rails_helper'
+
+describe 'WithNulledBlanks' do
+
+  it 'should define a new class method on the model its mixed into' do
+    expect(Variant.methods.include?(:columns_with_nulled_blanks)).to be(true)
+  end
+
+  it 'should make whitespace or empty string NULL before saving to the db' do
+    variant = Fabricate(:variant)
+
+    variant.reference_bases = ""
+    variant.save
+    expect(variant.reference_bases).to be_nil
+
+    variant.reference_bases = "    "
+    variant.save
+    expect(variant.reference_bases).to be_nil
+  end
+
+  it 'should preserve the value otherwise' do
+    variant = Fabricate(:variant)
+
+    variant.reference_bases = "ABC"
+    variant.save
+    expect(variant.reference_bases).to eq("ABC")
+  end
+
+  it 'should work with suggested changes' do
+    variant = Fabricate(:variant, reference_bases: "ABCD")
+    user = Fabricate(:user)
+    applying_user = Fabricate(:user)
+    org = Fabricate(:organization)
+
+    variant.reference_bases = ""
+    change = variant.suggest_change!(user, {}, {})
+    change.apply(applying_user, org, false)
+    variant.reload
+
+    expect(variant.reference_bases).to be_nil
+  end
+end