From 6128a2fb8f5d32a9bcc86789a81bc9e5b97b0052 Mon Sep 17 00:00:00 2001 From: Jay Montal Date: Thu, 1 Jun 2023 12:45:44 -0600 Subject: [PATCH 1/4] Add standards adherence tab to compliance report - Adds a new feature flag :adherence_report_ui - Adds new tab on the compliance report page - Adds base component for new adherence report - Updates router based on feature flag Changelog: added EE: true --- .../development/adherence_report_ui.yml | 8 + .../compliance_dashboard_bundle.js | 3 + .../components/reports_app.vue | 13 +- .../standards_adherence_report/report.vue | 9 + .../compliance_dashboard/constants.js | 1 + .../compliance_dashboard/router.js | 19 +- .../compliance_dashboards/show.html.haml | 3 +- .../security/compliance_dashboards_spec.rb | 162 ++++++++++-------- .../components/reports_app_spec.js | 17 +- .../standards_adherence_report/report_spec.js | 23 +++ .../show.html.haml_spec.rb | 30 ++++ locale/gitlab.pot | 3 + 12 files changed, 217 insertions(+), 74 deletions(-) create mode 100644 config/feature_flags/development/adherence_report_ui.yml create mode 100644 ee/app/assets/javascripts/compliance_dashboard/components/standards_adherence_report/report.vue create mode 100644 ee/spec/frontend/compliance_dashboard/components/standards_adherence_report/report_spec.js create mode 100644 ee/spec/views/groups/security/compliance_dashboards/show.html.haml_spec.rb diff --git a/config/feature_flags/development/adherence_report_ui.yml b/config/feature_flags/development/adherence_report_ui.yml new file mode 100644 index 00000000000000..21c4920bddd635 --- /dev/null +++ b/config/feature_flags/development/adherence_report_ui.yml @@ -0,0 +1,8 @@ +--- +name: adherence_report_ui +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122374 +rollout_issue_url: +milestone: '16.1' +type: development +group: group::compliance +default_enabled: false diff --git a/ee/app/assets/javascripts/compliance_dashboard/compliance_dashboard_bundle.js b/ee/app/assets/javascripts/compliance_dashboard/compliance_dashboard_bundle.js index 904cd9ee4c02eb..384c670605a6e0 100644 --- a/ee/app/assets/javascripts/compliance_dashboard/compliance_dashboard_bundle.js +++ b/ee/app/assets/javascripts/compliance_dashboard/compliance_dashboard_bundle.js @@ -20,6 +20,7 @@ export default () => { rootAncestorPath, pipelineConfigurationFullPathEnabled, pipelineConfigurationEnabled, + adherenceReportUiEnabled, } = el.dataset; Vue.use(VueApollo); @@ -33,6 +34,7 @@ export default () => { mergeCommitsCsvExportPath, groupPath, rootAncestorPath, + adherenceReportUiEnabled: parseBoolean(adherenceReportUiEnabled), }); return new Vue({ @@ -45,6 +47,7 @@ export default () => { canAddEdit, pipelineConfigurationFullPathEnabled: parseBoolean(pipelineConfigurationFullPathEnabled), pipelineConfigurationEnabled: parseBoolean(pipelineConfigurationEnabled), + adherenceReportUiEnabled: parseBoolean(adherenceReportUiEnabled), }, render: (createElement) => createElement(ReportsApp, { diff --git a/ee/app/assets/javascripts/compliance_dashboard/components/reports_app.vue b/ee/app/assets/javascripts/compliance_dashboard/components/reports_app.vue index 6408cf6c727b26..6e5a3c331c46ec 100644 --- a/ee/app/assets/javascripts/compliance_dashboard/components/reports_app.vue +++ b/ee/app/assets/javascripts/compliance_dashboard/components/reports_app.vue @@ -4,7 +4,7 @@ import { GlTab, GlTabs, GlButton, GlTooltipDirective } from '@gitlab/ui'; import { __, s__ } from '~/locale'; import { helpPagePath } from '~/helpers/help_page_helper'; -import { ROUTE_FRAMEWORKS, ROUTE_VIOLATIONS, TABS } from '../constants'; +import { ROUTE_STANDARDS_ADHERENCE, ROUTE_FRAMEWORKS, ROUTE_VIOLATIONS, TABS } from '../constants'; import MergeCommitsExportButton from './violations_report/shared/merge_commits_export_button.vue'; import ReportHeader from './shared/report_header.vue'; @@ -20,6 +20,7 @@ export default { directives: { GlTooltip: GlTooltipDirective, }, + inject: ['adherenceReportUiEnabled'], props: { mergeCommitsCsvExportPath: { type: String, @@ -56,15 +57,17 @@ export default { } }, }, + ROUTE_STANDARDS: ROUTE_STANDARDS_ADHERENCE, ROUTE_VIOLATIONS, ROUTE_FRAMEWORKS, i18n: { - frameworksTab: s__('Compliance Report|Frameworks'), export: s__('Compliance Report|Export as CSV'), exportTitle: s__( 'Compliance Report|Export frameworks as CSV. You will be emailed after export is processed.', ), + frameworksTab: s__('Compliance Report|Frameworks'), heading: __('Compliance report'), + standardsAdherenceTab: s__('Compliance Report|Standards Adherence'), subheading: __('Compliance violations and compliance frameworks for the group.'), violationsTab: s__('Compliance Report|Violations'), }, @@ -98,6 +101,12 @@ export default { + +export default { + name: 'ComplianceStandardsAdherenceReport', +}; + + + diff --git a/ee/app/assets/javascripts/compliance_dashboard/constants.js b/ee/app/assets/javascripts/compliance_dashboard/constants.js index dd2db8e457d07b..1d28190099e750 100644 --- a/ee/app/assets/javascripts/compliance_dashboard/constants.js +++ b/ee/app/assets/javascripts/compliance_dashboard/constants.js @@ -37,6 +37,7 @@ export const FRAMEWORK_BADGE_SIZE_SM = 'sm'; export const FRAMEWORK_BADGE_SIZE_MD = 'md'; export const FRAMEWORK_BADGE_SIZES = [FRAMEWORK_BADGE_SIZE_SM, FRAMEWORK_BADGE_SIZE_MD]; +export const ROUTE_STANDARDS_ADHERENCE = 'standards_adherence'; export const ROUTE_VIOLATIONS = 'violations'; export const ROUTE_FRAMEWORKS = 'frameworks'; export const TABS = [ROUTE_VIOLATIONS, ROUTE_FRAMEWORKS]; diff --git a/ee/app/assets/javascripts/compliance_dashboard/router.js b/ee/app/assets/javascripts/compliance_dashboard/router.js index 0300092cc15b17..fc2faebb39ef1d 100644 --- a/ee/app/assets/javascripts/compliance_dashboard/router.js +++ b/ee/app/assets/javascripts/compliance_dashboard/router.js @@ -2,14 +2,27 @@ import VueRouter from 'vue-router'; import { joinPaths } from '~/lib/utils/url_utility'; -import { ROUTE_FRAMEWORKS, ROUTE_VIOLATIONS } from './constants'; +import { ROUTE_STANDARDS_ADHERENCE, ROUTE_FRAMEWORKS, ROUTE_VIOLATIONS } from './constants'; import ViolationsReport from './components/violations_report/report.vue'; import FrameworksReport from './components/frameworks_report/report.vue'; +import StandardsReport from './components/standards_adherence_report/report.vue'; export function createRouter(basePath, props) { - const { mergeCommitsCsvExportPath, groupPath, rootAncestorPath } = props; + const { + adherenceReportUiEnabled, + mergeCommitsCsvExportPath, + groupPath, + rootAncestorPath, + } = props; + + const defaultRoute = adherenceReportUiEnabled ? ROUTE_STANDARDS_ADHERENCE : ROUTE_VIOLATIONS; const routes = [ + { + path: '/standards_adherence', + name: ROUTE_STANDARDS_ADHERENCE, + component: StandardsReport, + }, { path: '/violations', name: ROUTE_VIOLATIONS, @@ -28,7 +41,7 @@ export function createRouter(basePath, props) { rootAncestorPath, }, }, - { path: '*', redirect: { name: ROUTE_VIOLATIONS } }, + { path: '*', redirect: { name: defaultRoute } }, ]; return new VueRouter({ diff --git a/ee/app/views/groups/security/compliance_dashboards/show.html.haml b/ee/app/views/groups/security/compliance_dashboards/show.html.haml index 6421c11e4f58e5..17aa1321d5f762 100644 --- a/ee/app/views/groups/security/compliance_dashboards/show.html.haml +++ b/ee/app/views/groups/security/compliance_dashboards/show.html.haml @@ -9,4 +9,5 @@ root_ancestor_path: @group.root_ancestor.full_path, pipeline_configuration_full_path_enabled: can?(current_user, :admin_compliance_pipeline_configuration, @group), base_path: group_security_compliance_dashboard_path(@group), - pipeline_configuration_enabled: @group.licensed_feature_available?(:compliance_pipeline_configuration).to_s } } + pipeline_configuration_enabled: @group.licensed_feature_available?(:compliance_pipeline_configuration).to_s, + adherence_report_ui_enabled: Feature.enabled?(:adherence_report_ui).to_s } } diff --git a/ee/spec/features/groups/security/compliance_dashboards_spec.rb b/ee/spec/features/groups/security/compliance_dashboards_spec.rb index a99338178054c5..45cca1f5959c9b 100644 --- a/ee/spec/features/groups/security/compliance_dashboards_spec.rb +++ b/ee/spec/features/groups/security/compliance_dashboards_spec.rb @@ -8,114 +8,142 @@ let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, :public, namespace: group) } let_it_be(:project_2) { create(:project, :repository, :public, namespace: group) } + let(:adherence_ff) { false } before do + stub_feature_flags(adherence_report_ui: adherence_ff) stub_licensed_features(group_level_compliance_dashboard: true) group.add_owner(user) sign_in(user) end - it 'shows the violations report table', :aggregate_failures do - visit group_security_compliance_dashboard_path(group) + context 'standards adherence tab' do + let(:expected_path) { group_security_compliance_dashboard_path(group, vueroute: :standards_adherence) } - 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 compliance violations' do before do visit group_security_compliance_dashboard_path(group) end - 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(:user_2) { create(:user) } - let_it_be(:merge_request) { create(:merge_request, source_project: project, state: :merged, author: user, merge_commit_sha: 'b71a6483b96dc303b66fdcaa212d9db6b10591ce') } - let_it_be(:merge_request_2) { create(:merge_request, source_project: project_2, state: :merged, author: user_2, merge_commit_sha: '24327319d067f4101cd3edd36d023ab5e49a8579') } + context 'with feature flag `adherence_report_ui` enabled' do + let(:adherence_ff) { true } - context 'and there is a compliance violation' do - let_it_be(:violation) do - create(:compliance_violation, :approved_by_committer, severity_level: :high, merge_request: merge_request, - violating_user: user, title: merge_request.title, target_project_id: project.id, - target_branch: merge_request.target_branch, merged_at: 1.day.ago) + it 'shows the standards adherence tab by default' do + expect(page).to have_current_path(expected_path) end + end - let_it_be(:violation_2) do - create(:compliance_violation, :approved_by_merge_request_author, severity_level: :medium, - merge_request: merge_request_2, violating_user: user, title: merge_request_2.title, - target_project_id: project_2.id, target_branch: merge_request_2.target_branch, merged_at: 7.days.ago) + context 'with feature flag `adherence_report_ui` disabled' do + it 'shows the standards adherence tab by default' do + expect(page).not_to have_current_path(expected_path) end + end + end - let(:merged_at) { 1.day.ago } + context 'violations tab' do + it 'shows the violations report table', :aggregate_failures do + visit group_security_compliance_dashboard_path(group, vueroute: :violations) - before do - merge_request.metrics.update!(merged_at: merged_at) - merge_request_2.metrics.update!(merged_at: 7.days.ago) + 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 compliance violations' do + before do visit group_security_compliance_dashboard_path(group) - wait_for_requests end - it 'shows the compliance violations with details', :aggregate_failures do - expect(all('tbody > tr').count).to eq(2) - - 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(merged_at.to_date.to_s) + it 'shows an empty state' do + expect(page).to have_content('No violations found') end + end - it 'can sort the violations by clicking on a column header' do - click_column_header 'Severity' + context 'when there are merge requests' do + let_it_be(:user_2) { create(:user) } + let_it_be(:merge_request) { create(:merge_request, source_project: project, state: :merged, author: user, merge_commit_sha: 'b71a6483b96dc303b66fdcaa212d9db6b10591ce') } + let_it_be(:merge_request_2) { create(:merge_request, source_project: project_2, state: :merged, author: user_2, merge_commit_sha: '24327319d067f4101cd3edd36d023ab5e49a8579') } + + context 'and there is a compliance violation' do + let_it_be(:violation) do + create(:compliance_violation, + :approved_by_committer, severity_level: :high, merge_request: merge_request, violating_user: user, + title: merge_request.title, target_project_id: project.id, target_branch: merge_request.target_branch, + merged_at: 1.day.ago) + end - expect(first_row).to have_content(merge_request_2.title) - end + let_it_be(:violation_2) do + create(:compliance_violation, + :approved_by_merge_request_author, severity_level: :medium, merge_request: merge_request_2, + violating_user: user, title: merge_request_2.title, target_project_id: project_2.id, + target_branch: merge_request_2.target_branch, merged_at: 7.days.ago) + end - it 'shows the correct user avatar popover content when the drawer is switched', :aggregate_failures do - first_row.click - drawer_user_avatar.hover + let(:merged_at) { 1.day.ago } - within '.popover' do - expect(page).to have_content(user.name) - expect(page).to have_content(user.username) + before do + merge_request.metrics.update!(merged_at: merged_at) + merge_request_2.metrics.update!(merged_at: 7.days.ago) + + visit group_security_compliance_dashboard_path(group) + wait_for_requests end - second_row.click - drawer_user_avatar.hover + it 'shows the compliance violations with details', :aggregate_failures do + expect(all('tbody > tr').count).to eq(2) - within '.popover' do - expect(page).to have_content(user_2.name) - expect(page).to have_content(user_2.username) + 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(merged_at.to_date.to_s) end - 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) + it 'can sort the violations by clicking on a column header' do + click_column_header 'Severity' - expect(page).to have_content(merge_request_2.title) - expect(page).not_to have_content(merge_request.title) + expect(first_row).to have_content(merge_request_2.title) end - it 'can filter by project id' do - filter_by_project(merge_request_2.project) + it 'shows the correct user avatar popover content when the drawer is switched', :aggregate_failures do + first_row.click + drawer_user_avatar.hover + + within '.popover' do + expect(page).to have_content(user.name) + expect(page).to have_content(user.username) + end + + second_row.click + drawer_user_avatar.hover + + within '.popover' do + expect(page).to have_content(user_2.name) + expect(page).to have_content(user_2.username) + end + 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) - expect(page).to have_content(merge_request_2.title) - expect(page).not_to have_content(merge_request.title) + expect(page).to have_content(merge_request_2.title) + expect(page).not_to have_content(merge_request.title) + end end end end end - context 'framework exports' do + context 'frameworks tab' do before do visit group_security_compliance_dashboard_path(group, vueroute: :frameworks) end diff --git a/ee/spec/frontend/compliance_dashboard/components/reports_app_spec.js b/ee/spec/frontend/compliance_dashboard/components/reports_app_spec.js index 79e3a01854081b..514078eb2a951c 100644 --- a/ee/spec/frontend/compliance_dashboard/components/reports_app_spec.js +++ b/ee/spec/frontend/compliance_dashboard/components/reports_app_spec.js @@ -23,8 +23,9 @@ describe('ComplianceReportsApp component', () => { const findTabs = () => wrapper.findComponent(GlTabs); const findFrameworksTab = () => wrapper.findByTestId('frameworks-tab'); const findViolationsTab = () => wrapper.findByTestId('violations-tab'); + const findStandardsAdherenceTab = () => wrapper.findByTestId('standards-adherence-tab'); - const createComponent = (props = {}, mountFn = shallowMount, mocks = {}) => { + const createComponent = (props = {}, mountFn = shallowMount, mocks = {}, provide = {}) => { return extendedWrapper( mountFn(ComplianceReportsApp, { propsData: { @@ -40,10 +41,24 @@ describe('ComplianceReportsApp component', () => { stubs: { 'router-view': stubComponent({}), }, + provide: { + adherenceReportUiEnabled: false, + ...provide, + }, }), ); }; + describe('adherence standards report', () => { + beforeEach(() => { + wrapper = createComponent(defaultProps, mount, {}, { adherenceReportUiEnabled: true }); + }); + + it('renders the violations report tab', () => { + expect(findStandardsAdherenceTab().exists()).toBe(true); + }); + }); + describe('violations report', () => { beforeEach(() => { wrapper = createComponent(defaultProps, mount); diff --git a/ee/spec/frontend/compliance_dashboard/components/standards_adherence_report/report_spec.js b/ee/spec/frontend/compliance_dashboard/components/standards_adherence_report/report_spec.js new file mode 100644 index 00000000000000..eee28d777a5428 --- /dev/null +++ b/ee/spec/frontend/compliance_dashboard/components/standards_adherence_report/report_spec.js @@ -0,0 +1,23 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlAlert } from '@gitlab/ui'; +import ComplianceStandardsAdherenceReport from 'ee/compliance_dashboard/components/standards_adherence_report/report.vue'; + +describe('ComplianceStandardsAdherenceReport component', () => { + let wrapper; + + const findErrorMessage = () => wrapper.findComponent(GlAlert); + + const createComponent = () => { + wrapper = shallowMount(ComplianceStandardsAdherenceReport, {}); + }; + + describe('default behavior', () => { + beforeEach(() => { + createComponent(); + }); + + it('does not render an error message', () => { + expect(findErrorMessage().exists()).toBe(false); + }); + }); +}); diff --git a/ee/spec/views/groups/security/compliance_dashboards/show.html.haml_spec.rb b/ee/spec/views/groups/security/compliance_dashboards/show.html.haml_spec.rb new file mode 100644 index 00000000000000..3d5f3a4202efc1 --- /dev/null +++ b/ee/spec/views/groups/security/compliance_dashboards/show.html.haml_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe "groups/security/compliance_dashboards/show", type: :view, feature_category: :compliance_management do + let_it_be(:user) { build_stubbed(:user) } + let_it_be(:group) { build_stubbed(:group) } + let(:framework_csv_export_path) { group_security_compliance_framework_reports_path(group, format: :csv) } + let(:merge_commits_csv_export_path) { group_security_merge_commit_reports_path(group) } + + before do + allow(view).to receive(:current_user).and_return(user) + assign(:group, group) + render + end + + it 'renders with the correct data attributes', :aggregate_failures do + render + + expect(rendered).to have_selector('#js-compliance-report') + expect(rendered).to have_selector("[data-can-add-edit='true']") + expect(rendered).to have_selector("[data-frameworks-csv-export-path='#{framework_csv_export_path}']") + expect(rendered).to have_selector("[data-merge-commits-csv-export-path='#{merge_commits_csv_export_path}']") + expect(rendered).to have_selector("[data-group-path='#{group.full_path}']") + expect(rendered).to have_selector("[data-root-ancestor-path='#{group.root_ancestor.full_path}']") + expect(rendered).to have_selector("[data-base-path='#{group_security_compliance_dashboard_path(group)}']") + expect(rendered).to have_selector("[data-pipeline-configuration-enabled='false']") + expect(rendered).to have_selector("[data-adherence-report-ui-enabled='true']") + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b0bd90ed3bf827..dc32943405f8c0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11404,6 +11404,9 @@ msgstr "" msgid "Compliance Report|Frameworks" msgstr "" +msgid "Compliance Report|Standards Adherence" +msgstr "" + msgid "Compliance Report|Violations" msgstr "" -- GitLab From 3079a7298b7083395e75f6f91c9f12a35e0f9486 Mon Sep 17 00:00:00 2001 From: Sam Figueroa Date: Tue, 6 Jun 2023 14:49:04 +0000 Subject: [PATCH 2/4] Fix typo --- .../compliance_dashboard/components/reports_app_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/frontend/compliance_dashboard/components/reports_app_spec.js b/ee/spec/frontend/compliance_dashboard/components/reports_app_spec.js index 514078eb2a951c..c3e06d7d27b1ef 100644 --- a/ee/spec/frontend/compliance_dashboard/components/reports_app_spec.js +++ b/ee/spec/frontend/compliance_dashboard/components/reports_app_spec.js @@ -54,7 +54,7 @@ describe('ComplianceReportsApp component', () => { wrapper = createComponent(defaultProps, mount, {}, { adherenceReportUiEnabled: true }); }); - it('renders the violations report tab', () => { + it('renders the standards adherence report tab', () => { expect(findStandardsAdherenceTab().exists()).toBe(true); }); }); -- GitLab From 5cdb1a4115240726791a8c9f1f8733c30edb1d0f Mon Sep 17 00:00:00 2001 From: Huzaifa Iftikhar Date: Tue, 6 Jun 2023 16:06:20 +0000 Subject: [PATCH 3/4] Apply feedback from reviewer --- ee/spec/features/groups/security/compliance_dashboards_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/features/groups/security/compliance_dashboards_spec.rb b/ee/spec/features/groups/security/compliance_dashboards_spec.rb index 45cca1f5959c9b..0eebc6a20d285f 100644 --- a/ee/spec/features/groups/security/compliance_dashboards_spec.rb +++ b/ee/spec/features/groups/security/compliance_dashboards_spec.rb @@ -33,7 +33,7 @@ end context 'with feature flag `adherence_report_ui` disabled' do - it 'shows the standards adherence tab by default' do + it 'does not show the standards adherence tab' do expect(page).not_to have_current_path(expected_path) end end -- GitLab From f6c16b72e9d364867e5dd36aa49c94df22bbc225 Mon Sep 17 00:00:00 2001 From: Niko Belokolodov Date: Wed, 7 Jun 2023 15:18:09 +0000 Subject: [PATCH 4/4] add rollout issue to ff yml --- config/feature_flags/development/adherence_report_ui.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/development/adherence_report_ui.yml b/config/feature_flags/development/adherence_report_ui.yml index 21c4920bddd635..5648299c68919b 100644 --- a/config/feature_flags/development/adherence_report_ui.yml +++ b/config/feature_flags/development/adherence_report_ui.yml @@ -1,7 +1,7 @@ --- name: adherence_report_ui introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122374 -rollout_issue_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/414495 milestone: '16.1' type: development group: group::compliance -- GitLab