diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-06-03 15:33:41 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-06-03 15:33:41 +0300 |
commit | 704991252604fbc14eef72e2eb1c2ad4629a7616 (patch) | |
tree | 3d754a42746bb34cb35a4f7c3645d75e81508423 | |
parent | 1637e178b8bae13fb92456570e23243c7ddc7cba (diff) | |
parent | 86675194aa44236c3e9acc4aa7ede143f718685e (diff) |
Merge branch 'todos-filter-project-delete' into 'master'
Ensure we don't show TODOS for projects pending delete
## What does this MR do?
Joins the todos on the projects table in order to run the default scope. Also includes a where clause because the default scope is being removed soon.
## Are there points in the code the reviewer needs to double check?
An alternative approach, more like the Issues page, would be to filter down the list by passing user.authorized_projects into the where clause.
Or we could just be more defensive in the view when iterating.
## Why was this MR needed?
Todos page throws 500 error for users with todos in a project pending deletion.
## What are the relevant issue numbers?
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/17813
cc\ @stanhu
See merge request !4300
-rw-r--r-- | CHANGELOG | 3 | ||||
-rw-r--r-- | app/finders/todos_finder.rb | 14 | ||||
-rw-r--r-- | spec/features/todos/target_state_spec.rb | 2 | ||||
-rw-r--r-- | spec/features/todos/todos_spec.rb | 17 |
4 files changed, 32 insertions, 4 deletions
diff --git a/CHANGELOG b/CHANGELOG index ec026b8f398..6e11626e389 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -34,6 +34,9 @@ v 8.9.0 (unreleased) - Improve error handling importing projects - Put project Files and Commits tabs under Code tab +v 8.8.4 + - Fix todos page throwing errors when you have a project pending deletion + v 8.8.3 - Fix 404 page when viewing TODOs that contain milestones or labels in different projects. !4312 - Fixed JS error when trying to remove discussion form. !4303 diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 4bd46a76087..1d88116d7d2 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -30,7 +30,7 @@ class TodosFinder items = by_state(items) items = by_type(items) - items + items.reorder(id: :desc) end private @@ -78,6 +78,16 @@ class TodosFinder @project end + def projects + return @projects if defined?(@projects) + + if project? + @projects = project + else + @projects = ProjectsFinder.new.execute(current_user) + end + end + def type? type.present? && ['Issue', 'MergeRequest'].include?(type) end @@ -105,6 +115,8 @@ class TodosFinder def by_project(items) if project? items = items.where(project: project) + elsif projects + items = items.merge(projects).joins(:project) end items diff --git a/spec/features/todos/target_state_spec.rb b/spec/features/todos/target_state_spec.rb index 72491ac7e61..32fa88a2b21 100644 --- a/spec/features/todos/target_state_spec.rb +++ b/spec/features/todos/target_state_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' feature 'Todo target states', feature: true do let(:user) { create(:user) } let(:author) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } before do login_as user diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index 4e627753cc7..8e1833a069e 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe 'Dashboard Todos', feature: true do let(:user) { create(:user) } let(:author) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } let(:issue) { create(:issue) } describe 'GET /dashboard/todos' do @@ -49,7 +49,7 @@ describe 'Dashboard Todos', feature: true do note1 = create(:note_on_issue, note: "Hello #{label1.to_reference(format: :name)}", noteable_id: issue.id, noteable_type: 'Issue', project: issue.project) create(:todo, :mentioned, project: project, target: issue, user: user, note_id: note1.id) - project2 = create(:project) + project2 = create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) label2 = create(:label, project: project2) issue2 = create(:issue, project: project2) note2 = create(:note_on_issue, note: "Test #{label2.to_reference(format: :name)}", noteable_id: issue2.id, noteable_type: 'Issue', project: project2) @@ -98,5 +98,18 @@ describe 'Dashboard Todos', feature: true do end end end + + context 'User has a Todo in a project pending deletion' 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) + login_as(user) + visit dashboard_todos_path + end + + it 'shows "All done" message' do + expect(page).to have_content "You're all done!" + end + end end end |