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 searchable names function to data_models #427

Open
wants to merge 14 commits into
base: dev
Choose a base branch
from

Conversation

CarsonDavis
Copy link
Collaborator

@CarsonDavis CarsonDavis commented Dec 21, 2022

Most models have a variety of possible names which can refer to an object. short_name, long_name, aliases, etc. A user desiring to search for a campaign based on a string will expect to see a result that is matched against any of the valid name fields.

This PR implements a list of possible valid name fields for each model and updates the search function to use these fields in the default search.

PROBLEMS:
Currently, the list of returned results in the queryset is not unique, and adding a .distinct('uuid') constraint seems to massively slow down the application.

@CarsonDavis CarsonDavis marked this pull request as ready for review January 10, 2023 21:14
Copy link
Contributor

@edkeeble edkeeble left a comment

Choose a reason for hiding this comment

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

Can you provide a test case demonstrating non-unique results? I can't replicate it with a quick test:

for c in Campaign.search({"search": "convective processes"}):
    print(c.short_name, c.long_name)

>> CPEX Convective Processes Experiment
>> CPEX-AW Convective Processes ExperimentAerosols & Winds

Does it occur when multiple fields for the same record match the search criteria?

Comment on lines +67 to 75
def custom_search_fields(cls):
all_model_field_names = [field.name for field in cls._meta.get_fields()]
actual_searchable_names = [
field
for field in POSSIBLE_SEARCHABLE_FIELDS
if field.split('__')[0] in all_model_field_names
]
return actual_searchable_names

Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than supporting a list of each possible search field and checking against all fields of each subclass, could we not just override search_fields on the subclasses?

e.g.

class Image(BaseModel):
    ...
    @staticmethod
    def search_fields():
        return ["title"]

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