Merge pull request opf#5457 from opf/fix/list_cf_export

[ci skip]

app/controllers/time_entries/reports_controller.rb

@@ -135,27 +135,39 @@ def load_available_criterias
 
     # Add list and boolean custom fields as available criterias
     custom_fields = (@project.nil? ? WorkPackageCustomField.for_all : @project.all_work_package_custom_fields)
-    custom_fields.select { |cf| %w(list bool).include? cf.field_format }.each do |cf|
-      @available_criterias["cf_#{cf.id}"] = { sql: "(SELECT c.value FROM #{CustomValue.table_name} c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'WorkPackage' AND c.customized_id = #{WorkPackage.table_name}.id)",
-                                              format: cf.field_format,
-                                              label: cf.name }
-    end if @project
+
+    if @project
+      custom_fields.select { |cf| %w(list bool).include? cf.field_format }.each do |cf|
+        @available_criterias["cf_#{cf.id}"] = { sql: "(SELECT c.value FROM #{CustomValue.table_name} c
+                                                       WHERE c.custom_field_id = #{cf.id}
+                                                       AND c.customized_type = 'WorkPackage'
+                                                       AND c.customized_id = #{WorkPackage.table_name}.id)",
+                                                format: cf,
+                                                label: cf.name }
+      end
+    end
 
     # Add list and boolean time entry custom fields
     TimeEntryCustomField.all.select { |cf| %w(list bool).include? cf.field_format }.each do |cf|
-      @available_criterias["cf_#{cf.id}"] = { sql: "(SELECT c.value FROM #{CustomValue.table_name} c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'TimeEntry' AND c.customized_id = #{TimeEntry.table_name}.id)",
-                                              format: cf.field_format,
+      @available_criterias["cf_#{cf.id}"] = { sql: "(SELECT c.value FROM #{CustomValue.table_name} c
+                                                     WHERE c.custom_field_id = #{cf.id}
+                                                     AND c.customized_type = 'TimeEntry'
+                                                     AND c.customized_id = #{TimeEntry.table_name}.id)",
+                                              format: cf,
                                               label: cf.name }
     end
 
     # Add list and boolean time entry activity custom fields
     TimeEntryActivityCustomField.all.select { |cf| %w(list bool).include? cf.field_format }.each do |cf|
-      @available_criterias["cf_#{cf.id}"] = { sql: "(SELECT c.value FROM #{CustomValue.table_name} c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'Enumeration' AND c.customized_id = #{TimeEntry.table_name}.activity_id)",
-                                              format: cf.field_format,
+      @available_criterias["cf_#{cf.id}"] = { sql: "(SELECT c.value FROM #{CustomValue.table_name} c
+                                                     WHERE c.custom_field_id = #{cf.id}
+                                                     AND c.customized_type = 'Enumeration'
+                                                     AND c.customized_id = #{TimeEntry.table_name}.activity_id)",
+                                              format: cf,
                                               label: cf.name }
     end
 
-    call_hook(:controller_timelog_available_criterias,  available_criterias: @available_criterias, project: @project)
+    call_hook(:controller_timelog_available_criterias, available_criterias: @available_criterias, project: @project)
     @available_criterias
   end
 

app/helpers/custom_fields_helper.rb

@@ -1,4 +1,5 @@
 #-- encoding: UTF-8
+
 #-- copyright
 # OpenProject is a project management system.
 # Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
@@ -47,7 +48,7 @@ def custom_field_tag(name, custom_value)
     field_name = "#{name}[custom_field_values][#{custom_field.id}]"
     field_id = "#{name}_custom_field_values_#{custom_field.id}"
 
-    field_format = Redmine::CustomFieldFormat.find_by_name(custom_field.field_format)
+    field_format = OpenProject::CustomFieldFormat.find_by_name(custom_field.field_format)
 
     tag = case field_format.try(:edit_as)
           when 'date'
@@ -114,7 +115,7 @@ def custom_field_tag_with_label(name, custom_value)
   def custom_field_tag_for_bulk_edit(name, custom_field, project=nil)
     field_name = "#{name}[custom_field_values][#{custom_field.id}]"
     field_id = "#{name}_custom_field_values_#{custom_field.id}"
-    field_format = Redmine::CustomFieldFormat.find_by_name(custom_field.field_format)
+    field_format = OpenProject::CustomFieldFormat.find_by_name(custom_field.field_format)
     case field_format.try(:edit_as)
     when 'date'
       styled_text_field_tag(field_name, '', id: field_id, size: 10) +
@@ -135,27 +136,32 @@ def custom_field_tag_for_bulk_edit(name, custom_field, project=nil)
   # Return a string used to display a custom value
   def show_value(custom_value)
     return '' unless custom_value
-    format_value(custom_value.value, custom_value.custom_field.field_format)
+    custom_value.formatted_value
   end
 
   # Return a string used to display a custom value
-  def format_value(value, field_format)
-    Redmine::CustomFieldFormat.format_value(value, field_format) # Proxy
+  def format_value(value, custom_field)
+    custom_value = CustomValue.new(custom_field: custom_field,
+                                   value: value)
+
+    custom_value.formatted_value
   end
 
   # Return an array of custom field formats which can be used in select_tag
   def custom_field_formats_for_select(custom_field)
-    Redmine::CustomFieldFormat.as_select(custom_field.class.customized_class.name)
+    OpenProject::CustomFieldFormat
+      .all_for_field(custom_field)
+      .sort_by(&:order)
+      .map do |custom_field_format|
+        [label_for_custom_field_format(custom_field_format.name), custom_field_format.name]
+      end
   end
 
-  # Renders the custom_values in api views
-  def render_api_custom_values(custom_values, api)
-    api.array :custom_fields do
-      custom_values.each do |custom_value|
-        api.custom_field id: custom_value.custom_field_id, name: custom_value.custom_field.name do
-          api.value custom_value.value
-        end
-      end
-    end unless custom_values.empty?
+  def label_for_custom_field_format(format_string)
+    format = OpenProject::CustomFieldFormat.find_by_name(format_string)
+
+    if format
+      format.label.is_a?(Proc) ? format.label.call : I18n.t(format.label)
+    end
   end
 end

app/models/custom_field.rb

@@ -49,7 +49,7 @@ def uniqueness_of_name_with_scope
     errors.add(:name, :taken) if name.in?(taken_names)
   end
 
-  validates_inclusion_of :field_format, in: Redmine::CustomFieldFormat.available_formats
+  validates_inclusion_of :field_format, in: OpenProject::CustomFieldFormat.available_formats
 
   validate :validate_default_value
 
@@ -96,6 +96,10 @@ def validate_default_value
     end
   end
 
+  def required?
+    is_required?
+  end
+
   def possible_values_options(obj = nil)
     case field_format
     when 'user', 'version'

app/models/custom_value.rb

@@ -1,4 +1,5 @@
 #-- encoding: UTF-8
+
 #-- copyright
 # OpenProject is a project management system.
 # Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
@@ -28,18 +29,6 @@
 #++
 
 class CustomValue < ActiveRecord::Base
-  FORMAT_STRATEGIES = {
-    'string' => CustomValue::StringStrategy,
-    'text' => CustomValue::StringStrategy,
-    'int' => CustomValue::IntStrategy,
-    'float' => CustomValue::FloatStrategy,
-    'date' => CustomValue::DateStrategy,
-    'bool' => CustomValue::BoolStrategy,
-    'user' => CustomValue::UserStrategy,
-    'version' => CustomValue::VersionStrategy,
-    'list' => CustomValue::ListStrategy
-  }.freeze
-
   belongs_to :custom_field
   belongs_to :customized, polymorphic: true
 
@@ -48,23 +37,16 @@ class CustomValue < ActiveRecord::Base
   validate :validate_type_of_value
   validate :validate_length_of_value
 
-  # returns the value of this custom value, but converts it according to the field_format
-  # of the custom field beforehand
-  def typed_value
-    strategy.typed_value
-  end
-
-  def editable?
-    custom_field.editable?
-  end
-
-  def visible?
-    custom_field.visible?
-  end
+  delegate :typed_value,
+           :formatted_value,
+           to: :strategy
 
-  def required?
-    custom_field.is_required?
-  end
+  delegate :editable?,
+           :visible?,
+           :required?,
+           :max_length,
+           :min_length,
+           to: :custom_field
 
   def to_s
     value.to_s
@@ -79,7 +61,7 @@ def value=(val)
   protected
 
   def validate_presence_of_required_value
-    errors.add(:value, :blank) if custom_field.is_required? && !strategy.value_present?
+    errors.add(:value, :blank) if custom_field.required? && !strategy.value_present?
   end
 
   def validate_format_of_value
@@ -98,15 +80,23 @@ def validate_type_of_value
   end
 
   def validate_length_of_value
-    if value.present? && custom_field.min_length.present? && custom_field.max_length.present?
-      errors.add(:value, :too_short, count: custom_field.min_length) if custom_field.min_length > 0 and value.length < custom_field.min_length
-      errors.add(:value, :too_long, count: custom_field.max_length) if custom_field.max_length > 0 and value.length > custom_field.max_length
+    if value.present? && (min_length.present? || max_length.present?)
+      validate_min_length_of_value
+      validate_max_length_of_value
     end
   end
 
   private
 
+  def validate_min_length_of_value
+    errors.add(:value, :too_short, count: min_length) if min_length > 0 && value.length < min_length
+  end
+
+  def validate_max_length_of_value
+    errors.add(:value, :too_long, count: max_length) if max_length > 0 && value.length > max_length
+  end
+
   def strategy
-    @strategy ||= FORMAT_STRATEGIES[custom_field.field_format].new(self)
+    @strategy ||= OpenProject::CustomFieldFormat.find_by_name(custom_field.field_format).formatter.new(self)
   end
 end

app/models/custom_value/ar_object_strategy.rb

@@ -39,6 +39,10 @@ def typed_value
     end
   end
 
+  def formatted_value
+    typed_value.to_s
+  end
+
   def parse_value(val)
     if val.is_a?(ar_class)
       self.memoized_typed_value = val

app/models/custom_value/bool_strategy.rb

@@ -1,4 +1,5 @@
 #-- encoding: UTF-8
+
 #-- copyright
 # OpenProject is a project management system.
 # Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
@@ -41,6 +42,11 @@ def typed_value
     ActiveRecord::Type::Boolean.new.cast(value)
   end
 
+  def formatted_value
+    is_true = ActiveRecord::Type::Boolean.new.cast(value)
+    I18n.t(is_true ? :general_text_Yes : :general_text_No)
+  end
+
   def parse_value(val)
     parsed_val = if !present?(val)
                    nil
@@ -53,8 +59,7 @@ def parse_value(val)
     super(parsed_val)
   end
 
-  def validate_type_of_value
-  end
+  def validate_type_of_value; end
 
   private
 

app/models/custom_value/date_strategy.rb

@@ -1,4 +1,5 @@
 #-- encoding: UTF-8
+
 #-- copyright
 # OpenProject is a project management system.
 # Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
@@ -34,6 +35,12 @@ def typed_value
     end
   end
 
+  def formatted_value
+    format_date(value.to_date)
+  rescue
+    value.to_s
+  end
+
   def validate_type_of_value
     return nil if value.is_a? Date
 

app/models/custom_value/float_strategy.rb

@@ -1,4 +1,5 @@
 #-- encoding: UTF-8
+
 #-- copyright
 # OpenProject is a project management system.
 # Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
@@ -28,12 +29,18 @@
 #++
 
 class CustomValue::FloatStrategy < CustomValue::FormatStrategy
+  include ActionView::Helpers::NumberHelper
+
   def typed_value
     unless value.blank?
       value.to_f
     end
   end
 
+  def formatted_value
+    number_with_delimiter(value.to_s)
+  end
+
   def validate_type_of_value
     Kernel.Float(value)
     nil

app/models/custom_value/format_strategy.rb

@@ -1,4 +1,5 @@
 #-- encoding: UTF-8
+
 #-- copyright
 # OpenProject is a project management system.
 # Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
@@ -45,6 +46,12 @@ def typed_value
     raise 'SubclassResponsibility'
   end
 
+  # Returns the value of the CustomValue formatted to a string
+  # representation.
+  def formatted_value
+    value.to_s
+  end
+
   # Parses the value to
   # 1) have a unified representation for different inputs
   # 2) memoize typed values (if the subclass descides to do so

app/models/work_package/pdf_export/work_package_list_to_pdf.rb

@@ -171,11 +171,11 @@ def make_field_value(work_package, column_name)
   end
 
   def make_custom_field_value(work_package, column)
-    value = work_package
-      .custom_values
-      .detect { |v| v.custom_field_id == column.custom_field.id }
+    values = work_package
+             .custom_values
+             .select { |v| v.custom_field_id == column.custom_field.id }
 
-    pdf.make_cell show_value(value),
+    pdf.make_cell values.map(&:formatted_value).join(', '),
                   padding: cell_padding
   end
 end

app/views/custom_fields/_tab.html.erb

@@ -107,7 +107,7 @@ See doc/COPYRIGHT.rdoc for more details.
           <% (@custom_fields_by_type[tab[:name]] || []).sort.each do |custom_field| -%>
             <tr>
               <td><%= link_to h(custom_field.name), edit_custom_field_path(custom_field), lang: custom_field.name_locale %></td>
-              <td><%= Redmine::CustomFieldFormat.label_for(custom_field.field_format) %></td>
+              <td><%= label_for_custom_field_format(custom_field.field_format) %></td>
               <td><%= checked_image custom_field.is_required? %></td>
               <% if tab[:name] == 'WorkPackageCustomField' %>
                 <td><%= checked_image custom_field.is_for_all? %></td>

app/views/time_entries/reports/_report_criteria.html.erb

@@ -28,6 +28,7 @@ See doc/COPYRIGHT.rdoc for more details.
 ++#%>
 
 <% @hours.collect {|h| h[criterias[level]].to_s}.uniq.each do |value| %>
+
   <% hours_for_value = select_hours(hours, criterias[level], value) -%>
   <% next if hours_for_value.empty? -%>
     <tr class="<%= 'last-level' unless criterias.length > level+1 %>">