From 22e8212298b47e33bac02c95759bb86d066c09e5 Mon Sep 17 00:00:00 2001 From: Alishan Ladhani <aladhani@gitlab.com> Date: Tue, 9 Mar 2021 21:52:15 -0500 Subject: [PATCH] Remove Snowplow controller concern and usages Part of Snowplow standardization effort --- app/controllers/application_controller.rb | 1 - .../groups/email_campaigns_controller.rb | 3 +- app/helpers/packages_helper.rb | 3 +- .../concerns/group_invite_members.rb | 2 +- .../analytics/coverage_reports_controller.rb | 2 +- .../repository_analytics_controller.rb | 2 +- .../survey_responses_controller.rb | 4 +- .../survey_responses_controller_spec.rb | 4 +- lib/gitlab/tracking.rb | 15 ------- .../application_controller_spec.rb | 2 - .../projects/registry/tags_controller_spec.rb | 7 ++-- .../controllers/trackable_shared_examples.rb | 39 ------------------- 12 files changed, 13 insertions(+), 71 deletions(-) delete mode 100644 spec/support/shared_examples/controllers/trackable_shared_examples.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 379da90827a..bf1dfcd6416 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -16,7 +16,6 @@ class ApplicationController < ActionController::Base include SessionlessAuthentication include SessionsHelper include ConfirmEmailWarning - include Gitlab::Tracking::ControllerConcern include Gitlab::Experimentation::ControllerConcern include InitializesCurrentUserMode include Impersonation diff --git a/app/controllers/groups/email_campaigns_controller.rb b/app/controllers/groups/email_campaigns_controller.rb index cbb0176ea7b..8c3c289dc99 100644 --- a/app/controllers/groups/email_campaigns_controller.rb +++ b/app/controllers/groups/email_campaigns_controller.rb @@ -2,7 +2,6 @@ class Groups::EmailCampaignsController < Groups::ApplicationController include InProductMarketingHelper - include Gitlab::Tracking::ControllerConcern EMAIL_CAMPAIGNS_SCHEMA_URL = 'iglu:com.gitlab/email_campaigns/jsonschema/1-0-0' @@ -25,7 +24,7 @@ class Groups::EmailCampaignsController < Groups::ApplicationController subject_line: subject_line(@track, @series) } - track_self_describing_event(EMAIL_CAMPAIGNS_SCHEMA_URL, data: data) + ::Gitlab::Tracking.self_describing_event(EMAIL_CAMPAIGNS_SCHEMA_URL, data: data) end def redirect_link diff --git a/app/helpers/packages_helper.rb b/app/helpers/packages_helper.rb index 8f365fd0786..04465f7798c 100644 --- a/app/helpers/packages_helper.rb +++ b/app/helpers/packages_helper.rb @@ -50,6 +50,7 @@ module PackagesHelper def track_package_event(event_name, scope, **args) ::Packages::CreateEventService.new(nil, current_user, event_name: event_name, scope: scope).execute - track_event(event_name, **args) + category = args.delete(:category) || self.class.name + ::Gitlab::Tracking.event(category, event_name.to_s, **args) end end diff --git a/ee/app/controllers/concerns/group_invite_members.rb b/ee/app/controllers/concerns/group_invite_members.rb index b8030e7f052..457c114ca96 100644 --- a/ee/app/controllers/concerns/group_invite_members.rb +++ b/ee/app/controllers/concerns/group_invite_members.rb @@ -11,7 +11,7 @@ module GroupInviteMembers result = Members::CreateService.new(current_user, invite_params).execute(group) - track_event('invite_members', label: 'new_group_form') if result[:status] == :success + ::Gitlab::Tracking.event(self.class.name, 'invite_members', label: 'new_group_form') if result[:status] == :success end def emails_param diff --git a/ee/app/controllers/groups/analytics/coverage_reports_controller.rb b/ee/app/controllers/groups/analytics/coverage_reports_controller.rb index 2f06788e355..4eb8669b163 100644 --- a/ee/app/controllers/groups/analytics/coverage_reports_controller.rb +++ b/ee/app/controllers/groups/analytics/coverage_reports_controller.rb @@ -11,7 +11,7 @@ class Groups::Analytics::CoverageReportsController < Groups::Analytics::Applicat def index respond_to do |format| format.csv do - track_event(:download_code_coverage_csv, **download_tracker_params) + ::Gitlab::Tracking.event(self.class.name, 'download_code_coverage_csv', **download_tracker_params) send_data(render_csv(report_results), type: 'text/csv; charset=utf-8') end end diff --git a/ee/app/controllers/groups/analytics/repository_analytics_controller.rb b/ee/app/controllers/groups/analytics/repository_analytics_controller.rb index f3a871f700d..8a4a39891b2 100644 --- a/ee/app/controllers/groups/analytics/repository_analytics_controller.rb +++ b/ee/app/controllers/groups/analytics/repository_analytics_controller.rb @@ -12,7 +12,7 @@ class Groups::Analytics::RepositoryAnalyticsController < Groups::Analytics::Appl end def show - track_event(**pageview_tracker_params) + Gitlab::Tracking.event(self.class.name, 'show', **pageview_tracker_params) end private diff --git a/ee/app/controllers/survey_responses_controller.rb b/ee/app/controllers/survey_responses_controller.rb index e352a0e9722..d0971833a91 100644 --- a/ee/app/controllers/survey_responses_controller.rb +++ b/ee/app/controllers/survey_responses_controller.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class SurveyResponsesController < ApplicationController - include Gitlab::Tracking::ControllerConcern - SURVEY_RESPONSE_SCHEMA_URL = 'iglu:com.gitlab/survey_response/jsonschema/1-0-0' skip_before_action :authenticate_user! @@ -28,7 +26,7 @@ class SurveyResponsesController < ApplicationController response: params[:response] }.compact - track_self_describing_event(SURVEY_RESPONSE_SCHEMA_URL, data: data) + ::Gitlab::Tracking.self_describing_event(SURVEY_RESPONSE_SCHEMA_URL, data: data) end def to_number(param) diff --git a/ee/spec/controllers/survey_responses_controller_spec.rb b/ee/spec/controllers/survey_responses_controller_spec.rb index 0f5de3de546..0f15dc8ce07 100644 --- a/ee/spec/controllers/survey_responses_controller_spec.rb +++ b/ee/spec/controllers/survey_responses_controller_spec.rb @@ -21,7 +21,7 @@ RSpec.describe SurveyResponsesController do end it 'tracks a survey_response event' do - expect(controller).to receive(:track_self_describing_event).with( + expect(Gitlab::Tracking).to receive(:self_describing_event).with( SurveyResponsesController::SURVEY_RESPONSE_SCHEMA_URL, data: { survey_id: 1, response: 'bar' } ) @@ -32,7 +32,7 @@ RSpec.describe SurveyResponsesController do context 'not on GitLab.com' do it 'does not track a survey_response event' do - expect(controller).not_to receive(:track_self_describing_event) + expect(Gitlab::Tracking).not_to receive(:self_describing_event) subject end diff --git a/lib/gitlab/tracking.rb b/lib/gitlab/tracking.rb index 9bb793a75cc..1b64c97b6b5 100644 --- a/lib/gitlab/tracking.rb +++ b/lib/gitlab/tracking.rb @@ -4,21 +4,6 @@ module Gitlab module Tracking SNOWPLOW_NAMESPACE = 'gl' - module ControllerConcern - extend ActiveSupport::Concern - - protected - - def track_event(action = action_name, **args) - category = args.delete(:category) || self.class.name - Gitlab::Tracking.event(category, action.to_s, **args) - end - - def track_self_describing_event(schema_url, data:, **args) - Gitlab::Tracking.self_describing_event(schema_url, data: data, **args) - end - end - class << self def enabled? Gitlab::CurrentSettings.snowplow_enabled? diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 4a729008e67..4809a07f6e9 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -56,8 +56,6 @@ RSpec.describe ApplicationController do end end - it_behaves_like 'a Trackable Controller' - describe '#add_gon_variables' do before do Gon.clear diff --git a/spec/controllers/projects/registry/tags_controller_spec.rb b/spec/controllers/projects/registry/tags_controller_spec.rb index 5bff89b4308..c03a280d2cd 100644 --- a/spec/controllers/projects/registry/tags_controller_spec.rb +++ b/spec/controllers/projects/registry/tags_controller_spec.rb @@ -42,7 +42,7 @@ RSpec.describe Projects::Registry::TagsController do it 'tracks the event', :snowplow do get_tags - expect_snowplow_event(category: anything, action: 'list_tags') + expect_snowplow_event(category: 'Projects::Registry::TagsController', action: 'list_tags') end end @@ -107,11 +107,12 @@ RSpec.describe Projects::Registry::TagsController do destroy_tag('test.') end - it 'tracks the event' do + it 'tracks the event', :snowplow do expect_delete_tags(%w[test.]) - expect(controller).to receive(:track_event).with(:delete_tag) destroy_tag('test.') + + expect_snowplow_event(category: 'Projects::Registry::TagsController', action: 'delete_tag') end end end diff --git a/spec/support/shared_examples/controllers/trackable_shared_examples.rb b/spec/support/shared_examples/controllers/trackable_shared_examples.rb deleted file mode 100644 index dac7d8c94ff..00000000000 --- a/spec/support/shared_examples/controllers/trackable_shared_examples.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -RSpec.shared_examples 'a Trackable Controller' do - describe '#track_event', :snowplow do - before do - sign_in user - end - - context 'with no params' do - controller(described_class) do - def index - track_event - head :ok - end - end - - it 'tracks the action name', :snowplow do - get :index - - expect_snowplow_event(category: 'AnonymousController', action: 'index') - end - end - - context 'with params' do - controller(described_class) do - def index - track_event('some_event', category: 'SomeCategory', label: 'errorlabel') - head :ok - end - end - - it 'tracks with the specified param' do - get :index - - expect_snowplow_event(category: 'SomeCategory', action: 'some_event', label: 'errorlabel') - end - end - end -end -- 2.30.9