Improve error handling in protocol archive/restore actions [SCI-8177]

app/controllers/protocols_controller.rb

@@ -180,11 +180,11 @@ class ProtocolsController < ApplicationController
   end
 
   def archive
-    move_protocol('archive')
+    move_protocols('archive')
   end
 
   def restore
-    move_protocol('restore')
+    move_protocols('restore')
   end
 
   def edit
@@ -926,31 +926,20 @@ class ProtocolsController < ApplicationController
     return false
   end
 
-  def move_protocol(action)
-    rollbacked = false
-    begin
-      Protocol.transaction do
-        @protocols.find_each do |protocol|
-          protocol = protocol.parent if protocol.parent_id
-          protocol.method(action).call(current_user)
-        end
-      end
-    rescue StandardError => e
-      Rails.logger.error e.message
-      rollbacked = true
-    end
-
-    respond_to do |format|
-      if rollbacked
-        format.json do
-          render json: { message: I18n.t('errors.general') }, status: :unprocessable_entity
-        end
-      else
-        format.json do
-          render json: { message: t("protocols.index.#{action}_flash_html", count: @protocols.size) }
+  def move_protocols(action)
+    Protocol.transaction do
+      @protocols.find_each do |protocol|
+        protocol = protocol.parent if protocol.parent_id
+        unless protocol.method(action).call(current_user)
+          raise StandardError, protocol.errors&.messages&.values&.join(' ') || I18n.t('errors.general')
         end
       end
     end
+    render json: { message: t("protocols.index.#{action}_flash_html", count: @protocols.size) }
+  rescue StandardError => e
+    Rails.logger.error(e.message)
+    Rails.logger.error(e.backtrace.join("\n"))
+    render json: { message: e.message }, status: :unprocessable_entity
   end
 
   def set_inline_name_editing

app/models/protocol.rb

@@ -375,9 +375,7 @@ class Protocol < ApplicationRecord
   end
 
   def archive(user)
-    return nil unless can_destroy?
-    # We keep published_on present, so we know (upon restoring)
-    # where the protocol was located
+    return false unless can_destroy?
 
     self.archived_by = user
     self.archived_on = Time.now