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 all 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

10 changes: 7 additions & 3 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -77,16 +77,20 @@ def filter_by_css_id_or_name
render json: { users: json_users(users) }
end

def id
@id ||= params[:id] || params[:user_id]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could not for the life of me get a route were I could pass :id as the param, only :user_id. Very very open to suggestions 🙏

end

def user
unless css_id || params[:id]
unless css_id || id
fail(
Caseflow::Error::InvalidParameter,
parameter: "css_id or id",
message: "Must provide a css id or user id"
)
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
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
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
12 changes: 10 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rely on user being on a judge team before checking vacols

def update_status!(new_status)
transaction do
if new_status.eql?(Constants.USER_STATUSES.inactive)
Expand All @@ -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?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cannot paginate attorney or judge queues as they will have legacy tasks that are not persisted in the database.

end

def queue_tabs
Expand All @@ -337,7 +345,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
6 changes: 2 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}
requireDasRecord
/>
</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,6 @@ 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

success: PropTypes.shape({
title: PropTypes.string,
detail: PropTypes.string
Expand Down
6 changes: 1 addition & 5 deletions client/app/queue/ColocatedTaskListView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};
};

Expand All @@ -71,7 +69,5 @@ 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

success: PropTypes.object
};
Loading