Conversation
There was a problem hiding this comment.
Pull request overview
Integrates Find-powered document search into the Docs UI/API, removing the previous infinite-scroll pagination approach and adding optional path-based scoping for “current doc” searches.
Changes:
- Frontend doc search now calls a dedicated
documents/search/endpoint via a newuseSearchDocshook and removes infinite pagination. - Backend search endpoint proxies to the configured Find indexer, adds optional
pathfiltering, and returns a simplified{count, results}response. - Development environment variables are updated to enable the indexer and store OIDC tokens in session.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchSubPageContent.tsx | Switches “current doc” search to Find-backed useSearchDocs with path scoping. |
| src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchModal.tsx | Removes legacy filter props from search content components (target selection remains). |
| src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchContent.tsx | Replaces infinite query + InView pagination with useSearchDocs. |
| src/frontend/apps/impress/src/features/docs/doc-management/api/useDocs.tsx | Removes title query param support from the list-docs hook params builder. |
| src/frontend/apps/impress/src/features/docs/doc-management/api/searchDocs.tsx | Adds new React Query hook + request param construction for Find search endpoint. |
| src/backend/core/services/search_indexers.py | Extends indexer search call to accept a path filter and forwards it to Find. |
| src/backend/core/api/viewsets.py | Refactors search logic, removes pagination for Find-backed results, and adds path plumbing. |
| src/backend/core/api/serializers.py | Updates search query serializer to drop pagination params and add optional path. |
| env.d/development/common | Enables token storage and enables the indexer in dev env. |
| docs/search.md | Minor wording correction in search configuration docs. |
| CHANGELOG.md | Adds an “integrate Find search” entry under Unreleased. |
Comments suppressed due to low confidence (1)
src/backend/core/api/serializers.py:985
pathis not trimmed, so a value like' 'will pass validation even though it’s effectively blank. To matchqbehavior and avoid confusing/ineffective filters, settrim_whitespace=Trueforpath(and keepallow_blank=False).
q = serializers.CharField(required=True, allow_blank=False, trim_whitespace=True)
path = serializers.CharField(required=False, allow_blank=False)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchSubPageContent.tsx
Outdated
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-management/api/useSearchDocs.tsx
Outdated
Show resolved
Hide resolved
9759654 to
ac3aac5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
src/backend/core/api/viewsets.py:1267
_search_simplestill returns a paginated response (get_response_for_queryset), while the frontend search hook no longer sends pagination params. If the indexer is not configured,nextwill be non-null and the frontend infinite-query will keep refetching page 1 (duplicates / potential loop). Consider making the fallback path return a non-paginated response consistent with_search(and apply the optionalpathfilter there too).
indexer = get_document_indexer()
if indexer:
return self._search_with_indexer(indexer, request, params=params)
# The indexer is not configured, we fallback on a simple icontains filter by the
# model field 'title'.
return self._search_simple(request, text=params.validated_data["q"])
src/backend/core/api/serializers.py:984
SearchDocumentSerializerremovedpage/page_sizeand addedpath, but there are existing backend tests that assert pagination validation and behavior for/documents/search/(e.g.core/tests/documents/test_api_documents_search.py::test_api_documents_search_paginationand..._invalid_params). These tests will need to be updated/removed, and new assertions should cover thepathfilter behavior.
q = serializers.CharField(required=True, allow_blank=False, trim_whitespace=True)
page_size = serializers.IntegerField(
docs/search.md:22
- The configuration example still references
core.services.search_indexers.FindDocumentIndexer, but there is no such class in the backend (the implemented class isSearchIndexer). Update the docs snippet to use the correct class path so users can enable search successfully.
Add those Django settings to the Docs application to enable the feature.
```shell
SEARCH_INDEXER_CLASS="core.services.search_indexers.FindDocumentIndexer"
SEARCH_INDEXER_COUNTDOWN=10 # Debounce delay in seconds for the indexer calls.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/frontend/apps/impress/src/features/docs/doc-management/api/useSearchDocs.tsx
Outdated
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-management/api/useSearchDocs.tsx
Outdated
Show resolved
Hide resolved
984678c to
c1f4b12
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (3)
src/backend/core/api/viewsets.py:1180
- This change removes the
GET /documents/{id}/descendants/action entirely (tests have been migrated toGET /documents/search/?path=...). If external clients rely on the descendants endpoint, this is a breaking API change and should be called out explicitly (and/or handled via a backward-compatible alias/deprecation path). At minimum, consider updating the API documentation/changelog to mention the removal of the descendants route and its replacement.
@drf.decorators.action(detail=False, methods=["get"], url_path="search")
@method_decorator(refresh_oidc_access_token)
def search(self, request, *args, **kwargs):
"""
Returns a DRF response containing the filtered, annotated and ordered document list.
Applies filtering based on request parameter 'q' from `SearchDocumentSerializer`.
Depending of the configuration it can be:
- A fulltext search through the opensearch indexation app "find" if the backend is
enabled (see SEARCH_INDEXER_CLASS)
- A filtering by the model field 'title'.
The ordering is always by the most recent first.
src/backend/core/tests/documents/test_api_documents_search_descendants.py:283
- Variable _grand_child is not used.
src/backend/core/tests/documents/test_api_documents_search_descendants.py:521 - Variable _grand_child is not used.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchContent.tsx
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-management/api/useSearchDocs.tsx
Outdated
Show resolved
Hide resolved
c1f4b12 to
5e40d08
Compare
frontend is now calling the search endpoint
I am adding the path params to the search endpoint. This allows searching in subdocs.
I did a mistake in the env variables file
pagination is no longer supported by Find
handle filtering on sub-docs
I am fixing a bug about dupplicated documents
I update the changelog
I am removing dead code
I am fixing various things
I am adding this file containing the variables needed to run test without docker Signed-off-by: charles <charles.englebert@protonmail.com>
I remove DB access Signed-off-by: charles <charles.englebert@protonmail.com>
I am adding a title_search with all the logic Signed-off-by: charles <charles.englebert@protonmail.com>
The new logic allows to remove DocSearchSubPageContent.tsx Signed-off-by: charles <charles.englebert@protonmail.com>
I rename SearchIndexer to FindeDocumentIndexer
I am adding a simple test about the search method and search api Signed-off-by: charles <charles.englebert@protonmail.com>
I am fixing the _list_descendants logic based on the parent path istead of the parent id. Signed-off-by: charles <charles.englebert@protonmail.com>
I am fixing various things Signed-off-by: charles <charles.englebert@protonmail.com>
I am improving useSearchDocs by removing the q overwriting and circular imports
25153e1 to
a50c694
Compare
|
Size Change: -132 B (0%) Total Size: 4.2 MB
|
I am fixing various small things Signed-off-by: charles <charles.englebert@protonmail.com>
a50c694 to
d0c710e
Compare
3cccd8a to
d0c710e
Compare
Purpose
integrate Find to Docs
Proposal
useSeachDocshook in charged of calling the search endpoint.pathparam to thesearchroute. This param represents the parent document path in case of a sub-documents (descendants) search.DocSearchContentcomponents.DocSearchContentandDocSearchSubContentare now merged a unique component handling all search scenarios and relying on the uniquesearchroute.document/<document_id>/descendantsroute. This route is not used anymore. The logic of finding the descendants are moved to the internal_list_descendantsmethod. This method is based on the parentpathinstead of the parentidwhich has some consequence about the user access management. Relying on the path prevents the use of theself.get_object()method which used to handle the user access logic.titlefield. Find returns titles with a language extension (ex:{ title.fr: "rapport d'activité" }au lieu de { "title": "rapport d'activité" }common.testfile to allow running the tests without dockerSearchIndexer->FindDocumentIndexer. This class has to do with Find in particular and the convention is more coherent withBaseDocumentIndexerExternal contributions
Thank you for your contribution! 🎉
Please ensure the following items are checked before submitting your pull request:
git commit --signoff(DCO compliance)git commit -S)<gitmoji>(type) title description## [Unreleased]section (if noticeable change)