Permalink
Browse files

Merge pull request #1698 from resolve/canonical

Added canonical support which helps for #1457
  • Loading branch information...
2 parents f8b113e + c642439 commit 4c3202f82380c0c215924cecb36c0cc3c0da9379 @robyurkowski robyurkowski committed May 25, 2012
@@ -2,9 +2,9 @@
<meta charset='<%= Rails.application.config.encoding %>' />
<!--[if IE]><meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" /><![endif]-->
<title><%= browser_title(yield(:title)) %></title>
- <%= raw(%(<meta name="description" content="#{@meta.meta_description}" />)) if @meta.meta_description.present? -%>
- <%= raw(%(<meta name="keywords" content="#{@meta.meta_keywords}">)) if @meta.meta_keywords.present? -%>
- <%= raw(%(<link rel="canonical" content="#{@canonical}" />)) if @canonical.present? -%>
+ <%= raw %(<meta name="description" content="#{@meta.meta_description}" />) if @meta.meta_description.present? -%>
+ <%= raw %(<meta name="keywords" content="#{@meta.meta_keywords}">) if @meta.meta_keywords.present? -%>
+ <%= raw %(<link rel="canonical" content="#{@canonical}" />) if @canonical.present? -%>
<%= csrf_meta_tags if Refinery::Core.authenticity_token_on_frontend -%>
<%= yield :meta %>
@@ -1,6 +1,6 @@
module Refinery
class PagesController < ::ApplicationController
- before_filter :find_page, :except => [:preview]
+ before_filter :find_page, :set_canonical, :except => [:preview]
# Save whole Page after delivery
after_filter { |c| c.write_cache? }
@@ -94,6 +94,10 @@ def render_with_templates?(render_options = {})
render render_options if render_options.any?
end
+ def set_canonical
+ @canonical = refinery.url_for @page.canonical if @page.present?
+ end
+
def write_cache?
if Refinery::Pages.cache_pages_full
cache_page(response.body, File.join('', 'refinery', 'cache', 'pages', request.path.sub("//", "/")).to_s)
@@ -173,7 +173,7 @@ def menu_columns
# Returns how many pages per page should there be when paginating pages
def per_page(dialog = false)
- dialog ? Pages.pages_per_dialog : Pages.config.pages_per_admin_index
+ dialog ? Pages.pages_per_dialog : Pages.pages_per_admin_index
end
def expire_page_caching
@@ -188,16 +188,17 @@ def expire_page_caching
end
end
+ # The canonical page for this particular page.
+ # Consists of:
+ # * The default locale's translated slug
+ def canonical
+ Globalize.with_locale(::Refinery::I18n.default_frontend_locale){ url }
+ end
+
# Returns in cascading order: custom_slug or menu_title or title depending on
# which attribute is first found to be present for this page.
def custom_slug_or_title
- if custom_slug.present?
- custom_slug
- elsif menu_title.present?
- menu_title
- else
- title
- end
+ custom_slug.presence || menu_title.presence || title
end
# Am I allowed to delete this page?
@@ -291,7 +292,7 @@ def link_url_localised?
# For example, this might evaluate to /about for the "About" page.
def url_marketable
# :id => nil is important to prevent any other params[:id] from interfering with this route.
- url_normal.merge(:path => nested_url, :id => nil)
+ url_normal.merge :path => nested_url, :id => nil
end
# Returns a url suitable to be used in url_for in Rails (such as link_to).
@@ -302,26 +303,23 @@ def url_normal
# If the current locale is set to something other than the default locale
# then the :locale attribute will be set on the url hash, otherwise it won't be.
- def with_locale_param(url_hash)
- if self.class.different_frontend_locale?
- url_hash.update(:locale => ::Refinery::I18n.current_frontend_locale)
- end
+ def with_locale_param(url_hash, locale = nil)
+ locale ||= ::Refinery::I18n.current_frontend_locale if self.class.different_frontend_locale?
+ url_hash.update :locale => locale if locale
url_hash
end
+ def uncached_nested_url
+ [parent.try(:uncached_nested_url), to_param.to_s].compact.flatten
+ end
+
# Returns an array with all ancestors to_param, allow with its own
# Ex: with an About page and a Mission underneath,
# ::Refinery::Page.find('mission').nested_url would return:
#
# ['about', 'mission']
#
- def nested_url
- Rails.cache.fetch(url_cache_key) { uncached_nested_url }
- end
-
- def uncached_nested_url
- [parent.try(:nested_url), to_param.to_s].compact.flatten
- end
+ alias_method :nested_url, :uncached_nested_url
# Returns the string version of nested_url, i.e., the path that should be generated
# by the router
@@ -11,27 +11,25 @@
<% if Refinery.i18n_enabled? and Refinery::I18n.frontend_locales.many? %>
<span class='preview'>
<% page.translations.each do |translation| %>
- <% if translation.title.present? %>
- <%= link_to refinery_icon_tag("flags/#{translation.locale}.png", :size => '16x11'),
- refinery.edit_admin_page_path(page, :switch_locale => translation.locale),
- :class => 'locale' %>
- <% end %>
+ <%= link_to refinery_icon_tag("flags/#{translation.locale}.png", :size => '16x11'),
+ refinery.edit_admin_page_path(page, :switch_locale => translation.locale),
+ :class => 'locale' if translation.title.present? %>
<% end %>
</span>
<% end %>
</span>
<span class='actions'>
<%= link_to refinery_icon_tag('application_go.png'),
- refinery.page_path(page),
+ refinery.page_path(page.uncached_nested_url),
:target => "_blank",
:title => t('.view_live_html') %>
<%= link_to refinery_icon_tag('page_add.png'),
refinery.new_admin_page_path(:parent_id => page.id),
:title => t('new', :scope => 'refinery.admin.pages') %>
<%= link_to refinery_icon_tag('application_edit.png'),
- refinery.edit_admin_page_path(page.id),
+ refinery.edit_admin_page_path(page.uncached_nested_url),
:title => t('edit', :scope => 'refinery.admin.pages') %>
<%= link_to refinery_icon_tag('delete.png'),
@@ -1,3 +1,4 @@
+# encoding: utf-8
require 'spec_helper'
module Refinery
@@ -103,6 +104,41 @@ def turn_on_marketable_urls
end
end
+ describe '#canonical' do
+ before do
+ ::Refinery::I18n.stub(:default_frontend_locale).and_return(:en)
+ ::Refinery::I18n.stub(:frontend_locales).and_return([Refinery::I18n.default_frontend_locale, :ru])
+ ::Refinery::I18n.stub(:current_frontend_locale).and_return(Refinery::I18n.default_frontend_locale)
+
+ page.save
+ end
+ let(:page_title) { 'team' }
+ let(:child_title) { 'about' }
+ let(:ru_page_title) { 'Новости' }
+ let!(:default_canonical) {
+ Globalize.with_locale(::Refinery::I18n.default_frontend_locale) {
+ page.canonical
+ }
+ }
+
+ specify 'page returns itself' do
+ page.canonical.should == page.url
+ end
+
+ specify 'default canonical matches page#canonical' do
+ default_canonical.should == page.canonical
+ end
+
+ specify 'translated page returns master page' do
+ Globalize.with_locale(:ru) do
+ page.title = ru_page_title
+ page.save
+
+ page.canonical.should == default_canonical
+ end
+ end
+ end
+
context 'custom slugs' do
let(:custom_page_slug) { 'custom-page-slug' }
let(:custom_child_slug) { 'custom-child-slug' }
@@ -128,7 +128,7 @@ module Admin
page.body.should =~ /Remove this page forever/
page.body.should =~ /Edit this page/
- page.body.should =~ %r{/refinery/pages/#{Page.last.id}/edit}
+ page.body.should =~ %r{/refinery/pages/my-first-page/edit}
page.body.should =~ /Add a new child page/
page.body.should =~ %r{/refinery/pages/new\?parent_id=}
page.body.should =~ /View this page live/
@@ -302,11 +302,11 @@ module Admin
end
end
- it "uses the id for the edit link" do
+ it "shows title in the admin menu" do
p = ::Refinery::Page.by_slug('news').first
within "#page_#{p.id}" do
page.should have_content('News')
- page.find_link('Edit this page')[:href].should include(p.id.to_s)
+ page.find_link('Edit this page')[:href].should include('news')
end
end
@@ -390,11 +390,11 @@ module Admin
end
end
- it "uses the id of the page for the edit link" do
+ it "uses the slug from the default locale in admin" do
visit refinery.admin_pages_path
within "#page_#{news_page.id}" do
- page.find_link('Edit this page')[:href].should include(news_page.id.to_s)
+ page.find_link('Edit this page')[:href].should include(en_page_slug)
end
end

0 comments on commit 4c3202f

Please sign in to comment.