Fix Archived project merge requests add to group's Merge Requests

counter

This is done by:
 - Extending the IssuableFinder adding the non_archived option to the params
 - Overriding the #filter_params in the MergeRequestsAction
 - Passing the non_archived param in the nav/_group.html.haml navbar
   partial from the groups/merge_requests.html.haml

app/controllers/concerns/merge_requests_action.rb

@@ -6,7 +6,12 @@ module MergeRequestsAction
     @label = merge_requests_finder.labels.first
 
     @merge_requests = merge_requests_collection
-                      .non_archived
                       .page(params[:page])
   end
+
+  private
+
+  def filter_params
+    super.merge(non_archived: true)
+  end
 end

app/finders/issuable_finder.rb

@@ -15,6 +15,7 @@
 #     search: string
 #     label_name: string
 #     sort: string
+#     non_archived: boolean
 #
 class IssuableFinder
   NONE = '0'
@@ -38,6 +39,7 @@ class IssuableFinder
     items = by_author(items)
     items = by_label(items)
     items = by_due_date(items)
+    items = by_non_archived(items)
     sort(items)
   end
 
@@ -353,6 +355,10 @@ class IssuableFinder
     end
   end
 
+  def by_non_archived(items)
+    params[:non_archived].present? ? items.non_archived : items
+  end
+
   def current_user_related?
     params[:scope] == 'created-by-me' || params[:scope] == 'authored' || params[:scope] == 'assigned-to-me'
   end

app/finders/merge_requests_finder.rb

@@ -14,6 +14,7 @@
 #     search: string
 #     label_name: string
 #     sort: string
+#     non_archived: boolean
 #
 class MergeRequestsFinder < IssuableFinder
   def klass

app/views/layouts/nav/_group.html.haml

@@ -31,7 +31,7 @@
       = link_to merge_requests_group_path(@group), title: 'Merge Requests' do
         %span
           Merge Requests
-          - merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened').execute
+          - merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened', non_archived: true).execute
           %span.badge.count= number_with_delimiter(merge_requests.count)
     = nav_link(controller: [:group_members]) do
       = link_to group_group_members_path(@group), title: 'Members' do

app/views/shared/issuable/_nav.html.haml

@@ -4,22 +4,22 @@
 
 %ul.nav-links.issues-state-filters
   %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
+    = link_to page_filter_path(state: 'opened', label: true), id: 'state-opened', title: "Filter by #{page_context_word} that are currently opened." do
       #{issuables_state_counter_text(type, :opened)}
 
   - if 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
+      = link_to page_filter_path(state: 'merged', label: true), id: 'state-merged', title: 'Filter by merge requests that are currently merged.' do
         #{issuables_state_counter_text(type, :merged)}
 
     %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
+      = link_to page_filter_path(state: 'closed', label: true), id: 'state-closed', title: 'Filter by merge requests that are currently closed and unmerged.' do
         #{issuables_state_counter_text(type, :closed)}
   - 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
+      = link_to page_filter_path(state: 'closed', label: true), id: 'state-all', title: 'Filter by issues that are currently closed.' do
         #{issuables_state_counter_text(type, :closed)}
 
   %li{class: ("active" if params[:state] == 'all')}
-    = link_to page_filter_path(state: 'all', label: true), title: "Show all #{page_context_word}." do
+    = link_to page_filter_path(state: 'all', label: true), id: 'state-all', title: "Show all #{page_context_word}." do
       #{issuables_state_counter_text(type, :all)}

changelogs/unreleased/24733-archived-project-merge-request-count.yml

@@ -0,0 +1,4 @@
+---
+title: Fix Archived project merge requests add to group's Merge Requests
+merge_request: 7790
+author: Jacopo Beschi @jacopo-beschi

spec/features/groups/merge_requests_spec.rb

@@ -2,7 +2,35 @@ require 'spec_helper'
 
 feature 'Group merge requests page', feature: true do
   let(:path) { merge_requests_group_path(group) }
-  let(:issuable) { create(:merge_request, source_project: project, target_project: project, title: "this is my created issuable")}
+  let(:issuable) { create(:merge_request, source_project: project, target_project: project, title: 'this is my created issuable') }
 
   include_examples 'project features apply to issuables', MergeRequest
+
+  context 'archived issuable' do
+    let(:project_archived) { create(:project, group: group, merge_requests_access_level: ProjectFeature::ENABLED, archived: true) }
+    let(:issuable_archived) { create(:merge_request, source_project: project_archived, target_project: project_archived, title: 'issuable of an archived project') }
+    let(:access_level) { ProjectFeature::ENABLED }
+    let(:user) { user_in_group }
+
+    before do
+      issuable_archived
+      visit path
+    end
+
+    it 'hides archived merge requests' do
+      expect(page).to have_content(issuable.title)
+      expect(page).not_to have_content(issuable_archived.title)
+    end
+
+    it 'ignores archived merge request count badges in navbar' do
+      expect( page.find('[title="Merge Requests"] span.badge.count').text).to eq("1")
+    end
+
+    it 'ignores archived merge request count badges in state-filters' do
+      expect(page.find('#state-opened span.badge').text).to eq("1")
+      expect(page.find('#state-merged span.badge').text).to eq("0")
+      expect(page.find('#state-closed span.badge').text).to eq("0")
+      expect(page.find('#state-all span.badge').text).to eq("1")
+    end
+  end
 end

spec/finders/merge_requests_finder_spec.rb

@@ -6,14 +6,17 @@ describe MergeRequestsFinder do
 
   let(:project1) { create(:project) }
   let(:project2) { create(:project, forked_from_project: project1) }
+  let(:project3) { create(:project, forked_from_project: project1, archived: true) }
 
   let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) }
   let!(:merge_request2) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1, state: 'closed') }
   let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2) }
+  let!(:merge_request4) { create(:merge_request, :simple, author: user, source_project: project3, target_project: project3) }
 
   before do
     project1.team << [user, :master]
     project2.team << [user, :developer]
+    project3.team << [user, :developer]
     project2.team << [user2, :developer]
   end
 
@@ -21,7 +24,7 @@ describe MergeRequestsFinder do
     it 'filters by scope' do
       params = { scope: 'authored', state: 'opened' }
       merge_requests = MergeRequestsFinder.new(user, params).execute
-      expect(merge_requests.size).to eq(2)
+      expect(merge_requests.size).to eq(3)
     end
 
     it 'filters by project' do
@@ -29,5 +32,11 @@ describe MergeRequestsFinder do
       merge_requests = MergeRequestsFinder.new(user, params).execute
       expect(merge_requests.size).to eq(1)
     end
+
+    it 'filters by non_archived' do
+      params = { non_archived: true }
+      merge_requests = MergeRequestsFinder.new(user, params).execute
+      expect(merge_requests.size).to eq(3)
+    end
   end
 end