Merge pull request #864 from griffithlab/additional-name-validation-l…

…ogic

Validate name uniqueness, but be aware of revision context

server/app/models/actions/suggest_revision_set.rb

@@ -14,6 +14,8 @@ def initialize(existing_obj:, updated_obj:, originating_user:, organization_id:,
   end
 
   def execute
+    updated_obj.in_revision_validation_context = true
+    updated_obj.revision_target_id = existing_obj.id
     updated_obj.validate!
 
     any_changes = false

server/app/models/concerns/moderated.rb

@@ -25,6 +25,8 @@ module Moderated
       ->() { where(action: 'revision suggested').includes(:originating_user).order('events.updated_at DESC') },
       as: :subject,
       class_name: 'Event'
+
+    attr_accessor :in_revision_validation_context, :revision_target_id
   end
 
   #TODO: Refactor to use new Revision format

server/app/models/molecular_profile.rb

@@ -22,10 +22,7 @@ class MolecularProfile < ActiveRecord::Base
 
   validates :name, presence: true
 
-  #this breaks when we do updated_obj.validate! during propose revision set. we need a workaround
-  #validates_uniqueness_of :name,
-    #conditions: -> { where(deprecated: false) },
-    #message: 'must be unique'
+  validate :unique_name_in_context
 
   searchkick highlight: [:name, :aliases], callbacks: :async, word_start: [:name]
   scope :search_import, -> { includes(:molecular_profile_aliases, variants: [:gene])}
@@ -85,4 +82,30 @@ def self.timepoint_query
         .count
     }
   end
+
+  def unique_name_in_context
+    base_query = self.class.where(
+      deprecated: false,
+      name: name
+    )
+
+    duplicate_name = if in_revision_validation_context
+                       base_query
+                         .where.not(id: revision_target_id)
+                         .exists?
+                     else
+                       if persisted?
+                         base_query
+                           .where.not(id: id)
+                           .exists?
+                       else
+                         base_query
+                           .exists?
+                       end
+                     end
+
+    if duplicate_name
+      errors.add(:name, 'must be unique. There is already a Molecular Profile with this name.')
+    end
+  end
 end

server/app/models/variant.rb

@@ -32,6 +32,8 @@ class Variant < ApplicationRecord
 
   after_commit :reindex_mps
 
+  validates :name, presence: true
+
   validates :reference_bases, format: {
     with: /\A[ACTG]+\z|\A[ACTG]+\/[ACTG]+\z/,
     message: "only allows A,C,T,G or /"
@@ -42,10 +44,7 @@ class Variant < ApplicationRecord
     message: "only allows A,C,T,G or /"
   }, allow_nil: true
 
-  #this breaks when we do updated_obj.validate! during propose revision set. we need a workaround
-  #validates_uniqueness_of :name, scope: :gene_id,
-    #conditions: -> { where(deprecated: false) },
-    #message: 'must be unique within a Gene'
+  validate :unique_name_in_context
 
   searchkick highlight: [:name, :aliases], callbacks: :async
   scope :search_import, -> { includes(:variant_aliases, :gene) }
@@ -87,4 +86,31 @@ def on_revision_accepted
     SetAlleleRegistryIdSingleVariant.perform_later(self) if Rails.env.production?
     GenerateOpenCravatLink.perform_later(self)
   end
+
+  def unique_name_in_context
+    base_query = self.class.where(
+      deprecated: false,
+      gene_id: gene_id,
+      name: name
+    )
+
+    duplicate_name = if in_revision_validation_context
+                       base_query
+                         .where.not(id: revision_target_id)
+                         .exists?
+                     else
+                       if persisted?
+                         base_query
+                           .where.not(id: id)
+                           .exists?
+                       else
+                         base_query
+                           .exists?
+                       end
+                     end
+
+    if duplicate_name
+      errors.add(:name, 'must be unique within a Gene')
+    end
+  end
 end