From 27d78c298bdf7ccafb58a8987d051cbe991ced71 Mon Sep 17 00:00:00 2001 From: hschallhorn Date: Tue, 12 May 2020 14:02:28 -0400 Subject: [PATCH 1/6] Add task pagination endpoint for user tasks --- .../concerns/task_pagination_concern.rb | 41 ++++++++++++++ .../organizations/task_pages_controller.rb | 29 ++-------- .../users/task_pages_controller.rb | 34 ++++++++++++ app/controllers/users_controller.rb | 10 ++-- app/models/organization.rb | 4 -- app/models/queue_config.rb | 2 +- config/routes.rb | 5 +- .../task_pages_controller_spec.rb | 54 +++---------------- .../paged_tasks_shared_examples.rb | 50 +++++++++++++++++ .../users/task_pages_controller_spec.rb | 17 ++++++ 10 files changed, 165 insertions(+), 81 deletions(-) create mode 100644 app/controllers/concerns/task_pagination_concern.rb create mode 100644 app/controllers/users/task_pages_controller.rb create mode 100644 spec/controllers/paged_tasks_shared_examples.rb create mode 100644 spec/controllers/users/task_pages_controller_spec.rb diff --git a/app/controllers/concerns/task_pagination_concern.rb b/app/controllers/concerns/task_pagination_concern.rb new file mode 100644 index 00000000000..eb2ce725391 --- /dev/null +++ b/app/controllers/concerns/task_pagination_concern.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +# shared code for controllers that paginate tasks. +# requires methods: +# * total_items +# * allowed_params +# +module TaskPaginationConcern + extend ActiveSupport::Concern + + def pagination_json + { + tasks: json_tasks(task_pager.paged_tasks), + task_page_count: task_pager.task_page_count, + total_task_count: task_pager.total_task_count, + tasks_per_page: TaskPager::TASKS_PER_PAGE + } + end + + private + + def task_pager + @task_pager ||= TaskPager.new( + assignee: assignee, + tab_name: params[Constants.QUEUE_CONFIG.TAB_NAME_REQUEST_PARAM.to_sym], + page: params[Constants.QUEUE_CONFIG.PAGE_NUMBER_REQUEST_PARAM.to_sym], + sort_order: params[Constants.QUEUE_CONFIG.SORT_DIRECTION_REQUEST_PARAM.to_sym], + sort_by: params[Constants.QUEUE_CONFIG.SORT_COLUMN_REQUEST_PARAM.to_sym], + filters: params[Constants.QUEUE_CONFIG.FILTER_COLUMN_REQUEST_PARAM.to_sym] + ) + end + + def json_tasks(tasks) + tasks = AppealRepository.eager_load_legacy_appeals_for_tasks(tasks) + params = { user: current_user } + + AmaAndLegacyTaskSerializer.new( + tasks: tasks, params: params, ama_serializer: WorkQueue::TaskSerializer + ).call + end +end diff --git a/app/controllers/organizations/task_pages_controller.rb b/app/controllers/organizations/task_pages_controller.rb index d3a2cf42c4e..36a889de277 100644 --- a/app/controllers/organizations/task_pages_controller.rb +++ b/app/controllers/organizations/task_pages_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Organizations::TaskPagesController < OrganizationsController + include TaskPaginationConcern + before_action :verify_organization_access, only: [:index] before_action :verify_role_access, only: [:index] @@ -24,12 +26,7 @@ class Organizations::TaskPagesController < OrganizationsController # }> def index - render json: { - tasks: json_tasks(task_pager.paged_tasks), - task_page_count: task_pager.task_page_count, - total_task_count: task_pager.total_task_count, - tasks_per_page: TaskPager::TASKS_PER_PAGE - } + render json: pagination_json end private @@ -38,23 +35,7 @@ def organization_url params[:organization_url] end - def task_pager - @task_pager ||= TaskPager.new( - assignee: organization, - tab_name: params[Constants.QUEUE_CONFIG.TAB_NAME_REQUEST_PARAM.to_sym], - page: params[Constants.QUEUE_CONFIG.PAGE_NUMBER_REQUEST_PARAM.to_sym], - sort_order: params[Constants.QUEUE_CONFIG.SORT_DIRECTION_REQUEST_PARAM.to_sym], - sort_by: params[Constants.QUEUE_CONFIG.SORT_COLUMN_REQUEST_PARAM.to_sym], - filters: params[Constants.QUEUE_CONFIG.FILTER_COLUMN_REQUEST_PARAM.to_sym] - ) - end - - def json_tasks(tasks) - tasks = AppealRepository.eager_load_legacy_appeals_for_tasks(tasks) - params = { user: current_user } - - AmaAndLegacyTaskSerializer.new( - tasks: tasks, params: params, ama_serializer: organization.ama_task_serializer - ).call + def assignee + organization end end diff --git a/app/controllers/users/task_pages_controller.rb b/app/controllers/users/task_pages_controller.rb new file mode 100644 index 00000000000..a07ee5b0175 --- /dev/null +++ b/app/controllers/users/task_pages_controller.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class Users::TaskPagesController < UsersController + include TaskPaginationConcern + + # /users/{user.id}/task_pages? + # tab=on_hold& + # sort_by=case_details_link& + # order=desc& + # filter[]=col%3Ddocket_type%26val%3Dlegacy& + # filter[]=col%3Dtask_action%26val%3Dtranslation& + # page=3 + # + # params = "on_hold", + # "sort_by"=>"case_details_link", + # "order"=>"desc", + # "filter"=>[ + # "col=docketNumberColumn&val=legacy,evidence_submission", + # "col=taskColumn&val=Unaccredited rep,Extension" + # ], + # "page"=>"3" + # }> + + def index + render json: pagination_json + end + + private + + def assignee + user + end +end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index cc553f26830..41dce1c4794 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -27,7 +27,7 @@ def update end def represented_organizations - render json: { represented_organizations: User.find(params[:id]).vsos_user_represents } + render json: { represented_organizations: User.find(id).vsos_user_represents } end def judge @@ -77,8 +77,12 @@ def filter_by_css_id_or_name render json: { users: json_users(users) } end + def id + @id ||= params[:id] || params[:user_id] + end + def user - unless css_id || params[:id] + unless css_id || id fail( Caseflow::Error::InvalidParameter, parameter: "css_id or id", @@ -86,7 +90,7 @@ def user ) end - @user ||= params[:id] ? User.find(params[:id]) : User.find_by_css_id(css_id) + @user ||= id ? User.find(id) : User.find_by_css_id(css_id) end def user_to_modify diff --git a/app/models/organization.rb b/app/models/organization.rb index e60486b44e6..419a7eb9b13 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -141,10 +141,6 @@ def completed_tasks_tab ::OrganizationCompletedTasksTab.new(assignee: self, show_regional_office_column: show_regional_office_in_queue?) end - def ama_task_serializer - WorkQueue::TaskSerializer - end - private def clean_url diff --git a/app/models/queue_config.rb b/app/models/queue_config.rb index 652564ab827..f442d5cde08 100644 --- a/app/models/queue_config.rb +++ b/app/models/queue_config.rb @@ -48,7 +48,7 @@ def attach_tasks_to_tab(tab) # This allows us to only instantiate TaskPager if we are using the task pages API. task_page_count: task_pager.task_page_count, total_task_count: tab.tasks.count, - task_page_endpoint_base_path: "#{assignee_is_org? ? "#{assignee.path}/" : ''}#{endpoint}" + task_page_endpoint_base_path: "#{assignee_is_org? ? "#{assignee.path}/" : "users/#{assignee.id}/"}#{endpoint}" ) end diff --git a/config/routes.rb b/config/routes.rb index b6e289f1fdc..6f4c002a26d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -218,10 +218,11 @@ patch "/messages/:id", to: "inbox#update" end - resources :users, only: [:index, :update] - resources :users, only: [:index] do + resources :users, only: [:index, :update] do + resources :task_pages, only: [:index], controller: 'users/task_pages' get 'represented_organizations', on: :member end + get 'user', to: 'users#search' get 'user_info/represented_organizations' diff --git a/spec/controllers/organizations/task_pages_controller_spec.rb b/spec/controllers/organizations/task_pages_controller_spec.rb index 1621d693043..756686dbd97 100644 --- a/spec/controllers/organizations/task_pages_controller_spec.rb +++ b/spec/controllers/organizations/task_pages_controller_spec.rb @@ -1,62 +1,22 @@ # frozen_string_literal: true describe Organizations::TaskPagesController, :postgres, type: :controller do - let(:organization) { create(:organization) } - let(:url) { organization.url } + require_relative "../paged_tasks_shared_examples" + + let(:assignee) { create(:organization) } + let(:url) { assignee.url } + let(:params) { { organization_url: url, tab: tab_name } } let(:user) { create(:user) } before do - organization.add_user(user) + assignee.add_user(user) User.authenticate!(user: user) end describe "GET organization/:organization_url/task_pages" do context "when user is member of the organization and the organization has tasks" do - let(:tab_name) { Constants.QUEUE_CONFIG.UNASSIGNED_TASKS_TAB_NAME } - let(:task_count) { 4 } - - before { create_list(:ama_task, task_count, assigned_to: organization) } - - subject do - get(:index, params: { organization_url: url, tab: tab_name }) - expect(response.status).to eq(200) - JSON.parse(response.body) - end - - it "returns correct elements of the response" do - expect(subject.keys).to match_array(%w[tasks task_page_count total_task_count tasks_per_page]) - end - - it "returns correct number of tasks" do - expect(subject["tasks"]["data"].size).to eq(task_count) - end - - it "returns correct task_page_count" do - expect(subject["task_page_count"]).to eq((task_count.to_f / TaskPager::TASKS_PER_PAGE).ceil) - end - - it "returns correct total_task_count" do - expect(subject["total_task_count"]).to eq(task_count) - end - - it "returns correct tasks_per_page" do - expect(subject["tasks_per_page"]).to eq(TaskPager::TASKS_PER_PAGE) - end - - it "only instantiates TaskPager a single time" do - task_pager = instance_double(TaskPager) - expect(TaskPager).to receive(:new).and_return(task_pager).exactly(1).times - - expect(task_pager).to receive(:paged_tasks) - expect(task_pager).to receive(:task_page_count) - expect(task_pager).to receive(:total_task_count) - - # Prevent this call from actually firing since it will fail due to the instance_double. - allow_any_instance_of(::Organizations::TaskPagesController).to receive(:json_tasks).and_return([]) - - subject - end + it_behaves_like "paged assignee tasks" end end end diff --git a/spec/controllers/paged_tasks_shared_examples.rb b/spec/controllers/paged_tasks_shared_examples.rb new file mode 100644 index 00000000000..4577eb3797a --- /dev/null +++ b/spec/controllers/paged_tasks_shared_examples.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +# Shared examples for user and organization task pages controllers + +shared_examples "paged assignee tasks" do + let(:tab_name) { assignee.class.default_active_tab } + let(:task_count) { 4 } + + before { create_list(:ama_task, task_count, assigned_to: assignee) } + + subject do + get(:index, params: params) + expect(response.status).to eq(200) + JSON.parse(response.body) + end + + it "returns correct elements of the response" do + expect(subject.keys).to match_array(%w[tasks task_page_count total_task_count tasks_per_page]) + end + + it "returns correct number of tasks" do + expect(subject["tasks"]["data"].size).to eq(task_count) + end + + it "returns correct task_page_count" do + expect(subject["task_page_count"]).to eq((task_count.to_f / TaskPager::TASKS_PER_PAGE).ceil) + end + + it "returns correct total_task_count" do + expect(subject["total_task_count"]).to eq(task_count) + end + + it "returns correct tasks_per_page" do + expect(subject["tasks_per_page"]).to eq(TaskPager::TASKS_PER_PAGE) + end + + it "only instantiates TaskPager a single time" do + task_pager = instance_double(TaskPager) + expect(TaskPager).to receive(:new).and_return(task_pager).exactly(1).times + + expect(task_pager).to receive(:paged_tasks) + expect(task_pager).to receive(:task_page_count) + expect(task_pager).to receive(:total_task_count) + + # Prevent this call from actually firing since it will fail due to the instance_double. + allow_any_instance_of(TaskPaginationConcern).to receive(:json_tasks).and_return([]) + + subject + end +end diff --git a/spec/controllers/users/task_pages_controller_spec.rb b/spec/controllers/users/task_pages_controller_spec.rb new file mode 100644 index 00000000000..43bae613adc --- /dev/null +++ b/spec/controllers/users/task_pages_controller_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +describe Users::TaskPagesController, :postgres, type: :controller do + require_relative "../paged_tasks_shared_examples" + + let(:assignee) { create(:user) } + let(:url) { assignee.id } + let(:params) { { user_id: url, tab: tab_name } } + + before do + User.authenticate!(user: assignee) + end + + describe "GET user/:user_id/task_pages" do + it_behaves_like "paged assignee tasks" + end +end From 320a08cb89006ebcd9d73c45220b2999c73c148c Mon Sep 17 00:00:00 2001 From: hschallhorn Date: Tue, 12 May 2020 16:48:53 -0400 Subject: [PATCH 2/6] Fix tests --- app/models/user.rb | 2 +- spec/models/queue_config_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index b86727af63f..a2744321512 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -337,7 +337,7 @@ def queue_tabs end def self.default_active_tab - Constants.QUEUE_CONFIG.ASSIGNED_TASKS_TAB_NAME + Constants.QUEUE_CONFIG.INDIVIDUALLY_ASSIGNED_TASKS_TAB_NAME end def assigned_tasks_tab diff --git a/spec/models/queue_config_spec.rb b/spec/models/queue_config_spec.rb index 3f7b86b70e3..1d0166f38be 100644 --- a/spec/models/queue_config_spec.rb +++ b/spec/models/queue_config_spec.rb @@ -76,7 +76,7 @@ let(:assignee) { user } it "is the assigned tab" do - expect(subject[:active_tab]).to eq(Constants.QUEUE_CONFIG.ASSIGNED_TASKS_TAB_NAME) + expect(subject[:active_tab]).to eq(Constants.QUEUE_CONFIG.INDIVIDUALLY_ASSIGNED_TASKS_TAB_NAME) end end end From f953efba34acdb3c36b3a08083a2ede2afbf3482 Mon Sep 17 00:00:00 2001 From: hschallhorn Date: Tue, 12 May 2020 17:05:13 -0400 Subject: [PATCH 3/6] Add tests! --- spec/models/task_pager_spec.rb | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/spec/models/task_pager_spec.rb b/spec/models/task_pager_spec.rb index d934d8d0e74..2da65d29377 100644 --- a/spec/models/task_pager_spec.rb +++ b/spec/models/task_pager_spec.rb @@ -111,6 +111,15 @@ it "returns a single task" do expect(subject.count).to eq(1) end + + context "when the assignee is a user" do + let(:assignee) { create(:user) } + let(:tab_name) { Constants.QUEUE_CONFIG.INDIVIDUALLY_ASSIGNED_TASKS_TAB_NAME } + + it "returns a single task" do + expect(subject.count).to eq(1) + end + end end end @@ -185,6 +194,16 @@ expected_order = created_tasks.sort_by(&:closed_at).reverse expect(subject.map(&:id)).to eq(expected_order.map(&:id)) end + + context "when the assignee is a user" do + let(:assignee) { create(:user) } + let(:tab_name) { Constants.QUEUE_CONFIG.INDIVIDUALLY_COMPLETED_TASKS_TAB_NAME } + + it "sorts tasks in reserve by closed_at value" do + expected_order = created_tasks.sort_by(&:closed_at).reverse + expect(subject.map(&:id)).to eq(expected_order.map(&:id)) + end + end end end @@ -436,6 +455,16 @@ expect(subject.map(&:type).uniq).to match_array([TranslationTask.name, FoiaTask.name]) expect(subject.length).to eq(translation_tasks.count + foia_tasks.count) end + + context "when the assignee is a user" do + let(:assignee) { create(:user) } + let(:tab_name) { Constants.QUEUE_CONFIG.INDIVIDUALLY_ASSIGNED_TASKS_TAB_NAME } + + it "returns all translation and FOIA tasks assigned to the current organization", :aggregate_failures do + expect(subject.map(&:type).uniq).to match_array([TranslationTask.name, FoiaTask.name]) + expect(subject.length).to eq(translation_tasks.count + foia_tasks.count) + end + end end end end From 19ed55602cf37abaaacd55fae6626e2ed1af37a2 Mon Sep 17 00:00:00 2001 From: hschallhorn Date: Tue, 12 May 2020 17:13:04 -0400 Subject: [PATCH 4/6] Remove test that doesnt even run --- spec/models/task_pager_spec.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/models/task_pager_spec.rb b/spec/models/task_pager_spec.rb index 2da65d29377..bc05c5a8881 100644 --- a/spec/models/task_pager_spec.rb +++ b/spec/models/task_pager_spec.rb @@ -455,16 +455,6 @@ expect(subject.map(&:type).uniq).to match_array([TranslationTask.name, FoiaTask.name]) expect(subject.length).to eq(translation_tasks.count + foia_tasks.count) end - - context "when the assignee is a user" do - let(:assignee) { create(:user) } - let(:tab_name) { Constants.QUEUE_CONFIG.INDIVIDUALLY_ASSIGNED_TASKS_TAB_NAME } - - it "returns all translation and FOIA tasks assigned to the current organization", :aggregate_failures do - expect(subject.map(&:type).uniq).to match_array([TranslationTask.name, FoiaTask.name]) - expect(subject.length).to eq(translation_tasks.count + foia_tasks.count) - end - end end end end From f9ab9a5bb728edcbcb27d1ec1f36ce7b57c38762 Mon Sep 17 00:00:00 2001 From: hschallhorn Date: Thu, 14 May 2020 12:44:30 -0400 Subject: [PATCH 5/6] Feedback! --- .../organizations/task_pages_controller.rb | 2 ++ app/controllers/users/task_pages_controller.rb | 2 ++ app/controllers/users_controller.rb | 2 ++ spec/controllers/paged_tasks_shared_examples.rb | 11 +++++++++++ spec/models/task_pager_spec.rb | 4 ++-- 5 files changed, 19 insertions(+), 2 deletions(-) diff --git a/app/controllers/organizations/task_pages_controller.rb b/app/controllers/organizations/task_pages_controller.rb index 36a889de277..064615c8f87 100644 --- a/app/controllers/organizations/task_pages_controller.rb +++ b/app/controllers/organizations/task_pages_controller.rb @@ -6,6 +6,7 @@ class Organizations::TaskPagesController < OrganizationsController before_action :verify_organization_access, only: [:index] before_action :verify_role_access, only: [:index] + # This request: # /organizations/{org.url}/task_pages? # tab=on_hold& # sort_by=case_details_link& @@ -14,6 +15,7 @@ class Organizations::TaskPagesController < OrganizationsController # filter[]=col%3Dtask_action%26val%3Dtranslation& # page=3 # + # Will be parsed into these params: # params = "on_hold", # "sort_by"=>"case_details_link", diff --git a/app/controllers/users/task_pages_controller.rb b/app/controllers/users/task_pages_controller.rb index a07ee5b0175..dd2f0d26346 100644 --- a/app/controllers/users/task_pages_controller.rb +++ b/app/controllers/users/task_pages_controller.rb @@ -3,6 +3,7 @@ class Users::TaskPagesController < UsersController include TaskPaginationConcern + # This request: # /users/{user.id}/task_pages? # tab=on_hold& # sort_by=case_details_link& @@ -11,6 +12,7 @@ class Users::TaskPagesController < UsersController # filter[]=col%3Dtask_action%26val%3Dtranslation& # page=3 # + # Will be parsed into these params: # params = "on_hold", # "sort_by"=>"case_details_link", diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 41dce1c4794..88c8087d013 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -77,6 +77,8 @@ def filter_by_css_id_or_name render json: { users: json_users(users) } end + # Depending on the route and the requested resource, the requested user's id could be sent as :id or :user_id + # ex from rake routes: user GET /users/:id or user_task_pages GET /users/:user_id/task_pages def id @id ||= params[:id] || params[:user_id] end diff --git a/spec/controllers/paged_tasks_shared_examples.rb b/spec/controllers/paged_tasks_shared_examples.rb index 4577eb3797a..7aeb4fbe4d5 100644 --- a/spec/controllers/paged_tasks_shared_examples.rb +++ b/spec/controllers/paged_tasks_shared_examples.rb @@ -30,6 +30,17 @@ expect(subject["total_task_count"]).to eq(task_count) end + context "when the number of tasks are greater than the tasks per page" do + let(:number_of_pages) { 2 } + let(:task_count) { TaskPager::TASKS_PER_PAGE * number_of_pages } + + it "only returns the tasks per page" do + expect(subject["tasks"]["data"].size).to eq(TaskPager::TASKS_PER_PAGE) + expect(subject["task_page_count"]).to eq(number_of_pages) + expect(subject["total_task_count"]).to eq(task_count) + end + end + it "returns correct tasks_per_page" do expect(subject["tasks_per_page"]).to eq(TaskPager::TASKS_PER_PAGE) end diff --git a/spec/models/task_pager_spec.rb b/spec/models/task_pager_spec.rb index bc05c5a8881..d8068c5c234 100644 --- a/spec/models/task_pager_spec.rb +++ b/spec/models/task_pager_spec.rb @@ -190,7 +190,7 @@ context "with desc sort_order" do let(:sort_order) { Constants.QUEUE_CONFIG.COLUMN_SORT_ORDER_DESC } - it "sorts tasks in reserve by closed_at value" do + it "sorts tasks in reverse by closed_at value" do expected_order = created_tasks.sort_by(&:closed_at).reverse expect(subject.map(&:id)).to eq(expected_order.map(&:id)) end @@ -199,7 +199,7 @@ let(:assignee) { create(:user) } let(:tab_name) { Constants.QUEUE_CONFIG.INDIVIDUALLY_COMPLETED_TASKS_TAB_NAME } - it "sorts tasks in reserve by closed_at value" do + it "sorts tasks in reverse by closed_at value" do expected_order = created_tasks.sort_by(&:closed_at).reverse expect(subject.map(&:id)).to eq(expected_order.map(&:id)) end From 99928e0ec9ece89f66b0cd669157880d08754076 Mon Sep 17 00:00:00 2001 From: Hunter Schallhorn <45575454+hschallhorn@users.noreply.github.com> Date: Fri, 15 May 2020 10:26:34 -0400 Subject: [PATCH 6/6] Allow user queues to be paginated (#14253) --- .../concerns/collect_data_dog_metrics.rb | 11 +- app/controllers/tasks_controller.rb | 2 +- app/models/task_pager.rb | 2 +- app/models/user.rb | 10 +- client/app/queue/AttorneyTaskListView.jsx | 6 +- client/app/queue/ColocatedTaskListView.jsx | 6 +- client/app/queue/OrganizationQueue.jsx | 116 +----------------- client/app/queue/QueueApp.jsx | 2 +- client/app/queue/QueueTable.jsx | 2 +- client/app/queue/QueueTableBuilder.jsx | 76 +++++++++--- .../karma/queue/ColocatedTaskListView-test.js | 2 +- spec/controllers/tasks_controller_spec.rb | 18 +++ spec/models/queue_config_spec.rb | 3 +- 13 files changed, 104 insertions(+), 152 deletions(-) diff --git a/app/controllers/concerns/collect_data_dog_metrics.rb b/app/controllers/concerns/collect_data_dog_metrics.rb index 7a5109f0657..9bf11d49789 100644 --- a/app/controllers/concerns/collect_data_dog_metrics.rb +++ b/app/controllers/concerns/collect_data_dog_metrics.rb @@ -13,12 +13,11 @@ def collect_data_dog_metrics end def collect_postgres_metrics - # safe-navigating c.owner&.alive? even though c.in_use? is aliased to - # c.owner, because we were occasionally seeing the following error: - # NoMethodError: undefined method 'alive?' for nil:NilClass - active = ActiveRecord::Base.connection_pool.connections.count { |c| c.in_use? && c.owner&.alive? } - dead = ActiveRecord::Base.connection_pool.connections.count { |c| c.in_use? && !c.owner&.alive? } - idle = ActiveRecord::Base.connection_pool.connections.count { |c| !c.in_use? } + conns = ActiveRecord::Base.connection_pool.connections + + active = conns.count { |c| c.in_use? && c.owner.alive? } + dead = conns.count { |c| c.in_use? && !c.owner.alive? } + idle = conns.count { |c| !c.in_use? } emit_datadog_point("postgres", "active", active) emit_datadog_point("postgres", "dead", dead) diff --git a/app/controllers/tasks_controller.rb b/app/controllers/tasks_controller.rb index 8fc2d09e1fd..9266ff4f323 100644 --- a/app/controllers/tasks_controller.rb +++ b/app/controllers/tasks_controller.rb @@ -42,7 +42,7 @@ def set_application # GET /tasks?user_id=xxx&role=attorney # GET /tasks?user_id=xxx&role=judge def index - tasks = QueueForRole.new(user_role).create(user: user).tasks + tasks = user.use_task_pages_api? ? [] : QueueForRole.new(user_role).create(user: user).tasks render json: { tasks: json_tasks(tasks), queue_config: queue_config } end diff --git a/app/models/task_pager.rb b/app/models/task_pager.rb index 6517bdb421a..f06bd15b04f 100644 --- a/app/models/task_pager.rb +++ b/app/models/task_pager.rb @@ -23,7 +23,7 @@ def initialize(args) end def paged_tasks - sorted_tasks(filtered_tasks).page(page).per(TASKS_PER_PAGE) + @paged_tasks ||= sorted_tasks(filtered_tasks).page(page).per(TASKS_PER_PAGE) end def sorted_tasks(tasks) diff --git a/app/models/user.rb b/app/models/user.rb index a2744321512..4c5fa564bce 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -285,6 +285,10 @@ def administered_judge_teams administered_teams.select { |team| team.is_a?(JudgeTeam) } end + def non_administered_judge_teams + organizations_users.non_admin.where(organization: JudgeTeam.all) + end + def user_info_for_idt self.class.user_repository.user_info_for_idt(css_id) end @@ -312,6 +316,10 @@ def judge? !!JudgeTeam.for_judge(self) || judge_in_vacols? end + def attorney? + non_administered_judge_teams.any? || attorney_in_vacols? + end + def update_status!(new_status) transaction do if new_status.eql?(Constants.USER_STATUSES.inactive) @@ -325,7 +333,7 @@ def update_status!(new_status) end def use_task_pages_api? - false + FeatureToggle.enabled?(:user_queue_pagination, user: self) && !attorney? && !judge? end def queue_tabs diff --git a/client/app/queue/AttorneyTaskListView.jsx b/client/app/queue/AttorneyTaskListView.jsx index 350214e89cd..902f415256a 100644 --- a/client/app/queue/AttorneyTaskListView.jsx +++ b/client/app/queue/AttorneyTaskListView.jsx @@ -73,6 +73,7 @@ class AttorneyTaskListView extends React.PureComponent { assignedTasks={this.props.workableTasks} onHoldTasks={this.props.onHoldTasks} completedTasks={this.props.completedTasks} + requireDasRecord /> ; } @@ -86,8 +87,7 @@ const mapStateToProps = (state) => { } }, ui: { - messages, - organizations + messages } } = state; @@ -97,7 +97,6 @@ const mapStateToProps = (state) => { completedTasks: completeTasksByAssigneeCssIdSelector(state), success: messages.success, error: messages.error, - organizations, taskDecision }); }; @@ -124,7 +123,6 @@ AttorneyTaskListView.propTypes = { resetErrorMessages: PropTypes.func, showErrorMessage: PropTypes.func, onHoldTasks: PropTypes.array, - organizations: PropTypes.array, success: PropTypes.shape({ title: PropTypes.string, detail: PropTypes.string diff --git a/client/app/queue/ColocatedTaskListView.jsx b/client/app/queue/ColocatedTaskListView.jsx index 8fca8904cb5..f0a1fa66075 100644 --- a/client/app/queue/ColocatedTaskListView.jsx +++ b/client/app/queue/ColocatedTaskListView.jsx @@ -50,11 +50,9 @@ const mapStateToProps = (state) => { return { success, - organizations: state.ui.organizations, assignedTasks: newTasksByAssigneeCssIdSelector(state), onHoldTasks: onHoldTasksByAssigneeCssIdSelector(state), - completedTasks: completeTasksByAssigneeCssIdSelector(state), - queueConfig: state.queue.queueConfig + completedTasks: completeTasksByAssigneeCssIdSelector(state) }; }; @@ -71,7 +69,5 @@ ColocatedTaskListView.propTypes = { completedTasks: PropTypes.array, hideSuccessMessage: PropTypes.func, onHoldTasks: PropTypes.array, - organizations: PropTypes.array, - queueConfig: PropTypes.object, success: PropTypes.object }; diff --git a/client/app/queue/OrganizationQueue.jsx b/client/app/queue/OrganizationQueue.jsx index b07705723b7..6931b15c0cb 100644 --- a/client/app/queue/OrganizationQueue.jsx +++ b/client/app/queue/OrganizationQueue.jsx @@ -1,24 +1,13 @@ import React from 'react'; -import _ from 'lodash'; import { connect } from 'react-redux'; import { bindActionCreators } from 'redux'; import { css } from 'glamor'; -import { sprintf } from 'sprintf-js'; import PropTypes from 'prop-types'; -import BulkAssignButton from './components/BulkAssignButton'; -import TabWindow from '../components/TabWindow'; -import { docketNumberColumn, badgesColumn, detailsColumn, taskColumn, regionalOfficeColumn, issueCountColumn, - typeColumn, assignedToColumn, daysWaitingColumn, daysOnHoldColumn, readerLinkColumn, completedToNameColumn } from - './components/TaskTableColumns'; - -import QueueTable from './QueueTable'; -import QueueOrganizationDropdown from './components/QueueOrganizationDropdown'; +import QueueTableBuilder from './QueueTableBuilder'; import Alert from '../components/Alert'; import AppSegment from '@department-of-veterans-affairs/caseflow-frontend-toolkit/components/AppSegment'; -import { tasksWithAppealsFromRawTasks } from './utils'; import { clearCaseSelectSearch } from '../reader/CaseSelect/CaseSelectActions'; -import { fullWidth } from './constants'; const containerStyles = css({ position: 'relative' @@ -33,99 +22,8 @@ class OrganizationQueue extends React.PureComponent { this.props.clearCaseSelectSearch(); } - calculateActiveTabIndex = (config) => { - const tabNames = config.tabs.map((tab) => { - return tab.name; - }); - const { paginationOptions = {} } = this.props; - const activeTab = paginationOptions.tab || config.active_tab; - const index = _.indexOf(tabNames, activeTab); - - return index === -1 ? 0 : index; - } - - queueConfig = () => { - const config = this.props.queueConfig; - - config.active_tab_index = this.calculateActiveTabIndex(config); - - return config; - } - - filterValuesForColumn = (column) => column && column.filterable && column.filter_options; - - createColumnObject = (column, config, tasks) => { - const functionForColumn = { - badgesColumn: badgesColumn(tasks), - detailsColumn: detailsColumn(tasks, false, config.userRole), - taskColumn: taskColumn(tasks, this.filterValuesForColumn(column)), - regionalOfficeColumn: regionalOfficeColumn(tasks, this.filterValuesForColumn(column)), - typeColumn: typeColumn(tasks, this.filterValuesForColumn(column), false), - assignedToColumn: assignedToColumn(tasks, this.filterValuesForColumn(column)), - completedToNameColumn: completedToNameColumn(), - docketNumberColumn: docketNumberColumn(tasks, this.filterValuesForColumn(column), false), - daysWaitingColumn: daysWaitingColumn(false), - daysOnHoldColumn: daysOnHoldColumn(false), - readerLinkColumn: readerLinkColumn(false, true), - issueCountColumn: issueCountColumn(false) - }; - - return functionForColumn[column.name]; - } - - columnsFromConfig = (config, tabConfig, tasks) => - tabConfig.columns.map((column) => this.createColumnObject(column, config, tasks)); - - taskTableTabFactory = (tabConfig, config) => { - const { paginationOptions = {} } = this.props; - const tasks = tasksWithAppealsFromRawTasks(tabConfig.tasks); - const cols = this.columnsFromConfig(config, tabConfig, tasks); - const totalTaskCount = tabConfig.total_task_count; - - return { - label: sprintf(tabConfig.label, totalTaskCount), - page: -

{tabConfig.description}

- { tabConfig.allow_bulk_assign && } - task.uniqueId} - casesPerPage={config.tasks_per_page} - numberOfPages={tabConfig.task_page_count} - totalTaskCount={totalTaskCount} - taskPagesApiEndpoint={tabConfig.task_page_endpoint_base_path} - tabPaginationOptions={paginationOptions.tab === tabConfig.name && paginationOptions} - useTaskPagesApi - enablePagination - /> - - }; - } - - tabsFromConfig = (config) => { - return config.tabs.map((tabConfig) => { - return this.taskTableTabFactory(tabConfig, config); - }); - } - - makeQueueComponents = (config) => { - return
-

{config.table_title}

- - - -
; - } - render = () => { const { success, tasksAssignedByBulk } = this.props; - const body = this.makeQueueComponents(this.queueConfig()); return {success && } @@ -139,7 +37,7 @@ class OrganizationQueue extends React.PureComponent { type="success" styling={alertStyling} /> } - {body} + ; }; } @@ -148,10 +46,8 @@ OrganizationQueue.propTypes = { clearCaseSelectSearch: PropTypes.func, onHoldTasks: PropTypes.array, organizations: PropTypes.array, - queueConfig: PropTypes.object, success: PropTypes.object, - tasksAssignedByBulk: PropTypes.object, - paginationOptions: PropTypes.object + tasksAssignedByBulk: PropTypes.object }; const mapStateToProps = (state) => { @@ -159,12 +55,8 @@ const mapStateToProps = (state) => { return { success, - userRole: state.ui.userRole, - organizationName: state.ui.activeOrganization.name, - organizationIsVso: state.ui.activeOrganization.isVso, organizations: state.ui.organizations, - tasksAssignedByBulk: state.queue.tasksAssignedByBulk, - queueConfig: state.queue.queueConfig + tasksAssignedByBulk: state.queue.tasksAssignedByBulk }; }; diff --git a/client/app/queue/QueueApp.jsx b/client/app/queue/QueueApp.jsx index 8873052a6bc..bbe37bd35f0 100644 --- a/client/app/queue/QueueApp.jsx +++ b/client/app/queue/QueueApp.jsx @@ -280,7 +280,7 @@ class QueueApp extends React.PureComponent { return ( - + ); }; diff --git a/client/app/queue/QueueTable.jsx b/client/app/queue/QueueTable.jsx index 7c95b88cb9d..a7c6f5eabe8 100644 --- a/client/app/queue/QueueTable.jsx +++ b/client/app/queue/QueueTable.jsx @@ -660,7 +660,7 @@ HeaderRow.propTypes = FooterRow.propTypes = Row.propTypes = BodyRows.propTypes = useTaskPagesApi: PropTypes.bool, userReadableColumnNames: PropTypes.object, tabPaginationOptions: PropTypes.shape({ - [QUEUE_CONFIG.PAGE_NUMBER_REQUEST_PARAM]: PropTypes.number, + [QUEUE_CONFIG.PAGE_NUMBER_REQUEST_PARAM]: PropTypes.string, [QUEUE_CONFIG.SORT_DIRECTION_REQUEST_PARAM]: PropTypes.string, [QUEUE_CONFIG.SORT_COLUMN_REQUEST_PARAM]: PropTypes.string, [`${QUEUE_CONFIG.FILTER_COLUMN_REQUEST_PARAM}[]`]: PropTypes.arrayOf(PropTypes.string), diff --git a/client/app/queue/QueueTableBuilder.jsx b/client/app/queue/QueueTableBuilder.jsx index f52d326aded..9e193d1ddd4 100644 --- a/client/app/queue/QueueTableBuilder.jsx +++ b/client/app/queue/QueueTableBuilder.jsx @@ -1,22 +1,25 @@ import React from 'react'; +import _ from 'lodash'; import PropTypes from 'prop-types'; import { sprintf } from 'sprintf-js'; import { connect } from 'react-redux'; +import querystring from 'querystring'; +import BulkAssignButton from './components/BulkAssignButton'; import QueueTable from './QueueTable'; import TabWindow from '../components/TabWindow'; import QueueOrganizationDropdown from './components/QueueOrganizationDropdown'; -import { completedToNameColumn, daysOnHoldColumn, daysWaitingColumn, detailsColumn, docketNumberColumn, - badgesColumn, issueCountColumn, readerLinkColumn, readerLinkColumnWithNewDocsIcon, regionalOfficeColumn, - taskColumn, taskCompletedDateColumn, typeColumn } from './components/TaskTableColumns'; +import { assignedToColumn, completedToNameColumn, daysOnHoldColumn, daysWaitingColumn, detailsColumn, + docketNumberColumn, badgesColumn, issueCountColumn, readerLinkColumn, readerLinkColumnWithNewDocsIcon, + regionalOfficeColumn, taskColumn, taskCompletedDateColumn, typeColumn } from './components/TaskTableColumns'; +import { tasksWithAppealsFromRawTasks } from './utils'; import QUEUE_CONFIG from '../../constants/QUEUE_CONFIG'; -import USER_ROLE_TYPES from '../../constants/USER_ROLE_TYPES'; import { fullWidth } from './constants'; /** - * A component to create a queue table's tabs and columns from a queue config and the user's tasks - * The required props are: + * A component to create a queue table's tabs and columns from a queue config or the assignee's tasks + * The props are: * - @assignedTasks {array[object]} array of task objects to appear in the assigned tab * - @onHoldTasks {array[object]} array of task objects to appear in the on hold tab * - @completedTasks {array[object]} array of task objects to appear in the completed tab @@ -24,6 +27,26 @@ import { fullWidth } from './constants'; class QueueTableBuilder extends React.PureComponent { + paginationOptions = () => querystring.parse(window.location.search.slice(1)); + + calculateActiveTabIndex = (config) => { + const tabNames = config.tabs.map((tab) => { + return tab.name; + }); + const activeTab = this.paginationOptions().tab || config.active_tab; + const index = _.indexOf(tabNames, activeTab); + + return index === -1 ? 0 : index; + } + + queueConfig = () => { + const { config } = this.props; + + config.active_tab_index = this.calculateActiveTabIndex(config); + + return config; + } + tasksForTab = (tabName) => { const mapper = { [QUEUE_CONFIG.INDIVIDUALLY_ASSIGNED_TASKS_TAB_NAME]: this.props.assignedTasks, @@ -34,22 +57,26 @@ class QueueTableBuilder extends React.PureComponent { return mapper[tabName]; } + filterValuesForColumn = (column) => column && column.filterable && column.filter_options; + createColumnObject = (column, config, tasks) => { - const requireDasRecord = config.userRole === USER_ROLE_TYPES.attorney; + const { requireDasRecord } = this.props; + const filterOptions = this.filterValuesForColumn(column); const functionForColumn = { - [QUEUE_CONFIG.COLUMNS.APPEAL_TYPE.name]: typeColumn(tasks, false, requireDasRecord), + [QUEUE_CONFIG.COLUMNS.APPEAL_TYPE.name]: typeColumn(tasks, filterOptions, requireDasRecord), + [QUEUE_CONFIG.COLUMNS.BADGES.name]: badgesColumn(tasks), [QUEUE_CONFIG.COLUMNS.CASE_DETAILS_LINK.name]: detailsColumn(tasks, requireDasRecord, config.userRole), [QUEUE_CONFIG.COLUMNS.DAYS_ON_HOLD.name]: daysOnHoldColumn(requireDasRecord), [QUEUE_CONFIG.COLUMNS.DAYS_WAITING.name]: daysWaitingColumn(requireDasRecord), - [QUEUE_CONFIG.COLUMNS.DOCKET_NUMBER.name]: docketNumberColumn(tasks, false, requireDasRecord), + [QUEUE_CONFIG.COLUMNS.DOCKET_NUMBER.name]: docketNumberColumn(tasks, filterOptions, requireDasRecord), [QUEUE_CONFIG.COLUMNS.DOCUMENT_COUNT_READER_LINK.name]: readerLinkColumn(requireDasRecord, true), - [QUEUE_CONFIG.COLUMNS.BADGES.name]: badgesColumn(tasks), [QUEUE_CONFIG.COLUMNS.ISSUE_COUNT.name]: issueCountColumn(requireDasRecord), [QUEUE_CONFIG.COLUMNS.READER_LINK_WITH_NEW_DOCS_ICON.name]: readerLinkColumnWithNewDocsIcon(requireDasRecord), - [QUEUE_CONFIG.COLUMNS.REGIONAL_OFFICE.name]: regionalOfficeColumn(tasks, false), + [QUEUE_CONFIG.COLUMNS.REGIONAL_OFFICE.name]: regionalOfficeColumn(tasks, filterOptions), + [QUEUE_CONFIG.COLUMNS.TASK_ASSIGNEE.name]: assignedToColumn(tasks, filterOptions), [QUEUE_CONFIG.COLUMNS.TASK_ASSIGNER.name]: completedToNameColumn(), [QUEUE_CONFIG.COLUMNS.TASK_CLOSED_DATE.name]: taskCompletedDateColumn(), - [QUEUE_CONFIG.COLUMNS.TASK_TYPE.name]: taskColumn(tasks, false) + [QUEUE_CONFIG.COLUMNS.TASK_TYPE.name]: taskColumn(tasks, filterOptions) }; return functionForColumn[column.name]; @@ -59,17 +86,28 @@ class QueueTableBuilder extends React.PureComponent { (tabConfig.columns || []).map((column) => this.createColumnObject(column, config, tasks)); taskTableTabFactory = (tabConfig, config) => { - const tasks = this.tasksForTab(tabConfig.name); + const paginationOptions = this.paginationOptions(); + const tasks = config.use_task_pages_api ? + tasksWithAppealsFromRawTasks(tabConfig.tasks) : + this.tasksForTab(tabConfig.name); + const totalTaskCount = config.use_task_pages_api ? tabConfig.total_task_count : tasks.length; return { - label: sprintf(tabConfig.label, tasks.length), + label: sprintf(tabConfig.label, totalTaskCount), page:

{tabConfig.description}

+ { tabConfig.allow_bulk_assign && } task.uniqueId} + casesPerPage={config.tasks_per_page} + numberOfPages={tabConfig.task_page_count} + totalTaskCount={totalTaskCount} + taskPagesApiEndpoint={tabConfig.task_page_endpoint_base_path} + tabPaginationOptions={paginationOptions.tab === tabConfig.name && paginationOptions} + useTaskPagesApi={config.use_task_pages_api} enablePagination /> @@ -79,12 +117,12 @@ class QueueTableBuilder extends React.PureComponent { tabsFromConfig = (config) => (config.tabs || []).map((tabConfig) => this.taskTableTabFactory(tabConfig, config)); render = () => { - const { config } = this.props; + const config = this.queueConfig(); return

{config.table_title}

- + ; }; } @@ -102,8 +140,10 @@ QueueTableBuilder.propTypes = { onHoldTasks: PropTypes.array, completedTasks: PropTypes.array, config: PropTypes.shape({ - table_title: PropTypes.string - }) + table_title: PropTypes.string, + active_tab_index: PropTypes.number + }), + requireDasRecord: PropTypes.bool }; export default (connect(mapStateToProps)(QueueTableBuilder)); diff --git a/client/test/karma/queue/ColocatedTaskListView-test.js b/client/test/karma/queue/ColocatedTaskListView-test.js index d842cb2b512..2a7ca34d0e3 100644 --- a/client/test/karma/queue/ColocatedTaskListView-test.js +++ b/client/test/karma/queue/ColocatedTaskListView-test.js @@ -317,7 +317,7 @@ describe('ColocatedTaskListView', () => { } ], "tasks_per_page": 15, - "use_task_pages_api": true + "use_task_pages_api": false }; /* eslint-disable no-unused-expressions */ diff --git a/spec/controllers/tasks_controller_spec.rb b/spec/controllers/tasks_controller_spec.rb index 2ed0c6780e5..9229f2b0487 100644 --- a/spec/controllers/tasks_controller_spec.rb +++ b/spec/controllers/tasks_controller_spec.rb @@ -197,6 +197,24 @@ expect(data.size).to be(1) end + + context "when using task pages api" do + before do + expect(QueueForRole).not_to receive(:new) + allow_any_instance_of(User).to receive(:use_task_pages_api?).and_return(true) + end + + it "gets tasks from task pager, not queue for role" do + get :index, params: { user_id: user.id, role: "unknown" } + expect(response.status).to eq 200 + + queue_for_role_tasks = JSON.parse(response.body)["tasks"]["data"] + expect(queue_for_role_tasks.size).to be(0) + + paged_tasks = JSON.parse(response.body)["queue_config"]["tabs"].first["tasks"] + expect(paged_tasks.size).to be(1) + end + end end context "when a task is assignable" do diff --git a/spec/models/queue_config_spec.rb b/spec/models/queue_config_spec.rb index 1d0166f38be..c830da34c91 100644 --- a/spec/models/queue_config_spec.rb +++ b/spec/models/queue_config_spec.rb @@ -232,7 +232,8 @@ let!(:on_hold_tasks) { create_list(:ama_task, 5, :on_hold, assigned_to: assignee) } let!(:completed_tasks) { create_list(:ama_task, 7, :completed, assigned_to: assignee) } - before { allow(assignee).to receive(:use_task_pages_api?).and_return(true) } + before { FeatureToggle.enable!(:user_queue_pagination, users: [assignee.css_id]) } + after { FeatureToggle.disable!(:user_queue_pagination, users: [assignee.css_id]) } it "returns the tasks in the correct tabs" do tabs = subject