diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index 378c8adc69c20447179a05456f58c4ec5cd6f1a7..17612d463e8763dabd8da3024777f326f6cfb429 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -2681,7 +2681,6 @@ Layout/LineLength: - 'lib/gitlab/usage_data.rb' - 'lib/gitlab/usage_data/topology.rb' - 'lib/gitlab/usage_data_counters/base_counter.rb' - - 'lib/gitlab/usage_data_counters/editor_unique_counter.rb' - 'lib/gitlab/usage_data_counters/gitlab_cli_activity_unique_counter.rb' - 'lib/gitlab/usage_data_counters/hll_redis_counter.rb' - 'lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb' diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index df29ac4c8bda783cfc3ba4e8541ccd8a34b2efa3..71ac3caba14591e6361c9218165bff2db39a84fa 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -4052,7 +4052,6 @@ RSpec/FeatureCategory: - 'spec/lib/gitlab/usage_data_counters/code_review_events_spec.rb' - 'spec/lib/gitlab/usage_data_counters/cycle_analytics_counter_spec.rb' - 'spec/lib/gitlab/usage_data_counters/designs_counter_spec.rb' - - 'spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb' - 'spec/lib/gitlab/usage_data_counters/gitlab_cli_activity_unique_counter_spec.rb' - 'spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb' - 'spec/lib/gitlab/usage_data_counters/ipynb_diff_activity_counter_spec.rb' diff --git a/.rubocop_todo/style/keyword_parameters_order.yml b/.rubocop_todo/style/keyword_parameters_order.yml index 8f27a2c9f4584a7b4ad27468dfff884379afb526..567796e6906efb8d7031e9fa23024b81812e40f2 100644 --- a/.rubocop_todo/style/keyword_parameters_order.yml +++ b/.rubocop_todo/style/keyword_parameters_order.yml @@ -19,5 +19,4 @@ Style/KeywordParametersOrder: - 'lib/gitlab/email/smime/signer.rb' - 'lib/gitlab/exclusive_lease.rb' - 'lib/gitlab/merge_requests/mergeability/results_store.rb' - - 'lib/gitlab/usage_data_counters/editor_unique_counter.rb' - 'lib/microsoft_teams/notifier.rb' diff --git a/app/graphql/mutations/snippets/create.rb b/app/graphql/mutations/snippets/create.rb index 1c7dbfa751d7e492fef694f4b82d619d91161fa2..a5d97022198fd1eb4cc2aa691175616394de35c2 100644 --- a/app/graphql/mutations/snippets/create.rb +++ b/app/graphql/mutations/snippets/create.rb @@ -53,7 +53,11 @@ def resolve(project_path: nil, **args) # Only when the user is not an api user and the operation was successful if !api_user? && service_response.success? - ::Gitlab::UsageDataCounters::EditorUniqueCounter.track_snippet_editor_edit_action(author: current_user, project: project) + Gitlab::InternalEvents.track_event( + 'g_edit_by_snippet_ide', + user: current_user, + project: project + ) end snippet = service_response.payload[:snippet] diff --git a/app/graphql/mutations/snippets/update.rb b/app/graphql/mutations/snippets/update.rb index 7faf9cf90191fc690751d880d9706a25c51eea83..3e241a8cf2bce09b7f9cc4356b4ad7c2d4413b6c 100644 --- a/app/graphql/mutations/snippets/update.rb +++ b/app/graphql/mutations/snippets/update.rb @@ -42,7 +42,11 @@ def resolve(id:, **args) # Only when the user is not an api user and the operation was successful if !api_user? && service_response.success? - ::Gitlab::UsageDataCounters::EditorUniqueCounter.track_snippet_editor_edit_action(author: current_user, project: snippet.project) + Gitlab::InternalEvents.track_event( + 'g_edit_by_snippet_ide', + user: current_user, + project: snippet.project + ) end snippet = service_response.payload[:snippet] diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 021b3a9437c3203551f3c455f296f62a538a5d6b..7570702444d4fbd032b277e4bea1f554226ab0d0 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -32,7 +32,8 @@ def track_commit_events return unless find_user_from_warden Gitlab::UsageDataCounters::WebIdeCounter.increment_commits_count - Gitlab::UsageDataCounters::EditorUniqueCounter.track_web_ide_edit_action(author: current_user, project: user_project) + Gitlab::InternalEvents.track_event('g_edit_by_web_ide', user: current_user, project: user_project) + namespace = user_project.namespace Gitlab::Tracking.event( diff --git a/lib/gitlab/usage_data_counters/editor_unique_counter.rb b/lib/gitlab/usage_data_counters/editor_unique_counter.rb deleted file mode 100644 index 7955c19b7e6077cb17dad86701abecc06d8ba135..0000000000000000000000000000000000000000 --- a/lib/gitlab/usage_data_counters/editor_unique_counter.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module UsageDataCounters - module EditorUniqueCounter - EDIT_BY_SNIPPET_EDITOR = 'g_edit_by_snippet_ide' - EDIT_BY_SFE = 'g_edit_by_sfe' - EDIT_BY_WEB_IDE = 'g_edit_by_web_ide' - EDIT_CATEGORY = 'ide_edit' - - class << self - def track_web_ide_edit_action(author:, project:) - track_internal_event(EDIT_BY_WEB_IDE, author, project) - end - - def count_web_ide_edit_actions(date_from:, date_to:) - count_unique(EDIT_BY_WEB_IDE, date_from, date_to) - end - - def track_sfe_edit_action(author:, project:) - track_internal_event(EDIT_BY_SFE, author, project) - end - - def count_sfe_edit_actions(date_from:, date_to:) - count_unique(EDIT_BY_SFE, date_from, date_to) - end - - def track_snippet_editor_edit_action(author:, project:) - track_internal_event(EDIT_BY_SNIPPET_EDITOR, author, project) - end - - def count_snippet_editor_edit_actions(date_from:, date_to:) - count_unique(EDIT_BY_SNIPPET_EDITOR, date_from, date_to) - end - - private - - def track_internal_event(event_name, author, project = nil) - return unless author - - Gitlab::InternalEvents.track_event( - event_name, - user: author, - project: project, - namespace: project&.namespace - ) - end - - def count_unique(actions, date_from, date_to) - Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: actions, start_date: date_from, end_date: date_to) - end - end - end - end -end diff --git a/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb deleted file mode 100644 index cbf4d3c82619abb4954f3bad7d000875075cadb6..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb +++ /dev/null @@ -1,80 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::UsageDataCounters::EditorUniqueCounter, :clean_gitlab_redis_shared_state do - let(:user) { build(:user, id: 1) } - let(:user2) { build(:user, id: 2) } - let(:user3) { build(:user, id: 3) } - let(:project) { build(:project) } - let(:namespace) { project.namespace } - let(:time) { Time.zone.now } - - shared_examples 'tracks and counts action' do - subject { track_action(author: user, project: project) } - - before do - stub_application_setting(usage_ping_enabled: true) - end - - specify do - aggregate_failures do - track_action(author: user, project: project) - track_action(author: user2, project: project) - track_action(author: user3, project: project) - - expect(count_unique(date_from: time.beginning_of_week, date_to: 1.week.from_now)).to eq(3) - end - end - - it_behaves_like 'internal event tracking' - - it 'does not track edit actions if author is not present' do - track_action(author: nil, project: project) - - expect(count_unique(date_from: time.beginning_of_week, date_to: 1.week.from_now)).to eq(0) - end - end - - context 'for web IDE edit actions' do - let(:event) { described_class::EDIT_BY_WEB_IDE } - - it_behaves_like 'tracks and counts action' do - def track_action(params) - described_class.track_web_ide_edit_action(**params) - end - - def count_unique(params) - described_class.count_web_ide_edit_actions(**params) - end - end - end - - context 'for SFE edit actions' do - let(:event) { described_class::EDIT_BY_SFE } - - it_behaves_like 'tracks and counts action' do - def track_action(params) - described_class.track_sfe_edit_action(**params) - end - - def count_unique(params) - described_class.count_sfe_edit_actions(**params) - end - end - end - - context 'for snippet editor edit actions' do - let(:event) { described_class::EDIT_BY_SNIPPET_EDITOR } - - it_behaves_like 'tracks and counts action' do - def track_action(params) - described_class.track_snippet_editor_edit_action(**params) - end - - def count_unique(params) - described_class.count_snippet_editor_edit_actions(**params) - end - end - end -end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 06ef68f190ffd81a8be25713ca08ed93b5c3e3f7..8bea823fd64566ef7e8ab2a58273011cbad82a11 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -581,7 +581,7 @@ context 'when using access token authentication' do it 'does not increment the usage counters' do expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count) - expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).not_to receive(:track_web_ide_edit_action) + expect(::Gitlab::InternalEvents).not_to receive(:track_event) post api(url, user), params: valid_c_params end @@ -596,21 +596,16 @@ it 'increments usage counters' do expect(::Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_commits_count) - expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_web_ide_edit_action) subject end it_behaves_like 'internal event tracking' do - let(:event) { ::Gitlab::UsageDataCounters::EditorUniqueCounter::EDIT_BY_WEB_IDE } + let(:event) { 'g_edit_by_web_ide' } let(:namespace) { project.namespace.reload } end context 'counts.web_ide_commits Snowplow event tracking' do - before do - allow(::Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_web_ide_edit_action) - end - it_behaves_like 'Snowplow event tracking' do let(:action) { :commit } let(:category) { described_class.to_s } diff --git a/spec/requests/api/graphql/mutations/snippets/create_spec.rb b/spec/requests/api/graphql/mutations/snippets/create_spec.rb index 7094cb807b2f127e3ca281dc436feba5908ef7ad..1b01f43b829bce993426ce0a5ee760c69c2d5b70 100644 --- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb @@ -106,7 +106,9 @@ def mutation_response end context 'with PersonalSnippet' do - it_behaves_like 'creates snippet' + it_behaves_like 'creates snippet' do + let(:project) { nil } + end end context 'with ProjectSnippet' do diff --git a/spec/requests/api/graphql/mutations/snippets/update_spec.rb b/spec/requests/api/graphql/mutations/snippets/update_spec.rb index 0bc475c7105e7fe831b173ff85b0b20bab7238f8..c84aad855982f6cdc2791886058c5cde16a5f990 100644 --- a/spec/requests/api/graphql/mutations/snippets/update_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/update_spec.rb @@ -43,7 +43,8 @@ def mutation_response shared_examples 'graphql update actions' do context 'when the user does not have permission' do - let(:current_user) { create(:user) } + let(:user) { create(:user) } + let(:current_user) { user } it_behaves_like 'a mutation that returns top-level errors', errors: [Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] @@ -131,14 +132,18 @@ def blob_at(filename) it_behaves_like 'graphql update actions' it_behaves_like 'when the snippet is not found' - it_behaves_like 'snippet edit usage data counters' + it_behaves_like 'snippet edit usage data counters' do + let(:user) { current_user } + end + it_behaves_like 'has spam protection' do let(:mutation_class) { ::Mutations::Snippets::Update } end end describe 'ProjectSnippet' do - let_it_be(:project) { create(:project, :private) } + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project) { create(:project, :private, namespace: namespace) } let(:snippet) do create( @@ -181,7 +186,9 @@ def blob_at(filename) end end - it_behaves_like 'snippet edit usage data counters' + it_behaves_like 'snippet edit usage data counters' do + let(:user) { current_user } + end it_behaves_like 'has spam protection' do let(:mutation_class) { ::Mutations::Snippets::Update } @@ -193,9 +200,8 @@ def blob_at(filename) end it_behaves_like 'internal event tracking' do - let(:event) { ::Gitlab::UsageDataCounters::EditorUniqueCounter::EDIT_BY_SNIPPET_EDITOR } + let(:event) { 'g_edit_by_snippet_ide' } let(:user) { current_user } - let(:namespace) { project.namespace } end end end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 91d423fcd67915f51ebf70f2448212f7e2950b4f..a11f2cdb41bfb01f5274bafdff3ff5da04cb367b 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -6727,7 +6727,6 @@ - './spec/lib/gitlab/usage_data_counters/code_review_events_spec.rb' - './spec/lib/gitlab/usage_data_counters/cycle_analytics_counter_spec.rb' - './spec/lib/gitlab/usage_data_counters/designs_counter_spec.rb' -- './spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb' - './spec/lib/gitlab/usage_data_counters/gitlab_cli_activity_unique_counter_spec.rb' - './spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb' - './spec/lib/gitlab/usage_data_counters/ipynb_diff_activity_counter_spec.rb' diff --git a/spec/support/shared_examples/controllers/internal_event_tracking_examples.rb b/spec/support/shared_examples/controllers/internal_event_tracking_examples.rb index 0c19865999f5605912f85dc418d67eef6694e8ea..2332d6cd2d0ac1a96725d764eaf4d2f3de65b64e 100644 --- a/spec/support/shared_examples/controllers/internal_event_tracking_examples.rb +++ b/spec/support/shared_examples/controllers/internal_event_tracking_examples.rb @@ -23,7 +23,7 @@ project = try(:project) user = try(:user) - namespace = try(:namespace) + namespace = try(:namespace) || project&.namespace expect(Gitlab::Tracking::StandardContext) .to have_received(:new) diff --git a/spec/support/shared_examples/requests/api/graphql/mutations/snippets_shared_examples.rb b/spec/support/shared_examples/requests/api/graphql/mutations/snippets_shared_examples.rb index 53329c5caec1a818ab0c3b1b5797117c994ad53a..8ed3464b009034408e3e9067425a64a2e8cffdf3 100644 --- a/spec/support/shared_examples/requests/api/graphql/mutations/snippets_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/graphql/mutations/snippets_shared_examples.rb @@ -14,7 +14,7 @@ context 'when user is sessionless' do it 'does not track usage data actions' do - expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).not_to receive(:track_snippet_editor_edit_action) + expect(::Gitlab::InternalEvents).not_to receive(:track_event) post_graphql_mutation(mutation, current_user: current_user) end @@ -25,17 +25,19 @@ stub_session('warden.user.user.key' => [[current_user.id], current_user.authenticatable_salt]) end - it 'tracks usage data actions', :clean_gitlab_redis_sessions do - expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).to receive(:track_snippet_editor_edit_action) - + subject do post_graphql_mutation(mutation) end + it_behaves_like 'internal event tracking' do + let(:event) { 'g_edit_by_snippet_ide' } + end + context 'when mutation result raises an error' do it 'does not track usage data actions' do mutation_vars[:title] = nil - expect(::Gitlab::UsageDataCounters::EditorUniqueCounter).not_to receive(:track_snippet_editor_edit_action) + expect(::Gitlab::InternalEvents).not_to receive(:track_event) post_graphql_mutation(mutation) end