Allow QueueTab to return tasks (#11735)

Connects #11733. Moves logic for fetching tasks for a given tab from `TaskPager` to each individual `QueueTab`. Three major changes included in this PR: 1. `TaskPager` no longer controls which tasks a given tab is supposed to return. That responsibility is transferred to the various `QueueTab` subclasses' `tasks` method. 1. `QueueTab`s now require an `assignee` argument when they are created. 1. Added a new `QueueTab.from_name` function for returning a tab class given the tab's name string we receive from the front-end.
department-of-veterans-affairs · Aug 12, 2019 · 61fc690 · 61fc690
1 parent bab50bb
commit 61fc690
Show file tree

Hide file tree

Showing 16 changed files with 298 additions and 107 deletions.
diff --git a/app/models/organization.rb b/app/models/organization.rb
@@ -86,23 +86,23 @@ def queue_tabs
 
   def unassigned_tasks_tab
     ::UnassignedTasksTab.new(
-      assignee_name: name,
+      assignee: self,
       show_regional_office_column: show_regional_office_in_queue?,
       show_reader_link_column: show_reader_link_column?,
       allow_bulk_assign: can_bulk_assign_tasks?
     )
   end
 
   def assigned_tasks_tab
-    ::AssignedTasksTab.new(assignee_name: name, show_regional_office_column: show_regional_office_in_queue?)
+    ::AssignedTasksTab.new(assignee: self, show_regional_office_column: show_regional_office_in_queue?)
   end
 
   def on_hold_tasks_tab
-    ::OnHoldTasksTab.new(assignee_name: name, show_regional_office_column: show_regional_office_in_queue?)
+    ::OnHoldTasksTab.new(assignee: self, show_regional_office_column: show_regional_office_in_queue?)
   end
 
   def completed_tasks_tab
-    ::CompletedTasksTab.new(assignee_name: name, show_regional_office_column: show_regional_office_in_queue?)
+    ::CompletedTasksTab.new(assignee: self, show_regional_office_column: show_regional_office_in_queue?)
   end
 
   def ama_task_serializer

diff --git a/app/models/queue_config.rb b/app/models/queue_config.rb
@@ -49,7 +49,7 @@ def attach_tasks_to_tab(tab, user)
     tab.to_hash.merge(
       tasks: tasks,
       task_page_count: task_pager.task_page_count,
-      total_task_count: task_pager.total_task_count,
+      total_task_count: tab.tasks.count,
       task_page_endpoint_base_path: endpoint
     )
   end

diff --git a/app/models/queue_tab.rb b/app/models/queue_tab.rb
@@ -3,27 +3,73 @@
 class QueueTab
   include ActiveModel::Model
 
-  attr_accessor :assignee_name, :show_regional_office_column
+  validate :assignee_is_organization
+
+  attr_accessor :assignee, :show_regional_office_column
+
+  def initialize(args)
+    super
+    fail(Caseflow::Error::MissingRequiredProperty, message: errors.full_messages.join(", ")) unless valid?
+  end
+
+  def self.from_name(tab_name)
+    tab = subclasses.find { |subclass| subclass.tab_name == tab_name }
+    fail(Caseflow::Error::InvalidTaskTableTab, tab_name: tab_name) unless tab
+
+    tab
+  end
 
   def to_hash
     {
       label: label,
       name: name,
-      description: format(description, assignee_name),
+      description: format(description, assignee.name),
       columns: columns,
       allow_bulk_assign: allow_bulk_assign?
     }
   end
 
   def label; end
 
-  def name; end
+  def self.tab_name; end
+
+  def tasks; end
 
   def description; end
 
   def columns; end
 
+  def name
+    self.class.tab_name
+  end
+
   def allow_bulk_assign?
     false
   end
+
+  private
+
+  def on_hold_tasks
+    Task.includes(*task_includes).visible_in_queue_table_view.where(assigned_to: assignee).on_hold
+  end
+
+  def task_includes
+    [
+      { appeal: [:available_hearing_locations, :claimants] },
+      :assigned_by,
+      :assigned_to,
+      :children,
+      :parent
+    ]
+  end
+
+  def assignee_is_organization
+    errors.add(:assignee, COPY::TASK_PAGE_INVALID_ASSIGNEE_MESSAGE) unless assignee&.is_a?(Organization)
+  end
 end
+
+require_dependency "assigned_tasks_tab"
+require_dependency "completed_tasks_tab"
+require_dependency "on_hold_tasks_tab"
+require_dependency "tracking_tasks_tab"
+require_dependency "unassigned_tasks_tab"
diff --git a/app/models/queue_tabs/assigned_tasks_tab.rb b/app/models/queue_tabs/assigned_tasks_tab.rb
@@ -5,14 +5,18 @@ def label
     COPY::QUEUE_PAGE_ASSIGNED_TAB_TITLE
   end
 
-  def name
+  def self.tab_name
     Constants.QUEUE_CONFIG.ASSIGNED_TASKS_TAB_NAME
   end
 
   def description
     COPY::ORGANIZATIONAL_QUEUE_PAGE_ASSIGNED_TASKS_DESCRIPTION
   end
 
+  def tasks
+    Task.includes(*task_includes).visible_in_queue_table_view.active.where(parent: on_hold_tasks)
+  end
+
   def columns
     [
       Constants.QUEUE_CONFIG.HEARING_BADGE_COLUMN,

diff --git a/app/models/queue_tabs/completed_tasks_tab.rb b/app/models/queue_tabs/completed_tasks_tab.rb
@@ -5,14 +5,18 @@ def label
     COPY::QUEUE_PAGE_COMPLETE_TAB_TITLE
   end
 
-  def name
+  def self.tab_name
     Constants.QUEUE_CONFIG.COMPLETED_TASKS_TAB_NAME
   end
 
   def description
     COPY::QUEUE_PAGE_COMPLETE_TASKS_DESCRIPTION
   end
 
+  def tasks
+    Task.includes(*task_includes).visible_in_queue_table_view.where(assigned_to: assignee).recently_closed
+  end
+
   def columns
     [
       Constants.QUEUE_CONFIG.HEARING_BADGE_COLUMN,

diff --git a/app/models/queue_tabs/on_hold_tasks_tab.rb b/app/models/queue_tabs/on_hold_tasks_tab.rb
@@ -5,14 +5,18 @@ def label
     COPY::QUEUE_PAGE_ON_HOLD_TAB_TITLE
   end
 
-  def name
+  def self.tab_name
     Constants.QUEUE_CONFIG.ON_HOLD_TASKS_TAB_NAME
   end
 
   def description
     COPY::ORGANIZATIONAL_QUEUE_PAGE_ON_HOLD_TASKS_DESCRIPTION
   end
 
+  def tasks
+    Task.includes(*task_includes).visible_in_queue_table_view.on_hold.where(parent: on_hold_tasks)
+  end
+
   def columns
     [
       Constants.QUEUE_CONFIG.HEARING_BADGE_COLUMN,

diff --git a/app/models/queue_tabs/tracking_tasks_tab.rb b/app/models/queue_tabs/tracking_tasks_tab.rb
@@ -5,14 +5,18 @@ def label
     COPY::TRACKING_TASKS_TAB_TITLE
   end
 
-  def name
+  def self.tab_name
     Constants.QUEUE_CONFIG.TRACKING_TASKS_TAB_NAME
   end
 
   def description
     COPY::TRACKING_TASKS_TAB_DESCRIPTION
   end
 
+  def tasks
+    TrackVeteranTask.includes(*task_includes).active.where(assigned_to: assignee)
+  end
+
   def columns
     [
       Constants.QUEUE_CONFIG.CASE_DETAILS_LINK_COLUMN,

diff --git a/app/models/queue_tabs/unassigned_tasks_tab.rb b/app/models/queue_tabs/unassigned_tasks_tab.rb
@@ -7,14 +7,18 @@ def label
     COPY::ORGANIZATIONAL_QUEUE_PAGE_UNASSIGNED_TAB_TITLE
   end
 
-  def name
+  def self.tab_name
     Constants.QUEUE_CONFIG.UNASSIGNED_TASKS_TAB_NAME
   end
 
   def description
     COPY::ORGANIZATIONAL_QUEUE_PAGE_UNASSIGNED_TASKS_DESCRIPTION
   end
 
+  def tasks
+    Task.includes(*task_includes).visible_in_queue_table_view.where(assigned_to: assignee).active
+  end
+
   def columns
     [
       Constants.QUEUE_CONFIG.HEARING_BADGE_COLUMN,

diff --git a/app/models/representative.rb b/app/models/representative.rb
@@ -32,7 +32,7 @@ def queue_tabs
   end
 
   def tracking_tasks_tab
-    ::TrackingTasksTab.new(assignee_name: name)
+    ::TrackingTasksTab.new(assignee: self)
   end
 
   def ama_task_serializer

diff --git a/app/models/task_pager.rb b/app/models/task_pager.rb
@@ -92,63 +92,17 @@ def task_page_count
     @task_page_count ||= paged_tasks.total_pages
   end
 
-  def total_task_count
-    @total_task_count ||= tasks_for_tab.count
-  end
-
   def filtered_tasks
     where_clause = QueueWhereClauseArgumentsFactory.new(filter_params: filters).arguments
     where_clause.empty? ? tasks_for_tab : tasks_for_tab.joins(cached_attributes_join_clause).where(*where_clause)
   end
 
   def tasks_for_tab
-    case tab_name
-    when Constants.QUEUE_CONFIG.TRACKING_TASKS_TAB_NAME
-      tracking_tasks
-    when Constants.QUEUE_CONFIG.UNASSIGNED_TASKS_TAB_NAME
-      active_tasks
-    when Constants.QUEUE_CONFIG.ASSIGNED_TASKS_TAB_NAME
-      assigned_child_tasks
-    when Constants.QUEUE_CONFIG.ON_HOLD_TASKS_TAB_NAME
-      on_hold_child_tasks
-    when Constants.QUEUE_CONFIG.COMPLETED_TASKS_TAB_NAME
-      recently_completed_tasks
-    else
-      fail(Caseflow::Error::InvalidTaskTableTab, tab_name: tab_name)
-    end
+    QueueTab.from_name(tab_name).new(assignee: assignee).tasks
   end
 
   private
 
-  def tracking_tasks
-    TrackVeteranTask.includes(*task_includes).active.where(assigned_to: assignee)
-  end
-
-  def active_tasks
-    Task.includes(*task_includes)
-      .visible_in_queue_table_view.where(assigned_to: assignee).active
-  end
-
-  def on_hold_tasks
-    Task.includes(*task_includes)
-      .visible_in_queue_table_view.where(assigned_to: assignee).on_hold
-  end
-
-  def assigned_child_tasks
-    Task.includes(*task_includes)
-      .visible_in_queue_table_view.active.where(parent: on_hold_tasks)
-  end
-
-  def on_hold_child_tasks
-    Task.includes(*task_includes)
-      .visible_in_queue_table_view.on_hold.where(parent: on_hold_tasks)
-  end
-
-  def recently_completed_tasks
-    Task.includes(*task_includes)
-      .visible_in_queue_table_view.where(assigned_to: assignee).recently_closed
-  end
-
   def assignee_is_user_or_organization
     unless assignee.is_a?(User) || assignee.is_a?(Organization)
       errors.add(:assignee, COPY::TASK_PAGE_INVALID_ASSIGNEE_MESSAGE)
@@ -159,14 +113,4 @@ def sort_order_is_valid
     valid_sort_orders = [Constants.QUEUE_CONFIG.COLUMN_SORT_ORDER_ASC, Constants.QUEUE_CONFIG.COLUMN_SORT_ORDER_DESC]
     errors.add(:sort_order, COPY::TASK_PAGE_INVALID_SORT_ORDER) unless valid_sort_orders.include?(sort_order)
   end
-
-  def task_includes
-    [
-      { appeal: [:available_hearing_locations, :claimants] },
-      :assigned_by,
-      :assigned_to,
-      :children,
-      :parent
-    ]
-  end
 end
diff --git a/spec/models/queue_tab_spec.rb b/spec/models/queue_tab_spec.rb
@@ -1,24 +1,25 @@
 # frozen_string_literal: true
 
 require "rails_helper"
+require "support/database_cleaner"
 
-describe QueueTab do
+describe QueueTab, :postgres do
   # Use AssignedTasksTab as our example since we don't expect QueueTab to ever be instantiated directly.
   let(:tab) { AssignedTasksTab.new(params) }
   let(:params) do
     {
-      assignee_name: assignee_name,
+      assignee: assignee,
       show_regional_office_column: show_regional_office_column
     }
   end
-  let(:assignee_name) { "organization name" }
+  let(:assignee) { create(:organization) }
   let(:show_regional_office_column) { false }
 
   describe ".allow_bulk_assign?" do
     subject { tab.allow_bulk_assign? }
 
-    context "when no arguments are passed when instantiating the object" do
-      let(:params) { {} }
+    context "when only the assignee argument is passed when instantiating the object" do
+      let(:params) { { assignee: assignee } }
 
       it "returns false" do
         expect(subject).to eq(false)
@@ -33,8 +34,58 @@
       expect(subject.keys).to match_array([:label, :name, :description, :columns, :allow_bulk_assign])
     end
 
-    it "interpolates assignee_name name in description element of hash" do
-      expect(subject[:description]).to eq(format(tab.description, assignee_name))
+    it "interpolates assignee name in description element of hash" do
+      expect(subject[:description]).to eq(format(tab.description, assignee.name))
+    end
+  end
+
+  describe ".new" do
+    subject { tab }
+
+    context "when the assignee is not an organization" do
+      let(:assignee) { create(:user) }
+
+      it "raises an error" do
+        expect { subject }.to raise_error(Caseflow::Error::MissingRequiredProperty)
+      end
+    end
+
+    context "when there is no assignee parameter passed when instantiating the tab" do
+      let(:params) { { show_regional_office_column: show_regional_office_column } }
+
+      it "raises an error" do
+        expect { subject }.to raise_error(Caseflow::Error::MissingRequiredProperty)
+      end
+    end
+
+    context "when the assignee is an organization" do
+      let(:assignee) { create(:organization) }
+
+      it "is created successfully" do
+        expect { subject }.to_not raise_error
+        expect(subject).to be_a(AssignedTasksTab)
+      end
+    end
+  end
+
+  describe "#from_name" do
+    subject { QueueTab.from_name(tab_name) }
+
+    context "when not tab class exists with the given name" do
+      let(:tab_name) { "non-existent tab name" }
+
+      it "raises an error" do
+        expect { subject }.to raise_error(Caseflow::Error::InvalidTaskTableTab)
+      end
+    end
+
+    context "when a tab class with that name exists" do
+      let(:tab_name) { Constants.QUEUE_CONFIG.COMPLETED_TASKS_TAB_NAME }
+
+      it "returns the class" do
+        expect { subject }.to_not raise_error
+        expect(subject).to eq(CompletedTasksTab)
+      end
     end
   end
 end