diff --git a/app/components/admin/settings/project_reserved_identifiers/release_dialog_component.html.erb b/app/components/admin/settings/project_reserved_identifiers/release_dialog_component.html.erb index 87a3a5d7ee6..087cf763c46 100644 --- a/app/components/admin/settings/project_reserved_identifiers/release_dialog_component.html.erb +++ b/app/components/admin/settings/project_reserved_identifiers/release_dialog_component.html.erb @@ -33,6 +33,10 @@ render( Primer::OpenProject::DangerDialog.new( id: "release-identifier-dialog", + # The default :medium size caps the dialog height at 320px, which the + # description with the work package count can exceed, forcing an inner + # scrollbar. :medium_portrait keeps the same width with a 600px cap. + size: :medium_portrait, title: I18n.t("admin.reserved_identifiers.dialog.title"), confirm_button_text: I18n.t("admin.reserved_identifiers.dialog.confirm_button"), cancel_button_text: I18n.t("button_cancel"), @@ -47,9 +51,7 @@ I18n.t("admin.reserved_identifiers.dialog.heading", identifier: @slug.slug) end - message.with_description_content( - I18n.t("admin.reserved_identifiers.dialog.description") - ) + message.with_description_content(description_text) end dialog.with_confirmation_check_box_content( diff --git a/app/components/admin/settings/project_reserved_identifiers/release_dialog_component.rb b/app/components/admin/settings/project_reserved_identifiers/release_dialog_component.rb index 4f6d77bb671..40c11ea0eac 100644 --- a/app/components/admin/settings/project_reserved_identifiers/release_dialog_component.rb +++ b/app/components/admin/settings/project_reserved_identifiers/release_dialog_component.rb @@ -39,6 +39,21 @@ module Admin super @slug = slug end + + private + + def description_text + if affected_work_package_count.positive? + I18n.t("admin.reserved_identifiers.dialog.description_with_work_packages", + count: affected_work_package_count) + else + I18n.t("admin.reserved_identifiers.dialog.description") + end + end + + def affected_work_package_count + @affected_work_package_count ||= WorkPackage.resolving_via_slug_prefix(@slug.slug).count + end end end end diff --git a/app/controllers/admin/settings/project_reserved_identifiers_controller.rb b/app/controllers/admin/settings/project_reserved_identifiers_controller.rb index cf829fd75b0..58b028f7e39 100644 --- a/app/controllers/admin/settings/project_reserved_identifiers_controller.rb +++ b/app/controllers/admin/settings/project_reserved_identifiers_controller.rb @@ -34,7 +34,6 @@ module Admin::Settings include PaginationHelper before_action :require_admin - before_action :require_classic_mode before_action :find_slug, only: %i[confirm_dialog destroy] menu_item :project_reserved_identifiers_settings @@ -63,7 +62,7 @@ module Admin::Settings end def destroy - @slug.destroy! + ProjectIdentifiers::ReleaseReservedIdentifierService.new(@slug).call redirect_to admin_settings_project_reserved_identifiers_path, flash: { notice: t("admin.reserved_identifiers.released_notice", identifier: @slug.slug) } end @@ -85,12 +84,5 @@ module Admin::Settings query_class: Queries::ProjectReservedIdentifiers::ProjectReservedIdentifierQuery) .call(params) end - - def require_classic_mode - return unless Setting::WorkPackageIdentifier.semantic? - - redirect_to admin_settings_work_packages_identifier_path, - flash: { warning: t("admin.reserved_identifiers.not_available_in_semantic_mode") } - end end end diff --git a/app/models/queries/project_reserved_identifiers/project_reserved_identifier_query.rb b/app/models/queries/project_reserved_identifiers/project_reserved_identifier_query.rb index 2e1a3e29dce..ad45a7b550d 100644 --- a/app/models/queries/project_reserved_identifiers/project_reserved_identifier_query.rb +++ b/app/models/queries/project_reserved_identifiers/project_reserved_identifier_query.rb @@ -37,9 +37,15 @@ class Queries::ProjectReservedIdentifiers::ProjectReservedIdentifierQuery end def default_scope + # Pure-numeric slugs are legacy artifacts (identifier validation was tightened + # later; the semantic-conversion autofix renames such projects, reserving the + # old numeric identifier). Releasing them frees nothing — numeric identifiers + # are invalid in both formats — and would break friendly_id history resolution + # of old /projects/ links, letting them fall through to a primary-key + # lookup of a different project. Project.identifier_slugs .historically_reserved - .where("slug ~ ? AND slug !~ ?", "^[a-z0-9_-]+$", "^[0-9]+$") + .where("slug !~ ?", "^[0-9]+$") .joins("JOIN projects ON projects.id = friendly_id_slugs.sluggable_id") .order("projects.name ASC, friendly_id_slugs.created_at DESC") end diff --git a/app/models/work_package/semantic_identifier.rb b/app/models/work_package/semantic_identifier.rb index cadd43670b4..2a8fb487158 100644 --- a/app/models/work_package/semantic_identifier.rb +++ b/app/models/work_package/semantic_identifier.rb @@ -41,6 +41,16 @@ module WorkPackage::SemanticIdentifier # The frontend equivalent lives in WP_ID_URL_PATTERN (work-package-id-pattern.ts). ID_ROUTE_CONSTRAINT = /\d+|#{SEMANTIC_ID_PATTERN.source}/ + # Anchored POSIX regex matching identifiers of the exact form "-" + # for a concrete project slug, so prefixes containing dashes don't over-match + # (matching "my" must not touch "my-project-42"). Used by the for_slug_prefix + # scopes on WorkPackage and WorkPackageSemanticAlias. Regexp.escape output is + # valid in PostgreSQL's ARE syntax: it backslash-escapes punctuation (which + # ARE treats as literals) and never emits class escapes. + def self.slug_prefix_pattern(slug) + "^#{Regexp.escape(slug)}-[0-9]+$" + end + # 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 @@ -64,6 +74,21 @@ module WorkPackage::SemanticIdentifier joins(:project).semantically_sequenced .where("work_packages.identifier IS DISTINCT FROM projects.identifier || '-' || work_packages.sequence_number::text") } + # Work packages whose identifier column carries the given project slug + # prefix, i.e. is of the exact form "-". Counterpart to + # WorkPackageSemanticAlias.for_slug_prefix for the denormalized column. + scope :for_slug_prefix, ->(slug) { + where("identifier ~ ?", WorkPackage::SemanticIdentifier.slug_prefix_pattern(slug)) + } + # Work packages that currently resolve via identifiers of the form + # "-" — through the identifier column or an alias row. + # The single-identifier counterpart is FinderMethods#scope_for_semantic_identifier. + # ReleaseReservedIdentifierService severs exactly this set, and the release + # dialog counts it — keep the two in sync through this scope. + scope :resolving_via_slug_prefix, ->(slug) { + where(id: WorkPackageSemanticAlias.for_slug_prefix(slug).select(:work_package_id)) + .or(for_slug_prefix(slug)) + } attr_accessor :skip_semantic_id_allocation diff --git a/app/models/work_package_semantic_alias.rb b/app/models/work_package_semantic_alias.rb index e85e56cbad2..6769548a053 100644 --- a/app/models/work_package_semantic_alias.rb +++ b/app/models/work_package_semantic_alias.rb @@ -42,4 +42,12 @@ class WorkPackageSemanticAlias < ApplicationRecord validates :identifier, presence: true, uniqueness: true validates :work_package, presence: true + + # Aliases created for the given project slug prefix, i.e. identifiers of the + # exact form "-". Case-sensitive: aliases are always created + # verbatim from slug values, and slugs differing only in case (classic "proj" + # vs semantic "PROJ") are distinct reservations. + scope :for_slug_prefix, ->(slug) { + where("identifier ~ ?", WorkPackage::SemanticIdentifier.slug_prefix_pattern(slug)) + } end diff --git a/app/services/project_identifiers/release_reserved_identifier_service.rb b/app/services/project_identifiers/release_reserved_identifier_service.rb new file mode 100644 index 00000000000..2d46c3fe473 --- /dev/null +++ b/app/services/project_identifiers/release_reserved_identifier_service.rb @@ -0,0 +1,72 @@ +# 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. +#++ + +module ProjectIdentifiers + # Releases a historically reserved project identifier slug. + # Additionally removes the WorkPackageSemanticAlias rows created for that + # slug prefix ("-") and clears stale work package identifier + # columns carrying it, atomically with the slug destroy, so the released + # prefix no longer resolves work packages. This happens regardless of the + # current identifier mode: leftovers from a previous semantic phase must not + # survive a release, or they would keep old links resolving and shadow the + # alias rows of a new project claiming the identifier after a later + # re-conversion. + class ReleaseReservedIdentifierService + def initialize(slug) + @slug = slug + end + + def call + FriendlyId::Slug.transaction do + delete_aliases + clear_stale_work_package_identifiers + @slug.destroy! + end + + ServiceResult.success + end + + private + + def delete_aliases + WorkPackageSemanticAlias.for_slug_prefix(@slug.slug).delete_all + end + + # A historically reserved slug is no project's current identifier, so any + # work package still carrying "-" in its identifier column is + # stale (left over from a revert to classic mode). The finder resolves + # semantic identifiers against this column as well as the alias table, so + # clearing it severs resolution immediately instead of waiting for the next + # semantic conversion's reset_stale_identifiers to do the same. + def clear_stale_work_package_identifiers + WorkPackage.for_slug_prefix(@slug.slug).update_all(identifier: nil, sequence_number: nil) + end + end +end diff --git a/config/initializers/menus.rb b/config/initializers/menus.rb index 162c2712345..cd1894702f2 100644 --- a/config/initializers/menus.rb +++ b/config/initializers/menus.rb @@ -464,7 +464,7 @@ Redmine::MenuManager.map :admin_menu do |menu| menu.push :project_reserved_identifiers_settings, { controller: "/admin/settings/project_reserved_identifiers", action: :index }, - if: ->(_) { User.current.admin? && Setting::WorkPackageIdentifier.classic? }, + if: ->(_) { User.current.admin? }, caption: :label_reserved_identifiers, parent: :admin_projects_settings diff --git a/config/locales/en.yml b/config/locales/en.yml index 9f8e0446a38..e157349438c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -61,25 +61,27 @@ en: admin: reserved_identifiers: - title: "Reserved project identifiers" - lede_html: "When a project's identifier is renamed, the previous identifier is kept reserved so that existing links and integrations keep working.
Here you can release reserved identifiers so that they may be used by other projects." + btn_release: "Unreserve" col_identifier: "Identifier" col_project: "Project" col_reserved: "Reserved" - not_available_in_semantic_mode: "Reserved project identifiers are only available in numeric identifier mode." - filter_label: "Search identifiers" - btn_release: "Release" - released_notice: 'Identifier "%{identifier}" has been released.' - identifier_not_found: "The reserved identifier could not be found. It may have already been released or the project may have been deleted. Please refresh the page." dialog: - title: "Release identifier" - heading: 'Release "%{identifier}"?' - description: "Releasing this identifier cannot be undone. External links and integrations using it will stop resolving, and the name becomes available for any new project to claim." checkbox_label: "I understand that this cannot be undone." - confirm_button: "Release identifier" + confirm_button: "Unreserve identifier" + description: "Unreserving this project identifier cannot be undone. External links and integrations using it will stop resolving, and the name becomes available for any new project to claim." + description_with_work_packages: + one: "Unreserving this project identifier (and its 1 work package identifier) cannot be undone. External links and integrations using it will stop resolving, and the name becomes available for any new project to claim." + other: "Unreserving this project identifier (and its %{count} work package identifiers) cannot be undone. External links and integrations using it will stop resolving, and the name becomes available for any new project to claim." + heading: 'Unreserve "%{identifier}"?' + title: "Unreserve identifier" + empty_body: "When a project's identifier changes, the previous one will appear here so you can unreserve it once it's safe to do so." empty_heading: "No reserved identifiers" + filter_label: "Search identifiers" + identifier_not_found: "The reserved identifier could not be found. It may have already been unreserved or the project may have been deleted. Please refresh the page." + lede_html: "When a project's identifier is renamed, the previous identifier is kept reserved so that existing links and integrations keep working.
Here you can unreserve identifiers so that they may be used by other projects." + released_notice: 'Identifier "%{identifier}" has been unreserved.' reserved_ago: "%{time} ago" - empty_body: "When a project's identifier changes, the previous one will appear here so you can release it once it's safe to do so." + title: "Reserved project identifiers" plugins: no_results_title_text: There are currently no plugins installed. no_results_content_text: See our integrations and plugins page for more information. diff --git a/spec/components/admin/settings/project_reserved_identifiers/release_dialog_component_spec.rb b/spec/components/admin/settings/project_reserved_identifiers/release_dialog_component_spec.rb new file mode 100644 index 00000000000..39bedacfb65 --- /dev/null +++ b/spec/components/admin/settings/project_reserved_identifiers/release_dialog_component_spec.rb @@ -0,0 +1,72 @@ +# 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. +#++ + +require "rails_helper" + +RSpec.describe Admin::Settings::ProjectReservedIdentifiers::ReleaseDialogComponent, type: :component do + let!(:project) { create(:project) } + let!(:slug) { FriendlyId::Slug.create!(sluggable: project, slug: "old-id") } + + subject(:rendered_component) { render_inline(described_class.new(slug:)) } + + it "renders the heading with the identifier" do + expect(rendered_component) + .to have_text(I18n.t("admin.reserved_identifiers.dialog.heading", identifier: "old-id")) + end + + context "without affected work packages" do + it "renders the plain description" do + expect(rendered_component) + .to have_text(I18n.t("admin.reserved_identifiers.dialog.description")) + expect(rendered_component).to have_no_text("work package") + end + end + + context "with one affected work package" do + before { create(:work_package_semantic_alias, identifier: "old-id-1") } + + it "renders the singular description" do + expect(rendered_component) + .to have_text(I18n.t("admin.reserved_identifiers.dialog.description_with_work_packages", count: 1)) + end + end + + context "with several affected work packages" do + before do + create(:work_package_semantic_alias, identifier: "old-id-1") + create(:work_package_semantic_alias, identifier: "old-id-2") + end + + it "renders the pluralized description" do + expect(rendered_component) + .to have_text(I18n.t("admin.reserved_identifiers.dialog.description_with_work_packages", count: 2)) + end + end +end diff --git a/spec/controllers/admin/settings/project_reserved_identifiers_controller_spec.rb b/spec/controllers/admin/settings/project_reserved_identifiers_controller_spec.rb index 7c4da30977c..bc78bb5853c 100644 --- a/spec/controllers/admin/settings/project_reserved_identifiers_controller_spec.rb +++ b/spec/controllers/admin/settings/project_reserved_identifiers_controller_spec.rb @@ -36,40 +36,33 @@ RSpec.describe Admin::Settings::ProjectReservedIdentifiersController do current_user { admin } describe "GET #index" do - context "in semantic mode", with_settings: { work_packages_identifier: "semantic" } do - it "redirects to the identifier settings page" do - get :index - expect(response).to redirect_to(admin_settings_work_packages_identifier_path) - end - end + shared_examples "lists reserved slugs of all formats" do + let!(:project) { create(:project) } + + before do + FriendlyId::Slug.create!(sluggable: project, slug: "old-classic") + FriendlyId::Slug.create!(sluggable: project, slug: "OLDPROJ") + FriendlyId::Slug.create!(sluggable: project, slug: "12345") + end - context "in classic mode", with_settings: { work_packages_identifier: "classic" } do it "responds 200" do get :index expect(response).to have_http_status(:ok) end - context "with a classic reserved slug" do - let!(:project) { create(:project, identifier: "current-id") } - - before { FriendlyId::Slug.create!(sluggable: project, slug: "old-classic") } - - it "includes the slug in @slugs" do - get :index - expect(assigns(:slugs).map(&:slug)).to include("old-classic") - end + it "includes classic-format and semantic-format slugs, excluding pure-numeric ones" do + get :index + expect(assigns(:slugs).map(&:slug)).to include("old-classic", "OLDPROJ") + expect(assigns(:slugs).map(&:slug)).not_to include("12345") end + end - context "with a pure-numeric reserved slug" do - let!(:project) { create(:project, identifier: "current-id") } + context "in semantic mode", with_settings: { work_packages_identifier: "semantic" } do + it_behaves_like "lists reserved slugs of all formats" + end - before { FriendlyId::Slug.create!(sluggable: project, slug: "12345") } - - it "excludes pure-numeric slugs" do - get :index - expect(assigns(:slugs).map(&:slug)).not_to include("12345") - end - end + context "in classic mode", with_settings: { work_packages_identifier: "classic" } do + it_behaves_like "lists reserved slugs of all formats" end end @@ -154,26 +147,81 @@ RSpec.describe Admin::Settings::ProjectReservedIdentifiersController do expect(response.body).to include(I18n.t("admin.reserved_identifiers.identifier_not_found")) end end + + describe "work package warning" do + render_views + + let!(:project) { create(:project) } + let!(:slug) { FriendlyId::Slug.create!(sluggable: project, slug: "old-id") } + + context "with affected work packages" do + before do + create(:work_package_semantic_alias, identifier: "old-id-1") + create(:work_package_semantic_alias, identifier: "old-id-2") + end + + %w[semantic classic].each do |mode| + context "in #{mode} mode", with_settings: { work_packages_identifier: mode } do + it "shows the warning with the affected work package count" do + get :confirm_dialog, params: { id: slug.id }, format: :turbo_stream + expect(response.body) + .to include(I18n.t("admin.reserved_identifiers.dialog.description_with_work_packages", count: 2)) + end + end + end + end + + context "without affected work packages" do + it "does not show the warning" do + get :confirm_dialog, params: { id: slug.id }, format: :turbo_stream + expect(response.body).not_to include("work package") + end + end + end end - describe "DELETE #destroy", with_settings: { work_packages_identifier: "classic" } do - let!(:project) { create(:project, identifier: "current-id") } + describe "DELETE #destroy" do + let!(:project) { create(:project) } let!(:slug) { FriendlyId::Slug.create!(sluggable: project, slug: "old-id") } - it "destroys the slug and redirects with a flash notice" do - expect { delete :destroy, params: { id: slug.id } } - .to change(FriendlyId::Slug, :count).by(-1) + context "in classic mode", with_settings: { work_packages_identifier: "classic" } do + it "destroys the slug and redirects with a flash notice" do + expect { delete :destroy, params: { id: slug.id } } + .to change(FriendlyId::Slug, :count).by(-1) - expect(response).to redirect_to(admin_settings_project_reserved_identifiers_path) - expect(flash[:notice]).to include("old-id") + expect(response).to redirect_to(admin_settings_project_reserved_identifiers_path) + expect(flash[:notice]).to include("old-id") + end + + it "also deletes work package aliases left over from a previous semantic phase" do + create(:work_package_semantic_alias, identifier: "old-id-1") + + expect { delete :destroy, params: { id: slug.id } } + .to change(WorkPackageSemanticAlias, :count).by(-1) + end + + context "with an unknown id" do + it "responds with a turbo stream error flash" do + delete :destroy, params: { id: 0 }, format: :turbo_stream + expect(response).to have_http_status(:not_found) + expect(response.media_type).to eq("text/vnd.turbo-stream.html") + expect(response.body).to include(I18n.t("admin.reserved_identifiers.identifier_not_found")) + end + end end - context "with an unknown id" do - it "responds with a turbo stream error flash" do - delete :destroy, params: { id: 0 }, format: :turbo_stream - expect(response).to have_http_status(:not_found) - expect(response.media_type).to eq("text/vnd.turbo-stream.html") - expect(response.body).to include(I18n.t("admin.reserved_identifiers.identifier_not_found")) + # Exact-prefix and case-sensitivity edge cases live in the service and + # scope specs; this guards the formerly mode-gated endpoint end to end. + context "in semantic mode", with_settings: { work_packages_identifier: "semantic" } do + it "destroys the slug and deletes its aliases" do + create(:work_package_semantic_alias, identifier: "old-id-1") + + expect { delete :destroy, params: { id: slug.id } } + .to change(FriendlyId::Slug, :count).by(-1) + .and change(WorkPackageSemanticAlias, :count).by(-1) + + expect(response).to redirect_to(admin_settings_project_reserved_identifiers_path) + expect(flash[:notice]).to include("old-id") end end end diff --git a/spec/models/work_package/semantic_identifier_spec.rb b/spec/models/work_package/semantic_identifier_spec.rb index c07349d42c1..8935b4f142c 100644 --- a/spec/models/work_package/semantic_identifier_spec.rb +++ b/spec/models/work_package/semantic_identifier_spec.rb @@ -75,6 +75,39 @@ RSpec.describe WorkPackage::SemanticIdentifier do end end + describe ".for_slug_prefix" do + it "matches work packages whose identifier is of the form -" do + expect(WorkPackage.for_slug_prefix("MYPROJ")).to include(work_package) + end + + it "does not over-match when another slug starts with the given slug plus a dash" do + work_package.update_columns(identifier: "my-project-42") + expect(WorkPackage.for_slug_prefix("my")).not_to include(work_package) + end + + it "matches case-sensitively" do + expect(WorkPackage.for_slug_prefix("myproj")).not_to include(work_package) + end + end + + describe ".resolving_via_slug_prefix" do + # The auto-registered work_package carries the prefix both in its + # identifier column and as an alias row. + it "returns work packages matched via column and alias exactly once" do + expect(WorkPackage.resolving_via_slug_prefix("MYPROJ")).to contain_exactly(work_package) + end + + it "includes work packages matched only via an alias row" do + work_package.update_columns(identifier: nil, sequence_number: nil) + expect(WorkPackage.resolving_via_slug_prefix("MYPROJ")).to contain_exactly(work_package) + end + + it "includes work packages matched only via the identifier column" do + work_package.semantic_aliases.delete_all + expect(WorkPackage.resolving_via_slug_prefix("MYPROJ")).to contain_exactly(work_package) + end + end + describe "after_create registration", with_settings: { work_packages_identifier: "semantic" } do it "assigns a sequence number" do expect(work_package.reload.sequence_number).to eq(1) diff --git a/spec/models/work_package_semantic_alias_spec.rb b/spec/models/work_package_semantic_alias_spec.rb index 4d7f64a5572..ebbb4747948 100644 --- a/spec/models/work_package_semantic_alias_spec.rb +++ b/spec/models/work_package_semantic_alias_spec.rb @@ -66,6 +66,33 @@ RSpec.describe WorkPackageSemanticAlias do end end + describe ".for_slug_prefix" do + def alias_for(identifier) + described_class.create!(identifier:, work_package:) + end + + it "matches identifiers of the exact form -" do + matching = alias_for("my-1") + alias_for("my-abc") + + expect(described_class.for_slug_prefix("my")).to contain_exactly(matching) + end + + it "does not over-match when another slug starts with the given slug plus a dash" do + matching = alias_for("my-2") + alias_for("my-project-42") + + expect(described_class.for_slug_prefix("my")).to contain_exactly(matching) + end + + it "matches case-sensitively" do + matching = alias_for("PROJ-1") + alias_for("proj-1") + + expect(described_class.for_slug_prefix("PROJ")).to contain_exactly(matching) + end + end + describe WorkPackage do describe "#semantic_aliases" do let(:wp) { create(:work_package) } diff --git a/spec/services/project_identifiers/release_reserved_identifier_service_spec.rb b/spec/services/project_identifiers/release_reserved_identifier_service_spec.rb new file mode 100644 index 00000000000..120090d700b --- /dev/null +++ b/spec/services/project_identifiers/release_reserved_identifier_service_spec.rb @@ -0,0 +1,105 @@ +# 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. +#++ + +require "spec_helper" + +RSpec.describe ProjectIdentifiers::ReleaseReservedIdentifierService do + let!(:project) { create(:project) } + let!(:slug) { FriendlyId::Slug.create!(sluggable: project, slug: "old-id") } + + subject(:service_call) { described_class.new(slug).call } + + shared_examples "releases the slug and its aliases" do + # All fixtures share one project graph — deliberately NOT the slug-owning + # project: creating work packages there would auto-seed "old-id-*" aliases + # in semantic mode and collide with the explicit ones below. + let(:alias_work_package) { create(:work_package) } + let!(:matching_aliases) do + [create(:work_package_semantic_alias, identifier: "old-id-1", work_package: alias_work_package), + create(:work_package_semantic_alias, identifier: "old-id-2", work_package: alias_work_package)] + end + let!(:other_prefix_alias) do + create(:work_package_semantic_alias, identifier: "old-id-extra-7", work_package: alias_work_package) + end + let!(:case_differing_alias) do + create(:work_package_semantic_alias, identifier: "OLD-ID-3", work_package: alias_work_package) + end + let!(:stale_wp) do + create(:work_package, project: alias_work_package.project) + .tap { |wp| wp.update_columns(identifier: "old-id-9", sequence_number: 9) } + end + let!(:decoy_wp) do + create(:work_package, project: alias_work_package.project) + .tap { |wp| wp.update_columns(identifier: "old-id-extra-9", sequence_number: 8) } + end + + it "destroys the slug and returns a successful ServiceResult" do + expect(service_call).to be_success + expect(FriendlyId::Slug.exists?(slug.id)).to be(false) + end + + it "clears stale identifiers only from work packages carrying the released prefix" do + service_call + expect(stale_wp.reload).to have_attributes(identifier: nil, sequence_number: nil) + expect(decoy_wp.reload).to have_attributes(identifier: "old-id-extra-9", sequence_number: 8) + end + + it "deletes only the exact-prefix aliases" do + expect { service_call }.to change(WorkPackageSemanticAlias, :count).by(-2) + + remaining = WorkPackageSemanticAlias.pluck(:identifier) + expect(remaining).to include("old-id-extra-7", "OLD-ID-3") + expect(remaining).not_to include("old-id-1", "old-id-2") + end + + context "when destroying the slug fails" do + before do + allow(slug).to receive(:destroy!).and_raise(ActiveRecord::RecordNotDestroyed) + end + + it "rolls back the alias deletion and the work package identifier clearing" do + expect { service_call }.to raise_error(ActiveRecord::RecordNotDestroyed) + expect(WorkPackageSemanticAlias.where(identifier: %w[old-id-1 old-id-2]).count).to eq(2) + expect(stale_wp.reload).to have_attributes(identifier: "old-id-9", sequence_number: 9) + end + end + end + + context "in semantic mode", with_settings: { work_packages_identifier: "semantic" } do + it_behaves_like "releases the slug and its aliases" + end + + # Aliases left over from a previous semantic phase are cleaned up in classic + # mode too — otherwise they would shadow the alias rows of a new project + # claiming the identifier after a later re-conversion. + context "in classic mode", with_settings: { work_packages_identifier: "classic" } do + it_behaves_like "releases the slug and its aliases" + end +end