From 01685eed7674ec841b4249b42f9b350f4a105e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= Date: Fri, 19 Jul 2019 11:11:27 +0000 Subject: Added Usage Data for some Web IDE actions The actions tracked in the web IDE are: - creation of commits - creation of merge requests - projects loaded --- app/assets/javascripts/ide/stores/utils.js | 2 +- app/controllers/ide_controller.rb | 1 + .../merge_requests/creations_controller.rb | 8 +++++ .../creations/_new_compare.html.haml | 2 ++ .../merge_requests/creations/_new_submit.html.haml | 3 ++ .../unreleased/fj-count-web-ide-merge-requests.yml | 5 +++ lib/gitlab/usage_data.rb | 4 ++- lib/gitlab/usage_data_counters/web_ide_counter.rb | 18 ++++++++++ spec/controllers/ide_controller_spec.rb | 17 +++++++++ .../merge_requests/creations_controller_spec.rb | 42 ++++++++++++++++++++++ .../ide/stores/modules/commit/actions_spec.js | 2 +- .../usage_data_counters/web_ide_counter_spec.rb | 35 ++++++++++++------ spec/lib/gitlab/usage_data_spec.rb | 10 ++++++ 13 files changed, 136 insertions(+), 13 deletions(-) create mode 100644 changelogs/unreleased/fj-count-web-ide-merge-requests.yml create mode 100644 spec/controllers/ide_controller_spec.rb diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js index 04470064c1f..366314536c6 100644 --- a/app/assets/javascripts/ide/stores/utils.js +++ b/app/assets/javascripts/ide/stores/utils.js @@ -159,7 +159,7 @@ export const createCommitPayload = ({ }); export const createNewMergeRequestUrl = (projectUrl, source, target) => - `${projectUrl}/merge_requests/new?merge_request[source_branch]=${source}&merge_request[target_branch]=${target}`; + `${projectUrl}/merge_requests/new?merge_request[source_branch]=${source}&merge_request[target_branch]=${target}&nav_source=webide`; const sortTreesByTypeAndName = (a, b) => { if (a.type === 'tree' && b.type === 'blob') { diff --git a/app/controllers/ide_controller.rb b/app/controllers/ide_controller.rb index eeeebe430a7..af1e6cc703b 100644 --- a/app/controllers/ide_controller.rb +++ b/app/controllers/ide_controller.rb @@ -4,5 +4,6 @@ class IdeController < ApplicationController layout 'fullscreen' def index + Gitlab::UsageDataCounters::WebIdeCounter.increment_views_count end end diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index 32cefe54613..6ac5bb90706 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -23,6 +23,8 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap @merge_request = ::MergeRequests::CreateService.new(project, current_user, merge_request_params).execute if @merge_request.valid? + incr_count_webide_merge_request + redirect_to(merge_request_path(@merge_request)) else @source_project = @merge_request.source_project @@ -135,4 +137,10 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap def whitelist_query_limiting Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42384') end + + def incr_count_webide_merge_request + return if params[:nav_source] != 'webide' + + Gitlab::UsageDataCounters::WebIdeCounter.increment_merge_requests_count + end end diff --git a/app/views/projects/merge_requests/creations/_new_compare.html.haml b/app/views/projects/merge_requests/creations/_new_compare.html.haml index 11272a67f93..be01905dd35 100644 --- a/app/views/projects/merge_requests/creations/_new_compare.html.haml +++ b/app/views/projects/merge_requests/creations/_new_compare.html.haml @@ -2,6 +2,8 @@ New Merge Request = form_for [@project.namespace.becomes(Namespace), @project, @merge_request], url: project_new_merge_request_path(@project), method: :get, html: { class: "merge-request-form js-requires-input" } do |f| + - if params[:nav_source].present? + = hidden_field_tag(:nav_source, params[:nav_source]) .hide.alert.alert-danger.mr-compare-errors .js-merge-request-new-compare.row{ 'data-source-branch-url': project_new_merge_request_branch_from_path(@source_project), 'data-target-branch-url': project_new_merge_request_branch_to_path(@source_project) } .col-lg-6 diff --git a/app/views/projects/merge_requests/creations/_new_submit.html.haml b/app/views/projects/merge_requests/creations/_new_submit.html.haml index 464f8fa65e9..543441b9479 100644 --- a/app/views/projects/merge_requests/creations/_new_submit.html.haml +++ b/app/views/projects/merge_requests/creations/_new_submit.html.haml @@ -17,6 +17,9 @@ = f.hidden_field :target_project_id = f.hidden_field :target_branch, id: '' + - if params[:nav_source].present? + = hidden_field_tag(:nav_source, params[:nav_source]) + .mr-compare.merge-request.js-merge-request-new-submit{ 'data-mr-submit-action': "#{j params[:tab].presence || 'new'}" } - if @commits.empty? .commits-empty diff --git a/changelogs/unreleased/fj-count-web-ide-merge-requests.yml b/changelogs/unreleased/fj-count-web-ide-merge-requests.yml new file mode 100644 index 00000000000..adcd0af37e8 --- /dev/null +++ b/changelogs/unreleased/fj-count-web-ide-merge-requests.yml @@ -0,0 +1,5 @@ +--- +title: Add Web IDE Usage Ping for Create SMAU +merge_request: 30800 +author: +type: changed diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 7572c0bdbfd..43072053e57 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -130,7 +130,9 @@ module Gitlab def usage_counters { - web_ide_commits: Gitlab::UsageDataCounters::WebIdeCounter.total_commits_count + web_ide_commits: Gitlab::UsageDataCounters::WebIdeCounter.total_commits_count, + web_ide_merge_requests: Gitlab::UsageDataCounters::WebIdeCounter.total_merge_requests_count, + web_ide_views: Gitlab::UsageDataCounters::WebIdeCounter.total_views_count } end diff --git a/lib/gitlab/usage_data_counters/web_ide_counter.rb b/lib/gitlab/usage_data_counters/web_ide_counter.rb index 6fbffb94c58..899ad0db9c0 100644 --- a/lib/gitlab/usage_data_counters/web_ide_counter.rb +++ b/lib/gitlab/usage_data_counters/web_ide_counter.rb @@ -6,6 +6,8 @@ module Gitlab extend RedisCounter COMMITS_COUNT_KEY = 'WEB_IDE_COMMITS_COUNT' + MERGE_REQUEST_COUNT_KEY = 'WEB_IDE_MERGE_REQUESTS_COUNT' + VIEWS_COUNT_KEY = 'WEB_IDE_VIEWS_COUNT' class << self def increment_commits_count @@ -15,6 +17,22 @@ module Gitlab def total_commits_count total_count(COMMITS_COUNT_KEY) end + + def increment_merge_requests_count + increment(MERGE_REQUEST_COUNT_KEY) + end + + def total_merge_requests_count + total_count(MERGE_REQUEST_COUNT_KEY) + end + + def increment_views_count + increment(VIEWS_COUNT_KEY) + end + + def total_views_count + total_count(VIEWS_COUNT_KEY) + end end end end diff --git a/spec/controllers/ide_controller_spec.rb b/spec/controllers/ide_controller_spec.rb new file mode 100644 index 00000000000..0462f9520d5 --- /dev/null +++ b/spec/controllers/ide_controller_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe IdeController do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'increases the views counter' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_views_count) + + get :index + end +end diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index 5fefad86ef3..3816e1c7a31 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -212,4 +212,46 @@ describe Projects::MergeRequests::CreationsController do expect(response).to have_gitlab_http_status(200) end end + + describe 'POST create' do + let(:params) do + { + namespace_id: fork_project.namespace.to_param, + project_id: fork_project, + merge_request: { + title: 'Test merge request', + source_branch: 'remove-submodule', + target_branch: 'master' + } + } + end + + it 'creates merge request' do + expect do + post_request(params) + end.to change { MergeRequest.count }.by(1) + end + + context 'when the merge request is not created from the web ide' do + it 'counter is not increased' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_merge_requests_count) + + post_request(params) + end + end + + context 'when the merge request is created from the web ide' do + let(:nav_source) { { nav_source: 'webide' } } + + it 'counter is increased' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_merge_requests_count) + + post_request(params.merge(nav_source)) + end + end + + def post_request(merge_request_params) + post :create, params: merge_request_params + end + end end diff --git a/spec/javascripts/ide/stores/modules/commit/actions_spec.js b/spec/javascripts/ide/stores/modules/commit/actions_spec.js index 590af53b3f2..14d861f21d2 100644 --- a/spec/javascripts/ide/stores/modules/commit/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/commit/actions_spec.js @@ -411,7 +411,7 @@ describe('IDE commit module actions', () => { expect(visitUrl).toHaveBeenCalledWith( `webUrl/merge_requests/new?merge_request[source_branch]=${ store.getters['commit/placeholderBranchName'] - }&merge_request[target_branch]=master`, + }&merge_request[target_branch]=master&nav_source=webide`, ); done(); diff --git a/spec/lib/gitlab/usage_data_counters/web_ide_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/web_ide_counter_spec.rb index fa0cf15e1b2..b5e32d1875f 100644 --- a/spec/lib/gitlab/usage_data_counters/web_ide_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/web_ide_counter_spec.rb @@ -3,19 +3,34 @@ require 'spec_helper' describe Gitlab::UsageDataCounters::WebIdeCounter, :clean_gitlab_redis_shared_state do - describe '.increment_commits_count' do - it 'increments the web ide commits counter by 1' do - expect do - described_class.increment_commits_count - end.to change { described_class.total_commits_count }.by(1) + shared_examples 'counter examples' do + it 'increments counter and return the total count' do + expect(described_class.public_send(total_counter_method)).to eq(0) + + 2.times { described_class.public_send(increment_counter_method) } + + expect(described_class.public_send(total_counter_method)).to eq(2) end end - describe '.total_commits_count' do - it 'returns the total amount of web ide commits' do - 2.times { described_class.increment_commits_count } + describe 'commits counter' do + let(:increment_counter_method) { :increment_commits_count } + let(:total_counter_method) { :total_commits_count } - expect(described_class.total_commits_count).to eq(2) - end + it_behaves_like 'counter examples' + end + + describe 'merge requests counter' do + let(:increment_counter_method) { :increment_merge_requests_count } + let(:total_counter_method) { :total_merge_requests_count } + + it_behaves_like 'counter examples' + end + + describe 'views counter' do + let(:increment_counter_method) { :increment_views_count } + let(:total_counter_method) { :total_views_count } + + it_behaves_like 'counter examples' end end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 90a534de202..270dd652c20 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -57,12 +57,22 @@ describe Gitlab::UsageData do gitaly database avg_cycle_analytics + web_ide_views web_ide_commits + web_ide_merge_requests influxdb_metrics_enabled prometheus_metrics_enabled )) end + it 'calls expected usage data methods' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).to receive(:total_commits_count) + expect(Gitlab::UsageDataCounters::WebIdeCounter).to receive(:total_merge_requests_count) + expect(Gitlab::UsageDataCounters::WebIdeCounter).to receive(:total_views_count) + + subject + end + it "gathers usage counts" do expected_keys = %i( assignee_lists -- cgit v1.2.1