Add cr remarks

Refactor specs

And add views specs

Remove obsolete lines

Add cr remarks

Add cr remarks

Fix licence file

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

@@ -5,9 +5,7 @@ class Groups::Security::ComplianceDashboardsController < Groups::ApplicationCont
   before_action :authorize_compliance_dashboard!
 
   def show
-    preload_for_collection = [:approved_by_users, :metrics, source_project: :route, target_project: :namespace]
-
-    @merge_requests = MergeRequestsComplianceFinder.new(current_user, { group_id: @group.id, preloads: preload_for_collection })
+    @merge_requests = MergeRequestsComplianceFinder.new(current_user, { group_id: @group.id })
       .execute
     @merge_requests = Kaminari.paginate_array(@merge_requests).page(params[:page])
   end

ee/app/finders/merge_requests_compliance_finder.rb

 # frozen_string_literal: true
-
-# Finders::MergeRequest class
 #
 # Used to filter MergeRequests collections for compliance dashboard
 #
 # Arguments:
-#   current_user - which user use
+#   current_user - which user calls a class
 #   params:
 #     group_id: integer
 #     preloads: array of associations to preload
@@ -22,7 +20,10 @@ class MergeRequestsComplianceFinder < MergeRequestsFinder
       .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')
+    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
@@ -40,11 +41,15 @@ class MergeRequestsComplianceFinder < MergeRequestsFinder
   def select_sorted_mrs(records)
     hash = {}
     records.each { |row| hash[row['merge_request_id']] = nil }
-    mrs = MergeRequest.where(id: hash.keys).preload(params[:preloads]) # rubocop: disable CodeReuse/ActiveRecord
+    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/ee/merge_request.rb

@@ -73,7 +73,6 @@ module EE
       def with_api_entity_associations
         super.preload(:blocking_merge_requests)
       end
-<<<<<<< HEAD
 
       def sort_by_attribute(method, *args)
         if method.to_s == 'review_time_desc'
@@ -92,8 +91,6 @@ module EE
         grouping_columns << ::MergeRequest::Metrics.arel_table[:first_comment_at] if sort.to_s == 'review_time_desc'
         grouping_columns
       end
-=======
->>>>>>> Change query in finder
     end
 
     override :mergeable?

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

@@ -7,7 +7,7 @@
 
       .issuable-info
         %span.issuable-reference
-          #{issuable_reference(merge_request)}
+          = issuable_reference(merge_request)
 
     .issuable-meta
       %ul.controls.d-flex.align-items-end

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

 - return unless @group.feature_available?(:group_level_compliance_dashboard) || @group.feature_available?(:security_dashboard)
 
-- main_path = @group.feature_available?(:group_level_compliance_dashboard) ? 'groups/security/compliance_dashboards#show' : 'groups/security/dashboard#show'
-= nav_link(path: main_path) do
-  = link_to group_security_compliance_dashboard_path(@group), data: { qa_selector: 'security_dashboard_link' } do
+- 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

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

@@ -19,18 +19,16 @@ describe Groups::Security::ComplianceDashboardsController do
       end
 
       context 'and user is allowed to access group compliance dashboard' do
-        render_views
-
         before do
           group.add_owner(user)
         end
 
-        it { is_expected.to have_gitlab_http_status(200) }
+        it { is_expected.to have_gitlab_http_status(:success) }
 
         context 'when there are no merge requests' do
-          it 'renders empty state' do
+          it 'does not receive merge request collection' do
             subject
-            expect(response.body).to have_css("div.empty-state")
+            expect(assigns(:merge_requests)).to be_empty
           end
         end
 
@@ -44,20 +42,20 @@ describe Groups::Security::ComplianceDashboardsController do
             create(:event, :merged, project: project, target: mr_1, author: user)
           end
 
-          it 'renders merge request' do
+          it 'receives merge requests collection' do
             subject
-            expect(response.body).to have_css(".merge-request-title.title")
+            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(404) }
+        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(404) }
+      it { is_expected.to have_gitlab_http_status(:not_found) }
     end
   end
 end

ee/spec/finders/merge_requests_compliance_finder_spec.rb

@@ -5,11 +5,11 @@ require 'spec_helper'
 describe MergeRequestsComplianceFinder do
   subject { described_class.new(current_user, search_params) }
 
-  let(:current_user) { create(:admin) }
-  let(:search_params) { { group_id: group.id } }
-  let(:group) { create(:group) }
-  let(:project) { create(:project, namespace: group) }
-  let(:project_2) { create(:project, namespace: group) }
+  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) }
@@ -17,10 +17,10 @@ describe MergeRequestsComplianceFinder do
   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, id: 1)
-    create(:event, :merged, project: project_2, target: mr_2, author: current_user, created_at: 40.minutes.ago, id: 2)
-    create(:event, :merged, project: project, target: mr_3, author: current_user, created_at: 30.minutes.ago, id: 3)
-    create(:event, :merged, project: project, target: mr_1, author: current_user, created_at: 20.minutes.ago, id: 4)
+    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
@@ -28,10 +28,6 @@ describe MergeRequestsComplianceFinder do
       expect(subject.execute).to contain_exactly(mr_1, mr_2)
     end
 
-    it 'shows as many Merge Requests as they are projects with MR in group' do
-      expect(subject.execute.size).to eq(group.projects.size)
-    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) }
@@ -40,12 +36,8 @@ describe MergeRequestsComplianceFinder do
       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, id: 6)
-        create(:event, :merged, project: sub_project, target: mr_5, author: current_user, created_at: 10.minutes.ago, id: 7)
-      end
-
-      it 'shows only most recent Merge Request from each project' do
-        expect(subject.execute).to contain_exactly(mr_1, mr_2, mr_5)
+        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

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