Merge branch '346266-feature-flag-enable-compliance-violations-report' into 'master'

[Feature flag] Enable compliance violations report

See merge request gitlab-org/gitlab!83959

ee/app/assets/javascripts/approvals/components/group_settings/app.vue

@@ -28,7 +28,7 @@ export default {
   links: {
     groupSettingsDocsPath: helpPagePath('user/project/merge_requests/approvals/index.md'),
     separationOfDutiesDocsPath: helpPagePath('user/compliance/compliance_report/index', {
-      anchor: 'approval-status-and-separation-of-duties',
+      anchor: 'separation-of-duties',
     }),
   },
   i18n: {

ee/app/assets/javascripts/compliance_dashboard/compliance_dashboard_bundle.js

 import Vue from 'vue';
 import VueApollo from 'vue-apollo';
 import createDefaultClient from '~/lib/graphql';
-import { parseBoolean } from '~/lib/utils/common_utils';
-import ComplianceDashboard from './components/dashboard.vue';
 import ComplianceReport from './components/report.vue';
 import { buildDefaultFilterParams } from './utils';
 
 export default () => {
   const el = document.getElementById('js-compliance-report');
 
-  const {
-    mergeRequests,
-    emptyStateSvgPath,
-    isLastPage,
-    mergeCommitsCsvExportPath,
-    groupPath,
-  } = el.dataset;
+  const { mergeCommitsCsvExportPath, groupPath } = el.dataset;
 
-  if (gon.features.complianceViolationsReport) {
-    Vue.use(VueApollo);
+  Vue.use(VueApollo);
 
-    const apolloProvider = new VueApollo({
-      defaultClient: createDefaultClient(),
-    });
-
-    const defaultFilterParams = buildDefaultFilterParams(window.location.search);
+  const apolloProvider = new VueApollo({
+    defaultClient: createDefaultClient(),
+  });
 
-    return new Vue({
-      el,
-      apolloProvider,
-      render: (createElement) =>
-        createElement(ComplianceReport, {
-          props: {
-            mergeCommitsCsvExportPath,
-            groupPath,
-            defaultFilterParams,
-          },
-        }),
-    });
-  }
+  const defaultFilterParams = buildDefaultFilterParams(window.location.search);
 
   return new Vue({
     el,
+    apolloProvider,
     render: (createElement) =>
-      createElement(ComplianceDashboard, {
+      createElement(ComplianceReport, {
         props: {
-          mergeRequests: JSON.parse(mergeRequests),
-          isLastPage: parseBoolean(isLastPage),
-          emptyStateSvgPath,
           mergeCommitsCsvExportPath,
+          groupPath,
+          defaultFilterParams,
         },
       }),
   });

ee/app/assets/javascripts/compliance_dashboard/components/dashboard.vue

@@ -8,7 +8,7 @@ import { mapDashboardToDrawerData } from '../utils';
 import MergeRequestDrawer from './drawer.vue';
 import EmptyState from './empty_state.vue';
 import MergeRequestsGrid from './merge_requests/grid.vue';
-import MergeCommitsExportButton from './merge_requests/merge_commits_export_button.vue';
+import MergeCommitsExportButton from './shared/merge_commits_export_button.vue';
 
 export default {
   name: 'ComplianceDashboard',

ee/app/assets/javascripts/compliance_dashboard/components/merge_requests/statuses/approval.vue

 <script>
 import { GlLink, GlTooltipDirective } from '@gitlab/ui';
-
+import { helpPagePath } from '~/helpers/help_page_helper';
 import { s__ } from '~/locale';
 import CiIcon from '~/vue_shared/components/ci_icon.vue';
 
@@ -40,13 +40,14 @@ export default {
     warning: s__('ApprovalStatusTooltip|At least one rule does not adhere to separation of duties'),
     failed: s__('ApprovalStatusTooltip|Fails to adhere to separation of duties'),
   },
+  docLink: helpPagePath('user/compliance/compliance_report/index', {
+    anchor: 'separation-of-duties',
+  }),
 };
 </script>
 
 <template>
-  <gl-link
-    href="https://docs.gitlab.com/ee/user/compliance/compliance_report/#approval-status-and-separation-of-duties"
-  >
+  <gl-link :href="$options.docLink">
     <ci-icon v-gl-tooltip.left="tooltip" class="gl-display-flex" :status="{ icon, group }" />
   </gl-link>
 </template>

ee/app/assets/javascripts/compliance_dashboard/components/report.vue

@@ -12,7 +12,7 @@ import getComplianceViolationsQuery from '../graphql/compliance_violations.query
 import { mapViolations } from '../graphql/mappers';
 import { DEFAULT_SORT, GRAPHQL_PAGE_SIZE, DEFAULT_PAGINATION_CURSORS } from '../constants';
 import { parseViolationsQueryFilter } from '../utils';
-import MergeCommitsExportButton from './merge_requests/merge_commits_export_button.vue';
+import MergeCommitsExportButton from './shared/merge_commits_export_button.vue';
 import MergeRequestDrawer from './drawer.vue';
 import ViolationReason from './violations/reason.vue';
 import ViolationFilter from './violations/filter.vue';
@@ -209,9 +209,7 @@ export default {
     next: __('Next'),
     viewDetailsBtn: __('View details'),
   },
-  documentationPath: helpPagePath('user/compliance/compliance_report/index.md', {
-    anchor: 'approval-status-and-separation-of-duties',
-  }),
+  documentationPath: helpPagePath('user/compliance/compliance_report/index.md'),
   DRAWER_Z_INDEX,
 };
 </script>

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

@@ -6,9 +6,6 @@ class Groups::Security::ComplianceDashboardsController < Groups::ApplicationCont
   layout 'group'
 
   before_action :authorize_compliance_dashboard!
-  before_action do
-    push_frontend_feature_flag(:compliance_violations_report, @group, type: :development, default_enabled: :yaml)
-  end
 
   track_redis_hll_event :show, name: 'g_compliance_dashboard'
 

ee/app/models/concerns/enums/merge_requests/compliance_violation.rb

@@ -4,7 +4,7 @@ module Enums
   module MergeRequests
     module ComplianceViolation
       # Reasons are defined by GitLab in our public documentation.
-      # https://docs.gitlab.com/ee/user/compliance/compliance_dashboard/#approval-status-and-separation-of-duties
+      # https://docs.gitlab.com/ee/user/compliance/compliance_dashboard/#separation-of-duties
       def self.reasons
         {
           ::Gitlab::ComplianceManagement::Violations::ApprovedByMergeRequestAuthor::REASON => 0,

ee/app/views/admin/push_rules/_merge_request_approvals.html.haml

@@ -6,7 +6,7 @@
     %button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' }
       = expanded_by_default? ? _('Collapse') : _('Expand')
     %p
-      - duties_link_url = help_page_path('user/compliance/compliance_report/index', anchor: 'approval-status-and-separation-of-duties')
+      - duties_link_url = help_page_path('user/compliance/compliance_report/index', anchor: 'separation-of-duties')
       - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: duties_link_url }
       = s_('MergeRequestApprovals|Enforce %{link_start}separation of duties%{link_end} for all projects.').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
       = link_to _("Learn more."), help_page_path("user/project/merge_requests/approvals/index.md"), target: '_blank', rel: 'noopener noreferrer'

ee/app/views/projects/_merge_request_approvals_settings.html.haml

@@ -6,7 +6,7 @@
     %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Merge request approvals')
     %button.gl-button.btn.btn-default.js-settings-toggle{ type: 'button' }= expanded ? _("Collapse") : _("Expand")
     %p
-      - duties_link_url = help_page_path('user/compliance/compliance_report/index', anchor: 'approval-status-and-separation-of-duties')
+      - duties_link_url = help_page_path('user/compliance/compliance_report/index', anchor: 'separation-of-duties')
       - link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: duties_link_url }
       = s_('MergeRequestApprovals|Define approval rules and settings to ensure %{link_start}separation of duties%{link_end} for new merge requests.').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
       = link_to _("Learn more."), help_page_path("user/project/merge_requests/approvals/index.md"), target: '_blank', rel: 'noopener noreferrer'

ee/config/feature_flags/development/compliance_violations_report.yml

----
-name: compliance_violations_report
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75015
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346266
-milestone: '14.6'
-type: development
-group: group::compliance
-default_enabled: false

ee/lib/gitlab/compliance_management/violations/approved_by_insufficient_users.rb

@@ -8,7 +8,7 @@ module Gitlab
         SEVERITY_LEVEL = :high
 
         # The minimum number of approvers is defined by GitLab in our public documentation.
-        # https://docs.gitlab.com/ee/user/compliance/compliance_dashboard/#approval-status-and-separation-of-duties
+        # https://docs.gitlab.com/ee/user/compliance/compliance_dashboard/#separation-of-duties
         MINIMUM_NUMBER_OF_APPROVERS = 2
 
         def initialize(merge_request)

ee/spec/features/groups/security/compliance_dashboards_spec.rb

@@ -36,109 +36,70 @@ RSpec.describe 'Compliance Dashboard', :js do
     stub_licensed_features(group_level_compliance_dashboard: true)
     group.add_owner(user)
     sign_in(user)
+    visit group_security_compliance_dashboard_path(group)
   end
 
-  context 'when compliance_violations_report feature is disabled' do
-    before do
-      stub_feature_flags(compliance_violations_report: false)
-      visit group_security_compliance_dashboard_path(group)
+  it 'shows the violations report table', :aggregate_failures do
+    page.within('table') do
+      expect(page).to have_content 'Severity'
+      expect(page).to have_content 'Violation'
+      expect(page).to have_content 'Merge request'
+      expect(page).to have_content 'Date merged'
     end
+  end
 
-    context 'when there are no merge requests' do
-      it 'shows an empty state' do
-        expect(page).to have_selector('.empty-state')
-      end
-    end
-
-    context 'when there are merge requests' do
-      let_it_be(:merge_request) { create(:merge_request, source_project: project, state: :merged, merge_commit_sha: 'b71a6483b96dc303b66fdcaa212d9db6b10591ce') }
-      let_it_be(:merge_request_2) { create(:merge_request, source_project: project_2, state: :merged, merge_commit_sha: '24327319d067f4101cd3edd36d023ab5e49a8579') }
-
-      before_all do
-        create(:event, :merged, project: project, target: merge_request, author: user, created_at: 10.minutes.ago)
-        create(:event, :merged, project: project_2, target: merge_request_2, author: user, created_at: 15.minutes.ago)
-      end
-
-      it 'shows merge requests with details' do
-        expect(page).to have_link(merge_request.title)
-        expect(page).to have_content('merged 10 minutes ago')
-        expect(page).to have_content('no approvers')
-      end
-
-      context 'chain of custody report' do
-        it_behaves_like 'exports a merge commit-specific CSV'
-      end
+  context 'when there are no compliance violations' do
+    it 'shows an empty state' do
+      expect(page).to have_content('No violations found')
     end
   end
 
-  context 'when compliance_violations_report feature is enabled' do
-    before do
-      stub_feature_flags(compliance_violations_report: true)
-      visit group_security_compliance_dashboard_path(group)
-    end
+  context 'when there are merge requests' do
+    let_it_be(:merge_request) { create(:merge_request, source_project: project, state: :merged, merge_commit_sha: 'b71a6483b96dc303b66fdcaa212d9db6b10591ce') }
+    let_it_be(:merge_request_2) { create(:merge_request, source_project: project_2, state: :merged, merge_commit_sha: '24327319d067f4101cd3edd36d023ab5e49a8579') }
 
-    it 'shows the violations report table', :aggregate_failures do
-      page.within('table') do
-        expect(page).to have_content 'Severity'
-        expect(page).to have_content 'Violation'
-        expect(page).to have_content 'Merge request'
-        expect(page).to have_content 'Date merged'
-      end
+    context 'chain of custody report' do
+      it_behaves_like 'exports a merge commit-specific CSV'
     end
 
-    context 'when there are no compliance violations' do
-      it 'shows an empty state' do
-        expect(page).to have_content('No violations found')
-      end
-    end
-
-    context 'when there are merge requests' do
-      let_it_be(:merge_request) { create(:merge_request, source_project: project, state: :merged, merge_commit_sha: 'b71a6483b96dc303b66fdcaa212d9db6b10591ce') }
-      let_it_be(:merge_request_2) { create(:merge_request, source_project: project_2, state: :merged, merge_commit_sha: '24327319d067f4101cd3edd36d023ab5e49a8579') }
+    context 'and there is a compliance violation' do
+      let_it_be(:violation) { create(:compliance_violation, :approved_by_committer, severity_level: :high, merge_request: merge_request, violating_user: user) }
+      let_it_be(:violation_2) { create(:compliance_violation, :approved_by_merge_request_author, severity_level: :medium, merge_request: merge_request_2, violating_user: user) }
 
-      context 'chain of custody report' do
-        it_behaves_like 'exports a merge commit-specific CSV'
+      before do
+        merge_request.metrics.update!(merged_at: 1.day.ago)
+        merge_request_2.metrics.update!(merged_at: 7.days.ago)
+        wait_for_requests
       end
 
-      context 'and there is a compliance violation' do
-        let_it_be(:violation) { create(:compliance_violation, :approved_by_committer, severity_level: :high, merge_request: merge_request, violating_user: user) }
-        let_it_be(:violation_2) { create(:compliance_violation, :approved_by_merge_request_author, severity_level: :medium, merge_request: merge_request_2, violating_user: user) }
+      it 'shows the compliance violations with details', :aggregate_failures do
+        expect(all('tbody > tr').count).to eq(2)
 
-        before do
-          merge_request.metrics.update!(merged_at: 1.day.ago)
-          merge_request_2.metrics.update!(merged_at: 7.days.ago)
-          wait_for_requests
-        end
+        expect(first_row).to have_content('High')
+        expect(first_row).to have_content('Approved by committer')
+        expect(first_row).to have_content(merge_request.title)
+        expect(first_row).to have_content('1 day ago')
+      end
 
-        it 'shows the compliance violations with details', :aggregate_failures do
-          expect(all('tbody > tr').count).to eq(2)
+      it 'can sort the violations by clicking on a column header' do
+        click_column_header 'Severity'
 
-          expect(first_row).to have_content('High')
-          expect(first_row).to have_content('Approved by committer')
-          expect(first_row).to have_content(merge_request.title)
-          expect(first_row).to have_content('1 day ago')
-        end
+        expect(first_row).to have_content(merge_request_2.title)
+      end
 
-        it 'can sort the violations by clicking on a column header' do
-          click_column_header 'Severity'
+      context 'violations filter' do
+        it 'can filter by date range' do
+          set_date_range(7.days.ago.to_date, 6.days.ago.to_date)
 
-          expect(first_row).to have_content(merge_request_2.title)
+          expect(page).to have_content(merge_request_2.title)
+          expect(page).not_to have_content(merge_request.title)
         end
 
-        context 'violations filter' do
-          it 'can filter by date range' do
-            set_date_range(7.days.ago.to_date, 6.days.ago.to_date)
-
-            expect(page).to have_content(merge_request_2.title)
-            expect(page).not_to have_content(merge_request.title)
-          end
+        it 'can filter by project id' do
+          filter_by_project(merge_request_2.project)
 
-          it 'can filter by project id' do
-            filter_by_project(merge_request_2.project)
-
-            expect(page).to have_content(merge_request_2.title)
-            expect(page).not_to have_content(merge_request.title)
-          end
+          expect(page).to have_content(merge_request_2.title)
+          expect(page).not_to have_content(merge_request.title)
         end
       end
     end

ee/spec/frontend/approvals/components/group_settings/app_spec.js

@@ -70,7 +70,7 @@ describe('EE Approvals Group Settings App', () => {
 
   it.each`
     findComponent          | text                      | href
-    ${findDescriptionLink} | ${'separation of duties'} | ${'/help/user/compliance/compliance_report/index#approval-status-and-separation-of-duties'}
+    ${findDescriptionLink} | ${'separation of duties'} | ${'/help/user/compliance/compliance_report/index#separation-of-duties'}
     ${findLearnMoreLink}   | ${'Learn more.'}          | ${'/help/user/project/merge_requests/approvals/index.md'}
   `('has the correct link for $text', ({ findComponent, text, href }) => {
     createWrapper();

ee/spec/frontend/compliance_dashboard/components/dashboard_spec.js

@@ -6,7 +6,7 @@ import { nextTick } from 'vue';
 import ComplianceDashboard from 'ee/compliance_dashboard/components/dashboard.vue';
 import MergeRequestDrawer from 'ee/compliance_dashboard/components/drawer.vue';
 import MergeRequestGrid from 'ee/compliance_dashboard/components/merge_requests/grid.vue';
-import MergeCommitsExportButton from 'ee/compliance_dashboard/components/merge_requests/merge_commits_export_button.vue';
+import MergeCommitsExportButton from 'ee/compliance_dashboard/components/shared/merge_commits_export_button.vue';
 import { COMPLIANCE_TAB_COOKIE_KEY } from 'ee/compliance_dashboard/constants';
 import { mapDashboardToDrawerData } from 'ee/compliance_dashboard/utils';
 import { createMergeRequests } from '../mock_data';

ee/spec/frontend/compliance_dashboard/components/merge_requests/statuses/approval_spec.js

@@ -33,8 +33,8 @@ describe('ApprovalStatus component', () => {
     });
 
     it('links to the approval status', () => {
-      expect(findLink().attributes('href')).toEqual(
-        'https://docs.gitlab.com/ee/user/compliance/compliance_report/#approval-status-and-separation-of-duties',
+      expect(findLink().attributes('href')).toBe(
+        '/help/user/compliance/compliance_report/index#separation-of-duties',
       );
     });
 

ee/spec/frontend/compliance_dashboard/components/report_spec.js

@@ -6,7 +6,7 @@ import * as Sentry from '@sentry/browser';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import ComplianceReport from 'ee/compliance_dashboard/components/report.vue';
 import MergeRequestDrawer from 'ee/compliance_dashboard/components/drawer.vue';
-import MergeCommitsExportButton from 'ee/compliance_dashboard/components/merge_requests/merge_commits_export_button.vue';
+import MergeCommitsExportButton from 'ee/compliance_dashboard/components/shared/merge_commits_export_button.vue';
 import ViolationReason from 'ee/compliance_dashboard/components/violations/reason.vue';
 import ViolationFilter from 'ee/compliance_dashboard/components/violations/filter.vue';
 import getComplianceViolationsQuery from 'ee/compliance_dashboard/graphql/compliance_violations.query.graphql';
@@ -125,9 +125,7 @@ describe('ComplianceReport component', () => {
         'The compliance report shows the merge request violations merged in protected environments.',
       );
       expect(helpLink.text()).toBe('Learn more.');
-      expect(helpLink.attributes('href')).toBe(
-        '/help/user/compliance/compliance_report/index.md#approval-status-and-separation-of-duties',
-      );
+      expect(helpLink.attributes('href')).toBe('/help/user/compliance/compliance_report/index.md');
     });
 
     it('renders the merge commit export button', () => {

ee/spec/frontend/compliance_dashboard/components/merge_requests/merge_commits_export_button_spec.js → ee/spec/frontend/compliance_dashboard/components/shared/merge_commits_export_button_spec.js

 import { GlFormInput, GlForm, GlFormGroup } from '@gitlab/ui';
 import { mount, shallowMount } from '@vue/test-utils';
 
-import MergeCommitsExportButton from 'ee/compliance_dashboard/components/merge_requests/merge_commits_export_button.vue';
+import MergeCommitsExportButton from 'ee/compliance_dashboard/components/shared/merge_commits_export_button.vue';
 import { INPUT_DEBOUNCE, CUSTODY_REPORT_PARAMETER } from 'ee/compliance_dashboard/constants';
 
 const CSV_EXPORT_PATH = '/merge_commit_reports';

ee/spec/frontend/compliance_dashboard/graphql/mappers_spec.js

@@ -4,11 +4,19 @@ import { createComplianceViolation } from '../mock_data';
 
 describe('mapViolations', () => {
   it('returns the expected result', () => {
-    const { mergeRequest } = mapViolations([createComplianceViolation()])[0];
+    const violation = createComplianceViolation();
+    const { mergeRequest } = mapViolations([violation])[0];
 
     expect(mergeRequest).toMatchObject({
+      committers: [],
+      approvedByUsers: [],
+      participants: violation.mergeRequest.participants.nodes,
       reference: mergeRequest.ref,
       mergedBy: convertObjectPropsToSnakeCase(mergeRequest.mergeUser),
+      project: {
+        ...violation.project,
+        complianceFramework: violation.mergeRequest.project?.complianceFrameworks?.nodes[0],
+      },
     });
   });
 });