From b5c067b05a6b212fc10ffbe0abc7b91ceb277971 Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Tue, 14 Jul 2020 08:20:25 +0200 Subject: [PATCH] Fixes #3083 - KB answer search in ticket is broken --- .rubocop_todo.rspec.yml | 1 + .../knowledge_base/search_controller.rb | 6 +- lib/search_knowledge_base_backend.rb | 8 ++- .../lib/search_knowledge_base_backend_spec.rb | 70 ++++++++++++++----- .../search_with_details_spec.rb | 6 +- .../linking_knowledge_base_answer_spec.rb | 29 -------- spec/system/ticket/zoom_spec.rb | 47 +++++++++++++ 7 files changed, 111 insertions(+), 56 deletions(-) delete mode 100644 spec/system/ticket/linking_knowledge_base_answer_spec.rb diff --git a/.rubocop_todo.rspec.yml b/.rubocop_todo.rspec.yml index 63915f117..c03a2852d 100644 --- a/.rubocop_todo.rspec.yml +++ b/.rubocop_todo.rspec.yml @@ -620,6 +620,7 @@ RSpec/NestedGroups: - 'spec/lib/notification_factory/template_spec.rb' - 'spec/lib/notification_factory_spec.rb' - 'spec/lib/search_index_backend_spec.rb' + - 'spec/lib/search_knowledge_base_backend_spec.rb' - 'spec/lib/secure_mailing/smime_spec.rb' - 'spec/lib/sessions/backend/base_spec.rb' - 'spec/lib/sessions/backend/ticket_overview_list_spec.rb' diff --git a/app/controllers/knowledge_base/search_controller.rb b/app/controllers/knowledge_base/search_controller.rb index 287427524..580d5dd83 100644 --- a/app/controllers/knowledge_base/search_controller.rb +++ b/app/controllers/knowledge_base/search_controller.rb @@ -85,7 +85,7 @@ class KnowledgeBase::SearchController < ApplicationController end { - id: object.id.to_s, + id: object.id, type: object.class.name, icon: 'knowledge-base-answer', date: object.updated_at, @@ -108,7 +108,7 @@ class KnowledgeBase::SearchController < ApplicationController end { - id: object.id.to_s, + id: object.id, type: object.class.name, fontName: object.category.knowledge_base.iconset, date: object.updated_at, @@ -130,7 +130,7 @@ class KnowledgeBase::SearchController < ApplicationController end { - id: object.id.to_s, + id: object.id, type: object.class.name, icon: 'knowledge-base', date: object.updated_at, diff --git a/lib/search_knowledge_base_backend.rb b/lib/search_knowledge_base_backend.rb index 6c62e8a65..82f86b70e 100644 --- a/lib/search_knowledge_base_backend.rb +++ b/lib/search_knowledge_base_backend.rb @@ -17,7 +17,12 @@ class SearchKnowledgeBaseBackend def search(query, user: nil) raw_results = if SearchIndexBackend.enabled? - SearchIndexBackend.search(query, indexes, options) + SearchIndexBackend + .search(query, indexes, options) + .map do |hash| + hash[:id] = hash[:id].to_i + hash + end else search_fallback(query, indexes, user) end @@ -26,7 +31,6 @@ class SearchKnowledgeBaseBackend raw_results = raw_results[0, limit] end - #raw_results filter_results raw_results, user end diff --git a/spec/lib/search_knowledge_base_backend_spec.rb b/spec/lib/search_knowledge_base_backend_spec.rb index 6313ca666..676e6dd4d 100644 --- a/spec/lib/search_knowledge_base_backend_spec.rb +++ b/spec/lib/search_knowledge_base_backend_spec.rb @@ -1,31 +1,63 @@ require 'rails_helper' -RSpec.describe SearchKnowledgeBaseBackend, searchindex: true do +RSpec.describe SearchKnowledgeBaseBackend do include_context 'basic Knowledge Base' - before do - configure_elasticsearch(required: true, rebuild: true) do - published_answer + let(:instance) { described_class.new options } + let(:user) { create(:admin) } + + let(:options) do + { + knowledge_base: knowledge_base, + locale: primary_locale, + scope: nil + } + end + + context 'with ES', searchindex: true do + before do + configure_elasticsearch(required: true, rebuild: true) do + published_answer + end + end + + describe '#search' do + context 'when highlight enabled' do + let(:options) do + { + knowledge_base: knowledge_base, + locale: primary_locale, + scope: nil, + highlight_enabled: true + } + end + + # https://github.com/zammad/zammad/issues/3070 + it 'lists item with an attachment' do + expect(instance.search('Hello World', user: user)).to be_present + end + end end end - describe '#search' do - let(:instance) { described_class.new options } - let(:user) { create(:admin) } + context 'with (out) ES is identical' do + [true, false].each do |val| + context "when ES=#{val}", searchindex: val do + before do + if val + configure_elasticsearch(required: true, rebuild: true) do + published_answer + end + else + published_answer + end + end - context 'when highlight enabled' do - let(:options) do - { - knowledge_base: knowledge_base, - locale: primary_locale, - scope: nil, - highlight_enabled: true - } - end + let(:first_result) { instance.search(published_answer.translations.first.title, user: user).first } - # https://github.com/zammad/zammad/issues/3070 - it 'lists item with an attachment' do - expect(instance.search('Hello World', user: user)).to be_present + it 'ID is an Integer' do + expect(first_result.dig(:id)).to be_a(Integer) + end end end end diff --git a/spec/requests/knowledge_base/search_with_details_spec.rb b/spec/requests/knowledge_base/search_with_details_spec.rb index e52823866..018cbda11 100644 --- a/spec/requests/knowledge_base/search_with_details_spec.rb +++ b/spec/requests/knowledge_base/search_with_details_spec.rb @@ -15,19 +15,19 @@ RSpec.describe 'Knowledge Base search with details', type: :request, searchindex it 'for answers' do post endpoint, params: { query: published_answer.translations.first.title } - expect(json_response['details'][0]['id']).to be_a_kind_of String + expect(json_response['details'][0]['id']).to be_a_kind_of Integer end it 'for categories' do post endpoint, params: { query: category.translations.first.title } - expect(json_response['details'][0]['id']).to be_a_kind_of String + expect(json_response['details'][0]['id']).to be_a_kind_of Integer end it 'for knowledge base' do post endpoint, params: { query: knowledge_base.translations.first.title } - expect(json_response['details'][0]['id']).to be_a_kind_of String + expect(json_response['details'][0]['id']).to be_a_kind_of Integer end end end diff --git a/spec/system/ticket/linking_knowledge_base_answer_spec.rb b/spec/system/ticket/linking_knowledge_base_answer_spec.rb deleted file mode 100644 index ec940e623..000000000 --- a/spec/system/ticket/linking_knowledge_base_answer_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -require 'rails_helper' - -RSpec.describe 'linking Knowledge Base answer', type: :system, authenticated_as: true, searchindex: true do - include_context 'basic Knowledge Base' - - before do - configure_elasticsearch(required: true, rebuild: true) do - published_answer - end - - # refresh page to make sure it reflects updated settings - refresh - end - - it do - ticket = create :ticket, group: Group.find_by(name: 'Users') - visit "#ticket/zoom/#{ticket.id}" - - find(:css, '.active .link_kb_answers .js-add').click - - target_translation = published_answer.translations.first - - find(:css, '.active .link_kb_answers .js-input').send_keys target_translation.title - - find(:css, %(.active .link_kb_answers li[data-value="#{target_translation.id}"])).click - - expect(find(:css, '.active .link_kb_answers ol')).to have_text target_translation.title - end -end diff --git a/spec/system/ticket/zoom_spec.rb b/spec/system/ticket/zoom_spec.rb index 77d44a0aa..bdf6ed0a0 100644 --- a/spec/system/ticket/zoom_spec.rb +++ b/spec/system/ticket/zoom_spec.rb @@ -764,4 +764,51 @@ RSpec.describe 'Ticket zoom', type: :system do end end end + + describe 'linking Knowledge Base answer' do + include_context 'basic Knowledge Base' + + let(:ticket) { create :ticket, group: Group.find_by(name: 'Users') } + let(:answer) { published_answer } + let(:translation) { answer.translations.first } + + shared_examples 'verify linking' do + it 'allows to look up an answer' do + visit "#ticket/zoom/#{ticket.id}" + + within :active_content do + within '.link_kb_answers' do + find('.js-add').click + + find('.js-input').send_keys translation.title + + find(%(li[data-value="#{translation.id}"])).click + + expect(find('.link_kb_answers ol')).to have_text translation.title + end + end + end + end + + context 'with ES', searchindex: true, authenticated_as: :authenticate do + def authenticate + configure_elasticsearch(required: true, rebuild: true) do + answer + end + + true + end + + include_examples 'verify linking' + end + + context 'without ES', authenticated_as: :authenticate do + def authenticate + answer + true + end + + include_examples 'verify linking' + end + end end