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

1 files changed, 31 insertions, 17 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