content_tag_nested, little refactorings

lib/generators/dry_crud/templates/app/assets/stylesheets/sample.scss

@@ -237,7 +237,7 @@ a.brand:hover {
   display: block;
   float: left;
   margin: 0;
-  padding: 8px 12px 8px;
+  padding: 8px 12px 6px;
   text-decoration: none;
 }
 

lib/generators/dry_crud/templates/app/controllers/crud_controller.rb

@@ -29,15 +29,15 @@ class CrudController < ListController
   #   GET /entries/1
   #   GET /entries/1.json
   def show(&block)
-    respond_with(entry,block)
+    customizable_respond_with(entry, block)
   end
 
   # Display a form to create a new entry of this model.
   #   GET /entries/new
   #   GET /entries/new.json
   def new(&block)
     assign_attributes
-    respond_with(entry,block)
+    customizable_respond_with(entry, block)
   end
 
   # Create a new entry of this model from the passed params.

lib/generators/dry_crud/templates/app/controllers/list_controller.rb

@@ -20,7 +20,7 @@ class ListController < ApplicationController
   #   GET /entries.json
   def index(&block)
     @entries = list_entries
-    respond_with(@entries,block)
+    customizable_respond_with(@entries, block)
   end
 
   protected
@@ -44,9 +44,9 @@ def path_args(last)
   end
 
   # Convenience method to respond to various formats with the given object.
-  def respond_with(object,block=nil)
+  def customizable_respond_with(object, custom_block=nil)
     respond_to do |format|
-      block.call(format,object) if block
+      custom_block.call(format, object) if custom_block
       return if performed?
 
       format.html { render_with_callback action_name }

lib/generators/dry_crud/templates/app/helpers/standard_form_builder.rb

@@ -9,15 +9,14 @@ class StandardFormBuilder < ActionView::Helpers::FormBuilder
 
   attr_reader :template
 
-  delegate :association, :column_type, :column_property, :captionize, 
-           :content_tag, :capture, :ta, :add_css_class, :assoc_and_id_attr,:to => :template
+  delegate :association, :column_type, :column_property, :captionize, :ta,
+           :content_tag, :safe_join, :capture, :add_css_class, :assoc_and_id_attr,
+           :to => :template
 
   # Render multiple input fields together with a label for the given attributes.
   def labeled_input_fields(*attrs)
     options = attrs.extract_options!
-    attrs.collect do |a|
-      labeled_input_field(a, options.clone)
-    end.join("\n").html_safe
+    safe_join(attrs) { |a| labeled_input_field(a, options.clone) }
   end
 
   # Render a corresponding input field for the given attribute.
@@ -105,7 +104,7 @@ def datetime_field(attr, html_options = {})
   def belongs_to_field(attr, html_options = {})
     list = association_entries(attr, html_options)
     if list.present?
-      collection_select(attr, list, :id, :to_s, select_options(attr), html_options)
+      collection_select(attr, list, :id, :to_s, html_options[:multiple] ? {} : select_options(attr), html_options)
     else
       ta(:none_available, association(@object, attr))
     end
@@ -118,13 +117,8 @@ def belongs_to_field(attr, html_options = {})
   # define an instance variable with the pluralized name of the association.
   def has_many_field(attr, html_options = {})
     html_options[:multiple] = true
-    add_css_class(html_options,'multiselect')
-    list = association_entries(attr, html_options)
-    if list.present?
-      collection_select(attr,list,:id,:to_s,{},html_options)
-    else
-      ta(:none_available, association(@object, attr))
-    end
+    add_css_class(html_options, 'multiselect')
+    belongs_to_field(attr, html_options)
   end
 
   # Renders a marker if the given attr has to be present.
@@ -182,20 +176,19 @@ def respond_to?(name)
   # Returns true if attr is a non-polymorphic belongs_to association,
   # for which an input field may be automatically rendered.
   def belongs_to_association?(attr, type)
-    if type == :integer || type.nil?
-      assoc = association(@object, attr, :belongs_to)
-      assoc.present? && assoc.options[:polymorphic].nil?
-    else
-      false
-    end
+    association_kind?(attr, type, :belongs_to)
   end
 
   # Returns true if attr is a non-polymorphic has_many or
   # has_and_belongs_to_many association, for which an input field
   # may be automatically rendered.
   def has_many_association?(attr, type)
-    if type.nil?
-      assoc = association(@object, attr, :has_and_belongs_to_many) || association(@object, attr, :has_many)
+    association_kind?(attr, type, :has_and_belongs_to_many, :has_many)
+  end
+
+  def association_kind?(attr, type, *macros)
+    if type == :integer || type.nil?
+      assoc = association(@object, attr, *macros) 
       assoc.present? && assoc.options[:polymorphic].nil?
     else
       false

lib/generators/dry_crud/templates/app/helpers/standard_helper.rb

@@ -35,7 +35,7 @@ def format_attr(obj, attr)
       send(format_attr_method, obj)
     elsif assoc = association(obj, attr, :belongs_to)
       format_assoc(obj, assoc)
-    elsif assoc = (association(obj, attr, :has_and_belongs_to_many) || association(obj, attr, :has_many))
+    elsif assoc = association(obj, attr, :has_many, :has_and_belongs_to_many)
       format_many_assoc(obj, assoc)
     else
       format_type(obj, attr)
@@ -67,7 +67,7 @@ def captionize(text, clazz = nil)
   # Renders a list of attributes with label and value for a given object.
   # Optionally surrounded with a div.
   def render_attrs(obj, *attrs)
-    attrs.collect { |a| labeled_attr(obj, a) }.join("\n").html_safe
+    safe_join(attrs) { |a| labeled_attr(obj, a) }
   end
 
   # Renders the formatted content of the given attribute with a label.
@@ -127,17 +127,24 @@ def cancel_link(object)
   # Renders a simple unordered list, which will
   # simply render all passed items or yield them
   # to your block.
-  def simple_ul(items,ul_options={},&blk)
-    content_tag(:ul,ul_options) do
-      items.collect do |item|
-        content_tag(:li) do
-          block_given? ? (yield item) : item.to_s
-        end
-      end.join("\n").html_safe
+  def simple_list(items,ul_options={},&blk)
+    content_tag_nested(:ul, items, ul_options) do |item|
+      content_tag(:li, block_given? ? yield(item) : f(item))
     end
   end
-
-
+
+  # render a content tag with the collected contents rendered
+  # by &block for each item in collection.
+  def content_tag_nested(tag, collection, options = {}, &block)
+    content_tag(tag, safe_join(collection, &block), options)
+  end
+
+  # Overridden method that takes a block that is executed for each item in array
+  # before appending the results.
+  def safe_join(array, sep = $,, &block)
+    super(block_given? ? array.collect(&block) : array, sep)
+  end
+
   ######## ACTION LINKS ###################################################### :nodoc:
 
   # A generic helper method to create action links.
@@ -260,29 +267,31 @@ def column_property(obj, attr, property)
 
   # Formats an active record belongs_to association
   def format_assoc(obj, assoc)
-    if assoc_val = obj.send(assoc.name)
-      link_to_unless(no_assoc_link?(assoc, assoc_val), assoc_val, assoc_val)
+    if val = obj.send(assoc.name)
+      assoc_link(assoc, val)
     else
       ta(:no_entry, assoc)
     end
   end
 
   # Formats an active record has_and_belongs_to_many or
-  # has_many association
+  # has_many association.
   def format_many_assoc(obj, assoc)
-    assoc_vals = obj.send(assoc.name)
-    if assoc_vals.size == 1
-      assoc_val = assoc_vals.first
-      link_to_unless(no_assoc_link?(assoc, assoc_val), assoc_val, assoc_val)
-    elsif assoc_vals.present?
-      simple_ul(assoc_vals) do |li|
-        link_to_unless(no_assoc_link?(assoc, li), li, li)
-      end
+    values = obj.send(assoc.name)
+    if values.size == 1
+      assoc_link(assoc, values.first)
+    elsif values.present?
+      simple_list(values) { |val| assoc_link(assoc, val) }
     else
       ta(:no_entry, assoc)
     end
   end
 
+  # Renders a link to the given association entry.
+  def assoc_link(assoc, val)
+    link_to_unless(no_assoc_link?(assoc, val), val.to_s, val)
+  end
+
   # Returns true if no link should be created when formatting the given association.
   def no_assoc_link?(assoc, val)
     !respond_to?("#{val.class.model_name.underscore}_path".to_sym)
@@ -293,11 +302,11 @@ def no_assoc_link?(assoc, val)
   # is given, the association must be of this type, otherwise, any association
   # is returned. Returns nil if no association (or not of the given macro) was
   # found.
-  def association(obj, attr, macro = nil)
+  def association(obj, attr, *macros)
     if obj.class.respond_to?(:reflect_on_association)
       name = assoc_and_id_attr(attr).first.to_sym
       assoc = obj.class.reflect_on_association(name)
-      assoc if assoc && (macro.nil? || assoc.macro == macro)
+      assoc if assoc && (macros.blank? || macros.include?(assoc.macro))
     end
   end
 

lib/generators/dry_crud/templates/app/helpers/standard_table_builder.rb

@@ -11,7 +11,7 @@ class StandardTableBuilder
   # Delegate called methods to template.
   # including StandardHelper would lead to problems with indirectly called methods.
   delegate :content_tag, :format_attr, :column_type, :association,
-           :captionize, :add_css_class, :to => :template
+           :captionize, :add_css_class, :content_tag_nested, :to => :template
 
   def initialize(entries, template, options = {})
     @entries = entries
@@ -58,7 +58,7 @@ def to_html
     add_css_class options, 'table'
     content_tag :table, options do
       content_tag(:thead, html_header) + 
-      content_tag(:tbody, safe_join(entries) { |e| html_row(e) })
+      content_tag_nested(:tbody, entries) { |e| html_row(e) }
     end
   end
 
@@ -81,25 +81,17 @@ def attr_header(attr)
   private
 
   def html_header
-    content_tag :tr do
-      safe_join(cols) { |c| c.html_header }
-    end
+    content_tag_nested(:tr, cols) { |c| c.html_header }
   end
 
   def html_row(entry)
-    content_tag :tr do
-      safe_join(cols) { |c| c.html_cell(entry) }
-    end
+    content_tag_nested(:tr, cols) { |c| c.html_cell(entry) }
   end
 
   def entry_class
     entries.first.class
   end
 
-  def safe_join(collection, &block)
-    collection.collect(&block).join.html_safe
-  end
-
   # Helper class to store column information.
   class Col < Struct.new(:header, :html_options, :template, :block) #:nodoc:
 

lib/generators/dry_crud/templates/test/crud_test_model.rb

@@ -58,20 +58,14 @@ def destroy
   end
 
   def show
-    super do |format,object|
-      format.js { render :text => 'show_js' }
-    end
-  end
-
-  def new
-    super do |format,object|
-      format.js { render :text => 'new_js' }
+    super do |format, object|
+      format.html { render :text => 'custom html' } if object.name == 'BBBBB'
     end
   end
 
   def index
-    super do |format,object|
-      format.js { render :text => 'index_js' }
+    super do |format, object|
+      format.js { render :text => 'index js'}
     end
   end
 
@@ -217,7 +211,7 @@ def reset_db
   # Creates 6 dummy entries for the crud_test_models table.
   def create_test_data
     (1..6).inject(nil) {|prev, i| create(i, prev) }
-    (1..6).inject(nil) {|prev, i| create_other(i, [prev,OtherCrudTestModel.first].compact) }
+    (1..6).each {|i| create_other(i) }
   end
 
   # Fixture-style accessor method to get CrudTestModel instances by name
@@ -253,8 +247,9 @@ def create(index, companion)
                           :remarks => "#{c} #{str(index + 1)} #{str(index + 2)}\n" * (index % 3 + 1))
   end
 
-  def create_other(index, others)
+  def create_other(index)
     c = str(index)
+    others = CrudTestModel.all[index..(index+2)]
     OtherCrudTestModel.create!(:name => c, :other_ids => others.collect(&:id), :more_id => others.first.try(:id))
   end
 

lib/generators/dry_crud/templates/test/functional/crud_test_models_controller_test.rb

@@ -36,7 +36,7 @@ def test_index
   def test_index_js
     get :index, test_params(:format => 'js')
     assert_response :success
-    assert_equal 'index_js', @response.body
+    assert_equal 'index js', @response.body
     assert_present assigns(:entries)
   end
 
@@ -125,18 +125,10 @@ def test_new
     assert_equal [:before_render_new, :before_render_form], @controller.called_callbacks
   end
 
-  def test_new_js
-    get :new, test_params(:format => 'js')
+  def test_show_with_custom
+    get :show, test_params(:id => crud_test_models(:BBBBB).id)
     assert_response :success
-    assert_equal 'new_js', @response.body
-    assert assigns(:entry).new_record?
-  end
-
-  def test_show_js
-    get :show, test_params(:id => test_entry.id, :format => 'js')
-    assert_response :success
-    assert_equal 'show_js', @response.body
-    assert_equal test_entry, assigns(:entry)
+    assert_equal 'custom html', @response.body
   end
 
   def test_create

lib/generators/dry_crud/templates/test/unit/helpers/standard_helper_test.rb

@@ -140,7 +140,47 @@ def format_string_size(obj)
     assert_equal "<p>AAAAA BBBBB CCCCC\n<br />AAAAA BBBBB CCCCC\n</p>", format_type(m, :remarks)
     assert format_type(m, :remarks).html_safe?
   end
+
+  test "format belongs to column without content" do
+    m = crud_test_models(:AAAAA)
+    assert_equal t(:'global.associations.no_entry'), format_attr(m, :companion)
+  end
+
+  test "format belongs to column with content" do
+    m = crud_test_models(:BBBBB)
+    assert_equal "AAAAA", format_attr(m, :companion)
+  end
+
+  test "format has_many column with content" do
+    m = crud_test_models(:CCCCC)
+    assert_equal "<ul><li>AAAAA</li><li>BBBBB</li></ul>", format_attr(m, :others)
+  end
+
+  test "content_tag_nested escapes safe correctly" do
+    html = content_tag_nested(:div, ['a', 'b']) { |e| content_tag(:span, e) }
+    assert_equal "<div><span>a</span><span>b</span></div>", html
+  end
 
+  test "content_tag_nested escapes unsafe correctly" do
+    html = content_tag_nested(:div, ['a', 'b']) { |e| "<#{e}>" }
+    assert_equal "<div>&lt;a&gt;&lt;b&gt;</div>", html
+  end
+
+  test "content_tag_nested without block" do
+    html = content_tag_nested(:div, ['a', 'b']) 
+    assert_equal "<div>ab</div>", html
+  end
+
+  test "safe_join without block" do
+    html = safe_join(['<a>', '<b>'.html_safe]) 
+    assert_equal "&lt;a&gt;<b>", html
+  end
+
+  test "safe_join with block" do
+    html = safe_join(['a', 'b']) { |e| content_tag(:span, e) } 
+    assert_equal "<span>a</span><span>b</span>", html
+  end
+
   test "empty table should render message" do
     result = table([]) { }
     assert result.html_safe?