Skip to content
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

Remove ontology attribute type #1647

Merged
merged 10 commits into from
Nov 9, 2023
Merged

Conversation

stuzart
Copy link
Member

@stuzart stuzart commented Nov 9, 2023

Remove 'Ontology' SampleAttributeType, which was really a duplicate of 'Controlled Vocabulary'

required removing some legacy javascript behaviour that filtered the list of CV's available, but based on .custom_input? rather than whether they are ontology based.
An upgrade task migrates existing sample attributes linked to 'Ontololgy' to 'Controlled Vocabularly'

added as isa_template_attributes, to be consistent with SampleType.isa_template (to avoid clash with general use of template)
moves sample and template attributes to CV, and then deletes the ontology one. Only deletes if both Ontology and CV attribute types are found
lead to some odd behaviour, and was based on custom_input rather than being based on an ontology
@stuzart stuzart requested a review from kdp-cloud November 9, 2023 10:54
Copy link
Collaborator

@kdp-cloud kdp-cloud left a comment

Choose a reason for hiding this comment

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

There is some old 'hash rocket' syntax in app/views/sample_types/_sample_attribute_form.html.erb and app/views/templates/_template_attribute_form.erb you might consider updating to the new JSON syntax.
Other than that, no comments.

@stuzart stuzart merged commit 8bef10a into seek-1.14 Nov 9, 2023
21 checks passed
@stuzart stuzart deleted the remove-ontology-attribute-type-1632 branch November 9, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants