From 1631b9fcbf0a1b9c3311586b4d02274b0d87da56 Mon Sep 17 00:00:00 2001 From: Ross Byrne <robyrne@gitlab.com> Date: Mon, 3 Apr 2023 16:23:14 +0100 Subject: [PATCH] Remove the `security_reports_mr_widget_prompt` experiment Changelog: changed EE: true --- .rubocop_todo/lint/empty_block.yml | 1 - .../rspec/missing_feature_category.yml | 1 - ...ity_reports_mr_widget_prompt_experiment.rb | 6 -- .../security_reports_mr_widget_prompt.yml | 8 --- .../mr_widget_enable_feature_prompt.vue | 58 --------------- .../mr_widget_options.vue | 2 - .../ee/projects/merge_requests_controller.rb | 4 -- .../mr_widget_enable_feature_prompt_spec.js | 62 ---------------- .../merge_requests_controller_spec.rb | 70 ------------------- locale/gitlab.pot | 3 - ...eports_mr_widget_prompt_experiment_spec.rb | 9 --- spec/support/rspec_order_todo.yml | 1 - 12 files changed, 225 deletions(-) delete mode 100644 app/experiments/security_reports_mr_widget_prompt_experiment.rb delete mode 100644 config/feature_flags/experiment/security_reports_mr_widget_prompt.yml delete mode 100644 ee/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_enable_feature_prompt.vue delete mode 100644 ee/spec/frontend/vue_merge_request_widget/components/states/mr_widget_enable_feature_prompt_spec.js delete mode 100644 spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb diff --git a/.rubocop_todo/lint/empty_block.yml b/.rubocop_todo/lint/empty_block.yml index 8845fb3abe21d..43289b4bfbd81 100644 --- a/.rubocop_todo/lint/empty_block.yml +++ b/.rubocop_todo/lint/empty_block.yml @@ -6,7 +6,6 @@ Lint/EmptyBlock: - 'app/controllers/projects/boards_controller.rb' - 'app/controllers/projects/pipelines_controller.rb' - 'app/experiments/logged_out_marketing_header_experiment.rb' - - 'app/experiments/security_reports_mr_widget_prompt_experiment.rb' - 'config/application.rb' - 'ee/app/controllers/projects/learn_gitlab_controller.rb' - 'ee/spec/factories/incident_management/escalation_rules.rb' diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 72336d8d0c45d..a3822320e9834 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -1942,7 +1942,6 @@ RSpec/MissingFeatureCategory: - 'spec/experiments/in_product_guidance_environments_webide_experiment_spec.rb' - 'spec/experiments/ios_specific_templates_experiment_spec.rb' - 'spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb' - - 'spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb' - 'spec/features/admin/dashboard_spec.rb' - 'spec/features/groups/integrations/group_integrations_spec.rb' - 'spec/features/milestones/user_views_milestones_spec.rb' diff --git a/app/experiments/security_reports_mr_widget_prompt_experiment.rb b/app/experiments/security_reports_mr_widget_prompt_experiment.rb deleted file mode 100644 index 0a5778950fa6d..0000000000000 --- a/app/experiments/security_reports_mr_widget_prompt_experiment.rb +++ /dev/null @@ -1,6 +0,0 @@ -# frozen_string_literal: true - -class SecurityReportsMrWidgetPromptExperiment < ApplicationExperiment - control {} - candidate {} -end diff --git a/config/feature_flags/experiment/security_reports_mr_widget_prompt.yml b/config/feature_flags/experiment/security_reports_mr_widget_prompt.yml deleted file mode 100644 index 16cc3d890d79e..0000000000000 --- a/config/feature_flags/experiment/security_reports_mr_widget_prompt.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: security_reports_mr_widget_prompt -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70086 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340436 -milestone: '14.3' -type: experiment -group: group::acquisition -default_enabled: false diff --git a/ee/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_enable_feature_prompt.vue b/ee/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_enable_feature_prompt.vue deleted file mode 100644 index 59e0011272a20..0000000000000 --- a/ee/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_enable_feature_prompt.vue +++ /dev/null @@ -1,58 +0,0 @@ -<script> -import { GlButton } from '@gitlab/ui'; -import GitlabExperiment from '~/experimentation/components/gitlab_experiment.vue'; -import CiIcon from '~/vue_shared/components/ci_icon.vue'; - -export default { - name: 'MrWidgetEnableFeaturePrompt', - components: { - CiIcon, - GitlabExperiment, - GlButton, - }, - props: { - feature: { - type: String, - required: true, - }, - }, - data() { - const dismissalKey = [this.$options.name, this.feature, 'dismissed'].join('.'); - return { - dismissalKey, - status: { - group: 'notification', - icon: 'status-neutral', - }, - dismissed: localStorage.getItem(dismissalKey), - }; - }, - methods: { - dismiss() { - localStorage.setItem(this.dismissalKey, (this.dismissed = true)); - }, - }, -}; -</script> -<template> - <gitlab-experiment v-if="!dismissed" :name="feature"> - <template #control></template> - <template #candidate> - <div class="mr-widget-body media"> - <ci-icon class="gl-mr-3" :status="status" :size="24" /> - <div class="media-body gl-text-gray-400"> - <slot></slot> - </div> - <gl-button - category="tertiary" - size="small" - icon="close" - :aria-label="s__('mrWidget|Dismiss')" - data-track-action="dismissed" - :data-track-experiment="feature" - @click="dismiss" - /> - </div> - </template> - </gitlab-experiment> -</template> 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 e75768ff72776..1d17d58035908 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 @@ -5,7 +5,6 @@ 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'; import CEWidgetOptions from '~/vue_merge_request_widget/mr_widget_options.vue'; -import MrWidgetEnableFeaturePrompt from './components/states/mr_widget_enable_feature_prompt.vue'; import MrWidgetJiraAssociationMissing from './components/states/mr_widget_jira_association_missing.vue'; import MrWidgetPolicyViolation from './components/states/mr_widget_policy_violation.vue'; import MrWidgetGeoSecondaryNode from './components/states/mr_widget_secondary_geo_node.vue'; @@ -23,7 +22,6 @@ export default { WidgetContainer, MrWidgetGeoSecondaryNode, MrWidgetPolicyViolation, - MrWidgetEnableFeaturePrompt, MrWidgetJiraAssociationMissing, BlockingMergeRequestsReport: () => import('./components/blocking_merge_requests/blocking_merge_requests_report.vue'), diff --git a/ee/app/controllers/ee/projects/merge_requests_controller.rb b/ee/app/controllers/ee/projects/merge_requests_controller.rb index 40fc68322b14a..06d18db8c647a 100644 --- a/ee/app/controllers/ee/projects/merge_requests_controller.rb +++ b/ee/app/controllers/ee/projects/merge_requests_controller.rb @@ -10,10 +10,6 @@ module MergeRequestsController include GeoInstrumentation before_action only: [:show] do - if can_run_sast_experiments_on?(@project) - experiment(:security_reports_mr_widget_prompt, namespace: @project.namespace).publish - end - push_frontend_feature_flag(:anonymous_visual_review_feedback) push_frontend_feature_flag(:suggested_reviewers_control, @project) end diff --git a/ee/spec/frontend/vue_merge_request_widget/components/states/mr_widget_enable_feature_prompt_spec.js b/ee/spec/frontend/vue_merge_request_widget/components/states/mr_widget_enable_feature_prompt_spec.js deleted file mode 100644 index 9c5f5b6f8e5c9..0000000000000 --- a/ee/spec/frontend/vue_merge_request_widget/components/states/mr_widget_enable_feature_prompt_spec.js +++ /dev/null @@ -1,62 +0,0 @@ -import { mount } from '@vue/test-utils'; -import { nextTick } from 'vue'; -import MrWidgetEnableFeaturePrompt from 'ee/vue_merge_request_widget/components/states/mr_widget_enable_feature_prompt.vue'; -import { stubExperiments } from 'helpers/experimentation_helper'; -import CiIcon from '~/vue_shared/components/ci_icon.vue'; - -const FEATURE = 'my_feature_name'; -const LOCAL_STORAGE_KEY = `MrWidgetEnableFeaturePrompt.${FEATURE}.dismissed`; - -describe('MrWidgetEnableFeaturePrompt', () => { - let wrapper; - - const createComponent = () => { - wrapper = mount(MrWidgetEnableFeaturePrompt, { - propsData: { feature: FEATURE }, - slots: { - default: 'this is my content', - }, - }); - }; - - const findCiIcon = () => wrapper.findComponent(CiIcon); - const findDismissButton = () => wrapper.find('[data-track-action="dismissed"]'); - - beforeEach(() => { - createComponent(); - }); - - describe('when the experiment is not enabled', () => { - it('renders nothing', () => { - stubExperiments({ [FEATURE]: 'control' }); - expect(wrapper.text()).toBe(''); - }); - }); - - describe('when the experiment is enabled', () => { - beforeAll(() => { - stubExperiments({ [FEATURE]: 'candidate' }); - localStorage.removeItem(LOCAL_STORAGE_KEY); - }); - - it('shows a neutral icon', () => { - expect(findCiIcon().props('status').group).toBe('notification'); - expect(findCiIcon().props('status').icon).toBe('status-neutral'); - expect(findCiIcon().props('size')).toBe(24); - }); - - it('renders the provided slots', () => { - expect(wrapper.text()).toBe('this is my content'); - }); - - it('can be dismissed', async () => { - const button = findDismissButton(); - button.vm.$emit('click'); - - await nextTick(); - - expect(localStorage.getItem(LOCAL_STORAGE_KEY)).toBe('true'); - expect(wrapper.text()).toBe(''); - }); - }); -}); diff --git a/ee/spec/requests/projects/merge_requests_controller_spec.rb b/ee/spec/requests/projects/merge_requests_controller_spec.rb index 8a265a88d4d21..3679e3dce6d3d 100644 --- a/ee/spec/requests/projects/merge_requests_controller_spec.rb +++ b/ee/spec/requests/projects/merge_requests_controller_spec.rb @@ -11,76 +11,6 @@ login_as(user) end - describe 'GET #show' do - before do - stub_licensed_features(sast: true) - end - - def get_show - get project_merge_request_path(project, merge_request) - end - - context 'when the user has developer access' do - it 'publishes the security_reports_mr_widget_prompt experiment' do - # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/343375 - # More: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73034#note_720186839 - # https://docs.gitlab.com/ee/development/query_count_limits.html#disable-query-limiting - allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(110) - expect_next_instance_of(SecurityReportsMrWidgetPromptExperiment) do |instance| - expect(instance).to receive(:publish) - end - - get_show - end - end - - context 'when the user does not have developer access' do - let(:user) { create(:user) } - - it 'does not publish the security_reports_mr_widget_prompt experiment' do - expect(SecurityReportsMrWidgetPromptExperiment).not_to receive(:new) - - get_show - end - end - - context 'when the project is not licensed for sast' do - before do - stub_licensed_features(sast: false) - end - - it 'does not publish the security_reports_mr_widget_prompt experiment' do - expect(SecurityReportsMrWidgetPromptExperiment).not_to receive(:new) - - get_show - end - end - - context 'when the project has disabled the security and compliance features' do - before do - project.project_feature.update_column(:security_and_compliance_access_level, Featurable::DISABLED) - end - - it 'does not publish the security_reports_mr_widget_prompt experiment' do - expect(SecurityReportsMrWidgetPromptExperiment).not_to receive(:new) - - get_show - end - end - - context 'when the the user is a guest' do - let(:user) { create(:user) } - - it 'does not publish the security_reports_mr_widget_prompt experiment' do - project.add_guest(user) - - expect(SecurityReportsMrWidgetPromptExperiment).not_to receive(:new) - - get_show - end - end - end - describe 'GET #edit' do def get_edit get edit_project_merge_request_path(project, merge_request) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a0c5f5ba6c79f..06228f7819441 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -52455,9 +52455,6 @@ msgstr "" msgid "mrWidget|Did not close" msgstr "" -msgid "mrWidget|Dismiss" -msgstr "" - msgid "mrWidget|Failed to load deployment statistics" msgstr "" diff --git a/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb b/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb deleted file mode 100644 index ee02fa5f1f234..0000000000000 --- a/spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe SecurityReportsMrWidgetPromptExperiment do - it "defines a control and candidate" do - expect(subject.behaviors.keys).to match_array(%w[control candidate]) - end -end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 0e50ee953b7a3..80eab28470611 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -3613,7 +3613,6 @@ - './spec/experiments/in_product_guidance_environments_webide_experiment_spec.rb' - './spec/experiments/ios_specific_templates_experiment_spec.rb' - './spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb' -- './spec/experiments/security_reports_mr_widget_prompt_experiment_spec.rb' - './spec/features/abuse_report_spec.rb' - './spec/features/action_cable_logging_spec.rb' - './spec/features/admin/admin_abuse_reports_spec.rb' -- GitLab