From 2878c990052f78fae3cde21a1d2cd90dd5a85461 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 14 Jun 2016 23:09:48 -0300 Subject: Ensure Todos counters doesn't count Todos for projects pending delete --- app/controllers/dashboard/todos_controller.rb | 21 ++++++---------- app/finders/todos_finder.rb | 2 +- app/helpers/todos_helper.rb | 4 +-- features/dashboard/todos.feature | 7 +++++- features/steps/dashboard/todos.rb | 35 ++++++++++++++++++++++----- spec/features/todos/todos_spec.rb | 4 +++ 6 files changed, 50 insertions(+), 23 deletions(-) diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index 7842fb9ce63..3a2db3e6eeb 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -1,5 +1,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController - before_action :find_todos, only: [:index, :destroy, :destroy_all] + include TodosHelper + + before_action :find_todos, only: [:index, :destroy_all] def index @todos = @todos.page(params[:page]) @@ -8,14 +10,10 @@ class Dashboard::TodosController < Dashboard::ApplicationController def destroy TodoService.new.mark_todos_as_done([todo], current_user) - todo_notice = 'Todo was successfully marked as done.' - respond_to do |format| - format.html { redirect_to dashboard_todos_path, notice: todo_notice } + format.html { redirect_to dashboard_todos_path, notice: 'Todo was successfully marked as done.' } format.js { head :ok } - format.json do - render json: { count: @todos.size, done_count: current_user.todos_done_count } - end + format.json { render json: { count: todos_pending_count, done_count: todos_done_count } } end end @@ -25,20 +23,17 @@ 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 do - find_todos - render json: { count: @todos.size, done_count: current_user.todos_done_count } - end + format.json { render json: { count: todos_pending_count, done_count: todos_done_count } } end end private def todo - @todo ||= current_user.todos.find(params[:id]) + @todo ||= find_todos.find(params[:id]) end def find_todos - @todos = TodosFinder.new(current_user, params).execute + @todos ||= TodosFinder.new(current_user, params).execute end end diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index aa47c6c157e..58a00f88af7 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -123,7 +123,7 @@ class TodosFinder end def by_state(items) - case params[:state] + case params[:state].to_s when 'done' items.done else diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index c7aeed4b9fc..e1d8517f712 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -1,10 +1,10 @@ module TodosHelper def todos_pending_count - current_user.todos_pending_count + TodosFinder.new(current_user, state: :pending).execute.count end def todos_done_count - current_user.todos_done_count + TodosFinder.new(current_user, state: :done).execute.count end def todo_action_name(todo) diff --git a/features/dashboard/todos.feature b/features/dashboard/todos.feature index 8677b450813..42f5d6d2af7 100644 --- a/features/dashboard/todos.feature +++ b/features/dashboard/todos.feature @@ -14,7 +14,12 @@ Feature: Dashboard Todos Scenario: I mark todos as done Then I should see todos assigned to me And I mark the todo as done - And I click on the "Done" tab + Then I should see the todo marked as done + + @javascript + Scenario: I mark all todos as done + Then I should see todos assigned to me + And I mark all todos as done Then I should see all todos marked as done @javascript diff --git a/features/steps/dashboard/todos.rb b/features/steps/dashboard/todos.rb index 19fedfbfcdf..60152d3da55 100644 --- a/features/steps/dashboard/todos.rb +++ b/features/steps/dashboard/todos.rb @@ -26,14 +26,15 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps end step 'I should see todos assigned to me' do + page.within('.todos-pending-count') { expect(page).to have_content '4' } expect(page).to have_content 'To do 4' expect(page).to have_content 'Done 0' expect(page).to have_link project.name_with_namespace should_see_todo(1, "John Doe assigned you merge request #{merge_request.to_reference}", merge_request.title) - should_see_todo(2, "John Doe mentioned you on issue ##{issue.iid}", "#{current_user.to_reference} Wdyt?") - should_see_todo(3, "John Doe assigned you issue ##{issue.iid}", issue.title) - should_see_todo(4, "Mary Jane mentioned you on issue ##{issue.iid}", issue.title) + should_see_todo(2, "John Doe mentioned you on issue #{issue.to_reference}", "#{current_user.to_reference} Wdyt?") + should_see_todo(3, "John Doe assigned you issue #{issue.to_reference}", issue.title) + should_see_todo(4, "Mary Jane mentioned you on issue #{issue.to_reference}", issue.title) end step 'I mark the todo as done' do @@ -41,18 +42,40 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps click_link 'Done' end + page.within('.todos-pending-count') { expect(page).to have_content '3' } expect(page).to have_content 'To do 3' expect(page).to have_content 'Done 1' should_not_see_todo "John Doe assigned you merge request #{merge_request.to_reference}" end - step 'I click on the "Done" tab' do + step 'I mark all todos as done' do + click_link 'Mark all as done' + + page.within('.todos-pending-count') { expect(page).to have_content '0' } + expect(page).to have_content 'To do 0' + expect(page).to have_content 'Done 4' + expect(page).not_to have_link project.name_with_namespace + should_not_see_todo "John Doe assigned you merge request #{merge_request.to_reference}" + should_not_see_todo "John Doe mentioned you on issue #{issue.to_reference}" + should_not_see_todo "John Doe assigned you issue #{issue.to_reference}" + should_not_see_todo "Mary Jane mentioned you on issue #{issue.to_reference}" + end + + step 'I should see the todo marked as done' do click_link 'Done 1' + + expect(page).to have_link project.name_with_namespace + should_see_todo(1, "John Doe assigned you merge request #{merge_request.to_reference}", merge_request.title, false) end step 'I should see all todos marked as done' do + click_link 'Done 4' + expect(page).to have_link project.name_with_namespace should_see_todo(1, "John Doe assigned you merge request #{merge_request.to_reference}", merge_request.title, false) + should_see_todo(2, "John Doe mentioned you on issue #{issue.to_reference}", "#{current_user.to_reference} Wdyt?", false) + should_see_todo(3, "John Doe assigned you issue #{issue.to_reference}", issue.title, false) + should_see_todo(4, "Mary Jane mentioned you on issue #{issue.to_reference}", issue.title, false) end step 'I filter by "Enterprise"' do @@ -76,7 +99,7 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps end step 'I should not see todos related to "Mary Jane" in the list' do - should_not_see_todo "Mary Jane mentioned you on issue ##{issue.iid}" + should_not_see_todo "Mary Jane mentioned you on issue #{issue.to_reference}" end step 'I should not see todos related to "Merge Requests" in the list' do @@ -85,7 +108,7 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps step 'I should not see todos related to "Assignments" in the list' do should_not_see_todo "John Doe assigned you merge request #{merge_request.to_reference}" - should_not_see_todo "John Doe assigned you issue ##{issue.iid}" + should_not_see_todo "John Doe assigned you issue #{issue.to_reference}" end step 'I click on the todo' do diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index 8e1833a069e..0bdb1628c74 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -103,11 +103,15 @@ describe 'Dashboard Todos', feature: true do before do deleted_project = create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC, pending_delete: true) create(:todo, :mentioned, user: user, project: deleted_project, target: issue, author: author) + create(:todo, :mentioned, user: user, project: deleted_project, target: issue, author: author, state: :done) login_as(user) visit dashboard_todos_path end it 'shows "All done" message' do + within('.todos-pending-count') { expect(page).to have_content '0' } + expect(page).to have_content 'To do 0' + expect(page).to have_content 'Done 0' expect(page).to have_content "You're all done!" end end -- cgit v1.2.1 From 16387947341ccf49eeec366725fadf901a20b10e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Fri, 17 Jun 2016 13:43:41 -0300 Subject: Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index e16f5c331e5..a80a7768c57 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -126,6 +126,7 @@ v 8.9.0 (unreleased) - Use Git cached counters for branches and tags on project page - Filter parameters for request_uri value on instrumented transactions. - Cache user todo counts from TodoService + - Ensure Todos counters doesn't count Todos for projects pending delete v 8.8.5 - Import GitHub repositories respecting the API rate limit !4166 -- cgit v1.2.1