diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e77963a5411..66ed402b726 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -268,10 +268,12 @@ class ApplicationController < ActionController::Base @project = @object.project end - # Filter for bulk work package operations + # 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 - @work_packages = WorkPackage.includes(:project) - .where(id: params[:work_package_id] || params[: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/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/app/models/work_package/semantic_identifier/finder_methods.rb b/app/models/work_package/semantic_identifier/finder_methods.rb index debd2a238b1..553c4e92363 100644 --- a/app/models/work_package/semantic_identifier/finder_methods.rb +++ b/app/models/work_package/semantic_identifier/finder_methods.rb @@ -105,6 +105,22 @@ 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)) + scope = scope.or(scope_for_semantic_identifier(semantic)) if semantic.any? + scope + end + private def reject_semantic_id_in_find_by!(args) diff --git a/app/services/work_packages/update_service.rb b/app/services/work_packages/update_service.rb index 4a3bc8ff84d..4a22ea5a549 100644 --- a/app/services/work_packages/update_service.rb +++ b/app/services/work_packages/update_service.rb @@ -112,7 +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. + # 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.clear_attribute_changes(%i[identifier sequence_number]) + end end def delete_relations(work_packages) diff --git a/spec/controllers/work_packages/moves_controller_spec.rb b/spec/controllers/work_packages/moves_controller_spec.rb index 26c473868bc..05952ef2294 100644 --- a/spec/controllers/work_packages/moves_controller_spec.rb +++ b/spec/controllers/work_packages/moves_controller_spec.rb @@ -116,6 +116,37 @@ 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, + 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 end 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]) 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 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