Merge pull request #22718 from opf/feature/73756-adapt-routes-for-project-based-semantic-work-package-identifiers

Make find/exists? resolve semantic work package identifiers
This commit is contained in:
Kabiru Mwenja
2026-04-21 13:13:54 +03:00
committed by GitHub
21 changed files with 636 additions and 123 deletions
+2 -2
View File
@@ -170,8 +170,8 @@ Rails/ContentTag:
Rails/DynamicFindBy:
Enabled: true
AllowedMethods:
- find_by_id_or_identifier
- find_by_id_or_identifier!
- find_by_display_id
- find_by_display_id!
- find_by_semantic_identifier
Exclude:
- "spec/features/**/*.rb"
+1
View File
@@ -142,6 +142,7 @@ cd frontend && npm test && cd ..
- Keep controllers thin, models focused
- Document with [YARD](https://yardoc.org/)
- Write RSpec tests for all new features
- **Work package identifiers**: `WorkPackage.find("PROJ-42")` resolves semantic identifiers transparently. Use `find_by_display_id` only when input could legitimately be numeric OR semantic (controllers, URL-driven components, macro resolvers). Low-level code (queries, filters, services) should stick to `find_by(id:)` with primary keys. See `app/models/work_package/semantic_identifier/finder_methods.rb`.
### JavaScript/TypeScript
- **New development**: Use Hotwire (Turbo + Stimulus) with server-rendered HTML
@@ -13,7 +13,7 @@ module WorkPackages
@id = id
@tab = tab
@work_package = WorkPackage.visible.find_by(id:)
@work_package = WorkPackage.visible.find_by_display_id(id)
end
def wrapper_uniq_by
@@ -35,7 +35,7 @@ class WorkPackages::HoverCardComponent < ApplicationComponent
super
@id = id
@work_package = WorkPackage.visible.find_by(id:)
@work_package = WorkPackage.visible.find_by_display_id(id)
@assignee = @work_package.present? ? @work_package.assigned_to : nil
end
@@ -39,7 +39,7 @@ class WorkPackages::SplitViewComponent < ApplicationComponent
@id = id
@tab = tab
@work_package = WorkPackage.visible.find_by(id:)
@work_package = WorkPackage.visible.find_by_display_id(id)
@base_route = base_route
end
+1 -1
View File
@@ -243,7 +243,7 @@ class WorkPackagesController < ApplicationController
def work_package
return @work_package if defined?(@work_package)
@work_package = WorkPackage.visible(current_user).find_by(id: params[:id])
@work_package = WorkPackage.visible(current_user).find_by_display_id(params[:id])
end
def journals
@@ -120,7 +120,7 @@ module WorkPackage::Exports
return resolve_label_work_package(attribute) if type == "label"
return msg_macro_error(I18n.t("export.macro.model_not_found", model: type)) unless type == "value"
work_package = WorkPackage.visible(user).find_by(id:)
work_package = WorkPackage.visible(user).find_by_display_id(id)
if work_package.nil?
return msg_macro_error(I18n.t("export.macro.resource_not_found", resource: "#{WorkPackage.name} #{id}"))
end
+21 -30
View File
@@ -31,6 +31,18 @@
module WorkPackage::SemanticIdentifier
extend ActiveSupport::Concern
# Matches either a numeric ID ("12345") or a semantic identifier ("PROJ-42").
# Used in Rails route constraints so both forms are accepted in URLs.
# The frontend equivalent lives in WP_ID_URL_PATTERN (work-package-id-pattern.ts).
ID_ROUTE_CONSTRAINT = /\d+|[A-Z][A-Z0-9_]*-\d+/
# Raised when a finder is invoked in a way that cannot resolve a semantic
# identifier — e.g. find_by(id: "PROJ-42") which reduces to a raw SQL
# WHERE clause that cannot consult the alias table. Subclasses ArgumentError
# so callers that rescue ArgumentError still catch it, but it can be rescued
# specifically when needed.
class UnsupportedLookup < ArgumentError; end
included do
has_many :semantic_aliases,
class_name: "WorkPackageSemanticAlias",
@@ -42,37 +54,16 @@ module WorkPackage::SemanticIdentifier
end
class_methods do
def semantic_id?(identifier)
identifier.to_s.to_i.to_s != identifier.to_s
end
include FinderMethods
# Resolves any identifier form to a WorkPackage.
# - Numeric string ("12345") → find by primary key
# - Semantic string ("PROJ-42") → lookup via work_packages table and alias table
#
# Returns nil on miss.
def find_by_id_or_identifier(identifier)
return find_by(id: identifier) unless semantic_id?(identifier)
find_by_semantic_identifier(identifier)
end
# Same as find_by_id_or_identifier but raises ActiveRecord::RecordNotFound on miss.
def find_by_id_or_identifier!(identifier)
find_by_id_or_identifier(identifier) || raise(ActiveRecord::RecordNotFound, "WorkPackage not found: #{identifier}")
end
private
def find_by_semantic_identifier(identifier)
wp = find_by(identifier:)
return wp if wp
# Fallback: alias table lookup. The table holds every identifier a WP has ever been known by:
# Done via a single join to:
# * Respect any parent scoping (e.g. when called as WorkPackage.visible.find_by_semantic_identifier)
# * Reduce lookup to a single DB round trip
joins(:semantic_aliases).find_by(work_package_semantic_aliases: { identifier: })
# Extend every relation built from this model with semantic finder methods,
# so that WorkPackage.visible(user).find("PROJ-42") and
# project.work_packages.find_by_display_id("PROJ-42") both work. Overriding
# `relation` is the seam that reaches every scope and association proxy;
# including FinderMethods into class_methods alone only covers class-level
# calls like WorkPackage.find.
def relation
super.extending(FinderMethods)
end
end
@@ -0,0 +1,184 @@
# frozen_string_literal: true
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 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 COPYRIGHT and LICENSE files for more details.
#++
# Extends ActiveRecord finder methods to support semantic work package
# identifiers (e.g. "PROJ-42") in addition to numeric IDs.
#
# - find("PROJ-42") resolves transparently
# - find_by(id:)/find_by!(id:) raise UnsupportedLookup for semantic strings
# - find_by_display_id("PROJ-42") is the explicit nil-on-miss resolver
# - exists?("PROJ-42") resolves transparently
#
# The asymmetry between find (transparent) and find_by (guarded) is deliberate:
# controllers and URL-driven callers already pass user input into find, and
# losing semantic resolution there would break the feature. find_by on the other
# hand reduces to a raw SQL WHERE id = ? that cannot consult the alias table,
# so silently matching nothing would be a worse bug than raising.
#
# Convention: use find_by_display_id only when the input could legitimately be
# either numeric or semantic (controllers, view components fed from URL params,
# macro resolvers). Low-level code (queries, filters, services) should stick to
# find_by(id:) with primary keys.
#
# Included into WorkPackage class methods and extended into every
# ActiveRecord::Relation via WorkPackage::SemanticIdentifier.
module WorkPackage::SemanticIdentifier::FinderMethods
def find(*args)
if args.length == 1 && !args.first.is_a?(Array)
return semantic_id?(args.first) ? find_by_display_id!(args.first) : super
end
ids = args.first.is_a?(Array) ? args.first : args
if ids.any? { |id| semantic_id?(id) }
raise WorkPackage::SemanticIdentifier::UnsupportedLookup,
"Semantic identifiers in multi-argument find are not supported. " \
"Use primary keys for multi-argument lookup, or resolve each identifier " \
"individually via find_by_display_id! (raises) or find_by_display_id (nil on miss)."
end
super
end
# Guard find_by against semantic identifiers passed via `id:` or `identifier:`.
# Developers should use find("PROJ-42") or find_by_display_id("PROJ-42") instead.
def find_by(*args)
reject_semantic_id_in_find_by!(args)
super
end
def find_by!(*args)
reject_semantic_id_in_find_by!(args)
super
end
def exists?(conditions = :none)
return super unless semantic_id?(conditions)
exists_by_semantic_identifier?(conditions)
end
# Resolves any display-facing identifier to a WorkPackage.
# - Numeric string ("12345") → find by primary key
# - Semantic string ("PROJ-42") → lookup via identifier column and alias table
#
# Returns nil on miss.
def find_by_display_id(identifier)
if semantic_id?(identifier)
find_by_semantic_identifier(identifier)
else
where(id: identifier).take # rubocop:disable Rails/FindBy -- avoid find_by, it would rerun semantic_id?
end
end
# Same as find_by_display_id but raises ActiveRecord::RecordNotFound on miss.
def find_by_display_id!(identifier)
find_by_display_id(identifier) ||
raise(ActiveRecord::RecordNotFound.new(
"Couldn't find WorkPackage with identifier=#{identifier}", "WorkPackage", "identifier", identifier
))
end
private
def reject_semantic_id_in_find_by!(args)
return unless args.length == 1 && args.first.is_a?(Hash)
pair = id_or_identifier_pair(args.first)
return unless pair
key, value = pair
offending = first_semantic_value(value)
return unless offending
raise WorkPackage::SemanticIdentifier::UnsupportedLookup,
"find_by(#{key}: #{value.inspect}) does not support semantic identifiers " \
"because it cannot resolve aliases or match across identifier history. " \
"Use find(#{offending.inspect}) or find_by_display_id(#{offending.inspect}) instead."
end
def id_or_identifier_pair(hash)
(hash.assoc(:id) || hash.assoc("id")) ||
(hash.assoc(:identifier) || hash.assoc("identifier"))
end
def first_semantic_value(value)
if value.is_a?(Array)
value.detect { |v| semantic_id?(v) }
elsif semantic_id?(value)
value
end
end
# Returns true when value looks like a semantic work package identifier (e.g. "PROJ-42").
# Non-string values (Integer, Hash, nil, Array) and numeric strings ("123", " 456 ")
# return false — these fall through to standard ActiveRecord lookup.
def semantic_id?(value)
return false unless value.is_a?(String)
stripped = value.strip
stripped.to_i.to_s != stripped
end
def find_by_semantic_identifier(identifier)
scope_for_semantic_identifier(identifier).first
end
def exists_by_semantic_identifier?(identifier)
scope_for_semantic_identifier(identifier).exists?
end
# Builds a scope that matches work packages by semantic identifier, considering
# both the current identifier column and the alias table for historical identifiers.
#
# Generates:
#
# SELECT "work_packages".* FROM "work_packages"
# WHERE ("work_packages"."identifier" = 'PROJ-42'
# OR EXISTS (
# SELECT 1 FROM "work_package_semantic_aliases"
# WHERE "work_package_semantic_aliases"."work_package_id" = "work_packages"."id"
# AND "work_package_semantic_aliases"."identifier" = 'PROJ-42'
# ))
def scope_for_semantic_identifier(identifier)
where(identifier:).or(where(semantic_alias_exists(identifier)))
end
# Correlated EXISTS subquery that matches work packages having a
# semantic alias row with the given identifier.
def semantic_alias_exists(identifier)
alias_table = WorkPackageSemanticAlias.arel_table
WorkPackageSemanticAlias
.where(alias_table[:work_package_id].eq(arel_table[:id]))
.where(identifier:)
.arel
.exists
end
end
+1 -1
View File
@@ -37,7 +37,7 @@ module McpResources
default_description "Access work packages of this OpenProject instance."
def read(id:)
work_package = ::WorkPackage.visible.find_by(id:)
work_package = ::WorkPackage.visible.find_by_display_id(id)
return nil if work_package.nil?
API::V3::WorkPackages::WorkPackageRepresenter.create(work_package, current_user:, embed_links: true)
+1 -1
View File
@@ -40,5 +40,5 @@ See COPYRIGHT and LICENSE files for more details.
<% end %>
<% content_for :content_body do %>
<%= render WorkPackages::FullView::ShowComponent.new(id: work_package.id, tab: params[:tab] || "activity") %>
<%= render WorkPackages::FullView::ShowComponent.new(id: work_package.display_id, tab: params[:tab] || "activity") %>
<% end %>
+3 -2
View File
@@ -442,7 +442,7 @@ Rails.application.routes.draw do
get "/new" => "work_packages#new", on: :collection, as: "new"
get "(/:tab)" => "work_packages#show", on: :member, as: "",
constraints: { id: /\d+/, state: /(?!(shares|copy|dialog)).+/ }
constraints: { id: WorkPackage::SemanticIdentifier::ID_ROUTE_CONSTRAINT, state: /(?!(shares|copy|dialog)).+/ }
# states managed by client-side routing on work_package#index
get "(/*state)" => "work_packages#index", on: :collection, as: "", constraints: { state: /(?!(dialog|new)).+/ }
@@ -935,7 +935,8 @@ Rails.application.routes.draw do
on: :member
get "/copy" => "work_packages#copy", on: :member, as: "copy"
get "(/:tab)" => "work_packages#show", on: :member, as: "", constraints: { id: /\d+/, state: /(?!(shares|new|copy)).+/ }
get "(/:tab)" => "work_packages#show", on: :member, as: "",
constraints: { id: WorkPackage::SemanticIdentifier::ID_ROUTE_CONSTRAINT, state: /(?!(shares|new|copy)).+/ }
# states managed by client-side (angular) routing on work_package#show
get "/" => "work_packages#index", on: :collection, as: "index"
@@ -63,7 +63,7 @@ module API
})
.mount)
route_param :id, type: Integer, desc: "Work package ID" do
route_param :id, type: String, desc: "Work package ID or semantic identifier (e.g. PROJ-42)" do
helpers WorkPackagesSharedHelpers
helpers do
@@ -691,7 +691,7 @@ RSpec.describe CostlogController do
before do
grant_current_user_permissions user, %i[view_project view_work_packages view_cost_entries edit_cost_entries]
params["cost_entry"]["entity_id"] = "this-id-does-not-exist"
params["cost_entry"]["entity_id"] = "999999999"
end
it_behaves_like "invalid update"
@@ -37,14 +37,8 @@ RSpec.describe WorkPackagesController do
let(:project) { create(:project, identifier: "test_project", public: false) }
let(:other_project) { build_stubbed(:project) }
let(:stub_project) { build_stubbed(:project, identifier: "test_project", public: false) }
let(:type) { build_stubbed(:type) }
let(:stub_work_package) do
build_stubbed(:work_package,
id: 1337,
type:,
project: stub_project)
end
let(:type) { create(:type) }
let(:work_package) { create(:work_package, type:, project:) }
let(:current_user) { create(:user) }
@@ -63,11 +57,10 @@ RSpec.describe WorkPackagesController do
describe "with the permission to see the project " \
"with having the necessary permissions" do
let(:role) { create(:project_role, permissions: [:view_work_packages]) }
before do
expect(WorkPackage).to receive_message_chain("visible.find_by").and_return(stub_work_package)
mock_permissions_for(current_user) do |mock|
mock.allow_in_project :view_work_packages, project: stub_work_package.project
end
create(:member, project: work_package.project, principal: current_user, roles: [role])
end
instance_eval(&)
@@ -261,19 +254,34 @@ RSpec.describe WorkPackagesController do
end
describe "show.html" do
let(:call_action) { get("show", params: { id: "1337" }) }
let(:call_action) { get("show", params: { id: work_package.id.to_s }) }
requires_permission_in_project do
it "redirects to the full url" do
call_action
expect(response).to redirect_to("/projects/test_project/work_packages/1337/activity")
expect(response).to redirect_to("/projects/test_project/work_packages/#{work_package.id}/activity")
end
end
end
describe "show.html with semantic identifier",
with_flag: { semantic_work_package_ids: true },
with_settings: { work_packages_identifier: "semantic" } do
let(:project) { create(:project, identifier: "TESTPROJ") }
let(:call_action) { get("show", params: { id: work_package.display_id.to_s }) }
requires_permission_in_project do
it "resolves the semantic identifier and redirects to the full url" do
call_action
expect(response).to redirect_to("/projects/TESTPROJ/work_packages/#{work_package.display_id}/activity")
end
end
end
describe "show.pdf" do
let(:call_action) { get("show", params: { format: "pdf", id: "1337" }) }
let(:call_action) { get("show", params: { format: "pdf", id: work_package.id.to_s }) }
let(:exporter) { WorkPackage::PDFExport::WorkPackageToPdf }
let(:exporter_instance) { instance_double(exporter) }
@@ -286,8 +294,8 @@ RSpec.describe WorkPackagesController do
pdf_data = "foobar"
time = DateTime.new(2023, 6, 30, 23, 59)
allow(DateTime).to receive(:now).and_return(time)
expected_name = [stub_work_package.project.identifier, "##{stub_work_package.id}",
stub_work_package.subject, "2023-06-30_23-59"].join("_").tr(" ", "-")
expected_name = [work_package.project.identifier, "##{work_package.id}",
work_package.subject, "2023-06-30_23-59"].join("_").tr(" ", "-")
expected_type = "application/pdf"
pdf_result = double("pdf_result",
error?: false,
@@ -309,7 +317,7 @@ RSpec.describe WorkPackagesController do
end
describe "show.atom" do
let(:call_action) { get("show", params: { format: "atom", id: "1337" }) }
let(:call_action) { get("show", params: { format: "atom", id: work_package.id.to_s }) }
requires_permission_in_project do
it "render the journal/index template" do
@@ -137,4 +137,65 @@ RSpec.describe Projects::SemanticIdentifier, with_settings: { work_packages_iden
end
end
end
describe "relation-scoped finder methods" do
let(:project) { create(:project, identifier: "PROJ", wp_sequence_counter: 0) }
let(:other_project) { create(:project, identifier: "OTHER", wp_sequence_counter: 0) }
let!(:wp1) { create(:work_package, project:) }
let!(:wp2) { create(:work_package, project: other_project) }
describe "project.work_packages.find" do
it "resolves a semantic identifier scoped to the project" do
expect(project.work_packages.find("PROJ-1")).to eq(wp1)
end
it "raises RecordNotFound for a WP belonging to another project" do
expect { project.work_packages.find("OTHER-1") }
.to raise_error(ActiveRecord::RecordNotFound)
end
it "raises RecordNotFound for unknown semantic id" do
expect { project.work_packages.find("PROJ-999") }
.to raise_error(ActiveRecord::RecordNotFound)
end
it "resolves via historic alias" do
WorkPackageSemanticAlias.create!(identifier: "OLDPROJ-1", work_package: wp1)
expect(project.work_packages.find("OLDPROJ-1")).to eq(wp1)
end
end
describe "project.work_packages.exists?" do
it "returns true for a semantic identifier within the project" do
expect(project.work_packages.exists?("PROJ-1")).to be true
end
it "returns false for a WP belonging to another project" do
expect(project.work_packages.exists?("OTHER-1")).to be false
end
it "returns false for unknown semantic id" do
expect(project.work_packages.exists?("PROJ-999")).to be false
end
it "checks the alias table for historical identifiers" do
WorkPackageSemanticAlias.create!(identifier: "OLDPROJ-1", work_package: wp1)
expect(project.work_packages.exists?("OLDPROJ-1")).to be true
end
end
describe "project.work_packages.find_by_display_id" do
it "resolves a semantic identifier scoped to the project" do
expect(project.work_packages.find_by_display_id("PROJ-1")).to eq(wp1)
end
it "returns nil for a WP belonging to another project" do
expect(project.work_packages.find_by_display_id("OTHER-1")).to be_nil
end
it "returns nil for unknown semantic id" do
expect(project.work_packages.find_by_display_id("PROJ-999")).to be_nil
end
end
end
end
@@ -60,50 +60,70 @@ RSpec.describe WorkPackage::SemanticIdentifier do
end
end
describe "WorkPackage.find_by_id_or_identifier" do
context "with a numeric param" do
describe "WorkPackage.find" do
context "with a numeric id" do
it "finds by primary key" do
expect(WorkPackage.find_by_id_or_identifier(work_package.id.to_s)).to eq(work_package)
expect(WorkPackage.find(work_package.id)).to eq(work_package)
end
it "returns nil for unknown id" do
expect(WorkPackage.find_by_id_or_identifier("9999999")).to be_nil
it "raises RecordNotFound for unknown numeric id" do
expect { WorkPackage.find(9_999_999) }.to raise_error(ActiveRecord::RecordNotFound)
end
end
context "with a semantic param" do
context "when the identifier matches work_packages.identifier (fast path)" do
it "finds directly via identifier without hitting the alias table" do
expect(WorkPackage.find_by_id_or_identifier("MYPROJ-1")).to eq(work_package)
end
it "returns nil when no WP has that identifier and no alias or fallback matches" do
expect(WorkPackage.find_by_id_or_identifier("MYPROJ-999")).to be_nil
end
context "with a numeric string" do
it "falls through to standard AR find" do
expect(WorkPackage.find(work_package.id.to_s)).to eq(work_package)
end
context "when the identifier is a historic alias (alias table path)" do
it "resolves historic entries via the alias registry" do
WorkPackageSemanticAlias.create!(identifier: "OLDPROJ-1", work_package:)
expect(WorkPackage.find_by_id_or_identifier("OLDPROJ-1")).to eq(work_package)
end
it "strips whitespace before dispatching" do
expect(WorkPackage.find(" #{work_package.id} ")).to eq(work_package)
end
end
it "resolves when identifier differs but an alias row exists" do
work_package.update_columns(identifier: "OTHER-99")
expect(WorkPackage.find_by_id_or_identifier("MYPROJ-1")).to eq(work_package)
end
context "with a semantic identifier string" do
it "resolves via the semantic identifier" do
expect(WorkPackage.find("MYPROJ-1")).to eq(work_package)
end
it "returns nil for unknown sequence" do
expect(WorkPackage.find_by_id_or_identifier("MYPROJ-999")).to be_nil
it "raises RecordNotFound for unknown semantic id" do
expect { WorkPackage.find("MYPROJ-999") }.to raise_error(ActiveRecord::RecordNotFound)
end
it "returns nil for unknown project prefix" do
expect(WorkPackage.find_by_id_or_identifier("NOPE-1")).to be_nil
it "resolves via historic alias" do
WorkPackageSemanticAlias.create!(identifier: "OLDPROJ-1", work_package:)
expect(WorkPackage.find("OLDPROJ-1")).to eq(work_package)
end
end
context "with multiple ids" do
let(:work_package2) { create(:work_package, project:) }
it "delegates to standard AR find for an array of numeric ids" do
expect(WorkPackage.find([work_package.id, work_package2.id])).to contain_exactly(work_package, work_package2)
end
it "returns nil for an unparseable string" do
expect(WorkPackage.find_by_id_or_identifier("not-an-identifier!")).to be_nil
it "delegates to standard AR find for multiple numeric id arguments" do
expect(WorkPackage.find(work_package.id, work_package2.id)).to contain_exactly(work_package, work_package2)
end
it "raises UnsupportedLookup for a single-element array with a semantic id" do
expect { WorkPackage.find(["MYPROJ-1"]) }
.to raise_error(WorkPackage::SemanticIdentifier::UnsupportedLookup, /primary keys for multi-argument/)
end
it "raises UnsupportedLookup for multiple semantic ids" do
expect { WorkPackage.find("MYPROJ-1", "MYPROJ-2") }
.to raise_error(WorkPackage::SemanticIdentifier::UnsupportedLookup, /primary keys for multi-argument/)
end
it "raises UnsupportedLookup for mixed numeric and semantic ids" do
expect { WorkPackage.find([work_package.id, "MYPROJ-2"]) }
.to raise_error(WorkPackage::SemanticIdentifier::UnsupportedLookup, /primary keys for multi-argument/)
end
it "is rescuable as ArgumentError for backwards compatibility" do
expect { WorkPackage.find("MYPROJ-1", "MYPROJ-2") }.to raise_error(ArgumentError)
end
end
@@ -111,28 +131,231 @@ RSpec.describe WorkPackage::SemanticIdentifier do
let(:member_user) { create(:user, member_with_permissions: { project => [:view_work_packages] }) }
let(:non_member_user) { create(:user) }
it "returns the WP for a user who can see it" do
expect(WorkPackage.visible(member_user).find_by_id_or_identifier("MYPROJ-1")).to eq(work_package)
it "respects the scope for semantic ids" do
expect(WorkPackage.visible(member_user).find("MYPROJ-1")).to eq(work_package)
end
it "returns nil for a user who cannot see it" do
expect(WorkPackage.visible(non_member_user).find_by_id_or_identifier("MYPROJ-1")).to be_nil
end
it "also scopes numeric lookup" do
expect(WorkPackage.visible(non_member_user).find_by_id_or_identifier(work_package.id.to_s)).to be_nil
it "raises RecordNotFound when the user cannot see it" do
expect { WorkPackage.visible(non_member_user).find("MYPROJ-1") }
.to raise_error(ActiveRecord::RecordNotFound)
end
end
end
describe "WorkPackage.find_by_id_or_identifier!" do
it "returns the work package when found" do
expect(WorkPackage.find_by_id_or_identifier!(work_package.id.to_s)).to eq(work_package)
describe "WorkPackage.exists?" do
context "with a numeric id" do
it "returns true for existing record" do
expect(WorkPackage.exists?(work_package.id)).to be true
end
it "returns false for non-existing record" do
expect(WorkPackage.exists?(9_999_999)).to be false
end
end
it "raises ActiveRecord::RecordNotFound when not found" do
expect { WorkPackage.find_by_id_or_identifier!("MYPROJ-999") }
.to raise_error(ActiveRecord::RecordNotFound)
context "with a numeric string" do
it "falls through to standard AR exists?" do
expect(WorkPackage.exists?(work_package.id.to_s)).to be true
end
end
context "with a semantic identifier string" do
it "checks the identifier column" do
expect(WorkPackage.exists?("MYPROJ-1")).to be true
end
it "returns false for unknown semantic id" do
expect(WorkPackage.exists?("MYPROJ-999")).to be false
end
it "checks the alias table for historical identifiers" do
WorkPackageSemanticAlias.create!(identifier: "OLDPROJ-1", work_package:)
expect(WorkPackage.exists?("OLDPROJ-1")).to be true
end
end
context "with hash conditions" do
it "passes through to standard AR exists?" do
expect(WorkPackage.exists?(subject: work_package.subject)).to be true
end
end
context "with visibility scoping" do
let(:member_user) { create(:user, member_with_permissions: { project => [:view_work_packages] }) }
let(:non_member_user) { create(:user) }
it "respects the scope for semantic ids" do
expect(WorkPackage.visible(member_user).exists?("MYPROJ-1")).to be true
end
it "returns false when the user cannot see it" do
expect(WorkPackage.visible(non_member_user).exists?("MYPROJ-1")).to be false
end
it "respects the scope for historical aliases" do
WorkPackageSemanticAlias.create!(identifier: "OLDPROJ-1", work_package:)
expect(WorkPackage.visible(member_user).exists?("OLDPROJ-1")).to be true
expect(WorkPackage.visible(non_member_user).exists?("OLDPROJ-1")).to be false
end
end
end
describe "WorkPackage.find_by" do
context "with id: keyword and semantic identifier" do
it "raises UnsupportedLookup (an ArgumentError subclass) pointing to find_by_display_id" do
expect { WorkPackage.find_by(id: "MYPROJ-1") }
.to raise_error(WorkPackage::SemanticIdentifier::UnsupportedLookup, /find_by_display_id/)
end
it "is rescuable as ArgumentError for backwards compatibility" do
expect { WorkPackage.find_by(id: "MYPROJ-1") }.to raise_error(ArgumentError)
end
end
context "with identifier: keyword and semantic identifier" do
it "raises ArgumentError pointing to find_by_display_id" do
expect { WorkPackage.find_by(identifier: "MYPROJ-1") }
.to raise_error(ArgumentError, /find_by_display_id/)
end
end
context "with string-keyed hash (AR internal representation)" do
it "raises ArgumentError for string 'id' key with semantic value" do
expect { WorkPackage.find_by("id" => "MYPROJ-1") }
.to raise_error(ArgumentError, /find_by_display_id/)
end
it "raises ArgumentError for string 'identifier' key with semantic value" do
expect { WorkPackage.find_by("identifier" => "MYPROJ-1") }
.to raise_error(ArgumentError, /find_by_display_id/)
end
end
context "with id: keyword and numeric string" do
it "falls through to standard AR find_by" do
expect(WorkPackage.find_by(id: work_package.id.to_s)).to eq(work_package)
end
end
context "with id: keyword and an array" do
let(:work_package2) { create(:work_package, project:) }
it "falls through to standard AR find_by for an all-numeric array" do
expect(WorkPackage.find_by(id: [work_package.id, work_package2.id])).to eq(work_package)
end
it "raises UnsupportedLookup when the array contains a semantic identifier" do
expect { WorkPackage.find_by(id: [work_package.id, "MYPROJ-2"]) }
.to raise_error(WorkPackage::SemanticIdentifier::UnsupportedLookup, /does not support semantic identifiers/)
end
end
context "with non-id keywords" do
it "passes through to standard AR find_by" do
expect(WorkPackage.find_by(subject: work_package.subject)).to eq(work_package)
end
end
context "with multiple keywords including id:" do
it "passes through to standard AR find_by" do
expect(WorkPackage.find_by(id: work_package.id, project:)).to eq(work_package)
end
it "raises UnsupportedLookup when id: is semantic even among other keywords" do
expect { WorkPackage.find_by(subject: "anything", id: "MYPROJ-1") }
.to raise_error(WorkPackage::SemanticIdentifier::UnsupportedLookup)
end
end
context "with an unparseable semantic string" do
it "raises ArgumentError" do
expect { WorkPackage.find_by(id: "not-an-identifier!") }
.to raise_error(ArgumentError, /find_by_display_id/)
end
end
end
# rubocop:disable Rails/FindById -- testing find_by! override specifically, not suggesting find()
describe "WorkPackage.find_by!" do
context "with id: keyword and semantic identifier" do
it "raises ArgumentError pointing to find_by_display_id" do
expect { WorkPackage.find_by!(id: "MYPROJ-1") }
.to raise_error(ArgumentError, /find_by_display_id/)
end
end
context "with non-id keywords" do
it "passes through to standard AR find_by!" do
expect(WorkPackage.find_by!(subject: work_package.subject)).to eq(work_package)
end
end
end
# rubocop:enable Rails/FindById
describe "WorkPackage.find_by_display_id" do
context "with a semantic identifier" do
it "resolves via the semantic identifier" do
expect(WorkPackage.find_by_display_id("MYPROJ-1")).to eq(work_package)
end
it "returns nil for unknown semantic id" do
expect(WorkPackage.find_by_display_id("MYPROJ-999")).to be_nil
end
it "resolves via historic alias" do
WorkPackageSemanticAlias.create!(identifier: "OLDPROJ-1", work_package:)
expect(WorkPackage.find_by_display_id("OLDPROJ-1")).to eq(work_package)
end
it "resolves when identifier column differs but alias row exists" do
work_package.update_columns(identifier: "OTHER-99")
expect(WorkPackage.find_by_display_id("MYPROJ-1")).to eq(work_package)
end
end
context "with a numeric string" do
it "falls through to standard AR find_by" do
expect(WorkPackage.find_by_display_id(work_package.id.to_s)).to eq(work_package)
end
it "returns nil for unknown numeric id" do
expect(WorkPackage.find_by_display_id("9999999")).to be_nil
end
end
context "with visibility scoping" do
let(:member_user) { create(:user, member_with_permissions: { project => [:view_work_packages] }) }
let(:non_member_user) { create(:user) }
it "respects the scope for semantic ids" do
expect(WorkPackage.visible(member_user).find_by_display_id("MYPROJ-1")).to eq(work_package)
end
it "returns nil when the user cannot see it" do
expect(WorkPackage.visible(non_member_user).find_by_display_id("MYPROJ-1")).to be_nil
end
it "also scopes numeric lookup" do
expect(WorkPackage.visible(non_member_user).find_by_display_id(work_package.id.to_s)).to be_nil
end
end
end
describe "WorkPackage.find_by_display_id!" do
context "with a semantic identifier" do
it "resolves via the semantic identifier" do
expect(WorkPackage.find_by_display_id!("MYPROJ-1")).to eq(work_package)
end
it "raises RecordNotFound for unknown semantic id" do
expect { WorkPackage.find_by_display_id!("MYPROJ-999") }.to raise_error(ActiveRecord::RecordNotFound)
end
end
context "with a numeric string" do
it "falls through to standard AR find" do
expect(WorkPackage.find_by_display_id!(work_package.id.to_s)).to eq(work_package)
end
end
end
@@ -94,10 +94,8 @@ RSpec.describe "API v3 Work package form resource" do
include_context "with post request"
it_behaves_like "param validation error" do
let(:id) { "eeek" }
let(:type) { "WorkPackage" }
end
it_behaves_like "not found",
I18n.t("api_v3.errors.not_found.work_package")
end
context "with existing work package" do
@@ -217,6 +217,26 @@ RSpec.describe "API v3 Work package resource",
it_behaves_like "not found response based on login_required",
I18n.t("api_v3.errors.not_found.work_package")
end
context "with a semantic identifier",
with_flag: { semantic_work_package_ids: true },
with_settings: { work_packages_identifier: "semantic" } do
let(:get_path) { api_v3_paths.work_package work_package.display_id }
before do
get get_path
end
it "resolves the semantic identifier and responds with 200" do
expect(last_response).to have_http_status(:ok)
end
it "returns the correct work package" do
expect(last_response.body)
.to be_json_eql(work_package.id.to_json)
.at_path("id")
end
end
end
describe "GET /api/v3/work_packages/:id?timestamps=" do
+12
View File
@@ -92,6 +92,18 @@ RSpec.describe WorkPackagesController do
id: "1")
end
it "connects GET /work_packages/:id to work_packages#show with semantic identifier" do
expect(get("/work_packages/PROJ-42")).to route_to(controller: "work_packages",
action: "show",
id: "PROJ-42")
end
it "routes lowercase identifiers to show (caught by resource route, resolved at controller level)" do
expect(get("/work_packages/proj-42")).to route_to(controller: "work_packages",
action: "show",
id: "proj-42")
end
it "connects GET /work_packages/:id/share to work_packages/shares#index" do
expect(get("/work_packages/1/shares")).to route_to(controller: "shares",
action: "index",
@@ -50,6 +50,20 @@ RSpec.describe "SemanticIds registry integration", type: :model do
login_as(user)
end
describe "find_by guard rejects semantic identifiers" do
let!(:work_package) { create(:work_package, project:) }
it "raises ArgumentError for find_by(id:) with a semantic string" do
expect { WorkPackage.find_by(id: "PROJ-1") }
.to raise_error(ArgumentError, /find_by_display_id/)
end
it "raises ArgumentError for find_by(id:) with a semantic string on a relation" do
expect { WorkPackage.where(project:).find_by(id: "PROJ-1") }
.to raise_error(ArgumentError, /find_by_display_id/)
end
end
describe "WP creation via CreateService" do
let(:attributes) do
{
@@ -97,12 +111,12 @@ RSpec.describe "SemanticIds registry integration", type: :model do
it "old identifier still resolves to the WP" do
WorkPackages::UpdateService.new(user:, model: work_package).call(project: target_project)
expect(WorkPackage.find_by_id_or_identifier("PROJ-5")).to eq(work_package)
expect(WorkPackage.find_by_display_id("PROJ-5")).to eq(work_package)
end
it "new identifier also resolves to the WP" do
WorkPackages::UpdateService.new(user:, model: work_package).call(project: target_project)
expect(WorkPackage.find_by_id_or_identifier(work_package.reload.identifier)).to eq(work_package)
expect(WorkPackage.find_by_display_id(work_package.reload.identifier)).to eq(work_package)
end
end
@@ -130,15 +144,15 @@ RSpec.describe "SemanticIds registry integration", type: :model do
it "old identifiers still resolve to the correct WPs" do
Projects::UpdateService.new(user:, model: project).call(identifier: "RENAMED")
expect(WorkPackage.find_by_id_or_identifier("PROJ-1")).to eq(wp1)
expect(WorkPackage.find_by_id_or_identifier("PROJ-2")).to eq(wp2)
expect(WorkPackage.find_by_display_id("PROJ-1")).to eq(wp1)
expect(WorkPackage.find_by_display_id("PROJ-2")).to eq(wp2)
end
it "new identifiers resolve to the correct WPs" do
Projects::UpdateService.new(user:, model: project).call(identifier: "RENAMED")
expect(WorkPackage.find_by_id_or_identifier("RENAMED-1")).to eq(wp1)
expect(WorkPackage.find_by_id_or_identifier("RENAMED-2")).to eq(wp2)
expect(WorkPackage.find_by_display_id("RENAMED-1")).to eq(wp1)
expect(WorkPackage.find_by_display_id("RENAMED-2")).to eq(wp2)
end
it "old prefix resolves for WPs created after the rename" do
@@ -147,8 +161,8 @@ RSpec.describe "SemanticIds registry integration", type: :model do
Projects::UpdateService.new(user:, model: project).call(identifier: "RENAMED")
wp3 = create(:work_package, project: project.reload)
expect(WorkPackage.find_by_id_or_identifier("RENAMED-3")).to eq(wp3)
expect(WorkPackage.find_by_id_or_identifier("PROJ-3")).to eq(wp3)
expect(WorkPackage.find_by_display_id("RENAMED-3")).to eq(wp3)
expect(WorkPackage.find_by_display_id("PROJ-3")).to eq(wp3)
end
end
@@ -161,7 +175,7 @@ RSpec.describe "SemanticIds registry integration", type: :model do
# PROJ is then renamed to RENAMED (bulk-inserts RENAMED-1 from the retired PROJ-1 row)
Projects::UpdateService.new(user:, model: project).call(identifier: "RENAMED")
expect(WorkPackage.find_by_id_or_identifier("RENAMED-1")).to eq(wp1)
expect(WorkPackage.find_by_display_id("RENAMED-1")).to eq(wp1)
end
it "rename then move: both old identifiers resolve after the WP moves" do
@@ -170,8 +184,8 @@ RSpec.describe "SemanticIds registry integration", type: :model do
# WP moves to DEST (appends DEST-1 registry row, updates identifier)
WorkPackages::UpdateService.new(user:, model: wp1.reload).call(project: target_project)
expect(WorkPackage.find_by_id_or_identifier("PROJ-1")).to eq(wp1)
expect(WorkPackage.find_by_id_or_identifier("RENAMED-1")).to eq(wp1)
expect(WorkPackage.find_by_display_id("PROJ-1")).to eq(wp1)
expect(WorkPackage.find_by_display_id("RENAMED-1")).to eq(wp1)
end
it "rename then new WP then move: pre-rename identifier resolves via alias table" do
@@ -183,7 +197,7 @@ RSpec.describe "SemanticIds registry integration", type: :model do
# wp2 moves to DEST — old identifier RENAMED-2 kept as alias, gets DEST-1
WorkPackages::UpdateService.new(user:, model: wp2).call(project: target_project)
expect(WorkPackage.find_by_id_or_identifier("PROJ-2")).to eq(wp2)
expect(WorkPackage.find_by_display_id("PROJ-2")).to eq(wp2)
end
end
@@ -202,9 +216,9 @@ RSpec.describe "SemanticIds registry integration", type: :model do
WorkPackages::UpdateService.new(user:, model: wp1.reload).call(project: project_c)
projc_identifier = wp1.reload.identifier
expect(WorkPackage.find_by_id_or_identifier("PROJ-1")).to eq(wp1)
expect(WorkPackage.find_by_id_or_identifier(dest_identifier)).to eq(wp1)
expect(WorkPackage.find_by_id_or_identifier(projc_identifier)).to eq(wp1)
expect(WorkPackage.find_by_display_id("PROJ-1")).to eq(wp1)
expect(WorkPackage.find_by_display_id(dest_identifier)).to eq(wp1)
expect(WorkPackage.find_by_display_id(projc_identifier)).to eq(wp1)
end
end
end