Skip to content

Commit 0d11eab

Browse files
Copilotnorthrup
andcommitted
Refocus on description field semicolon handling per review feedback
Revert parsed_predicate changes and instead add tests ensuring semicolons in the description free-form text area are preserved correctly and not treated as predicate separators. Co-authored-by: northrup <[email protected]>
1 parent f463b20 commit 0d11eab

7 files changed

Lines changed: 24 additions & 46 deletions

File tree

lib/entitlements/data/groups/calculated/text.rb

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -316,13 +316,8 @@ def parsed_predicate(val)
316316
return { key: v } unless v.include?(";")
317317

318318
parts = v.split(/\s*;\s*/)
319-
primary_value = parts.shift
320-
if primary_value.nil? || primary_value.strip.empty?
321-
raise ArgumentError, "Rule Error: Empty value with semicolon predicate in #{filename}!"
322-
end
323-
324-
op_hash = { key: primary_value.strip }
325-
parts.reject { |part| part.strip.empty? }.each do |part|
319+
op_hash = { key: parts.shift }
320+
parts.each do |part|
326321
if part =~ /\A(\w+)\s*=\s*(\S+)\s*\z/
327322
predicate_keyword, predicate_value = Regexp.last_match(1), Regexp.last_match(2)
328323
unless SEMICOLON_PREDICATES.include?(predicate_keyword)

spec/unit/entitlements/data/groups/calculated/text_spec.rb

Lines changed: 18 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,24 @@
6060
subject.description
6161
end.to raise_error(RuntimeError, /description cannot use '!=' operator in .+not-equals-description.txt!/)
6262
end
63+
64+
it "preserves semicolons in a free-form narrative description" do
65+
filename = fixture("ldap-config/text/description-with-semicolon-narrative.txt")
66+
subject = described_class.new(filename: filename)
67+
expect(subject.description).to eq("This group manages auth; it also handles access control for the team")
68+
end
69+
70+
it "does not parse semicolons in description as predicates" do
71+
filename = fixture("ldap-config/text/semicolons-in-description.txt")
72+
subject = described_class.new(filename: filename)
73+
expect(subject.description).to eq("the; description; can; have; semicolons")
74+
end
75+
76+
it "does not treat predicate-like text after semicolons in description as predicates" do
77+
filename = fixture("ldap-config/text/description-with-predicate-like-semicolon.txt")
78+
subject = described_class.new(filename: filename)
79+
expect(subject.description).to eq("This group provides access to resources; expiration = 2099-12-31 is not a predicate here")
80+
end
6381
end
6482

6583
describe "#initialize_filters" do
@@ -589,38 +607,5 @@
589607
expect(result).to eq(answer)
590608
end
591609
end
592-
593-
context "with a leading semicolon in the value" do
594-
let(:filename) { fixture("ldap-config/text/leading-semicolon.txt") }
595-
596-
it "raises an error about empty value" do
597-
expect do
598-
subject.send(:parsed_data)
599-
end.to raise_error(ArgumentError, /Rule Error: Empty value with semicolon predicate in .+leading-semicolon.txt!/)
600-
end
601-
end
602-
603-
context "with only a semicolon as the value" do
604-
let(:filename) { fixture("ldap-config/text/only-semicolon.txt") }
605-
606-
it "raises an error about empty value" do
607-
expect do
608-
subject.send(:parsed_data)
609-
end.to raise_error(ArgumentError, /Rule Error: Empty value with semicolon predicate in .+only-semicolon.txt!/)
610-
end
611-
end
612-
613-
context "with a trailing semicolon in the value" do
614-
let(:filename) { fixture("ldap-config/text/trailing-semicolon.txt") }
615-
616-
it "parses correctly ignoring trailing semicolon" do
617-
result = subject.send(:parsed_data)
618-
answer = {
619-
"description" => {"=" => [{ key: "Trailing semicolon test" }], "!=" => [], "&=" => []},
620-
"username" => {"=" => [{ key: "blackmanx" }], "!=" => [], "&=" => []}
621-
}
622-
expect(result).to eq(answer)
623-
end
624-
end
625610
end
626611
end
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
description = This group provides access to resources; expiration = 2099-12-31 is not a predicate here
2+
username = mainecoon
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
description = This group manages auth; it also handles access control for the team
2+
username = mainecoon

spec/unit/fixtures/ldap-config/text/leading-semicolon.txt

Lines changed: 0 additions & 2 deletions
This file was deleted.

spec/unit/fixtures/ldap-config/text/only-semicolon.txt

Lines changed: 0 additions & 2 deletions
This file was deleted.

spec/unit/fixtures/ldap-config/text/trailing-semicolon.txt

Lines changed: 0 additions & 2 deletions
This file was deleted.

0 commit comments

Comments
 (0)