Skip to content

G
gitlab-ce
Commit 1f781bb3 authored by Peter Leitzen's avatar Peter Leitzen
Browse files
Options

Move Tracing operations controller to Core 

This commit moves the tracing operations controller from Ultimate to
Core.
parent 0991f5a7
Hide whitespace changes
Inline Side-by-side
Showing with 146 additions and 224 deletions
app/controllers/projects/settings/operations_controller.rb
View file @ 1f781bb3
......@@ -9,6 +9,7 @@ module Projects
respond_to :json, only: [:reset_alerting_token, :reset_pagerduty_token]
helper_method :error_tracking_setting
helper_method :tracing_setting
def update
result = ::Projects::Operations::UpdateService.new(project, current_user, update_params).execute
......@@ -17,15 +18,6 @@ module Projects
render_update_response(result)
end
# overridden in EE
def track_events(result)
if result[:status] == :success
::Gitlab::Tracking::IncidentManagement.track_from_params(
update_params[:incident_management_setting_attributes]
)
end
end
def reset_alerting_token
result = ::Projects::Operations::UpdateService
.new(project, current_user, alerting_params)
......@@ -55,6 +47,24 @@ module Projects
private
def track_events(result)
if result[:status] == :success
::Gitlab::Tracking::IncidentManagement.track_from_params(
update_params[:incident_management_setting_attributes]
)
track_tracing_external_url
end
end
def track_tracing_external_url
external_url_previous_change = project&.tracing_setting&.external_url_previous_change
return unless external_url_previous_change
return unless external_url_previous_change[0].blank? && external_url_previous_change[1].present?
::Gitlab::Tracking.event('project:operations:tracing', 'external_url_populated')
end
def alerting_params
{ alerting_setting_attributes: { regenerate_token: true } }
end
......@@ -106,6 +116,10 @@ module Projects
project.build_error_tracking_setting
end
def tracing_setting
@tracing_setting ||= project.tracing_setting || project.build_tracing_setting
end
def update_params
params.require(:project).permit(permitted_project_params)
end
......@@ -124,7 +138,8 @@ module Projects
project: [:slug, :name, :organization_slug, :organization_name]
],
grafana_integration_attributes: [:token, :grafana_url, :enabled]
grafana_integration_attributes: [:token, :grafana_url, :enabled],
tracing_setting_attributes: [:external_url]
}
if Feature.enabled?(:settings_operations_prometheus_service, project)
......
ee/app/controllers/ee/projects/settings/operations_controller.rb
View file @ 1f781bb3
......@@ -8,43 +8,23 @@ module EE
extend ActiveSupport::Concern
prepended do
helper_method :tracing_setting, :status_page_setting
helper_method :status_page_setting
private
def tracing_setting
@tracing_setting ||= project.tracing_setting || project.build_tracing_setting
end
def status_page_setting
@status_page_setting ||= project.status_page_setting || project.build_status_page_setting
end
def has_tracing_license?
project.feature_available?(:tracing, current_user)
end
def has_status_page_license?
project.feature_available?(:status_page, current_user)
end
def track_tracing_external_url
external_url_previous_change = project&.tracing_setting&.external_url_previous_change
return unless external_url_previous_change
return unless external_url_previous_change[0].blank? && external_url_previous_change[1].present?
::Gitlab::Tracking.event('project:operations:tracing', "external_url_populated")
end
end
override :permitted_project_params
def permitted_project_params
permitted_params = super
if has_tracing_license?
permitted_params[:tracing_setting_attributes] = [:external_url]
end
if has_status_page_license?
permitted_params.merge!(status_page_setting_params)
end
......@@ -55,13 +35,6 @@ module EE
def status_page_setting_params
{ status_page_setting_attributes: [:status_page_url, :aws_s3_bucket_name, :aws_region, :aws_access_key, :aws_secret_key, :enabled] }
end
override :track_events
def track_events(result)
super
track_tracing_external_url if result[:status] == :success
end
end
end
end
......
ee/spec/controllers/projects/settings/operations_controller_spec.rb
View file @ 1f781bb3
......@@ -3,17 +3,21 @@
require 'spec_helper'
RSpec.describe Projects::Settings::OperationsController do
let_it_be_with_reload(:project) { create(:project) }
let_it_be(:user) { create(:user) }
before do
sign_in(user)
end
before_all do
project.add_maintainer(user)
end
describe 'GET show' do
shared_examples 'user without read access' do |project_visibility, project_role|
let(:project) { create(:project, project_visibility) }
before do
project.update!(visibility: project_visibility.to_s)
project.add_role(user, project_role)
end
......@@ -25,9 +29,8 @@ RSpec.describe Projects::Settings::OperationsController do
end
shared_examples 'user with read access' do |project_visibility|
let(:project) { create(:project, project_visibility) }
before do
project.update!(visibility: project_visibility.to_s)
project.add_maintainer(user)
end
......@@ -40,9 +43,11 @@ RSpec.describe Projects::Settings::OperationsController do
end
shared_examples 'user needs to login' do |project_visibility|
it 'redirects for private project' do
project = create(:project, project_visibility)
before do
project.update!(visibility: project_visibility.to_s)
end
it 'redirects for private project' do
get :show, params: project_params(project)
expect(response).to redirect_to(new_user_session_path)
......@@ -51,7 +56,7 @@ RSpec.describe Projects::Settings::OperationsController do
context 'with a license' do
before do
stub_licensed_features(tracing: true, incident_management: true)
stub_licensed_features(status_page: true)
end
context 'with maintainer role' do
......@@ -82,7 +87,7 @@ RSpec.describe Projects::Settings::OperationsController do
context 'without license' do
before do
stub_licensed_features(tracing: false, incident_management: false)
stub_licensed_features(status_page: false)
end
it_behaves_like 'user with read access', :public
......@@ -92,32 +97,12 @@ RSpec.describe Projects::Settings::OperationsController do
end
describe 'PATCH update' do
let(:public_project) { create(:project, :public) }
let(:private_project) { create(:project, :private) }
let(:internal_project) { create(:project, :internal) }
before do
public_project.add_maintainer(user)
private_project.add_maintainer(user)
internal_project.add_maintainer(user)
end
shared_examples 'user without write access' do |project_visibility, project_role|
let(:project) { create(:project, project_visibility) }
before do
project.update!(visibility: project_visibility.to_s)
project.add_role(user, project_role)
end
it 'does not create tracing_setting' do
update_project(
project,
tracing_params: { external_url: 'https://gitlab.com' }
)
expect(project.tracing_setting).to be_nil
end
it 'does not create status_page_setting' do
update_project(
project,
......@@ -128,70 +113,9 @@ RSpec.describe Projects::Settings::OperationsController do
end
end
context 'format html' do
let(:project) { create(:project) }
let(:operations_update_service) { spy(:operations_update_service) }
before do
stub_licensed_features(tracing: true)
project.add_maintainer(user)
end
context 'when update succeeds' do
before do
stub_operations_update_service_returning(status: :success)
end
it 'shows a notice' do
update_project(project, tracing_params: { external_url: 'http://gitlab.com' })
expect(response).to redirect_to(project_settings_operations_url(project))
expect(flash[:notice]).to eq _('Your changes have been saved')
end
end
context 'when update fails' do
before do
stub_operations_update_service_returning(status: :error)
end
it 'renders show page' do
update_project(project, tracing_params: { external_url: 'http://gitlab.com' })
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template(:show)
end
end
end
context 'with a license' do
before do
stub_licensed_features(tracing: true, incident_management: true, status_page: true)
end
shared_examples 'user with write access' do |project_visibility|
let(:project) { create(:project, project_visibility) }
let(:tracing_url) { 'https://gitlab.com' }
before do
project.add_maintainer(user)
end
it 'creates tracing setting' do
update_project(
project,
tracing_params: { external_url: tracing_url }
)
expect(project.tracing_setting.external_url).to eq(tracing_url)
end
end
context 'with maintainer role' do
it_behaves_like 'user with write access', :public
it_behaves_like 'user with write access', :private
it_behaves_like 'user with write access', :internal
stub_licensed_features(status_page: true)
end
context 'with non maintainer roles' do
......@@ -214,80 +138,7 @@ RSpec.describe Projects::Settings::OperationsController do
it_behaves_like 'user without write access', :internal, :maintainer
end
context 'with existing tracing_setting' do
let(:project) { create(:project) }
before do
project.create_tracing_setting!(external_url: 'https://gitlab.com')
project.add_maintainer(user)
end
it 'unsets external_url with nil' do
update_project(project, tracing_params: { external_url: nil })
expect(project.tracing_setting).to be_nil
end
it 'unsets external_url with empty string' do
update_project(project, tracing_params: { external_url: '' })
expect(project.tracing_setting).to be_nil
end
it 'fails validation with invalid url' do
expect do
update_project(project, tracing_params: { external_url: "invalid" })
end.not_to change(project.tracing_setting, :external_url)
end
it 'does not set external_url if not present in params' do
expect do
update_project(project, tracing_params: { some_param: 'some_value' })
end.not_to change(project.tracing_setting, :external_url)
end
end
context 'without existing tracing_setting' do
let(:project) { create(:project) }
before do
project.add_maintainer(user)
end
it 'fails validation with invalid url' do
update_project(project, tracing_params: { external_url: "invalid" })
expect(project.tracing_setting).to be_nil
end
it 'does not set external_url if not present in params' do
update_project(project, tracing_params: { some_param: 'some_value' })
expect(project.tracing_setting).to be_nil
end
end
context 'updating tracing settings' do
let(:project) { create(:project) }
let(:new_tracing_settings) { {} }
before do
project.add_maintainer(user)
end
it 'creates a gitlab tracking event' do
expect(Gitlab::Tracking).to receive(:event).with('project:operations:tracing', 'external_url_populated')
update_project(project, tracing_params: { external_url: "http://example.com" } )
end
end
context 'without existing status page setting' do
let(:project) { create(:project) }
before do
project.add_maintainer(user)
end
subject(:status_page_setting) do
valid_attributes = attributes_for(:status_page_setting).except(:enabled)
update_project(project, status_page_params: valid_attributes )
......@@ -299,11 +150,9 @@ RSpec.describe Projects::Settings::OperationsController do
end
context 'with existing status page setting' do
let(:project) { create(:project) }
let(:status_page_attributes) { attributes_for(:status_page_setting) }
before do
project.add_maintainer(user)
project.create_status_page_setting!(status_page_attributes)
end
......@@ -336,7 +185,7 @@ RSpec.describe Projects::Settings::OperationsController do
context 'without a license' do
before do
stub_licensed_features(tracing: false, incident_management: false, status_page: false)
stub_licensed_features(status_page: false)
end
it_behaves_like 'user without write access', :public, :maintainer
......@@ -346,11 +195,9 @@ RSpec.describe Projects::Settings::OperationsController do
private
def update_project(project, tracing_params: nil, incident_management_params: nil, status_page_params: nil)
def update_project(project, status_page_params: nil)
patch :update, params: project_params(
project,
tracing_params: tracing_params,
incident_management_params: incident_management_params,
status_page_params: status_page_params
)
......@@ -360,13 +207,11 @@ RSpec.describe Projects::Settings::OperationsController do
private
def project_params(project, tracing_params: nil, incident_management_params: nil, status_page_params: nil)
def project_params(project, status_page_params: nil)
{
namespace_id: project.namespace,
project_id: project,
project: {
tracing_setting_attributes: tracing_params,
incident_management_setting_attributes: incident_management_params,
status_page_setting_attributes: status_page_params
}
}
......
spec/controllers/projects/settings/operations_controller_spec.rb
View file @ 1f781bb3
......@@ -6,9 +6,12 @@ RSpec.describe Projects::Settings::OperationsController do
let_it_be(:user) { create(:user) }
let_it_be(:project, reload: true) { create(:project) }
before_all do
project.add_maintainer(user)
end
before do
sign_in(user)
project.add_maintainer(user)
end
shared_examples 'PATCHable' do
......@@ -163,10 +166,6 @@ RSpec.describe Projects::Settings::OperationsController do
context 'updating each incident management setting' do
let(:new_incident_management_settings) { {} }
before do
project.add_maintainer(user)
end
shared_examples 'a gitlab tracking event' do |params, event_key|
it "creates a gitlab tracking event #{event_key}" do
new_incident_management_settings = params
......@@ -194,10 +193,6 @@ RSpec.describe Projects::Settings::OperationsController do
end
describe 'POST #reset_pagerduty_token' do
before do
project.add_maintainer(user)
end
context 'with existing incident management setting has active PagerDuty webhook' do
let!(:incident_management_setting) do
create(:project_incident_management_setting, project: project, pagerduty_active: true)
......@@ -392,10 +387,6 @@ RSpec.describe Projects::Settings::OperationsController do
end
describe 'POST #reset_alerting_token' do
before do
project.add_maintainer(user)
end
context 'with existing alerting setting' do
let!(:alerting_setting) do
create(:project_alerting_setting, project: project)
......@@ -478,6 +469,104 @@ RSpec.describe Projects::Settings::OperationsController do
end
end
context 'tracing integration' do
describe 'GET #show' do
context 'with existing setting' do
let_it_be(:setting) do
create(:project_tracing_setting, project: project)
end
it 'loads existing setting' do
get :show, params: project_params(project)
expect(controller.helpers.tracing_setting).to eq(setting)
end
end
context 'without an existing setting' do
it 'builds a new setting' do
get :show, params: project_params(project)
expect(controller.helpers.tracing_setting).to be_new_record
end
end
end
describe 'PATCH #update' do
let_it_be(:external_url) { 'https://gitlab.com' }
let(:params) do
{
tracing_setting_attributes: {
external_url: external_url
}
}
end
it_behaves_like 'PATCHable'
describe 'gitlab tracking', :snowplow do
shared_examples 'event tracking' do
it 'tracks an event' do
expect_snowplow_event(
category: 'project:operations:tracing',
action: 'external_url_populated'
)
end
end
shared_examples 'no event tracking' do
it 'does not track an event' do
expect_no_snowplow_event
end
end
before do
make_request
end
subject(:make_request) do
patch :update, params: project_params(project, params), format: :json
end
context 'without existing setting' do
context 'when creating a new setting' do
it_behaves_like 'event tracking'
end
context 'with invalid external_url' do
let_it_be(:external_url) { nil }
it_behaves_like 'no event tracking'
end
end
context 'with existing setting' do
let_it_be(:existing_setting) do
create(:project_tracing_setting,
project: project,
external_url: external_url)
end
context 'when changing external_url' do
let_it_be(:external_url) { 'https://example.com' }
it_behaves_like 'no event tracking'
end
context 'with unchanged external_url' do
it_behaves_like 'no event tracking'
end
context 'with invalid external_url' do
let_it_be(:external_url) { nil }
it_behaves_like 'no event tracking'
end
end
end
end
end
private
def project_params(project, params = {})
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7