diff --git a/app/assets/javascripts/app/controllers/knowledge_base/content_controller.coffee b/app/assets/javascripts/app/controllers/knowledge_base/content_controller.coffee index eeae35a0c..0620b019e 100644 --- a/app/assets/javascripts/app/controllers/knowledge_base/content_controller.coffee +++ b/app/assets/javascripts/app/controllers/knowledge_base/content_controller.coffee @@ -58,6 +58,10 @@ class App.KnowledgeBaseContentController extends App.Controller App.KnowledgeBaseFormController.compareParams(remoteParams, @startingParams) objectRefreshed: -> + if !@object.exists() + @parentController.renderNotAvailableAnymore() + return + @renderAvailabilityWidgets() if @remoteDidntChangeSinceStart() diff --git a/app/controllers/knowledge_bases_controller.rb b/app/controllers/knowledge_bases_controller.rb index 4667c42c9..f8d49f363 100644 --- a/app/controllers/knowledge_bases_controller.rb +++ b/app/controllers/knowledge_bases_controller.rb @@ -6,7 +6,7 @@ class KnowledgeBasesController < KnowledgeBase::BaseController end def visible_ids - render json: KnowledgeBase::InternalAssets.new(current_user).visible_ids + render json: calculate_visible_ids end private @@ -22,6 +22,20 @@ class KnowledgeBasesController < KnowledgeBase::BaseController public_assets end + def calculate_visible_ids + if KnowledgeBase.granular_permissions? && kb_permissions? + return KnowledgeBase::InternalAssets.new(current_user).visible_ids + end + + if kb_permission_editor? + editor_assets_visible_ids + elsif kb_permission_reader? + reader_assets_visible_ids + else + {} + end + end + def kb_permissions? current_user&.permissions?(%w[knowledge_base.editor knowledge_base.reader]) end @@ -63,6 +77,13 @@ class KnowledgeBasesController < KnowledgeBase::BaseController assets end + def editor_assets_visible_ids + { + answer_ids: KnowledgeBase::Answer.pluck(:id), + category_ids: KnowledgeBase::Category.pluck(:id) + } + end + def reader_assets(answer_translation_content_ids) assets = [ KnowledgeBase, @@ -96,6 +117,13 @@ class KnowledgeBasesController < KnowledgeBase::BaseController assets end + def reader_assets_visible_ids + { + answer_ids: KnowledgeBase::Answer.internal.pluck(:id), + category_ids: KnowledgeBase::Category.pluck(:id) + } + end + # assets for users who don't have KB permissions def public_assets return [] if !Setting.get('kb_active_publicly') diff --git a/app/jobs/checks_kb_client_notification_job.rb b/app/jobs/checks_kb_client_notification_job.rb index 5402c9641..151990eec 100644 --- a/app/jobs/checks_kb_client_notification_job.rb +++ b/app/jobs/checks_kb_client_notification_job.rb @@ -12,49 +12,39 @@ class ChecksKbClientNotificationJob < ApplicationJob object = klass_name.constantize.find_by(id: object_id) return if object.blank? - level = needs_editor?(object) ? 'editor' : '*' - payload = { event: 'kb_data_changed', - data: build_data(object, event) + data: build_data(object) } - users_for(level).each { |user| notify(user, payload) } - end - - def build_data(object, event) - timestamp = event == :destroy ? Time.zone.now : object.updated_at - url = event == :destroy ? nil : object.try(:api_url) - - { - class: object.class.to_s, - event: event, - id: object.id, - timestamp: timestamp, - url: url - } - end - - def needs_editor?(object) - case object - when KnowledgeBase::Answer - object.can_be_published_aasm.draft? - when KnowledgeBase::Category - !object.internal_content? - else - false + active_users.each do |user| + notify(user, object, payload) end end - def notify(user, payload) - PushMessages.send_to(user.id, payload) + def build_data(object) + { + class: object.class.name, + id: object.id, + timestamp: object.updated_at, + url: object.try(:api_url) + } end - def users_for(permission_suffix) + def notify(user, object, payload) + return if !user.permissions? 'knowledge_base.*' + + Pundit.authorize user, object, :show? + + PushMessages.send_to(user.id, payload) + rescue Pundit::NotAuthorizedError + # do nothing if user is not authorized to access + end + + def active_users Sessions .sessions .filter_map { |client_id| Sessions.get(client_id)&.dig(:user, 'id') } .filter_map { |user_id| User.find_by(id: user_id) } - .select { |user| user.permissions? "knowledge_base.#{permission_suffix}" } end end diff --git a/app/models/concerns/checks_kb_client_notification.rb b/app/models/concerns/checks_kb_client_notification.rb index f98a57a92..157809643 100644 --- a/app/models/concerns/checks_kb_client_notification.rb +++ b/app/models/concerns/checks_kb_client_notification.rb @@ -30,6 +30,10 @@ module ChecksKbClientNotification def notify_kb_clients_after return if self.class.notify_kb_clients_suspend? + # do not leak details about deleted items. See ChecksKbClientVisibilityJob + # after_commit does not allow on: :touch + return if destroyed? + ChecksKbClientNotificationJob.perform_later(self.class.name, id) end end diff --git a/spec/jobs/checks_kb_client_notification_job_spec.rb b/spec/jobs/checks_kb_client_notification_job_spec.rb new file mode 100644 index 000000000..959a24838 --- /dev/null +++ b/spec/jobs/checks_kb_client_notification_job_spec.rb @@ -0,0 +1,40 @@ +# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe ChecksKbClientNotificationJob, type: :job, performs_jobs: true do + include_context 'basic Knowledge Base' + + describe 'pushes to users who have access' do + let(:admin) { create(:admin) } + let(:agent) { create(:agent) } + let(:customer) { create(:customer) } + + before do + allow_any_instance_of(described_class) # rubocop:disable RSpec/AnyInstance + .to receive(:active_users) + .and_return([admin, agent, customer]) + + allow(PushMessages).to receive(:send_to) + + described_class.perform_now 'KnowledgeBase::Answer', answer.id + end + + shared_examples 'message pushed to given users' do |prefix, **args| + context "when answer is #{prefix}" do + let(:answer) { send("#{prefix}_answer") } + + args.each do |key, value| + it "#{key} #{value ? 'is' : 'not'} notified" do + expect(PushMessages).send(value ? :to : :not_to, have_received(:send_to).with(send(key).id, any_args)) + end + end + end + end + + include_examples 'message pushed to given users', 'published', admin: true, agent: true, customer: false + include_examples 'message pushed to given users', 'internal', admin: true, agent: true, customer: false + include_examples 'message pushed to given users', 'draft', admin: true, agent: false, customer: false + include_examples 'message pushed to given users', 'archived', admin: true, agent: false, customer: false + end +end diff --git a/spec/models/concerns/checks_kb_client_notification_examples.rb b/spec/models/concerns/checks_kb_client_notification_examples.rb index 8eb76ff16..0ff69f8f0 100644 --- a/spec/models/concerns/checks_kb_client_notification_examples.rb +++ b/spec/models/concerns/checks_kb_client_notification_examples.rb @@ -5,7 +5,7 @@ RSpec.shared_examples 'ChecksKbClientNotification' do before { subject } it 'on creation' do - expect(ChecksKbClientNotificationJob).to have_been_enqueued.at_least(:once) # some object have associations that triggers touch job after creation + expect(ChecksKbClientNotificationJob).to have_been_enqueued.with(subject.class.name, subject.id) end context 'after initial notifications are cleared' do @@ -13,17 +13,17 @@ RSpec.shared_examples 'ChecksKbClientNotification' do it 'on update' do subject.update(updated_at: Time.zone.now) - expect(ChecksKbClientNotificationJob).to have_been_enqueued.at_least(:once) # some object have associations that triggers touch job after creation + expect(ChecksKbClientNotificationJob).to have_been_enqueued.with(subject.class.name, subject.id) end it 'on touch' do subject.touch - expect(ChecksKbClientNotificationJob).to have_been_enqueued.at_least(:once) # some object have associations that triggers touch job after creation + expect(ChecksKbClientNotificationJob).to have_been_enqueued.with(subject.class.name, subject.id) end it 'on destroy' do subject.destroy - expect(ChecksKbClientNotificationJob).to have_been_enqueued.at_least(:once) # some object have associations that triggers touch job after creation + expect(ChecksKbClientNotificationJob).not_to have_been_enqueued.with(subject.class.name, subject.id) end it 'notifications be disabled' do diff --git a/spec/system/knowledge_base/locale/answer/edit_spec.rb b/spec/system/knowledge_base/locale/answer/edit_spec.rb index d4d3b70e3..399e9ca47 100644 --- a/spec/system/knowledge_base/locale/answer/edit_spec.rb +++ b/spec/system/knowledge_base/locale/answer/edit_spec.rb @@ -124,4 +124,50 @@ RSpec.describe 'Knowledge Base Locale Answer Edit', type: :system do end end end + + context 'deleted by another user' do + before do + visit "#knowledge_base/#{knowledge_base.id}/locale/#{primary_locale.system_locale.locale}/answer/#{published_answer.id}/edit" + end + + it 'shows not available', performs_jobs: true do + find(:active_content, text: published_answer.translations.first.title) + + perform_enqueued_jobs do + ActiveRecord::Base.transaction do + published_answer.destroy + end + end + + within :active_content do + expect(page).to have_text('The page is not available anymore') + end + end + end + + context 'updated by another user' do + before do + ensure_websocket do + visit "#knowledge_base/#{knowledge_base.id}/locale/#{primary_locale.system_locale.locale}/answer/#{published_answer.id}/edit" + end + + travel 1.minute + end + + it 'shows new content', performs_jobs: true do + find(:active_content, text: published_answer.translations.first.title) + + accept_prompt do + perform_enqueued_jobs do + Transaction.execute do + published_answer.translations.first.update! title: 'new title' + end + end + end + + within :active_content do + expect(page).to have_text('new title') + end + end + end end diff --git a/spec/system/knowledge_base/locale/answer/read_spec.rb b/spec/system/knowledge_base/locale/answer/read_spec.rb index 1f6c1e50f..024232bb5 100644 --- a/spec/system/knowledge_base/locale/answer/read_spec.rb +++ b/spec/system/knowledge_base/locale/answer/read_spec.rb @@ -48,4 +48,48 @@ RSpec.describe 'Knowledge Base Locale Answer Read', type: :system, authenticated end end end + + context 'deleted by another user' do + before do + visit "#knowledge_base/#{knowledge_base.id}/locale/#{locale_name}/answer/#{published_answer.id}" + end + + it 'shows not available', performs_jobs: true do + find(:active_content, text: published_answer.translations.first.title) + + perform_enqueued_jobs do + ActiveRecord::Base.transaction do + published_answer.destroy + end + end + + within :active_content do + expect(page).to have_text('The page is not available anymore') + end + end + end + + context 'updated by another user' do + before do + ensure_websocket do + visit "#knowledge_base/#{knowledge_base.id}/locale/#{locale_name}/answer/#{published_answer.id}" + end + + travel 1.minute + end + + it 'shows new content', performs_jobs: true do + find(:active_content, text: published_answer.translations.first.title) + + perform_enqueued_jobs do + Transaction.execute do + published_answer.translations.first.update! title: 'new title' + end + end + + within :active_content do + expect(page).to have_text('new title') + end + end + end end