Commit e53ecac1 authored by Douwe Maan's avatar Douwe Maan Committed by Rémy Coutable

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
parent 493037d5
......@@ -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
......
......@@ -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 }
......
......@@ -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
......@@ -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 }
......
......@@ -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
......
......@@ -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 += " "
......
%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)}
......@@ -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
......
......@@ -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
......
......@@ -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
......
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