Commit 883ad665 authored by Mayra Cabrera's avatar Mayra Cabrera

Merge branch 'jejacks0n/experiment/security_reports_mr_widget_prompt_exclusions_two' into 'master'

Consolidate the logic for SAST prompt experiment

See merge request gitlab-org/gitlab!73034
parents 021f1346 a7458c87
......@@ -9,7 +9,7 @@ module EE
include DescriptionDiffActions
before_action only: [:show] do
if @project.licensed_feature_available?(:sast) && can?(current_user, :developer_access, @project)
if can_run_sast_experiments_on?(@project)
experiment(:security_reports_mr_widget_prompt, namespace: @project.namespace).publish
end
......@@ -33,6 +33,11 @@ module EE
feature_category :code_review, [:delete_description_version, :description_diff]
end
def can_run_sast_experiments_on?(project)
project.licensed_feature_available?(:sast) &&
project.feature_available?(:security_and_compliance, current_user)
end
def license_scanning_reports
reports_response(merge_request.compare_license_scanning_reports(current_user))
end
......
......@@ -13,12 +13,6 @@ RSpec.describe Projects::MergeRequestsController do
describe 'GET #show' do
before do
# To avoid adjusting this controller, we just want to allow these tests to pass. This action wasn't tested before
# these were added for the experiment, and already exceeded the threshold.
# Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/343375
# More: https://docs.gitlab.com/ee/development/query_count_limits.html#disable-query-limiting
stub_const('Gitlab::QueryLimiting::Transaction::THRESHOLD', 103)
stub_licensed_features(sast: true)
end
......@@ -28,6 +22,10 @@ RSpec.describe Projects::MergeRequestsController do
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
stub_const('Gitlab::QueryLimiting::Transaction::THRESHOLD', 110)
expect_next_instance_of(SecurityReportsMrWidgetPromptExperiment) do |instance|
expect(instance).to receive(:publish)
end
......@@ -48,7 +46,19 @@ RSpec.describe Projects::MergeRequestsController do
context 'when the project is not licensed for sast' do
before do
expect(License).to receive(:feature_available?).with(:sast).and_return(false)
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
......@@ -57,6 +67,18 @@ RSpec.describe Projects::MergeRequestsController do
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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment