Refactor Refinery::Page#url and friends.

* Add replacement message to deprecated Refinery::Page#url_marketable method.

pages/app/models/refinery/page.rb

@@ -233,55 +233,23 @@ def path(options = {})
       end
     end
 
-    # When this page is rendered in the navigation, where should it link?
-    # If a custom "link_url" is set, it uses that otherwise it defaults to a normal page URL.
-    # The "link_url" is often used to link to a plugin rather than a page.
-    #
-    # For example if I had a "Contact" page I don't want it to just render a contact us page
-    # I want it to show the Inquiries form so I can collect inquiries. So I would set the "link_url"
-    # to "/contact"
     def url
-      if link_url.present?
-        link_url_localised?
-      elsif Refinery::Pages.marketable_urls
-        with_locale_param url_marketable
-      elsif to_param.present?
-        with_locale_param url_normal
-      end
+      Refinery::Pages::Url.build(self)
     end
 
-    # Adds the locale key into the URI for this page's link_url attribute, unless
-    # the current locale is set as the default locale.
     def link_url_localised?
-      current_url = link_url
-
-      if current_url =~ %r{^/} && ::Refinery::I18n.current_frontend_locale != ::Refinery::I18n.default_frontend_locale
-        current_url = "/#{::Refinery::I18n.current_frontend_locale}#{current_url}"
-      end
-
-      current_url
+      Refinery.deprecate "Refinery::Page#link_url_localised?", :when => '2.2', :replacement => "Refinery::Pages::Url::Localised#url"
+      Refinery::Pages::Url::Localised.new(self).url
     end
 
-    # Add 'marketable url' attributes into this page's url.
-    # This sets 'path' as the nested_url value and sets 'id' to nil.
-    # 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
-    end
-
-    # Returns a url suitable to be used in url_for in Rails (such as link_to).
-    # For example, this might evaluate to /pages/about for the "About" page.
     def url_normal
-      {:controller => '/refinery/pages', :action => 'show', :path => nil, :id => to_param, :only_path => true}
+      Refinery.deprecate "Refinery::Page#url_normal", :when => '2.2', :replacement => "Refinery::Pages::Url::Normal#url"
+      Refinery::Pages::Url::Normal.new(self).url
     end
 
-    # 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, locale = nil)
-      locale ||= ::Refinery::I18n.current_frontend_locale if self.class.different_frontend_locale?
-      url_hash.update :locale => locale if locale
-      url_hash
+    def url_marketable
+      Refinery.deprecate "Refinery::Page#url_marketable", :when => '2.2', :replacement => "Refinery::Pages::Url::Marketable#url"
+      Refinery::Pages::Url::Marketable.new(self).url
     end
 
     def uncached_nested_url

pages/lib/refinery/pages/url.rb

@@ -0,0 +1,74 @@
+module Refinery
+  module Pages
+    class Url
+
+      class Localised < Url
+        def self.handle?(page)
+          page.link_url.present?
+        end
+
+        def url
+          current_url = page.link_url
+
+          if current_url =~ %r{^/} &&
+            Refinery::I18n.current_frontend_locale != Refinery::I18n.default_frontend_locale
+            current_url = "/#{Refinery::I18n.current_frontend_locale}#{current_url}"
+          end
+
+          current_url
+        end
+      end
+
+      class Marketable < Url
+        def self.handle?(page)
+          Refinery::Pages.marketable_urls
+        end
+
+        def url
+          url_hash = base_url_hash.merge(:path => page.nested_url, :id => nil)
+          with_locale_param(url_hash)
+        end
+      end
+
+      class Normal < Url
+        def self.handle?(page)
+          page.to_param.present?
+        end
+
+        def url
+          url_hash = base_url_hash.merge(:path => nil, :id => page.to_param)
+          with_locale_param(url_hash)
+        end
+      end
+
+      def self.build(page)
+        klass = [ Localised, Marketable, Normal ].detect { |d| d.handle?(page) } || self
+        klass.new(page).url
+      end
+
+      def initialize(page)
+        @page = page
+      end
+
+      def url
+        raise NotImplementedError
+      end
+
+      private
+
+      attr_reader :page
+
+      def with_locale_param(url_hash)
+        if (locale = Refinery::I18n.current_frontend_locale) != ::I18n.locale
+          url_hash.update :locale => locale if locale
+        end
+        url_hash
+      end
+
+      def base_url_hash
+        { :controller => '/refinery/pages', :action => 'show', :only_path => true }
+      end
+
+    end
+  end
+end

pages/spec/lib/pages/url_spec.rb

@@ -0,0 +1,73 @@
+require "spec_helper"
+
+module Refinery
+  module Pages
+    describe Url::Localised do
+      describe ".handle?" do
+        it "returns true if link_url is present" do
+          page = stub(:page, :link_url => "/")
+          Url::Localised.handle?(page).should be_true
+        end
+      end
+
+      describe "#url" do
+        let(:page) { stub(:page, :link_url => "/test") }
+
+        context "when current frontend locale != default frontend locale" do
+          it "returns link_url prefixed with current frontend locale" do
+            Refinery::I18n.stub(:current_frontend_locale).and_return(:lv)
+            Refinery::I18n.stub(:default_frontend_locale).and_return(:en)
+            Url::Localised.new(page).url.should eq("/lv/test")
+          end
+        end
+
+        context "when current frontend locale == default frontend locale" do
+          it "returns unaltered link_url" do
+            Refinery::I18n.stub(:current_frontend_locale).and_return(:en)
+            Refinery::I18n.stub(:default_frontend_locale).and_return(:en)
+            Url::Localised.new(page).url.should eq("/test")
+          end
+        end
+      end
+    end
+
+    describe Url::Marketable do
+      describe ".handle?" do
+        it "returns true if marketable_url config is set to true" do
+          page = stub(:page)
+          Refinery::Pages.stub(:marketable_url).and_return(true)
+          Url::Marketable.handle?(page).should be_true
+        end
+      end
+
+      describe "#url" do
+        it "returns hash" do
+          page = stub(:page, :nested_url => "test")
+          Url::Marketable.new(page).url.should eq({
+            :controller => "/refinery/pages", :action => "show", :only_path => true,
+            :path => "test", :id => nil
+          })
+        end
+      end
+    end
+
+    describe Url::Normal do
+      describe ".handle?" do
+        it "returns true if to_param is present" do
+          page = stub(:page, :to_param => "test")
+          Url::Normal.handle?(page).should be_true
+        end
+      end
+
+      describe "#url" do
+        it "returns hash" do
+          page = stub(:page, :to_param => "test")
+          Url::Normal.new(page).url.should eq({
+            :controller => "/refinery/pages", :action => "show", :only_path => true,
+            :path => nil, :id => "test"
+          })
+        end
+      end
+    end
+  end
+end