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

[PART 1] Add task pagination endpoint for user tasks #14252

Merged
merged 10 commits into from
May 15, 2020

Conversation

hschallhorn
Copy link
Contributor

@hschallhorn hschallhorn commented May 12, 2020

THIS PR

Part 1 of stack to speed up user queues and bump #14142
Part 2

Description

Creates an endpoint to request the paged tasks of a user

AC

  • Tests pass (no visible changes)

Testing

# Log in as a colocated user
user = User.find_by_css_id("BVALSPORER")
FeatureToggle.enable!(:user_queue_pagination, users: [user.css_id])
User.authentication_service = Fakes::AuthenticationService
User.authentication_service.user_session = User.authentication_service.get_user_session(user.id)

# Create some tasks for them
3.times do 
  ColocatedTask.actions_assigned_to_colocated.map(&:to_sym).each do |action|
    appeal = FactoryBot.create(:appeal)
    parent = FactoryBot.create(
      :ama_task,
      assigned_to: User.first,
      assigned_by: User.first,
      appeal: appeal
    )
    task = FactoryBot.create(
      :ama_colocated_task,
      action,
      assigned_to: Colocated.singleton,
      appeal: appeal,
      assigned_by: User.first,
      parent_id: parent.id
    )
    task.children.first.on_hold!
  end
end

# Get set up to make some requests!
app.get '/'
csrf=app.session.to_hash["_csrf_token"]
base_url = "/users/#{user.id}/task_pages?authenticity_token=#{CGI.escape csrf}&"

# Base request for assigned tasks
params = "tab=assigned_person"
app.get "#{base_url}#{params}"
response_body = JSON.parse(app.response.body); nil
response_body["tasks"]["data"].count
=> 15 # May be different for you but should be no higher than 15
response_body["total_task_count"].eql? Task.active.where(assigned_to: user).count
=> true

# on hold tasks
params = "tab=on_hold_person"
app.get "#{base_url}#{params}"
response_body = JSON.parse(app.response.body); nil
response_body["tasks"]["data"].count
=> 15
response_body["total_task_count"].eql? Task.on_hold.where(assigned_to: user).count
=> true

# sorted tasks
params = "tab=on_hold_person&sort_by=taskColumn"
app.get "#{base_url}#{params}"
response_body = JSON.parse(app.response.body); nil
tasks = response_body["tasks"]["data"]; nil
tasks.map { |task| task["attributes"]["type"] }
=> ["AddressVerificationColocatedTask",
 "AddressVerificationColocatedTask",
 "AddressVerificationColocatedTask",
 "AddressVerificationColocatedTask",
 "AddressVerificationColocatedTask",
 "ExtensionColocatedTask",
 "ExtensionColocatedTask",
 "ExtensionColocatedTask",
 "ExtensionColocatedTask",
 "ExtensionColocatedTask",
 "HearingClarificationColocatedTask",
 "HearingClarificationColocatedTask",
 "HearingClarificationColocatedTask",
 "HearingClarificationColocatedTask",
 "HearingClarificationColocatedTask"]


# paged sorted tasks
params = "tab=on_hold_person&sort_by=taskColumn&page=2"
app.get "#{base_url}#{params}"
response_body = JSON.parse(app.response.body); nil
tasks = response_body["tasks"]["data"]; nil
tasks.map { |task| task["attributes"]["type"] }
=> ["IhpColocatedTask",
 "IhpColocatedTask",
 "IhpColocatedTask",
 "IhpColocatedTask",
 "IhpColocatedTask",
 "NewRepArgumentsColocatedTask",
 "NewRepArgumentsColocatedTask",
 "NewRepArgumentsColocatedTask",
 "NewRepArgumentsColocatedTask",
 "NewRepArgumentsColocatedTask",
 "RetiredVljColocatedTask",
 "RetiredVljColocatedTask",
 "RetiredVljColocatedTask",
 "RetiredVljColocatedTask",
 "RetiredVljColocatedTask"]

# Filtered tasks
params = "tab=on_hold_person&filter%5B%5D=col%3DtaskColumn%26val%3DIhpColocatedTask"
app.get "#{base_url}#{params}"
response_body = JSON.parse(app.response.body); nil
tasks = response_body["tasks"]["data"]; nil
tasks.map { |task| task["attributes"]["type"] }.uniq
=> ["IhpColocatedTask"]

THE FULL STACK

spoilers! Part 2 of stack to bump #14142 [Part 1](https://github.com//pull/14252)

Description

Allows user queues to be paginated

Testing

  1. Log in as BVALSPORER
  2. Pad your queue with some tasks
3.times do 
  ColocatedTask.actions_assigned_to_colocated.map(&:to_sym).each do |action|
    appeal = FactoryBot.create(:appeal)
    parent = FactoryBot.create(
      :ama_task,
      assigned_to: User.first,
      assigned_by: User.first,
      appeal: appeal
    )
    task = FactoryBot.create(
      :ama_colocated_task,
      action,
      assigned_to: Colocated.singleton,
      appeal: appeal,
      assigned_by: User.first,
      parent_id: parent.id
    )
    task.children.first.on_hold!
  end
end
  1. Enable pagination
FeatureToggle.enable!(:user_queue_pagination, users: ["BVALSPORER"])
  1. Test Pagination!

Initial queue load only loads first page of tasks

With dev tools open, got to the user's queue
Screen Shot 2020-05-12 at 3 58 34 PM

  • This request for tasks only returns the first 15 tasks for each tab.
  • total_task_count and task_page_count are correct
  • This request does not return all tasks assigned to the user in tasks: { data: [] }

Filtering, sorting, paging, and switching tabs work

Try a combination of filtering and sorting to confirm the correct tasks are show. Bonus points for trying the same combination more than once and noticing the loading page is not shown because we're pulling from cached results. Each request should only return 15 tasks.
Screen Shot 2020-05-12 at 4 44 16 PM

Sorting

Screen Shot 2020-05-12 at 4 06 09 PM
Screen Shot 2020-05-12 at 4 06 22 PM

Filtering

Screen Shot 2020-05-12 at 4 14 55 PM
Screen Shot 2020-05-12 at 4 15 06 PM

Paging

Screen Shot 2020-05-12 at 4 12 27 PM
Screen Shot 2020-05-12 at 4 12 35 PM

  • Going to a new page
    • Preserves any filtering
    • Preserves any sorting
    • Displays the correct "showing tasks x of xx"
    • Updates the address bar to show paging (confirm this url is correct by bookmarking this page and clicking the bookmark. It should show the same page of tasks and the correct page indicator http://localhost:3000/queue?tab=on_hold_person&page=2)

Changing tabs

Screen Shot 2020-05-12 at 4 18 40 PM
Screen Shot 2020-05-12 at 4 18 53 PM

  • Changing tabs
    • Changes the tab (I ddon't know what you expected here
    • Updates the address bar to show the updated tab (confirm this url is correct by bookmarking this page and clicking the bookmark. It should show the same tab of tasks and the correct tab indicator http://localhost:3000/queue?tab=on_hold_person&page=1)

Any combinations of the above

THINGS THAT SHOULD NOT HAVE CHANGED

  • The appearance and functionality of attorney queues
BEFORE AFTER
Screen Shot 2020-05-12 at 4 36 33 PM Screen Shot 2020-05-12 at 4 37 32 PM
Screen Shot 2020-05-12 at 4 36 52 PM Screen Shot 2020-05-12 at 4 37 39 PM
  • The appearance and functionality of judge queues
BEFORE AFTER
Screen Shot 2020-05-12 at 4 35 04 PM Screen Shot 2020-05-12 at 4 35 34 PM
  • The appearance and functionality of organization queues
BEFORE AFTER
Screen Shot 2020-05-12 at 4 33 28 PM Screen Shot 2020-05-12 at 4 33 44 PM

@codeclimate
Copy link

codeclimate bot commented May 12, 2020

Code Climate has analyzed commit b9cf987 and detected 0 issues on this pull request.

View more on Code Climate.

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 out of app/controllers/organizations/task_pages_controller.rb to be reused in app/controllers/users/task_pages_controller.rb

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.

@@ -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.

URL the front end will use to request paged tasks

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.

New route for the pagination of user tasks

allow_any_instance_of(::Organizations::TaskPagesController).to receive(:json_tasks).and_return([])

subject
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 into spec/controllers/paged_tasks_shared_examples.rb

@hschallhorn hschallhorn self-assigned this May 12, 2020
Copy link
Contributor

@yoomlam yoomlam left a comment

Choose a reason for hiding this comment

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

Left some suggestions.

Also manually tested:

  • the sum of tasks.count equals the response_body["total_task_count"]
  • other filters work
  • bad filter returns empty list

app/controllers/users_controller.rb Show resolved Hide resolved
spec/models/task_pager_spec.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@yoomlam yoomlam left a comment

Choose a reason for hiding this comment

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

Good to go! 🥇

@hschallhorn hschallhorn added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label May 15, 2020
@va-bot
Copy link
Collaborator

va-bot commented May 15, 2020

1 Warning
⚠️ This is a Big PR. Try to break this down if possible. Stacked pull requests encourage more detailed and thorough code reviews

Generated by 🚫 Danger

@va-bot va-bot merged commit 7270430 into master May 15, 2020
@va-bot va-bot deleted the hschallhorn/14142-user-queue-pagination-part-1 branch May 15, 2020 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants