From 0bd53cc8190f2533070a91f9355ea9551f66e507 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Fri, 6 Sep 2024 14:38:54 +0200 Subject: [PATCH 1/4] [#55021] remove unjust space in copyright --- app/models/queries/projects/selects/default.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/queries/projects/selects/default.rb b/app/models/queries/projects/selects/default.rb index 59cb279f11c7..1cd2085619bf 100644 --- a/app/models/queries/projects/selects/default.rb +++ b/app/models/queries/projects/selects/default.rb @@ -1,4 +1,4 @@ -# -- copyright +#-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH # From 439e21bc03db985aa4dbc0fa1a1f97917e0ddf10 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 10 Sep 2024 11:22:10 +0200 Subject: [PATCH 2/4] [#55021] add id and identifier to project list query --- app/components/projects/row_component.rb | 14 ++++++++++++++ .../queries/projects/orders/default_order.rb | 2 +- app/models/queries/projects/selects/default.rb | 2 +- spec/models/queries/projects/project_query_spec.rb | 4 ++++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/app/components/projects/row_component.rb b/app/components/projects/row_component.rb index 5ba909a75f23..6a610b21aa6d 100644 --- a/app/components/projects/row_component.rb +++ b/app/components/projects/row_component.rb @@ -107,6 +107,20 @@ def required_disk_space number_to_human_size(project.required_disk_space, precision: 2) end + def id + content = "".html_safe + + content << project.id.to_s + content + end + + def identifier + content = "".html_safe + + content << project.identifier + content + end + def name content = content_tag(:i, "", class: "projects-table--hierarchy-icon") diff --git a/app/models/queries/projects/orders/default_order.rb b/app/models/queries/projects/orders/default_order.rb index e612807171e3..de5154c5b002 100644 --- a/app/models/queries/projects/orders/default_order.rb +++ b/app/models/queries/projects/orders/default_order.rb @@ -30,6 +30,6 @@ class Queries::Projects::Orders::DefaultOrder < Queries::Orders::Base self.model = Project def self.key - /\A(id|created_at|public|lft)\z/ + /\A(id|identifier|created_at|public|lft)\z/ end end diff --git a/app/models/queries/projects/selects/default.rb b/app/models/queries/projects/selects/default.rb index 1cd2085619bf..55945c4ace5b 100644 --- a/app/models/queries/projects/selects/default.rb +++ b/app/models/queries/projects/selects/default.rb @@ -27,7 +27,7 @@ # ++ class Queries::Projects::Selects::Default < Queries::Selects::Base - KEYS = %i[status_explanation hierarchy name public description].freeze + KEYS = %i[id identifier status_explanation hierarchy name public description].freeze def self.key Regexp.new(KEYS.join("|")) diff --git a/spec/models/queries/projects/project_query_spec.rb b/spec/models/queries/projects/project_query_spec.rb index ea79563c9317..57fa3e5596f6 100644 --- a/spec/models/queries/projects/project_query_spec.rb +++ b/spec/models/queries/projects/project_query_spec.rb @@ -132,6 +132,8 @@ it "lists registered selects" do expect(instance.available_selects.map(&:attribute)) .to match_array(%i[ + id + identifier name favored public @@ -151,6 +153,8 @@ it "includes admin columns" do expect(instance.available_selects.map(&:attribute)) .to match_array(%i[ + id + identifier name favored public From a8d365ec4007a096b449d7b6e1f77fba23c043ab Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Tue, 10 Sep 2024 15:45:06 +0200 Subject: [PATCH 3/4] [#55021] project list export drops the `id` and `identifier` default From now on, if you want to export these columns, you must explicitly select these in the view configuration before exporting. Your export will always reflect the columns that you see in the web view. --- app/models/projects/exports/query_exporter.rb | 9 +---- .../project/exporter/xls_integration_spec.rb | 33 +++++++++++-------- .../projects/exporter/csv_integration_spec.rb | 31 +++++++++-------- 3 files changed, 38 insertions(+), 35 deletions(-) diff --git a/app/models/projects/exports/query_exporter.rb b/app/models/projects/exports/query_exporter.rb index a26d2ec7e69e..e4a110afd9bf 100644 --- a/app/models/projects/exports/query_exporter.rb +++ b/app/models/projects/exports/query_exporter.rb @@ -32,7 +32,7 @@ class QueryExporter < Exports::Exporter alias :query :object def columns - @columns ||= (forced_columns + selected_columns) + @columns ||= selected_columns end def page @@ -51,13 +51,6 @@ def projects private - def forced_columns - [ - { name: :id, caption: Project.human_attribute_name(:id) }, - { name: :identifier, caption: Project.human_attribute_name(:identifier) } - ] - end - def selected_columns query .selects diff --git a/modules/xls_export/spec/models/xls_export/project/exporter/xls_integration_spec.rb b/modules/xls_export/spec/models/xls_export/project/exporter/xls_integration_spec.rb index bddf083e1391..69081bee1c31 100644 --- a/modules/xls_export/spec/models/xls_export/project/exporter/xls_integration_spec.rb +++ b/modules/xls_export/spec/models/xls_export/project/exporter/xls_integration_spec.rb @@ -27,8 +27,7 @@ it "performs a successful export" do expect(rows.count).to eq(1) - expect(sheet.row(1)).to eq [project.id.to_s, project.identifier, - project.name, project.description, "Off track", "false"] + expect(sheet.row(1)).to eq [project.name, project.description, "Off track", "false"] end context "with project description containing html" do @@ -38,8 +37,7 @@ it "performs a successful export" do expect(rows.count).to eq(1) - expect(sheet.row(1)).to eq [project.id.to_s, project.identifier, project.name, - "This is an html description.", "Off track", "false"] + expect(sheet.row(1)).to eq [project.name, "This is an html description.", "Off track", "false"] end end @@ -48,12 +46,21 @@ it "performs a successful export" do expect(rows.count).to eq(1) - expect(sheet.row(1)).to eq [project.id.to_s, project.identifier, - project.name, project.description, + expect(sheet.row(1)).to eq [project.name, project.description, "Off track", project.status_explanation, "false"] end end + context "with id and identifier enabled" do + let(:query_columns) { %w[name description project_status public id identifier] } + + it "performs a successful export" do + expect(rows.count).to eq(1) + expect(sheet.row(1)).to eq [project.name, project.description, "Off track", + "false", project.id.to_s, project.identifier] + end + end + describe "custom field columns selected" do let(:query_columns) { %w[name description project_status public] + global_project_custom_fields.map(&:column_name) } @@ -66,7 +73,7 @@ it "renders all those columns" do cf_names = global_project_custom_fields.map(&:name) - expect(header).to eq ["ID", "Identifier", "Name", "Description", "Status", "Public", *cf_names] + expect(header).to eq ["Name", "Description", "Status", "Public", *cf_names] expect(header).to include not_used_string_cf.name expect(header).to include hidden_cf.name @@ -85,8 +92,7 @@ end expect(sheet.row(1)) - .to eq [project.id.to_s, project.identifier, project.name, project.description, "Off track", "false", - *custom_values] + .to eq [project.name, project.description, "Off track", "false", *custom_values] # The column for the project-level-disabled custom field is blank expect(sheet.row(1)[header.index(not_used_string_cf.name)]).to be_nil @@ -97,7 +103,7 @@ it "renders available project custom fields in the header if enabled in any project" do cf_names = global_project_custom_fields.map(&:name) - expect(header).to eq ["ID", "Identifier", "Name", "Description", "Status", "Public", *cf_names] + expect(header).to eq ["Name", "Description", "Status", "Public", *cf_names] expect(header).not_to include not_used_string_cf.name expect(header).not_to include hidden_cf.name @@ -116,8 +122,7 @@ end expect(sheet.row(1)) - .to eq [project.id.to_s, project.identifier, project.name, project.description, "Off track", "false", - *custom_values] + .to eq [project.name, project.description, "Off track", "false", *custom_values] end end @@ -125,10 +130,10 @@ let(:permissions) { %i(view_projects) } it "does not render project custom fields in the header" do - expect(header).to eq ["ID", "Identifier", "Name", "Description", "Status", "Public"] + expect(header).to eq %w[Name Description Status Public] expect(sheet.row(1)) - .to eq [project.id.to_s, project.identifier, project.name, project.description, "Off track", "false"] + .to eq [project.name, project.description, "Off track", "false"] end end end diff --git a/spec/models/projects/exporter/csv_integration_spec.rb b/spec/models/projects/exporter/csv_integration_spec.rb index 11dcb17aa1db..77d5e362d2c4 100644 --- a/spec/models/projects/exporter/csv_integration_spec.rb +++ b/spec/models/projects/exporter/csv_integration_spec.rb @@ -43,8 +43,7 @@ it "performs a successful export" do expect(parsed.size).to eq(2) - expect(parsed.last).to eq [project.id.to_s, project.identifier, - project.name, project.description, "Off track", "false"] + expect(parsed.last).to eq [project.name, project.description, "Off track", "false"] end context "with status_explanation enabled" do @@ -52,12 +51,21 @@ it "performs a successful export" do expect(parsed.size).to eq(2) - expect(parsed.last).to eq [project.id.to_s, project.identifier, - project.name, project.description, + expect(parsed.last).to eq [project.name, project.description, "Off track", "some explanation", "false"] end end + context "with id and identifier selected" do + let(:query_columns) { %w[name description id identifier project_status public] } + + it "performs a successful export" do + expect(parsed.size).to eq(2) + expect(parsed.last).to eq [project.name, project.description, project.id.to_s, + project.identifier, "Off track", "false"] + end + end + describe "custom field columns selected" do let(:query_columns) do %w[name description project_status public] + global_project_custom_fields.map(&:column_name) @@ -74,13 +82,12 @@ it "does not render project custom fields in the header" do expect(parsed.size).to eq 2 - expect(header).to eq ["\xEF\xBB\xBFid", "Identifier", "Name", "Description", "Status", "Public"] + expect(header).to eq ["\xEF\xBB\xBFName", "Description", "Status", "Public"] end it "does not render the custom field values in the rows if enabled for a project" do expect(rows.first) - .to eq [project.id.to_s, project.identifier, project.name, - project.description, "Off track", "false"] + .to eq [project.name, project.description, "Off track", "false"] end end @@ -93,7 +100,7 @@ expect(cf_names).not_to include(not_used_string_cf.name) expect(cf_names).not_to include(hidden_cf.name) - expect(header).to eq ["\xEF\xBB\xBFid", "Identifier", "Name", "Description", "Status", "Public", *cf_names] + expect(header).to eq ["\xEF\xBB\xBFName", "Description", "Status", "Public", *cf_names] end it "renders the custom field values in the rows if enabled for a project" do @@ -110,8 +117,7 @@ end end expect(rows.first) - .to eq [project.id.to_s, project.identifier, project.name, - project.description, "Off track", "false", *custom_values] + .to eq [project.name, project.description, "Off track", "false", *custom_values] end end @@ -126,7 +132,7 @@ expect(cf_names).to include(not_used_string_cf.name) expect(cf_names).to include(hidden_cf.name) - expect(header).to eq ["\xEF\xBB\xBFid", "Identifier", "Name", "Description", "Status", "Public", *cf_names] + expect(header).to eq ["\xEF\xBB\xBFName", "Description", "Status", "Public", *cf_names] end it "renders the custom field values in the rows if enabled for a project" do @@ -145,8 +151,7 @@ end end expect(rows.first) - .to eq [project.id.to_s, project.identifier, project.name, - project.description, "Off track", "false", *custom_values] + .to eq [project.name, project.description, "Off track", "false", *custom_values] end end end From 3c2a48ce22265fa584cc559a37ff48580159fd80 Mon Sep 17 00:00:00 2001 From: Tobias Dillmann Date: Wed, 11 Sep 2024 13:46:21 +0200 Subject: [PATCH 4/4] [#55021] remove unnecessary `html_safe` Both id and identifier are simple strings. No need no mark them as html_safe. --- app/components/projects/row_component.rb | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/app/components/projects/row_component.rb b/app/components/projects/row_component.rb index 6a610b21aa6d..bcfac8bb6aa4 100644 --- a/app/components/projects/row_component.rb +++ b/app/components/projects/row_component.rb @@ -30,6 +30,7 @@ module Projects class RowComponent < ::RowComponent delegate :favored_project_ids, to: :table + delegate :identifier, to: :project def project model.first @@ -108,17 +109,7 @@ def required_disk_space end def id - content = "".html_safe - - content << project.id.to_s - content - end - - def identifier - content = "".html_safe - - content << project.identifier - content + project.id.to_s end def name