Commit d97b3d14 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'fix-todos-counters' into 'master'

Ensure Todos counters doesn't count Todos for projects pending delete

Use `TodosFinder` instead of `current_user.todos` to filter projects
pending delete on Todos counters helpers.

Counters should not reflect the number of Todos displayed on the tabs.

Fixes #18633

See merge request !4663
parents d9d14924 16387947
Pipeline #3543005 passed with stages
in 1018 minutes and 42 seconds
......@@ -127,6 +127,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
......
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
......@@ -123,7 +123,7 @@ class TodosFinder
end
def by_state(items)
case params[:state]
case params[:state].to_s
when 'done'
items.done
else
......
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)
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
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