use a random suffix instead

This commit is contained in:
Tomas Hykel
2026-05-28 13:07:05 +02:00
parent ccc3a004d6
commit 5343c1139d
2 changed files with 20 additions and 14 deletions
@@ -51,24 +51,26 @@ module ProjectIdentifiers
attr_reader :project
def restore_classic_identifier
classic = identifier_generator.restore_identifier(project) ||
identifier_generator.suggest_identifier(project.name)
classic_id = identifier_generator.restore_identifier(project) ||
identifier_generator.suggest_identifier(project.name)
# Suppress notifications: this is a background system operation, not a user edit.
Journal::NotificationConfiguration.with(false) do
project.update!(identifier: classic)
project.update!(identifier: classic_id)
rescue ActiveRecord::RecordInvalid => e
handle_identifier_conflict(classic, e)
handle_update_failure(classic_id, e)
end
end
def handle_identifier_conflict(classic, error)
Rails.logger.warn "#{self.class}: identifier '#{classic}' taken for project #{project.id}, " \
"falling back. (#{error.message})"
project.update!(identifier: identifier_generator.suggest_identifier(project.name))
def handle_update_failure(classic_id, error)
Rails.logger.warn "#{self.class}: Could not set identifier '#{classic_id}' for project #{project.id}; " \
"falling back to a randomized suffix. (#{error.message})"
suffix = SecureRandom.alphanumeric(5).downcase
base = classic_id.first(Projects::Identifier::CLASSIC_IDENTIFIER_MAX_LENGTH - 6)
project.update!(identifier: "#{base}-#{suffix}")
end
def identifier_generator
ProjectIdentifiers::ClassicIdentifierSuggestionGenerator.new
@identifier_generator ||= ProjectIdentifiers::ClassicIdentifierSuggestionGenerator.new
end
end
end
@@ -116,7 +116,13 @@ RSpec.describe ProjectIdentifiers::RevertProjectToClassicService do
let!(:project) do
create(:project).tap do |p|
p.update_columns(identifier: "MYAPP", wp_sequence_counter: 0)
FriendlyId::Slug.create!(sluggable: p, slug: "my-app")
# Remove p's own initial slug so "my-app" is the only entry in its slug history.
# Without this, the factory slug (newer created_at) would be returned first by
# restore_identifier, the update would succeed, and the conflict path would never fire.
FriendlyId::Slug.where(sluggable_id: p.id, sluggable_type: "Project").delete_all
# blocking_project already owns the "my-app" FriendlyId slug; reassign it so that
# restore_identifier returns "my-app" and project.update! conflicts with blocking_project.
FriendlyId::Slug.where(slug: "my-app", sluggable_type: "Project").update_all(sluggable_id: p.id)
end
end
@@ -124,11 +130,9 @@ RSpec.describe ProjectIdentifiers::RevertProjectToClassicService do
expect { described_class.new(project).call }.not_to raise_error
end
it "assigns a valid classic identifier that is not the conflicting one" do
it "assigns the conflicting identifier with a 5-character random suffix" do
described_class.new(project).call
reloaded = project.reload
expect(reloaded.identifier).to match(Projects::Identifier::CLASSIC_FORMAT)
expect(reloaded.identifier).not_to eq("my-app")
expect(project.reload.identifier).to match(/\Amy-app-[a-z0-9]{5}\z/)
end
it "logs a warning containing the project id and the conflicting identifier" do