Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pagination to user queues #14242

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fe83559
Add assigned colocated tasks to user on hold tabs
hschallhorn May 11, 2020
dcdcb73
Use pagination feature toggle
hschallhorn May 11, 2020
b0bf1be
Use paged tasks in task controller
hschallhorn May 11, 2020
08fcd51
Create a task pager for users
hschallhorn May 11, 2020
0d964de
Only request paged tasks once
hschallhorn May 11, 2020
704aadc
Enable pagination on the front end for users
hschallhorn May 11, 2020
5cf38a1
combine task pages controllers
hschallhorn May 11, 2020
93e1f6a
Merge branch 'master' into hschallhorn/14142-user-queue-pagination
hschallhorn May 11, 2020
78bbd55
Code climate part 1!
hschallhorn May 11, 2020
aab8fea
Merge queue table builder and organization queue
hschallhorn May 11, 2020
6eb17a9
Merge branch 'master' into hschallhorn/14142-user-queue-pagination
hschallhorn May 11, 2020
9bb778d
Move pagination option parsing into queue table builder
hschallhorn May 11, 2020
9d04534
Fix js test
hschallhorn May 12, 2020
aa99bda
Fix tests
hschallhorn May 12, 2020
82590b1
Merge branch 'master' into hschallhorn/14142-user-queue-pagination
hschallhorn May 12, 2020
9aea0a9
List and test fix
hschallhorn May 12, 2020
b46a11e
Merge branch 'hschallhorn/14142-user-queue-pagination' of https://git…
hschallhorn May 12, 2020
99a85ae
Do not paginate attorney or judge queues
hschallhorn May 12, 2020
d794622
Remove tests
hschallhorn May 12, 2020
8d7d82e
Merge branch 'master' into hschallhorn/14142-user-queue-pagination
hschallhorn May 12, 2020
e0fbe61
Test fix
hschallhorn May 12, 2020
0ad1143
Merge branch 'master' into hschallhorn/14142-user-queue-pagination
hschallhorn May 12, 2020
56e9531
Remove unused
hschallhorn May 12, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/tasks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

@hschallhorn hschallhorn May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only improves the initial load time of a queue. Will file followup tickets to improve load times after creating or updating tasks. These tickets would remove QueueForRole here

def create
return invalid_type_error unless task_classes_valid?
tasks = []
param_groups = create_params.group_by { |param| param[:type] }
param_groups.each do |task_type, param_group|
tasks << valid_task_classes[task_type.to_sym].create_many_from_params(param_group, current_user)
end
modified_tasks = [parent_tasks_from_params, tasks].flatten!
tasks_to_return = (QueueForRole.new(user_role).create(user: current_user).tasks + modified_tasks).uniq
render json: { tasks: json_tasks(tasks_to_return) }
and here
def update
tasks = task.update_from_params(update_params, current_user)
tasks.each { |t| return invalid_record_error(t) unless t.valid? }
tasks_to_return = (QueueForRole.new(user_role).create(user: current_user).tasks + tasks).uniq
render json: { tasks: json_tasks(tasks_to_return) }
end

and should exist outside of pagination

render json: { tasks: json_tasks(tasks), queue_config: queue_config }
end

Expand Down
53 changes: 53 additions & 0 deletions app/controllers/users/task_pages_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# frozen_string_literal: true

class Users::TaskPagesController < UsersController
# /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 = <ActionController::Parameters {
# "tab"=>"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: {
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: user,
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New controller for requesting paged tasks assigned to a user

1 change: 1 addition & 0 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def filter_by_css_id_or_name
end

def user
params[:id] = params[:user_id] if params[:user_id] && !params[:id]
unless css_id || params[:id]
fail(
Caseflow::Error::InvalidParameter,
Expand Down
2 changes: 1 addition & 1 deletion app/models/queue_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the endpoint the front end uses to request paged tasks

)
end

Expand Down
10 changes: 10 additions & 0 deletions app/models/queue_tabs/on_hold_tasks_tab.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ def description

def tasks
Task.includes(*task_includes).visible_in_queue_table_view.on_hold.where(assigned_to: assignee)
.or(legacy_colocated_tasks)
end

def legacy_colocated_tasks
Task.includes(*task_includes).open.where(
assigned_by: assignee,
assigned_to_type: Organization.name,
appeal_type: LegacyAppeal.name,
type: ColocatedTask.subclasses.map(&:name)
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copies logic from AttorneyQueue. Only issue is that it will return multiple colocated tasks for one appeal. Debating on how to fix this. Currently affects 42 cases out of ~2500

legacy_colocated_tasks = ColocatedTask.open.where(appeal_type: LegacyAppeal.name, assigned_to: Colocated.singleton)
legacy_colocated_tasks.pluck(:appeal_id).uniq.count
=> 2593
legacy_colocated_tasks.group(:appeal_id).count.values.select { |count| count > 1 }.count
=> 42

end

# rubocop:disable Metrics/AbcSize
Expand Down
2 changes: 1 addition & 1 deletion app/models/task_pager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were calling this again every time we called paged tasks

end

def sorted_tasks(tasks)
Expand Down
4 changes: 2 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ def update_status!(new_status)
end

def use_task_pages_api?
false
FeatureToggle.enabled?(:user_queue_pagination)
end

def queue_tabs
Expand All @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error from previous PRs

end

def assigned_tasks_tab
Expand Down
7 changes: 3 additions & 4 deletions client/app/queue/AttorneyTaskListView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class AttorneyTaskListView extends React.PureComponent {
assignedTasks={this.props.workableTasks}
onHoldTasks={this.props.onHoldTasks}
completedTasks={this.props.completedTasks}
paginationOptions={this.props.paginationOptions}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass pagination options though to queue table builder

/>
</AppSegment>;
}
Expand All @@ -86,8 +87,7 @@ const mapStateToProps = (state) => {
}
},
ui: {
messages,
organizations
messages
}
} = state;

Expand All @@ -97,7 +97,6 @@ const mapStateToProps = (state) => {
completedTasks: completeTasksByAssigneeCssIdSelector(state),
success: messages.success,
error: messages.error,
organizations,
taskDecision
});
};
Expand All @@ -124,7 +123,7 @@ AttorneyTaskListView.propTypes = {
resetErrorMessages: PropTypes.func,
showErrorMessage: PropTypes.func,
onHoldTasks: PropTypes.array,
organizations: PropTypes.array,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orgs was unused

paginationOptions: PropTypes.object,
success: PropTypes.shape({
title: PropTypes.string,
detail: PropTypes.string
Expand Down
8 changes: 3 additions & 5 deletions client/app/queue/ColocatedTaskListView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class ColocatedTaskListView extends React.PureComponent {
assignedTasks={this.props.assignedTasks}
onHoldTasks={this.props.onHoldTasks}
completedTasks={this.props.completedTasks}
paginationOptions={this.props.paginationOptions}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass pagination options though to queue table builder

/>
</AppSegment>;
};
Expand All @@ -50,11 +51,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)
};
};

Expand All @@ -71,7 +70,6 @@ ColocatedTaskListView.propTypes = {
completedTasks: PropTypes.array,
hideSuccessMessage: PropTypes.func,
onHoldTasks: PropTypes.array,
organizations: PropTypes.array,
queueConfig: PropTypes.object,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orgs and queue config were unused

paginationOptions: PropTypes.object,
success: PropTypes.object
};
4 changes: 2 additions & 2 deletions client/app/queue/QueueApp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ class QueueApp extends React.PureComponent {
const { userRole } = this.props;

if (userRole === USER_ROLE_TYPES.attorney) {
return <AttorneyTaskListView />;
return <AttorneyTaskListView paginationOptions={querystring.parse(window.location.search.slice(1))} />;
} else if (userRole === USER_ROLE_TYPES.judge) {
return <JudgeDecisionReviewTaskListView {...this.props} />;
}

return <ColocatedTaskListView />;
return <ColocatedTaskListView paginationOptions={querystring.parse(window.location.search.slice(1))} />;
};

routedQueueList = () => (
Expand Down
55 changes: 45 additions & 10 deletions client/app/queue/QueueTableBuilder.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import _ from 'lodash';
import PropTypes from 'prop-types';
import { sprintf } from 'sprintf-js';
import { connect } from 'react-redux';
Expand All @@ -9,6 +10,7 @@ import QueueOrganizationDropdown from './components/QueueOrganizationDropdown';
import { 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';
Expand All @@ -24,6 +26,25 @@ import { fullWidth } from './constants';

class QueueTableBuilder extends React.PureComponent {

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;

config.active_tab_index = this.calculateActiveTabIndex(config);

return config;
}

tasksForTab = (tabName) => {
const mapper = {
[QUEUE_CONFIG.INDIVIDUALLY_ASSIGNED_TASKS_TAB_NAME]: this.props.assignedTasks,
Expand All @@ -34,22 +55,25 @@ 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 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.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_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];
Expand All @@ -59,17 +83,26 @@ 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.props;
const tasks = config.use_task_pages_api ?
tasksWithAppealsFromRawTasks(tabConfig.tasks) :
this.tasksForTab(tabConfig.name);

return {
label: sprintf(tabConfig.label, tasks.length),
label: sprintf(tabConfig.label, tabConfig.total_task_count),
page: <React.Fragment>
<p className="cf-margin-top-0">{tabConfig.description}</p>
<QueueTable
key={tabConfig.name}
columns={this.columnsFromConfig(config, tabConfig, tasks)}
rowObjects={tasks}
getKeyForRow={(_rowNumber, task) => task.uniqueId}
casesPerPage={config.tasks_per_page}
numberOfPages={tabConfig.task_page_count}
totalTaskCount={tabConfig.total_task_count}
useTaskPagesApi={config.use_task_pages_api}
taskPagesApiEndpoint={tabConfig.task_page_endpoint_base_path}
tabPaginationOptions={paginationOptions.tab === tabConfig.name && paginationOptions}
enablePagination
/>
</React.Fragment>
Expand All @@ -79,12 +112,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 <React.Fragment>
<h1 {...fullWidth}>{config.table_title}</h1>
<QueueOrganizationDropdown organizations={this.props.organizations} />
<TabWindow name="tasks-tabwindow" tabs={this.tabsFromConfig(config)} />
<TabWindow name="tasks-tabwindow" tabs={this.tabsFromConfig(config)} defaultPage={config.active_tab_index} />
</React.Fragment>;
};
}
Expand All @@ -102,8 +135,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
}),
paginationOptions: PropTypes.object
};

export default (connect(mapStateToProps)(QueueTableBuilder));
5 changes: 3 additions & 2 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,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'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Route for user tasks pager

get 'represented_organizations', on: :member
end

get 'user', to: 'users#search'
get 'user_info/represented_organizations'

Expand Down
19 changes: 19 additions & 0 deletions spec/controllers/tasks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,25 @@

expect(data.size).to be(1)
end

context "when using task pages api" do
before do
expect(QueueForRole).not_to receive(:new)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure we are not pulling tasks from QueueForRole

FeatureToggle.enable!(:user_queue_pagination)
end
after { FeatureToggle.disable!(:user_queue_pagination) }

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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensure tasks are not being sent to the front end outside of queue config


paged_tasks = JSON.parse(response.body)["queue_config"]["tabs"].first["tasks"]
expect(paged_tasks.size).to be(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensure tasks are being sent though queue config

end
end
end

context "when a task is assignable" do
Expand Down
Loading