diff options
| -rw-r--r-- | app/controllers/dashboard/todos_controller.rb | 30 | ||||
| -rw-r--r-- | changelogs/unreleased/save-a-query-on-todos-with-no-filters.yml | 5 | ||||
| -rw-r--r-- | spec/controllers/dashboard/todos_controller_spec.rb | 26 |
3 files changed, 56 insertions, 5 deletions
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index a8b2b93b458..02c5857eea7 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -7,9 +7,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController def index @sort = params[:sort] @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)) - end + + return if redirect_out_of_range(@todos) end def destroy @@ -60,7 +59,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController end def find_todos - @todos ||= TodosFinder.new(current_user, params).execute + @todos ||= TodosFinder.new(current_user, todo_params).execute end def todos_counts @@ -69,4 +68,27 @@ class Dashboard::TodosController < Dashboard::ApplicationController done_count: number_with_delimiter(current_user.todos_done_count) } 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 diff --git a/changelogs/unreleased/save-a-query-on-todos-with-no-filters.yml b/changelogs/unreleased/save-a-query-on-todos-with-no-filters.yml new file mode 100644 index 00000000000..c9fb042aa37 --- /dev/null +++ b/changelogs/unreleased/save-a-query-on-todos-with-no-filters.yml @@ -0,0 +1,5 @@ +--- +title: Remove a SQL query from the todos index page +merge_request: +author: +type: other diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb index c8c6b9f41bf..9df4ebf2fa0 100644 --- a/spec/controllers/dashboard/todos_controller_spec.rb +++ b/spec/controllers/dashboard/todos_controller_spec.rb @@ -57,7 +57,7 @@ describe Dashboard::TodosController do expect(response).to redirect_to(dashboard_todos_path(page: last_page)) end - it 'redirects to correspondent page' do + it 'goes to the correct page' do get :index, page: last_page expect(assigns(:todos).current_page).to eq(last_page) @@ -70,6 +70,30 @@ describe Dashboard::TodosController do expect(response).to redirect_to(dashboard_todos_path(page: last_page)) 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 |