Merge pull request #20565 from opf/bug/67907-wrong-cell-format-in-xls-export-do-not-allow-for-calculations

[#67907] Wrong cell format in XLS export do not allow for calculations

app/models/exports/formatters/default.rb

@@ -87,6 +87,10 @@ module Exports
         "0.00"
       end
 
+      def percentage_format
+        "0%"
+      end
+
       protected
 
       # By default, use try as a non-destructive accessor

app/models/projects/exports/formatters/pdf/active.rb

@@ -29,15 +29,17 @@
 #++
 module Projects::Exports
   module Formatters
-    class Active < ::Exports::Formatters::Default
-      def self.apply?(attribute, export_format)
-        export_format == :pdf && attribute.to_sym == :active
-      end
+    module PDF
+      class Active < ::Exports::Formatters::Default
+        def self.apply?(attribute, export_format)
+          export_format == :pdf && attribute.to_sym == :active
+        end
 
-      ##
-      # Takes a project and returns yes/no depending on the active attribute
-      def format(project, **)
-        project.active? ? I18n.t(:general_text_Yes) : I18n.t(:general_text_No)
+        ##
+        # Takes a project and returns yes/no depending on the active attribute
+        def format(project, **)
+          project.active? ? I18n.t(:general_text_Yes) : I18n.t(:general_text_No)
+        end
       end
     end
   end

app/models/projects/exports/formatters/pdf/favorited.rb

@@ -29,15 +29,17 @@
 #++
 module Projects::Exports
   module Formatters
-    class Favorited < ::Exports::Formatters::Default
-      def self.apply?(attribute, export_format)
-        export_format == :pdf && attribute.to_sym == :favorited
-      end
+    module PDF
+      class Favorited < ::Exports::Formatters::Default
+        def self.apply?(attribute, export_format)
+          export_format == :pdf && attribute.to_sym == :favorited
+        end
 
-      ##
-      # Takes a project and returns yes/no depending on the favorited attribute
-      def format(project, **)
-        project.favorited_by?(User.current) ? I18n.t(:general_text_Yes) : I18n.t(:general_text_No)
+        ##
+        # Takes a project and returns yes/no depending on the favorited attribute
+        def format(project, **)
+          project.favorited_by?(User.current) ? I18n.t(:general_text_Yes) : I18n.t(:general_text_No)
+        end
       end
     end
   end

app/models/projects/exports/formatters/pdf/public.rb

@@ -29,15 +29,17 @@
 #++
 module Projects::Exports
   module Formatters
-    class Public < ::Exports::Formatters::Default
-      def self.apply?(attribute, export_format)
-        export_format == :pdf && attribute.to_sym == :public
-      end
+    module PDF
+      class Public < ::Exports::Formatters::Default
+        def self.apply?(attribute, export_format)
+          export_format == :pdf && attribute.to_sym == :public
+        end
 
-      ##
-      # Takes a project and returns yes/no depending on the public attribute
-      def format(project, **)
-        project.public? ? I18n.t(:general_text_Yes) : I18n.t(:general_text_No)
+        ##
+        # Takes a project and returns yes/no depending on the public attribute
+        def format(project, **)
+          project.public? ? I18n.t(:general_text_Yes) : I18n.t(:general_text_No)
+        end
       end
     end
   end

app/models/projects/exports/formatters/pdf/required_disk_space.rb

@@ -29,17 +29,19 @@
 #++
 module Projects::Exports
   module Formatters
-    class RequiredDiskSpace < ::Exports::Formatters::Default
-      def self.apply?(attribute, export_format)
-        export_format == :pdf && attribute.to_sym == :required_disk_space
-      end
+    module PDF
+      class RequiredDiskSpace < ::Exports::Formatters::Default
+        def self.apply?(attribute, export_format)
+          export_format == :pdf && attribute.to_sym == :required_disk_space
+        end
 
-      ##
-      # Takes a project and returns the formatted value if the required disk space is greater than 0.
-      def format(project, **)
-        return "" unless project.required_disk_space.to_i > 0
+        ##
+        # Takes a project and returns the formatted value if the required disk space is greater than 0.
+        def format(project, **)
+          return "" unless project.required_disk_space.to_i > 0
 
-        number_to_human_size(project.required_disk_space, precision: 2)
+          number_to_human_size(project.required_disk_space, precision: 2)
+        end
       end
     end
   end

app/models/work_package/exports/formatters/pdf/compound_done_ratio.rb

@@ -29,21 +29,23 @@
 #++
 module WorkPackage::Exports
   module Formatters
-    class CompoundDoneRatio < ::Exports::Formatters::Default
-      def self.apply?(name, export_format)
-        name.to_sym == :done_ratio && export_format == :pdf
-      end
+    module PDF
+      class CompoundDoneRatio < ::Exports::Formatters::Default
+        def self.apply?(name, export_format)
+          name.to_sym == :done_ratio && export_format == :pdf
+        end
 
-      def format(work_package, **)
-        derived_done_ratio = work_package.derived_done_ratio
-        derived = derived_done_ratio&.positive? ? " · Σ #{format_value(derived_done_ratio)}" : ""
-        "#{format_value(work_package.done_ratio)}#{derived}"
-      end
+        def format(work_package, **)
+          derived_done_ratio = work_package.derived_done_ratio
+          derived = derived_done_ratio&.positive? ? " · Σ #{format_value(derived_done_ratio)}" : ""
+          "#{format_value(work_package.done_ratio)}#{derived}"
+        end
 
-      def format_value(value, _options = {})
-        return "" if value.nil?
+        def format_value(value, _options = {})
+          return "" if value.nil?
 
-        "#{value}%"
+          "#{value}%"
+        end
       end
     end
   end

app/models/work_package/exports/formatters/pdf/compound_hours.rb

@@ -29,26 +29,28 @@
 #++
 module WorkPackage::Exports
   module Formatters
-    class CompoundHours < ::Exports::Formatters::Default
-      def self.apply?(name, export_format)
-        name.to_sym.in?(%i[estimated_hours remaining_hours]) && export_format == :pdf
-      end
+    module PDF
+      class CompoundHours < ::Exports::Formatters::Default
+        def self.apply?(name, export_format)
+          name.to_sym.in?(%i[estimated_hours remaining_hours]) && export_format == :pdf
+        end
 
-      def format(work_package, **)
-        hours = format_value(work_package.public_send(attribute))
-        derived_hours = total_prefix(format_value(work_package.public_send(:"derived_#{attribute}")))
+        def format(work_package, **)
+          hours = format_value(work_package.public_send(attribute))
+          derived_hours = total_prefix(format_value(work_package.public_send(:"derived_#{attribute}")))
 
-        [hours, derived_hours].compact.join(" ").presence
-      end
+          [hours, derived_hours].compact.join(" ").presence
+        end
 
-      def format_value(value, _options = nil)
-        DurationConverter.output(value)
-      end
+        def format_value(value, _options = nil)
+          DurationConverter.output(value)
+        end
 
-      private
+        private
 
-      def total_prefix(value)
-        value && "· Σ #{value}"
+        def total_prefix(value)
+          value && "· Σ #{value}"
+        end
       end
     end
   end

app/models/work_package/exports/formatters/pdf/currency.rb

@@ -29,13 +29,15 @@
 #++
 module WorkPackage::Exports
   module Formatters
-    class Currency < ::Exports::Formatters::Default
-      def self.apply?(name, export_format)
-        %i[material_costs labor_costs overall_costs].include?(name.to_sym) && export_format == :pdf
-      end
+    module PDF
+      class Currency < ::Exports::Formatters::Default
+        def self.apply?(name, export_format)
+          %i[material_costs labor_costs overall_costs].include?(name.to_sym) && export_format == :pdf
+        end
 
-      def format_value(value, _options)
-        value.nil? || value.zero? ? "" : number_to_currency(value)
+        def format_value(value, _options)
+          value.nil? || value.zero? ? "" : number_to_currency(value)
+        end
       end
     end
   end

app/models/work_package/exports/formatters/pdf/date.rb

@@ -29,15 +29,17 @@
 #++
 module WorkPackage::Exports
   module Formatters
-    class Date < ::Exports::Formatters::Default
-      include WorkPackagesHelper
+    module PDF
+      class Date < ::Exports::Formatters::Default
+        include WorkPackagesHelper
 
-      def self.apply?(name, export_format)
-        name.to_sym == :date && export_format == :pdf
-      end
+        def self.apply?(name, export_format)
+          name.to_sym == :date && export_format == :pdf
+        end
 
-      def format(work_package, **)
-        work_package_formatted_dates(work_package)
+        def format(work_package, **)
+          work_package_formatted_dates(work_package)
+        end
       end
     end
   end

app/models/work_package/exports/formatters/pdf/days.rb

@@ -29,19 +29,21 @@
 #++
 module WorkPackage::Exports
   module Formatters
-    class Days < ::Exports::Formatters::Default
-      def self.apply?(name, export_format)
-        name.to_sym == :duration && export_format == :pdf
-      end
+    module PDF
+      class Days < ::Exports::Formatters::Default
+        def self.apply?(name, export_format)
+          name.to_sym == :duration && export_format == :pdf
+        end
 
-      def format_value(value, _options)
-        formatted_days(value)
-      end
+        def format_value(value, _options)
+          formatted_days(value)
+        end
 
-      private
+        private
 
-      def formatted_days(value)
-        value.nil? ? "" : "#{value} #{I18n.t('export.units.days')}"
+        def formatted_days(value)
+          value.nil? ? "" : "#{value} #{I18n.t('export.units.days')}"
+        end
       end
     end
   end

app/models/work_package/exports/formatters/pdf/hours.rb

@@ -29,13 +29,15 @@
 #++
 module WorkPackage::Exports
   module Formatters
-    class Hours < ::Exports::Formatters::Default
-      def self.apply?(name, export_format)
-        name.to_sym == :spent_hours && export_format == :pdf
-      end
+    module PDF
+      class Hours < ::Exports::Formatters::Default
+        def self.apply?(name, export_format)
+          name.to_sym == :spent_hours && export_format == :pdf
+        end
 
-      def format_value(value, _options)
-        DurationConverter.output(value)
+        def format_value(value, _options)
+          DurationConverter.output(value)
+        end
       end
     end
   end

app/models/work_package/exports/formatters/xls/costs.rb

@@ -29,22 +29,24 @@
 #++
 module WorkPackage::Exports
   module Formatters
-    class Costs < ::Exports::Formatters::Default
-      def self.apply?(name, export_format)
-        %i[material_costs labor_costs overall_costs].include?(name.to_sym) && export_format == :csv
-      end
+    module XLS
+      class Costs < ::Exports::Formatters::Default
+        def self.apply?(name, export_format)
+          %i[material_costs labor_costs overall_costs].include?(name.to_sym) && export_format == :csv
+        end
 
-      def format_options
-        { number_format: number_format_string }
-      end
+        def format_options
+          { number_format: number_format_string }
+        end
 
-      def number_format_string
-        # [$CUR] makes sure we have an actually working currency format with arbitrary currencies
-        curr = "[$CUR]".gsub "CUR", ERB::Util.h(Setting.costs_currency)
-        format = ERB::Util.h Setting.costs_currency_format
-        number = "#,##0.00"
+        def number_format_string
+          # [$CUR] makes sure we have an actually working currency format with arbitrary currencies
+          curr = "[$CUR]".gsub "CUR", ERB::Util.h(Setting.costs_currency)
+          format = ERB::Util.h Setting.costs_currency_format
+          number = "#,##0.00"
 
-        format.gsub("%n", number).gsub("%u", curr)
+          format.gsub("%n", number).gsub("%u", curr)
+        end
       end
     end
   end

app/models/work_package/exports/formatters/xls/done_ratio.rb

@@ -29,15 +29,21 @@
 #++
 module WorkPackage::Exports
   module Formatters
-    class DoneRatio < ::Exports::Formatters::Default
-      def self.apply?(name, export_format)
-        name.to_sym.in?(%i[done_ratio derived_done_ratio]) && export_format != :pdf
-      end
+    module XLS
+      class DoneRatio < ::Exports::Formatters::Default
+        def self.apply?(name, export_format)
+          name.to_sym.in?(%i[done_ratio derived_done_ratio]) && export_format == :csv
+        end
 
-      def format_value(value, _options = {})
-        return "" if value.nil?
+        def format_value(value, _options = {})
+          return if value.nil?
 
-        "#{value}%"
+          (value.to_f / 100).ceil(2)
+        end
+
+        def format_options
+          { number_format: percentage_format }
+        end
       end
     end
   end

app/models/work_package/exports/formatters/xls/hours.rb

@@ -30,13 +30,29 @@
 
 module WorkPackage::Exports
   module Formatters
-    class WorkHours < ::Exports::Formatters::Default
-      def self.apply?(name, export_format)
-        %i[estimated_hours remaining_hours].include?(name.to_sym) && export_format == :csv
-      end
+    module XLS
+      class Hours < ::Exports::Formatters::Default
+        HOUR_FIELDS = %i[
+          estimated_hours
+          derived_estimated_hours
+          remaining_hours
+          derived_remaining_hours
+          spent_hours
+        ].freeze
 
-      def format_options
-        { number_format: }
+        def self.apply?(name, export_format)
+          HOUR_FIELDS.include?(name.to_sym) && export_format == :csv
+        end
+
+        def format_value(value, _options)
+          # Note: Keep the value as a float, without converting it to a string. Otherwise the formatting
+          # decimal formatting will be ignored and the column will be exported as string.
+          value
+        end
+
+        def format_options
+          { number_format: "#{number_format} \"h\"" } # 0.00 "h"
+        end
       end
     end
   end

config/initializers/export_formats.rb

@@ -38,17 +38,17 @@ Rails.application.configure do |application|
 
       formatter WorkPackage, Exports::Formatters::CustomField
       formatter WorkPackage, Exports::Formatters::CustomFieldPdf
-      formatter WorkPackage, WorkPackage::Exports::Formatters::CompoundDoneRatio
-      formatter WorkPackage, WorkPackage::Exports::Formatters::CompoundHours
-      formatter WorkPackage, WorkPackage::Exports::Formatters::Costs
-      formatter WorkPackage, WorkPackage::Exports::Formatters::Currency
-      formatter WorkPackage, WorkPackage::Exports::Formatters::Date
-      formatter WorkPackage, WorkPackage::Exports::Formatters::Days
-      formatter WorkPackage, WorkPackage::Exports::Formatters::DoneRatio
-      formatter WorkPackage, WorkPackage::Exports::Formatters::Hours
+      formatter WorkPackage, WorkPackage::Exports::Formatters::PDF::CompoundDoneRatio
+      formatter WorkPackage, WorkPackage::Exports::Formatters::PDF::CompoundHours
+      formatter WorkPackage, WorkPackage::Exports::Formatters::XLS::Costs
+      formatter WorkPackage, WorkPackage::Exports::Formatters::XLS::Hours
+      formatter WorkPackage, WorkPackage::Exports::Formatters::PDF::Currency
+      formatter WorkPackage, WorkPackage::Exports::Formatters::PDF::Date
+      formatter WorkPackage, WorkPackage::Exports::Formatters::PDF::Days
+      formatter WorkPackage, WorkPackage::Exports::Formatters::XLS::DoneRatio
+      formatter WorkPackage, WorkPackage::Exports::Formatters::PDF::Hours
       formatter WorkPackage, WorkPackage::Exports::Formatters::ProjectPhase
       formatter WorkPackage, WorkPackage::Exports::Formatters::SpentUnits
-      formatter WorkPackage, WorkPackage::Exports::Formatters::WorkHours
 
       list Project, Projects::Exports::CSV
       list Project, Projects::Exports::PDF
@@ -56,10 +56,10 @@ Rails.application.configure do |application|
       formatter Project, Exports::Formatters::CustomFieldPdf
       formatter Project, Projects::Exports::Formatters::Status
       formatter Project, Projects::Exports::Formatters::Description
-      formatter Project, Projects::Exports::Formatters::Public
-      formatter Project, Projects::Exports::Formatters::Active
-      formatter Project, Projects::Exports::Formatters::Favorited
-      formatter Project, Projects::Exports::Formatters::RequiredDiskSpace
+      formatter Project, Projects::Exports::Formatters::PDF::Public
+      formatter Project, Projects::Exports::Formatters::PDF::Active
+      formatter Project, Projects::Exports::Formatters::PDF::Favorited
+      formatter Project, Projects::Exports::Formatters::PDF::RequiredDiskSpace
     end
   end
 end

modules/budgets/app/models/projects/exports/formatters/budget_spent_ratio.rb

@@ -38,7 +38,12 @@ module Projects::Exports::Formatters
       return unless project.module_enabled?("budgets") && User.current.allowed_in_project?(:view_budgets, project)
 
       project_budgets = ::Budgets::Patches::Projects::RowComponentPatch::ProjectBudgets.new(project)
-      number_to_percentage(project_budgets.total_ratio, precision: 0) if project_budgets
+
+      (project_budgets.total_ratio.to_f / 100).ceil(2) if project_budgets&.total_ratio
+    end
+
+    def format_options
+      { number_format: percentage_format }
     end
   end
 end

modules/budgets/spec/models/projects/exports/formatters/budget_spent_ratio_spec.rb

@@ -90,7 +90,7 @@ RSpec.describe Projects::Exports::Formatters::BudgetSpentRatio do
         allow(project_budgets_double).to receive(:total_ratio).and_return(42.7)
         instance = described_class.new(:budget_spent_ratio)
 
-        expect(instance.format(project)).to eq("43%")
+        expect(instance.format(project)).to eq(0.43)
       end
     end
   end

modules/reporting/app/helpers/reporting_helper.rb

@@ -174,7 +174,7 @@ module ReportingHelper
 
     days_between = (end_timestamp.to_date - start_timestamp.to_date).to_i
     if days_between.positive?
-      " (+#{WorkPackage::Exports::Formatters::Days.new(nil)
+      " (+#{WorkPackage::Exports::Formatters::PDF::Days.new(nil)
                                                   .format_value(days_between, nil)
                                                   .delete(' ')})"
     end

modules/xls_export/spec/models/xls_export/work_package/exporter/xls_integration_spec.rb

@@ -212,7 +212,7 @@ RSpec.describe XlsExport::WorkPackage::Exporter::XLS do
 
       # Check row after header row
       hours = sheet.rows[1].values_at(2)
-      expect(hours).to include("27.5")
+      expect(hours).to include(27.5)
     end
 
     context "with duration format being set to 'days and hours'", with_settings: { duration_format: "days_and_hours" } do
@@ -221,7 +221,7 @@ RSpec.describe XlsExport::WorkPackage::Exporter::XLS do
 
         # Check row after header row
         hours = sheet.rows[1].values_at(2)
-        expect(hours).to include("27.5")
+        expect(hours).to include(27.5)
       end
     end
   end
@@ -317,7 +317,7 @@ RSpec.describe XlsExport::WorkPackage::Exporter::XLS do
       headers_row.compact!
       expect(headers_row.zip(values_row)).to eq [
         ["Work", nil],
-        ["Total work", "15.0"],
+        ["Total work", 15.0],
         ["Remaining work", nil],
         ["Total remaining work", nil],
         ["% Complete", nil],
@@ -347,12 +347,12 @@ RSpec.describe XlsExport::WorkPackage::Exporter::XLS do
       # why is there a nil at the end of the headers row?
       headers_row.compact!
       expect(headers_row.zip(values_row)).to eq [
-        ["Work", "10.0"],
-        ["Total work", "15.0"],
-        ["Remaining work", "5.0"],
-        ["Total remaining work", "8.0"],
-        ["% Complete", "50%"],
-        ["Total % complete", "75%"]
+        ["Work", 10.0],
+        ["Total work", 15.0],
+        ["Remaining work", 5.0],
+        ["Total remaining work", 8.0],
+        ["% Complete", 0.5],
+        ["Total % complete", 0.75]
       ]
     end
   end
@@ -378,12 +378,12 @@ RSpec.describe XlsExport::WorkPackage::Exporter::XLS do
       # why is there a nil at the end of the headers row?
       headers_row.compact!
       expect(headers_row.zip(values_row)).to eq [
-        ["Work", "0.0"],
-        ["Total work", "15.0"],
-        ["Remaining work", "0.0"],
-        ["Total remaining work", "8.0"],
-        ["% Complete", "0%"],
-        ["Total % complete", "42%"]
+        ["Work", 0.0],
+        ["Total work", 15.0],
+        ["Remaining work", 0.0],
+        ["Total remaining work", 8.0],
+        ["% Complete", 0.0],
+        ["Total % complete", 0.42]
       ]
     end
   end

spec/models/work_package/exports/formatters/pdf/compound_done_ratio_spec.rb

@@ -30,7 +30,7 @@
 
 require "spec_helper"
 
-RSpec.describe WorkPackage::Exports::Formatters::CompoundDoneRatio do
+RSpec.describe WorkPackage::Exports::Formatters::PDF::CompoundDoneRatio do
   subject { described_class.new(:done_ratio) }
 
   describe "#format" do

spec/models/work_package/exports/formatters/xls/hours_spec.rb

@@ -30,7 +30,7 @@
 
 require "spec_helper"
 
-RSpec.describe WorkPackage::Exports::Formatters::WorkHours do
+RSpec.describe WorkPackage::Exports::Formatters::XLS::Hours do
   let(:formatter_instance) { described_class.new(:estimated_hours) }
 
   describe ".apply?" do
@@ -38,12 +38,20 @@ RSpec.describe WorkPackage::Exports::Formatters::WorkHours do
       expect(described_class.apply?(:estimated_hours, :csv)).to be true
     end
 
+    it "returns true for derived_estimated_hours and csv format" do
+      expect(described_class.apply?(:derived_estimated_hours, :csv)).to be true
+    end
+
     it "returns true for remaining_hours and csv format" do
       expect(described_class.apply?(:remaining_hours, :csv)).to be true
     end
 
-    it "returns false for spent_hours and csv format" do
-      expect(described_class.apply?(:spent_hours, :csv)).to be false
+    it "returns true for derived_remaining_hours and csv format" do
+      expect(described_class.apply?(:derived_remaining_hours, :csv)).to be true
+    end
+
+    it "returns true for spent_hours and csv format" do
+      expect(described_class.apply?(:spent_hours, :csv)).to be true
     end
 
     it "returns false for estimated_hours and pdf format" do
@@ -55,9 +63,16 @@ RSpec.describe WorkPackage::Exports::Formatters::WorkHours do
     end
   end
 
+  describe "#format" do
+    it "returns the number of hours as a float" do
+      work_package = build_stubbed(:work_package, estimated_hours: 1.2)
+      expect(formatter_instance.format(work_package)).to eq(1.2)
+    end
+  end
+
   describe "#format_options" do
     it "returns number format for hours" do
-      expect(formatter_instance.format_options).to eq({ number_format: "0.00" })
+      expect(formatter_instance.format_options).to eq({ number_format: '0.00 "h"' })
     end
   end
 end