Merge branch '36524-group-level-compliance-dashboard-mvc' into 'master'

Resolve "Group-level compliance dashboard MVC"

Closes #36524

See merge request gitlab-org/gitlab!20844

app/models/event.rb

@@ -76,6 +76,7 @@ class Event < ApplicationRecord
   # Scopes
   scope :recent, -> { reorder(id: :desc) }
   scope :code_push, -> { where(action: PUSHED) }
+  scope :merged, -> { where(action: MERGED) }
 
   scope :with_associations, -> do
     # We're using preload for "push_event_payload" as otherwise the association

ee/app/controllers/groups/security/compliance_dashboards_controller.rb

+# frozen_string_literal: true
+class Groups::Security::ComplianceDashboardsController < Groups::ApplicationController
+  layout 'group'
+
+  before_action :authorize_compliance_dashboard!
+
+  def show
+    @merge_requests = MergeRequestsComplianceFinder.new(current_user, { group_id: @group.id })
+      .execute
+    @merge_requests = Kaminari.paginate_array(@merge_requests).page(params[:page])
+  end
+
+  private
+
+  def authorize_compliance_dashboard!
+    render_404 unless group.feature_available?(:group_level_compliance_dashboard) &&
+      can?(current_user, :read_group_compliance_dashboard, group)
+  end
+end

ee/app/finders/merge_requests_compliance_finder.rb

+# frozen_string_literal: true
+#
+# Used to filter MergeRequests collections for compliance dashboard
+#
+# Arguments:
+#   current_user - which user calls a class
+#   params:
+#     group_id: integer
+#     preloads: array of associations to preload
+#
+class MergeRequestsComplianceFinder < MergeRequestsFinder
+  def execute
+    # rubocop: disable CodeReuse/ActiveRecord
+    lateral = Event
+      .select(:created_at, :target_id)
+      .where('projects.id = project_id')
+      .merged
+      .recent
+      .limit(1)
+      .to_sql
+
+    sql = find_group_projects.as('projects').to_sql
+    records = Project
+      .select('projects.id, events.target_id as merge_request_id')
+      .from([Arel.sql("#{sql} JOIN LATERAL (#{lateral}) #{Event.table_name} ON true")])
+      .order('events.created_at DESC')
+    select_sorted_mrs(records)
+    # rubocop: enable CodeReuse/ActiveRecord
+  end
+
+  private
+
+  def params
+    finder_options = {
+      include_subgroups: true,
+      attempt_group_search_optimizations: true
+    }
+    super.merge(finder_options)
+  end
+
+  def select_sorted_mrs(records)
+    hash = {}
+    records.each { |row| hash[row['merge_request_id']] = nil }
+    mrs = MergeRequest.where(id: hash.keys).preload(preloads) # rubocop: disable CodeReuse/ActiveRecord
+
+    mrs.each { |mr| hash[mr.id] = mr }
+
+    hash.compact!
+    hash.values # sorted MRs
+  end
+
+  def preloads
+    [:approved_by_users, :metrics, source_project: :route, target_project: :namespace]
+  end
+end

ee/app/models/license.rb

@@ -113,6 +113,7 @@ class License < ApplicationRecord
     dependency_scanning
     epics
     group_ip_restriction
+    group_level_compliance_dashboard
     incident_management
     insights
     license_management

ee/app/policies/ee/group_policy.rb

@@ -133,6 +133,10 @@ module EE
 
       rule { security_dashboard_enabled & developer }.enable :read_group_security_dashboard
 
+      rule { admin | owner }.policy do
+        enable :read_group_compliance_dashboard
+      end
+
       rule { needs_new_sso_session }.policy do
         prevent :read_group
       end

ee/app/views/groups/security/compliance_dashboards/_approvers.html.haml

+- presentable_approvers_limit = 2
+- approvers_over_presentable_limit = merge_request.approved_by_users.size - presentable_approvers_limit
+- project = merge_request.project
+
+= _('Approved by: ')
+- merge_request.approved_by_users.take(presentable_approvers_limit).each do |approver| # rubocop: disable CodeReuse/ActiveRecord
+  = link_to_member(project, approver, name: true, title: "Approved by  :name")
+- if approvers_over_presentable_limit.positive?
+  %span{ class: 'avatar-counter has-tooltip', data: { container: 'body', placement: 'bottom', 'line-type' => 'old', 'original-title' => "+#{approvers_over_presentable_limit} more approvers", qa_selector: 'avatar_counter' } }
+    = "+ #{approvers_over_presentable_limit}"

ee/app/views/groups/security/compliance_dashboards/_empty_state.html.haml

+.row.empty-state.merge-requests
+  .col-12
+    .svg-content
+      = image_tag 'illustrations/merge_requests.svg'
+  .col-12
+    .text-content
+      %h4
+        = _("Merge requests are a place to propose changes you've made to a project and discuss those changes with others")
+      %p
+        = _("Interested parties can even contribute by pushing commits if they want to.")

ee/app/views/groups/security/compliance_dashboards/_merge_request.html.haml

+%li{ id: dom_id(merge_request), data: { id: merge_request.id } }
+  .issuable-info-container
+    .issuable-main-info
+      .merge-request-title.title
+        %span.merge-request-title-text.js-onboarding-mr-item
+          = link_to merge_request.title, merge_request_path(merge_request)
+
+      .issuable-info
+        %span.issuable-reference
+          = issuable_reference(merge_request)
+
+    .issuable-meta
+      %ul.controls.d-flex.align-items-end
+        - if merge_request.approved_by_users.any?
+          %li.d-flex
+            = render 'approvers', merge_request: merge_request
+      %span
+        = _('merged %{time_ago}').html_safe % { time_ago: time_ago_with_tooltip(merge_request.merged_at, placement: 'bottom', html_class: 'merge_request_updated_ago') }

ee/app/views/groups/security/compliance_dashboards/_merge_requests.html.haml

+- if @merge_requests.present?
+  .card.card-small.card-without-border
+    %ul.content-list.mr-list.issuable-list
+      = render partial: 'merge_request', collection: @merge_requests
+
+  = paginate @merge_requests
+
+- else
+  = render 'empty_state'

ee/app/views/groups/security/compliance_dashboards/show.html.haml

+- breadcrumb_title _("Compliance Dashboard")
+- page_title _("Compliance Dashboard")
+
+= render "merge_requests"

ee/app/views/layouts/nav/ee/_security_link.html.haml

-- if @group.feature_available?(:security_dashboard)
-  = nav_link(path: 'groups/security/dashboard#show') do
-    = link_to group_security_dashboard_path(@group), data: { qa_selector: 'security_dashboard_link' } do
+- return unless @group.feature_available?(:group_level_compliance_dashboard) || @group.feature_available?(:security_dashboard)
+
+- main_path = @group.feature_available?(:group_level_compliance_dashboard) ? group_security_compliance_dashboard_path(@group) : group_security_dashboard_path(@group)
+= nav_link(path: %w[groups/security/compliance_dashboard#show groups/security/dashboard#show]) do
+  = link_to main_path, data: { qa_selector: 'security_dashboard_link' } do
     .nav-icon-container
       = sprite_icon('shield')
     %span.nav-item-name
-        = _('Security')
-    %ul.sidebar-sub-level-items.is-fly-out-only
+      = _('Security & Compliance')
+  %ul.sidebar-sub-level-items
+    - if @group.feature_available?(:security_dashboard)
       = nav_link(path: 'groups/security/dashboard#show') do
         = link_to group_security_dashboard_path(@group), title: _('Security') do
-          %strong.fly-out-top-item-name
-            = _('Security')
+          %span= _('Security')
+
+    - if @group.feature_available?(:group_level_compliance_dashboard)
+      = nav_link(path: 'groups/security/compliance_dashboard#show') do
+        = link_to group_security_compliance_dashboard_path(@group), title: _('Compliance') do
+          %span= _('Compliance')

ee/changelogs/unreleased/36524-group-level-compliance-dashboard-mvc.yml

+---
+title: Add Group-level compliance dashboard MVC
+merge_request: 20844
+author:
+type: added

ee/config/routes/group.rb

@@ -113,6 +113,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
 
     namespace :security do
       resource :dashboard, only: [:show], controller: :dashboard
+      resource :compliance_dashboard, only: [:show]
       resources :vulnerable_projects, only: [:index]
 
       resources :vulnerability_findings, only: [:index] do

ee/spec/controllers/groups/security/compliance_dashboards_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::Security::ComplianceDashboardsController do
+  let(:user) { create(:user) }
+  let(:group) { create(:group) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'GET show' do
+    subject { get :show, params: { group_id: group.to_param } }
+
+    context 'when compliance dashboard feature is enabled' do
+      before do
+        stub_licensed_features(group_level_compliance_dashboard: true)
+      end
+
+      context 'and user is allowed to access group compliance dashboard' do
+        before do
+          group.add_owner(user)
+        end
+
+        it { is_expected.to have_gitlab_http_status(:success) }
+
+        context 'when there are no merge requests' do
+          it 'does not receive merge request collection' do
+            subject
+            expect(assigns(:merge_requests)).to be_empty
+          end
+        end
+
+        context 'when there are merge requests' do
+          let(:project) { create(:project, namespace: group) }
+
+          let(:mr_1) { create(:merge_request, source_project: project, state: :merged) }
+          let(:mr_2) { create(:merge_request, source_project: project, source_branch: 'A', state: :merged) }
+
+          before do
+            create(:event, :merged, project: project, target: mr_1, author: user)
+          end
+
+          it 'receives merge requests collection' do
+            subject
+            expect(assigns(:merge_requests)).not_to be_empty
+          end
+        end
+      end
+
+      context 'when user is not allowed to access group compliance dashboard' do
+        it { is_expected.to have_gitlab_http_status(:not_found) }
+      end
+    end
+
+    context 'when compliance dashboard feature is disabled' do
+      it { is_expected.to have_gitlab_http_status(:not_found) }
+    end
+  end
+end

ee/spec/finders/merge_requests_compliance_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe MergeRequestsComplianceFinder do
+  subject { described_class.new(current_user, search_params) }
+
+  let_it_be(:current_user) { create(:admin) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, namespace: group) }
+  let_it_be(:project_2) { create(:project, namespace: group) }
+  let_it_be(:search_params) { { group_id: group.id } }
+
+  let(:mr_1) { create(:merge_request, source_project: project, state: :merged) }
+  let(:mr_2) { create(:merge_request, source_project: project_2, state: :merged) }
+  let(:mr_3) { create(:merge_request, source_project: project, source_branch: 'A', state: :merged) }
+  let(:mr_4) { create(:merge_request, source_project: project_2, source_branch: 'A', state: :merged) }
+
+  before do
+    create(:event, :merged, project: project_2, target: mr_4, author: current_user, created_at: 50.minutes.ago)
+    create(:event, :merged, project: project_2, target: mr_2, author: current_user, created_at: 40.minutes.ago)
+    create(:event, :merged, project: project, target: mr_3, author: current_user, created_at: 30.minutes.ago)
+    create(:event, :merged, project: project, target: mr_1, author: current_user, created_at: 20.minutes.ago)
+  end
+
+  context 'when there are merge requests from projects in group' do
+    it 'shows only most recent Merge Request from each project' do
+      expect(subject.execute).to contain_exactly(mr_1, mr_2)
+    end
+
+    context 'when there are merge requests from projects in group and subgroups' do
+      let(:subgroup) { create(:group, parent: group) }
+      let(:sub_project) { create(:project, namespace: subgroup) }
+
+      let(:mr_5) { create(:merge_request, source_project: sub_project, state: :merged) }
+      let(:mr_6) { create(:merge_request, source_project: sub_project, state: :merged) }
+
+      before do
+        create(:event, :merged, project: sub_project, target: mr_6, author: current_user, created_at: 30.minutes.ago)
+        create(:event, :merged, project: sub_project, target: mr_5, author: current_user, created_at: 10.minutes.ago)
+      end
+
+      it 'shows Merge Requests from most recent to least recent' do
+        expect(subject.execute).to eq([mr_5, mr_1, mr_2])
+      end
+    end
+  end
+end

ee/spec/views/groups/security/compliance_dashboards/show.html.haml_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'groups/security/compliance_dashboards/show.html.haml' do
+  set(:user) { create(:user) }
+  let(:group) { create(:group) }
+
+  before do
+    group.add_owner(user)
+
+    assign(:group, group)
+    allow(view).to receive(:current_user) { user }
+  end
+
+  it 'shows empty state if there are no merge requests' do
+    render
+
+    expect(rendered).to have_css("div.empty-state")
+  end
+
+  context 'when there are merge requests' do
+    let(:mr) { create(:merge_request, :merged) }
+
+    before do
+      mr.metrics.update!(merged_at: 1.day.ago)
+      assign(:merge_requests, Kaminari.paginate_array([mr]).page(1))
+    end
+
+    it 'shows merge requests' do
+      render
+
+      expect(rendered).to have_css(".merge-request-title.title")
+    end
+  end
+end

ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb

@@ -70,30 +70,47 @@ describe 'layouts/nav/sidebar/_group' do
   end
 
   describe 'security dashboard tab' do
+    let(:group) { create(:group, plan: :gold_plan) }
+
     before do
-      stub_licensed_features(security_dashboard: true)
       enable_namespace_license_check!
 
       create(:gitlab_subscription, hosted_plan: group.plan, namespace: group)
     end
 
     context 'when security dashboard feature is enabled' do
-      let(:group) { create(:group, plan: :gold_plan) }
+      before do
+        stub_licensed_features(security_dashboard: true)
+      end
 
       it 'is visible' do
         render
 
+        expect(rendered).to have_link 'Security & Compliance'
         expect(rendered).to have_link 'Security'
       end
     end
 
+    context 'when compliance dashboard feature is enabled' do
+      before do
+        stub_licensed_features(group_level_compliance_dashboard: true)
+      end
+
+      it 'is visible' do
+        render
+
+        expect(rendered).to have_link 'Security & Compliance'
+        expect(rendered).to have_link 'Compliance'
+      end
+    end
+
     context 'when security dashboard feature is disabled' do
       let(:group) { create(:group, plan: :bronze_plan) }
 
       it 'is not visible' do
         render
 
-        expect(rendered).not_to have_link 'Security'
+        expect(rendered).not_to have_link 'Security & Compliance'
       end
     end
   end

locale/gitlab.pot

@@ -2098,6 +2098,9 @@ msgstr ""
 msgid "Approve the current merge request."
 msgstr ""
 
+msgid "Approved by: "
+msgstr ""
+
 msgid "Approved the current merge request."
 msgstr ""
 
@@ -4897,6 +4900,12 @@ msgstr ""
 msgid "Complete"
 msgstr ""
 
+msgid "Compliance"
+msgstr ""
+
+msgid "Compliance Dashboard"
+msgstr ""
+
 msgid "Confidence: %{confidence}"
 msgstr ""
 
@@ -22490,6 +22499,9 @@ msgid_plural "merge requests"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "merged %{time_ago}"
+msgstr ""
+
 msgid "milestone should belong either to a project or a group."
 msgstr ""