Merge branch 'ld-add-action-to-notes-webhooks' into 'master'

Add object_attributes.action for note webhooks

See merge request !147856



Merged-by: Gary Holtz <gholtz@gitlab.com>
Approved-by: Ashraf Khamis <akhamis@gitlab.com>
Approved-by: James Nutt <jnutt@gitlab.com>
Approved-by: Rudy Crespo <rcrespo@gitlab.com>
Approved-by: Kushal Pandya <kushal@gitlab.com>
Approved-by: Gary Holtz <gholtz@gitlab.com>
Co-authored-by: Luke Duncalfe <lduncalfe@eml.cc>

app/services/concerns/integrations/project_test_data.rb

@@ -23,7 +23,7 @@ def note_events_data
 
       no_data_error(s_('TestHooks|Ensure the project has notes.')) unless note.present?
 
-      Gitlab::DataBuilder::Note.build(note, current_user)
+      Gitlab::DataBuilder::Note.build(note, current_user, :create)
     end
 
     def issues_events_data

app/services/notes/post_process_service.rb

@@ -29,7 +29,7 @@ def create_design_discussion_system_note?
     end
 
     def hook_data
-      Gitlab::DataBuilder::Note.build(note, note.author)
+      Gitlab::DataBuilder::Note.build(note, note.author, :create)
     end
 
     def execute_note_hooks

app/services/notes/update_service.rb

@@ -102,7 +102,7 @@ def track_note_edit_usage_for_issues(note)
     def execute_note_webhook(note)
       return unless note.project && note.previous_changes.include?('note')
 
-      note_data = Gitlab::DataBuilder::Note.build(note, note.author)
+      note_data = Gitlab::DataBuilder::Note.build(note, note.author, :update)
       is_confidential = note.confidential?(include_noteable: true)
       hooks_scope = is_confidential ? :confidential_note_hooks : :note_hooks
 

app/views/shared/web_hooks/_form.html.haml

@@ -26,11 +26,11 @@
     %li.gl-pb-3
       = form.gitlab_ui_checkbox_component :note_events,
           integration_webhook_event_human_name(:note_events),
-          help_text: s_('Webhooks|A comment is added to an issue or merge request.')
+          help_text: s_('Webhooks|A comment is made or edited on an issue or merge request.')
     %li.gl-pb-3
       = form.gitlab_ui_checkbox_component :confidential_note_events,
           integration_webhook_event_human_name(:confidential_note_events),
-          help_text: s_('Webhooks|A comment is added to a confidential issue.')
+          help_text: s_('Webhooks|A comment is made or edited on a confidential issue.')
     %li.gl-pb-3
       = form.gitlab_ui_checkbox_component :issues_events,
           integration_webhook_event_human_name(:issues_events),

doc/user/project/integrations/webhook_events.md

@@ -21,7 +21,7 @@ Event type                                   | Trigger
 [Push event](#push-events)                   | A push is made to the repository.
 [Tag event](#tag-events)                     | Tags are created or deleted in the repository.
 [Issue event](#issue-events)                 | A new issue is created or an existing issue is updated, closed, or reopened.
-[Comment event](#comment-events)             | A new comment is made on commits, merge requests, issues, and code snippets.
+[Comment event](#comment-events)             | A new comment is made or edited on commits, merge requests, issues, and code snippets. <sup>1</sup>
 [Merge request event](#merge-request-events) | A merge request is created, updated, merged, or closed, or a commit is added in the source branch.
 [Wiki page event](#wiki-page-events)         | A wiki page is created, updated, or deleted.
 [Pipeline event](#pipeline-events)           | A pipeline status changes.
@@ -32,6 +32,10 @@ Event type                                   | Trigger
 [Emoji event](#emoji-events)                 | An emoji reaction is added or removed.
 [Project or group access token event](#project-and-group-access-token-events) | A project or group access token will expire in seven days.
 
+**Footnotes:**
+
+1. Comment events triggered when the comment is edited [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127169) in GitLab 16.11.
+
 **Events triggered for group webhooks only:**
 
 Event type                                   | Trigger
@@ -380,20 +384,27 @@ Payload example:
 
 ## Comment events
 
-Comment events are triggered when a new comment is made on commits,
+> - `object_attributes.action` property [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/147856) in GitLab 16.11.
+
+Comment events are triggered when a new comment is made or edited on commits,
 merge requests, issues, and code snippets.
 
 The note data is stored in `object_attributes` (for example, `note` or `noteable_type`).
 The payload includes information about the target of the comment. For example,
 a comment on an issue includes specific issue information under the `issue` key.
 
-The valid target types are:
+The available target types are:
 
 - `commit`
 - `merge_request`
 - `issue`
 - `snippet`
 
+The available values for `object_attributes.action` in the payload are:
+
+- `create`
+- `update`
+
 ### Comment on a commit
 
 Request header:
@@ -462,6 +473,7 @@ Payload example:
       "renamed_file": false,
       "deleted_file": false
     },
+    "action": "create",
     "url": "http://example.com/gitlab-org/gitlab-test/commit/cfe32cf61b73a0d5e9f13e774abde7ff789b1660#note_1243"
   },
   "commit": {
@@ -536,6 +548,7 @@ Payload example:
     "noteable_id": 7,
     "system": false,
     "st_diff": null,
+    "action": "create",
     "url": "http://example.com/gitlab-org/gitlab-test/merge_requests/1#note_1244"
   },
   "merge_request": {
@@ -697,6 +710,7 @@ Payload example:
     "noteable_id": 92,
     "system": false,
     "st_diff": null,
+    "action": "create",
     "url": "http://example.com/gitlab-org/gitlab-test/issues/17#note_1241"
   },
   "issue": {
@@ -803,6 +817,7 @@ Payload example:
     "noteable_id": 53,
     "system": false,
     "st_diff": null,
+    "action": "create",
     "url": "http://example.com/gitlab-org/gitlab-test/-/snippets/53#note_1245"
   },
   "snippet": {

lib/gitlab/data_builder/note.rb

@@ -5,6 +5,8 @@ module DataBuilder
     module Note
       extend self
 
+      VALID_ACTIONS = %i[create update].freeze
+
       # Produce a hash of post-receive data
       #
       # For all notes:
@@ -36,9 +38,11 @@ module Note
       #  - merge_request
       #  - snippet
       #
-      def build(note, user)
+      def build(note, user, action)
+        raise ArgumentError, 'invalid action' unless action.in?(VALID_ACTIONS)
+
         project = note.project
-        data = build_base_data(project, user, note)
+        data = build_base_data(project, user, note, action)
 
         if note.for_commit?
           data[:commit] = build_data_for_commit(project, user, note)
@@ -53,22 +57,22 @@ def build(note, user)
         data
       end
 
-      def build_base_data(project, user, note)
+      def build_base_data(project, user, note, action)
         event_type = note.confidential?(include_noteable: true) ? 'confidential_note' : 'note'
 
-        base_data = {
+        {
           object_kind: "note",
           event_type: event_type,
           user: user.hook_attrs,
           project_id: project.id,
           project: project.hook_attrs,
-          object_attributes: note.hook_attrs,
+          object_attributes: note.hook_attrs.merge(
+            action: action.to_s,
+            url: Gitlab::UrlBuilder.build(note)
+          ),
           # DEPRECATED
           repository: project.hook_attrs.slice(:name, :url, :description, :homepage)
         }
-
-        base_data[:object_attributes][:url] = Gitlab::UrlBuilder.build(note)
-        base_data
       end
 
       def build_data_for_commit(project, user, note)

locale/gitlab.pot

@@ -57065,10 +57065,10 @@ msgstr ""
 msgid "Webhooks|+ Mask another portion of URL"
 msgstr ""
-msgid "Webhooks|A comment is added to a confidential issue."
+msgid "Webhooks|A comment is made or edited on a confidential issue."
 msgstr ""
-msgid "Webhooks|A comment is added to an issue or merge request."
+msgid "Webhooks|A comment is made or edited on an issue or merge request."
 msgstr ""
 msgid "Webhooks|A confidential issue is created, updated, closed, or reopened."

spec/lib/gitlab/data_builder/note_spec.rb

@@ -2,18 +2,19 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::DataBuilder::Note do
-  let(:project) { create(:project, :repository) }
-  let(:user) { create(:user) }
-  let(:data) { described_class.build(note, user) }
+RSpec.describe Gitlab::DataBuilder::Note, feature_category: :webhooks do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
+  let(:action) { :create }
+  let(:data) { described_class.build(note, user, action) }
   let(:fixed_time) { Time.at(1425600000) } # Avoid time precision errors
 
   shared_examples 'includes general data' do
     specify do
       expect(data).to have_key(:object_attributes)
       expect(data[:object_attributes]).to have_key(:url)
-      expect(data[:object_attributes][:url])
-        .to eq(Gitlab::UrlBuilder.build(note))
+      expect(data[:object_attributes][:url]).to eq(Gitlab::UrlBuilder.build(note))
+      expect(data[:object_attributes][:action]).to eq('create')
       expect(data[:object_kind]).to eq('note')
       expect(data[:user]).to eq(user.hook_attrs)
     end
@@ -158,4 +159,20 @@
     include_examples 'project hook data'
     include_examples 'deprecated repository hook data'
   end
+
+  describe 'object_attributes.action value' do
+    let_it_be(:note) { create(:note, project: project) }
+
+    describe 'when action is `:update`' do
+      let(:action) { :update }
+
+      it { expect(data[:object_attributes][:action]).to eq('update') }
+    end
+
+    describe 'when action is invalid' do
+      let(:action) { :invalid }
+
+      it { expect { data }.to raise_error(ArgumentError) }
+    end
+  end
 end

spec/models/integrations/base_chat_notification_spec.rb

@@ -118,7 +118,7 @@ def build_channel_list(count)
       let_it_be(:issue) { create(:labeled_issue, project: project, labels: [label, label_2, label_3]) }
       let_it_be(:note) { create(:note, noteable: issue, project: project) }
 
-      let(:data) { Gitlab::DataBuilder::Note.build(note, user) }
+      let(:data) { Gitlab::DataBuilder::Note.build(note, user, :create) }
 
       shared_examples 'notifies the chat integration' do
         specify do
@@ -262,7 +262,7 @@ def build_channel_list(count)
         context 'labels are distributed on multiple objects' do
           let(:label_filter) { '~Bug, ~Backend' }
           let(:data) do
-            Gitlab::DataBuilder::Note.build(note, user).merge({
+            Gitlab::DataBuilder::Note.build(note, user, :create).merge({
               issue: {
                 labels: [
                   { title: 'Bug' }

spec/models/integrations/hangouts_chat_spec.rb

@@ -144,7 +144,7 @@
       end
 
       it "adds thread key" do
-        data = Gitlab::DataBuilder::Note.build(commit_note, user)
+        data = Gitlab::DataBuilder::Note.build(commit_note, user, :create)
 
         WebMock.stub_request(:post, webhook_url_regex)
           .with { |request| expect(thread_key_from_request(request)).to match(/commit .*?/) }
@@ -163,7 +163,7 @@
       end
 
       it "adds thread key" do
-        data = Gitlab::DataBuilder::Note.build(merge_request_note, user)
+        data = Gitlab::DataBuilder::Note.build(merge_request_note, user, :create)
 
         WebMock.stub_request(:post, webhook_url_regex)
           .with { |request| expect(thread_key_from_request(request)).to match(/merge request .*?/) }
@@ -182,7 +182,7 @@
       end
 
       it "adds thread key" do
-        data = Gitlab::DataBuilder::Note.build(issue_note, user)
+        data = Gitlab::DataBuilder::Note.build(issue_note, user, :create)
 
         WebMock.stub_request(:post, webhook_url_regex)
           .with { |request| expect(thread_key_from_request(request)).to match(/issue .*?/) }
@@ -201,7 +201,7 @@
       end
 
       it "adds thread key" do
-        data = Gitlab::DataBuilder::Note.build(snippet_note, user)
+        data = Gitlab::DataBuilder::Note.build(snippet_note, user, :create)
 
         WebMock.stub_request(:post, webhook_url_regex)
           .with { |request| expect(thread_key_from_request(request)).to match(/snippet .*?/) }

spec/models/integrations/microsoft_teams_spec.rb

@@ -143,7 +143,7 @@
       end
 
       it "calls Microsoft Teams API for commit comment events" do
-        data = Gitlab::DataBuilder::Note.build(commit_note, user)
+        data = Gitlab::DataBuilder::Note.build(commit_note, user, :create)
 
         chat_integration.execute(data)
 
@@ -157,7 +157,7 @@
       end
 
       it "calls Microsoft Teams API for merge request comment events" do
-        data = Gitlab::DataBuilder::Note.build(merge_request_note, user)
+        data = Gitlab::DataBuilder::Note.build(merge_request_note, user, :create)
 
         chat_integration.execute(data)
 
@@ -171,7 +171,7 @@
       end
 
       it "calls Microsoft Teams API for issue comment events" do
-        data = Gitlab::DataBuilder::Note.build(issue_note, user)
+        data = Gitlab::DataBuilder::Note.build(issue_note, user, :create)
 
         chat_integration.execute(data)
 
@@ -185,7 +185,7 @@
       end
 
       it "calls Microsoft Teams API for snippet comment events" do
-        data = Gitlab::DataBuilder::Note.build(snippet_note, user)
+        data = Gitlab::DataBuilder::Note.build(snippet_note, user, :create)
 
         chat_integration.execute(data)
 

spec/services/integrations/group_mention_service_spec.rb

@@ -93,7 +93,7 @@
   end
 
   context 'for issue notes' do
-    let(:hook_data) { Gitlab::DataBuilder::Note.build(mentionable, mentionable.author) }
+    let(:hook_data) { Gitlab::DataBuilder::Note.build(mentionable, mentionable.author, :create) }
     let(:is_confidential) { mentionable.confidential?(include_noteable: true) }
 
     context 'in public projects' do

spec/support/shared_examples/models/chat_integration_shared_examples.rb

@@ -223,7 +223,7 @@
     end
 
     context "with note events" do
-      let(:sample_data) { Gitlab::DataBuilder::Note.build(note, user) }
+      let(:sample_data) { Gitlab::DataBuilder::Note.build(note, user, :create) }
 
       context "with commit comment" do
         let_it_be(:note) do

spec/support/shared_examples/models/concerns/integrations/base_slack_notification_shared_examples.rb

@@ -105,7 +105,7 @@ def usage_tracking_key(action)
       context 'for note notification' do
         let_it_be(:issue_note) { create(:note_on_issue, project: project, note: 'issue note') }
 
-        let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) }
+        let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user, :create) }
 
         it_behaves_like 'increases the usage data counter', :note
       end
@@ -126,7 +126,7 @@ def usage_tracking_key(action)
           create(:note_on_issue, project: project, note: 'issue note', confidential: true)
         end
 
-        let(:data) { Gitlab::DataBuilder::Note.build(confidential_issue_note, user) }
+        let(:data) { Gitlab::DataBuilder::Note.build(confidential_issue_note, user, :create) }
 
         it_behaves_like 'increases the usage data counter', :confidential_note
       end

spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb

@@ -238,7 +238,7 @@ def execute_with_options(options)
     context 'note event' do
       let_it_be(:issue_note) { create(:note_on_issue, project: project, note: "issue note") }
 
-      let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) }
+      let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user, :create) }
 
       it_behaves_like 'calls the integration API with the event message', /commented on issue/
 
@@ -475,7 +475,7 @@ def execute_with_options(options)
       end
 
       let(:data) do
-        Gitlab::DataBuilder::Note.build(commit_note, user)
+        Gitlab::DataBuilder::Note.build(commit_note, user, :create)
       end
 
       it_behaves_like "triggered #{integration_name} integration", event_type: "commit comment"
@@ -487,7 +487,7 @@ def execute_with_options(options)
       end
 
       let(:data) do
-        Gitlab::DataBuilder::Note.build(merge_request_note, user)
+        Gitlab::DataBuilder::Note.build(merge_request_note, user, :create)
       end
 
       it_behaves_like "triggered #{integration_name} integration", event_type: "merge request comment"
@@ -499,7 +499,7 @@ def execute_with_options(options)
       end
 
       let(:data) do
-        Gitlab::DataBuilder::Note.build(issue_note, user)
+        Gitlab::DataBuilder::Note.build(issue_note, user, :create)
       end
 
       it_behaves_like "triggered #{integration_name} integration", event_type: "issue comment"
@@ -511,7 +511,7 @@ def execute_with_options(options)
       end
 
       let(:data) do
-        Gitlab::DataBuilder::Note.build(snippet_note, user)
+        Gitlab::DataBuilder::Note.build(snippet_note, user, :create)
       end
 
       it_behaves_like "triggered #{integration_name} integration", event_type: "snippet comment"