Merge branch 'wolf/feat/milestone-hide-mr' into 'master'

Hide MRs in milestones when they're disabled

See merge request gitlab-org/gitlab!20985

app/models/dashboard_group_milestone.rb

@@ -22,4 +22,8 @@ class DashboardGroupMilestone < GlobalMilestone
   def dashboard_milestone?
     true
   end
+
+  def merge_requests_enabled?
+    true
+  end
 end

app/models/dashboard_milestone.rb

@@ -12,4 +12,8 @@ class DashboardMilestone < GlobalMilestone
   def project_milestone?
     true
   end
+
+  def merge_requests_enabled?
+    project.merge_requests_enabled?
+  end
 end

app/models/group_milestone.rb

@@ -41,4 +41,8 @@ class GroupMilestone < GlobalMilestone
   def legacy_group_milestone?
     true
   end
+
+  def merge_requests_enabled?
+    true
+  end
 end

app/models/milestone.rb

@@ -274,6 +274,16 @@ class Milestone < ApplicationRecord
     project_id.present?
   end
 
+  def merge_requests_enabled?
+    if group_milestone?
+      # Assume that groups have at least one project with merge requests enabled.
+      # Otherwise, we would need to load all of the projects from the database.
+      true
+    elsif project_milestone?
+      project&.merge_requests_enabled?
+    end
+  end
+
   private
 
   # Milestone titles must be unique across project milestones and group milestones

app/views/shared/milestones/_milestone.html.haml

@@ -43,8 +43,9 @@
     .col-sm-4.milestone-progress
       = milestone_progress_bar(milestone)
       = link_to pluralize(milestone.total_issues_count(current_user), 'Issue'), issues_path
-      ·
-      = link_to pluralize(milestone.merge_requests_visible_to_user(current_user).size, 'Merge Request'), merge_requests_path
+      - if milestone.merge_requests_enabled?
+        ·
+        = link_to pluralize(milestone.merge_requests_visible_to_user(current_user).size, 'Merge Request'), merge_requests_path
       .float-lg-right.light #{milestone.percent_complete(current_user)}% complete
     .col-sm-2
       .milestone-actions.d-flex.justify-content-sm-start.justify-content-md-end

app/views/shared/milestones/_sidebar.html.haml

@@ -105,38 +105,39 @@
 
     = render_if_exists 'shared/milestones/weight', milestone: milestone
 
-    .block.merge-requests
-      .sidebar-collapsed-icon.has-tooltip{ title: milestone_merge_requests_tooltip_text(milestone), data: { container: 'body', html: 'true', placement: 'left', boundary: 'viewport' } }
-        %strong
-          = custom_icon('mr_bold')
-        %span= milestone.merge_requests.count
-      .title.hide-collapsed
-        Merge requests
-        %span.badge.badge-pill= milestone.merge_requests.count
-      .value.hide-collapsed.bold
-        - if !project || can?(current_user, :read_merge_request, project)
-          %span.milestone-stat
-            = link_to milestones_browse_issuables_path(milestone, type: :merge_requests) do
+    - if milestone.merge_requests_enabled?
+      .block.merge-requests
+        .sidebar-collapsed-icon.has-tooltip{ title: milestone_merge_requests_tooltip_text(milestone), data: { container: 'body', html: 'true', placement: 'left', boundary: 'viewport' } }
+          %strong
+            = custom_icon('mr_bold')
+          %span= milestone.merge_requests.count
+        .title.hide-collapsed
+          Merge requests
+          %span.badge.badge-pill= milestone.merge_requests.count
+        .value.hide-collapsed.bold
+          - if !project || can?(current_user, :read_merge_request, project)
+            %span.milestone-stat
+              = link_to milestones_browse_issuables_path(milestone, type: :merge_requests) do
+                Open:
+                = milestone.merge_requests.opened.count
+            %span.milestone-stat
+              = link_to milestones_browse_issuables_path(milestone, type: :merge_requests, state: 'closed') do
+                Closed:
+                = milestone.merge_requests.closed.count
+            %span.milestone-stat
+              = link_to milestones_browse_issuables_path(milestone, type: :merge_requests, state: 'merged') do
+                Merged:
+                = milestone.merge_requests.merged.count
+          - else
+            %span.milestone-stat
               Open:
               = milestone.merge_requests.opened.count
-          %span.milestone-stat
-            = link_to milestones_browse_issuables_path(milestone, type: :merge_requests, state: 'closed') do
+            %span.milestone-stat
               Closed:
               = milestone.merge_requests.closed.count
-          %span.milestone-stat
-            = link_to milestones_browse_issuables_path(milestone, type: :merge_requests, state: 'merged') do
+            %span.milestone-stat
               Merged:
               = milestone.merge_requests.merged.count
-        - else
-          %span.milestone-stat
-            Open:
-            = milestone.merge_requests.opened.count
-          %span.milestone-stat
-            Closed:
-            = milestone.merge_requests.closed.count
-          %span.milestone-stat
-            Merged:
-            = milestone.merge_requests.merged.count
 
     - if project
       - recent_releases, total_count, more_count = recent_releases_with_counts(milestone)

app/views/shared/milestones/_tabs.html.haml

@@ -6,10 +6,11 @@
       = link_to '#tab-issues', class: 'nav-link active', data: { toggle: 'tab', show: '.tab-issues-buttons' } do
         = _('Issues')
         %span.badge.badge-pill= milestone.issues_visible_to_user(current_user).size
-    %li.nav-item
-      = link_to '#tab-merge-requests', class: 'nav-link', data: { toggle: 'tab', endpoint: milestone_tab_path(milestone, 'merge_requests') } do
-        = _('Merge Requests')
-        %span.badge.badge-pill= milestone.merge_requests_visible_to_user(current_user).size
+    - if milestone.merge_requests_enabled?
+      %li.nav-item
+        = link_to '#tab-merge-requests', class: 'nav-link', data: { toggle: 'tab', endpoint: milestone_tab_path(milestone, 'merge_requests') } do
+          = _('Merge Requests')
+          %span.badge.badge-pill= milestone.merge_requests_visible_to_user(current_user).size
     %li.nav-item
       = link_to '#tab-participants', class: 'nav-link', data: { toggle: 'tab', endpoint: milestone_tab_path(milestone, 'participants') } do
         = _('Participants')
@@ -26,9 +27,10 @@
 .tab-content.milestone-content
   .tab-pane.active#tab-issues{ data: { sort_endpoint: (sort_issues_project_milestone_path(@project, @milestone) if @project && current_user) } }
     = render 'shared/milestones/issues_tab', issues: issues, show_project_name: show_project_name, show_full_project_name: show_full_project_name
-  .tab-pane#tab-merge-requests
-    -# loaded async
-    = render "shared/milestones/tab_loading"
+  - if milestone.merge_requests_enabled?
+    .tab-pane#tab-merge-requests
+      -# loaded async
+      = render "shared/milestones/tab_loading"
   .tab-pane#tab-participants
     -# loaded async
     = render "shared/milestones/tab_loading"

changelogs/unreleased/wolf-feat-milestone-hide-mr.yml

+---
+title: Hide Merge Request information on milestones when MRs are disabled for project
+merge_request: 20985
+author: Wolfgang Faust
+type: changed

spec/features/dashboard/milestones_spec.rb

@@ -30,6 +30,7 @@ describe 'Dashboard > Milestones' do
       expect(current_path).to eq dashboard_milestones_path
       expect(page).to have_content(milestone.title)
       expect(page).to have_content(group.name)
+      expect(first('.milestone')).to have_content('Merge Requests')
     end
 
     describe 'new milestones dropdown', :js do
@@ -46,4 +47,23 @@ describe 'Dashboard > Milestones' do
       end
     end
   end
+
+  describe 'with merge requests disabled' do
+    let(:user) { create(:user) }
+    let(:group) { create(:group) }
+    let(:project) { create(:project, :merge_requests_disabled, namespace: user.namespace) }
+    let!(:milestone) { create(:milestone, project: project) }
+
+    before do
+      group.add_developer(user)
+      sign_in(user)
+      visit dashboard_milestones_path
+    end
+
+    it 'does not see milestones' do
+      expect(current_path).to eq dashboard_milestones_path
+      expect(page).to have_content(milestone.title)
+      expect(first('.milestone')).to have_no_content('Merge Requests')
+    end
+  end
 end

spec/features/milestones/user_views_milestones_spec.rb

@@ -18,6 +18,7 @@ describe "User views milestones" do
     expect(page).to have_content(milestone.title)
       .and have_content(milestone.expires_at)
       .and have_content("Issues")
+      .and have_content("Merge Requests")
   end
 
   context "with issues" do
@@ -32,6 +33,7 @@ describe "User views milestones" do
         .and have_selector("#tab-issues li.issuable-row", count: 2)
         .and have_content(issue.title)
         .and have_content(closed_issue.title)
+        .and have_selector("#tab-merge-requests")
     end
   end
 
@@ -62,3 +64,32 @@ describe "User views milestones" do
     end
   end
 end
+
+describe "User views milestones with no MR" do
+  set(:user) { create(:user) }
+  set(:project) { create(:project, :merge_requests_disabled) }
+  set(:milestone) { create(:milestone, project: project) }
+
+  before do
+    project.add_developer(user)
+    sign_in(user)
+
+    visit(project_milestones_path(project))
+  end
+
+  it "shows milestone" do
+    expect(page).to have_content(milestone.title)
+      .and have_content(milestone.expires_at)
+      .and have_content("Issues")
+      .and have_no_content("Merge Requests")
+  end
+
+  it "opens milestone" do
+    click_link(milestone.title)
+
+    expect(current_path).to eq(project_milestone_path(project, milestone))
+    expect(page).to have_content(milestone.title)
+      .and have_selector("#tab-issues")
+      .and have_no_selector("#tab-merge-requests")
+  end
+end

spec/models/milestone_spec.rb

@@ -106,6 +106,40 @@ describe Milestone do
     end
   end
 
+  describe '#merge_requests_enabled?' do
+    context "per project" do
+      it "is true for projects with MRs enabled" do
+        project = create(:project, :merge_requests_enabled)
+        milestone = create(:milestone, project: project)
+
+        expect(milestone.merge_requests_enabled?).to be(true)
+      end
+
+      it "is false for projects with MRs disabled" do
+        project = create(:project, :repository_enabled, :merge_requests_disabled)
+        milestone = create(:milestone, project: project)
+
+        expect(milestone.merge_requests_enabled?).to be(false)
+      end
+
+      it "is false for projects with repository disabled" do
+        project = create(:project, :repository_disabled)
+        milestone = create(:milestone, project: project)
+
+        expect(milestone.merge_requests_enabled?).to be(false)
+      end
+    end
+
+    context "per group" do
+      let(:group) { create(:group) }
+      let(:milestone) { create(:milestone, group: group) }
+
+      it "is always true for groups, for performance reasons" do
+        expect(milestone.merge_requests_enabled?).to be(true)
+      end
+    end
+  end
+
   describe "unique milestone title" do
     context "per project" do
       it "does not accept the same title in a project twice" do