From f2c8f67791e04786a122516c6f523e4062178c92 Mon Sep 17 00:00:00 2001
From: Oleksii Kriuchykhin <oleksii@scinote.net>
Date: Thu, 23 Mar 2023 15:32:22 +0100
Subject: [PATCH] Improve error handling in protocol archive/restore actions
 [SCI-8177]

---
 app/controllers/protocols_controller.rb | 37 +++++++++----------------
 app/models/protocol.rb                  |  4 +--
 2 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/app/controllers/protocols_controller.rb b/app/controllers/protocols_controller.rb
index 5d9677249..ad3f1b146 100644
--- a/app/controllers/protocols_controller.rb
+++ b/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
diff --git a/app/models/protocol.rb b/app/models/protocol.rb
index 890335247..c0470440d 100644
--- a/app/models/protocol.rb
+++ b/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