From c33bd9c7e365fc625129ce4e8fbd1c103f1401b6 Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Tue, 24 Sep 2019 17:44:32 +0200 Subject: [PATCH] Maintenance: Added test coverage for Knowledge Base initial data loading and translation update. --- .../knowledge_base/base_controller.rb | 2 +- .../knowledge_base/layouts_controller.rb | 4 -- .../knowledge_base/locales_controller.rb | 4 -- app/controllers/knowledge_bases_controller.rb | 1 - config/routes/knowledge_base.rb | 7 +- .../loading_initial_data_spec.rb | 52 ++++++++++++++ .../knowledge_base/translation_update_spec.rb | 70 +++++++++++++++++++ spec/support/assets_matchers.rb | 59 ++++++++++++++++ spec/support/knowledge_base_contexts.rb | 8 +-- .../editor_search_spec.rb | 4 +- .../knowledge_base_public/editor_spec.rb | 4 +- .../guest_search_spec.rb | 4 +- .../knowledge_base_public/guest_spec.rb | 4 +- 13 files changed, 192 insertions(+), 31 deletions(-) delete mode 100644 app/controllers/knowledge_base/layouts_controller.rb delete mode 100644 app/controllers/knowledge_base/locales_controller.rb create mode 100644 spec/requests/knowledge_base/loading_initial_data_spec.rb create mode 100644 spec/requests/knowledge_base/translation_update_spec.rb create mode 100644 spec/support/assets_matchers.rb diff --git a/app/controllers/knowledge_base/base_controller.rb b/app/controllers/knowledge_base/base_controller.rb index e56bf05c9..86b9c1d82 100644 --- a/app/controllers/knowledge_base/base_controller.rb +++ b/app/controllers/knowledge_base/base_controller.rb @@ -32,7 +32,7 @@ class KnowledgeBase::BaseController < ApplicationController end def ensure_editor - permission_check %w[knowledge_base.editor] + permission_check 'knowledge_base.editor' end def ensure_editor_or_reader diff --git a/app/controllers/knowledge_base/layouts_controller.rb b/app/controllers/knowledge_base/layouts_controller.rb deleted file mode 100644 index 4873d088a..000000000 --- a/app/controllers/knowledge_base/layouts_controller.rb +++ /dev/null @@ -1,4 +0,0 @@ -# Copyright (C) 2012-2017 Zammad Foundation, http://zammad-foundation.org/ - -class KnowledgeBase::LayoutsController < KnowledgeBase::BaseController -end diff --git a/app/controllers/knowledge_base/locales_controller.rb b/app/controllers/knowledge_base/locales_controller.rb deleted file mode 100644 index 4fde4266a..000000000 --- a/app/controllers/knowledge_base/locales_controller.rb +++ /dev/null @@ -1,4 +0,0 @@ -# Copyright (C) 2012-2017 Zammad Foundation, http://zammad-foundation.org/ - -class KnowledgeBase::LocalesController < KnowledgeBase::BaseController -end diff --git a/app/controllers/knowledge_bases_controller.rb b/app/controllers/knowledge_bases_controller.rb index 3fab2af50..85c4aaf22 100644 --- a/app/controllers/knowledge_bases_controller.rb +++ b/app/controllers/knowledge_bases_controller.rb @@ -1,7 +1,6 @@ # Copyright (C) 2012-2017 Zammad Foundation, http://zammad-foundation.org/ class KnowledgeBasesController < KnowledgeBase::BaseController - skip_before_action :authentication_check, only: :init skip_before_action :ensure_editor_or_reader, only: :init def init diff --git a/config/routes/knowledge_base.rb b/config/routes/knowledge_base.rb index 56a8cd17c..7741b07b6 100644 --- a/config/routes/knowledge_base.rb +++ b/config/routes/knowledge_base.rb @@ -33,11 +33,6 @@ Zammad::Application.routes.draw do end end - # add routes - %w[locales layouts].each do |route_name| - resources route_name, controller: "knowledge_base/#{route_name}", except: %i[new edit] - end - resources :categories, controller: 'knowledge_base/categories', except: %i[new edit] do @@ -51,7 +46,7 @@ Zammad::Application.routes.draw do end resources :answers, controller: 'knowledge_base/answers', - except: %i[new edit], + only: %i[create update show destroy], concerns: :has_publishing do resources :attachments, controller: 'knowledge_base/answer/attachments', only: %i[create destroy] diff --git a/spec/requests/knowledge_base/loading_initial_data_spec.rb b/spec/requests/knowledge_base/loading_initial_data_spec.rb new file mode 100644 index 000000000..e6354d740 --- /dev/null +++ b/spec/requests/knowledge_base/loading_initial_data_spec.rb @@ -0,0 +1,52 @@ +require 'rails_helper' + +RSpec.describe 'KnowledgeBase loading initial data', type: :request, searchindex: true do + include_context 'basic Knowledge Base' do + before do + draft_answer + internal_answer + published_answer + end + end + + before do + post '/api/v1/knowledge_bases/init' + end + + shared_examples 'returning valid JSON' do + it { expect(response).to have_http_status(:ok) } + it { expect(json_response).to be_a_kind_of(Hash) } + end + + describe 'for admin', authenticated_as: :admin_user do + it_behaves_like 'returning valid JSON' + + it 'returns assets for all KB objects' do + expect(json_response).to include_assets_of(knowledge_base, category, draft_answer, internal_answer, published_answer) + end + end + + describe 'for agent', authenticated_as: :agent_user do + it_behaves_like 'returning valid JSON' + + it 'returns assets for all KB objects except drafts' do + expect(json_response) + .to include_assets_of(knowledge_base, category, internal_answer, published_answer) + .and not_include_assets_of(draft_answer) + end + end + + describe 'for customer', authenticated_as: :customer_user do + it_behaves_like 'returning valid JSON' + + it 'only returns assets for KB itself' do + expect(json_response) + .to include_assets_of(knowledge_base) + .and not_include_assets_of(category, draft_answer, internal_answer, published_answer) + end + end + + describe 'for guests without authorization' do + it { expect(response).to have_http_status(:unauthorized) } + end +end diff --git a/spec/requests/knowledge_base/translation_update_spec.rb b/spec/requests/knowledge_base/translation_update_spec.rb new file mode 100644 index 000000000..9ba1e3c4c --- /dev/null +++ b/spec/requests/knowledge_base/translation_update_spec.rb @@ -0,0 +1,70 @@ +require 'rails_helper' + +RSpec.describe 'KnowledgeBase translation update', type: :request, authentication: true do + include_context 'basic Knowledge Base' + + let(:new_title) { 'new title for update test' } + + let(:params_for_updating) do + { + "translations_attributes": [ + { + "title": new_title, + "footer_note": 'new footer', + "id": knowledge_base.kb_locales.first.id + } + ] + } + end + + let(:request) do + patch "/api/v1/knowledge_bases/#{knowledge_base.id}?full=true", params: params_for_updating, as: :json + end + + describe 'changes KB translation title' do + describe 'as editor', authenticated_as: :admin_user do + it 'updates title' do + expect { request }.to change { knowledge_base.reload.translations.first.title }.to(new_title) + end + end + + describe 'as reader', authenticated_as: :agent_user do + it 'does not change title' do + expect { request }.not_to change { knowledge_base.reload.translations.first.title } + end + end + + describe 'as non-KB user', authenticated_as: :customer do + it 'does not change title' do + expect { request }.not_to change { knowledge_base.reload.translations.first.title } + end + end + + describe 'as a guest' do + it 'does not change title' do + expect { request }.not_to change { knowledge_base.reload.translations.first.title } + end + end + end + + describe 'can make request to KB translation' do + before { request } + + describe 'as editor', authenticated_as: :admin_user do + it { expect(response).to have_http_status(:ok) } + it { expect(json_response).to be_a_kind_of(Hash) } + end + + describe 'as reader', authenticated_as: :agent_user do + it { expect(response).to have_http_status(:unauthorized) } + end + + describe 'as non-KB user', authenticated_as: :customer do + it { expect(response).to have_http_status(:unauthorized) } + end + + describe 'as a guest' do + it { expect(response).to have_http_status(:unauthorized) } + end + end +end diff --git a/spec/support/assets_matchers.rb b/spec/support/assets_matchers.rb new file mode 100644 index 000000000..7aea5df79 --- /dev/null +++ b/spec/support/assets_matchers.rb @@ -0,0 +1,59 @@ +RSpec::Matchers.define :include_assets_of do + match do |actual| + expected_array.all? { |elem| find_assets_of(elem, actual) } + end + + match_when_negated do |actual| + expected_array.none? { |elem| find_assets_of(elem, actual) } + end + + description do + "include assets of #{expected_name}" + end + + failure_message do |actual| + list = expected_array.reject { |elem| find_assets_of(elem, actual) } + "Expected hash to include, but not included:\n" + items_for_message(list) + end + + failure_message_when_negated do |actual| + list = expected_array.select { |elem| find_assets_of(elem, actual) } + "Expected hash to not include, but was included:\n" + items_for_message(list) + end + + def items_for_message(items) + items + .map { |elem| "- #{item_name(elem)}" } + .join("\n") + end + + def expected_name + expected_array + .map { |elem| item_name(elem) } + .join(', ') + end + + def item_name(item) + "#{item.class.name}##{item.id}" + end + + def expected_array + Array(expected) + end + + # Finds corresponding object's data in assets hash + # + # @param [ActiveRecord::Base] object to look for + # @param [Hash] assets hash to use + # + # @example + # assets = Ticket.first.assets + # find_assets_of(Ticket.first, assets) + # + # @return [Hash, nil] + def find_assets_of(object, actual) + actual.dig(object.class.name.gsub(/::/, ''), object.id.to_s) + end +end + +RSpec::Matchers.define_negated_matcher :not_include_assets_of, :include_assets_of diff --git a/spec/support/knowledge_base_contexts.rb b/spec/support/knowledge_base_contexts.rb index 3e8b7b354..814639039 100644 --- a/spec/support/knowledge_base_contexts.rb +++ b/spec/support/knowledge_base_contexts.rb @@ -1,9 +1,3 @@ -RSpec.shared_context 'Knowledge Base users' do - let(:admin_user) { create :admin_user } - let(:agent_user) { create :agent_user } - let(:customer_user) { create :customer_user } -end - RSpec.shared_context 'basic Knowledge Base', current_user_id: 1 do let :knowledge_base do create(:knowledge_base) @@ -21,7 +15,7 @@ RSpec.shared_context 'basic Knowledge Base', current_user_id: 1 do create(:knowledge_base_category, knowledge_base: knowledge_base) end - let :answer do + let :draft_answer do create(:knowledge_base_answer, category: category) end diff --git a/spec/system/knowledge_base_public/editor_search_spec.rb b/spec/system/knowledge_base_public/editor_search_spec.rb index 7b01dd6cd..11d39e16f 100644 --- a/spec/system/knowledge_base_public/editor_search_spec.rb +++ b/spec/system/knowledge_base_public/editor_search_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'Public Knowledge Base for guest search', type: :system, authenti before do configure_elasticsearch(required: true, rebuild: true) do - published_answer && answer && internal_answer + published_answer && draft_answer && internal_answer end visit help_no_locale_path @@ -21,7 +21,7 @@ RSpec.describe 'Public Knowledge Base for guest search', type: :system, authenti end it 'list draft article' do - expect(page).to allow_to_search_for answer + expect(page).to allow_to_search_for draft_answer end it 'list internal article' do diff --git a/spec/system/knowledge_base_public/editor_spec.rb b/spec/system/knowledge_base_public/editor_spec.rb index c85e4c5cf..3edc09f22 100644 --- a/spec/system/knowledge_base_public/editor_spec.rb +++ b/spec/system/knowledge_base_public/editor_spec.rb @@ -4,7 +4,7 @@ RSpec.describe 'Public Knowledge Base for editor', type: :system, authenticated: include_context 'basic Knowledge Base' before do - published_answer && answer && internal_answer + published_answer && draft_answer && internal_answer end context 'homepage' do @@ -29,7 +29,7 @@ RSpec.describe 'Public Knowledge Base for editor', type: :system, authenticated: it 'shows draft answer' do within '.main' do - expect(page).to have_selector(:link_containing, answer.translation.title) + expect(page).to have_selector(:link_containing, draft_answer.translation.title) end end diff --git a/spec/system/knowledge_base_public/guest_search_spec.rb b/spec/system/knowledge_base_public/guest_search_spec.rb index e7abc247a..fc1e3e290 100644 --- a/spec/system/knowledge_base_public/guest_search_spec.rb +++ b/spec/system/knowledge_base_public/guest_search_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'Public Knowledge Base for guest search', type: :system, authenti before do configure_elasticsearch(required: true, rebuild: true) do - published_answer && answer && internal_answer + published_answer && draft_answer && internal_answer end visit help_no_locale_path @@ -21,7 +21,7 @@ RSpec.describe 'Public Knowledge Base for guest search', type: :system, authenti end it 'not list draft article' do - expect(page).not_to allow_to_search_for answer + expect(page).not_to allow_to_search_for draft_answer end it 'not list internal article' do diff --git a/spec/system/knowledge_base_public/guest_spec.rb b/spec/system/knowledge_base_public/guest_spec.rb index a44b2974e..f949e7838 100644 --- a/spec/system/knowledge_base_public/guest_spec.rb +++ b/spec/system/knowledge_base_public/guest_spec.rb @@ -4,7 +4,7 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated: include_context 'basic Knowledge Base' before do - published_answer && answer && internal_answer + published_answer && draft_answer && internal_answer end context 'homepage' do @@ -41,7 +41,7 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated: it 'does not show draft answer' do within '.main' do - expect(page).not_to have_selector(:link_containing, answer.translation.title) + expect(page).not_to have_selector(:link_containing, draft_answer.translation.title) end end