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

Add project_scope_tag_id scope-to-tag functionality to some api otu queries #4232

Draft
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

kleintom
Copy link
Contributor

No description provided.

@kleintom kleintom marked this pull request as draft March 10, 2025 20:19
@kleintom
Copy link
Contributor Author

In progress - there may still be some additions.


if project_scope_tag_id.present?
q = q.joins(:tags)
.where("tags.tag_object_type = 'Otu' AND tags.keyword_id = #{project_scope_tag_id}")
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 don't need tags.tag_object_type = 'Otu' here, it's covered by joins(:tags)

descendants = descendants
.joins(:otus)
.joins("JOIN tags ON tags.tag_object_type = 'Otu' AND tags.tag_object_id = otus.id")
.where("tags.keyword_id = #{project_scope_tag_id}")
Copy link
Member

Choose a reason for hiding this comment

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

where(:tags: {keyword_id: project_scope_tag_id}

# The project scope tag has the property that any ancestor of a tagged
# otu is also tagged, so this is legit.
descendants = descendants
.joins(:otus)
Copy link
Member

Choose a reason for hiding this comment

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

.joins(otus: [:tags]) probably

@@ -101,9 +120,12 @@ def otu_descendants_and_synonyms(
end
data[:leaf_node] = data[:descendants].empty?
else
data[:leaf_node] = descendants.where.not(id: similar_otus).none?
Copy link
Member

Choose a reason for hiding this comment

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

see about adding .unscope(:order) on call using descendant etc. it can save a lot of time`

@@ -26,6 +26,10 @@ class Autocomplete < Query::Autocomplete
# if 'true' then only #name = query_string results are returned (no fuzzy matching)
attr_accessor :exact

# @return Integer, nil
# Only return otus tagged with this tag id
attr_accessor :project_scope_tag_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'm worried about the scope in various other places, there are various other filter results that need this scoping that are used. Before we get too far I'd like to enumerate where else we'd need this tag param to ensure consistency across the site. Remember too that for basic OTUs this works fine, but as as soon as we add all the filters/search that others want to use this will be quite difficult to maintain without a lot of thought.

descendants = otu.taxon_name.descendants
.where(parent: otu.taxon_name).that_is_valid

if project_scope_tag_id
Copy link
Member

Choose a reason for hiding this comment

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

@kleintom Internal to catalog, rather than a tag Id. I think we should try to use an Otu scope as the join, that would let us pre-define any set of OTUs. The concern would be performance, but let's experiment with it.

External to Catalog their might be a configure wrapper, that handles creating the OTU scopes.

So in the tag case the otu_bounding_scope= Otu.joins(:tags).where(tags: {keyword_id: 123})

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