Skip to content

Commit 855b75c

Browse files
committed
Fix alphanumeric identifier validation error cascading
Replace two independent format validators with a single custom validation that short-circuits on the first error. Previously, "3INVALID" showed both "must start with a letter" and "may only contain uppercase letters..." — the second message was misleading since the input contained only valid characters. Also pin url identifier tests to numeric mode since the test environment default is now alphanumeric.
1 parent 99a819e commit 855b75c

File tree

3 files changed

+16
-8
lines changed

3 files changed

+16
-8
lines changed

app/models/projects/identifier.rb

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,8 @@ module Projects::Identifier
7070
}
7171

7272
# Validators for the alphanumeric identifier format (e.g. "PROJ1")
73-
validates :identifier,
74-
format: { with: /\A[A-Z]/, message: :must_start_with_letter },
75-
if: ->(p) { p.identifier_changed? && p.identifier.present? && Setting::WorkPackageIdentifier.alphanumeric? }
76-
validates :identifier,
77-
format: { with: /\A[A-Z][A-Z0-9_]*\z/, message: :no_special_characters },
78-
length: { maximum: SEMANTIC_IDENTIFIER_MAX_LENGTH },
79-
if: ->(p) { p.identifier_changed? && p.identifier.present? && Setting::WorkPackageIdentifier.alphanumeric? }
73+
validate :identifier_alphanumeric_format,
74+
if: ->(p) { p.identifier_changed? && p.identifier.present? && Setting::WorkPackageIdentifier.alphanumeric? }
8075

8176
validate :identifier_not_reserved, if: -> { identifier.present? }
8277

@@ -132,6 +127,18 @@ def validation_context
132127

133128
private
134129

130+
def identifier_alphanumeric_format
131+
unless identifier.match?(/\A[A-Z]/)
132+
errors.add(:identifier, :must_start_with_letter)
133+
return
134+
end
135+
136+
errors.add(:identifier, :no_special_characters) unless identifier.match?(/\A[A-Z][A-Z0-9_]*\z/)
137+
if identifier.length > SEMANTIC_IDENTIFIER_MAX_LENGTH
138+
errors.add(:identifier, :too_long, count: SEMANTIC_IDENTIFIER_MAX_LENGTH)
139+
end
140+
end
141+
135142
def identifier_not_reserved
136143
if RESERVED_IDENTIFIERS.include?(identifier&.downcase)
137144
errors.add(:identifier, :exclusion)

spec/features/projects/create_spec.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,7 @@
592592

593593
fill_in "Name", with: "Flight Planning Algorithm"
594594
find("body").click
595+
expect(page).to have_field "Identifier", with: "FPA"
595596

596597
fill_in "Identifier", with: "3INVALID"
597598
click_on "Complete"

spec/models/projects/identifier_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@
8181
end
8282
end
8383

84-
describe "url identifier" do
84+
describe "url identifier", with_settings: { work_packages_identifier: "numeric" } do
8585
let(:reserved) do
8686
Rails.application.routes.routes
8787
.map { |route| route.path.spec.to_s }

0 commit comments

Comments
 (0)