summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-09-22 14:22:16 +0000
committerDouwe Maan <douwe@gitlab.com>2016-09-22 14:22:16 +0000
commit6c7cf9afef03af25c2f85ba9686dbd4f7303ccfe (patch)
treee038cc34c25e4b63056561a3cddc2c6f965f9567
parent2a5cf05b1d55652493c165ec32100e0c99770ccc (diff)
parentd4e91b22fcef64cef412eb81086ef1c90ca55f22 (diff)
downloadgitlab-ce-6c7cf9afef03af25c2f85ba9686dbd4f7303ccfe.tar.gz
Merge branch 'revert-6455-and-4960' into 'master'
Revert the "accurate issuable counts in issuable list" feature ## Why was this MR needed? !6455 introduced a performance killer, so we revert it until we find a proper solution that's not killing performance. ## What are the relevant issue numbers? Revert !6455 and !4960. See merge request !6476
-rw-r--r--app/controllers/concerns/issuable_collections.rb12
-rw-r--r--app/controllers/concerns/issues_action.rb2
-rw-r--r--app/controllers/concerns/merge_requests_action.rb2
-rw-r--r--app/controllers/projects/issues_controller.rb2
-rw-r--r--app/controllers/projects/merge_requests_controller.rb2
-rw-r--r--app/helpers/application_helper.rb24
-rw-r--r--app/views/shared/issuable/_nav.html.haml12
-rw-r--r--spec/features/dashboard_issues_spec.rb9
-rw-r--r--spec/features/issues/filter_by_labels_spec.rb20
-rw-r--r--spec/features/issues/filter_issues_spec.rb32
10 files changed, 23 insertions, 94 deletions
diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb
index 4a447735fa7..b5e79099e39 100644
--- a/app/controllers/concerns/issuable_collections.rb
+++ b/app/controllers/concerns/issuable_collections.rb
@@ -13,18 +13,10 @@ module IssuableCollections
issues_finder.execute
end
- def all_issues_collection
- IssuesFinder.new(current_user, filter_params_all).execute
- end
-
def merge_requests_collection
merge_requests_finder.execute
end
- def all_merge_requests_collection
- MergeRequestsFinder.new(current_user, filter_params_all).execute
- end
-
def issues_finder
@issues_finder ||= issuable_finder_for(IssuesFinder)
end
@@ -62,10 +54,6 @@ module IssuableCollections
@filter_params
end
- def filter_params_all
- @filter_params_all ||= filter_params.merge(state: 'all', sort: nil)
- end
-
def set_default_scope
params[:scope] = 'all' if params[:scope].blank?
end
diff --git a/app/controllers/concerns/issues_action.rb b/app/controllers/concerns/issues_action.rb
index eced9d9d678..b89fb94be6e 100644
--- a/app/controllers/concerns/issues_action.rb
+++ b/app/controllers/concerns/issues_action.rb
@@ -10,8 +10,6 @@ module IssuesAction
.preload(:author, :project)
.page(params[:page])
- @all_issues = all_issues_collection.non_archived
-
respond_to do |format|
format.html
format.atom { render layout: false }
diff --git a/app/controllers/concerns/merge_requests_action.rb b/app/controllers/concerns/merge_requests_action.rb
index 729763169e2..a1b0eee37f9 100644
--- a/app/controllers/concerns/merge_requests_action.rb
+++ b/app/controllers/concerns/merge_requests_action.rb
@@ -9,7 +9,5 @@ module MergeRequestsAction
.non_archived
.preload(:author, :target_project)
.page(params[:page])
-
- @all_merge_requests = all_merge_requests_collection.non_archived
end
end
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 19b8b1576c4..3eb13a121bf 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -28,8 +28,6 @@ class Projects::IssuesController < Projects::ApplicationController
@labels = @project.labels.where(title: params[:label_name])
- @all_issues = all_issues_collection
-
respond_to do |format|
format.html
format.atom { render layout: false }
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index e972376df4c..935417d4ae8 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -37,8 +37,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@labels = @project.labels.where(title: params[:label_name])
- @all_merge_requests = all_merge_requests_collection
-
respond_to do |format|
format.html
format.json do
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 2163a437c48..1df430e6279 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -280,25 +280,23 @@ module ApplicationHelper
end
end
- def state_filters_text_for(state, records)
+ def state_filters_text_for(entity, project)
titles = {
opened: "Open"
}
- state_title = titles[state] || state.to_s.humanize
- records_with_state = records.public_send(state)
+ entity_title = titles[entity] || entity.to_s.humanize
- # When filtering by multiple labels, the result of query.count is a Hash
- # of the form { issuable_id1 => N, issuable_id2 => N }, where N is the
- # number of labels selected. The ugly "trick" is to load the issuables
- # as an array and get the size of the array...
- # We should probably try to solve this properly in the future.
- # See https://gitlab.com/gitlab-org/gitlab-ce/issues/22414
- label_names = Array(params.fetch(:label_name, []))
- records_with_state = records_with_state.to_a if label_names.many?
+ count =
+ if project.nil?
+ nil
+ elsif current_controller?(:issues)
+ project.issues.visible_to_user(current_user).send(entity).count
+ elsif current_controller?(:merge_requests)
+ project.merge_requests.send(entity).count
+ end
- count = records_with_state.size
- html = content_tag :span, state_title
+ html = content_tag :span, entity_title
if count.present?
html += " "
diff --git a/app/views/shared/issuable/_nav.html.haml b/app/views/shared/issuable/_nav.html.haml
index fb592c2b1e2..1d9b09a5ef1 100644
--- a/app/views/shared/issuable/_nav.html.haml
+++ b/app/views/shared/issuable/_nav.html.haml
@@ -1,27 +1,25 @@
%ul.nav-links.issues-state-filters
- if defined?(type) && type == :merge_requests
- page_context_word = 'merge requests'
- - records = @all_merge_requests
- else
- page_context_word = 'issues'
- - records = @all_issues
%li{class: ("active" if params[:state] == 'opened')}
= link_to page_filter_path(state: 'opened', label: true), title: "Filter by #{page_context_word} that are currently opened." do
- #{state_filters_text_for(:opened, records)}
+ #{state_filters_text_for(:opened, @project)}
- if defined?(type) && type == :merge_requests
%li{class: ("active" if params[:state] == 'merged')}
= link_to page_filter_path(state: 'merged', label: true), title: 'Filter by merge requests that are currently merged.' do
- #{state_filters_text_for(:merged, records)}
+ #{state_filters_text_for(:merged, @project)}
%li{class: ("active" if params[:state] == 'closed')}
= link_to page_filter_path(state: 'closed', label: true), title: 'Filter by merge requests that are currently closed and unmerged.' do
- #{state_filters_text_for(:closed, records)}
+ #{state_filters_text_for(:closed, @project)}
- else
%li{class: ("active" if params[:state] == 'closed')}
= link_to page_filter_path(state: 'closed', label: true), title: 'Filter by issues that are currently closed.' do
- #{state_filters_text_for(:closed, records)}
+ #{state_filters_text_for(:closed, @project)}
%li{class: ("active" if params[:state] == 'all')}
= link_to page_filter_path(state: 'all', label: true), title: "Show all #{page_context_word}." do
- #{state_filters_text_for(:all, records)}
+ #{state_filters_text_for(:all, @project)}
diff --git a/spec/features/dashboard_issues_spec.rb b/spec/features/dashboard_issues_spec.rb
index fc914022a59..3fb1cb37544 100644
--- a/spec/features/dashboard_issues_spec.rb
+++ b/spec/features/dashboard_issues_spec.rb
@@ -21,9 +21,6 @@ describe "Dashboard Issues filtering", feature: true, js: true do
click_link 'No Milestone'
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '1')
- end
expect(page).to have_selector('.issue', count: 1)
end
@@ -32,9 +29,6 @@ describe "Dashboard Issues filtering", feature: true, js: true do
click_link 'Any Milestone'
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '2')
- end
expect(page).to have_selector('.issue', count: 2)
end
@@ -45,9 +39,6 @@ describe "Dashboard Issues filtering", feature: true, js: true do
click_link milestone.title
end
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '1')
- end
expect(page).to have_selector('.issue', count: 1)
end
end
diff --git a/spec/features/issues/filter_by_labels_spec.rb b/spec/features/issues/filter_by_labels_spec.rb
index 7e2abd759e1..908b18e5339 100644
--- a/spec/features/issues/filter_by_labels_spec.rb
+++ b/spec/features/issues/filter_by_labels_spec.rb
@@ -6,19 +6,20 @@ feature 'Issue filtering by Labels', feature: true do
let(:project) { create(:project, :public) }
let!(:user) { create(:user)}
let!(:label) { create(:label, project: project) }
- let(:bug) { create(:label, project: project, title: 'bug') }
- let(:feature) { create(:label, project: project, title: 'feature') }
- let(:enhancement) { create(:label, project: project, title: 'enhancement') }
- let(:issue1) { create(:issue, title: "Bugfix1", project: project) }
- let(:issue2) { create(:issue, title: "Bugfix2", project: project) }
- let(:issue3) { create(:issue, title: "Feature1", project: project) }
before do
+ bug = create(:label, project: project, title: 'bug')
+ feature = create(:label, project: project, title: 'feature')
+ enhancement = create(:label, project: project, title: 'enhancement')
+
+ issue1 = create(:issue, title: "Bugfix1", project: project)
issue1.labels << bug
+ issue2 = create(:issue, title: "Bugfix2", project: project)
issue2.labels << bug
issue2.labels << enhancement
+ issue3 = create(:issue, title: "Feature1", project: project)
issue3.labels << feature
project.team << [user, :master]
@@ -158,13 +159,6 @@ feature 'Issue filtering by Labels', feature: true do
wait_for_ajax
end
- it 'shows a correct "Open" counter' do
- page.within '.issues-state-filters' do
- expect(page).not_to have_content "{#{issue2.id} => 1}"
- expect(page).to have_content "Open 1"
- end
- end
-
it 'shows issue "Bugfix2" in issues list' do
expect(page).to have_content "Bugfix2"
end
diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb
index 72f39e2fbca..d1501c9791a 100644
--- a/spec/features/issues/filter_issues_spec.rb
+++ b/spec/features/issues/filter_issues_spec.rb
@@ -230,10 +230,6 @@ describe 'Filter issues', feature: true do
expect(page).to have_selector('.issue', count: 2)
end
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '2')
- end
-
click_button 'Label'
page.within '.labels-filter' do
click_link 'bug'
@@ -243,10 +239,6 @@ describe 'Filter issues', feature: true do
page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 1)
end
-
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '1')
- end
end
it 'filters by text and milestone' do
@@ -256,10 +248,6 @@ describe 'Filter issues', feature: true do
expect(page).to have_selector('.issue', count: 2)
end
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '2')
- end
-
click_button 'Milestone'
page.within '.milestone-filter' do
click_link '8'
@@ -268,10 +256,6 @@ describe 'Filter issues', feature: true do
page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 1)
end
-
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '1')
- end
end
it 'filters by text and assignee' do
@@ -281,10 +265,6 @@ describe 'Filter issues', feature: true do
expect(page).to have_selector('.issue', count: 2)
end
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '2')
- end
-
click_button 'Assignee'
page.within '.dropdown-menu-assignee' do
click_link user.name
@@ -293,10 +273,6 @@ describe 'Filter issues', feature: true do
page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 1)
end
-
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '1')
- end
end
it 'filters by text and author' do
@@ -306,10 +282,6 @@ describe 'Filter issues', feature: true do
expect(page).to have_selector('.issue', count: 2)
end
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '2')
- end
-
click_button 'Author'
page.within '.dropdown-menu-author' do
click_link user.name
@@ -318,10 +290,6 @@ describe 'Filter issues', feature: true do
page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 1)
end
-
- page.within '.issues-state-filters' do
- expect(page).to have_selector('.active .badge', text: '1')
- end
end
end
end