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 8 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
41 changes: 41 additions & 0 deletions app/controllers/concerns/task_pagination_concern.rb
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pulled all this out of organization task pages controller to be reused in user task pages controller.

29 changes: 5 additions & 24 deletions app/controllers/organizations/task_pages_controller.rb
Original file line number Diff line number Diff line change
@@ -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]

Expand All @@ -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
Expand All @@ -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
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
34 changes: 34 additions & 0 deletions app/controllers/users/task_pages_controller.rb
Original file line number Diff line number Diff line change
@@ -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 = <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: pagination_json
end

private

def assignee
user
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
4 changes: 0 additions & 4 deletions app/models/organization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pulled out here


private

def clean_url
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
Loading