-
Notifications
You must be signed in to change notification settings - Fork 57
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.
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: whomingbird <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for an allow_free_text flag in extended metadata controlled vocabulary attributes, enabling controlled vocabulary fields to optionally accept free-text values in addition to predefined terms.
- Added
allow_free_textto the EMT JSON schema for both basic and CV/linked attribute definitions - Persisted the flag from JSON imports to the
allow_cv_free_textdatabase column - Exposed
allow_free_textin the API serializer for consumer applications - Updated tests and fixtures to validate the new functionality
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
lib/seek/extended_metadata_type/extended_metadata_type_schema.json |
Added allow_free_text boolean property to both oneOf schema variants for attribute definitions |
lib/seek/extended_metadata_type/extended_metadata_type_extractor.rb |
Modified to extract and persist allow_free_text from JSON to allow_cv_free_text attribute |
app/serializers/extended_metadata_type_serializer.rb |
Added allow_free_text field to API response serialization |
public/api/definitions/_schemas.yml |
Added allow_free_text boolean property to API schema definition |
test/fixtures/files/extended_metadata_type/valid_emt_with_cv_with_ontologies.json |
Added "allow_free_text": true to test fixture |
test/unit/extended_metadatas/emt_extractor_test.rb |
Added assertion to verify allow_cv_free_text is correctly extracted |
test/factories/extended_metadata_types.rb |
Set allow_cv_free_text: true for CV list and false for CV attribute to support test scenarios |
test/functional/studies_controller_test.rb |
Added test case for free text validation behavior; numerous formatting improvements (spacing around hash colons) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "allow_free_text": { | ||
| "type": "boolean" | ||
| }, |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
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.
| "allow_free_text": { | |
| "type": "boolean" | |
| }, |
| "allow_free_text": { | ||
| "type": "boolean" | ||
| }, |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
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.
| required: attr['required'].present? ? attr['required'] : false, | ||
| allow_cv_free_text: attr['allow_free_text'].present? ? attr['allow_free_text'] : false, |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
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.
| 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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| raise StandardError, "The schema file is not readable!" unless File.readable?(SCHEMA_PATH) | ||
|
|
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is a duplicate of line 27 and should be removed.
| raise StandardError, "The schema file is not readable!" unless File.readable?(SCHEMA_PATH) |
Extended metadata type attributes needed to accept both controlled vocabulary suggestions and free text via an
allow_free_textflag in the EMT JSON schema.allow_free_textto EMT attribute schema (both basic and CV/linked variants) so JSON uploads can declare free-text allowance.allow_free_textintoallow_cv_free_textwhen creating extended metadata attributes from JSON.allow_free_textin the extended metadata type serializer for API consumers.allow_free_textbehavior.Example:
{ "attributes": [ { "title": "Topics", "type": "Controlled Vocabulary", "ID": "CV_TOPICS_ID", "allow_free_text": true } ] }Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
esm.ubuntu.com/usr/lib/apt/methods/https /usr/lib/apt/methods/https -I . -I ./libsass/include --gdwarf-5 --64 -o utf8_string.o /tmp/ccrlT5ol.s -I -g++ comp�� ITS=64 RCE cal/share/gem/ru-I or/bundle/ruby/3echo -I GES as(dns block)oss-binaries.phusionpassenger.com/usr/bin/curl curl -f -L -o rubyext-ruby-3.2.3-x86_64-linux.tar.gz.tmp -s -S --connect-timeout 4 --speed-time 5 --speed-limit 1 --max-time 10 REDACTED lient or/bundle/ruby/3-I p or/bundle/ruby/3-I(dns block)s3.amazonaws.com/usr/bin/curl curl -f -L -o rubyext-ruby-3.2.3-x86_64-linux.tar.gz.tmp -s -S --connect-timeout 4 --speed-time 5 --speed-limit 1 --max-time 10 REDACTED lient bgit2/deps/pcre p bgit2/deps/xdiff(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.