From 369585475e26223dbef213195bfe794ed40eb51b Mon Sep 17 00:00:00 2001
From: Fernando Arias <farias@gitlab.com>
Date: Sun, 4 Sep 2022 12:23:34 +0200
Subject: [PATCH 1/2] First pass legacy license compliance widget removal
* Remove frontend flags
* Update unit and browser automation tests
Changelog: changed
EE: true
---
.../mr_widget_options.vue | 20 +----
.../ee/projects/merge_requests_controller.rb | 1 -
.../projects/merge_requests/show.html.haml | 2 +-
.../refactor_license_compliance_extension.yml | 8 --
.../user_sees_status_checks_widget_spec.rb | 1 -
.../ee_mr_widget_options_spec.js | 85 ++++++++++++-------
qa/qa/ee/page/component/license_management.rb | 26 +-----
qa/qa/ee/page/merge_request/show.rb | 11 +--
8 files changed, 61 insertions(+), 93 deletions(-)
delete mode 100644 ee/config/feature_flags/development/refactor_license_compliance_extension.yml
diff --git a/ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
index 5b086fe767074437..acb30e78bd0ee099 100644
--- a/ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
+++ b/ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
@@ -1,6 +1,5 @@
<script>
import { GlSprintf, GlLink, GlSafeHtmlDirective } from '@gitlab/ui';
-import MrWidgetLicenses from 'ee/vue_shared/license_compliance/mr_widget_license_report.vue';
import reportsMixin from 'ee/vue_shared/security_reports/mixins/reports_mixin';
import { registerExtension } from '~/vue_merge_request_widget/components/extensions';
import { s__, __, sprintf } from '~/locale';
@@ -20,7 +19,6 @@ export default {
components: {
GlSprintf,
GlLink,
- MrWidgetLicenses,
WidgetContainer,
MrWidgetGeoSecondaryNode,
MrWidgetPolicyViolation,
@@ -93,9 +91,6 @@ export default {
!this.shouldShowExtension
);
},
- shouldShowLicenseComplianceExtension() {
- return window.gon?.features?.refactorLicenseComplianceExtension;
- },
hasLoadPerformanceMetrics() {
return (
this.mr.loadPerformanceMetrics?.degraded?.length > 0 ||
@@ -224,7 +219,7 @@ export default {
},
methods: {
registerLicenseCompliance() {
- if (this.shouldShowLicenseComplianceExtension) {
+ if (this.shouldShowExtension) {
registerExtension(licenseComplianceExtension);
}
},
@@ -486,19 +481,6 @@ export default {
</gl-sprintf>
</mr-widget-enable-feature-prompt>
- <mr-widget-licenses
- v-if="shouldRenderLicenseReport && !shouldShowLicenseComplianceExtension"
- :api-url="mr.licenseScanning.managed_licenses_path"
- :approvals-api-path="mr.apiApprovalsPath"
- :licenses-api-path="licensesApiPath"
- :pipeline-path="mr.pipeline.path"
- :can-manage-licenses="mr.licenseScanning.can_manage_licenses"
- :full-report-path="mr.licenseScanning.full_report_path"
- :license-management-settings-path="mr.licenseScanning.settings_path"
- :license-compliance-docs-path="mr.licenseComplianceDocsPath"
- report-section-class="mr-widget-border-top"
- />
-
<grouped-test-reports-app
v-if="shouldRenderTestReport && !shouldRenderRefactoredTestReport"
class="js-reports-container"
diff --git a/ee/app/controllers/ee/projects/merge_requests_controller.rb b/ee/app/controllers/ee/projects/merge_requests_controller.rb
index 233cdcbfde237ae4..b3364b258a8e8713 100644
--- a/ee/app/controllers/ee/projects/merge_requests_controller.rb
+++ b/ee/app/controllers/ee/projects/merge_requests_controller.rb
@@ -19,7 +19,6 @@ module MergeRequestsController
push_frontend_feature_flag(:refactor_mr_widgets_extensions, @project)
push_frontend_feature_flag(:refactor_mr_widget_test_summary, @project)
push_frontend_feature_flag(:refactor_mr_widgets_extensions_user, current_user)
- push_frontend_feature_flag(:refactor_license_compliance_extension, @project)
push_frontend_feature_flag(:suggested_reviewers, @project)
end
diff --git a/ee/app/views/projects/merge_requests/show.html.haml b/ee/app/views/projects/merge_requests/show.html.haml
index 49f09ac910202566..446ba2b1f925c0a8 100644
--- a/ee/app/views/projects/merge_requests/show.html.haml
+++ b/ee/app/views/projects/merge_requests/show.html.haml
@@ -15,7 +15,7 @@
window.gl.mrWidgetData.coverage_fuzzing_help_path = '#{help_page_path("user/application_security/coverage_fuzzing/index")}';
window.gl.mrWidgetData.visual_review_app_available = '#{@project.feature_available?(:visual_review_app)}' === 'true';
window.gl.mrWidgetData.license_scanning_comparison_path = '#{license_scanning_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:license_scanning)}'
- window.gl.mrWidgetData.license_scanning_comparison_collapsed_path = '#{license_scanning_reports_collapsed_project_merge_request_path(@project, @merge_request) if Feature.enabled?(:refactor_license_compliance_extension, @project) && @project.feature_available?(:license_scanning)}'
+ window.gl.mrWidgetData.license_scanning_comparison_collapsed_path = '#{license_scanning_reports_collapsed_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:license_scanning)}'
window.gl.mrWidgetData.container_scanning_comparison_path = '#{container_scanning_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:container_scanning)}'
window.gl.mrWidgetData.dependency_scanning_comparison_path = '#{dependency_scanning_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:dependency_scanning)}'
window.gl.mrWidgetData.sast_comparison_path = '#{sast_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:sast)}'
diff --git a/ee/config/feature_flags/development/refactor_license_compliance_extension.yml b/ee/config/feature_flags/development/refactor_license_compliance_extension.yml
deleted file mode 100644
index b6d3508c1fb078bd..0000000000000000
--- a/ee/config/feature_flags/development/refactor_license_compliance_extension.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: refactor_license_compliance_extension
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84128
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/367703
-milestone: '15.0'
-type: development
-group: group::composition analysis
-default_enabled: true
diff --git a/ee/spec/features/merge_request/user_sees_status_checks_widget_spec.rb b/ee/spec/features/merge_request/user_sees_status_checks_widget_spec.rb
index 58116535ca5b2503..c6fb29793816332a 100644
--- a/ee/spec/features/merge_request/user_sees_status_checks_widget_spec.rb
+++ b/ee/spec/features/merge_request/user_sees_status_checks_widget_spec.rb
@@ -26,7 +26,6 @@
stub_feature_flags(refactor_mr_widgets_extensions: false)
stub_feature_flags(refactor_mr_widgets_extensions_user: false)
stub_feature_flags(refactor_security_extension: false)
- stub_feature_flags(refactor_license_compliance_extension: false)
end
context 'user is authorized' do
diff --git a/ee/spec/frontend/vue_merge_request_widget/ee_mr_widget_options_spec.js b/ee/spec/frontend/vue_merge_request_widget/ee_mr_widget_options_spec.js
index 44dbf308cd8579ce..5681bc7c550e68c7 100644
--- a/ee/spec/frontend/vue_merge_request_widget/ee_mr_widget_options_spec.js
+++ b/ee/spec/frontend/vue_merge_request_widget/ee_mr_widget_options_spec.js
@@ -3,6 +3,11 @@ import MockAdapter from 'axios-mock-adapter';
import Vue, { nextTick } from 'vue';
import VueApollo from 'vue-apollo';
+import {
+ registerExtension,
+ registeredExtensions,
+} from '~/vue_merge_request_widget/components/extensions';
+
// Force Jest to transpile and cache
// eslint-disable-next-line no-unused-vars
import _GroupedBrowserPerformanceReportsApp from 'ee/reports/browser_performance_report/grouped_browser_performance_reports_app.vue';
@@ -15,6 +20,10 @@ import MrWidgetOptions from 'ee/vue_merge_request_widget/mr_widget_options.vue';
// Force Jest to transpile and cache
// eslint-disable-next-line no-unused-vars
import _GroupedSecurityReportsApp from 'ee/vue_shared/security_reports/grouped_security_reports_app.vue';
+
+// EE Widget Extensions
+import licenseComplianceExtension from 'ee/vue_merge_request_widget/extensions/license_compliance';
+
import {
sastDiffSuccessMock,
dastDiffSuccessMock,
@@ -935,38 +944,6 @@ describe('ee merge request widget options', () => {
});
});
- describe('license scanning report', () => {
- const licenseManagementApiUrl = `${TEST_HOST}/manage_license_api`;
-
- it('should be rendered if license scanning data is set', () => {
- gl.mrWidgetData = {
- ...mockData,
- enabled_reports: {
- license_scanning: true,
- },
- license_scanning: {
- managed_licenses_path: licenseManagementApiUrl,
- can_manage_licenses: false,
- },
- };
-
- createComponent({ propsData: { mrData: gl.mrWidgetData } });
-
- expect(wrapper.find('.license-report-widget').exists()).toBe(true);
- });
-
- it('should not be rendered if license scanning data is not set', () => {
- gl.mrWidgetData = {
- ...mockData,
- license_scanning: {},
- };
-
- createComponent({ propsData: { mrData: gl.mrWidgetData } });
-
- expect(wrapper.find('.license-report-widget').exists()).toBe(false);
- });
- });
-
describe('CE security report', () => {
describe.each`
context | canReadVulnerabilities | hasPipeline | shouldRender
@@ -1187,10 +1164,54 @@ describe('ee merge request widget options', () => {
},
},
});
+
wrapper.vm.mr.state = mergeState;
await nextTick();
expect(findStatusChecksReport().exists()).toBe(shouldRender);
});
});
+
+ describe('license scanning report', () => {
+ afterEach(() => {
+ registeredExtensions.extensions = [];
+ });
+
+ it('should be rendered if license widget is registered', async () => {
+ const licenseComparisonPath =
+ '/group-name/project-name/-/merge_requests/78/license_scanning_reports';
+ const licenseComparisonPathCollapsed =
+ '/group-name/project-name/-/merge_requests/78/license_scanning_reports_collapsed';
+ const fullReportPath = '/group-name/project-name/-/merge_requests/78/full_report';
+ const settingsPath = '/group-name/project-name/-/licenses#licenses';
+ const apiApprovalsPath = '/group-name/project-name/-/licenses#policies';
+
+ gl.mrWidgetData = {
+ ...mockData,
+ license_scanning_comparison_path: licenseComparisonPath,
+ license_scanning_comparison_collapsed_path: licenseComparisonPathCollapsed,
+ api_approvals_path: apiApprovalsPath,
+ license_scanning: {
+ settings_path: settingsPath,
+ full_report_path: fullReportPath,
+ },
+ };
+
+ registerExtension(licenseComplianceExtension);
+
+ await createComponent({ propsData: { mrData: gl.mrWidgetData } });
+
+ expect(wrapper.findComponent({ name: 'WidgetLicenseCompliance' }).exists()).toBe(true);
+ });
+
+ it('should not be rendered if license widget is not registered', () => {
+ gl.mrWidgetData = {
+ ...mockData,
+ };
+
+ createComponent({ propsData: { mrData: gl.mrWidgetData } });
+
+ expect(wrapper.findComponent({ name: 'WidgetLicenseCompliance' }).exists()).toBe(false);
+ });
+ });
});
diff --git a/qa/qa/ee/page/component/license_management.rb b/qa/qa/ee/page/component/license_management.rb
index 858fbd27912b6445..e4c424daeb9953ce 100644
--- a/qa/qa/ee/page/component/license_management.rb
+++ b/qa/qa/ee/page/component/license_management.rb
@@ -19,11 +19,6 @@ def self.prepended(base)
element :icon_status, ':data-qa-selector="`status_${status}_icon`" ' # rubocop:disable QA/ElementWithPattern
end
- view 'ee/app/assets/javascripts/vue_shared/license_compliance/mr_widget_license_report.vue' do
- element :license_report_widget
- element :manage_licenses_button
- end
-
view 'app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue' do
element :mr_widget_extension
end
@@ -39,19 +34,13 @@ def self.prepended(base)
end
def has_approved_license?(name)
- content_element = feature_flag_controlled_element(:refactor_license_compliance_extension,
- :child_content,
- :report_item_row)
- within_element(content_element, text: name) do
+ within_element(:child_content, text: name) do
has_element?(:status_success_icon, wait: 1)
end
end
def has_denied_license?(name)
- content_element = feature_flag_controlled_element(:refactor_license_compliance_extension,
- :child_content,
- :report_item_row)
- within_element(content_element, text: name) do
+ within_element(:child_content, text: name) do
has_element?(:status_failed_icon, wait: 1)
end
end
@@ -59,15 +48,8 @@ def has_denied_license?(name)
def click_manage_licenses_button
previous_page = page.current_url
- widget_element = feature_flag_controlled_element(:refactor_license_compliance_extension,
- :mr_widget_extension,
- :license_report_widget)
- within_element(widget_element) do
- if widget_element == :mr_widget_extension
- click_element(:mr_widget_extension_actions_button, text: 'Manage Licenses')
- else
- click_element(:manage_licenses_button)
- end
+ within_element(:mr_widget_extension) do
+ click_element(:mr_widget_extension_actions_button, text: 'Manage Licenses')
end
# TODO workaround for switched to a new window UI
wait_until(max_duration: 15, reload: false) do
diff --git a/qa/qa/ee/page/merge_request/show.rb b/qa/qa/ee/page/merge_request/show.rb
index a21bf03e82286511..9bcb0ab4a1ce4db3 100644
--- a/qa/qa/ee/page/merge_request/show.rb
+++ b/qa/qa/ee/page/merge_request/show.rb
@@ -102,15 +102,8 @@ def click_approve
end
def expand_license_report
- widget_name = feature_flag_controlled_element(:refactor_license_compliance_extension,
- :mr_widget_extension,
- :license_report_widget)
- within_element(widget_name) do
- if widget_name == :mr_widget_extension
- click_element(:toggle_button)
- else
- click_element(:expand_report_button)
- end
+ within_element(:mr_widget_extension) do
+ click_element(:toggle_button)
end
end
--
GitLab
From 1a8792a723782e4f4ee580d7eebec8a2861b3f76 Mon Sep 17 00:00:00 2001
From: Fernando Arias <farias@gitlab.com>
Date: Mon, 5 Sep 2022 18:55:36 +0200
Subject: [PATCH 2/2] Apply maintainer feedback
* Apply patch for unit test refactor
---
.../ee_mr_widget_options_spec.js | 65 ++++++++++---------
1 file changed, 33 insertions(+), 32 deletions(-)
diff --git a/ee/spec/frontend/vue_merge_request_widget/ee_mr_widget_options_spec.js b/ee/spec/frontend/vue_merge_request_widget/ee_mr_widget_options_spec.js
index 5681bc7c550e68c7..838ff62ae11a3467 100644
--- a/ee/spec/frontend/vue_merge_request_widget/ee_mr_widget_options_spec.js
+++ b/ee/spec/frontend/vue_merge_request_widget/ee_mr_widget_options_spec.js
@@ -1177,41 +1177,42 @@ describe('ee merge request widget options', () => {
registeredExtensions.extensions = [];
});
- it('should be rendered if license widget is registered', async () => {
- const licenseComparisonPath =
- '/group-name/project-name/-/merge_requests/78/license_scanning_reports';
- const licenseComparisonPathCollapsed =
- '/group-name/project-name/-/merge_requests/78/license_scanning_reports_collapsed';
- const fullReportPath = '/group-name/project-name/-/merge_requests/78/full_report';
- const settingsPath = '/group-name/project-name/-/licenses#licenses';
- const apiApprovalsPath = '/group-name/project-name/-/licenses#policies';
+ it.each`
+ shouldRegisterExtension | description
+ ${true} | ${'extension is registered'}
+ ${false} | ${'extension is not registered'}
+ `(
+ 'should render license widget is "$shouldRegisterExtension" when $description',
+ ({ shouldRegisterExtension }) => {
+ const licenseComparisonPath =
+ '/group-name/project-name/-/merge_requests/78/license_scanning_reports';
+ const licenseComparisonPathCollapsed =
+ '/group-name/project-name/-/merge_requests/78/license_scanning_reports_collapsed';
+ const fullReportPath = '/group-name/project-name/-/merge_requests/78/full_report';
+ const settingsPath = '/group-name/project-name/-/licenses#licenses';
+ const apiApprovalsPath = '/group-name/project-name/-/licenses#policies';
- gl.mrWidgetData = {
- ...mockData,
- license_scanning_comparison_path: licenseComparisonPath,
- license_scanning_comparison_collapsed_path: licenseComparisonPathCollapsed,
- api_approvals_path: apiApprovalsPath,
- license_scanning: {
- settings_path: settingsPath,
- full_report_path: fullReportPath,
- },
- };
-
- registerExtension(licenseComplianceExtension);
-
- await createComponent({ propsData: { mrData: gl.mrWidgetData } });
-
- expect(wrapper.findComponent({ name: 'WidgetLicenseCompliance' }).exists()).toBe(true);
- });
+ gl.mrWidgetData = {
+ ...mockData,
+ license_scanning_comparison_path: licenseComparisonPath,
+ license_scanning_comparison_collapsed_path: licenseComparisonPathCollapsed,
+ api_approvals_path: apiApprovalsPath,
+ license_scanning: {
+ settings_path: settingsPath,
+ full_report_path: fullReportPath,
+ },
+ };
- it('should not be rendered if license widget is not registered', () => {
- gl.mrWidgetData = {
- ...mockData,
- };
+ if (shouldRegisterExtension) {
+ registerExtension(licenseComplianceExtension);
+ }
- createComponent({ propsData: { mrData: gl.mrWidgetData } });
+ createComponent({ propsData: { mrData: gl.mrWidgetData } });
- expect(wrapper.findComponent({ name: 'WidgetLicenseCompliance' }).exists()).toBe(false);
- });
+ expect(wrapper.findComponent({ name: 'WidgetLicenseCompliance' }).exists()).toBe(
+ shouldRegisterExtension,
+ );
+ },
+ );
});
});
--
GitLab