From dfc2fe774de3a1768afe4e2fb24f32858b430da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 22 Sep 2016 14:39:13 +0200 Subject: Revert "Merge branch '22421-fix-issuable-counter-when-more-than-one-label-is-selected' into 'master' " This reverts commit 8ed5be5935a77fffe730baa188d1e1fc1c739d72, reversing changes made to fb0d1378f4f3365a9cae0938829da0560a92b6e6. --- app/helpers/application_helper.rb | 15 ++------------- spec/features/issues/filter_by_labels_spec.rb | 20 +++++++------------- 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 2163a437c48..ed41bf04fc0 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -286,19 +286,8 @@ module ApplicationHelper } state_title = titles[state] || state.to_s.humanize - records_with_state = records.public_send(state) - - # 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 = records_with_state.size - html = content_tag :span, state_title + count = records.public_send(state).size + html = content_tag :span, state_title if count.present? html += " " 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 -- cgit v1.2.3 From d4e91b22fcef64cef412eb81086ef1c90ca55f22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 22 Sep 2016 15:23:01 +0200 Subject: Revert part of "Merge branch 'update_issues_mr_counter' into 'master' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/controllers/concerns/issuable_collections.rb | 12 -------- app/controllers/concerns/issues_action.rb | 2 -- app/controllers/concerns/merge_requests_action.rb | 2 -- app/controllers/projects/issues_controller.rb | 2 -- .../projects/merge_requests_controller.rb | 2 -- app/helpers/application_helper.rb | 17 +++++++++--- app/views/shared/issuable/_nav.html.haml | 12 ++++---- spec/features/dashboard_issues_spec.rb | 9 ------ spec/features/issues/filter_issues_spec.rb | 32 ---------------------- 9 files changed, 18 insertions(+), 72 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 ed41bf04fc0..1df430e6279 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -280,14 +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 - count = records.public_send(state).size - html = content_tag :span, state_title + entity_title = titles[entity] || entity.to_s.humanize + + 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 + + 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_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 -- cgit v1.2.3