GitBlixt
teagles/steam

the prefix_default_locale site property was not applied when building an url

did committed May 14, 2016
commit 3022602be59fd2c03fa2cee85b242fa5a45ea63f
Showing 8 changed files with 108 additions and 41 deletions
locomotive/steam/middlewares/locale_redirection.rb b/lib/locomotive/steam/middlewares/locale_redirection.rb +17 -7
@@ @@ -11,19 +11,29 @@ module Locomotive::Steam
include Helpers
def _call
- if apply_redirection?
- redirect_to path_with_default_locale
+ if url = redirect_url
+ redirect_to url
end
end
protected
+ def redirect_url
+ if apply_redirection?
+ if site.prefix_default_locale
+ path_with_default_locale if locale_not_mentioned_in_path?
+ else
+ env['steam.path'] if default_locale? && locale_mentioned_in_path?
+ end
+ end
+ end
+
def apply_redirection?
- site.locales.size > 1 &&
- site.prefix_default_locale &&
- request.get? &&
- env['PATH_INFO'] != '/sitemap.xml' &&
- locale_not_mentioned_in_path?
+ site.locales.size > 1 && request.get? && env['PATH_INFO'] != '/sitemap.xml'
+ end
+
+ def default_locale?
+ locale.to_s == site.default_locale.to_s
end
def locale_mentioned_in_path?
locomotive/steam/services/url_builder_service.rb b/lib/locomotive/steam/services/url_builder_service.rb +5 -2
@@ @@ -14,8 +14,11 @@ module Locomotive
locale ||= current_locale
same_locale = locale.to_sym == site.default_locale.to_sym
- # locale
- segments << locale unless same_locale
+ # if the prefix_default_locale is enabled, we need to
+ # add the locale no matter if the locale is the same as the default one
+ if site.prefix_default_locale || !same_locale
+ segments << locale
+ end
# fullpath
segments << sanitized_fullpath(page, same_locale)
spec/unit/liquid/tags/link_to_spec.rb +12 -4
@@ @@ -2,10 +2,11 @@ require 'spec_helper'
describe Locomotive::Steam::Liquid::Tags::PathTo do
- let(:assigns) { {} }
- let(:services) { Locomotive::Steam::Services.build_instance }
- let(:site) { instance_double('Site', default_locale: 'en') }
- let(:context) { ::Liquid::Context.new(assigns, {}, { services: services, site: site, locale: 'en' }) }
+ let(:prefix_default) { false }
+ let(:assigns) { {} }
+ let(:services) { Locomotive::Steam::Services.build_instance }
+ let(:site) { instance_double('Site', default_locale: 'en', prefix_default_locale: prefix_default) }
+ let(:context) { ::Liquid::Context.new(assigns, {}, { services: services, site: site, locale: 'en' }) }
subject { render_template(source, context) }
@@ @@ -67,6 +68,13 @@ describe Locomotive::Steam::Liquid::Tags::PathTo do
end
+ context 'prefix_default_locale is true' do
+
+ let(:prefix_default) { true }
+ it { is_expected.to eq 'My link: <a href="/en/">Home</a>!' }
+
+ end
+
end
describe 'used as a block' do
spec/unit/liquid/tags/locale_switcher_spec.rb +15 -7
@@ @@ -2,13 +2,14 @@ require 'spec_helper'
describe Locomotive::Steam::Liquid::Tags::LocaleSwitcher do
- let(:locale) { 'en' }
- let(:assigns) { { 'page' => drop } }
- let(:services) { Locomotive::Steam::Services.build_instance }
- let(:site) { instance_double('Site', locales: %w(en fr), default_locale: 'en') }
- let(:drop) { Locomotive::Steam::Liquid::Drops::Page.new(page) }
- let(:page) { liquid_instance_double('Index', localized_attributes: { title: true, fullpath: true }, title: { en: 'Home', fr: 'Accueil' }, fullpath: { en: 'index', fr: 'index' }, templatized?: false) }
- let(:context) { ::Liquid::Context.new(assigns, {}, { services: services, site: site, locale: locale }) }
+ let(:locale) { 'en' }
+ let(:assigns) { { 'page' => drop } }
+ let(:prefix_default) { false }
+ let(:services) { Locomotive::Steam::Services.build_instance }
+ let(:site) { instance_double('Site', locales: %w(en fr), default_locale: 'en', prefix_default_locale: prefix_default) }
+ let(:drop) { Locomotive::Steam::Liquid::Drops::Page.new(page) }
+ let(:page) { liquid_instance_double('Index', localized_attributes: { title: true, fullpath: true }, title: { en: 'Home', fr: 'Accueil' }, fullpath: { en: 'index', fr: 'index' }, templatized?: false) }
+ let(:context) { ::Liquid::Context.new(assigns, {}, { services: services, site: site, locale: locale }) }
subject { render_template(source, context) }
@@ @@ -26,6 +27,13 @@ describe Locomotive::Steam::Liquid::Tags::LocaleSwitcher do
end
+ context 'prefix_default_locale is true' do
+
+ let(:prefix_default) { true }
+ it { is_expected.to eq '<div id="locale-switcher"><a href="/en/" class="en current">en</a> | <a href="/fr" class="fr">fr</a></div>' }
+
+ end
+
end
describe 'using the locale to display the links' do
spec/unit/liquid/tags/nav_spec.rb +19 -11
@@ @@ -19,17 +19,18 @@ describe 'Locomotive::Steam::Liquid::Tags::Nav' do
]
end
- let(:source) { '{% nav site %}' }
- let(:site) { instance_double('Site', name: 'My portfolio', default_locale: 'en') }
- let(:page) { index }
- let(:services) { Locomotive::Steam::Services.build_instance }
- let(:repository) { services.repositories.page }
- let(:assigns) { {} }
- let(:registers) { { services: services, site: site, page: page } }
- let(:context) { ::Liquid::Context.new(assigns, {}, registers) }
- let(:options) { { services: services } }
-
- let(:output) { render_template(source, context, options) }
+ let(:prefix_default) { false }
+ let(:source) { '{% nav site %}' }
+ let(:site) { instance_double('Site', name: 'My portfolio', default_locale: 'en', prefix_default_locale: prefix_default) }
+ let(:page) { index }
+ let(:services) { Locomotive::Steam::Services.build_instance }
+ let(:repository) { services.repositories.page }
+ let(:assigns) { {} }
+ let(:registers) { { services: services, site: site, page: page } }
+ let(:context) { ::Liquid::Context.new(assigns, {}, registers) }
+ let(:options) { { services: services } }
+
+ let(:output) { render_template(source, context, options) }
before { allow(services).to receive(:current_site).and_return(site) }
@@ @@ -46,6 +47,13 @@ describe 'Locomotive::Steam::Liquid::Tags::Nav' do
it { is_expected.to eq %{<nav id="nav"><ul><li id="child-1-link" class="link first"><a href="/child-1">Child #1</a></li>\n<li id="child-2-link" class="link last"><a href="/child-2">Child #2</a></li></ul></nav>} }
+ context 'prefix_default_locale is true' do
+
+ let(:prefix_default) { true }
+ it { is_expected.to eq %{<nav id="nav"><ul><li id="child-1-link" class="link first"><a href="/en/child-1">Child #1</a></li>\n<li id="child-2-link" class="link last"><a href="/en/child-2">Child #2</a></li></ul></nav>} }
+
+ end
+
end
describe 'from a page' do
spec/unit/liquid/tags/path_to_spec.rb +12 -4
@@ @@ -2,10 +2,11 @@ require 'spec_helper'
describe Locomotive::Steam::Liquid::Tags::PathTo do
- let(:assigns) { {} }
- let(:services) { Locomotive::Steam::Services.build_instance }
- let(:site) { instance_double('Site', locales: ['en'], default_locale: 'en') }
- let(:context) { ::Liquid::Context.new(assigns, {}, { services: services, site: site, locale: 'en' }) }
+ let(:prefix_default) { false }
+ let(:assigns) { {} }
+ let(:services) { Locomotive::Steam::Services.build_instance }
+ let(:site) { instance_double('Site', locales: ['en'], default_locale: 'en', prefix_default_locale: prefix_default) }
+ let(:context) { ::Liquid::Context.new(assigns, {}, { services: services, site: site, locale: 'en' }) }
subject { render_template(source, context) }
@@ @@ -51,6 +52,13 @@ describe Locomotive::Steam::Liquid::Tags::PathTo do
end
+ context 'prefix_default_locale is true' do
+
+ let(:prefix_default) { true }
+ it { is_expected.to eq '/en/' }
+
+ end
+
end
describe 'from a page (drop) itself' do
spec/unit/middlewares/locale_redirection_spec.rb +14 -1
@@ @@ -26,7 +26,20 @@ describe Locomotive::Steam::Middlewares::LocaleRedirection do
describe 'prefix_default_locale is false' do
let(:prefixed) { false }
- it { is_expected.to eq [200, nil] }
+
+ describe 'locale is not part of the path' do
+
+ let(:locale_in_path) { false }
+ it { is_expected.to eq [200, nil] }
+
+ end
+
+ describe 'for seo purpose redirect to the path without the locale' do
+
+ let(:url) { 'http://models.example.com/de/hello' }
+ it { is_expected.to eq [301, '/hello'] }
+
+ end
end
spec/unit/services/url_builder_service_spec.rb +14 -5
@@ @@ -2,11 +2,12 @@ require 'spec_helper'
describe Locomotive::Steam::UrlBuilderService do
- let(:mounted_on) { nil }
- let(:request) { instance_double('Request', env: { 'steam.mounted_on' => mounted_on }) }
- let(:site) { instance_double('Site', default_locale: 'en') }
- let(:locale) { 'en' }
- let(:service) { described_class.new(site, locale, request) }
+ let(:prefix_default) { false }
+ let(:mounted_on) { nil }
+ let(:request) { instance_double('Request', env: { 'steam.mounted_on' => mounted_on }) }
+ let(:site) { instance_double('Site', default_locale: 'en', prefix_default_locale: prefix_default) }
+ let(:locale) { 'en' }
+ let(:service) { described_class.new(site, locale, request) }
describe '#url_for' do
@@ @@ -16,6 +17,14 @@ describe Locomotive::Steam::UrlBuilderService do
it { is_expected.to eq '/about-us' }
+ describe 'the prefix_default_locale site property is enabled' do
+
+ let(:prefix_default) { true }
+
+ it { is_expected.to eq '/en/about-us' }
+
+ end
+
describe 'a locale different from the default one' do
let(:locale) { 'fr' }