Commit 55be3f42 authored by fjsanpedro's avatar fjsanpedro

Generate canonical url and remove trailing slash

In this commit we add the logic to generate automatically
the canonical link for urls that end up in `/`.

Only when the url is the host we don't generate it.
parent 6ff0aaff
# frozen_string_literal: true
module PageLayoutHelper
include Gitlab::Utils::StrongMemoize
def page_title(*titles)
@page_title ||= []
......@@ -44,7 +46,7 @@ module PageLayoutHelper
if link
@page_canonical_link = link
else
@page_canonical_link
@page_canonical_link ||= generic_canonical_url
end
end
......@@ -147,4 +149,31 @@ module PageLayoutHelper
css_class.join(' ')
end
private
def generic_canonical_url
strong_memoize(:generic_canonical_url) do
next unless request.get? || request.head?
next unless generate_generic_canonical_url?
next unless Feature.enabled?(:generic_canonical, current_user)
# Request#url builds the url without the trailing slash
request.url
end
end
def generate_generic_canonical_url?
# For the main domain it doesn't matter whether there is
# a trailing slash or not, they're not considered different
# pages
return false if request.path == '/'
# We only need to generate the canonical url when the request has a trailing
# slash. In the request object, only the `original_fullpath` and
# `original_url` keep the slash if it's present. Both `path` and
# `fullpath` would return the path without the slash.
# Therefore, we need to process `original_fullpath`
request.original_fullpath.sub(request.path, '')[0] == '/'
end
end
---
title: Generate canonical url and remove trailing slash
merge_request: 46435
author:
type: changed
---
name: generic_canonical
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46435
rollout_issue_url:
type: development
group: group::editor
default_enabled: false
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Canonical link' do
include Spec::Support::Helpers::Features::CanonicalLinkHelpers
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :public, namespace: user.namespace) }
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:issue_request) { issue_url(issue) }
let_it_be(:project_request) { project_url(project) }
before do
sign_in(user)
end
shared_examples 'shows canonical link' do
specify do
visit request_url
expect(page).to have_canonical_link(expected_url)
end
end
shared_examples 'does not show canonical link' do
specify do
visit request_url
expect(page).not_to have_any_canonical_links
end
end
it_behaves_like 'does not show canonical link' do
let(:request_url) { issue_request }
end
it_behaves_like 'shows canonical link' do
let(:request_url) { issue_request + '/' }
let(:expected_url) { issue_request }
end
it_behaves_like 'shows canonical link' do
let(:request_url) { project_issues_url(project) + "/?state=opened" }
let(:expected_url) { project_issues_url(project, state: 'opened') }
end
it_behaves_like 'does not show canonical link' do
let(:request_url) { project_request }
end
it_behaves_like 'shows canonical link' do
let(:request_url) { project_request + '/' }
let(:expected_url) { project_request }
end
it_behaves_like 'shows canonical link' do
let(:query_params) { '?foo=bar' }
let(:request_url) { project_request + "/#{query_params}" }
let(:expected_url) { project_request + query_params }
end
# Hard-coded canonical links
it_behaves_like 'shows canonical link' do
let(:request_url) { explore_root_path }
let(:expected_url) { explore_projects_url }
end
context 'when feature flag generic_canonical is disabled' do
before do
stub_feature_flags(generic_canonical: false)
end
it_behaves_like 'does not show canonical link' do
let(:request_url) { issue_request + '/' }
end
it_behaves_like 'does not show canonical link' do
let(:request_url) { project_request + '/' }
end
end
end
......@@ -30,12 +30,4 @@ RSpec.describe 'Root explore' do
include_examples 'shows public projects'
end
it 'includes canonical link to explore projects url' do
visit explore_root_path
canonial_link = page.find("head link[rel='canonical']", visible: false)
expect(canonial_link[:href]).to eq explore_projects_url
end
end
......@@ -137,4 +137,75 @@ RSpec.describe PageLayoutHelper do
end
end
end
describe '#page_canonical_link' do
let(:user) { build(:user) }
subject { helper.page_canonical_link(link) }
before do
allow(helper).to receive(:current_user).and_return(user)
end
context 'when link is passed' do
let(:link) { 'https://gitlab.com' }
it 'stores and returns the link value' do
expect(subject).to eq link
expect(helper.page_canonical_link(nil)).to eq link
end
end
context 'when no link is provided' do
let(:link) { nil }
let(:request) { ActionDispatch::Request.new(env) }
let(:env) do
{
'ORIGINAL_FULLPATH' => '/foo/',
'PATH_INFO' => '/foo',
'HTTP_HOST' => 'test.host',
'REQUEST_METHOD' => method,
'rack.url_scheme' => 'http'
}
end
before do
allow(helper).to receive(:request).and_return(request)
end
shared_examples 'generates the canonical url using the params in the context' do
specify { expect(subject).to eq 'http://test.host/foo' }
end
shared_examples 'does not return a canonical url' do
specify { expect(subject).to be_nil }
end
it_behaves_like 'generates the canonical url using the params in the context' do
let(:method) { 'GET' }
end
it_behaves_like 'generates the canonical url using the params in the context' do
let(:method) { 'HEAD' }
end
it_behaves_like 'does not return a canonical url' do
let(:method) { 'POST' }
end
it_behaves_like 'does not return a canonical url' do
let(:method) { 'PUT' }
end
context 'when feature flag generic_canonical is disabled' do
let(:method) { 'GET' }
before do
stub_feature_flags(generic_canonical: false)
end
it_behaves_like 'does not return a canonical url'
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment