Commit 063b9edc authored by Sean McGivern's avatar Sean McGivern

Save a query on the todos index page

When there are no filters, we can get the total todos count from the cached
count on the user object, instead of performing a DB query. This query takes
about 80ms for me on GitLab.com.
parent 4a4f8093
...@@ -7,9 +7,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -7,9 +7,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController
def index def index
@sort = params[:sort] @sort = params[:sort]
@todos = @todos.page(params[:page]) @todos = @todos.page(params[:page])
if @todos.out_of_range? && @todos.total_pages != 0
redirect_to url_for(params.merge(page: @todos.total_pages, only_path: true)) return if redirect_out_of_range(@todos)
end
end end
def destroy def destroy
...@@ -60,7 +59,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -60,7 +59,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
end end
def find_todos def find_todos
@todos ||= TodosFinder.new(current_user, params).execute @todos ||= TodosFinder.new(current_user, todo_params).execute
end end
def todos_counts def todos_counts
...@@ -69,4 +68,27 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -69,4 +68,27 @@ class Dashboard::TodosController < Dashboard::ApplicationController
done_count: number_with_delimiter(current_user.todos_done_count) done_count: number_with_delimiter(current_user.todos_done_count)
} }
end end
def todo_params
params.permit(:action_id, :author_id, :project_id, :type, :sort, :state)
end
def redirect_out_of_range(todos)
total_pages =
if todo_params.except(:sort, :page).empty?
(current_user.todos_pending_count / todos.limit_value).ceil
else
todos.total_pages
end
return false if total_pages.zero?
out_of_range = todos.current_page > total_pages
if out_of_range
redirect_to url_for(params.merge(page: total_pages, only_path: true))
end
out_of_range
end
end end
---
title: Remove a SQL query from the todos index page
merge_request:
author:
type: other
...@@ -57,7 +57,7 @@ describe Dashboard::TodosController do ...@@ -57,7 +57,7 @@ describe Dashboard::TodosController do
expect(response).to redirect_to(dashboard_todos_path(page: last_page)) expect(response).to redirect_to(dashboard_todos_path(page: last_page))
end end
it 'redirects to correspondent page' do it 'goes to the correct page' do
get :index, page: last_page get :index, page: last_page
expect(assigns(:todos).current_page).to eq(last_page) expect(assigns(:todos).current_page).to eq(last_page)
...@@ -70,6 +70,30 @@ describe Dashboard::TodosController do ...@@ -70,6 +70,30 @@ describe Dashboard::TodosController do
expect(response).to redirect_to(dashboard_todos_path(page: last_page)) expect(response).to redirect_to(dashboard_todos_path(page: last_page))
end end
context 'when providing no filters' do
it 'does not perform a query to get the page count, but gets that from the user' do
allow(controller).to receive(:current_user).and_return(user)
expect(user).to receive(:todos_pending_count).and_call_original
get :index, page: (last_page + 1).to_param, sort: :created_asc
expect(response).to redirect_to(dashboard_todos_path(page: last_page, sort: :created_asc))
end
end
context 'when providing filters' do
it 'performs a query to get the correct page count' do
allow(controller).to receive(:current_user).and_return(user)
expect(user).not_to receive(:todos_pending_count)
get :index, page: (last_page + 1).to_param, project_id: project.id
expect(response).to redirect_to(dashboard_todos_path(page: last_page, project_id: project.id))
end
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment