TableBuilder: Make a new type for columns

Instead of making the columns a hash where the types get all confused
and mussed and we have to type check wherever we use the column value,
make the columns into their own real type.

Adds a new `Column` class to the helper module which lets us interact
with table column data in a much more natural way. Here, when mapping
over the column list, instead of getting a key and value which could be
whatever, we get a real `Column` object that we can call methods on to
get the appropriate data.

It's still not perfect, as evidenced by the `#tbody` method, which has
remained largely the same, but at least we're not checking
`attribute.is_a?(Hash)` like I had tried before.

Update the tests to use the new `TableBuilderHelper::Column` syntax for
defining columns.

Refs #3479

2 files changed, 86 insertions, 34 deletions

diff --git a/app/helpers/table_builder_helper.rb b/app/helpers/table_builder_helper.rb
index 954ec5314..e13dcd6c9 100644
--- a/app/helpers/table_builder_helper.rb
+++ b/app/helpers/table_builder_helper.rb
@@ -1,4 +1,27 @@
 module TableBuilderHelper
+  class Column
+    attr_reader :key, :name, :attribute, :sortable
+
+    def initialize(key: nil, name: '', attribute:, sortable: true)
+      if key.nil? && name.empty?
+        raise ColumnMustHaveKeyOrNameError
+      end
+
+      @key = key
+      @name = name
+      @attribute = attribute
+      @sortable = sortable
+    end
+
+    def key_or_name
+      return @key unless @key.nil?
+      return @name unless @name.empty?
+    end
+  end
+
+  class ColumnMustHaveKeyOrNameError < StandardError; end
+
+
   # TODO: rename this after migration from `table_builder`
   def table_builder_2(
     collection,
@@ -35,16 +58,11 @@ module TableBuilderHelper
           hcont << content_tag(:th, checkbox(id_name: '0', value: 'all'))
         end
 
-        columns.map do |key, attribute|
-          sortable = true
-          if attribute.is_a?(Hash) && attribute.has_key?(:sortable)
-            sortable = attribute[:sortable]
-          end
-
+        columns.map do |column|
           hcont << content_tag(:th, build_column_header(
             collection.model,
-            key,
-            sortable,
+            column.key_or_name,
+            column.sortable,
             params[:sort],
             params[:direction]
           ))
@@ -72,19 +90,15 @@ module TableBuilderHelper
             )
           end
 
-          columns.map do |k, attribute|
-            if attribute.is_a?(Hash)
-              attribute = attribute[:attribute]
-            end
-
+          columns.map do |column|
             value =
-              if Proc === attribute
-                attribute.call(item)
+              if Proc === column.attribute
+                column.attribute.call(item)
               else
-                item.try(attribute)
+                item.try(column.attribute)
               end
             # if so this column's contents get transformed into a link to the object
-            if attribute == 'name' || attribute == 'comment'
+            if column.attribute == 'name' || column.attribute == 'comment'
               polymorph_url = polymorphic_url_parts(item)
               bcont << content_tag(:td, link_to(value, polymorph_url), title: 'Voir')
             else
diff --git a/spec/helpers/table_builder_helper_spec.rb b/spec/helpers/table_builder_helper_spec.rb
index 029748dd2..10cd6772b 100644
--- a/spec/helpers/table_builder_helper_spec.rb
+++ b/spec/helpers/table_builder_helper_spec.rb
@@ -101,15 +101,40 @@ describe TableBuilderHelper, type: :helper do
 
       html_str = helper.table_builder_2(
         referentials,
-        { :name => 'name',
-          :status => Proc.new {|w| w.archived? ? ("<div class='td-block'><span class='fa fa-archive'></span><span>Conservé</span></div>").html_safe : ("<div class='td-block'><span class='sb sb-lg sb-preparing'></span><span>En préparation</span></div>").html_safe},
-          :status => Proc.new {|w| ("<div class='td-block'><span class='sb sb-lg sb-preparing'></span><span>En préparation</span></div>").html_safe},
-          :organisation => Proc.new {|w| w.organisation.name},
-          :validity_period => Proc.new {|w| w.validity_period.nil? ? '-' : t('validity_range', debut: l(w.try(:validity_period).try(:begin), format: :short), end: l(w.try(:validity_period).try(:end), format: :short))},
-          :lines => Proc.new {|w| w.lines.count},
-          :created_at => Proc.new {|w| l(w.created_at, format: :short)},
-          :updated_at => Proc.new {|w| l(w.updated_at, format: :short)},
-          :published_at => ''},
+        [
+          TableBuilderHelper::Column.new(
+            key: :name,
+            attribute: 'name'
+          ),
+          TableBuilderHelper::Column.new(
+            key: :status,
+            attribute: Proc.new {|w| w.archived? ? ("<div class='td-block'><span class='fa fa-archive'></span><span>Conservé</span></div>").html_safe : ("<div class='td-block'><span class='sb sb-lg sb-preparing'></span><span>En préparation</span></div>").html_safe}
+          ),
+          TableBuilderHelper::Column.new(
+            key: :organisation,
+            attribute: Proc.new {|w| w.organisation.name}
+          ),
+          TableBuilderHelper::Column.new(
+            key: :validity_period,
+            attribute: Proc.new {|w| w.validity_period.nil? ? '-' : t('validity_range', debut: l(w.try(:validity_period).try(:begin), format: :short), end: l(w.try(:validity_period).try(:end), format: :short))}
+          ),
+          TableBuilderHelper::Column.new(
+            key: :lines,
+            attribute: Proc.new {|w| w.lines.count}
+          ),
+          TableBuilderHelper::Column.new(
+            key: :created_at,
+            attribute: Proc.new {|w| l(w.created_at, format: :short)}
+          ),
+          TableBuilderHelper::Column.new(
+            key: :updated_at,
+            attribute: Proc.new {|w| l(w.updated_at, format: :short)}
+          ),
+          TableBuilderHelper::Column.new(
+            key: :published_at,
+            attribute: ''
+          )
+        ],
         selectable: true,
         links: [:show, :edit, :archive, :unarchive, :delete],
         cls: 'table has-filter has-search'
@@ -190,16 +215,29 @@ describe TableBuilderHelper, type: :helper do
 
       html_str = helper.table_builder_2(
         companies,
-        {
-          'ID Codif' => {
+        [
+          TableBuilderHelper::Column.new(
+            name: 'ID Codif',
             attribute: Proc.new { |n| n.try(:objectid).try(:local_id) },
             sortable: false
-          },
-          :name => 'name',
-          :phone => 'phone',
-          :email => 'email',
-          :url => 'url'
-        },
+          ),
+          TableBuilderHelper::Column.new(
+            key: :name,
+            attribute: 'name'
+          ),
+          TableBuilderHelper::Column.new(
+            key: :phone,
+            attribute: 'phone'
+          ),
+          TableBuilderHelper::Column.new(
+            key: :email,
+            attribute: 'email'
+          ),
+          TableBuilderHelper::Column.new(
+            key: :url,
+            attribute: 'url'
+          ),
+        ],
         links: [:show, :edit, :delete],
         cls: 'table has-search'
       )