Skip to content
New issue

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

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

Already on GitHub? # to your account

Update feature uniqueness validation to take into account revision context #1124

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

Conversation

susannasiebert
Copy link
Contributor

No description provided.

@susannasiebert susannasiebert added the bugfix PR Label for bug fixes. Will appear in release notes. label Oct 8, 2024
Copy link
Member

@acoffman acoffman left a comment

Choose a reason for hiding this comment

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

+1


duplicate_name = if in_revision_validation_context
base_query
.where.not(feature_instance_id: revision_target_id)
Copy link
Member

Choose a reason for hiding this comment

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

I think the revision target id here will always be the feature id, not the feature instance id, but we can double check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made that change specifically because I was running into this issue where the revision_target_id was the feature_instance_id when I was testing fusion variant coordinate revisions locally. However, maybe that is not intended and the revision should capture the feature id?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bugfix PR Label for bug fixes. Will appear in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants