From 244134f9c33dea0003dc2403dceace4b94a87d2e Mon Sep 17 00:00:00 2001
From: Paco Guzman <pacoguzmanp@gmail.com>
Date: Mon, 11 Jul 2016 08:10:04 +0200
Subject: [PATCH] Cache todos pending/done dashboard query counts
---
CHANGELOG | 1 +
app/controllers/dashboard/todos_controller.rb | 13 +++-
app/controllers/projects/todos_controller.rb | 2 +-
app/finders/todos_finder.rb | 2 +-
app/helpers/todos_helper.rb | 4 +-
.../projects/todo_controller_spec.rb | 78 ++++++++++++-------
6 files changed, 62 insertions(+), 38 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index ee3ee4c37d6..3758baa0a0a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -49,6 +49,7 @@ v 8.10.0 (unreleased)
- Collapse large diffs by default (!4990)
- Check for conflicts with existing Project's wiki path when creating a new project.
- Show last push widget in upstream after push to fork
+ - Cache todos pending/done dashboard query counts.
- Don't instantiate a git tree on Projects show default view
- Bump Rinku to 2.0.0
- Remove unused front-end variable -> default_issues_tracker
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb
index 3a2db3e6eeb..19a76a5b5d8 100644
--- a/app/controllers/dashboard/todos_controller.rb
+++ b/app/controllers/dashboard/todos_controller.rb
@@ -1,6 +1,4 @@
class Dashboard::TodosController < Dashboard::ApplicationController
- include TodosHelper
-
before_action :find_todos, only: [:index, :destroy_all]
def index
@@ -13,7 +11,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
respond_to do |format|
format.html { redirect_to dashboard_todos_path, notice: 'Todo was successfully marked as done.' }
format.js { head :ok }
- format.json { render json: { count: todos_pending_count, done_count: todos_done_count } }
+ format.json { render json: todos_counts }
end
end
@@ -23,7 +21,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
respond_to do |format|
format.html { redirect_to dashboard_todos_path, notice: 'All todos were marked as done.' }
format.js { head :ok }
- format.json { render json: { count: todos_pending_count, done_count: todos_done_count } }
+ format.json { render json: todos_counts }
end
end
@@ -36,4 +34,11 @@ class Dashboard::TodosController < Dashboard::ApplicationController
def find_todos
@todos ||= TodosFinder.new(current_user, params).execute
end
+
+ def todos_counts
+ {
+ count: TodosFinder.new(current_user, state: :pending).execute.count,
+ done_count: TodosFinder.new(current_user, state: :done).execute.count
+ }
+ end
end
diff --git a/app/controllers/projects/todos_controller.rb b/app/controllers/projects/todos_controller.rb
index 23868d986e9..5685d0f4e7c 100644
--- a/app/controllers/projects/todos_controller.rb
+++ b/app/controllers/projects/todos_controller.rb
@@ -5,7 +5,7 @@ class Projects::TodosController < Projects::ApplicationController
todo = TodoService.new.mark_todo(issuable, current_user)
render json: {
- count: current_user.todos_pending_count,
+ count: TodosFinder.new(current_user, state: :pending).execute.count,
delete_path: dashboard_todo_path(todo)
}
end
diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index 7806d9e4cc5..1a8f490355b 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -8,7 +8,7 @@
# action_id: integer
# author_id: integer
# project_id; integer
-# state: 'pending' or 'done'
+# state: 'pending' (default) or 'done'
# type: 'Issue' or 'MergeRequest'
#
diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index a832a6c8df7..0925760e69c 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -1,10 +1,10 @@
module TodosHelper
def todos_pending_count
- TodosFinder.new(current_user, state: :pending).execute.count
+ @todos_pending_count ||= TodosFinder.new(current_user, state: :pending).execute.count
end
def todos_done_count
- TodosFinder.new(current_user, state: :done).execute.count
+ @todos_done_count ||= TodosFinder.new(current_user, state: :done).execute.count
end
def todo_action_name(todo)
diff --git a/spec/controllers/projects/todo_controller_spec.rb b/spec/controllers/projects/todo_controller_spec.rb
index 5a8bba28594..936320a3709 100644
--- a/spec/controllers/projects/todo_controller_spec.rb
+++ b/spec/controllers/projects/todo_controller_spec.rb
@@ -1,6 +1,8 @@
require('spec_helper')
describe Projects::TodosController do
+ include ApiHelpers
+
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:issue) { create(:issue, project: project) }
@@ -8,43 +10,51 @@ describe Projects::TodosController do
context 'Issues' do
describe 'POST create' do
+ def go
+ post :create,
+ namespace_id: project.namespace.path,
+ project_id: project.path,
+ issuable_id: issue.id,
+ issuable_type: 'issue',
+ format: 'html'
+ end
+
context 'when authorized' do
before do
sign_in(user)
project.team << [user, :developer]
end
- it 'should create todo for issue' do
+ it 'creates todo for issue' do
expect do
- post(:create, namespace_id: project.namespace.path,
- project_id: project.path,
- issuable_id: issue.id,
- issuable_type: 'issue')
+ go
end.to change { user.todos.count }.by(1)
expect(response).to have_http_status(200)
end
+
+ it 'returns todo path and pending count' do
+ go
+
+ expect(response).to have_http_status(200)
+ expect(json_response['count']).to eq 1
+ expect(json_response['delete_path']).to match(/\/dashboard\/todos\/\d{1}/)
+ end
end
context 'when not authorized' do
- it 'should not create todo for issue that user has no access to' do
+ it 'does not create todo for issue that user has no access to' do
sign_in(user)
expect do
- post(:create, namespace_id: project.namespace.path,
- project_id: project.path,
- issuable_id: issue.id,
- issuable_type: 'issue')
+ go
end.to change { user.todos.count }.by(0)
expect(response).to have_http_status(404)
end
- it 'should not create todo for issue when user not logged in' do
+ it 'does not create todo for issue when user not logged in' do
expect do
- post(:create, namespace_id: project.namespace.path,
- project_id: project.path,
- issuable_id: issue.id,
- issuable_type: 'issue')
+ go
end.to change { user.todos.count }.by(0)
expect(response).to have_http_status(302)
@@ -55,43 +65,51 @@ describe Projects::TodosController do
context 'Merge Requests' do
describe 'POST create' do
+ def go
+ post :create,
+ namespace_id: project.namespace.path,
+ project_id: project.path,
+ issuable_id: merge_request.id,
+ issuable_type: 'merge_request',
+ format: 'html'
+ end
+
context 'when authorized' do
before do
sign_in(user)
project.team << [user, :developer]
end
- it 'should create todo for merge request' do
+ it 'creates todo for merge request' do
expect do
- post(:create, namespace_id: project.namespace.path,
- project_id: project.path,
- issuable_id: merge_request.id,
- issuable_type: 'merge_request')
+ go
end.to change { user.todos.count }.by(1)
expect(response).to have_http_status(200)
end
+
+ it 'returns todo path and pending count' do
+ go
+
+ expect(response).to have_http_status(200)
+ expect(json_response['count']).to eq 1
+ expect(json_response['delete_path']).to match(/\/dashboard\/todos\/\d{1}/)
+ end
end
context 'when not authorized' do
- it 'should not create todo for merge request user has no access to' do
+ it 'does not create todo for merge request user has no access to' do
sign_in(user)
expect do
- post(:create, namespace_id: project.namespace.path,
- project_id: project.path,
- issuable_id: merge_request.id,
- issuable_type: 'merge_request')
+ go
end.to change { user.todos.count }.by(0)
expect(response).to have_http_status(404)
end
- it 'should not create todo for merge request user has no access to' do
+ it 'does not create todo for merge request user has no access to' do
expect do
- post(:create, namespace_id: project.namespace.path,
- project_id: project.path,
- issuable_id: merge_request.id,
- issuable_type: 'merge_request')
+ go
end.to change { user.todos.count }.by(0)
expect(response).to have_http_status(302)
--
GitLab