From 4935d3a3d4dc1215edf272cf94ed1ef5624f1253 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Thu, 28 May 2020 17:32:49 +0200 Subject: [PATCH 1/4] Fix #33448: Format WP column "BCF snapshot" as string Render an "X" if there is at least one viewpoint for the associated BCF issue. https://community.openproject.org/wp/33448 --- .../work_package/exporter/formatters.rb | 12 ++-- .../pdf_export/work_package_list_to_pdf.rb | 6 ++ modules/bim/lib/open_project/bim/engine.rb | 1 + .../exporter/formatters/bcf_thumbnail.rb | 11 ++++ .../exporter/formatters/bcf_thumbnail_spec.rb | 63 +++++++++++++++++++ 5 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 modules/bim/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail.rb create mode 100644 modules/bim/spec/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail_spec.rb diff --git a/app/models/work_package/exporter/formatters.rb b/app/models/work_package/exporter/formatters.rb index 0b6e86ac26..d3b9358f77 100644 --- a/app/models/work_package/exporter/formatters.rb +++ b/app/models/work_package/exporter/formatters.rb @@ -1,11 +1,11 @@ module WorkPackage::Exporter module Formatters + @@formatters = %i[default costs estimated_hours].map do |key| + Kernel.const_get("WorkPackage::Exporter::Formatters::#{key.to_s.camelize}") + end + def self.all - @formatters ||= begin - %i[default costs estimated_hours].map do |key| - Kernel.const_get("WorkPackage::Exporter::Formatters::#{key.to_s.camelize}") - end - end + @@formatters end def self.keys @@ -13,7 +13,7 @@ module WorkPackage::Exporter end def self.register(clz) - @formatters << clz + @@formatters << clz end ## diff --git a/app/models/work_package/pdf_export/work_package_list_to_pdf.rb b/app/models/work_package/pdf_export/work_package_list_to_pdf.rb index 06d5706bfe..4e79dd5af5 100644 --- a/app/models/work_package/pdf_export/work_package_list_to_pdf.rb +++ b/app/models/work_package/pdf_export/work_package_list_to_pdf.rb @@ -317,4 +317,10 @@ class WorkPackage::PDFExport::WorkPackageListToPdf < WorkPackage::Exporter::Base false end end + + def make_bcf_thumbnail_field_value(work_package) + # A BCF viewpoint must have a snapshot. So it's safe to check for + # viewpoint existence only. + work_package.bcf_issue.viewpoints.any? ? 'X' : '' + end end diff --git a/modules/bim/lib/open_project/bim/engine.rb b/modules/bim/lib/open_project/bim/engine.rb index e9bc9723bd..0e23108e1c 100644 --- a/modules/bim/lib/open_project/bim/engine.rb +++ b/modules/bim/lib/open_project/bim/engine.rb @@ -213,6 +213,7 @@ module OpenProject::Bim config.to_prepare do ::WorkPackage::Exporter .register_for_list(:bcf, OpenProject::Bim::BcfXml::Exporter) + ::WorkPackage::Exporter::Formatters.register(OpenProject::Bim::WorkPackage::Exporter::Formatters::BcfThumbnail) ::Queries::Register.filter ::Query, ::Bim::Queries::WorkPackages::Filter::BcfIssueAssociatedFilter ::Queries::Register.column ::Query, ::Bim::Queries::WorkPackages::Columns::BcfThumbnailColumn diff --git a/modules/bim/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail.rb b/modules/bim/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail.rb new file mode 100644 index 0000000000..404be2ecc5 --- /dev/null +++ b/modules/bim/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail.rb @@ -0,0 +1,11 @@ +module OpenProject::Bim::WorkPackage::Exporter::Formatters + class BcfThumbnail < WorkPackage::Exporter::Formatters::Default + def self.apply?(column) + column.is_a? ::Bim::Queries::WorkPackages::Columns::BcfThumbnailColumn + end + + def format(work_package, column, **options) + work_package&.bcf_issue&.viewpoints&.any? ? "x" : '' + end + end +end diff --git a/modules/bim/spec/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail_spec.rb b/modules/bim/spec/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail_spec.rb new file mode 100644 index 0000000000..9ff8f1c276 --- /dev/null +++ b/modules/bim/spec/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail_spec.rb @@ -0,0 +1,63 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2021 the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See docs/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' + +describe OpenProject::Bim::WorkPackage::Exporter::Formatters::BcfThumbnail do + let(:bcf_thumbnail_column) { ::Bim::Queries::WorkPackages::Columns::BcfThumbnailColumn.new("Some column name") } + let(:not_bcf_thumbnail_column) { "This not a BcfThumbnailColumn" } + + describe '::apply?' do + it 'returns TRUE for any other class' do + expect(described_class.apply?(bcf_thumbnail_column)).to be_truthy + end + + it 'returns FALSE for any other class' do + expect(described_class.apply?(not_bcf_thumbnail_column)).to be_falsey + end + end + + describe '::format' do + let(:work_package_with_viewpoint) { FactoryBot.create(:work_package) } + let(:bcf_issue) { FactoryBot.create(:bcf_issue_with_viewpoint, work_package: work_package_with_viewpoint) } + let(:work_package_without_viewpoint) { FactoryBot.create(:work_package) } + + before do + bcf_issue + end + + it 'returns "x" for work packages that have BCF issues with at least one viewpoint' do + expect(described_class.new.format(work_package_with_viewpoint, bcf_thumbnail_column)).to eql('x') + end + + it 'returns "" for work packages without viewpoints attached' do + expect(described_class.new.format(work_package_without_viewpoint, bcf_thumbnail_column)).to eql('') + end + end +end + From eff86a228ceeb429efa55ce6203659d39a9a4059 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Thu, 5 Aug 2021 15:53:00 +0200 Subject: [PATCH 2/4] Post review modifications - Don't use class variables. - Use memoized class instance variables. - Fix class reloading in development environment. --- app/models/work_package/exporter/formatters.rb | 18 +++++++++++++----- .../exporter/formatters/default.rb | 2 +- .../pdf_export/work_package_list_to_pdf.rb | 6 ------ modules/bim/lib/open_project/bim/engine.rb | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/app/models/work_package/exporter/formatters.rb b/app/models/work_package/exporter/formatters.rb index d3b9358f77..ea96b7b0cc 100644 --- a/app/models/work_package/exporter/formatters.rb +++ b/app/models/work_package/exporter/formatters.rb @@ -1,19 +1,27 @@ module WorkPackage::Exporter module Formatters - @@formatters = %i[default costs estimated_hours].map do |key| - Kernel.const_get("WorkPackage::Exporter::Formatters::#{key.to_s.camelize}") + def self.default_formatter_strings + @default_formatters_strings ||= %i[default costs estimated_hours].map do |key| + "WorkPackage::Exporter::Formatters::#{key.to_s.camelize}" + end + end + + def self.all_formatter_strings + @all_formatter_strings ||= default_formatter_strings end def self.all - @@formatters + all_formatter_strings.each do |formatter_string| + Kernel.const_get(formatter_string) + end end def self.keys all.map(&:key) end - def self.register(clz) - @@formatters << clz + def self.register(class_string) + @all_formatter_strings = all_formatter_strings + [class_string] end ## diff --git a/app/models/work_package/exporter/formatters/default.rb b/app/models/work_package/exporter/formatters/default.rb index 789b493203..320bc9b955 100644 --- a/app/models/work_package/exporter/formatters/default.rb +++ b/app/models/work_package/exporter/formatters/default.rb @@ -41,4 +41,4 @@ module WorkPackage::Exporter end end end -end \ No newline at end of file +end diff --git a/app/models/work_package/pdf_export/work_package_list_to_pdf.rb b/app/models/work_package/pdf_export/work_package_list_to_pdf.rb index 4e79dd5af5..06d5706bfe 100644 --- a/app/models/work_package/pdf_export/work_package_list_to_pdf.rb +++ b/app/models/work_package/pdf_export/work_package_list_to_pdf.rb @@ -317,10 +317,4 @@ class WorkPackage::PDFExport::WorkPackageListToPdf < WorkPackage::Exporter::Base false end end - - def make_bcf_thumbnail_field_value(work_package) - # A BCF viewpoint must have a snapshot. So it's safe to check for - # viewpoint existence only. - work_package.bcf_issue.viewpoints.any? ? 'X' : '' - end end diff --git a/modules/bim/lib/open_project/bim/engine.rb b/modules/bim/lib/open_project/bim/engine.rb index 0e23108e1c..af4e75a9ba 100644 --- a/modules/bim/lib/open_project/bim/engine.rb +++ b/modules/bim/lib/open_project/bim/engine.rb @@ -213,7 +213,7 @@ module OpenProject::Bim config.to_prepare do ::WorkPackage::Exporter .register_for_list(:bcf, OpenProject::Bim::BcfXml::Exporter) - ::WorkPackage::Exporter::Formatters.register(OpenProject::Bim::WorkPackage::Exporter::Formatters::BcfThumbnail) + ::WorkPackage::Exporter::Formatters.register("OpenProject::Bim::WorkPackage::Exporter::Formatters::BcfThumbnail") ::Queries::Register.filter ::Query, ::Bim::Queries::WorkPackages::Filter::BcfIssueAssociatedFilter ::Queries::Register.column ::Query, ::Bim::Queries::WorkPackages::Columns::BcfThumbnailColumn From 8332b660b4d5c2e59d9a6e83ce0ae2b0286e71f3 Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Thu, 5 Aug 2021 16:05:44 +0200 Subject: [PATCH 3/4] Satisfy rubocop --- app/models/work_package/exporter/formatters.rb | 2 +- .../bim/work_package/exporter/formatters/bcf_thumbnail.rb | 2 +- .../bim/work_package/exporter/formatters/bcf_thumbnail_spec.rb | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/models/work_package/exporter/formatters.rb b/app/models/work_package/exporter/formatters.rb index ea96b7b0cc..e7657e98bc 100644 --- a/app/models/work_package/exporter/formatters.rb +++ b/app/models/work_package/exporter/formatters.rb @@ -1,7 +1,7 @@ module WorkPackage::Exporter module Formatters def self.default_formatter_strings - @default_formatters_strings ||= %i[default costs estimated_hours].map do |key| + @default_formatter_strings ||= %i[default costs estimated_hours].map do |key| "WorkPackage::Exporter::Formatters::#{key.to_s.camelize}" end end diff --git a/modules/bim/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail.rb b/modules/bim/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail.rb index 404be2ecc5..2050145f9e 100644 --- a/modules/bim/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail.rb +++ b/modules/bim/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail.rb @@ -4,7 +4,7 @@ module OpenProject::Bim::WorkPackage::Exporter::Formatters column.is_a? ::Bim::Queries::WorkPackages::Columns::BcfThumbnailColumn end - def format(work_package, column, **options) + def format(work_package, _column, **_options) work_package&.bcf_issue&.viewpoints&.any? ? "x" : '' end end diff --git a/modules/bim/spec/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail_spec.rb b/modules/bim/spec/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail_spec.rb index 9ff8f1c276..bedcb71cd4 100644 --- a/modules/bim/spec/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail_spec.rb +++ b/modules/bim/spec/lib/open_project/bim/work_package/exporter/formatters/bcf_thumbnail_spec.rb @@ -60,4 +60,3 @@ describe OpenProject::Bim::WorkPackage::Exporter::Formatters::BcfThumbnail do end end end - From 2850eee2b81a61ac8f28fbfd110d1e7a7bbd925b Mon Sep 17 00:00:00 2001 From: Wieland Lindenthal Date: Fri, 6 Aug 2021 13:03:23 +0200 Subject: [PATCH 4/4] fixing logic --- app/models/work_package/exporter/formatters.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/work_package/exporter/formatters.rb b/app/models/work_package/exporter/formatters.rb index e7657e98bc..7b1f2beab0 100644 --- a/app/models/work_package/exporter/formatters.rb +++ b/app/models/work_package/exporter/formatters.rb @@ -11,7 +11,7 @@ module WorkPackage::Exporter end def self.all - all_formatter_strings.each do |formatter_string| + all_formatter_strings.map do |formatter_string| Kernel.const_get(formatter_string) end end