diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-06-03 12:33:41 +0000 |
---|---|---|
committer | Tomasz Maczukin <tomasz@maczukin.pl> | 2016-06-14 22:14:16 +0200 |
commit | a834be61eb3d5698fe298eabd8758ef0b861a332 (patch) | |
tree | 3eb888a8635a5440f49a260bda06c2f8dd7357c1 | |
parent | b240450845e5d8f6d0c1012f9df75efb79946d5e (diff) | |
download | gitlab-ce-a834be61eb3d5698fe298eabd8758ef0b861a332.tar.gz |
Merge branch 'todos-filter-project-delete' into 'master'
Ensure we don't show TODOS for projects pending delete
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.
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.
Todos page throws 500 error for users with todos in a project pending deletion.
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/17813
cc\ @stanhu
See merge request !4300
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/finders/todos_finder.rb | 14 | ||||
-rw-r--r-- | spec/features/todos/todos_spec.rb | 17 |
3 files changed, 29 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG index 7a80af3bece..adab410f58c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.8.5 - Import GitHub repositories respecting the API rate limit + - Fix todos page throwing errors when you have a project pending deletion v 8.8.4 - Fix LDAP-based login for users with 2FA enabled. !4493 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/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 |