-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Add page search endpoint #64
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces a new search endpoint by adding a PageSearchView that invokes Page.objects.search and filters the results, and exposes it via a dedicated URL route. Sequence diagram for the new page search endpointsequenceDiagram
actor User
participant API as "PageSearchView"
participant Page as "Page.objects"
participant PageContent as "PageContent.objects"
User->>API: GET /<language>/search_pages/?q=term
API->>Page: search(term, language, current_site_only=False)
Page-->>API: queryset of matching pages
API->>PageContent: filter(page__in=qs).distinct()
PageContent-->>API: queryset of matching PageContent
API-->>User: Response with search results
Class diagram for the new PageSearchViewclassDiagram
class PageListView
class PageSearchView {
+get(request, language)
+get_queryset()
-search_term
-language
}
PageSearchView --|> PageListView
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
==========================================
- Coverage 91.75% 91.01% -0.74%
==========================================
Files 18 18
Lines 825 835 +10
Branches 89 90 +1
==========================================
+ Hits 757 760 +3
- Misses 42 49 +7
Partials 26 26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Consider renaming the "search_pages" endpoint to simply "search" to align with the existing URL conventions and keep things consistent.
- Add explicit handling for an empty
q
parameter (e.g. return an empty result or validation error) to avoid unintentionally returning all pages when no search term is provided. - Rather than overriding the
get
method, you might integrate the search logic into a filter backend or extend the existingget_queryset
flow to preserve pagination and DRF conventions without duplicating request handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider renaming the "search_pages" endpoint to simply "search" to align with the existing URL conventions and keep things consistent.
- Add explicit handling for an empty `q` parameter (e.g. return an empty result or validation error) to avoid unintentionally returning all pages when no search term is provided.
- Rather than overriding the `get` method, you might integrate the search logic into a filter backend or extend the existing `get_queryset` flow to preserve pagination and DRF conventions without duplicating request handling.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Add a page search endpoint to expose
Page.objects.search
via the REST API with pagination, and include a corresponding test to verify the search response structure.New Features:
Tests:
Fixes #60 by adding an endpoint for
Page.objects.search
method.Test added (but fails due to bug in django-cms): The
Page.objects.search
method is broken for django CMS 4+. Fix is here: django-cms/django-cms#8355