Moved ruby code from _content_page template into helper methods (with…

… specs). Also added some comments to the newly introduced classes.

core/app/views/refinery/_content_page.html.erb

@@ -1,21 +1,2 @@
-<%
-  # supported variables for this partial and their default values
-  sections_to_hide = local_assigns[:hide_sections] || []
-  content_page = local_assigns[:sections]
-  allowed_to_use_fallback = !local_assigns[:show_empty_sections] && !local_assigns[:remove_automatic_sections]
-
-  if content_page
-    if !content_page.is_a?(Refinery::Pages::ContentPagePresenter)
-      raise ArgumentError.new("Sections passed to the content page partial must be Refinery::Pages::SectionPresenter objects inside a Refinery::Pages::ContentPagePresenter")
-    end
-  else
-    content_page = Refinery::Pages::ContentPagePresenter.new(@page, page_title)
-  end
-
-  content_page.hide_sections(sections_to_hide)
-  content_page.fetch_template_overrides {|section_id| content_for(section_id)}
--%>
-<section id='body_content' class='<%= content_page.blank_section_css_classes(allowed_to_use_fallback).join(' ') %>'>
-  <%= content_page.to_html(allowed_to_use_fallback) -%>
-</section>
+<%= render_content_page(@page, {:hide_sections => local_assigns[:hide_sections], :can_use_fallback => !local_assigns[:show_empty_sections] && !local_assigns[:remove_automatic_sections]}) %>
 <%= render :partial => '/refinery/draft_page_message' unless @page.nil? or @page.live? -%>

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

@@ -1,6 +1,8 @@
 module Refinery
   module Admin
     class PagesController < Refinery::AdminController
+      helper Pages::ContentPagesHelper
+
       cache_sweeper Refinery::PageSweeper
 
       crudify :'refinery/page',

pages/app/controllers/refinery/pages_controller.rb

@@ -1,5 +1,7 @@
 module Refinery
   class PagesController < ::ApplicationController
+    helper Pages::ContentPagesHelper
+
     before_filter :find_page
 
     # Save whole Page after delivery

pages/app/helpers/refinery/pages/content_pages_helper.rb

@@ -0,0 +1,26 @@
+module ::Refinery
+  module Pages
+    module ContentPagesHelper
+      # Build the html for a Refinery CMS page object by creating a ContentPagePresenter. This is a
+      # specialised type of ContentPresenter, so the object is then passed to render_content_presenter
+      # to get it's html. The options are passed to that method, so see render_content_presenter for
+      # more details.
+      def render_content_page(page, options = {})
+        content_page_presenter = Refinery::Pages::ContentPagePresenter.new(page, page_title)
+        render_content_presenter(content_page_presenter, options)
+      end
+
+      # Pass the options into a ContentPresenter object and return it's html. For more
+      # details see Refinery::Pages::ContentPresenter (and it's subclasses).
+      # This method also checks for template overrides. Any template rendered by the
+      # current action may specify content_for a section using the section's id. For this
+      # reason, sections should not have an ID which you would normally be using for content_for,
+      # so avoid common layout names such as :header, :footer, etc.
+      def render_content_presenter(content_page, options = {})
+        content_page.hide_sections(options[:hide_sections]) if options[:hide_sections]
+        content_page.fetch_template_overrides {|section_id| content_for(section_id)}
+        content_page.to_html(options[:can_use_fallback])
+      end
+    end
+  end
+end

pages/lib/refinery/pages/content_page_presenter.rb

@@ -1,5 +1,9 @@
 module Refinery
   module Pages
+    # A type of ContentPresenter which specifically knows how to render the html for a Refinery CMS
+    # page object. Pass the page object into the constructor, and it will build sections from the
+    # pages parts. The page is not stored internally, so if the page changes, you need to rebuild
+    # this ContentPagePresenter
     class ContentPagePresenter < ContentPresenter
       def initialize(page, page_title)
         super()

pages/lib/refinery/pages/content_presenter.rb

@@ -1,16 +1,21 @@
 module Refinery
   module Pages
+    # Knows how to render a set of sections as html. This can be used in any refinery view that is built from
+    # a group of sections. Pass the sections into the constructor or call add_section on the instance,
+    # then render by calling to_html.
     class ContentPresenter
+      include ActionView::Helpers::TagHelper
+
       def initialize(initial_sections = [])
         @sections = initial_sections
       end
 
-      def blank_section_css_classes(allowed_to_use_fallback)
-        @sections.reject {|section| section.has_content?(allowed_to_use_fallback)}.map(&:not_present_css_class)
+      def blank_section_css_classes(can_use_fallback)
+        @sections.reject {|section| section.has_content?(can_use_fallback)}.map(&:not_present_css_class)
       end
 
       def hide_sections(ids_to_hide)
-        @sections.select {|section| ids_to_hide.include?(section.id)}.each(&:hide)
+        @sections.select {|section| ids_to_hide.include?(section.id)}.each(&:hide) unless ids_to_hide.empty?
       end
 
       def fetch_template_overrides
@@ -29,12 +34,16 @@ def get_section(index)
         @sections[index]
       end
 
-      def to_html(allowed_to_use_fallback = true)
-        @sections.map {|section| section.wrapped_html(allowed_to_use_fallback)}.join("\n").html_safe
+      def to_html(can_use_fallback = true)
+        content_tag :section, sections_html(can_use_fallback), :id => 'body_content', :class => blank_section_css_classes(can_use_fallback).join(' ')
       end
 
       private
 
+        def sections_html(can_use_fallback)
+          @sections.map {|section| section.wrapped_html(can_use_fallback)}.join("\n").html_safe
+        end
+
         def add_section_if_missing(options)
           add_section SectionPresenter.new(options) unless has_section?(options[:id])
         end

pages/lib/refinery/pages/page_part_section_presenter.rb

@@ -1,5 +1,7 @@
 module Refinery
   module Pages
+    # A type of SectionPresenter which knows how to render a section which displays
+    # a PagePart model.
     class PagePartSectionPresenter < SectionPresenter
       def initialize(page_part)
         super()

pages/lib/refinery/pages/section_presenter.rb

@@ -1,5 +1,14 @@
 module Refinery
   module Pages
+    # Knows how to build the html for a section. A section is part of the visible html, that has
+    # content wrapped in some particular markup. Construct with the relevant options, and then
+    # call wrapped_html to get the resultant html.
+    #
+    # The content rendered will usually be the value of fallback_html, unless an override_html
+    # is specified. However, on rendering, you can elect not display sections that have no
+    # override_html by passing in false for can_use_fallback.
+    #
+    # Sections may be hidden, in which case they wont display at all.
     class SectionPresenter
       include ActionView::Helpers::TagHelper
 
@@ -17,14 +26,14 @@ def visible?
         !hidden?
       end
 
-      def has_content?(allowed_to_use_fallback)
-        visible? && content_html(allowed_to_use_fallback).present?
+      def has_content?(can_use_fallback)
+        visible? && content_html(can_use_fallback).present?
       end
 
-      def wrapped_html(allowed_to_use_fallback)
+      def wrapped_html(can_use_fallback)
         return if hidden?
 
-        content = content_html(allowed_to_use_fallback)
+        content = content_html(can_use_fallback)
         if content.present?
           wrap_content_in_tag(content)
         end
@@ -40,12 +49,12 @@ def not_present_css_class
 
       protected
 
-        def content_html(allowed_to_use_fallback)
-          override_html.present? ? override_html : html_from_fallback(allowed_to_use_fallback)
+        def content_html(can_use_fallback)
+          override_html.present? ? override_html : html_from_fallback(can_use_fallback)
         end
 
-        def html_from_fallback(allowed_to_use_fallback)
-          fallback_html if fallback_html.present? && allowed_to_use_fallback
+        def html_from_fallback(can_use_fallback)
+          fallback_html if fallback_html.present? && can_use_fallback
         end
 
       private

pages/lib/refinery/pages/title_section_presenter.rb

@@ -1,14 +1,14 @@
 module Refinery
   module Pages
+    # A type of SectionPresenter which knows how to render a section which displays
+    # a title. These are much like normal sections except they are wrapped in
+    # a h1 tag rather than a div.
     class TitleSectionPresenter < SectionPresenter
-
       private
 
         def wrap_content_in_tag(content)
           content_tag(:h1, content, :id => id)
         end
-
-
     end
   end
 end

pages/spec/helpers/refinery/pages/content_pages_helper_spec.rb

@@ -0,0 +1,44 @@
+require 'spec_helper'
+
+module Refinery
+  module Pages
+    describe ContentPagesHelper do
+      let(:content_presenter) { double(ContentPresenter, :hide_sections => nil, :fetch_template_overrides => nil, :to_html => nil) }
+
+      describe "when rendering content presenter" do
+        it "asks to content presenter to hide sections if told to" do
+          content_presenter.should_receive(:hide_sections).with(['foo', 'bar'])
+          render_content_presenter(content_presenter, :hide_sections => ['foo', 'bar'])
+        end
+
+        it "attempts to fetch template overrides declared elsewhere via content_for" do
+          content_presenter.should_receive(:fetch_template_overrides).and_yield(12)
+          self.should_receive(:content_for).with(12)
+          render_content_presenter(content_presenter)
+        end
+
+        it "outputs the html rendered by the content presenter" do
+          content_presenter.should_receive(:to_html).and_return('foobar')
+          render_content_presenter(content_presenter).should == 'foobar'
+        end
+
+        it "passes can_use_fallback option through to html rendering" do
+          content_presenter.should_receive(:to_html).with(true)
+          render_content_presenter(content_presenter, :can_use_fallback => true)
+        end
+      end
+
+      describe "when rendering page" do
+        let(:page) { double(Page) }
+
+        it "builds a content page presenter and returns its html" do
+          self.should_receive(:page_title).and_return('some title')
+          Refinery::Pages::ContentPagePresenter.should_receive(:new).with(page, 'some title').and_return(content_presenter)
+          content_presenter.should_receive(:to_html).and_return('barfoo')
+
+          render_content_page(page).should == 'barfoo'
+        end
+      end
+    end
+  end
+end

pages/spec/lib/pages/content_presenter_spec.rb

@@ -3,8 +3,8 @@
 module Refinery
   module Pages
     describe ContentPresenter do
-      let(:section1) { double(SectionPresenter, :id => 'foo') }
-      let(:section2) { double(SectionPresenter, :id => 'bar') }
+      let(:section1) { double(SectionPresenter, :id => 'foo', :has_content? => true) }
+      let(:section2) { double(SectionPresenter, :id => 'bar', :has_content? => true) }
 
       describe "when building css classes for blank sections" do
         let(:section) { double(SectionPresenter, :not_present_css_class => 'no_section1') }
@@ -67,19 +67,19 @@ module Pages
       end
 
       describe "when rendering as html" do
-        it "is blank if it has no sections" do
+        it "is empty section tag if it has no sections" do
           content = ContentPresenter.new
-          content.to_html.should be_blank
+          content.to_html.should == "<section class=\"\" id=\"body_content\"></section>"
         end
 
-        it "returns sections joined by a newline" do
+        it "returns sections joined by a newline inside section tag" do
           section1.stub(:wrapped_html).and_return('foo')
           section2.stub(:wrapped_html).and_return('bar')
           content = ContentPresenter.new([section1, section2])
-          content.to_html.should == "foo\nbar"
+          content.to_html.should == "<section class=\"\" id=\"body_content\">foo\nbar</section>"
         end
 
-        it "passes allowed_to_use_fallback option on to sections" do
+        it "passes can_use_fallback option on to sections" do
           section1.should_receive(:wrapped_html).with(false).and_return('foo')
           content = ContentPresenter.new([section1])
           content.to_html(false)