Fixes #3978 - KB: Unpermitted categories are displayed after browsing

app/models/knowledge_base/answer.rb

@@ -47,19 +47,8 @@ class KnowledgeBase::Answer < ApplicationModel
     data = category.assets(data)
 
     # include all siblings to make sure ordering is always up to date. Reader gets only accessible siblings.
-    siblings = category.answers
+    data = ApplicationModel::CanAssets.reduce(assets_siblings, data)
 
-    if !User.lookup(id: UserInfo.current_user_id)&.permissions?('knowledge_base.editor')
-      ep = KnowledgeBase::EffectivePermission.new User.find(UserInfo.current_user_id), category
-
-      siblings = if ep.access_effective == 'public_reader'
-                   siblings.published
-                 else
-                   siblings.internal
-                 end
-    end
-
-    data = ApplicationModel::CanAssets.reduce(siblings, data)
     ApplicationModel::CanAssets.reduce(translations, data)
   end
 
@@ -111,6 +100,27 @@ class KnowledgeBase::Answer < ApplicationModel
 
   private
 
+  def assets_siblings(siblings: category.answers, current_user: User.lookup(id: UserInfo.current_user_id))
+    if KnowledgeBase.granular_permissions?
+      ep = KnowledgeBase::EffectivePermission.new current_user, category
+
+      case ep.access_effective
+      when 'public_reader'
+        siblings.published
+      when 'none'
+        siblings.none
+      when 'reader'
+        siblings.internal
+      else
+        siblings
+      end
+    elsif !current_user&.permissions?('knowledge_base.editor')
+      siblings.internal
+    else
+      siblings
+    end
+  end
+
   def reordering_callback
     return if !category_id_changed? && !position_changed?
 

app/models/knowledge_base/category.rb

@@ -46,13 +46,7 @@ class KnowledgeBase::Category < ApplicationModel
     data = knowledge_base.assets(data)
 
     # include all siblings to make sure ordering is always up to date
-    siblings = sibling_categories
-
-    if !User.lookup(id: UserInfo.current_user_id)&.permissions?('knowledge_base.editor')
-      siblings = siblings.select(&:internal_content?)
-    end
-
-    data = ApplicationModel::CanAssets.reduce(siblings, data)
+    data = ApplicationModel::CanAssets.reduce(assets_siblings, data)
     data = ApplicationModel::CanAssets.reduce(translations, data)
 
     # include parent category or KB for root to have full path
@@ -141,6 +135,31 @@ class KnowledgeBase::Category < ApplicationModel
 
   private
 
+  def assets_siblings(siblings: sibling_categories, current_user: User.lookup(id: UserInfo.current_user_id))
+    granular = KnowledgeBase.granular_permissions?
+
+    if !granular && !current_user&.permissions?('knowledge_base.editor')
+      siblings.select(&:internal_content?)
+    elsif granular
+      siblings.select { |elem| assets_siblings_applicable?(elem, current_user) }
+    else
+      siblings
+    end
+  end
+
+  def assets_siblings_applicable?(elem, current_user)
+    ep = KnowledgeBase::EffectivePermission.new(current_user, elem)
+
+    case ep.access_effective
+    when 'none'
+      false
+    when 'reader'
+      elem.internal_content?
+    else
+      true
+    end
+  end
+
   def cannot_be_child_of_parent
     errors.add(:parent_id, 'cannot be a child of the parent') if self_parent?(self)
   end

spec/models/knowledge_base/answer_spec.rb

@@ -23,4 +23,106 @@ RSpec.describe KnowledgeBase::Answer, type: :model, current_user_id: 1 do
 
     it { expect(kb_answer.attachments).to be_present }
   end
+
+  describe '#assets', current_user_id: -> { user.id } do
+    let(:assets) { another_category_answer && internal_answer.assets }
+    let(:user) { create(:agent) }
+    let(:another_category) { create(:knowledge_base_category, knowledge_base: knowledge_base) }
+    let(:another_category_answer) { create(:knowledge_base_answer, :internal, category: another_category) }
+
+    include_context 'basic Knowledge Base'
+
+    context 'without permissions' do
+      it { expect(assets).to include_assets_of internal_answer }
+      it { expect(assets).to include_assets_of category }
+      it { expect(assets).to include_assets_of another_category }
+
+      context 'with internal and published articles in category' do
+        before do
+          internal_answer
+          published_answer
+        end
+
+        it 'internal sibling returned' do
+          expect(published_answer.assets).to include_assets_of(internal_answer, category)
+        end
+
+        it 'published sibling returned' do
+          expect(internal_answer.assets).to include_assets_of(published_answer, category)
+        end
+      end
+    end
+
+    context 'with readable another category' do
+      before do
+        KnowledgeBase::PermissionsUpdate
+          .new(another_category)
+          .update! user.roles.first => 'reader'
+      end
+
+      it { expect(assets).to include_assets_of internal_answer }
+      it { expect(assets).to include_assets_of category }
+      it { expect(assets).to include_assets_of another_category }
+
+      context 'with internal and published articles in category' do
+        before do
+          KnowledgeBase::PermissionsUpdate
+            .new(category)
+            .update! user.roles.first => 'reader'
+
+          internal_answer
+          published_answer
+        end
+
+        it 'internal sibling returned' do
+          expect(published_answer.assets).to include_assets_of(internal_answer, category)
+        end
+
+        it 'published sibling returned' do
+          expect(internal_answer.assets).to include_assets_of(published_answer, category)
+        end
+      end
+    end
+
+    context 'with hidden another category' do
+      before do
+        KnowledgeBase::PermissionsUpdate
+          .new(another_category)
+          .update! user.roles.first => 'none'
+      end
+
+      it { expect(assets).to include_assets_of internal_answer }
+      it { expect(assets).to include_assets_of category }
+      it { expect(assets).not_to include_assets_of another_category }
+
+      context 'with internal and published articles in category' do
+        before do
+          KnowledgeBase::PermissionsUpdate
+            .new(category)
+            .update! user.roles.first => 'none'
+
+          internal_answer
+          published_answer
+        end
+
+        it 'internal sibling not returned' do
+          expect(published_answer.assets).to not_include_assets_of(internal_answer).and(include_assets_of(category))
+        end
+
+        it 'published sibling returned' do
+          expect(internal_answer.assets).to include_assets_of(published_answer, category)
+        end
+      end
+
+      context 'with published answer' do
+        let(:another_category_published_answer) { create(:knowledge_base_answer, :published, category: another_category) }
+
+        before { another_category_published_answer }
+
+        it { expect(assets).to include_assets_of internal_answer }
+        it { expect(assets).to include_assets_of category }
+        it { expect(assets).to include_assets_of another_category }
+      end
+    end
+  end
 end

spec/models/knowledge_base/category_spec.rb

@@ -131,4 +131,50 @@ RSpec.describe KnowledgeBase::Category, type: :model, current_user_id: 1 do
     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 '#assets', current_user_id: -> { user.id } do
+    subject(:assets) { another_category_answer && internal_answer && category.assets }
+
+    include_context 'basic Knowledge Base'
+
+    let(:user) { create(:agent) }
+    let(:another_category) { create(:knowledge_base_category, knowledge_base: knowledge_base) }
+    let(:another_category_answer) { create(:knowledge_base_answer, :internal, category: another_category) }
+
+    context 'without permissions' do
+      it { expect(assets).to include_assets_of category }
+      it { expect(assets).to include_assets_of another_category }
+    end
+
+    context 'with readable another category' do
+      before do
+        KnowledgeBase::PermissionsUpdate
+          .new(another_category)
+          .update! user.roles.first => 'reader'
+      end
+
+      it { expect(assets).to include_assets_of category }
+      it { expect(assets).to include_assets_of another_category }
+    end
+
+    context 'with hidden another category' do
+      before do
+        KnowledgeBase::PermissionsUpdate
+          .new(another_category)
+          .update! user.roles.first => 'none'
+      end
+
+      it { expect(assets).to include_assets_of category }
+      it { expect(assets).not_to include_assets_of another_category }
+
+      context 'with published answer' do
+        let(:another_category_published_answer) { create(:knowledge_base_answer, :published, category: another_category) }
+
+        before { another_category_published_answer }
+
+        it { expect(assets).to include_assets_of category }
+        it { expect(assets).to include_assets_of another_category }
+      end
+    end
+  end
 end