Browse Source

Fixes #3083 - KB answer search in ticket is broken

Mantas Masalskis 4 years ago
parent
commit
b5c067b05a

+ 1 - 0
.rubocop_todo.rspec.yml

@@ -620,6 +620,7 @@ RSpec/NestedGroups:
     - 'spec/lib/notification_factory/template_spec.rb'
     - 'spec/lib/notification_factory/template_spec.rb'
     - 'spec/lib/notification_factory_spec.rb'
     - 'spec/lib/notification_factory_spec.rb'
     - 'spec/lib/search_index_backend_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/secure_mailing/smime_spec.rb'
     - 'spec/lib/sessions/backend/base_spec.rb'
     - 'spec/lib/sessions/backend/base_spec.rb'
     - 'spec/lib/sessions/backend/ticket_overview_list_spec.rb'
     - 'spec/lib/sessions/backend/ticket_overview_list_spec.rb'

+ 3 - 3
app/controllers/knowledge_base/search_controller.rb

@@ -85,7 +85,7 @@ class KnowledgeBase::SearchController < ApplicationController
           end
           end
 
 
     {
     {
-      id:       object.id.to_s,
+      id:       object.id,
       type:     object.class.name,
       type:     object.class.name,
       icon:     'knowledge-base-answer',
       icon:     'knowledge-base-answer',
       date:     object.updated_at,
       date:     object.updated_at,
@@ -108,7 +108,7 @@ class KnowledgeBase::SearchController < ApplicationController
           end
           end
 
 
     {
     {
-      id:       object.id.to_s,
+      id:       object.id,
       type:     object.class.name,
       type:     object.class.name,
       fontName: object.category.knowledge_base.iconset,
       fontName: object.category.knowledge_base.iconset,
       date:     object.updated_at,
       date:     object.updated_at,
@@ -130,7 +130,7 @@ class KnowledgeBase::SearchController < ApplicationController
           end
           end
 
 
     {
     {
-      id:    object.id.to_s,
+      id:    object.id,
       type:  object.class.name,
       type:  object.class.name,
       icon:  'knowledge-base',
       icon:  'knowledge-base',
       date:  object.updated_at,
       date:  object.updated_at,

+ 6 - 2
lib/search_knowledge_base_backend.rb

@@ -17,7 +17,12 @@ class SearchKnowledgeBaseBackend
 
 
   def search(query, user: nil)
   def search(query, user: nil)
     raw_results = if SearchIndexBackend.enabled?
     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
                   else
                     search_fallback(query, indexes, user)
                     search_fallback(query, indexes, user)
                   end
                   end
@@ -26,7 +31,6 @@ class SearchKnowledgeBaseBackend
       raw_results = raw_results[0, limit]
       raw_results = raw_results[0, limit]
     end
     end
 
 
-    #raw_results
     filter_results raw_results, user
     filter_results raw_results, user
   end
   end
 
 

+ 52 - 20
spec/lib/search_knowledge_base_backend_spec.rb

@@ -1,31 +1,63 @@
 require 'rails_helper'
 require 'rails_helper'
 
 
-RSpec.describe SearchKnowledgeBaseBackend, searchindex: true do
+RSpec.describe SearchKnowledgeBaseBackend do
   include_context 'basic Knowledge Base'
   include_context 'basic Knowledge Base'
 
 
-  before do
-    configure_elasticsearch(required: true, rebuild: true) do
-      published_answer
-    end
+  let(:instance) { described_class.new options }
+  let(:user)     { create(:admin) }
+
+  let(:options) do
+    {
+      knowledge_base: knowledge_base,
+      locale:         primary_locale,
+      scope:          nil
+    }
   end
   end
 
 
-  describe '#search' do
-    let(:instance) { described_class.new options }
-    let(:user)     { create(:admin) }
-
-    context 'when highlight enabled' do
-      let(:options) do
-        {
-          knowledge_base:    knowledge_base,
-          locale:            primary_locale,
-          scope:             nil,
-          highlight_enabled: true
-        }
+  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
+  end
+
+  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
+
+        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
     end
   end
   end

+ 3 - 3
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
     it 'for answers' do
       post endpoint, params: { query: published_answer.translations.first.title }
       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
     end
 
 
     it 'for categories' do
     it 'for categories' do
       post endpoint, params: { query: category.translations.first.title }
       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
     end
 
 
     it 'for knowledge base' do
     it 'for knowledge base' do
       post endpoint, params: { query: knowledge_base.translations.first.title }
       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
   end
 end
 end

+ 0 - 29
spec/system/ticket/linking_knowledge_base_answer_spec.rb

@@ -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

+ 47 - 0
spec/system/ticket/zoom_spec.rb

@@ -764,4 +764,51 @@ RSpec.describe 'Ticket zoom', type: :system do
       end
       end
     end
     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
 end