Merge remote-tracking branch 'origin/cherry_picks_from_master' into 2…

…-0-stable

changelog.md

@@ -9,6 +9,7 @@
 * Fixed page reorder issue on Ruby 1.8.x. [#1585](https://github.com/resolve/refinerycms/issues/1585). [Uģis Ozols](https://github.com/ugisozols) & [Philip Arndt](https://github.com/parndt).
 * Allowed to override presenters using `rake refinery:override`. [#1790](https://github.com/resolve/refinerycms/pull/1790). [Kevin Bullock](https://github.com/krbullock).
 * Fixed issue with saving settings in generated form extension by completely rewriting settings controller. [#1817](https://github.com/resolve/refinerycms/issues/1817). [Uģis Ozols](https://github.com/ugisozols)
+* Removed Refinery::Page#title_with_meta in favour of view helpers. [#1847](https://github.com/resolve/refinerycms/pull/1847). [Philip Arndt](https://github.com/parndt)
 
 ## 2.0.5 [11 June 2012]
 * Now extension/form generators will add all attributes to attr_accessible. [#1613](https://github.com/resolve/refinerycms/pull/1613). [Uģis Ozols](https://github.com/ugisozols)

core/app/controllers/refinery/admin/dialogs_controller.rb

@@ -1,26 +1,42 @@
 module ::Refinery
   module Admin
     class DialogsController < ::Refinery::AdminController
+      TYPES = %w[image link]
+
+      before_filter :find_dialog_type, :find_iframe_src, :only => [:show]
+
       def index
         redirect_to refinery.admin_root_path
       end
 
       def show
-        @dialog_type = params[:id].downcase
+        render :layout => false
+      end
+
+      def from_dialog?
+        true
+      end
+
+      protected
+      def find_dialog_type
+        error_404 if TYPES.exclude? params[:id].downcase
 
-        url_params = params.reject {|key, value| key =~ /(action)|(controller)/}
+        @dialog_type = params[:id].downcase
+      end
 
-        @iframe_src = if @dialog_type == 'image'
-          refinery.insert_admin_images_path(url_params.merge(:id => nil, :modal => true))
+      def find_iframe_src
+        if @dialog_type == 'image'
+          @iframe_src = refinery.insert_admin_images_path(
+            url_params.merge(:modal => true)
+          )
         elsif @dialog_type == 'link'
-          refinery.link_to_admin_pages_dialogs_path(url_params.merge(:id => nil))
+          @iframe_src = refinery.link_to_admin_pages_dialogs_path url_params
         end
-
-        render :layout => false
       end
 
-      def from_dialog?
-        true
+      def url_params
+        params.reject {|key, value| /(action)|(controller)/ === key}.
+               merge :id => nil
       end
 
     end

core/spec/requests/refinery/admin/dialogs_spec.rb

@@ -19,9 +19,10 @@ module Refinery
     end
 
     context "a" do
-      it "have empty iframe src" do
+      it "404s" do
+        Admin::DialogsController.any_instance.should_receive(:error_404).once
+
         visit refinery.admin_dialog_path('a')
-        page.should have_selector("iframe[src='']")
       end
     end
   end

pages/app/controllers/refinery/admin/pages_dialogs_controller.rb

@@ -4,6 +4,8 @@ module Refinery
   module Admin
     class PagesDialogsController < ::Refinery::Admin::DialogsController
 
+      helper :'refinery/admin/pages'
+
       def link_to
         # Get the switch_local variable to determine the locale we're currently editing
         # Set up Globalize with our current locale

pages/app/controllers/refinery/pages_controller.rb

@@ -2,7 +2,7 @@ module Refinery
   class PagesController < ::ApplicationController
     before_filter :find_page, :set_canonical, :except => [:preview]
     before_filter :find_page_for_preview, :only => [:preview]
-    
+
     # Save whole Page after delivery
     after_filter { |c| c.write_cache? }
 
@@ -68,13 +68,13 @@ def first_live_child
     def find_page_for_preview
       if page(fallback_to_404 = false)
         # Preview existing pages
-        @page.attributes = params[:page]
+        @page.attributes = view_context.sanitize_hash params[:page]
       elsif params[:page]
         # Preview a non-persisted page
-        @page = Page.new(params[:page])
+        @page = Page.new params[:page]
       end
-    end    
-    
+    end
+
     def find_page(fallback_to_404 = true)
       @page ||= case action_name
                 when "home"

pages/app/helpers/refinery/admin/pages_helper.rb

@@ -22,6 +22,31 @@ def template_options(template_type, current_page)
           { :selected => Refinery::Pages.send("#{template_type}_whitelist").first }
         end
       end
+
+      # In the admin area we use a slightly different title
+      # to inform the which pages are draft or hidden pages
+      def page_meta_information(page)
+        meta_information = ActiveSupport::SafeBuffer.new
+        meta_information << content_tag(:span, :class => 'label') do
+          ::I18n.t('hidden', :scope => 'refinery.admin.pages.page')
+        end unless page.show_in_menu?
+
+        meta_information << content_tag(:span, :class => 'label notice') do
+          ::I18n.t('draft', :scope => 'refinery.admin.pages.page')
+        end if page.draft?
+
+        meta_information.html_safe
+      end
+
+      # We show the title from the next available locale
+      # if there is no title for the current locale
+      def page_title_with_translations(page)
+        if page.title.present?
+          page.title
+        else
+          page.translations.detect {|t| t.title.present?}.title
+        end
+      end
     end
   end
 end

pages/app/helpers/refinery/pages_helper.rb

@@ -1,4 +1,20 @@
 module Refinery
   module PagesHelper
+
+    def sanitize_hash(input_hash)
+      sanitized_hash = HashWithIndifferentAccess.new
+      input_hash.each do |key, value|
+        # ActionPack's sanitize calls html_safe on sanitized input.
+        sanitized_hash[key] = if value.respond_to? :html_safe
+          sanitize value
+        elsif value.respond_to? :each
+          sanitize_hash value
+        else
+          value
+        end
+      end
+      sanitized_hash
+    end
+
   end
 end

pages/app/models/refinery/page.rb

@@ -420,21 +420,6 @@ def part_with_title(part_title)
       end
     end
 
-    # In the admin area we use a slightly different title to inform the which pages are draft or hidden pages
-    # We show the title from the next available locale if there is no title for the current locale
-    def title_with_meta
-      if self.title.present?
-        title = [self.title]
-      else
-        title = [self.translations.detect {|t| t.title.present?}.title]
-      end
-
-      title << "<em>(#{::I18n.t('hidden', :scope => 'refinery.admin.pages.page')})</em>" unless show_in_menu?
-      title << "<em>(#{::I18n.t('draft', :scope => 'refinery.admin.pages.page')})</em>" if draft?
-
-      title.join(' ')
-    end
-
     # Used to index all the content on this page so it can be easily searched.
     def all_page_part_content
       parts.map(&:body).join(" ")

pages/app/views/refinery/admin/pages/_page.html.erb

@@ -7,7 +7,8 @@
     <% end %>
 
     <span class='title <%= 'toggle' if page.children.present? %>'>
-      <%= page.title_with_meta.html_safe %>
+      <%= page_title_with_translations page %>
+      <%= page_meta_information page %>
     </span>
     <% if Refinery.i18n_enabled? and Refinery::I18n.frontend_locales.many? %>
       <span class='locales'>
@@ -37,7 +38,10 @@
                   :class => "cancel confirm-delete",
                   :title => t('delete', :scope => 'refinery.admin.pages'),
                   :data => {
-                    :confirm => t('message', :scope => 'refinery.admin.delete', :title => page.title_with_meta.gsub(/\ ?<em>.*<\/em>/, ""))
+                    :confirm => t('message',
+                      :scope => 'refinery.admin.delete',
+                      :title => page_title_with_translations(page)
+                    )
                   },
                   :method => :delete if page.deletable? %>
     </span>

pages/app/views/refinery/admin/pages_dialogs/_page_link.html.erb

@@ -6,11 +6,14 @@
   page_link_url = "#{[request.protocol, request.host_with_port].join}#{page_link_url}" if Refinery::Pages.absolute_page_links
 -%>
 <li class='clearfix<%= " child#{child}" if child %><%= " linked" if linked%>' id="<%= dom_id(page_link) -%>">
-  <%= link_to page_link.title_with_meta.html_safe, page_link_url, {
+  <%= link_to page_link_url, {
                 :title => t('.link_to_this_page'),
                 :rel => page_link.title,
                 :class => 'page_link'
-              }.merge(link_args) %>
+              }.merge(link_args) do %>
+    <%= page_title_with_translations page_link %>
+    <%= page_meta_information page_link %>
+  <% end %>
 </li>
 <%= render :partial => 'page_link',
            :collection => page_link.children,

pages/spec/helpers/refinery/pages/admin/pages_helper_spec.rb

@@ -49,6 +49,55 @@ module Admin
           end
         end
       end
+
+      describe "#page_meta_information" do
+        let(:page) { FactoryGirl.build(:page) }
+
+        context "when show_in_menu is false" do
+          it "adds 'hidden' label" do
+            page.show_in_menu = false
+
+            helper.page_meta_information(page).should eq("<span class=\"label\">hidden</span>")
+          end
+        end
+
+        context "when draft is true" do
+          it "adds 'draft' label" do
+            page.draft = true
+
+            helper.page_meta_information(page).should eq("<span class=\"label notice\">draft</span>")
+          end
+        end
+      end
+
+      describe "#page_title_with_translations" do
+        let(:page) { FactoryGirl.build(:page) }
+
+        before do
+          Globalize.with_locale(:en) do
+            page.title = "draft"
+            page.save!
+          end
+
+          Globalize.with_locale(:lv) do
+            page.title = "melnraksts"
+            page.save!
+          end
+        end
+
+        context "when title is present" do
+          it "returns it" do
+            helper.page_title_with_translations(page).should eq("draft")
+          end
+        end
+
+        context "when title for current locale isn't available" do
+          it "returns existing title from translations" do
+            Refinery::Page::Translation.where(:locale => :en).first.delete
+            helper.page_title_with_translations(page).should eq("melnraksts")
+          end
+        end
+      end
     end
   end
 end

pages/spec/helpers/refinery/pages/pages_helper_spec.rb

@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+module Refinery
+  describe PagesHelper do
+    describe "#sanitize_hash" do
+      context "when hash value responds to html_safe" do
+        it "returns hash with sanitized values" do
+          hash = { :key => "hack<script>" }
+          helper.sanitize_hash(hash).should eq("key" => "hack")
+        end
+      end
+
+      context "when hash value is an array" do
+        it "returns hash with sanitized values" do
+          hash = { :key => { :x => "hack<script>", :y => "malware<script>" } }
+          helper.sanitize_hash(hash).should eq("key"=>{"x"=>"hack", "y"=>"malware"})
+        end
+      end
+
+      context "when string value doesn't repsond to html_safe" do
+        it "returns hash with value untouched" do
+          String.any_instance.stub(:respond_to?).and_return(false)
+
+          hash = { :key => "hack<script>" }
+          helper.sanitize_hash(hash).should eq("key"=>"hack<script>")
+        end
+      end
+    end
+  end
+end