From c069e1bcb1626f10068fa53752b4768ef33325a5 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 30 Apr 2026 16:45:27 +0300 Subject: [PATCH 1/9] Resolve semantic work package id in find_work_packages Single-WP routes pass :work_package_id from the URL, which can be a semantic identifier (e.g. "PROJ-42") since commit 4dfdd6ec5db dropped numeric pins on the move and copy HAL action links. The bulk where(id:) lookup in find_work_packages only matches numeric primary keys, so /work_packages/PROJ-42/move/new returned 404. Translate :work_package_id through find_by_display_id first; the bulk :ids branch is untouched since form submissions already send numeric ids. --- app/controllers/application_controller.rb | 14 +++++++-- .../work_packages/moves_controller_spec.rb | 29 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e77963a5411..9db0726a367 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -268,10 +268,20 @@ class ApplicationController < ActionController::Base @project = @object.project end - # Filter for bulk work package operations + # Filter for bulk work package operations. + # + # :work_package_id (single-WP routes) may be semantic ("PROJ-42") and is + # resolved to a primary key first; :ids (bulk routes) already arrives numeric + # from form submissions. def find_work_packages + ids = if params[:work_package_id] + WorkPackage.find_by_display_id(params[:work_package_id])&.id + else + params[:ids] + end + @work_packages = WorkPackage.includes(:project) - .where(id: params[:work_package_id] || params[:ids]) + .where(id: ids) .order("id ASC") fail ActiveRecord::RecordNotFound if @work_packages.empty? diff --git a/spec/controllers/work_packages/moves_controller_spec.rb b/spec/controllers/work_packages/moves_controller_spec.rb index 26c473868bc..53bcfc27e92 100644 --- a/spec/controllers/work_packages/moves_controller_spec.rb +++ b/spec/controllers/work_packages/moves_controller_spec.rb @@ -116,6 +116,35 @@ RSpec.describe WorkPackages::MovesController, with_settings: { journal_aggregati expect(response).to render_template("work_packages/moves/new") end end + + describe "with a semantic work package identifier", + with_flag: { semantic_work_package_ids: true }, + with_settings: { work_packages_identifier: "semantic" } do + let(:semantic_project) { create(:project, :semantic, public: false, types: [type, type2]) } + let(:semantic_target_project) { create(:project, :semantic, public: false, types: [type, type2]) } + let(:semantic_work_package) do + create(:work_package, project: semantic_project, type:, author: user, priority:) + end + let!(:semantic_member) { create(:member, user: current_user, project: semantic_project, roles: [role]) } + let!(:semantic_target_member) { create(:member, user: current_user, project: semantic_target_project, roles: [role]) } + + it "resolves the semantic identifier and renders the new builder template" do + get "new", params: { work_package_id: semantic_work_package.display_id } + + expect(response).to render_template("work_packages/moves/new") + end + + it "resolves the semantic identifier on create and moves the work package" do + post :create, params: { + work_package_id: semantic_work_package.display_id, + new_project_id: semantic_target_project.id, + new_type_id: semantic_target_project.types.first.id + } + + expect(response).to be_redirect + expect(semantic_work_package.reload.project_id).to eq(semantic_target_project.id) + end + end end end From a54362020f741baf78d38ceb12cc653d98965e86 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 30 Apr 2026 16:56:27 +0300 Subject: [PATCH 2/9] Refresh in-memory identifier after WP move re-allocation Project#reserve_semantic_id_block! rewrites work_packages.identifier and sequence_number via a raw SQL UPDATE, leaving the in-memory records with the nil identifier set by SetAttributesService when the project changed. Callers that read the WP straight out of the ServiceResult (HAL action links, the move-and-follow redirect in WorkPackages::BulkJob#redirect_path) then saw an empty display_id and fell back to numeric URLs. Reload after the bulk allocation so the in-memory state matches the database. --- app/services/work_packages/update_service.rb | 5 +++++ .../work_packages/semantic_ids/integration_spec.rb | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/app/services/work_packages/update_service.rb b/app/services/work_packages/update_service.rb index 4a3bc8ff84d..8a6b8bcb1fb 100644 --- a/app/services/work_packages/update_service.rb +++ b/app/services/work_packages/update_service.rb @@ -113,6 +113,11 @@ class WorkPackages::UpdateService < BaseServices::Update return if work_packages.empty? work_packages.first.project.reserve_semantic_id_block!(work_packages.map(&:id)) + # reserve_semantic_id_block! writes via raw SQL UPDATE, so the in-memory + # records still carry the nil identifier left by SetAttributesService. + # Refresh them here so callers (HAL representers, redirect helpers) see + # the freshly allocated semantic id without an explicit reload. + work_packages.each(&:reload) end def delete_relations(work_packages) diff --git a/spec/services/work_packages/semantic_ids/integration_spec.rb b/spec/services/work_packages/semantic_ids/integration_spec.rb index ec603f08d84..e876a5ed7bd 100644 --- a/spec/services/work_packages/semantic_ids/integration_spec.rb +++ b/spec/services/work_packages/semantic_ids/integration_spec.rb @@ -129,6 +129,13 @@ RSpec.describe "SemanticIds registry integration", WorkPackages::UpdateService.new(user:, model: work_package).call(project: target_project) expect(WorkPackage.find_by_display_id(work_package.reload.identifier)).to eq(work_package) end + + it "refreshes the in-memory identifier so to_param produces the semantic URL" do + WorkPackages::UpdateService.new(user:, model: work_package).call(project: target_project) + + expect(work_package.identifier).to start_with("DEST-") + expect(work_package.to_param).to start_with("DEST-") + end end describe "WP move in classic mode when sequence numbers linger from semantic mode" do From e59937858cb630895c0ad241eaf96a803dee6661 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 30 Apr 2026 17:44:01 +0300 Subject: [PATCH 3/9] Add WorkPackage.where_display_id_in batch finder The plural counterpart to find_by_display_id resolves a mix of numeric and semantic identifiers in a single composite scope so callers (notably ApplicationController#find_work_packages, used by bulk WP routes) do not have to branch on the input shape or partition into numeric/semantic themselves. Mirrors scope_for_semantic_identifier in using `.or` to union the primary-key, current-identifier, and historical-alias arms; unknown values resolve to no match rather than poisoning the rest of the set. --- .../semantic_identifier/finder_methods.rb | 19 +++++ .../work_package/semantic_identifier_spec.rb | 71 +++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/app/models/work_package/semantic_identifier/finder_methods.rb b/app/models/work_package/semantic_identifier/finder_methods.rb index debd2a238b1..c450015e1fd 100644 --- a/app/models/work_package/semantic_identifier/finder_methods.rb +++ b/app/models/work_package/semantic_identifier/finder_methods.rb @@ -105,6 +105,25 @@ module WorkPackage::SemanticIdentifier::FinderMethods )) end + # Plural counterpart to find_by_display_id: returns a chainable relation that + # matches any work package whose primary key, current identifier, or + # historical alias matches one of the supplied display ids. Numeric and + # semantic strings may be freely mixed; unknown values produce no match + # rather than poisoning the rest of the set. + def where_display_id_in(values) + values = Array(values).map(&:to_s) + return none if values.empty? + + semantic, numeric = values.partition { semantic_id?(it) } + + scope = where(id: numeric.map(&:to_i)) + if semantic.any? + scope = scope.or(where(identifier: semantic)) + .or(where(id: WorkPackageSemanticAlias.where(identifier: semantic).select(:work_package_id))) + end + scope + end + private def reject_semantic_id_in_find_by!(args) diff --git a/spec/models/work_package/semantic_identifier_spec.rb b/spec/models/work_package/semantic_identifier_spec.rb index 6620e2f04aa..d6de9c83cb0 100644 --- a/spec/models/work_package/semantic_identifier_spec.rb +++ b/spec/models/work_package/semantic_identifier_spec.rb @@ -370,6 +370,77 @@ RSpec.describe WorkPackage::SemanticIdentifier do end end + describe "WorkPackage.where_display_id_in" do + let(:work_package2) { create(:work_package, project:) } + let(:work_package3) { create(:work_package, project:) } + let(:other_project) { create(:project, identifier: "OTHER") } + let(:other_wp) { create(:work_package, project: other_project) } + + before do + work_package2 + work_package3 + other_wp + end + + it "returns a chainable ActiveRecord relation" do + expect(WorkPackage.where_display_id_in(["MYPROJ-1"])).to be_a(ActiveRecord::Relation) + end + + it "returns an empty relation for an empty input" do + expect(WorkPackage.where_display_id_in([])).to be_empty + end + + it "wraps a single non-array value" do + expect(WorkPackage.where_display_id_in("MYPROJ-1")).to contain_exactly(work_package) + end + + it "resolves a single numeric string" do + expect(WorkPackage.where_display_id_in([work_package.id.to_s])).to contain_exactly(work_package) + end + + it "resolves multiple numeric strings" do + expect(WorkPackage.where_display_id_in([work_package.id.to_s, work_package2.id.to_s])) + .to contain_exactly(work_package, work_package2) + end + + it "resolves a single semantic identifier via the identifier column" do + expect(WorkPackage.where_display_id_in(["MYPROJ-1"])).to contain_exactly(work_package) + end + + it "resolves multiple semantic identifiers via the identifier column" do + expect(WorkPackage.where_display_id_in(["MYPROJ-1", "MYPROJ-2"])) + .to contain_exactly(work_package, work_package2) + end + + it "resolves a semantic identifier via the alias table for historical ids" do + WorkPackageSemanticAlias.create!(identifier: "OLDPROJ-1", work_package:) + expect(WorkPackage.where_display_id_in(["OLDPROJ-1"])).to contain_exactly(work_package) + end + + it "resolves a mix of numeric and semantic identifiers in one query" do + expect(WorkPackage.where_display_id_in([work_package.id.to_s, "MYPROJ-2", "OTHER-1"])) + .to contain_exactly(work_package, work_package2, other_wp) + end + + it "drops unknown values without poisoning the rest of the set" do + expect(WorkPackage.where_display_id_in(["MYPROJ-1", "MYPROJ-999", "ZZZ-1"])) + .to contain_exactly(work_package) + end + + it "is composable with includes and order" do + relation = WorkPackage.where_display_id_in(["MYPROJ-1", "MYPROJ-2"]) + .includes(:project) + .order(id: :asc) + expect(relation.to_a).to eq([work_package, work_package2]) + end + + it "respects upstream visibility scoping" do + member_user = create(:user, member_with_permissions: { project => [:view_work_packages] }) + expect(WorkPackage.visible(member_user).where_display_id_in(["MYPROJ-1", "OTHER-1"])) + .to contain_exactly(work_package) + end + end + describe "WorkPackage.find_by_display_id!" do context "with a semantic identifier" do it "resolves via the semantic identifier" do From a88698c2309afedebcb73981d71fbda5a0cdbf9e Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 30 Apr 2026 17:44:12 +0300 Subject: [PATCH 4/9] Use where_display_id_in in find_work_packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both :work_package_id (single-WP routes) and :ids (bulk routes) come from URLs or HTML forms and may carry semantic identifiers, so the prior numeric-only assumption on :ids was wrong. Route both inputs through WorkPackage.where_display_id_in, which returns a chainable scope that matches numeric, current-identifier, and historical-alias forms in a single query — no per-id round trip and no controller-side coupling to WorkPackageSemanticAlias. Tighten the post-move controller spec to assert the redirect URL contains the destination project's semantic prefix when follow: 1 is set, which is the path that exercises the moved WP's display_id. --- app/controllers/application_controller.rb | 18 +++++------------- .../work_packages/moves_controller_spec.rb | 4 +++- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9db0726a367..66ed402b726 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -268,20 +268,12 @@ class ApplicationController < ActionController::Base @project = @object.project end - # Filter for bulk work package operations. - # - # :work_package_id (single-WP routes) may be semantic ("PROJ-42") and is - # resolved to a primary key first; :ids (bulk routes) already arrives numeric - # from form submissions. + # Filter for bulk work package operations. Either :work_package_id (single-WP + # routes) or :ids (bulk routes) may carry numeric or semantic identifiers + # ("PROJ-42") since both originate from human-facing URLs or forms. def find_work_packages - ids = if params[:work_package_id] - WorkPackage.find_by_display_id(params[:work_package_id])&.id - else - params[:ids] - end - - @work_packages = WorkPackage.includes(:project) - .where(id: ids) + @work_packages = WorkPackage.where_display_id_in(params[:work_package_id] || params[:ids]) + .includes(:project) .order("id ASC") fail ActiveRecord::RecordNotFound if @work_packages.empty? diff --git a/spec/controllers/work_packages/moves_controller_spec.rb b/spec/controllers/work_packages/moves_controller_spec.rb index 53bcfc27e92..05952ef2294 100644 --- a/spec/controllers/work_packages/moves_controller_spec.rb +++ b/spec/controllers/work_packages/moves_controller_spec.rb @@ -138,11 +138,13 @@ RSpec.describe WorkPackages::MovesController, with_settings: { journal_aggregati post :create, params: { work_package_id: semantic_work_package.display_id, new_project_id: semantic_target_project.id, - new_type_id: semantic_target_project.types.first.id + new_type_id: semantic_target_project.types.first.id, + follow: "1" } expect(response).to be_redirect expect(semantic_work_package.reload.project_id).to eq(semantic_target_project.id) + expect(response.location).to match(%r{/work_packages/#{semantic_target_project.identifier}-\d+}) end end end From 41c2c09f9ed5cf5049bf81d8e401c3782140e8fb Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 30 Apr 2026 17:44:27 +0300 Subject: [PATCH 5/9] Return wp_id => identifier assignments from reserve_semantic_id_block! The bulk SQL UPDATE persists identifier and sequence_number directly, but callers holding live records have no cheap way to learn what was written without reloading. Returning the {wp_id => "PROJ-N"} mapping lets callers (next commit: WorkPackages::UpdateService) refresh in-memory state without N round trips. Empty input returns an empty hash for symmetric Hash-typed callers. --- app/models/projects/semantic_identifier.rb | 7 ++++++- spec/models/projects/semantic_identifier_spec.rb | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/models/projects/semantic_identifier.rb b/app/models/projects/semantic_identifier.rb index 68d80e53196..ca0726951b9 100644 --- a/app/models/projects/semantic_identifier.rb +++ b/app/models/projects/semantic_identifier.rb @@ -65,12 +65,15 @@ module Projects::SemanticIdentifier # and (by default) inserts alias rows for every historical slug prefix of # this project. # + # Returns a hash of { work_package_id => semantic_identifier } so callers + # already holding live records can refresh in-memory state without reloading. + # # Pass insert_aliases: false when the caller will run seed_alias_table # immediately after (e.g. the conversion backfill path), to avoid # duplicating the alias insertion work. def reserve_semantic_id_block!(work_package_ids, insert_aliases: true) count = work_package_ids.size - return if count.zero? + return {} if count.zero? range = allocate_sequence_range!(count) sorted_ids = work_package_ids.sort @@ -79,6 +82,8 @@ module Projects::SemanticIdentifier bulk_assign_sequence_numbers!(sorted_ids, range) insert_sequence_aliases!(sorted_ids, range) if insert_aliases end + + sorted_ids.zip(range).to_h { |wp_id, seq| [wp_id, "#{identifier}-#{seq}"] } end # Called after this project's identifier is renamed. Atomically: diff --git a/spec/models/projects/semantic_identifier_spec.rb b/spec/models/projects/semantic_identifier_spec.rb index 9370185c683..bf54b49821c 100644 --- a/spec/models/projects/semantic_identifier_spec.rb +++ b/spec/models/projects/semantic_identifier_spec.rb @@ -80,6 +80,10 @@ RSpec.describe Projects::SemanticIdentifier, with_settings: { work_packages_iden project.reserve_semantic_id_block!([]) expect(project.reload.wp_sequence_counter).to eq(before_count) end + + it "returns an empty hash" do + expect(project.reserve_semantic_id_block!([])).to eq({}) + end end context "with work package ids" do @@ -117,6 +121,17 @@ RSpec.describe Projects::SemanticIdentifier, with_settings: { work_packages_iden expect(wp2.reload.sequence_number).to eq(12) end + it "returns the wp_id => identifier assignments produced by the allocation" do + result = project.reserve_semantic_id_block!([wp1.id, wp2.id, wp3.id]) + expect(result).to eq(wp1.id => "PROJ-1", wp2.id => "PROJ-2", wp3.id => "PROJ-3") + end + + it "pairs ids with sequence numbers in ascending wp-id order regardless of input order" do + result = project.reserve_semantic_id_block!([wp3.id, wp1.id, wp2.id]) + sorted_ids = [wp1.id, wp2.id, wp3.id] + expect(result).to eq(sorted_ids[0] => "PROJ-1", sorted_ids[1] => "PROJ-2", sorted_ids[2] => "PROJ-3") + end + context "when insert_aliases: true (default)" do it "creates alias rows for each slug prefix" do project.reserve_semantic_id_block!([wp1.id, wp2.id]) From 8338a8158a8664edfb4b9fc7249815902eb3249f Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 30 Apr 2026 17:44:38 +0300 Subject: [PATCH 6/9] Apply WP identifier assignments in-memory after move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous fix reloaded each moved work package after the bulk SQL UPDATE so HAL representers and the move-and-follow redirect saw the freshly allocated semantic id. That cost one round trip per WP, which matters when a parent moves with descendants. Now that reserve_semantic_id_block! returns the {wp_id => identifier} assignments directly, apply them with assign_attributes + changes_applied — same observable result, zero extra queries regardless of bulk size. --- app/services/work_packages/update_service.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/services/work_packages/update_service.rb b/app/services/work_packages/update_service.rb index 8a6b8bcb1fb..3a3840f63ad 100644 --- a/app/services/work_packages/update_service.rb +++ b/app/services/work_packages/update_service.rb @@ -112,12 +112,17 @@ class WorkPackages::UpdateService < BaseServices::Update def update_semantic_ids(work_packages) return if work_packages.empty? - work_packages.first.project.reserve_semantic_id_block!(work_packages.map(&:id)) # reserve_semantic_id_block! writes via raw SQL UPDATE, so the in-memory # records still carry the nil identifier left by SetAttributesService. - # Refresh them here so callers (HAL representers, redirect helpers) see - # the freshly allocated semantic id without an explicit reload. - work_packages.each(&:reload) + # Apply the returned assignments in-memory so callers (HAL representers, + # redirect helpers) see the freshly allocated semantic id without N reloads. + assignments = work_packages.first.project.reserve_semantic_id_block!(work_packages.map(&:id)) + work_packages.each do |wp| + next unless (identifier = assignments[wp.id]) + + wp.assign_attributes(identifier:, sequence_number: identifier.split("-").last.to_i) + wp.changes_applied + end end def delete_relations(work_packages) From 7a868d2c4b06b7609233ef0b6954241981d71d6e Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 30 Apr 2026 17:48:31 +0300 Subject: [PATCH 7/9] Reuse scope_for_semantic_identifier in where_display_id_in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The semantic arm reimplemented the union of the identifier column and the alias-table lookup that scope_for_semantic_identifier already encapsulates. Both rely on `where(identifier:)` semantics that accept arrays as IN clauses, so the scalar helper composes correctly for the plural case. Keeps the SQL shape of "match by display id" defined in one place — if the correlated EXISTS ever needs to become a semi-join under bulk pressure, both callers benefit. --- .../work_package/semantic_identifier/finder_methods.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/models/work_package/semantic_identifier/finder_methods.rb b/app/models/work_package/semantic_identifier/finder_methods.rb index c450015e1fd..553c4e92363 100644 --- a/app/models/work_package/semantic_identifier/finder_methods.rb +++ b/app/models/work_package/semantic_identifier/finder_methods.rb @@ -117,10 +117,7 @@ module WorkPackage::SemanticIdentifier::FinderMethods semantic, numeric = values.partition { semantic_id?(it) } scope = where(id: numeric.map(&:to_i)) - if semantic.any? - scope = scope.or(where(identifier: semantic)) - .or(where(id: WorkPackageSemanticAlias.where(identifier: semantic).select(:work_package_id))) - end + scope = scope.or(scope_for_semantic_identifier(semantic)) if semantic.any? scope end From 8602aff8386d6540e1130de276a2961c25154689 Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Thu, 30 Apr 2026 18:14:24 +0300 Subject: [PATCH 8/9] Use clear_changes_information after raw-SQL identifier assignment clear_changes_information is the established pattern in this codebase for syncing in-memory dirty state after a side-channel persist (queries, historic-attribute eager loading, progress/date-picker controllers). changes_applied has the same effect but no other call site, so prefer the local convention. --- app/services/work_packages/update_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/work_packages/update_service.rb b/app/services/work_packages/update_service.rb index 3a3840f63ad..4b9c34625d4 100644 --- a/app/services/work_packages/update_service.rb +++ b/app/services/work_packages/update_service.rb @@ -121,7 +121,7 @@ class WorkPackages::UpdateService < BaseServices::Update next unless (identifier = assignments[wp.id]) wp.assign_attributes(identifier:, sequence_number: identifier.split("-").last.to_i) - wp.changes_applied + wp.clear_changes_information end end From 59a631e983b6926e7f64b9a7b373d1fe34e442aa Mon Sep 17 00:00:00 2001 From: Kabiru Mwenja Date: Tue, 5 May 2026 15:55:28 +0300 Subject: [PATCH 9/9] Scope dirty-state reset to identifier and sequence_number clear_changes_information would also clobber any unrelated dirty attributes the work package happened to be carrying at this point. The intent is narrower: only the two attributes we just synced from the raw-SQL UPDATE should be marked clean. Use clear_attribute_changes with the explicit attribute list instead. --- app/services/work_packages/update_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/work_packages/update_service.rb b/app/services/work_packages/update_service.rb index 4b9c34625d4..4a22ea5a549 100644 --- a/app/services/work_packages/update_service.rb +++ b/app/services/work_packages/update_service.rb @@ -121,7 +121,7 @@ class WorkPackages::UpdateService < BaseServices::Update next unless (identifier = assignments[wp.id]) wp.assign_attributes(identifier:, sequence_number: identifier.split("-").last.to_i) - wp.clear_changes_information + wp.clear_attribute_changes(%i[identifier sequence_number]) end end