Merge branch '292829-track-thread-metrics-for-a-merge-request' into 'master'

Track thread un/resolve metrics by user

See merge request gitlab-org/gitlab!52130

app/controllers/projects/discussions_controller.rb

@@ -18,7 +18,7 @@ class Projects::DiscussionsController < Projects::ApplicationController
   end
 
   def unresolve
-    discussion.unresolve!
+    Discussions::UnresolveService.new(discussion, current_user).execute
 
     render_discussion
   end

app/graphql/mutations/discussions/toggle_resolve.rb

@@ -69,7 +69,7 @@ module Mutations
       end
 
       def unresolve!(discussion)
-        discussion.unresolve!
+        ::Discussions::UnresolveService.new(discussion, current_user).execute
       end
     end
   end

app/services/discussions/resolve_service.rb

@@ -40,7 +40,13 @@ module Discussions
       discussion.resolve!(current_user)
       @resolved_count += 1
 
-      MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) if merge_request
+      if merge_request
+        Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter
+          .track_resolve_thread_action(user: current_user)
+
+        MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request)
+      end
+
       SystemNoteService.discussion_continued_in_issue(discussion, project, current_user, follow_up_issue) if follow_up_issue
     end
 

app/services/discussions/unresolve_service.rb

+# frozen_string_literal: true
+
+module Discussions
+  class UnresolveService < Discussions::BaseService
+    include Gitlab::Utils::StrongMemoize
+
+    def initialize(discussion, user)
+      @discussion = discussion
+      @user = user
+
+      super
+    end
+
+    def execute
+      @discussion.unresolve!
+
+      Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter
+        .track_unresolve_thread_action(user: @user)
+    end
+  end
+end

config/feature_flags/development/usage_data_i_code_review_user_resolve_thread.yml

+---
+name: usage_data_i_code_review_user_resolve_thread
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52130
+rollout_issue_url:
+milestone: '13.9'
+type: development
+group: group::code review
+default_enabled: true

config/feature_flags/development/usage_data_i_code_review_user_unresolve_thread.yml

+---
+name: usage_data_i_code_review_user_unresolve_thread
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52130
+rollout_issue_url:
+milestone: '13.9'
+type: development
+group: group::code review
+default_enabled: true

lib/api/helpers/notes_helpers.rb

@@ -138,7 +138,7 @@ module API
           parent = noteable_parent(noteable)
           ::Discussions::ResolveService.new(parent, current_user, one_or_more_discussions: discussion).execute
         else
-          discussion.unresolve!
+          ::Discussions::UnresolveService.new(discussion, current_user).execute
         end
 
         present discussion, with: Entities::Discussion

lib/gitlab/usage_data_counters/known_events/common.yml

@@ -476,6 +476,16 @@
   category: code_review
   aggregation: weekly
   feature_flag: usage_data_i_code_review_user_reopen_mr
+- name: i_code_review_user_resolve_thread
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_resolve_thread
+- name: i_code_review_user_unresolve_thread
+  redis_slot: code_review
+  category: code_review
+  aggregation: weekly
+  feature_flag: usage_data_i_code_review_user_unresolve_thread
 - name: i_code_review_user_merge_mr
   redis_slot: code_review
   category: code_review

lib/gitlab/usage_data_counters/merge_request_activity_unique_counter.rb

@@ -20,6 +20,8 @@ module Gitlab
       MR_REMOVE_MULTILINE_COMMENT_ACTION = 'i_code_review_user_remove_multiline_mr_comment'
       MR_ADD_SUGGESTION_ACTION = 'i_code_review_user_add_suggestion'
       MR_APPLY_SUGGESTION_ACTION = 'i_code_review_user_apply_suggestion'
+      MR_RESOLVE_THREAD_ACTION = 'i_code_review_user_resolve_thread'
+      MR_UNRESOLVE_THREAD_ACTION = 'i_code_review_user_unresolve_thread'
 
       class << self
         def track_mr_diffs_action(merge_request:)
@@ -47,6 +49,14 @@ module Gitlab
           track_unique_action_by_user(MR_REOPEN_ACTION, user)
         end
 
+        def track_resolve_thread_action(user:)
+          track_unique_action_by_user(MR_RESOLVE_THREAD_ACTION, user)
+        end
+
+        def track_unresolve_thread_action(user:)
+          track_unique_action_by_user(MR_UNRESOLVE_THREAD_ACTION, user)
+        end
+
         def track_create_comment_action(note:)
           track_unique_action_by_user(MR_CREATE_COMMENT_ACTION, note.author)
           track_multiline_unique_action(MR_CREATE_MULTILINE_COMMENT_ACTION, note)

spec/controllers/projects/discussions_controller_spec.rb

@@ -186,6 +186,13 @@ RSpec.describe Projects::DiscussionsController do
           expect(Note.find(note.id).discussion.resolved?).to be false
         end
 
+        it "tracks thread unresolve usage data" do
+          expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+            .to receive(:track_unresolve_thread_action).with(user: user)
+
+          delete :unresolve, params: request_params
+        end
+
         it "returns status 200" do
           delete :unresolve, params: request_params
 

spec/lib/gitlab/usage_data_counters/merge_request_activity_unique_counter_spec.rb

@@ -73,6 +73,22 @@ RSpec.describe Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter, :cl
     end
   end
 
+  describe '.track_resolve_thread_action' do
+    subject { described_class.track_resolve_thread_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_RESOLVE_THREAD_ACTION }
+    end
+  end
+
+  describe '.track_unresolve_thread_action' do
+    subject { described_class.track_unresolve_thread_action(user: user) }
+
+    it_behaves_like 'a tracked merge request unique event' do
+      let(:action) { described_class::MR_UNRESOLVE_THREAD_ACTION }
+    end
+  end
+
   describe '.track_create_comment_action' do
     subject { described_class.track_create_comment_action(note: note) }
 

spec/services/discussions/resolve_service_spec.rb

@@ -24,6 +24,13 @@ RSpec.describe Discussions::ResolveService do
       expect(discussion).to be_resolved
     end
 
+    it 'tracks thread resolve usage data' do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_resolve_thread_action).with(user: user)
+
+      service.execute
+    end
+
     it 'executes the notification service' do
       expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance|
         expect(instance).to receive(:execute).with(discussion.noteable)
@@ -101,6 +108,13 @@ RSpec.describe Discussions::ResolveService do
         service.execute
       end
 
+      it 'does not track thread resolve usage data' do
+        expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+          .not_to receive(:track_resolve_thread_action).with(user: user)
+
+        service.execute
+      end
+
       it 'does not schedule an auto-merge' do
         expect(AutoMergeProcessWorker).not_to receive(:perform_async)
 

spec/services/discussions/unresolve_service_spec.rb

+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe Discussions::UnresolveService do
+  describe "#execute" do
+    let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:user) { create(:user, developer_projects: [project]) }
+    let_it_be(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds, source_project: project) }
+    let(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
+
+    let(:service) { described_class.new(discussion, user) }
+
+    before do
+      project.add_developer(user)
+      discussion.resolve!(user)
+    end
+
+    it "unresolves the discussion" do
+      service.execute
+
+      expect(discussion).not_to be_resolved
+    end
+
+    it "counts the unresolve event" do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_unresolve_thread_action).with(user: user)
+
+      service.execute
+    end
+  end
+end

spec/support/shared_examples/requests/api/resolvable_discussions_shared_examples.rb

@@ -13,6 +13,9 @@ RSpec.shared_examples 'resolvable discussions API' do |parent_type, noteable_typ
     end
 
     it "unresolves discussion if resolved is false" do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_unresolve_thread_action).with(user: user)
+
       put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
               "discussions/#{note.discussion_id}", user), params: { resolved: false }