Skip to content

Support allow_free_text in extended metadata controlled vocab attributes #2451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Support allow_free_text in extended metadata controlled vocab attributes #2451

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
58267e4
Initial plan
Copilot Dec 10, 2025
9dee27a
Support allow_free_text for extended metadata CV attributes
Copilot Dec 10, 2025
a1e197a
test update for "allow_free_text" in CV type
whomingbird Dec 11, 2025
b742837
Add allow_free_text property to schema for boolean validation
whomingbird Dec 11, 2025
fc06f54
Rename allow_free_text to allow_cv_free_text in schema and related files
whomingbird Dec 11, 2025
da05b60
remove a duplicate line
whomingbird Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/serializers/extended_metadata_type_serializer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def get_extended_metadata_attribute(attribute)
"sample_attribute_type": get_sample_attribute_type(attribute),
"required": attribute.required,
"pos": attribute.pos,
"allow_free_text": attribute.allow_cv_free_text,
"sample_controlled_vocab_id": attribute.sample_controlled_vocab_id&.to_s,
"linked_extended_metadata_type_id": attribute.linked_extended_metadata_type_id&.to_s
}
Expand Down
3 changes: 2 additions & 1 deletion lib/seek/extended_metadata_type/extended_metadata_type_extractor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,12 @@ def self.create_extended_metadata_type_from_json(data)
sample_controlled_vocab: sample_controlled_vocab,
linked_extended_metadata_type: linked_extended_metadata_type,
required: attr['required'].present? ? attr['required'] : false,
allow_cv_free_text: attr['allow_free_text'].present? ? attr['allow_free_text'] : false,
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using .present? to check for a boolean attribute is non-idiomatic. While it works in this case (because false.present? returns false, so it defaults to false), it's clearer to use .key? to check for the attribute's existence. Consider: allow_cv_free_text: attr.key?('allow_free_text') ? attr['allow_free_text'] : false. This makes the intent clearer and handles both true and false values explicitly. Note that line 65 has the same pattern for the required attribute.

Suggested change
required: attr['required'].present? ? attr['required'] : false,
allow_cv_free_text: attr['allow_free_text'].present? ? attr['allow_free_text'] : false,
required: attr.key?('required') ? attr['required'] : false,
allow_cv_free_text: attr.key?('allow_free_text') ? attr['allow_free_text'] : false,

Copilot uses AI. Check for mistakes.
pid: attr['pid'].present? ? attr['pid'] : nil
)
end
emt
end
end
end
end
end
8 changes: 7 additions & 1 deletion lib/seek/extended_metadata_type/extended_metadata_type_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
"description": {
"type": ["string", "null"]
},
"allow_free_text": {
"type": "boolean"
},
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The allow_free_text property is defined in the basic attribute schema variant (first oneOf branch) which doesn't restrict the type to controlled vocabulary types. This means allow_free_text could be set for non-CV attribute types like "String", "Integer", "Date", etc., where it has no meaning. Consider either: (1) only including allow_free_text in the second oneOf branch (lines 70-72) where controlled vocabulary types are explicitly enumerated, or (2) adding validation logic to ignore or reject allow_free_text when the type is not a controlled vocabulary type.

Suggested change
"allow_free_text": {
"type": "boolean"
},

Copilot uses AI. Check for mistakes.
"label": {
"type": "string"
},
Expand Down Expand Up @@ -64,6 +67,9 @@
"description": {
"type": ["string", "null"]
},
"allow_free_text": {
"type": "boolean"
},
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The allow_free_text property is defined for all types in this schema variant, including "Linked Extended Metadata" and "Linked Extended Metadata (multiple)" types where free text doesn't apply (these link to other metadata structures, not controlled vocabularies). While this may be intentionally permissive, consider whether allow_free_text should only be applicable to "Controlled Vocabulary" and "Controlled Vocabulary List" types in the enum on lines 60-65.

Copilot uses AI. Check for mistakes.
"label": {
"type": "string"
},
Expand All @@ -86,4 +92,4 @@
},
"required": ["title", "supported_type", "enabled", "attributes"],
"additionalProperties": false
}
}
2 changes: 2 additions & 0 deletions public/api/definitions/_schemas.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4840,6 +4840,8 @@ extendedMetadataTypeExtendedMetadataAttributeResponse:
type: boolean
pos:
$ref: "#/components/schemas/nullableInteger"
allow_free_text:
type: boolean
sample_controlled_vocab_id:
$ref: "#/components/schemas/nullableNonEmptyString"
linked_extended_metadata_type_id:
Expand Down
4 changes: 2 additions & 2 deletions test/factories/extended_metadata_types.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@
after(:build) do |a|
a.extended_metadata_attributes << FactoryBot.create(:name_extended_metadata_attribute, title:'apple name')
cv_list_attribute = ExtendedMetadataAttribute.new(title: 'apple list', sample_attribute_type: FactoryBot.create(:cv_list_attribute_type),
sample_controlled_vocab: FactoryBot.create(:apples_sample_controlled_vocab), description: "apple samples list", label: "apple samples list")
sample_controlled_vocab: FactoryBot.create(:apples_sample_controlled_vocab), description: "apple samples list", label: "apple samples list", allow_cv_free_text: true)
a.extended_metadata_attributes << cv_list_attribute
cv_attribute = ExtendedMetadataAttribute.new(title: 'apple controlled vocab', sample_attribute_type: FactoryBot.create(:controlled_vocab_attribute_type),
sample_controlled_vocab: FactoryBot.create(:apples_sample_controlled_vocab), description: "apple samples controlled vocab", label: "apple samples controlled vocab")
sample_controlled_vocab: FactoryBot.create(:apples_sample_controlled_vocab), description: "apple samples controlled vocab", label: "apple samples controlled vocab", allow_cv_free_text: false)
a.extended_metadata_attributes << cv_attribute
end
end
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/files/extended_metadata_type/valid_emt_with_cv_with_ontologies.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"description": "Topics, used for annotating. Describes the domain, field of interest, of study, application, work, data, or technology. Initially seeded from the EDAM ontology.",
"type": "Controlled Vocabulary",
"required": true,
"allow_free_text": true,
"pos": 1,
"pid": "http://edamontology.org/topic_0003",
"ID": "CV_TOPICS_ID"
Expand Down
258 changes: 138 additions & 120 deletions test/functional/studies_controller_test.rb
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion test/unit/extended_metadatas/emt_extractor_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ class EmtExtractorTest < ActiveSupport::TestCase
assert_equal SampleAttributeType.find_by(title: 'Controlled Vocabulary'), topics_attr.sample_attribute_type
assert topics_attr.required
assert_equal 'http://edamontology.org/topic_0003', topics_attr.pid
assert topics_attr.allow_cv_free_text

assert_equal topic_cv, topics_attr.sample_controlled_vocab
assert_equal 4, topics_attr.sample_controlled_vocab.sample_controlled_vocab_terms.count
Expand Down Expand Up @@ -153,4 +154,3 @@ def update_id(emt_file_name, person_emt, replaced)

end