Skip to content

Commit

Permalink
Move trip search out of model method
Browse files Browse the repository at this point in the history
Moving this code closer to where it's used fixes a typing error and lets
us update some typing deps!
  • Loading branch information
DavidCain committed Dec 7, 2024
1 parent 848b0c1 commit ed55c51
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 64 deletions.
74 changes: 40 additions & 34 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ ruff = "*"
optional = true
[tool.poetry.group.mypy.dependencies]
celery-types = "^0.19.0"
django-stubs = { version = "==5.0.2" } # Temporarily pin for QuerySet issue
mypy = { version = "<1.12"} # Can unpin when `django-stubs` is unpinned
django-stubs = { version = "*" } # Temporarily pin for QuerySet issue
mypy = { version = "*"} # Can unpin when `django-stubs` is unpinned
types-certifi = { version = "*" }
types-cryptography = { version = "*" }
types-requests = { version = "*" }
Expand Down
4 changes: 1 addition & 3 deletions ws/decorators.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
from collections.abc import Callable, Collection
from functools import wraps
from typing import cast

from django.contrib.auth import REDIRECT_FIELD_NAME
from django.contrib.auth.decorators import user_passes_test
from django.contrib.auth.models import AnonymousUser, User
from django.contrib.auth.views import redirect_to_login
from django.core.exceptions import PermissionDenied
from django.shortcuts import resolve_url
Expand Down Expand Up @@ -106,6 +104,6 @@ def _wrapped_view(request, *args, **kwargs):
)

admin_only = user_passes_test(
lambda u: cast(User | AnonymousUser, u).is_superuser,
lambda u: u.is_superuser,
login_url=reverse_lazy("account_login"),
)
25 changes: 1 addition & 24 deletions ws/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from django.contrib.auth.models import AnonymousUser, User
from django.contrib.contenttypes.models import ContentType
from django.contrib.postgres.indexes import GistIndex
from django.contrib.postgres.search import SearchQuery, SearchRank, SearchVector
from django.contrib.postgres.search import SearchVector
from django.core.exceptions import ValidationError
from django.core.validators import MaxValueValidator, MinValueValidator, RegexValidator
from django.db import models
Expand Down Expand Up @@ -1426,29 +1426,6 @@ def leaders_with_rating(self):
"""All leaders with the rating they had at the time of the trip."""
return [leader.name_with_rating(self) for leader in self.leaders.all()]

@classmethod
def search_trips(
cls,
text: str,
filters: None | Q,
limit: int = 100,
) -> QuerySet["Trip"]:
trips = cls.objects.filter(filters) if filters else cls.objects.all()
# It's valid to not provide a search term at all.
# In this case, there's no real meaningful way to "rank" matches; put newest first.
if not text:
return trips.order_by("-pk")[:limit]

query = SearchQuery(text)
return (
trips.annotate(
search=trips_search_vector,
rank=SearchRank(trips_search_vector, query),
)
.filter(search=text)
.order_by("-rank")[:limit]
)

@property
def prefilled_atlas_form_link(self) -> str:
"""Pre-fill known information for submission on the Atlas form link."""
Expand Down
28 changes: 27 additions & 1 deletion ws/views/trips.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

from django.contrib import messages
from django.contrib.auth.decorators import login_required
from django.contrib.postgres.search import SearchQuery, SearchRank
from django.core.exceptions import PermissionDenied
from django.db import transaction
from django.db.models import Q, QuerySet
Expand Down Expand Up @@ -623,6 +624,31 @@ def get_context_data(self, **kwargs):
return context


def search_trips(
text: str,
filters: None | Q,
limit: int = 100,
) -> QuerySet[models.Trip]:
trips = models.Trip.objects.all()
if filters:
trips = trips.filter(filters)

# It's valid to not provide a search term at all.
# In this case, there's no real meaningful way to "rank" matches; put newest first.
if not text:
return trips.order_by("-pk")[:limit]

query = SearchQuery(text)
return (
trips.annotate(
search=models.trips_search_vector,
rank=SearchRank(models.trips_search_vector, query),
)
.filter(search=text)
.order_by("-rank")[:limit]
)


class TripSearchView(ListView, FormView):
form_class = forms.TripSearchForm

Expand Down Expand Up @@ -675,7 +701,7 @@ def get_queryset(self) -> QuerySet[models.Trip]:
if value and field in {"winter_terrain_level", "trip_type", "program"}
}
return annotated_for_trip_list(
models.Trip.search_trips(
search_trips(
query,
filters=Q(**specified_filters) if specified_filters else None,
limit=self.limit,
Expand Down

0 comments on commit ed55c51

Please sign in to comment.