Add base class for hook builders, and use it for notes and wikis

10 files changed, 95 insertions, 45 deletions

diff --git a/app/models/note.rb b/app/models/note.rb
index 3893fe25fb7..b407d3c18ad 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -202,9 +202,7 @@ class Note < ActiveRecord::Base
   end
 
   def hook_attrs
-    attributes.merge({
-      "note" => MarkdownUtils.absolute_image_urls(self.note)
-    })
+    Gitlab::HookData::NoteBuilder.new(self).build
   end
 
   def for_commit?
diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index 5f8552212ad..fc97ebe9f63 100644
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -59,9 +59,7 @@ class WikiPage
   attr_accessor :attributes
 
   def hook_attrs
-    attributes.merge({
-      "content" => MarkdownUtils.absolute_image_urls(self.content)
-    })
+    Gitlab::HookData::WikiPageBuilder.new(self).build
   end
 
   def initialize(wiki, page = nil, persisted = false)
diff --git a/lib/gitlab/hook_data/base_builder.rb b/lib/gitlab/hook_data/base_builder.rb
new file mode 100644
index 00000000000..1d99e793a03
--- /dev/null
+++ b/lib/gitlab/hook_data/base_builder.rb
@@ -0,0 +1,20 @@
+module Gitlab
+  module HookData
+    class BaseBuilder
+      attr_accessor :object
+
+      def initialize(object)
+        @object = object
+      end
+
+      private
+
+      def absolute_image_urls(markdown_text)
+        return markdown_text unless markdown_text.present?
+
+        markdown_text.gsub(/!\[(.*?)\]\((.*?)\)/,
+                           "![\\1](#{Settings.gitlab.url}\\2)")
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/hook_data/issuable_builder.rb b/lib/gitlab/hook_data/issuable_builder.rb
index 6ab36676127..ad0d6f6352b 100644
--- a/lib/gitlab/hook_data/issuable_builder.rb
+++ b/lib/gitlab/hook_data/issuable_builder.rb
@@ -1,13 +1,9 @@
 module Gitlab
   module HookData
-    class IssuableBuilder
+    class IssuableBuilder < BaseBuilder
       CHANGES_KEYS = %i[previous current].freeze
 
-      attr_accessor :issuable
-
-      def initialize(issuable)
-        @issuable = issuable
-      end
+      alias_method :issuable, :object
 
       def build(user: nil, changes: {})
         hook_data = {
@@ -64,6 +60,13 @@ module Gitlab
           hash
         end
       end
+
+      def absolute_image_urls(markdown_text)
+        return markdown_text unless markdown_text.present?
+
+        markdown_text.gsub(/!\[(.*?)\]\((.*?)\)/,
+                           "![\\1](#{Settings.gitlab.url}\\2)")
+      end
     end
   end
 end
diff --git a/lib/gitlab/hook_data/issue_builder.rb b/lib/gitlab/hook_data/issue_builder.rb
index 55dc84a60d4..0d71c748dc6 100644
--- a/lib/gitlab/hook_data/issue_builder.rb
+++ b/lib/gitlab/hook_data/issue_builder.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module HookData
-    class IssueBuilder
+    class IssueBuilder < BaseBuilder
       SAFE_HOOK_ATTRIBUTES = %i[
         assignee_id
         author_id
@@ -30,15 +30,11 @@ module Gitlab
         total_time_spent
       ].freeze
 
-      attr_accessor :issue
-
-      def initialize(issue)
-        @issue = issue
-      end
+      alias_method :issue, :object
 
       def build
         attrs = {
-          description: MarkdownUtils.absolute_image_urls(issue.description),
+          description: absolute_image_urls(issue.description),
           url: Gitlab::UrlBuilder.build(issue),
           total_time_spent: issue.total_time_spent,
           human_total_time_spent: issue.human_total_time_spent,
diff --git a/lib/gitlab/hook_data/merge_request_builder.rb b/lib/gitlab/hook_data/merge_request_builder.rb
index 6e5ef09de9e..dfbed0597ed 100644
--- a/lib/gitlab/hook_data/merge_request_builder.rb
+++ b/lib/gitlab/hook_data/merge_request_builder.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module HookData
-    class MergeRequestBuilder
+    class MergeRequestBuilder < BaseBuilder
       SAFE_HOOK_ATTRIBUTES = %i[
         assignee_id
         author_id
@@ -35,15 +35,11 @@ module Gitlab
         total_time_spent
       ].freeze
 
-      attr_accessor :merge_request
-
-      def initialize(merge_request)
-        @merge_request = merge_request
-      end
+      alias_method :merge_request, :object
 
       def build
         attrs = {
-          description: MarkdownUtils.absolute_image_urls(merge_request.description),
+          description: absolute_image_urls(merge_request.description),
           url: Gitlab::UrlBuilder.build(merge_request),
           source: merge_request.source_project.try(:hook_attrs),
           target: merge_request.target_project.hook_attrs,
diff --git a/lib/gitlab/hook_data/note_builder.rb b/lib/gitlab/hook_data/note_builder.rb
new file mode 100644
index 00000000000..81873e345d5
--- /dev/null
+++ b/lib/gitlab/hook_data/note_builder.rb
@@ -0,0 +1,43 @@
+module Gitlab
+  module HookData
+    class NoteBuilder < BaseBuilder
+      SAFE_HOOK_ATTRIBUTES = %i[
+        attachment
+        author_id
+        change_position
+        commit_id
+        created_at
+        discussion_id
+        id
+        line_code
+        note
+        noteable_id
+        noteable_type
+        original_position
+        position
+        project_id
+        resolved_at
+        resolved_by_id
+        resolved_by_push
+        st_diff
+        system
+        type
+        updated_at
+        updated_by_id
+      ].freeze
+
+      alias_method :note, :object
+
+      def build
+        note
+          .attributes
+          .with_indifferent_access
+          .slice(*SAFE_HOOK_ATTRIBUTES)
+          .merge(
+            description: absolute_image_urls(note.note),
+            url: Gitlab::UrlBuilder.build(note)
+          )
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/hook_data/wiki_page_builder.rb b/lib/gitlab/hook_data/wiki_page_builder.rb
new file mode 100644
index 00000000000..59c94a61cf2
--- /dev/null
+++ b/lib/gitlab/hook_data/wiki_page_builder.rb
@@ -0,0 +1,15 @@
+module Gitlab
+  module HookData
+    class WikiPageBuilder < BaseBuilder
+      alias_method :wiki_page, :object
+
+      def build
+        wiki_page
+          .attributes
+          .merge(
+            'content' => absolute_image_urls(wiki_page.content)
+          )
+      end
+    end
+  end
+end
diff --git a/lib/markdown_utils.rb b/lib/markdown_utils.rb
deleted file mode 100644
index f33d80da14c..00000000000
--- a/lib/markdown_utils.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-# Class to have all utility functions related to markdown
-class MarkdownUtils
-  # Convert image urls in the markdown text to absolute urls
-  def self.absolute_image_urls(markdown_text)
-    if markdown_text.present?
-      markdown_text.gsub(/!\[(.*?)\]\((.*?)\)/, "![\\1](#{Settings.gitlab.url}\\2)")
-    else
-      markdown_text
-    end
-  end
-end
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 41d84ded1b2..6a6c71e6c82 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -829,12 +829,4 @@ describe Note do
       note.destroy!
     end
   end
-
-  describe '#hook_attrs' do
-    let(:note) { create(:note, note: 'test![Note_Image](/uploads/abc/Note_Image.png)') }
-
-    it 'adds absolute urls for images in the description' do
-      expect(note.hook_attrs['note']).to eq("test![Note_Image](#{Settings.gitlab.url}/uploads/abc/Note_Image.png)")
-    end
-  end
 end