From 880f1d5e3ae6f61c22eb2d1898a3fc5f4cafe536 Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Mon, 20 Apr 2020 11:47:45 +0200 Subject: [PATCH] Fixes #2644 - Including Knowledge Base Answers into Ticket Articles doesn't attach Attachments --- .../controllers/_ui_element/richtext.coffee | 9 +++ .../ticket_zoom/article_new.coffee | 2 +- .../app/lib/base/jquery.textmodule.js | 32 +++++++- .../answer/attachments_controller.rb | 11 ++- app/models/concerns/can_clone_attachments.rb | 79 +++++++++++++++++++ app/models/knowledge_base/answer.rb | 6 ++ app/models/ticket/article.rb | 77 +----------------- .../answer/attachments_controller_policy.rb | 1 + config/routes/knowledge_base.rb | 6 +- spec/factories/knowledge_base/answer.rb | 16 ++++ spec/models/knowledge_base/answer_spec.rb | 7 ++ .../answer_attachments_cloning_spec.rb | 20 +++++ spec/support/knowledge_base_contexts.rb | 2 +- .../inserting_knowledge_base_answer_spec.rb | 43 ++++++++++ 14 files changed, 226 insertions(+), 85 deletions(-) create mode 100644 app/models/concerns/can_clone_attachments.rb create mode 100644 spec/requests/knowledge_base/answer_attachments_cloning_spec.rb create mode 100644 spec/system/ticket/inserting_knowledge_base_answer_spec.rb diff --git a/app/assets/javascripts/app/controllers/_ui_element/richtext.coffee b/app/assets/javascripts/app/controllers/_ui_element/richtext.coffee index c3cd13d6d..e20b1479d 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/richtext.coffee +++ b/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') diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee index b6b1a57f6..624a702cc 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_new.coffee +++ b/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) diff --git a/app/assets/javascripts/app/lib/base/jquery.textmodule.js b/app/assets/javascripts/app/lib/base/jquery.textmodule.js index 0b9e19572..6ba675b69 100644 --- a/app/assets/javascripts/app/lib/base/jquery.textmodule.js +++ b/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 = $('
  • ').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) { diff --git a/app/controllers/knowledge_base/answer/attachments_controller.rb b/app/controllers/knowledge_base/answer/attachments_controller.rb index e7e658690..cf8c8cc9d 100644 --- a/app/controllers/knowledge_base/answer/attachments_controller.rb +++ b/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 diff --git a/app/models/concerns/can_clone_attachments.rb b/app/models/concerns/can_clone_attachments.rb new file mode 100644 index 000000000..c77ca39c0 --- /dev/null +++ b/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 diff --git a/app/models/knowledge_base/answer.rb b/app/models/knowledge_base/answer.rb index d0504751e..3574d8bb5 100644 --- a/app/models/knowledge_base/answer.rb +++ b/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 diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index 9b2cb2538..db7fc8b84 100644 --- a/app/models/ticket/article.rb +++ b/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 diff --git a/app/policies/controllers/knowledge_base/answer/attachments_controller_policy.rb b/app/policies/controllers/knowledge_base/answer/attachments_controller_policy.rb index edb808b64..d25a206bc 100644 --- a/app/policies/controllers/knowledge_base/answer/attachments_controller_policy.rb +++ b/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 diff --git a/config/routes/knowledge_base.rb b/config/routes/knowledge_base.rb index 7741b07b6..8bece6e44 100644 --- a/config/routes/knowledge_base.rb +++ b/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 diff --git a/spec/factories/knowledge_base/answer.rb b/spec/factories/knowledge_base/answer.rb index 8f5789bdd..51169bf45 100644 --- a/spec/factories/knowledge_base/answer.rb +++ b/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 diff --git a/spec/models/knowledge_base/answer_spec.rb b/spec/models/knowledge_base/answer_spec.rb index b3f0c4c2d..542df2056 100644 --- a/spec/models/knowledge_base/answer_spec.rb +++ b/spec/models/knowledge_base/answer_spec.rb @@ -11,4 +11,11 @@ RSpec.describe KnowledgeBase::Answer, type: :model, current_user_id: 1 do it { is_expected.not_to validate_presence_of(:category_id) } it { is_expected.to belong_to(:category) } + it { expect(kb_answer.attachments).to be_blank } + + context 'with attachment' do + subject(:kb_answer) { create(:knowledge_base_answer, :with_attachment) } + + it { expect(kb_answer.attachments).to be_present } + end end diff --git a/spec/requests/knowledge_base/answer_attachments_cloning_spec.rb b/spec/requests/knowledge_base/answer_attachments_cloning_spec.rb new file mode 100644 index 000000000..b384f2d1f --- /dev/null +++ b/spec/requests/knowledge_base/answer_attachments_cloning_spec.rb @@ -0,0 +1,20 @@ +require 'rails_helper' + +RSpec.describe 'KnowledgeBase answer attachments cloning', type: :request, authenticated_as: :agent_user do + include_context 'basic Knowledge Base' do + before do + published_answer + end + end + + it 'copies to given UploadCache' do + form_id = Random.rand(999..9999) + endpoint = "/api/v1/knowledge_bases/#{knowledge_base.id}/answers/#{published_answer.id}/attachments/clone_to_form" + params = { form_id: form_id } + + expect { post endpoint, params: params } + .to change { Store.list(object: 'UploadCache', o_id: form_id).count } + .from(0) + .to(1) + end +end diff --git a/spec/support/knowledge_base_contexts.rb b/spec/support/knowledge_base_contexts.rb index d6ff5fe08..780106da2 100644 --- a/spec/support/knowledge_base_contexts.rb +++ b/spec/support/knowledge_base_contexts.rb @@ -20,7 +20,7 @@ RSpec.shared_context 'basic Knowledge Base', current_user_id: 1 do end let :published_answer do - create(:knowledge_base_answer, category: category, published_at: 1.week.ago) + create(:knowledge_base_answer, :with_attachment, category: category, published_at: 1.week.ago) end let :published_answer_with_video do diff --git a/spec/system/ticket/inserting_knowledge_base_answer_spec.rb b/spec/system/ticket/inserting_knowledge_base_answer_spec.rb new file mode 100644 index 000000000..04073d612 --- /dev/null +++ b/spec/system/ticket/inserting_knowledge_base_answer_spec.rb @@ -0,0 +1,43 @@ +require 'rails_helper' + +RSpec.describe 'inserting Knowledge Base answer', type: :system, authenticated: true, searchindex: true do + include_context 'basic Knowledge Base' + + let(:field) { find(:richtext) } + let(:target_translation) { published_answer.translations.first } + + before do + configure_elasticsearch(required: true, rebuild: true) do + published_answer + end + end + + it 'adds text' do + open_page + insert_kb_answer(target_translation, field) + + expect(field).to have_text target_translation.content.body + end + + it 'attaches file' do + open_page + insert_kb_answer(target_translation, field) + + within(:active_content) do + expect(page).to have_css '.attachments .attachment--row' + end + end + + private + + def open_page + visit 'ticket/create' + end + + def insert_kb_answer(translation, target_field) + target_field.send_keys('??') + translation.title.slice(0, 3).split('').each { |letter| target_field.send_keys(letter) } + + find(:text_module, translation.id).click + end +end