Adds unique validation to alternative identifiers

All alternate identifiers should be unique.

.rubocop_todo.yml

@@ -1,12 +1,12 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-03-13 14:17:57 -0400 using RuboCop version 0.52.1.
+# on 2019-03-21 14:28:32 -0400 using RuboCop version 0.52.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 5
+# Offense count: 7
 # Configuration parameters: CountComments, ExcludedMethods.
 Metrics/BlockLength:
   Max: 63
@@ -45,7 +45,7 @@ RSpec/LetSetup:
 RSpec/MessageSpies:
   EnforcedStyle: receive
 
-# Offense count: 25
+# Offense count: 26
 # Configuration parameters: Max.
 RSpec/NestedGroups:
   Exclude:
@@ -57,12 +57,13 @@ RSpec/NestedGroups:
     - 'spec/cho/work/import/work_hash_validator_spec.rb'
     - 'spec/cho/work/submissions/change_set_spec.rb'
 
-# Offense count: 2
+# Offense count: 3
 # Cop supports --auto-correct.
 # Configuration parameters: Whitelist.
 # Whitelist: find_by_sql
 Rails/DynamicFindBy:
   Exclude:
+    - 'app/cho/validation/member.rb'
     - 'app/cho/work/import/work_hash_validator.rb'
     - 'lib/tasks/ingest.rake'
 

app/cho/data_dictionary/fields_for_change_set.rb

@@ -36,7 +36,7 @@ def blank_field?(field)
 
     def validate_field(field)
       validation_name = DataDictionary::Field.where(label: field.to_s).first.validation.to_sym
-      validation_instance = Validation::Factory.validators[validation_name]
+      validation_instance = Validation::Factory.validators[validation_name].new(change_set: self, field: field)
       unless validation_instance.validate(self[field])
         validation_instance.errors.each { |error| errors.add(field, error) }
       end

app/cho/validation/base.rb

@@ -2,7 +2,12 @@
 
 module Validation
   class Base
-    attr_accessor :errors
+    attr_accessor :errors, :change_set, :field
+
+    def initialize(change_set: nil, field: nil)
+      @change_set = change_set
+      @field = field
+    end
 
     def validate(_field)
       raise Error, 'Validation.validate is abstract. Children must implement.'

app/cho/validation/factory.rb

@@ -15,10 +15,11 @@ def default_key
 
         def default_items
           {
-            default_key => Validation::None.new,
-            resource_exists: Validation::ResourceExists.new,
-            edtf_date: Validation::EDTFDate.new,
-            creator: Validation::Creator.new
+            default_key => Validation::None,
+            resource_exists: Validation::ResourceExists,
+            edtf_date: Validation::EDTFDate,
+            creator: Validation::Creator,
+            unique: Validation::Unique
           }
         end
 

app/cho/validation/member.rb

@@ -8,11 +8,20 @@ def initialize(id)
       @id = Valkyrie::ID.new(id)
     end
 
-    def exists?
+    def exists_by_id?
       Valkyrie.config.metadata_adapter.query_service.find_by(id: id)
       true
     rescue Valkyrie::Persistence::ObjectNotFoundError
       false
     end
+
+    alias :exists? :exists_by_id?
+
+    def exists_by_alternate_id?
+      Valkyrie.config.metadata_adapter.query_service.find_by_alternate_identifier(alternate_identifier: id)
+      true
+    rescue Valkyrie::Persistence::ObjectNotFoundError
+      false
+    end
   end
 end

app/cho/validation/unique.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module Validation
+  class Unique < Base
+    def validate(field_value)
+      @errors = []
+      return true if field_value.blank? || unchanged_edit?
+
+      Array.wrap(field_value).each do |value|
+        member = Validation::Member.new(value)
+        next if !member.exists_by_alternate_id?
+
+        @errors << "#{value} already exists"
+      end
+      errors.empty?
+    end
+
+    private
+
+      def unchanged_edit?
+        return false unless change_set.resource.persisted?
+        change_set.send(field) == change_set.resource.send(field)
+      end
+  end
+end

config/data_dictionary/data_dictionary_development.csv

@@ -1,7 +1,7 @@
 Label,Field Type,Requirement Designation,Validation,Multiple,Controlled Vocabulary,Default Value,Display Name,Display Transformation,Index Type,Help Text,Core Field
 title,string,required,no_validation,false,no_vocabulary,,Title,no_transformation,no_facet,help me,true
 member_of_collection_ids,valkyrie_id,optional,resource_exists,false,cho_collections,,Member of Collection,no_transformation,no_facet,help me,false
-alternate_ids,alternate_id,required_to_publish,no_validation,true,no_vocabulary,,Identifier,no_transformation,no_facet,help me,true
+alternate_ids,alternate_id,required_to_publish,unique,true,no_vocabulary,,Identifier,no_transformation,no_facet,help me,true
 creator,creator,optional,creator,true,creator_vocabulary,,Creator,no_transformation,linked_field,help me,true
 date_created,string,required_to_publish,no_validation,true,no_vocabulary,,Date Created,no_transformation,date,help me,true
 subject,string,optional,no_validation,true,no_vocabulary,,Subject,no_transformation,no_facet,help me,true

config/data_dictionary/data_dictionary_production.csv

@@ -1,7 +1,7 @@
 Label,Field Type,Requirement Designation,Validation,Multiple,Controlled Vocabulary,Default Value,Display Name,Display Transformation,Index Type,Help Text,Core Field
 title,string,required,no_validation,false,no_vocabulary,,Title,no_transformation,no_facet,help me,true
 member_of_collection_ids,valkyrie_id,optional,resource_exists,false,cho_collections,,Member of Collection,no_transformation,no_facet,help me,false
-alternate_ids,alternate_id,required_to_publish,no_validation,true,no_vocabulary,,Identifier,no_transformation,no_facet,help me,true
+alternate_ids,alternate_id,required_to_publish,unique,true,no_vocabulary,,Identifier,no_transformation,no_facet,help me,true
 creator,creator,optional,creator,true,creator_vocabulary,,Creator,no_transformation,linked_field,help me,true
 date_created,string,required_to_publish,no_validation,true,no_vocabulary,,Date Created,no_transformation,date,help me,true
 subject,string,optional,no_validation,true,no_vocabulary,,Subject,no_transformation,no_facet,help me,true

config/data_dictionary/data_dictionary_test.csv

@@ -10,5 +10,5 @@ still_image_field,string,optional,no_validation,true,no_vocabulary,,,no_transfor
 moving_image_field,string,optional,no_validation,true,no_vocabulary,,,no_transformation,no_facet,help me,false
 map_field,string,optional,no_validation,true,no_vocabulary,,,no_transformation,no_facet,help me,false
 audio_field,string,optional,no_validation,true,no_vocabulary,,,no_transformation,no_facet,help me,false
-alternate_ids,alternate_id,required_to_publish,no_validation,true,no_vocabulary,,Identifier,no_transformation,no_facet,help me,true
+alternate_ids,alternate_id,required_to_publish,unique,true,no_vocabulary,,Identifier,no_transformation,no_facet,help me,true
 creator,creator,optional,creator,true,creator_vocabulary,,Creator,no_transformation,linked_field,help me,true

spec/cho/collection/archival_collections/new_spec.rb

@@ -21,10 +21,14 @@
   end
 
   context 'without providing the required metadata' do
+    before { create(:collection, alternate_ids: 'existing-id') }
+
     it 'reports the errors' do
       visit(new_archival_collection_path)
+      fill_in('archival_collection[alternate_ids]', with: 'existing-id')
       click_button('Create Archival collection')
       expect(page).to have_content("Title can't be blank")
+      expect(page).to have_content('Alternate ids existing-id already exists')
     end
   end
 end

spec/cho/collection/curated_collections/new_spec.rb

@@ -21,10 +21,14 @@
   end
 
   context 'without providing the required metadata' do
+    before { create(:collection, alternate_ids: 'existing-id') }
+
     it 'reports the errors' do
       visit(new_curated_collection_path)
+      fill_in('curated_collection[alternate_ids]', with: 'existing-id')
       click_button('Create Curated collection')
       expect(page).to have_content("Title can't be blank")
+      expect(page).to have_content('Alternate ids existing-id already exists')
     end
   end
 end

spec/cho/collection/library_collections/new_spec.rb

@@ -21,10 +21,14 @@
   end
 
   context 'without providing the required metadata' do
+    before { create(:collection, alternate_ids: 'existing-id') }
+
     it 'reports the errors' do
       visit(new_library_collection_path)
+      fill_in('library_collection[alternate_ids]', with: 'existing-id')
       click_button('Create Library collection')
       expect(page).to have_content("Title can't be blank")
+      expect(page).to have_content('Alternate ids existing-id already exists')
     end
   end
 end

spec/cho/validation/factory_spec.rb

@@ -29,7 +29,16 @@ class OtherValidator < Validation::Base
     describe '#validator_names' do
       subject { described_class.validator_names }
 
-      it { is_expected.to contain_exactly('creator', 'my_validator', 'no_validation', 'resource_exists', 'edtf_date') }
+      it do
+        is_expected.to contain_exactly(
+          'creator',
+          'my_validator',
+          'no_validation',
+          'resource_exists',
+          'unique',
+          'edtf_date'
+        )
+      end
     end
   end
 
@@ -60,9 +69,17 @@ class OtherValidator < Validation::Base
     describe '#validator_names' do
       subject(:names) { described_class.validator_names }
 
-      it { expect(names).to contain_exactly(
-        'creator', 'my_validator', 'other_validator', 'no_validation', 'resource_exists', 'edtf_date'
-      ) }
+      it do
+        expect(names).to contain_exactly(
+          'creator',
+          'my_validator',
+          'other_validator',
+          'no_validation',
+          'resource_exists',
+          'edtf_date',
+          'unique'
+        )
+      end
     end
   end
 end

spec/cho/validation/member_spec.rb

@@ -3,19 +3,35 @@
 require 'rails_helper'
 
 RSpec.describe Validation::Member, type: :model do
-  describe '#exists?' do
+  describe '#exists_by_id?' do
     context 'with an id' do
       subject { described_class.new(collection.id.to_s) }
 
       let(:collection) { create(:archival_collection) }
 
-      its(:exists?) { is_expected.to be(true) }
+      its(:exists_by_id?) { is_expected.to be(true) }
     end
 
     context 'with a non-existent id' do
       subject { described_class.new('non-existent-id') }
 
-      its(:exists?) { is_expected.to be(false) }
+      its(:exists_by_id?) { is_expected.to be(false) }
+    end
+  end
+
+  describe '#exists_by_alternate_id?' do
+    context 'with an id' do
+      subject { described_class.new(collection.alternate_ids.first) }
+
+      let(:collection) { create(:archival_collection, alternate_ids: 'alt-id') }
+
+      its(:exists_by_alternate_id?) { is_expected.to be(true) }
+    end
+
+    context 'with a non-existent id' do
+      subject { described_class.new('non-existent-id') }
+
+      its(:exists_by_alternate_id?) { is_expected.to be(false) }
     end
   end
 end

spec/cho/validation/unique_spec.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Validation::Unique, type: :model do
+  let(:validation_instance) { described_class.new(change_set: change_set, field: field) }
+
+  describe 'validating alternate_ids' do
+    let(:field) { :alternate_ids }
+    let(:change_set) { Work::SubmissionChangeSet.new(work) }
+
+    context 'when creating a new resource and the alternate id is unique or does not exist' do
+      let(:work) { Work::Submission.new }
+
+      it 'is valid' do
+        expect(validation_instance.validate('i-am-special')).to be_truthy
+        expect(validation_instance.errors).to be_empty
+      end
+    end
+
+    context 'when creating a new resource and the alternate already exists' do
+      let(:work) { Work::Submission.new }
+
+      before { create(:work, alternate_ids: 'already-used') }
+
+      it 'is valid' do
+        expect(validation_instance.validate('already-used')).to be_falsey
+        expect(validation_instance.errors).to contain_exactly('already-used already exists')
+      end
+    end
+
+    context 'when updating an existing resource and the id already exists' do
+      let(:work) { create(:work, alternate_ids: ['my-alt-id']) }
+
+      before do
+        create(:work, alternate_ids: 'my-updated-alt-id')
+        change_set.alternate_ids = ['my-updated-alt-id']
+      end
+
+      it 'is valid' do
+        expect(validation_instance.validate('my-updated-alt-id')).to be_falsey
+        expect(validation_instance.errors).to contain_exactly('my-updated-alt-id already exists')
+      end
+    end
+
+    context 'with a blank id' do
+      let(:work) { Work::Submission.new }
+
+      it 'is valid' do
+        expect(validation_instance.validate('')).to be_truthy
+        expect(validation_instance.errors).to be_empty
+      end
+    end
+
+    context 'with a nil id' do
+      let(:work) { Work::Submission.new }
+
+      it 'is valid' do
+        expect(validation_instance.validate(nil)).to be_truthy
+        expect(validation_instance.errors).to be_empty
+      end
+    end
+  end
+end

spec/cho/work/submissions/edit_spec.rb

@@ -3,7 +3,7 @@
 require 'rails_helper'
 
 RSpec.describe 'Editing works', type: :feature do
-  let(:resource) { create(:work, :with_creator, title: 'Work to edit', work_type_id: work_type.id) }
+  let(:resource) { create(:work, :with_creator, :with_metadata, title: 'Work to edit', work_type_id: work_type.id) }
   let(:work_type)  { Work::Type.where(label: 'Document').first }
   let(:adapter) { Valkyrie::MetadataAdapter.find(:indexing_persister) }
   let(:solr_document) { SolrDocument.find(resource.id) }

spec/cho/work/submissions/new_spec.rb

@@ -56,7 +56,7 @@
     end
   end
 
-  context 'without providing a title' do
+  context 'missing or incorrect metadata' do
     let!(:archival_collection) { create(:archival_collection, title: 'Sample Collection') }
 
     it 'reports the errors' do

spec/factories/works.rb

@@ -62,4 +62,8 @@
       end
     end
   end
+
+  trait :with_metadata do
+    alternate_ids { ['alt-work-id'] }
+  end
 end