Merge pull request #2810 from aignatov-bio/ai-sci-4968-add-error-handling-for-state-change

Add error handling for state change [SCI-4968]

app/assets/javascripts/my_modules.js

@@ -246,7 +246,7 @@ function applyTaskStatusChangeCallBack() {
         if (e.status === 403) {
           HelperModule.flashAlertMsg(I18n.t('my_module_statuses.update_status.error.no_permission'), 'danger');
         } else if (e.status === 422) {
-          HelperModule.flashAlertMsg(e.errors, 'danger');
+          HelperModule.flashAlertMsg(e.responseJSON.errors, 'danger');
         } else {
           HelperModule.flashAlertMsg('error', 'danger');
         }

app/controllers/my_modules_controller.rb

@@ -263,18 +263,18 @@ class MyModulesController < ApplicationController
   end
 
   def update_state
-    new_status = @my_module.my_module_status_flow.my_module_statuses.find_by(id: update_status_params[:status_id])
-    return render_404 unless new_status
-
     old_status_id = @my_module.my_module_status_id
-    @my_module.update(my_module_status: new_status)
+    if @my_module.update(my_module_status_id: update_status_params[:status_id])
+      log_activity(:change_status_on_task_flow, @my_module, my_module_status_old: old_status_id,
+                   my_module_status_new: @my_module.my_module_status.id)
 
-    log_activity(:change_status_on_task_flow, @my_module, my_module_status_old: old_status_id,
-                 my_module_status_new: @my_module.my_module_status.id)
-
-    render json: { content: render_to_string(
-      partial: 'my_modules/status_flow/task_flow_button.html.erb', locals: { my_module: @my_module }
-    ) }, status: :ok
+      render json: { content: render_to_string(
+        partial: 'my_modules/status_flow/task_flow_button.html.erb',
+        locals: { my_module: @my_module }
+      ) }, status: :ok
+    else
+      render json: { errors: @my_module.errors.messages.values.flatten.join('\n') }, status: :unprocessable_entity
+    end
   end
 
   private

app/models/my_module.rb

@@ -7,8 +7,9 @@ class MyModule < ApplicationRecord
   enum state: Extends::TASKS_STATES
 
   before_create :create_blank_protocol
+  before_create :assign_default_status_flow
   before_validation :set_completed_on, if: :state_changed?
-  before_validation :assign_default_status_flow
+
   before_save :exec_status_consequences, if: :my_module_status_id_changed?
 
   auto_strip_attributes :name, :description, nullify: false
@@ -22,6 +23,7 @@ class MyModule < ApplicationRecord
   validate :coordinates_uniqueness_check, if: :active?
   validates :completed_on, presence: true, if: proc { |mm| mm.completed? }
 
+  validate :check_status, if: :my_module_status_id_changed?
   validate :check_status_conditions, if: :my_module_status_id_changed?
   validate :check_status_implications, unless: :my_module_status_id_changed?
 
@@ -538,6 +540,16 @@ class MyModule < ApplicationRecord
     end
   end
 
+  def check_status
+    return unless my_module_status_id_was
+
+    original_status = MyModuleStatus.find_by(id: my_module_status_id_was)
+    unless my_module_status && [original_status.next_status, original_status.previous_status].include?(my_module_status)
+      errors.add(:my_module_status_id,
+                 I18n.t('activerecord.errors.models.my_module.attributes.my_module_status_id.not_correct_order'))
+    end
+  end
+
   def exec_status_consequences
     return if my_module_status.blank?
 

config/locales/en.yml

@@ -108,8 +108,10 @@ en:
               same_team: "Inventory can't be shared to the same team as it belongs to"
         my_module:
           attributes:
+            my_module_status_id:
+              not_correct_order: "Status can be changed only on next or previous status."
             position:
-             not_unique: "X and Y position has already been taken by another task in the experiment."
+              not_unique: "X and Y position has already been taken by another task in the experiment."
         my_module_status:
           attributes:
             next_status:

spec/controllers/my_modules_controller_spec.rb

@@ -153,23 +153,24 @@ describe MyModulesController, type: :controller do
       end
     end
 
-    context 'when status not found' do
+    context 'when status not exist' do
       let(:status_id) { -1 }
 
-      it 'renders 404' do
+      it 'renders 422' do
+        my_module.my_module_status_flow
         action
 
-        expect(response).to have_http_status 404
+        expect(response).to have_http_status 422
       end
     end
 
-    context 'when my_module does not have assign flow yet' do
-      let(:status_id) { -1 }
+    context 'when status not correct' do
+      let(:status_id) { my_module.my_module_status.next_status.next_status.id }
 
-      it 'renders 404' do
+      it 'renders 422' do
         action
 
-        expect(response).to have_http_status 404
+        expect(response).to have_http_status 422
       end
     end