From c10fe657078d2e73e8c5224a9396b29523a20abd Mon Sep 17 00:00:00 2001 From: as-op Date: Thu, 22 Oct 2026 12:49:53 +0200 Subject: [PATCH 1/3] [JIM-263] Jira migrator creates a new numbered custom field on every import run instead of reusing the one it created before https://community.openproject.org/wp/JIM-263 --- .../custom_field_names.rb | 11 ++++ .../jira_import_custom_field_builder.rb | 15 ++++- .../jira-migration/custom-fields/README.md | 3 + .../custom_field_names_spec.rb | 29 ++++++++++ .../jira_import_custom_field_builder_spec.rb | 23 ++++---- ...ct_work_packages_job_custom_fields_spec.rb | 57 +++++++++++++++++++ 6 files changed, 126 insertions(+), 12 deletions(-) diff --git a/app/workers/import/jira_create_project_job/custom_field_names.rb b/app/workers/import/jira_create_project_job/custom_field_names.rb index 4bfd03a77fed..d3b571dc20b1 100644 --- a/app/workers/import/jira_create_project_job/custom_field_names.rb +++ b/app/workers/import/jira_create_project_job/custom_field_names.rb @@ -30,6 +30,9 @@ module Import class JiraCreateProjectJob module CustomFieldNames + # The counter `unique_name` appends when a name is taken ahd cannot be reused (eg. int vs float type of the field) + NUMBERING_SUFFIX = /\s*\(\d+\)\z/ + module_function # Matches `Example`, `Example (2)`, `Example (DYX)` and `Example (DYX, ABC) @@ -41,6 +44,14 @@ def candidates(base_name) .order(:id) end + def same_field?(name, other) + without_numbering(name).casecmp?(without_numbering(other)) + end + + def without_numbering(name) + name.to_s.sub(NUMBERING_SUFFIX, "") + end + def unique_name(name) unique_name = name suffix = 2 diff --git a/app/workers/import/jira_create_project_job/jira_import_custom_field_builder.rb b/app/workers/import/jira_create_project_job/jira_import_custom_field_builder.rb index f9146007fa9e..2416f224aad8 100644 --- a/app/workers/import/jira_create_project_job/jira_import_custom_field_builder.rb +++ b/app/workers/import/jira_create_project_job/jira_import_custom_field_builder.rb @@ -299,8 +299,19 @@ def compatible_multi_value?(custom_field) def best_matching_candidate(candidates, run_custom_field_ids) return best_value_bearing_candidate(candidates, run_custom_field_ids) if value_bearing_format? - candidates.find { |cf| cf.name.casecmp?(@import_name) } || - candidates.find { |cf| cf.name.casecmp?(stable_base_name) } + exact_name_match(candidates) || numbered_name_match(candidates) + end + + def exact_name_match(candidates) + import_names.filter_map { |name| candidates.find { |cf| cf.name.casecmp?(name) } }.first + end + + def numbered_name_match(candidates) + import_names.filter_map { |name| candidates.find { |cf| CustomFieldNames.same_field?(cf.name, name) } }.first + end + + def import_names + [@import_name, stable_base_name].uniq end def value_bearing_format? diff --git a/docs/installation-and-operations/jira-migration/custom-fields/README.md b/docs/installation-and-operations/jira-migration/custom-fields/README.md index 6ad5fa4883ab..75eaf11b9939 100644 --- a/docs/installation-and-operations/jira-migration/custom-fields/README.md +++ b/docs/installation-and-operations/jira-migration/custom-fields/README.md @@ -141,6 +141,9 @@ If an OpenProject custom field with the same name and format already exists (fro the migrator reuses it instead of creating a duplicate. The existing custom field is linked to the import and its values are preserved. If a name collision exists but the formats differ, the migrator appends a numeric suffix to the new field name (e.g., `My Field (2)`). +Such a numeric suffix is disregarded when the migrator looks for a field to reuse, so a later run reuses `My Field (2)` +instead of creating `My Field (3)`. Only a numeric suffix is treated this way - the project-key suffixes described under +[Field contexts](#field-contexts) stay part of the name. For **Hierarchy** and **List** fields, deduplication is attempted but not guaranteed because option lists may differ. A new custom field with a numeric suffix may be created for these types. diff --git a/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb b/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb index 84acb4e6c915..95c4f22e2c23 100644 --- a/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb +++ b/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb @@ -65,6 +65,35 @@ end end + describe ".same_field?" do + it "matches the same name regardless of case" do + expect(described_class).to be_same_field("Story Points", "STORY POINTS") + end + + it "matches a name carrying the counter unique_name appends" do + expect(described_class).to be_same_field("Story Points (2)", "Story Points") + expect(described_class).to be_same_field("Story Points", "Story Points (12)") + end + + it "matches two names carrying a different counter" do + expect(described_class).to be_same_field("Story Points (2)", "Story Points (3)") + end + + it "keeps a project key suffix apart from the counter" do + expect(described_class).not_to be_same_field("Story Points (XYZ)", "Story Points") + expect(described_class).not_to be_same_field("Story Points (XYZ)", "Story Points (2)") + end + + it "matches a project key suffix with itself despite a counter" do + expect(described_class).to be_same_field("Story Points (XYZ) (2)", "Story Points (XYZ)") + end + + it "does not match a different name" do + expect(described_class).not_to be_same_field("Story Points Estimate", "Story Points") + expect(described_class).not_to be_same_field("Story Points 2", "Story Points") + end + end + describe ".unique_name" do subject(:unique_name) { described_class.unique_name("Resolution") } diff --git a/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb b/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb index 30fbfb1e8152..04f34cdaf5ac 100644 --- a/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb +++ b/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb @@ -981,21 +981,24 @@ def date_builder expect(date_builder.find_existing_custom_field).to eq(exact) end - it "does not reuse a suffixed candidate when no exact name matches" do - create(:date_wp_custom_field, name: "Due (archived)") + it "prefers the exact base name over a numbered candidate created before it" do create(:date_wp_custom_field, name: "Due (2)") + exact = create(:date_wp_custom_field, name: "Due") - expect(date_builder.find_existing_custom_field).to be_nil + expect(date_builder.find_existing_custom_field).to eq(exact) end - it "keeps two Jira fields apart when one is named like the other's dedup suffix" do - suffixed = create(:string_wp_custom_field, name: "foo (2)") - schema = { "type" => "string", "custom" => "com.atlassian.jira.plugin.system.customfieldtypes:textfield" } - builder = described_class.new(jira_field_for(name: "foo", schema:), jira_import:) + it "reuses the numbered field an earlier run created when no exact name matches" do + create(:date_wp_custom_field, name: "Due (archived)") + numbered = create(:date_wp_custom_field, name: "Due (2)") - expect(builder.find_existing_custom_field).to be_nil - expect(builder.custom_field_settings.first).to eq("foo") - expect(suffixed.reload.name).to eq("foo (2)") + expect(date_builder.find_existing_custom_field).to eq(numbered) + end + + it "ignores a candidate whose suffix is not the counter unique_name appends" do + create(:date_wp_custom_field, name: "Due (archived)") + + expect(date_builder.find_existing_custom_field).to be_nil end end diff --git a/spec/workers/import/jira_create_project_work_packages_job_custom_fields_spec.rb b/spec/workers/import/jira_create_project_work_packages_job_custom_fields_spec.rb index 95849b754afc..f02836220470 100644 --- a/spec/workers/import/jira_create_project_work_packages_job_custom_fields_spec.rb +++ b/spec/workers/import/jira_create_project_work_packages_job_custom_fields_spec.rb @@ -1384,4 +1384,61 @@ def per_issue_union(job, registry, jira_project) end end end + + describe "a custom field an earlier import run had to name with a counter" do + let!(:jira_field) do + create(:jira_field, jira_import:, + origin_id: "customfield_10254", + payload: { + "id" => "customfield_10254", + "name" => "CF Number", + "schema" => { + "type" => "number", + "custom" => "com.atlassian.jira.plugin.system.customfieldtypes:float", + "customId" => 10254 + } + }) + end + let!(:jira_issue) do + create(:jira_issue, jira_import:, + origin_id: "10200", + jira_project:, + payload: issue_payload) + end + + # An existing field of an incompatible format cannot be reused, so the first run has to fall + # back to the next free name. + let!(:incompatible_custom_field) { create(:integer_wp_custom_field, name: "CF Number") } + + let(:second_jira_import) do + create(:jira_import, jira:, author:, + projects: [{ "id" => jira_project_id, "key" => jira_project_key, + "name" => jira_project_name }]) + end + + def run_second_import + second_jira_project = create(:jira_project, jira_import: second_jira_import, + origin_id: jira_project_id, + payload: jira_project_payload) + create(:jira_field, jira_import: second_jira_import, + origin_id: jira_field.origin_id, + payload: jira_field.payload) + create(:jira_issue, jira_import: second_jira_import, + origin_id: "10200", + jira_project: second_jira_project, + payload: issue_payload) + Import::JiraCreateCustomFieldsJob.perform_now(second_jira_import.id) + end + + before { import_project } + + it "names it with the next free counter" do + expect(WorkPackageCustomField.pluck(:name)).to contain_exactly("CF Number", "CF Number (2)") + end + + it "reuses it on a later import run instead of allocating the next counter" do + expect { run_second_import }.not_to change(WorkPackageCustomField, :count) + expect(WorkPackageCustomField.pluck(:name)).to contain_exactly("CF Number", "CF Number (2)") + end + end end From 5567280b986fd2bf128c73406886a57ccadf1ec3 Mon Sep 17 00:00:00 2001 From: Andrej <77627197+as-op@users.noreply.github.com> Date: Mon, 26 Oct 2026 12:48:38 +0100 Subject: [PATCH 2/3] Apply suggestion from @klappradla Co-authored-by: Max Mulatz --- .../import/jira_create_project_job/custom_field_names.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/import/jira_create_project_job/custom_field_names.rb b/app/workers/import/jira_create_project_job/custom_field_names.rb index d3b571dc20b1..cf5eb8c3be99 100644 --- a/app/workers/import/jira_create_project_job/custom_field_names.rb +++ b/app/workers/import/jira_create_project_job/custom_field_names.rb @@ -30,7 +30,7 @@ module Import class JiraCreateProjectJob module CustomFieldNames - # The counter `unique_name` appends when a name is taken ahd cannot be reused (eg. int vs float type of the field) + # The counter `unique_name` appends when a name is taken and cannot be reused (e.g. int vs float type of the field) NUMBERING_SUFFIX = /\s*\(\d+\)\z/ module_function From df759c0343b282dec2b23558f8185d7c062139c7 Mon Sep 17 00:00:00 2001 From: as-op Date: Mon, 26 Oct 2026 12:59:26 +0100 Subject: [PATCH 3/3] remove over-normalisation https://github.com/opf/openproject/pull/26237/changes#r4376351208 --- .../custom_field_names.rb | 4 ++-- .../custom_field_names_spec.rb | 13 ++++++++----- .../jira_import_custom_field_builder_spec.rb | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/app/workers/import/jira_create_project_job/custom_field_names.rb b/app/workers/import/jira_create_project_job/custom_field_names.rb index d3b571dc20b1..0b6cd265a2d9 100644 --- a/app/workers/import/jira_create_project_job/custom_field_names.rb +++ b/app/workers/import/jira_create_project_job/custom_field_names.rb @@ -44,8 +44,8 @@ def candidates(base_name) .order(:id) end - def same_field?(name, other) - without_numbering(name).casecmp?(without_numbering(other)) + def same_field?(custom_field_name, import_name) + without_numbering(custom_field_name).casecmp?(import_name.to_s) end def without_numbering(name) diff --git a/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb b/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb index 95c4f22e2c23..4990790d7f38 100644 --- a/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb +++ b/spec/workers/import/jira_create_project_job/custom_field_names_spec.rb @@ -70,18 +70,21 @@ expect(described_class).to be_same_field("Story Points", "STORY POINTS") end - it "matches a name carrying the counter unique_name appends" do + it "matches a stored name carrying the counter unique_name appends" do expect(described_class).to be_same_field("Story Points (2)", "Story Points") - expect(described_class).to be_same_field("Story Points", "Story Points (12)") + expect(described_class).to be_same_field("Story Points (12)", "Story Points") end - it "matches two names carrying a different counter" do - expect(described_class).to be_same_field("Story Points (2)", "Story Points (3)") + it "strips the counter from the stored name only" do + expect(described_class).not_to be_same_field("Story Points", "Story Points (2)") + end + + it "matches a Jira field whose own name ends in a number against its renamed field" do + expect(described_class).to be_same_field("Story Points (2) (2)", "Story Points (2)") end it "keeps a project key suffix apart from the counter" do expect(described_class).not_to be_same_field("Story Points (XYZ)", "Story Points") - expect(described_class).not_to be_same_field("Story Points (XYZ)", "Story Points (2)") end it "matches a project key suffix with itself despite a counter" do diff --git a/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb b/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb index 04f34cdaf5ac..63fc61004d6d 100644 --- a/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb +++ b/spec/workers/import/jira_create_project_job/jira_import_custom_field_builder_spec.rb @@ -1000,6 +1000,23 @@ def date_builder expect(date_builder.find_existing_custom_field).to be_nil end + + it "reuses the renamed field of a Jira field whose own name ends in a number" do + create(:string_wp_custom_field, name: "Due (2)") + renamed = create(:date_wp_custom_field, name: "Due (2) (2)") + schema = { "type" => "date", "custom" => "com.atlassian.jira.plugin.system.customfieldtypes:datepicker" } + builder = described_class.new(jira_field_for(name: "Due (2)", schema:), jira_import:) + + expect(builder.find_existing_custom_field).to eq(renamed) + end + + it "does not let a Jira field whose name ends in a number take over the plain field" do + create(:date_wp_custom_field, name: "Due") + schema = { "type" => "date", "custom" => "com.atlassian.jira.plugin.system.customfieldtypes:datepicker" } + builder = described_class.new(jira_field_for(name: "Due (2)", schema:), jira_import:) + + expect(builder.find_existing_custom_field).to be_nil + end end context "with a hierarchy field", with_ee: [:custom_field_hierarchies] do