Browse Source

Fixes #2644 - Including Knowledge Base Answers into Ticket Articles doesn't attach Attachments

Mantas Masalskis 4 years ago
parent
commit
880f1d5e3a

+ 9 - 0
app/assets/javascripts/app/controllers/_ui_element/richtext.coffee

@@ -36,6 +36,15 @@ class App.UiElement.richtext
         for file in attribute.attachments
           renderFile(file)
 
+      App.Event.bind('ui::ticket::addArticleAttachent', (data) ->
+        form_id = item.closest('form').find('[name=form_id]').val()
+
+        return if data.form_id isnt form_id
+        return if _.isEmpty(data.attachments)
+        for file in data.attachments
+          renderFile(file)
+      , form.form_id)
+
       # remove items
       item.find('.attachments').on('click', '.js-delete', (e) =>
         id = $(e.currentTarget).data('id')

+ 1 - 1
app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee

@@ -81,7 +81,7 @@ class App.TicketZoomArticleNew extends App.Controller
 
     # add article attachment
     @bind('ui::ticket::addArticleAttachent', (data) =>
-      return if data.ticket.id.toString() isnt @ticket_id.toString()
+      return if data.ticket?.id?.toString() isnt @ticket_id.toString() && data.form_id isnt @form_id
       return if _.isEmpty(data.attachments)
       for file in data.attachments
         @renderAttachment(file)

+ 28 - 4
app/assets/javascripts/app/lib/base/jquery.textmodule.js

@@ -375,10 +375,17 @@
     if (trigger) {
       var _this     = this;
 
-      trigger.renderValue(this, elem, function(text) {
+      var form_id = this.$element.closest('form').find('[name=form_id]').val()
+
+      trigger.renderValue(this, elem, function(text, attachments) {
         _this.cutInput()
         _this.paste(text)
         _this.close(true)
+
+        App.Event.trigger('ui::ticket::addArticleAttachent', {
+          attachments: attachments,
+          form_id: form_id
+        })
       })
     }
   }
@@ -455,7 +462,7 @@
 
     if (!item) return
 
-    callback(item.content)
+    callback(item.content, [])
   }
 
   Collection.renderResults = function(textmodule, term) {
@@ -497,6 +504,8 @@
     var element = $('<li>').text(App.i18n.translateInline('Please wait...'))
     textmodule.appendResults(element)
 
+    var form_id = textmodule.$element.closest('form').find('[name=form_id]').val()
+
     App.Ajax.request({
       id:   'textmoduleKbAnswer',
       type: 'GET',
@@ -508,8 +517,23 @@
 
         var body = translation.content().bodyWithPublicURLs()
 
-        App.Utils.htmlImage2DataUrlAsync(body, function(output){
-          callback(output)
+        App.Ajax.request({
+          id:   'textmoduleKbAnswerAttachments',
+          type: 'POST',
+          data: JSON.stringify({
+            form_id: form_id
+          }),
+          url:  translation.parent().generateURL('/attachments/clone_to_form'),
+          success: function(data, status, xhr) {
+            translation.parent().attachments += data.attachments
+
+            App.Utils.htmlImage2DataUrlAsync(body, function(output){
+              callback(output, translation.parent().attachments)
+            })
+          },
+          error: function(xhr) {
+            callback('')
+          }
         })
       },
       error: function(xhr) {

+ 9 - 2
app/controllers/knowledge_base/answer/attachments_controller.rb

@@ -2,8 +2,7 @@
 
 class KnowledgeBase::Answer::AttachmentsController < ApplicationController
   prepend_before_action :authentication_check
-  prepend_before_action :authorize!
-
+  before_action :authorize!
   before_action :fetch_answer
 
   def create
@@ -18,6 +17,14 @@ class KnowledgeBase::Answer::AttachmentsController < ApplicationController
     render json: @answer.assets({})
   end
 
+  def clone_to_form
+    new_attachments = @answer.clone_attachments('UploadCache', params[:form_id], only_attached_attachments: true)
+
+    render json: {
+      attachments: new_attachments
+    }
+  end
+
   private
 
   def fetch_answer

+ 79 - 0
app/models/concerns/can_clone_attachments.rb

@@ -0,0 +1,79 @@
+module CanCloneAttachments
+  extend ActiveSupport::Concern
+
+=begin
+
+clone existing attachments of article to the target object
+
+  article_parent = Ticket::Article.find(123)
+  article_new = Ticket::Article.find(456)
+
+  attached_attachments = article_parent.clone_attachments(article_new.class.name, article_new.id, only_attached_attachments: true)
+
+  inline_attachments = article_parent.clone_attachments(article_new.class.name, article_new.id, only_inline_attachments: true)
+
+returns
+
+  [attachment1, attachment2, ...]
+
+=end
+
+  def clone_attachments(object_type, object_id, options = {})
+    existing_attachments = Store.list(
+      object: object_type,
+      o_id:   object_id,
+    )
+
+    is_html_content = false
+    if content_type.present? && content_type =~ %r{text/html}i
+      is_html_content = true
+    end
+
+    new_attachments = []
+    attachments.each do |new_attachment|
+      next if new_attachment.preferences['content-alternative'] == true
+
+      # only_attached_attachments mode is used by apply attached attachments to forwared article
+      if options[:only_attached_attachments] == true
+        if is_html_content == true
+
+          content_id = new_attachment.preferences['Content-ID'] || new_attachment.preferences['content_id']
+          next if content_id.present? && body.present? && body.match?(/#{Regexp.quote(content_id)}/i)
+        end
+      end
+
+      # only_inline_attachments mode is used when quoting HTML mail with #{article.body_as_html}
+      if options[:only_inline_attachments] == true
+        next if is_html_content == false
+        next if body.blank?
+
+        content_disposition = new_attachment.preferences['Content-Disposition'] || new_attachment.preferences['content_disposition']
+        next if content_disposition.present? && content_disposition !~ /inline/
+
+        content_id = new_attachment.preferences['Content-ID'] || new_attachment.preferences['content_id']
+        next if content_id.blank?
+        next if !body.match?(/#{Regexp.quote(content_id)}/i)
+      end
+
+      already_added = false
+      existing_attachments.each do |existing_attachment|
+        next if existing_attachment.filename != new_attachment.filename || existing_attachment.size != new_attachment.size
+
+        already_added = true
+        break
+      end
+      next if already_added == true
+
+      file = Store.add(
+        object:      object_type,
+        o_id:        object_id,
+        data:        new_attachment.content,
+        filename:    new_attachment.filename,
+        preferences: new_attachment.preferences,
+      )
+      new_attachments.push file
+    end
+
+    new_attachments
+  end
+end

+ 6 - 0
app/models/knowledge_base/answer.rb

@@ -5,6 +5,7 @@ class KnowledgeBase::Answer < ApplicationModel
   include CanBePublished
   include HasKnowledgeBaseAttachmentPermissions
   include ChecksKbClientNotification
+  include CanCloneAttachments
 
   AGENT_ALLOWED_ATTRIBUTES       = %i[category_id promoted internal_note].freeze
   AGENT_ALLOWED_NESTED_RELATIONS = %i[translations].freeze
@@ -96,6 +97,11 @@ class KnowledgeBase::Answer < ApplicationModel
     Rails.application.routes.url_helpers.knowledge_base_answer_path(category.knowledge_base, self)
   end
 
+  # required by CanCloneAttachments
+  def content_type
+    'text/html'
+  end
+
   private
 
   def reordering_callback

+ 1 - 76
app/models/ticket/article.rb

@@ -6,6 +6,7 @@ class Ticket::Article < ApplicationModel
   include HasHistory
   include ChecksHtmlSanitized
   include CanCsvImport
+  include CanCloneAttachments
   include HasObjectManagerAttributesValidation
 
   include Ticket::Article::Assets
@@ -134,82 +135,6 @@ returns
     new_attachments
   end
 
-=begin
-
-clone existing attachments of article to the target object
-
-  article_parent = Ticket::Article.find(123)
-  article_new = Ticket::Article.find(456)
-
-  attached_attachments = article_parent.clone_attachments(article_new.class.name, article_new.id, only_attached_attachments: true)
-
-  inline_attachments = article_parent.clone_attachments(article_new.class.name, article_new.id, only_inline_attachments: true)
-
-returns
-
-  [attachment1, attachment2, ...]
-
-=end
-
-  def clone_attachments(object_type, object_id, options = {})
-    existing_attachments = Store.list(
-      object: object_type,
-      o_id:   object_id,
-    )
-
-    is_html_content = false
-    if content_type.present? && content_type =~ %r{text/html}i
-      is_html_content = true
-    end
-
-    new_attachments = []
-    attachments.each do |new_attachment|
-      next if new_attachment.preferences['content-alternative'] == true
-
-      # only_attached_attachments mode is used by apply attached attachments to forwared article
-      if options[:only_attached_attachments] == true
-        if is_html_content == true
-
-          content_id = new_attachment.preferences['Content-ID'] || new_attachment.preferences['content_id']
-          next if content_id.present? && body.present? && body.match?(/#{Regexp.quote(content_id)}/i)
-        end
-      end
-
-      # only_inline_attachments mode is used when quoting HTML mail with #{article.body_as_html}
-      if options[:only_inline_attachments] == true
-        next if is_html_content == false
-        next if body.blank?
-
-        content_disposition = new_attachment.preferences['Content-Disposition'] || new_attachment.preferences['content_disposition']
-        next if content_disposition.present? && content_disposition !~ /inline/
-
-        content_id = new_attachment.preferences['Content-ID'] || new_attachment.preferences['content_id']
-        next if content_id.blank?
-        next if !body.match?(/#{Regexp.quote(content_id)}/i)
-      end
-
-      already_added = false
-      existing_attachments.each do |existing_attachment|
-        next if existing_attachment.filename != new_attachment.filename || existing_attachment.size != new_attachment.size
-
-        already_added = true
-        break
-      end
-      next if already_added == true
-
-      file = Store.add(
-        object:      object_type,
-        o_id:        object_id,
-        data:        new_attachment.content,
-        filename:    new_attachment.filename,
-        preferences: new_attachment.preferences,
-      )
-      new_attachments.push file
-    end
-
-    new_attachments
-  end
-
   def self.last_customer_agent_article(ticket_id)
     sender = Ticket::Article::Sender.lookup(name: 'System')
     Ticket::Article.where('ticket_id = ? AND sender_id NOT IN (?)', ticket_id, sender.id).order(created_at: :desc).first

+ 1 - 0
app/policies/controllers/knowledge_base/answer/attachments_controller_policy.rb

@@ -1,3 +1,4 @@
 class Controllers::KnowledgeBase::Answer::AttachmentsControllerPolicy < Controllers::ApplicationControllerPolicy
+  permit! :clone_to_form, to: 'knowledge_base.*'
   default_permit!('knowledge_base.editor')
 end

+ 5 - 1
config/routes/knowledge_base.rb

@@ -49,7 +49,11 @@ Zammad::Application.routes.draw do
                           only:       %i[create update show destroy],
                           concerns:   :has_publishing do
 
-        resources :attachments, controller: 'knowledge_base/answer/attachments', only: %i[create destroy]
+        resources :attachments, controller: 'knowledge_base/answer/attachments', only: %i[create destroy] do
+          collection do
+            post :clone_to_form
+          end
+        end
       end
     end
   end

+ 16 - 0
spec/factories/knowledge_base/answer.rb

@@ -18,5 +18,21 @@ FactoryBot.define do
         translation_traits { [:with_video] }
       end
     end
+
+    trait :with_attachment do
+      transient do
+        attachment { File.open('spec/fixtures/upload/hello_world.txt') }
+      end
+
+      after(:create) do |answer, context|
+        Store.add(
+          object:      answer.class.name,
+          o_id:        answer.id,
+          data:        context.attachment.read,
+          filename:    File.basename(context.attachment.path),
+          preferences: {}
+        )
+      end
+    end
   end
 end

Some files were not shown because too many files changed in this diff