Maintenance: Enhance KB global search result scope.

.rubocop/default.yml

@@ -292,6 +292,9 @@ Layout/MultilineMethodCallIndentation:
   Include:
     - "**/*_spec.rb"
 
+Lint/UnusedMethodArgument:
+  AllowUnusedKeywordArguments: true
+
 Zammad/PreferNegatedIfOverUnless:
   Exclude:
     - 'bin/rspec'

app/models/knowledge_base/answer/translation.rb

@@ -96,13 +96,33 @@ class KnowledgeBase::Answer::Translation < ApplicationModel
       }
     end
 
-    def search_fallback(query, scope = nil)
+    def search_es_filter(es_response, _query, kb_locale, options)
+      return es_response if options[:user]&.permissions?('knowledge_base.editor')
+
+      answer_translations_id = es_response.pluck(:id)
+
+      allowed_answer_translation_ids = KnowledgeBase::Answer
+        .internal
+        .joins(:translations)
+        .where(knowledge_base_answer_translations: { id: answer_translations_id, kb_locale_id: kb_locale.id })
+        .pluck('knowledge_base_answer_translations.id')
+
+      es_response.filter { |elem| allowed_answer_translation_ids.include? elem[:id].to_i }
+    end
+
+    def search_fallback(query, scope = nil, options: {})
       fields = %w[title]
       fields << KnowledgeBase::Answer::Translation::Content.arel_table[:body]
 
       output = where_or_cis(fields, query)
                .joins(:content)
 
+      if !options[:user]&.permissions?('knowledge_base.editor')
+        answer_ids = KnowledgeBase::Answer.internal.pluck(:id)
+
+        output = output.where(answer_id: answer_ids)
+      end
+
       if scope.present?
         output = output
                  .joins(:answer)

app/models/knowledge_base/category.rb

@@ -93,7 +93,7 @@ class KnowledgeBase::Category < ApplicationModel
   def internal_content?(kb_locale = nil)
     scope = self_with_children_answers.internal
 
-    scope = scope.localed(kb_locale) if kb_locale
+    scope = scope.localed(kb_locale.system_locale) if kb_locale
 
     scope.any?
   end

app/models/knowledge_base/category/translation.rb

@@ -39,7 +39,7 @@ class KnowledgeBase::Category::Translation < ApplicationModel
   end
 
   class << self
-    def search_fallback(query, scope = nil)
+    def search_fallback(query, scope = nil, options: {})
       fields = %w[title]
 
       output = where_or_cis(fields, query)

app/models/knowledge_base/search.rb

@@ -16,7 +16,8 @@ class KnowledgeBase
           limit:    params[:limit] || 10,
           from:     params[:offset] || 0,
           sort_by:  search_get_sort_by(params, 'updated_at'),
-          order_by: search_get_order_by(params, 'desc')
+          order_by: search_get_order_by(params, 'desc'),
+          user:     current_user
         }
 
         kb_locale = KnowledgeBase::Locale.preferred(current_user, KnowledgeBase.first)
@@ -33,10 +34,10 @@ class KnowledgeBase
       def search_es(query, kb_locale, options)
         options[:query_extension] = { bool: { filter: { term: { kb_locale_id: kb_locale.id } } } }
 
-        SearchIndexBackend
+        es_response = SearchIndexBackend.search(query, name, options)
-          .search(query, name, options)
+        es_response = search_es_filter(es_response, query, kb_locale, options) if defined? :search_es_filter
-          .map { |item| lookup(id: item[:id]) }
+
-          .compact
+        es_response.map { |item| lookup(id: item[:id]) }.compact
       end
 
       def search_sql(query, kb_locale, options)
@@ -46,9 +47,9 @@ class KnowledgeBase
         # - stip out * we already search for *query* -
         query.delete! '*'
 
-        search_fallback("%#{query}%")
+        search_fallback("%#{query}%", options: options)
           .where(kb_locale: kb_locale)
-          .order(order_sql)
+          .order(Arel.sql(order_sql))
           .offset(options[:from])
           .limit(options[:limit])
           .to_a

app/models/knowledge_base/translation.rb

@@ -28,7 +28,7 @@ class KnowledgeBase::Translation < ApplicationModel
   end
 
   class << self
-    def search_fallback(query, scope = nil)
+    def search_fallback(query, scope = nil, options: {})
       fields = %w[title]
 
       output = where_or_cis(fields, query)

lib/search_knowledge_base_backend.rb

@@ -24,7 +24,7 @@ class SearchKnowledgeBaseBackend
                         hash
                       end
                   else
-                    search_fallback(query, indexes, user)
+                    search_fallback(query, indexes, { user: user })
                   end
 
     if (limit = @params.fetch(:limit, nil))
@@ -40,10 +40,10 @@ class SearchKnowledgeBaseBackend
       .flatten
   end
 
-  def search_fallback_for_index(query, index, _options)
+  def search_fallback_for_index(query, index, options)
     index
       .constantize
-      .search_fallback("%#{query}%", @cached_scope_ids)
+      .search_fallback("%#{query}%", @cached_scope_ids, options: options)
       .where(kb_locale: kb_locales)
       .pluck(:id)
       .map { |id| { id: id, type: index } }

spec/factories/knowledge_base/answer.rb

@@ -3,9 +3,10 @@ FactoryBot.define do
     transient do
       add_translation { true }
       translation_traits { [] }
+      knowledge_base { nil }
     end
 
-    category { create(:knowledge_base_category) }
+    category { create(:knowledge_base_category, { knowledge_base: knowledge_base }.compact) }
 
     before(:create) do |answer, context|
       next if answer.translations.present?
@@ -13,6 +14,20 @@ FactoryBot.define do
       answer.translations << build('knowledge_base/answer/translation', *context.translation_traits, answer: answer)
     end
 
+    trait :draft # empty placeholder for better readability
+
+    trait :internal do
+      internal_at { 1.week.ago }
+    end
+
+    trait :published do
+      published_at { 1.week.ago }
+    end
+
+    trait :archived do
+      archived_at { 1.week.ago }
+    end
+
     trait :with_video do
       transient do
         translation_traits { [:with_video] }

spec/factories/knowledge_base/category.rb

@@ -12,6 +12,18 @@ FactoryBot.define do
 
       category.translations << create('knowledge_base/category/translation', category: category)
     end
+
+    trait :empty # empty placeholder for better readability
+
+    %i[published internal draft archived].each do |state|
+      trait "containing_#{state}" do
+        after(:create) do |obj|
+          create(:knowledge_base_answer, state, parent: obj)
+
+          obj.reload
+        end
+      end
+    end
   end
 
   factory 'kb_category_with_tree', parent: 'knowledge_base/category' do

spec/lib/search_knowledge_base_backend_spec.rb

@@ -40,25 +40,74 @@ RSpec.describe SearchKnowledgeBaseBackend do
     end
   end
 
-  context 'with (out) ES is identical' do
+  context 'with successful API response' do
-    [true, false].each do |val|
+    shared_examples 'verify response' do |elasticsearch:|
-      context "when ES=#{val}", searchindex: val do
+      it "ID is an Integer when ES=#{elasticsearch}", searchindex: elasticsearch do
-        before do
-          if val
-            configure_elasticsearch(required: true, rebuild: true) do
         published_answer
-            end
+        configure_elasticsearch(required: true, rebuild: true) if elasticsearch
-          else
+        first_result = instance.search(published_answer.translations.first.title, user: user).first
-            published_answer
-          end
-        end
-
-        let(:first_result) { instance.search(published_answer.translations.first.title, user: user).first }
-
-        it 'ID is an Integer' do
         expect(first_result.dig(:id)).to be_a(Integer)
       end
     end
+
+    include_examples 'verify response', elasticsearch: true
+    include_examples 'verify response', elasticsearch: false
   end
+
+  context 'with user trait and object state' do
+    def expected_visibility_instance(ui_identifier)
+      options = {
+        knowledge_base: knowledge_base,
+        locale:         primary_locale,
+        scope:          nil,
+        flavor:         ui_identifier
+      }
+
+      described_class.new options
+    end
+
+    shared_examples 'verify given search backend' do |permissions:, ui:, elasticsearch:|
+      is_visible = permissions == :all || permissions == ui
+      prefix     = is_visible ? 'lists' : 'does not list'
+
+      it "#{prefix} in #{ui} interface when ES=#{elasticsearch}", searchindex: elasticsearch do
+        instance = expected_visibility_instance ui
+        object
+        configure_elasticsearch(required: true, rebuild: true) if elasticsearch
+        expect(instance.search(object.translations.first.title, user: user)).to is_visible ? be_present : be_blank
+      end
+    end
+
+    shared_examples 'verify given permissions' do |scope:, trait:, admin:, agent:|
+      context "with #{trait} #{scope}" do
+        let(:object) { create("knowledge_base_#{scope}", trait, knowledge_base: knowledge_base) }
+
+        include_examples 'verify given user', user_id: :admin, permissions: admin
+        include_examples 'verify given user', user_id: :agent, permissions: agent
+      end
+    end
+
+    shared_examples 'verify given user' do |user_id:, permissions:|
+      context "with #{user_id}" do
+        let(:user) { create(user_id) }
+
+        include_examples 'verify given search backend', permissions: permissions, ui: :agent, elasticsearch: true
+        include_examples 'verify given search backend', permissions: permissions, ui: :agent, elasticsearch: false
+
+        include_examples 'verify given search backend', permissions: permissions, ui: :public, elasticsearch: true
+        include_examples 'verify given search backend', permissions: permissions, ui: :public, elasticsearch: false
+      end
+    end
+
+    include_examples 'verify given permissions', scope: :answer, trait: :published, admin: :all, agent: :all
+    include_examples 'verify given permissions', scope: :answer, trait: :internal,  admin: :all, agent: :agent
+    include_examples 'verify given permissions', scope: :answer, trait: :draft,     admin: :all, agent: :none
+    include_examples 'verify given permissions', scope: :answer, trait: :archived,  admin: :all, agent: :none
+
+    include_examples 'verify given permissions', scope: :category, trait: :empty,                admin: :all, agent: :none
+    include_examples 'verify given permissions', scope: :category, trait: :containing_published, admin: :all, agent: :all
+    include_examples 'verify given permissions', scope: :category, trait: :containing_internal,  admin: :all, agent: :agent
+    include_examples 'verify given permissions', scope: :category, trait: :containing_draft,     admin: :all, agent: :none
+    include_examples 'verify given permissions', scope: :category, trait: :containing_archived,  admin: :all, agent: :none
   end
 end

spec/models/knowledge_base/answer/translation_spec.rb

@@ -11,4 +11,37 @@ RSpec.describe KnowledgeBase::Answer::Translation, type: :model, current_user_id
 
   it { is_expected.to belong_to(:answer) }
   it { is_expected.to belong_to(:kb_locale) }
+
+  describe '.search' do
+    include_context 'basic Knowledge Base'
+
+    shared_examples 'verify given search backend' do |trait:, user_id:, is_visible:, elasticsearch:|
+      prefix = is_visible ? 'lists' : 'does not list'
+
+      it "#{prefix} #{trait} answer to #{user_id} when ES=#{elasticsearch}", searchindex: elasticsearch do
+        user   = create(user_id)
+        object = create(:knowledge_base_answer, trait, knowledge_base: knowledge_base)
+        configure_elasticsearch(required: true, rebuild: true) if elasticsearch
+
+        expect(described_class.search({ query: object.translations.first.title, current_user: user }))
+          .to is_visible ? be_present : be_blank
+      end
+    end
+
+    shared_examples 'verify given user' do |trait:, user_id:, is_visible:|
+      include_examples 'verify given search backend', trait: trait, user_id: user_id, is_visible: is_visible, elasticsearch: true
+      include_examples 'verify given search backend', trait: trait, user_id: user_id, is_visible: is_visible, elasticsearch: false
+    end
+
+    shared_examples 'verify given permissions' do |trait:, admin:, agent:, customer:|
+      include_examples 'verify given user', trait: trait, user_id: :admin,    is_visible: admin
+      include_examples 'verify given user', trait: trait, user_id: :agent,    is_visible: agent
+      include_examples 'verify given user', trait: trait, user_id: :customer, is_visible: customer
+    end
+
+    include_examples 'verify given permissions', trait: :published, admin: true, agent: true,  customer: false
+    include_examples 'verify given permissions', trait: :internal,  admin: true, agent: true,  customer: false
+    include_examples 'verify given permissions', trait: :draft,     admin: true, agent: false, customer: false
+    include_examples 'verify given permissions', trait: :archived,  admin: true, agent: false, customer: false
+  end
 end

spec/models/knowledge_base/category_spec.rb

@@ -94,4 +94,34 @@ RSpec.describe KnowledgeBase::Category, type: :model, current_user_id: 1 do
       end
     end
   end
+
+  describe '#public_content?' do
+    shared_examples 'verify visibility in given state' do |state:, is_visible:|
+      it "returns #{is_visible} when contains #{state} answer" do
+        object = create(:knowledge_base_category, "containing_#{state}")
+
+        expect(object).send is_visible ? :to : :not_to, be_public_content(object.translations.first.kb_locale)
+      end
+    end
+
+    include_examples 'verify visibility in given state', state: :published, is_visible: true
+    include_examples 'verify visibility in given state', state: :internal,  is_visible: false
+    include_examples 'verify visibility in given state', state: :draft,     is_visible: false
+    include_examples 'verify visibility in given state', state: :archived,  is_visible: false
+  end
+
+  describe '#internal_content?' do
+    shared_examples 'verify visibility in given state' do |state:, is_visible:|
+      it "returns #{is_visible} when contains #{state} answer" do
+        object = create(:knowledge_base_category, "containing_#{state}")
+
+        expect(object).send is_visible ? :to : :not_to, be_internal_content(object.translations.first.kb_locale)
+      end
+    end
+
+    include_examples 'verify visibility in given state', state: :published, is_visible: true
+    include_examples 'verify visibility in given state', state: :internal,  is_visible: true
+    include_examples 'verify visibility in given state', state: :draft,     is_visible: false
+    include_examples 'verify visibility in given state', state: :archived,  is_visible: false
+  end
 end

spec/support/knowledge_base_contexts.rb

@@ -20,19 +20,19 @@ RSpec.shared_context 'basic Knowledge Base', current_user_id: 1 do
   end
 
   let :published_answer do
-    create(:knowledge_base_answer, :with_attachment, category: category, published_at: 1.week.ago)
+    create(:knowledge_base_answer, :published, :with_attachment, category: category)
   end
 
   let :published_answer_with_video do
-    create(:knowledge_base_answer, :with_video, category: category, published_at: 1.week.ago)
+    create(:knowledge_base_answer, :published, :with_video, category: category)
   end
 
   let :internal_answer do
-    create(:knowledge_base_answer, category: category, internal_at: 1.week.ago)
+    create(:knowledge_base_answer, :internal, category: category)
   end
 
   let :archived_answer do
-    create(:knowledge_base_answer, category: category, archived_at: 1.week.ago)
+    create(:knowledge_base_answer, :archived, category: category)
   end
 end