Expand the only_open_task_of_type validation to AttorneyTask and JudgeDecisionReviewTask

app/models/task.rb

@@ -95,6 +95,8 @@ class << self; undef_method :open; end
 
   scope :with_cached_appeals, -> { joins(Task.joins_with_cached_appeals_clause) }
 
+  attr_accessor :skip_check_for_only_open_task_of_type
+
   ############################################################################################
   ## class methods
   class << self
@@ -562,7 +564,7 @@ def verify_user_can_update!(user)
   def reassign(reassign_params, current_user)
     # We do not validate the number of tasks in this scenario because when a
     # task is reassigned, more than one open task of the same type must exist during the reassignment.
-    Thread.current.thread_variable_set(:skip_check_for_only_open_task_of_type, true)
+    @skip_check_for_only_open_task_of_type = true
     replacement = dup.tap do |task|
       begin
         ActiveRecord::Base.transaction do
@@ -573,10 +575,8 @@ def reassign(reassign_params, current_user)
 
           task.save!
         end
-      # The ensure block guarantees that the thread-local variable check_for_open_task_type
-      # does not leak outside of this method
       ensure
-        Thread.current.thread_variable_set(:skip_check_for_only_open_task_of_type, nil)
+        @skip_check_for_only_open_task_of_type = nil
       end
     end
 
@@ -834,10 +834,6 @@ def assignee_status_is_valid_on_create
     true
   end
 
-  def skip_check_for_only_open_task_of_type?
-    Thread.current.thread_variable_get(:skip_check_for_only_open_task_of_type)
-  end
-
   def only_open_task_of_type
     if appeal.reload.tasks.open.of_type(type).any?
       fail Caseflow::Error::MultipleOpenTasksOfSameTypeError, task_type: type

app/models/tasks/attorney_task.rb

@@ -7,6 +7,9 @@
 #   - adding admin actions (like translating documents)
 
 class AttorneyTask < Task
+  validate :only_open_task_of_type, on: :create,
+                                    unless: :skip_check_for_only_open_task_of_type
+
   validates :assigned_by, presence: true
   validates :parent, presence: true, if: :ama?
 

app/models/tasks/docket_switch/docket_switch_abstract_attorney_task.rb

@@ -38,4 +38,12 @@ def default_actions
       ]
     end
   end
+
+  private
+
+  def only_open_task_of_type
+    # This overrides the validation inherited from the parent class, AttorneyTask.
+    # Docket switch requires an appeal to have two DocketSwitchAbstractAttorneyTasks,
+    # with the parent assigned to COTB and the child assigned to a user.
+  end
 end

app/models/tasks/judge_assign_task.rb

@@ -15,7 +15,7 @@
 
 class JudgeAssignTask < JudgeTask
   validate :only_open_task_of_type, on: :create,
-                                    unless: :skip_check_for_only_open_task_of_type?
+                                    unless: :skip_check_for_only_open_task_of_type
 
   def additional_available_actions(_user)
     [Constants.TASK_ACTIONS.ASSIGN_TO_ATTORNEY.to_h]

app/models/tasks/judge_decision_review_task.rb

@@ -9,7 +9,8 @@
 # and create a new JudgeAssignTask, because another assignment by a judge is needed.
 
 class JudgeDecisionReviewTask < JudgeTask
-  before_create :verify_user_task_unique
+  validate :only_open_task_of_type, on: :create,
+                                    unless: :skip_check_for_only_open_task_of_type
 
   def additional_available_actions(user)
     return [] unless assigned_to == user
@@ -31,24 +32,6 @@ def self.label
     COPY::JUDGE_DECISION_REVIEW_TASK_LABEL
   end
 
-  # Use the existence of another open JudgeDecisionReviewTask to prevent duplicates since there should only
-  # ever be one open JudgeDecisionReviewTask at a time for an appeal.
-  def verify_user_task_unique
-    return if !open?
-
-    if appeal.tasks.open.where(
-      type: type,
-      assigned_to: assigned_to,
-      parent: parent
-    ).any? && assigned_to.is_a?(User)
-      fail(
-        Caseflow::Error::DuplicateUserTask,
-        docket_number: appeal.docket_number,
-        task_type: self.class.name
-      )
-    end
-  end
-
   private
 
   def ama_judge_actions(user)

spec/feature/queue/case_details_spec.rb

@@ -1219,15 +1219,13 @@ def wait_for_page_render
         )
       end
       let!(:attorney_task) { create(:ama_attorney_task, parent: judge_task, assigned_to: user) }
-      let!(:attorney_task2) { create(:ama_attorney_task, parent: root_task, assigned_to: user) }
 
       before do
         FeatureToggle.enable!(:view_nod_date_updates)
         # The status attribute needs to be set here due to update_parent_status hook in the task model
         # the updated_at attribute needs to be set here due to the set_timestamps hook in the task model
         assign_task.update!(status: Constants.TASK_STATUSES.completed, closed_at: "2019-01-01")
         attorney_task.update!(status: Constants.TASK_STATUSES.completed, closed_at: "2019-02-01")
-        attorney_task2.update!(status: Constants.TASK_STATUSES.completed, closed_at: "2019-03-01")
         judge_task.update!(status: Constants.TASK_STATUSES.completed, closed_at: Time.zone.now)
         nod_date_update.update!(updated_at: "2019-01-05")
       end
@@ -1246,11 +1244,9 @@ def wait_for_page_render
         first_row_with_date = case_timeline_rows[1]
         second_row_with_date = case_timeline_rows[2]
         third_row_with_date = case_timeline_rows[3]
-        fourth_row_with_date = case_timeline_rows[4]
         expect(first_row_with_date).to have_content("01/01/2020")
-        expect(second_row_with_date).to have_content("03/01/2019")
-        expect(third_row_with_date).to have_content("02/01/2019")
-        expect(fourth_row_with_date).to have_content("01/05/2019")
+        expect(second_row_with_date).to have_content("02/01/2019")
+        expect(third_row_with_date).to have_content("01/05/2019")
       end
 
       it "should NOT display judge & attorney tasks" do

spec/feature/queue/judge_assignment_spec.rb

@@ -232,8 +232,9 @@
         click_on "Assign 2 cases"
         expect(page).to have_content("#{attorney_one.full_name} (0)")
         expect(page).to have_content("Error assigning tasks")
-        expect(page).to have_content("Docket (#{appeal_two.docket_number}) already "\
-                                     "has an open task type of #{JudgeDecisionReviewTask.name}")
+
+        expect(page).to have_content("Looks like this appeal already has an open JudgeDecisionReviewTask "\
+          "and this action cannot be completed.")
 
         visit "/queue/#{judge_one.user.css_id}/assign"
         expect(page).to have_content("#{attorney_one.full_name} (0)")

spec/feature/queue/task_queue_spec.rb

@@ -17,16 +17,6 @@
       )
     end
 
-    let!(:attorney_task_new_docs) do
-      create(
-        :ama_attorney_task,
-        :on_hold,
-        assigned_to: attorney_user,
-        placed_on_hold_at: 4.days.ago,
-        appeal: attorney_task.appeal
-      )
-    end
-
     let!(:colocated_task) do
       create(
         :ama_colocated_task,

spec/models/appeal_spec.rb

@@ -250,10 +250,16 @@
 
   context "#latest_attorney_case_review" do
     let(:appeal) { create(:appeal) }
-    let(:task1) { create(:ama_attorney_task, appeal: appeal) }
-    let(:task2) { create(:ama_attorney_task, appeal: appeal) }
+    let!(:task1) { create(:ama_attorney_task, appeal: appeal) }
     let!(:attorney_case_review1) { create(:attorney_case_review, task: task1, created_at: 2.days.ago) }
-    let!(:attorney_case_review2) { create(:attorney_case_review, task: task2, created_at: 1.day.ago) }
+    let!(:attorney_case_review2) do
+      task1.update!(status: Constants.TASK_STATUSES.completed)
+      if appeal.tasks.open.of_type("JudgeDecisionReviewTask").any?
+        appeal.tasks.open.find_by(type: "JudgeDecisionReviewTask").completed!
+      end
+      task2 = create(:ama_attorney_task, appeal: appeal)
+      create(:attorney_case_review, task: task2, created_at: 1.day.ago)
+    end
 
     subject { appeal.latest_attorney_case_review }
 
@@ -909,8 +915,15 @@
       let!(:attorney) { create(:user) }
       let!(:attorney2) { create(:user) }
       let!(:appeal) { create(:appeal) }
-      let!(:task) { create(:ama_attorney_task, assigned_to: attorney, appeal: appeal, created_at: 1.day.ago) }
-      let!(:task2) { create(:ama_attorney_task, assigned_to: attorney2, appeal: appeal) }
+      let(:judge_decision_review_task) { create(:ama_judge_decision_review_task, appeal: appeal) }
+      let!(:task) do
+        create(:ama_attorney_task, parent: judge_decision_review_task, assigned_to: attorney,
+                                   appeal: appeal, created_at: 1.day.ago)
+      end
+      let!(:task2) do
+        task.completed!
+        create(:ama_attorney_task, parent: judge_decision_review_task, assigned_to: attorney2, appeal: appeal)
+      end
 
       subject { appeal.assigned_attorney }
 
@@ -919,7 +932,7 @@
       end
 
       it "should know the right assigned attorney with a cancelled task" do
-        task2.update(status: "cancelled")
+        task2.cancelled!
         expect(subject).to eq attorney
       end
     end

spec/models/concerns/task_tree_render_module_spec.rb

@@ -5,7 +5,7 @@
   let(:root_task) { appeal.root_task }
   let!(:ama_judge_assign_task) { create(:ama_judge_assign_task, parent: root_task, created_at: 1.day.ago.round) }
   let!(:ama_attorney_task) { create(:ama_attorney_task, parent: root_task) }
-  let!(:ama_attorney_task_no_parent) { create(:ama_attorney_task, appeal: appeal) }
+  let!(:task_no_parent) { create(:track_veteran_task, appeal: appeal) }
 
   context "#tree is called on an appeal" do
     it "returns all tasks for the appeal" do
@@ -131,7 +131,7 @@ def tree2(obj, *atts, **kwargs)
       initial_count = appeal.tasks.count + 3
       expect(appeal.tree.lines.count).to eq initial_count
 
-      ama_attorney_task_no_parent.delete
+      task_no_parent.delete
       appeal.reload
       expect(appeal.tree.lines.count).to eq initial_count - 1
     end

spec/models/tasks/attorney_task_spec.rb

@@ -51,6 +51,15 @@
         expect(subject.errors.messages[:assigned_to].first).to eq "has to be an attorney"
       end
     end
+
+    context "when an AttorneyTask is already open for the appeal" do
+      let!(:attorney_task) { create(:ama_attorney_task, appeal: appeal, parent: parent) }
+      it "throws an error when a second task is created" do
+        expect { subject }.to raise_error do |error|
+          expect(error).to be_a(Caseflow::Error::MultipleOpenTasksOfSameTypeError)
+        end
+      end
+    end
   end
 
   context ".update" do

spec/models/tasks/colocated_task_spec.rb

@@ -611,8 +611,7 @@
       create(:judge_address_motion_to_vacate_task,
              appeal: appeal,
              assigned_to: judge,
-             parent: vacate_motion_mail_task
-      )
+             parent: vacate_motion_mail_task)
     end
     let(:post_decision_motion_params) do
       {
@@ -629,7 +628,7 @@
       Appeal.find_by(stream_docket_number: appeal.docket_number, stream_type: Constants.AMA_STREAM_TYPES.vacate)
     end
     let(:attorney_task) { AttorneyTask.find_by(assigned_to: attorney) }
-    let(:parent) { create(:ama_judge_decision_review_task, assigned_to: judge, appeal: vacate_stream ) }
+    let(:parent) { attorney_task.parent }
 
     let(:params_list) do
       [{

spec/models/tasks/judge_task_spec.rb

@@ -16,6 +16,9 @@
     let!(:first_assign_task) do
       create(:ama_judge_assign_task, assigned_to: judge, appeal: appeal)
     end
+    let!(:first_review_task) do
+      create(:ama_judge_decision_review_task, assigned_to: judge, appeal: appeal)
+    end
 
     context "when one judge assign task is open for an appeal" do
       it "throws an error when a second task is created" do
@@ -24,11 +27,18 @@
         end
       end
     end
+
+    context "when one judge decision review task is open for an appeal" do
+      it "throws an error when a second task is created" do
+        expect { create(:ama_judge_decision_review_task, assigned_to: judge2, appeal: appeal) }.to raise_error do |err|
+          expect(err).to be_a(Caseflow::Error::MultipleOpenTasksOfSameTypeError)
+        end
+      end
+    end
   end
 
   describe "reassign" do
     let(:root_task) { create(:root_task) }
-    let(:task) { create(:ama_judge_assign_task, parent: root_task) }
     let(:old_assignee) { task.assigned_to }
     let(:new_assignee) { create(:user) }
     let(:params) do
@@ -40,25 +50,21 @@
     end
     subject { task.reassign(params, old_assignee) }
 
-    context "when a judge task is reassigned successfully" do
-      it "should not violate the only_open_task_of_type validation" do
-        expect { subject }.to_not raise_error
-        expect(Thread.current.thread_variable_get(:skip_check_for_only_open_task_of_type)).to be_nil
+    context "when the task is a judge decision review task" do
+      let(:task) { create(:ama_judge_decision_review_task, parent: root_task) }
+      context "when the judge decision review task is reassigned successfully" do
+        it "does not violate the only_open_task_of_type validation" do
+          expect { subject }.to_not raise_error
+        end
       end
     end
 
-    context "when a judge task throws an error during attempted reassignment" do
-      let(:params) do
-        {
-          assigned_to_id: nil,
-          assigned_to_type: new_assignee.class.name,
-          instructions: "instructions"
-        }
-      end
-
-      it "sets the thread local variable of skip_only_open_task_of_type to nil" do
-        expect { subject }.to raise_error ActiveRecord::RecordNotFound
-        expect(Thread.current.thread_variable_get(:skip_check_for_only_open_task_of_type)).to be_nil
+    context "when the task is a judge assign task" do
+      let(:task) { create(:ama_judge_assign_task, parent: root_task) }
+      context "when the judge assign task is reassigned successfully" do
+        it "should not violate the only_open_task_of_type validation" do
+          expect { subject }.to_not raise_error
+        end
       end
     end
   end
@@ -246,7 +252,7 @@
           it "should fail creation of second JudgeDecisionReviewTask" do
             expect(root_task.appeal.tasks.count).to eq(2), root_task.appeal.tasks.to_a.to_s
             jdrt.update(status: o_status)
-            expect { subject }.to raise_error(Caseflow::Error::DuplicateUserTask)
+            expect { subject }.to raise_error(Caseflow::Error::MultipleOpenTasksOfSameTypeError)
             expect(root_task.appeal.tasks.count).to eq(2), root_task.appeal.tasks.to_a.to_s
           end
         end