Fixes #3370 - "canonical"-tag is missing in knowledge base
This commit is contained in:
parent
2a61be02be
commit
6c9148b45b
4 changed files with 143 additions and 1 deletions
|
@ -11,7 +11,7 @@ module KnowledgeBaseHelper
|
||||||
custom_address = knowledge_base.custom_address_uri
|
custom_address = knowledge_base.custom_address_uri
|
||||||
return path if !custom_address
|
return path if !custom_address
|
||||||
|
|
||||||
custom_path = path.gsub(%r{^/help}, custom_address.path || '').presence || '/'
|
custom_path = knowledge_base.custom_address_path(path)
|
||||||
prefix = full ? knowledge_base.custom_address_prefix(request) : ''
|
prefix = full ? knowledge_base.custom_address_prefix(request) : ''
|
||||||
|
|
||||||
"#{prefix}#{custom_path}"
|
"#{prefix}#{custom_path}"
|
||||||
|
@ -61,4 +61,18 @@ module KnowledgeBaseHelper
|
||||||
def dropdown_menu_direction
|
def dropdown_menu_direction
|
||||||
system_locale_via_uri.dir == 'ltr' ? 'right' : 'left'
|
system_locale_via_uri.dir == 'ltr' ? 'right' : 'left'
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def canonical_link_tag(knowledge_base, *objects)
|
||||||
|
path = kb_public_system_path(*objects)
|
||||||
|
|
||||||
|
tag :link, rel: 'canonical', href: knowledge_base.canonical_url(path)
|
||||||
|
end
|
||||||
|
|
||||||
|
def kb_public_system_path(*objects)
|
||||||
|
objects
|
||||||
|
.compact
|
||||||
|
.map { |elem| elem.translation.to_param }
|
||||||
|
.unshift(help_root_path)
|
||||||
|
.join('/')
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -102,6 +102,29 @@ class KnowledgeBase < ApplicationModel
|
||||||
"#{custom_address_uri.scheme}://#{host}#{port_string}"
|
"#{custom_address_uri.scheme}://#{host}#{port_string}"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def custom_address_path(path)
|
||||||
|
uri = custom_address_uri
|
||||||
|
|
||||||
|
return path if !uri
|
||||||
|
|
||||||
|
custom_path = custom_address_uri.path || ''
|
||||||
|
applied_path = path.gsub(%r{^/help}, custom_path)
|
||||||
|
|
||||||
|
applied_path.presence || '/'
|
||||||
|
end
|
||||||
|
|
||||||
|
def canonical_host
|
||||||
|
custom_address_uri&.host || Setting.get('fqdn')
|
||||||
|
end
|
||||||
|
|
||||||
|
def canonical_scheme_host
|
||||||
|
"#{Setting.get('http_type')}://#{canonical_host}"
|
||||||
|
end
|
||||||
|
|
||||||
|
def canonical_url(path)
|
||||||
|
"#{canonical_scheme_host}#{custom_address_path(path)}"
|
||||||
|
end
|
||||||
|
|
||||||
def full_destroy!
|
def full_destroy!
|
||||||
ChecksKbClientNotification.disable_in_all_classes!
|
ChecksKbClientNotification.disable_in_all_classes!
|
||||||
|
|
||||||
|
|
|
@ -13,6 +13,7 @@
|
||||||
<meta name="viewport" content="width=device-width,initial-scale=1.0,viewport-fit=cover">
|
<meta name="viewport" content="width=device-width,initial-scale=1.0,viewport-fit=cover">
|
||||||
<%= stylesheet_link_tag "knowledge_base.css", :media => 'all' %>
|
<%= stylesheet_link_tag "knowledge_base.css", :media => 'all' %>
|
||||||
<%= render 'knowledge_base/public/inline_stylesheet', knowledge_base: @knowledge_base, locale: system_locale_via_uri %>
|
<%= render 'knowledge_base/public/inline_stylesheet', knowledge_base: @knowledge_base, locale: system_locale_via_uri %>
|
||||||
|
<%= canonical_link_tag @knowledge_base, @category, @object %>
|
||||||
|
|
||||||
<div class="wrapper js-wrapper">
|
<div class="wrapper js-wrapper">
|
||||||
<%= render 'knowledge_base/public/top_banner', object: @object || @knowledge_base if editor? %>
|
<%= render 'knowledge_base/public/top_banner', object: @object || @knowledge_base if editor? %>
|
||||||
|
|
104
spec/system/knowledge_base_public/canonical_link_spec.rb
Normal file
104
spec/system/knowledge_base_public/canonical_link_spec.rb
Normal file
|
@ -0,0 +1,104 @@
|
||||||
|
require 'rails_helper'
|
||||||
|
|
||||||
|
RSpec.describe 'Public Knowledge Base canonical link', type: :system, current_user_id: 1, authenticated_as: false do
|
||||||
|
include_context 'basic Knowledge Base'
|
||||||
|
|
||||||
|
let(:path) { '/path' }
|
||||||
|
let(:subdomain) { 'subdomain.example.net' }
|
||||||
|
let(:locale) { primary_locale.system_locale.locale }
|
||||||
|
let(:category_slug) { category.translations.first.to_param }
|
||||||
|
let(:answer_slug) { published_answer.translations.first.to_param }
|
||||||
|
|
||||||
|
before do
|
||||||
|
published_answer
|
||||||
|
knowledge_base.update! custom_address: custom_address
|
||||||
|
end
|
||||||
|
|
||||||
|
shared_examples 'having canonical links on all pages' do
|
||||||
|
it 'includes canonical link on home page' do
|
||||||
|
visit help_root_path(locale)
|
||||||
|
expect(page).to have_canonical_url("#{prefix}/#{locale}")
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'includes canonical link on category page' do
|
||||||
|
visit help_category_path(locale, category)
|
||||||
|
expect(page).to have_canonical_url("#{prefix}/#{locale}/#{category_slug}")
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'includes canonical link on answer page' do
|
||||||
|
visit help_answer_path(locale, published_answer.category, published_answer)
|
||||||
|
expect(page).to have_canonical_url("#{prefix}/#{locale}/#{category_slug}/#{answer_slug}")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
shared_examples 'core locations' do
|
||||||
|
let(:scheme) { ssl ? 'https' : 'http' }
|
||||||
|
before { Setting.set('http_type', scheme) }
|
||||||
|
|
||||||
|
context 'with custom domain' do
|
||||||
|
let(:custom_address) { subdomain }
|
||||||
|
let(:prefix) { "#{scheme}://#{subdomain}" }
|
||||||
|
|
||||||
|
it_behaves_like 'having canonical links on all pages'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with custom path' do
|
||||||
|
let(:custom_address) { path }
|
||||||
|
let(:prefix) { "#{scheme}://#{Setting.get('fqdn')}#{path}" }
|
||||||
|
|
||||||
|
it_behaves_like 'having canonical links on all pages'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with custom domain and path' do
|
||||||
|
let(:custom_address) { "#{subdomain}#{path}" }
|
||||||
|
let(:prefix) { "#{scheme}://#{subdomain}#{path}" }
|
||||||
|
|
||||||
|
it_behaves_like 'having canonical links on all pages'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'without custom address' do
|
||||||
|
let(:custom_address) { nil }
|
||||||
|
let(:prefix) { "#{scheme}://#{Setting.get('fqdn')}/help" }
|
||||||
|
|
||||||
|
it_behaves_like 'having canonical links on all pages'
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when SSL disabled' do
|
||||||
|
let(:ssl) { false }
|
||||||
|
|
||||||
|
include_examples 'core locations'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when SSL enabled' do
|
||||||
|
let(:ssl) { true }
|
||||||
|
|
||||||
|
include_examples 'core locations'
|
||||||
|
end
|
||||||
|
|
||||||
|
matcher :have_canonical_url do |expected|
|
||||||
|
match do
|
||||||
|
return false if canonical_link_element.blank?
|
||||||
|
|
||||||
|
canonical_link_target == expected
|
||||||
|
end
|
||||||
|
|
||||||
|
failure_message do
|
||||||
|
return 'no canonical link found' if canonical_link_element.blank?
|
||||||
|
|
||||||
|
"expected canonical link pointing to \"#{expected}\", but found \"#{canonical_link_target}\" instead"
|
||||||
|
end
|
||||||
|
|
||||||
|
def canonical_link_element
|
||||||
|
return @canonical_link_element if defined?(@canonical_link_element)
|
||||||
|
|
||||||
|
@canonical_link_element = actual.first('head link[rel=canonical]', visible: :hidden, minimum: 0)
|
||||||
|
end
|
||||||
|
|
||||||
|
def canonical_link_target
|
||||||
|
@canonical_link_target ||= canonical_link_element[:href]
|
||||||
|
end
|
||||||
|
|
||||||
|
description { "have canonical tag with href of #{expected}" }
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in a new issue