Return error to user on CollectingEvent#clone when increment_id fails

app/controllers/collecting_events_controller.rb

@@ -52,18 +52,19 @@ def create
 
   # POST /collecting_events/1/clone.json
   def clone
-    @collecting_event = @collecting_event.clone(
-      annotations: params[:annotations],
-      incremented_identifier_id: params[:incremented_identifier_id]
-    )
-
-    respond_to do |format|
-      if @collecting_event.persisted?
+    begin
+      @collecting_event = @collecting_event.clone(
+        annotations: params[:annotations],
+        incremented_identifier_id: params[:incremented_identifier_id]
+      )
+      respond_to do |format|
         format.html { redirect_to new_collecting_event_task_path(@collecting_event), notice: 'Clone successful, editing new record.' }
         format.json { render :show }
-      else
-        format.html { redirect_to new_collecting_event_task_path(@collecting_event), notice: 'Failed to clone the collecting event..' }
-        format.json {render json: @collecting_event.errors, status: :unprocessable_entity}
+      end
+    rescue TaxonWorks::Error => e
+      respond_to do |format|
+        format.html { redirect_to new_collecting_event_task_path, notice: e.message }
+        format.json { render json: e.message, status: :unprocessable_entity }
       end
     end
   end

app/models/collecting_event.rb

@@ -1038,7 +1038,10 @@ def clone(annotations: false, incremented_identifier_id: nil)
         end
 
         if incremented_identifier_id
-          add_incremented_identifier(to_object: a, incremented_identifier_id:)
+          if !add_incremented_identifier(to_object: a,
+                                         incremented_identifier_id:)
+            raise TaxonWorks::Error, 'Clone failed: Unable to increment identifier, maybe no part of it is numeric?'
+          end
         end
 
         if annotations.present? # TODO: boolean param this
@@ -1047,11 +1050,12 @@ def clone(annotations: false, incremented_identifier_id: nil)
 
         a.save! # TODO: confirm behaviour is OK in case of comprehensive.
 
-      rescue ActiveRecord::RecordInvalid
-        raise ActiveRecord::Rollback
+      rescue ActiveRecord::RecordInvalid => e
+        raise TaxonWorks::Error, "Clone failed: '#{e.message}'", cause: e
       end
-      a
-    end
+    end # end transaction
+
+    a
   end
 
   # @return [String, nil]

app/models/concerns/shared/identifiers.rb

@@ -37,7 +37,7 @@ def self.with_identifiers_sorted(sort_order = 'ASC')
     def add_incremented_identifier(to_object: nil, incremented_identifier_id: nil)
       if to_object.respond_to?(:identifiers)
         i = Identifier.find(incremented_identifier_id).dup
-        i.increment_identifier
+        return false if !i.increment_identifier
         i.identifier_object = to_object
         i.save!
       else

app/models/identifier/local.rb

@@ -65,7 +65,10 @@ def is_local?
   end
 
   def increment_identifier
-    self[:identifier] = ::Utilities::Strings.increment_contained_integer(identifier)
+    # Increment can fail if the identifier has no numeric component
+    if (incremented_value = ::Utilities::Strings.increment_contained_integer(identifier))
+      self[:identifier] = incremented_value
+    end
   end
 
   protected

spec/lib/utilities/strings_spec.rb

@@ -91,6 +91,10 @@
     expect(l.increment_contained_integer('a09')).to eq('a10')
   end
 
+  specify '.increment_contained_integer 7' do
+    expect(l.increment_contained_integer('asdf')).to eq(false)
+  end
+
   specify '.escape_single_quote 1' do
     expect(l.escape_single_quote("'")).to eq("''")
   end

spec/models/collecting_event_spec.rb

@@ -460,6 +460,18 @@
       expect(b.local_identifiers.first.identifier).to eq('2')
     end
 
+    specify 'increment raises on non-incrementable identifier' do
+      a = Identifier::Local::FieldNumber.create(
+        namespace: FactoryBot.create(:valid_namespace),
+        identifier_object: collecting_event,
+        identifier: 'asdf'
+      )
+
+      expect {
+        collecting_event.clone(incremented_identifier_id: a.id)
+      }.to raise_error(TaxonWorks::Error, /failed.*identifier/)
+    end
+
     specify 'does not infinite loop dwc indexing' do
       collecting_event.collectors << FactoryBot.create(:valid_person)
       collecting_event.update!(verbatim_label: 'Some text')

spec/models/concerns/shared/identifiers_spec.rb

@@ -16,6 +16,17 @@
     expect(b.local_identifiers.first.identifier.to_f).to eq(i.identifier.to_f + 1.0)
   end
 
+  specify '#add_incremented_identifier when increment fails' do
+    a = FactoryBot.create(:valid_otu)
+    b = FactoryBot.create(:valid_otu)
+
+    i = Identifier::Local::OtuUtility.create!(identifier: 'asdf', identifier_object: a, namespace:)
+
+    expect(
+      a.add_incremented_identifier(to_object: b, incremented_identifier_id: i.id)
+    ).to be_falsey
+  end
+
   context 'associations' do
     specify 'has many identifiers' do
       expect(identifiable_instance.identifiers << Identifier.new).to be_truthy