diff --git a/CHANGELOG.md b/CHANGELOG.md index 282ce166f4..8ce126ce7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to - ✨(frontend) Add stat for Crisp #1824 - ✨(auth) add silent login #1690 - 🔧(project) add DJANGO_EMAIL_URL_APP environment variable #1825 +- ✨ integrate Find search ### Changed diff --git a/README.md b/README.md index 1b059d305e..561322aa50 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,10 @@ $ make frontend-test $ make frontend-lint ``` +Backend tests can be run without docker with the env files +`env.d/development/common` and `env.d/development/common.test`. +`common.test` must overwrite some variables in `common`. + **Adding content** You can create a basic demo site by running this command: diff --git a/docs/search.md b/docs/search.md index 416f972bd3..e6f35d2dbb 100644 --- a/docs/search.md +++ b/docs/search.md @@ -15,7 +15,7 @@ See [how-to-use-indexer.md](how-to-use-indexer.md) for details. ## Configure settings of Docs -Add those Django settings the Docs application to enable the feature. +Add those Django settings to the Docs application to enable the feature. ```shell SEARCH_INDEXER_CLASS="core.services.search_indexers.FindDocumentIndexer" diff --git a/env.d/development/common b/env.d/development/common index fad2bac597..231f0fda74 100644 --- a/env.d/development/common +++ b/env.d/development/common @@ -52,8 +52,8 @@ OIDC_REDIRECT_ALLOWED_HOSTS="localhost:8083,localhost:3000" OIDC_AUTH_REQUEST_EXTRA_PARAMS={"acr_values": "eidas1"} # Store OIDC tokens in the session. Needed by search/ endpoint. -# OIDC_STORE_ACCESS_TOKEN = True -# OIDC_STORE_REFRESH_TOKEN = True # Store the encrypted refresh token in the session. +OIDC_STORE_ACCESS_TOKEN=True +OIDC_STORE_REFRESH_TOKEN=True # Store the encrypted refresh token in the session. # Must be a valid Fernet key (32 url-safe base64-encoded bytes) # To create one, use the bin/fernetkey command. @@ -82,8 +82,9 @@ DOCSPEC_API_URL=http://docspec:4000/conversion # Theme customization THEME_CUSTOMIZATION_CACHE_TIMEOUT=15 -# Indexer (disabled) -# SEARCH_INDEXER_CLASS="core.services.search_indexers.SearchIndexer" +# Indexer +SEARCH_INDEXER_CLASS=core.services.search_indexers.FindDocumentIndexer SEARCH_INDEXER_SECRET=find-api-key-for-docs-with-exactly-50-chars-length # Key generated by create_demo in Find app. -SEARCH_INDEXER_URL="http://find:8000/api/v1.0/documents/index/" -SEARCH_INDEXER_QUERY_URL="http://find:8000/api/v1.0/documents/search/" +SEARCH_INDEXER_URL=http://find:8000/api/v1.0/documents/index/ +SEARCH_INDEXER_QUERY_URL=http://find:8000/api/v1.0/documents/search/ +SEARCH_INDEXER_QUERY_LIMIT=50 diff --git a/env.d/development/common.test b/env.d/development/common.test new file mode 100644 index 0000000000..476a0d6b80 --- /dev/null +++ b/env.d/development/common.test @@ -0,0 +1,7 @@ +# Test environment configuration for running tests without docker +# Base configuration is loaded from 'common' file + +DJANGO_SETTINGS_MODULE=impress.settings +DJANGO_CONFIGURATION=Test +DB_PORT=15432 +AWS_S3_ENDPOINT_URL=http://localhost:9000 \ No newline at end of file diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index affd9b0707..80df97f84d 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -12,6 +12,7 @@ ACTION_FOR_METHOD_TO_PERMISSION = { "versions_detail": {"DELETE": "versions_destroy", "GET": "versions_retrieve"}, "children": {"GET": "children_list", "POST": "children_create"}, + "search": {"GET": "retrieve"}, } diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 349e0191e5..2471ecedb1 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -980,8 +980,5 @@ def get_abilities(self, thread): class SearchDocumentSerializer(serializers.Serializer): """Serializer for fulltext search requests through Find application""" - q = serializers.CharField(required=True, allow_blank=False, trim_whitespace=True) - page_size = serializers.IntegerField( - required=False, min_value=1, max_value=50, default=20 - ) - page = serializers.IntegerField(required=False, min_value=1, default=1) + q = serializers.CharField(required=True, allow_blank=True, trim_whitespace=True) + path = serializers.CharField(required=False, allow_blank=False) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 85bc59e31b..10ed31e8c6 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -64,7 +64,11 @@ from core.utils import extract_attachments, filter_descendants from . import permissions, serializers, utils -from .filters import DocumentFilter, ListDocumentFilter, UserSearchFilter +from .filters import ( + DocumentFilter, + ListDocumentFilter, + UserSearchFilter, +) from .throttling import ( DocumentThrottle, UserListThrottleBurst, @@ -459,14 +463,12 @@ def list(self, request, *args, **kwargs): It performs early filtering on model fields, annotates user roles, and removes descendant documents to keep only the highest ancestors readable by the current user. """ - user = self.request.user + user = request.user # Not calling filter_queryset. We do our own cooking. queryset = self.get_queryset() - filterset = ListDocumentFilter( - self.request.GET, queryset=queryset, request=self.request - ) + filterset = ListDocumentFilter(request.GET, queryset=queryset, request=request) if not filterset.is_valid(): raise drf.exceptions.ValidationError(filterset.errors) filter_data = filterset.form.cleaned_data @@ -956,26 +958,6 @@ def all(self, request, *args, **kwargs): return self.get_response_for_queryset(queryset) - @drf.decorators.action( - detail=True, - methods=["get"], - ordering=["path"], - ) - def descendants(self, request, *args, **kwargs): - """Handle listing descendants of a document""" - document = self.get_object() - - queryset = document.get_descendants().filter(ancestors_deleted_at__isnull=True) - queryset = self.filter_queryset(queryset) - - filterset = DocumentFilter(request.GET, queryset=queryset) - if not filterset.is_valid(): - raise drf.exceptions.ValidationError(filterset.errors) - - queryset = filterset.qs - - return self.get_response_for_queryset(queryset) - @drf.decorators.action( detail=True, methods=["get"], @@ -1183,57 +1165,6 @@ def duplicate(self, request, *args, **kwargs): {"id": str(duplicated_document.id)}, status=status.HTTP_201_CREATED ) - def _search_simple(self, request, text): - """ - Returns a queryset filtered by the content of the document title - """ - # As the 'list' view we get a prefiltered queryset (deleted docs are excluded) - queryset = self.get_queryset() - filterset = DocumentFilter({"title": text}, queryset=queryset) - - if not filterset.is_valid(): - raise drf.exceptions.ValidationError(filterset.errors) - - queryset = filterset.filter_queryset(queryset) - - return self.get_response_for_queryset( - queryset.order_by("-updated_at"), - context={ - "request": request, - }, - ) - - def _search_fulltext(self, indexer, request, params): - """ - Returns a queryset from the results the fulltext search of Find - """ - access_token = request.session.get("oidc_access_token") - user = request.user - text = params.validated_data["q"] - queryset = models.Document.objects.all() - - # Retrieve the documents ids from Find. - results = indexer.search( - text=text, - token=access_token, - visited=get_visited_document_ids_of(queryset, user), - ) - - docs_by_uuid = {str(d.pk): d for d in queryset.filter(pk__in=results)} - ordered_docs = [docs_by_uuid[id] for id in results] - - page = self.paginate_queryset(ordered_docs) - - serializer = self.get_serializer( - page if page else ordered_docs, - many=True, - context={ - "request": request, - }, - ) - - return self.get_paginated_response(serializer.data) - @drf.decorators.action(detail=False, methods=["get"], url_path="search") @method_decorator(refresh_oidc_access_token) def search(self, request, *args, **kwargs): @@ -1252,13 +1183,82 @@ def search(self, request, *args, **kwargs): params.is_valid(raise_exception=True) indexer = get_document_indexer() - if indexer: - return self._search_fulltext(indexer, request, params=params) + return self._search_with_indexer(indexer, request, params=params) + + # The indexer is not configured, we fallback on title search + return self.title_search(request, params.validated_data, *args, **kwargs) - # 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"]) + @staticmethod + def _search_with_indexer(indexer, request, params): + """ + Returns a list of documents matching the query (q) according to the configured indexer. + """ + queryset = models.Document.objects.all() + user_query = params.validated_data["q"] + + results = indexer.search( + q=user_query if user_query != "" else "*", + token=request.session.get("oidc_access_token"), + path=( + params.validated_data["path"] + if "path" in params.validated_data + else None + ), + visited=get_visited_document_ids_of(queryset, request.user), + ) + + return drf_response.Response( + { + "count": len(results), + "next": None, + "previous": None, + "results": results, + } + ) + + def title_search(self, request, validated_data, *args, **kwargs): + """ + Fallback search by title when indexer is not configured. + If path is provided, list descendants, otherwise list all documents. + """ + request.GET = request.GET.copy() + request.GET["title"] = validated_data["q"] + if not "path" in request.GET or not request.GET["path"]: + return self.list(request, *args, **kwargs) + return self._list_descendants(request) + + def _list_descendants(self, request): + """ + List all documents whose path starts with the provided path parameter. + Includes the parent document itself. + Used internally by the search endpoint when path filtering is requested. + """ + # Get parent document without access filtering + parent_path = request.GET["path"] + try: + parent = models.Document.objects.get(path=parent_path) + except models.Document.DoesNotExist as exc: + raise drf.exceptions.NotFound("Document not found from path.") from exc + + # Check object-level permissions using DocumentPermission logic + self.check_object_permissions(request, parent) + + # Get descendants and include the parent, ordered by path + queryset = ( + parent.get_descendants(include_self=True) + .filter(ancestors_deleted_at__isnull=True) + .order_by("path") + ) + queryset = self.filter_queryset(queryset) + + # filter by title + filterset = DocumentFilter(request.GET, queryset=queryset) + if not filterset.is_valid(): + raise drf.exceptions.ValidationError(filterset.errors) + + queryset = filterset.qs + return self.get_response_for_queryset(queryset) @drf.decorators.action(detail=True, methods=["get"], url_path="versions") def versions_list(self, request, *args, **kwargs): diff --git a/src/backend/core/services/search_indexers.py b/src/backend/core/services/search_indexers.py index a4bb9eec6b..a08a3f0aa3 100644 --- a/src/backend/core/services/search_indexers.py +++ b/src/backend/core/services/search_indexers.py @@ -8,7 +8,6 @@ from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.core.exceptions import ImproperlyConfigured -from django.db.models import Subquery from django.utils.module_loading import import_string import requests @@ -78,7 +77,9 @@ def get_visited_document_ids_of(queryset, user): if isinstance(user, AnonymousUser): return [] - qs = models.LinkTrace.objects.filter(user=user) + visited_ids = models.LinkTrace.objects.filter(user=user).values_list( + "document_id", flat=True + ) docs = ( queryset.exclude(accesses__user=user) @@ -86,7 +87,7 @@ def get_visited_document_ids_of(queryset, user): deleted_at__isnull=True, ancestors_deleted_at__isnull=True, ) - .filter(pk__in=Subquery(qs.values("document_id"))) + .filter(pk__in=visited_ids) .order_by("pk") .distinct("pk") ) @@ -185,7 +186,7 @@ def push(self, data): """ # pylint: disable-next=too-many-arguments,too-many-positional-arguments - def search(self, text, token, visited=(), nb_results=None): + def search(self, q, token, visited=(), nb_results=None, path=None): """ Search for documents in Find app. Ensure the same default ordering as "Docs" list : -updated_at @@ -193,7 +194,7 @@ def search(self, text, token, visited=(), nb_results=None): Returns ids of the documents Args: - text (str): Text search content. + q (str): user query. token (str): OIDC Authentication token. visited (list, optional): List of ids of active public documents with LinkTrace @@ -201,21 +202,24 @@ def search(self, text, token, visited=(), nb_results=None): nb_results (int, optional): The number of results to return. Defaults to 50 if not specified. + path (str, optional): + The parent path to search descendants of. """ nb_results = nb_results or self.search_limit - response = self.search_query( + results = self.search_query( data={ - "q": text, + "q": q, "visited": visited, "services": ["docs"], "nb_results": nb_results, "order_by": "updated_at", "order_direction": "desc", + "path": path, }, token=token, ) - return [d["_id"] for d in response] + return results @abstractmethod def search_query(self, data, token) -> dict: @@ -226,11 +230,57 @@ def search_query(self, data, token) -> dict: """ -class SearchIndexer(BaseDocumentIndexer): +class FindDocumentIndexer(BaseDocumentIndexer): """ - Document indexer that pushes documents to La Suite Find app. + Document indexer that indexes and searches documents with La Suite Find app. """ + # pylint: disable=too-many-arguments,too-many-positional-arguments + def search(self, q, token, visited=(), nb_results=None, path=None): + """format Find search results""" + search_results = super().search(q, token, visited, nb_results, path) + return [ + { + **hit["_source"], + "id": hit["_id"], + "title": self.get_title(hit["_source"]), + } + for hit in search_results + ] + + @staticmethod + def get_title(source): + """ + Find returns the titles with an extension depending on the language. + This function extracts the title in a generic way. + + Handles multiple cases: + - Localized title fields like "title." + - Fallback to plain "title" field if localized version not found + - Returns empty string if no title field exists + + Args: + source (dict): The _source dictionary from a search hit + + Returns: + str: The extracted title or empty string if not found + + Example: + >>> get_title({"title.fr": "Bonjour", "id": 1}) + "Bonjour" + >>> get_title({"title": "Hello", "id": 1}) + "Hello" + >>> get_title({"id": 1}) + "" + """ + titles = utils.get_value_by_pattern(source, r"^title\.") + for title in titles: + if title: + return title + if "title" in source: + return source["title"] + return "" + def serialize_document(self, document, accesses): """ Convert a Document to the JSON format expected by La Suite Find. diff --git a/src/backend/core/tasks/search.py b/src/backend/core/tasks/search.py index 4b30c6a7de..e1c39e6bea 100644 --- a/src/backend/core/tasks/search.py +++ b/src/backend/core/tasks/search.py @@ -63,7 +63,7 @@ def batch_document_indexer_task(timestamp): logger.info("Indexed %d documents", count) -def trigger_batch_document_indexer(item): +def trigger_batch_document_indexer(document): """ Trigger indexation task with debounce a delay set by the SEARCH_INDEXER_COUNTDOWN setting. @@ -82,14 +82,14 @@ def trigger_batch_document_indexer(item): if batch_indexer_throttle_acquire(timeout=countdown): logger.info( "Add task for batch document indexation from updated_at=%s in %d seconds", - item.updated_at.isoformat(), + document.updated_at.isoformat(), countdown, ) batch_document_indexer_task.apply_async( - args=[item.updated_at], countdown=countdown + args=[document.updated_at], countdown=countdown ) else: - logger.info("Skip task for batch document %s indexation", item.pk) + logger.info("Skip task for batch document %s indexation", document.pk) else: - document_indexer_task.apply(args=[item.pk]) + document_indexer_task.apply(args=[document.pk]) diff --git a/src/backend/core/tests/commands/test_index.py b/src/backend/core/tests/commands/test_index.py index ad7d39e6e0..78d3024958 100644 --- a/src/backend/core/tests/commands/test_index.py +++ b/src/backend/core/tests/commands/test_index.py @@ -11,7 +11,7 @@ import pytest from core import factories -from core.services.search_indexers import SearchIndexer +from core.services.search_indexers import FindDocumentIndexer @pytest.mark.django_db @@ -19,7 +19,7 @@ def test_index(): """Test the command `index` that run the Find app indexer for all the available documents.""" user = factories.UserFactory() - indexer = SearchIndexer() + indexer = FindDocumentIndexer() with transaction.atomic(): doc = factories.DocumentFactory() @@ -36,7 +36,7 @@ def test_index(): str(no_title_doc.path): {"users": [user.sub]}, } - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: call_command("index") push_call_args = [call.args[0] for call in mock_push.call_args_list] diff --git a/src/backend/core/tests/conftest.py b/src/backend/core/tests/conftest.py index 65e3926934..4e357fae94 100644 --- a/src/backend/core/tests/conftest.py +++ b/src/backend/core/tests/conftest.py @@ -39,7 +39,7 @@ def indexer_settings_fixture(settings): get_document_indexer.cache_clear() - settings.SEARCH_INDEXER_CLASS = "core.services.search_indexers.SearchIndexer" + settings.SEARCH_INDEXER_CLASS = "core.services.search_indexers.FindDocumentIndexer" settings.SEARCH_INDEXER_SECRET = "ThisIsAKeyForTest" settings.SEARCH_INDEXER_URL = "http://localhost:8081/api/v1.0/documents/index/" settings.SEARCH_INDEXER_QUERY_URL = ( diff --git a/src/backend/core/tests/documents/test_api_documents_descendants_filters.py b/src/backend/core/tests/documents/test_api_documents_descendants_filters.py deleted file mode 100644 index 342ead7056..0000000000 --- a/src/backend/core/tests/documents/test_api_documents_descendants_filters.py +++ /dev/null @@ -1,95 +0,0 @@ -""" -Tests for Documents API endpoint in impress's core app: list -""" - -import pytest -from faker import Faker -from rest_framework.test import APIClient - -from core import factories -from core.api.filters import remove_accents - -fake = Faker() -pytestmark = pytest.mark.django_db - - -# Filters: unknown field - - -def test_api_documents_descendants_filter_unknown_field(): - """ - Trying to filter by an unknown field should be ignored. - """ - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - factories.DocumentFactory() - - document = factories.DocumentFactory(users=[user]) - expected_ids = { - str(document.id) - for document in factories.DocumentFactory.create_batch(2, parent=document) - } - - response = client.get( - f"/api/v1.0/documents/{document.id!s}/descendants/?unknown=true" - ) - - assert response.status_code == 200 - results = response.json()["results"] - assert len(results) == 2 - assert {result["id"] for result in results} == expected_ids - - -# Filters: title - - -@pytest.mark.parametrize( - "query,nb_results", - [ - ("Project Alpha", 1), # Exact match - ("project", 2), # Partial match (case-insensitive) - ("Guide", 2), # Word match within a title - ("Special", 0), # No match (nonexistent keyword) - ("2024", 2), # Match by numeric keyword - ("", 6), # Empty string - ("velo", 1), # Accent-insensitive match (velo vs vélo) - ("bêta", 1), # Accent-insensitive match (bêta vs beta) - ], -) -def test_api_documents_descendants_filter_title(query, nb_results): - """Authenticated users should be able to search documents by their unaccented title.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(users=[user]) - - # Create documents with predefined titles - titles = [ - "Project Alpha Documentation", - "Project Beta Overview", - "User Guide", - "Financial Report 2024", - "Annual Review 2024", - "Guide du vélo urbain", # <-- Title with accent for accent-insensitive test - ] - for title in titles: - factories.DocumentFactory(title=title, parent=document) - - # Perform the search query - response = client.get( - f"/api/v1.0/documents/{document.id!s}/descendants/?title={query:s}" - ) - - assert response.status_code == 200 - results = response.json()["results"] - assert len(results) == nb_results - - # Ensure all results contain the query in their title - for result in results: - assert ( - remove_accents(query).lower().strip() - in remove_accents(result["title"]).lower() - ) diff --git a/src/backend/core/tests/documents/test_api_documents_search.py b/src/backend/core/tests/documents/test_api_documents_search.py index c6d0d8e3ac..49bef7142d 100644 --- a/src/backend/core/tests/documents/test_api_documents_search.py +++ b/src/backend/core/tests/documents/test_api_documents_search.py @@ -1,46 +1,30 @@ """ -Tests for Documents API endpoint in impress's core app: list +Tests for Documents API endpoint in impress's core app: search """ -import random -from json import loads as json_loads - -from django.test import RequestFactory +from unittest import mock import pytest import responses from faker import Faker +from rest_framework import response as drf_response from rest_framework.test import APIClient -from core import factories, models +from core import factories from core.services.search_indexers import get_document_indexer fake = Faker() pytestmark = pytest.mark.django_db -def build_search_url(**kwargs): - """Build absolute uri for search endpoint with ORDERED query arguments""" - return ( - RequestFactory() - .get("/api/v1.0/documents/search/", dict(sorted(kwargs.items()))) - .build_absolute_uri() - ) - - -@pytest.mark.parametrize("role", models.LinkRoleChoices.values) -@pytest.mark.parametrize("reach", models.LinkReachChoices.values) @responses.activate -def test_api_documents_search_anonymous(reach, role, indexer_settings): +def test_api_documents_search_anonymous(indexer_settings): """ - Anonymous users should not be allowed to search documents whatever the - link reach and link role + Anonymous users should be allowed to search documents with Find. """ indexer_settings.SEARCH_INDEXER_QUERY_URL = "http://find/api/v1.0/search" - factories.DocumentFactory(link_reach=reach, link_role=role) - - # Find response + # mock Find response responses.add( responses.POST, "http://find/api/v1.0/search", @@ -48,7 +32,25 @@ def test_api_documents_search_anonymous(reach, role, indexer_settings): status=200, ) - response = APIClient().get("/api/v1.0/documents/search/", data={"q": "alpha"}) + with mock.patch( + "core.services.search_indexers.FindDocumentIndexer.search_query" + ) as search_query: + q = "alpha" + response = APIClient().get("/api/v1.0/documents/search/", data={"q": q}) + + assert search_query.call_count == 1 + assert search_query.call_args[1] == { + "data": { + "q": q, + "visited": [], + "services": ["docs"], + "nb_results": 50, + "order_by": "updated_at", + "order_direction": "desc", + "path": None, + }, + "token": None, + } assert response.status_code == 200 assert response.json() == { @@ -59,64 +61,78 @@ def test_api_documents_search_anonymous(reach, role, indexer_settings): } -def test_api_documents_search_endpoint_is_none(indexer_settings): +def test_api_documents_search_fall_back_on_search_list(indexer_settings): """ - Missing SEARCH_INDEXER_QUERY_URL, so the indexer is not properly configured. - Should fallback on title filter + When indexer is not configured and no path is provided, + should fall back on list method """ indexer_settings.SEARCH_INDEXER_QUERY_URL = None - assert get_document_indexer() is None user = factories.UserFactory() - document = factories.DocumentFactory(title="alpha") - access = factories.UserDocumentAccessFactory(document=document, user=user) + client = APIClient() + client.force_login(user) + + with mock.patch("core.api.viewsets.DocumentViewSet.list") as mock_list: + mocked_response = { + "count": 0, + "next": None, + "previous": None, + "results": [{"title": "mocked list result"}], + } + mock_list.return_value = drf_response.Response(mocked_response) + + response = client.get("/api/v1.0/documents/search/", data={"q": "alpha"}) + + assert mock_list.call_count == 1 + assert mock_list.call_args[0][0].GET.get("title") == "alpha" + assert response.json() == mocked_response + + +def test_api_documents_search_fallback_on_search_list_sub_docs(indexer_settings): + """ + When indexer is not configured and path parameter is provided, + should call _list_descendants() method + """ + indexer_settings.SEARCH_INDEXER_QUERY_URL = None + assert get_document_indexer() is None + + user = factories.UserFactory() client = APIClient() client.force_login(user) - response = client.get("/api/v1.0/documents/search/", data={"q": "alpha"}) + parent = factories.DocumentFactory(title="parent", users=[user]) - assert response.status_code == 200 - content = response.json() - results = content.pop("results") - assert content == { - "count": 1, - "next": None, - "previous": None, - } - assert len(results) == 1 - assert results[0] == { - "id": str(document.id), - "abilities": document.get_abilities(user), - "ancestors_link_reach": None, - "ancestors_link_role": None, - "computed_link_reach": document.computed_link_reach, - "computed_link_role": document.computed_link_role, - "created_at": document.created_at.isoformat().replace("+00:00", "Z"), - "creator": str(document.creator.id), - "depth": 1, - "excerpt": document.excerpt, - "link_reach": document.link_reach, - "link_role": document.link_role, - "nb_accesses_ancestors": 1, - "nb_accesses_direct": 1, - "numchild": 0, - "path": document.path, - "title": document.title, - "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), - "deleted_at": None, - "user_role": access.role, - } + with mock.patch( + "core.api.viewsets.DocumentViewSet._list_descendants" + ) as mock_list_descendants: + mocked_response = { + "count": 0, + "next": None, + "previous": None, + "results": [{"title": "mocked _list_descendants result"}], + } + mock_list_descendants.return_value = drf_response.Response(mocked_response) + + response = client.get( + "/api/v1.0/documents/search/", data={"q": "alpha", "path": parent.path} + ) + + assert mock_list_descendants.call_count == 1 + assert mock_list_descendants.call_args[0][0].GET.get("title") == "alpha" + assert mock_list_descendants.call_args[0][0].GET.get("path") == parent.path + + assert response.json() == mocked_response @responses.activate def test_api_documents_search_invalid_params(indexer_settings): """Validate the format of documents as returned by the search view.""" indexer_settings.SEARCH_INDEXER_QUERY_URL = "http://find/api/v1.0/search" + assert get_document_indexer() is not None user = factories.UserFactory() - client = APIClient() client.force_login(user) @@ -125,49 +141,28 @@ def test_api_documents_search_invalid_params(indexer_settings): assert response.status_code == 400 assert response.json() == {"q": ["This field is required."]} - response = client.get("/api/v1.0/documents/search/", data={"q": " "}) - - assert response.status_code == 400 - assert response.json() == {"q": ["This field may not be blank."]} - - response = client.get( - "/api/v1.0/documents/search/", data={"q": "any", "page": "NaN"} - ) - - assert response.status_code == 400 - assert response.json() == {"page": ["A valid integer is required."]} - @responses.activate -def test_api_documents_search_format(indexer_settings): +def test_api_documents_search_success(indexer_settings): """Validate the format of documents as returned by the search view.""" indexer_settings.SEARCH_INDEXER_QUERY_URL = "http://find/api/v1.0/search" - assert get_document_indexer() is not None - user = factories.UserFactory() - - client = APIClient() - client.force_login(user) - - user_a, user_b, user_c = factories.UserFactory.create_batch(3) - document = factories.DocumentFactory( - title="alpha", - users=(user_a, user_c), - link_traces=(user, user_b), - ) - access = factories.UserDocumentAccessFactory(document=document, user=user) + document = {"id": "doc-123", "title": "alpha", "path": "path/to/alpha.pdf"} # Find response responses.add( responses.POST, "http://find/api/v1.0/search", json=[ - {"_id": str(document.pk)}, + { + "_id": str(document["id"]), + "_source": {"title": document["title"], "path": document["path"]}, + }, ], status=200, ) - response = client.get("/api/v1.0/documents/search/", data={"q": "alpha"}) + response = APIClient().get("/api/v1.0/documents/search/", data={"q": "alpha"}) assert response.status_code == 200 content = response.json() @@ -177,249 +172,6 @@ def test_api_documents_search_format(indexer_settings): "next": None, "previous": None, } - assert len(results) == 1 - assert results[0] == { - "id": str(document.id), - "abilities": document.get_abilities(user), - "ancestors_link_reach": None, - "ancestors_link_role": None, - "computed_link_reach": document.computed_link_reach, - "computed_link_role": document.computed_link_role, - "created_at": document.created_at.isoformat().replace("+00:00", "Z"), - "creator": str(document.creator.id), - "depth": 1, - "excerpt": document.excerpt, - "link_reach": document.link_reach, - "link_role": document.link_role, - "nb_accesses_ancestors": 3, - "nb_accesses_direct": 3, - "numchild": 0, - "path": document.path, - "title": document.title, - "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), - "deleted_at": None, - "user_role": access.role, - } - - -@responses.activate -@pytest.mark.parametrize( - "pagination, status, expected", - ( - ( - {"page": 1, "page_size": 10}, - 200, - { - "count": 10, - "previous": None, - "next": None, - "range": (0, None), - }, - ), - ( - {}, - 200, - { - "count": 10, - "previous": None, - "next": None, - "range": (0, None), - "api_page_size": 21, # default page_size is 20 - }, - ), - ( - {"page": 2, "page_size": 10}, - 404, - {}, - ), - ( - {"page": 1, "page_size": 5}, - 200, - { - "count": 10, - "previous": None, - "next": {"page": 2, "page_size": 5}, - "range": (0, 5), - }, - ), - ( - {"page": 2, "page_size": 5}, - 200, - { - "count": 10, - "previous": {"page_size": 5}, - "next": None, - "range": (5, None), - }, - ), - ({"page": 3, "page_size": 5}, 404, {}), - ), -) -def test_api_documents_search_pagination( - indexer_settings, pagination, status, expected -): - """Documents should be ordered by descending "score" by default""" - indexer_settings.SEARCH_INDEXER_QUERY_URL = "http://find/api/v1.0/search" - - assert get_document_indexer() is not None - - user = factories.UserFactory() - - client = APIClient() - client.force_login(user) - - docs = factories.DocumentFactory.create_batch(10, title="alpha", users=[user]) - - docs_by_uuid = {str(doc.pk): doc for doc in docs} - api_results = [{"_id": id} for id in docs_by_uuid.keys()] - - # reorder randomly to simulate score ordering - random.shuffle(api_results) - - # Find response - # pylint: disable-next=assignment-from-none - api_search = responses.add( - responses.POST, - "http://find/api/v1.0/search", - json=api_results, - status=200, - ) - - response = client.get( - "/api/v1.0/documents/search/", - data={ - "q": "alpha", - **pagination, - }, - ) - - assert response.status_code == status - - if response.status_code < 300: - previous_url = ( - build_search_url(q="alpha", **expected["previous"]) - if expected["previous"] - else None - ) - next_url = ( - build_search_url(q="alpha", **expected["next"]) - if expected["next"] - else None - ) - start, end = expected["range"] - - content = response.json() - - assert content["count"] == expected["count"] - assert content["previous"] == previous_url - assert content["next"] == next_url - - results = content.pop("results") - - # The find api results ordering by score is kept - assert [r["id"] for r in results] == [r["_id"] for r in api_results[start:end]] - - # Check the query parameters. - assert api_search.call_count == 1 - assert api_search.calls[0].response.status_code == 200 - assert json_loads(api_search.calls[0].request.body) == { - "q": "alpha", - "visited": [], - "services": ["docs"], - "nb_results": 50, - "order_by": "updated_at", - "order_direction": "desc", - } - - -@responses.activate -@pytest.mark.parametrize( - "pagination, status, expected", - ( - ( - {"page": 1, "page_size": 10}, - 200, - {"count": 10, "previous": None, "next": None, "range": (0, None)}, - ), - ( - {}, - 200, - {"count": 10, "previous": None, "next": None, "range": (0, None)}, - ), - ( - {"page": 2, "page_size": 10}, - 404, - {}, - ), - ( - {"page": 1, "page_size": 5}, - 200, - { - "count": 10, - "previous": None, - "next": {"page": 2, "page_size": 5}, - "range": (0, 5), - }, - ), - ( - {"page": 2, "page_size": 5}, - 200, - { - "count": 10, - "previous": {"page_size": 5}, - "next": None, - "range": (5, None), - }, - ), - ({"page": 3, "page_size": 5}, 404, {}), - ), -) -def test_api_documents_search_pagination_endpoint_is_none( - indexer_settings, pagination, status, expected -): - """Documents should be ordered by descending "-updated_at" by default""" - indexer_settings.SEARCH_INDEXER_QUERY_URL = None - - assert get_document_indexer() is None - - user = factories.UserFactory() - - client = APIClient() - client.force_login(user) - - factories.DocumentFactory.create_batch(10, title="alpha", users=[user]) - - response = client.get( - "/api/v1.0/documents/search/", - data={ - "q": "alpha", - **pagination, - }, - ) - - assert response.status_code == status - - if response.status_code < 300: - previous_url = ( - build_search_url(q="alpha", **expected["previous"]) - if expected["previous"] - else None - ) - next_url = ( - build_search_url(q="alpha", **expected["next"]) - if expected["next"] - else None - ) - queryset = models.Document.objects.order_by("-updated_at") - start, end = expected["range"] - expected_results = [str(d.pk) for d in queryset[start:end]] - - content = response.json() - - assert content["count"] == expected["count"] - assert content["previous"] == previous_url - assert content["next"] == next_url - - results = content.pop("results") - - assert [r["id"] for r in results] == expected_results + assert results == [ + {"id": document["id"], "title": document["title"], "path": document["path"]} + ] diff --git a/src/backend/core/tests/documents/test_api_documents_descendants.py b/src/backend/core/tests/documents/test_api_documents_search_descendants.py similarity index 78% rename from src/backend/core/tests/documents/test_api_documents_descendants.py rename to src/backend/core/tests/documents/test_api_documents_search_descendants.py index f320b0707a..0da3635a0e 100644 --- a/src/backend/core/tests/documents/test_api_documents_descendants.py +++ b/src/backend/core/tests/documents/test_api_documents_search_descendants.py @@ -1,5 +1,6 @@ """ -Tests for Documents API endpoint in impress's core app: descendants +Tests for search API endpoint in impress's core app when indexer is not +available and a path param is given. """ import random @@ -10,26 +11,61 @@ from rest_framework.test import APIClient from core import factories +from core.api.filters import remove_accents pytestmark = pytest.mark.django_db +@pytest.fixture(autouse=True) +def disable_indexer(indexer_settings): + """Disable search indexer for all tests in this file.""" + indexer_settings.SEARCH_INDEXER_CLASS = None + + def test_api_documents_descendants_list_anonymous_public_standalone(): """Anonymous users should be allowed to retrieve the descendants of a public document.""" - document = factories.DocumentFactory(link_reach="public") - child1, child2 = factories.DocumentFactory.create_batch(2, parent=document) - grand_child = factories.DocumentFactory(parent=child1) + document = factories.DocumentFactory(link_reach="public", title="doc parent") + child1, child2 = factories.DocumentFactory.create_batch( + 2, parent=document, title="doc child" + ) + grand_child = factories.DocumentFactory(parent=child1, title="doc grand child") factories.UserDocumentAccessFactory(document=child1) - response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/descendants/") + response = APIClient().get( + "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + ) assert response.status_code == 200 assert response.json() == { - "count": 3, + "count": 4, "next": None, "previous": None, "results": [ + { + # the search should include the parent document itself + "abilities": document.get_abilities(AnonymousUser()), + "ancestors_link_reach": None, + "ancestors_link_role": None, + "computed_link_reach": "public", + "computed_link_role": document.computed_link_role, + "created_at": document.created_at.isoformat().replace("+00:00", "Z"), + "creator": str(document.creator.id), + "deleted_at": None, + "depth": 1, + "excerpt": document.excerpt, + "id": str(document.id), + "is_favorite": False, + "link_reach": document.link_reach, + "link_role": document.link_role, + "numchild": 2, + "nb_accesses_ancestors": 0, + "nb_accesses_direct": 0, + "path": document.path, + "title": document.title, + "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_role": None, + }, { "abilities": child1.get_abilities(AnonymousUser()), "ancestors_link_reach": "public", @@ -110,26 +146,60 @@ def test_api_documents_descendants_list_anonymous_public_parent(): Anonymous users should be allowed to retrieve the descendants of a document who has a public ancestor. """ - grand_parent = factories.DocumentFactory(link_reach="public") + grand_parent = factories.DocumentFactory( + link_reach="public", title="grand parent doc" + ) parent = factories.DocumentFactory( - parent=grand_parent, link_reach=random.choice(["authenticated", "restricted"]) + parent=grand_parent, + link_reach=random.choice(["authenticated", "restricted"]), + title="parent doc", ) document = factories.DocumentFactory( - link_reach=random.choice(["authenticated", "restricted"]), parent=parent + link_reach=random.choice(["authenticated", "restricted"]), + parent=parent, + title="document", + ) + child1, child2 = factories.DocumentFactory.create_batch( + 2, parent=document, title="child doc" ) - child1, child2 = factories.DocumentFactory.create_batch(2, parent=document) - grand_child = factories.DocumentFactory(parent=child1) + grand_child = factories.DocumentFactory(parent=child1, title="grand child doc") factories.UserDocumentAccessFactory(document=child1) - response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/descendants/") + response = APIClient().get( + "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + ) assert response.status_code == 200 assert response.json() == { - "count": 3, + "count": 4, "next": None, "previous": None, "results": [ + { + # the search should include the parent document itself + "abilities": document.get_abilities(AnonymousUser()), + "ancestors_link_reach": "public", + "ancestors_link_role": grand_parent.link_role, + "computed_link_reach": document.computed_link_reach, + "computed_link_role": document.computed_link_role, + "created_at": document.created_at.isoformat().replace("+00:00", "Z"), + "creator": str(document.creator.id), + "deleted_at": None, + "depth": 3, + "excerpt": document.excerpt, + "id": str(document.id), + "is_favorite": False, + "link_reach": document.link_reach, + "link_role": document.link_role, + "numchild": 2, + "nb_accesses_ancestors": 0, + "nb_accesses_direct": 0, + "path": document.path, + "title": document.title, + "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_role": None, + }, { "abilities": child1.get_abilities(AnonymousUser()), "ancestors_link_reach": "public", @@ -208,11 +278,13 @@ def test_api_documents_descendants_list_anonymous_restricted_or_authenticated(re """ Anonymous users should not be able to retrieve descendants of a document that is not public. """ - document = factories.DocumentFactory(link_reach=reach) - child = factories.DocumentFactory(parent=document) - _grand_child = factories.DocumentFactory(parent=child) + document = factories.DocumentFactory(title="parent", link_reach=reach) + child = factories.DocumentFactory(title="child", parent=document) + _grand_child = factories.DocumentFactory(title="grand child", parent=child) - response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/descendants/") + response = APIClient().get( + "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + ) assert response.status_code == 401 assert response.json() == { @@ -232,17 +304,18 @@ def test_api_documents_descendants_list_authenticated_unrelated_public_or_authen client = APIClient() client.force_login(user) - document = factories.DocumentFactory(link_reach=reach) + document = factories.DocumentFactory(link_reach=reach, title="parent") child1, child2 = factories.DocumentFactory.create_batch( - 2, parent=document, link_reach="restricted" + 2, parent=document, link_reach="restricted", title="child" ) - grand_child = factories.DocumentFactory(parent=child1) + grand_child = factories.DocumentFactory(parent=child1, title="grand child") factories.UserDocumentAccessFactory(document=child1) response = client.get( - f"/api/v1.0/documents/{document.id!s}/descendants/", + "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} ) + assert response.status_code == 200 assert response.json() == { "count": 3, @@ -335,17 +408,23 @@ def test_api_documents_descendants_list_authenticated_public_or_authenticated_pa client = APIClient() client.force_login(user) - grand_parent = factories.DocumentFactory(link_reach=reach) - parent = factories.DocumentFactory(parent=grand_parent, link_reach="restricted") - document = factories.DocumentFactory(link_reach="restricted", parent=parent) + grand_parent = factories.DocumentFactory(link_reach=reach, title="grand parent") + parent = factories.DocumentFactory( + parent=grand_parent, link_reach="restricted", title="parent" + ) + document = factories.DocumentFactory( + link_reach="restricted", parent=parent, title="document" + ) child1, child2 = factories.DocumentFactory.create_batch( - 2, parent=document, link_reach="restricted" + 2, parent=document, link_reach="restricted", title="child" ) - grand_child = factories.DocumentFactory(parent=child1) + grand_child = factories.DocumentFactory(parent=child1, title="grand child") factories.UserDocumentAccessFactory(document=child1) - response = client.get(f"/api/v1.0/documents/{document.id!s}/descendants/") + response = client.get( + "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + ) assert response.status_code == 200 assert response.json() == { @@ -435,15 +514,18 @@ def test_api_documents_descendants_list_authenticated_unrelated_restricted(): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(link_reach="restricted") - child1, _child2 = factories.DocumentFactory.create_batch(2, parent=document) - _grand_child = factories.DocumentFactory(parent=child1) + document = factories.DocumentFactory(link_reach="restricted", title="parent") + child1, _child2 = factories.DocumentFactory.create_batch( + 2, parent=document, title="child" + ) + _grand_child = factories.DocumentFactory(parent=child1, title="grand child") factories.UserDocumentAccessFactory(document=child1) response = client.get( - f"/api/v1.0/documents/{document.id!s}/descendants/", + "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} ) + assert response.status_code == 403 assert response.json() == { "detail": "You do not have permission to perform this action." @@ -460,17 +542,19 @@ def test_api_documents_descendants_list_authenticated_related_direct(): client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = factories.DocumentFactory(title="parent") access = factories.UserDocumentAccessFactory(document=document, user=user) factories.UserDocumentAccessFactory(document=document) - child1, child2 = factories.DocumentFactory.create_batch(2, parent=document) + child1, child2 = factories.DocumentFactory.create_batch( + 2, parent=document, title="child" + ) factories.UserDocumentAccessFactory(document=child1) - grand_child = factories.DocumentFactory(parent=child1) + grand_child = factories.DocumentFactory(parent=child1, title="grand child") response = client.get( - f"/api/v1.0/documents/{document.id!s}/descendants/", + "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} ) assert response.status_code == 200 assert response.json() == { @@ -561,21 +645,27 @@ def test_api_documents_descendants_list_authenticated_related_parent(): client = APIClient() client.force_login(user) - grand_parent = factories.DocumentFactory(link_reach="restricted") + grand_parent = factories.DocumentFactory(link_reach="restricted", title="parent") grand_parent_access = factories.UserDocumentAccessFactory( document=grand_parent, user=user ) - parent = factories.DocumentFactory(parent=grand_parent, link_reach="restricted") - document = factories.DocumentFactory(parent=parent, link_reach="restricted") + parent = factories.DocumentFactory( + parent=grand_parent, link_reach="restricted", title="parent" + ) + document = factories.DocumentFactory( + parent=parent, link_reach="restricted", title="document" + ) - child1, child2 = factories.DocumentFactory.create_batch(2, parent=document) + child1, child2 = factories.DocumentFactory.create_batch( + 2, parent=document, title="child" + ) factories.UserDocumentAccessFactory(document=child1) - grand_child = factories.DocumentFactory(parent=child1) + grand_child = factories.DocumentFactory(parent=child1, title="grand child") response = client.get( - f"/api/v1.0/documents/{document.id!s}/descendants/", + "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} ) assert response.status_code == 200 assert response.json() == { @@ -673,7 +763,7 @@ def test_api_documents_descendants_list_authenticated_related_child(): factories.UserDocumentAccessFactory(document=document) response = client.get( - f"/api/v1.0/documents/{document.id!s}/descendants/", + "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} ) assert response.status_code == 403 assert response.json() == { @@ -694,12 +784,15 @@ def test_api_documents_descendants_list_authenticated_related_team_none( client = APIClient() client.force_login(user) - document = factories.DocumentFactory(link_reach="restricted") - factories.DocumentFactory.create_batch(2, parent=document) + document = factories.DocumentFactory(link_reach="restricted", title="document") + factories.DocumentFactory.create_batch(2, parent=document, title="child") factories.TeamDocumentAccessFactory(document=document, team="myteam") - response = client.get(f"/api/v1.0/documents/{document.id!s}/descendants/") + response = client.get( + "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + ) + assert response.status_code == 403 assert response.json() == { "detail": "You do not have permission to perform this action." @@ -719,13 +812,17 @@ def test_api_documents_descendants_list_authenticated_related_team_members( client = APIClient() client.force_login(user) - document = factories.DocumentFactory(link_reach="restricted") - child1, child2 = factories.DocumentFactory.create_batch(2, parent=document) - grand_child = factories.DocumentFactory(parent=child1) + document = factories.DocumentFactory(link_reach="restricted", title="parent") + child1, child2 = factories.DocumentFactory.create_batch( + 2, parent=document, title="child" + ) + grand_child = factories.DocumentFactory(parent=child1, title="grand child") access = factories.TeamDocumentAccessFactory(document=document, team="myteam") - response = client.get(f"/api/v1.0/documents/{document.id!s}/descendants/") + response = client.get( + "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + ) # pylint: disable=R0801 assert response.status_code == 200 @@ -805,3 +902,53 @@ def test_api_documents_descendants_list_authenticated_related_team_members( }, ], } + + +@pytest.mark.parametrize( + "query,nb_results", + [ + ("", 7), # Empty string + ("Project Alpha", 1), # Exact match + ("project", 2), # Partial match (case-insensitive) + ("Guide", 2), # Word match within a title + ("Special", 0), # No match (nonexistent keyword) + ("2024", 2), # Match by numeric keyword + ("velo", 1), # Accent-insensitive match (velo vs vélo) + ("bêta", 1), # Accent-insensitive match (bêta vs beta) + ], +) +def test_api_documents_descendants_search_on_title(query, nb_results): + """Authenticated users should be able to search documents by their unaccented title.""" + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + parent = factories.DocumentFactory(users=[user]) + + # Create documents with predefined titles + titles = [ + "Project Alpha Documentation", + "Project Beta Overview", + "User Guide", + "Financial Report 2024", + "Annual Review 2024", + "Guide du vélo urbain", # <-- Title with accent for accent-insensitive test + ] + for title in titles: + factories.DocumentFactory(title=title, parent=parent) + + # Perform the search query + response = client.get( + "/api/v1.0/documents/search/", data={"q": query, "path": parent.path} + ) + + assert response.status_code == 200 + results = response.json()["results"] + assert len(results) == nb_results + + # Ensure all results contain the query in their title + for result in results: + assert ( + remove_accents(query).lower().strip() + in remove_accents(result["title"]).lower() + ) diff --git a/src/backend/core/tests/test_models_documents_indexer.py b/src/backend/core/tests/test_services_find_document_indexer.py similarity index 80% rename from src/backend/core/tests/test_models_documents_indexer.py rename to src/backend/core/tests/test_services_find_document_indexer.py index 9e171f724d..711d73f372 100644 --- a/src/backend/core/tests/test_models_documents_indexer.py +++ b/src/backend/core/tests/test_services_find_document_indexer.py @@ -1,5 +1,5 @@ """ -Unit tests for the Document model +Unit tests for FindDocumentIndexer """ # pylint: disable=too-many-lines @@ -12,7 +12,7 @@ import pytest from core import factories, models -from core.services.search_indexers import SearchIndexer +from core.services.search_indexers import FindDocumentIndexer pytestmark = pytest.mark.django_db @@ -30,7 +30,7 @@ def reset_throttle(): reset_batch_indexer_throttle() -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer(mock_push): @@ -41,7 +41,7 @@ def test_models_documents_post_save_indexer(mock_push): accesses = {} data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() assert len(data) == 1 @@ -64,14 +64,14 @@ def test_models_documents_post_save_indexer_no_batches(indexer_settings): """Test indexation task on doculment creation, no throttle""" indexer_settings.SEARCH_INDEXER_COUNTDOWN = 0 - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: with transaction.atomic(): doc1, doc2, doc3 = factories.DocumentFactory.create_batch(3) accesses = {} data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() # 3 calls assert len(data) == 3 @@ -91,7 +91,7 @@ def test_models_documents_post_save_indexer_no_batches(indexer_settings): assert cache.get("file-batch-indexer-throttle") is None -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_not_configured(mock_push, indexer_settings): """Task should not start an indexation when disabled""" @@ -106,7 +106,7 @@ def test_models_documents_post_save_indexer_not_configured(mock_push, indexer_se assert mock_push.assert_not_called -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_wrongly_configured( mock_push, indexer_settings @@ -123,7 +123,7 @@ def test_models_documents_post_save_indexer_wrongly_configured( assert mock_push.assert_not_called -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_with_accesses(mock_push): @@ -145,7 +145,7 @@ def test_models_documents_post_save_indexer_with_accesses(mock_push): data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() assert len(data) == 1 assert sorted(data[0], key=itemgetter("id")) == sorted( @@ -158,7 +158,7 @@ def test_models_documents_post_save_indexer_with_accesses(mock_push): ) -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_deleted(mock_push): @@ -207,7 +207,7 @@ def test_models_documents_post_save_indexer_deleted(mock_push): data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() assert len(data) == 2 @@ -244,14 +244,14 @@ def test_models_documents_indexer_hard_deleted(): factories.UserDocumentAccessFactory(document=doc, user=user) # Call task on deleted document. - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: doc.delete() # Hard delete document are not re-indexed. assert mock_push.assert_not_called -@mock.patch.object(SearchIndexer, "push") +@mock.patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") @pytest.mark.django_db(transaction=True) def test_models_documents_post_save_indexer_restored(mock_push): @@ -308,7 +308,7 @@ def test_models_documents_post_save_indexer_restored(mock_push): data = [call.args[0] for call in mock_push.call_args_list] - indexer = SearchIndexer() + indexer = FindDocumentIndexer() # All docs are re-indexed assert len(data) == 2 @@ -337,16 +337,16 @@ def test_models_documents_post_save_indexer_restored(mock_push): @pytest.mark.usefixtures("indexer_settings") def test_models_documents_post_save_indexer_throttle(): """Test indexation task skipping on document update""" - indexer = SearchIndexer() + indexer = FindDocumentIndexer() user = factories.UserFactory() - with mock.patch.object(SearchIndexer, "push"): + with mock.patch.object(FindDocumentIndexer, "push"): with transaction.atomic(): docs = factories.DocumentFactory.create_batch(5, users=(user,)) accesses = {str(item.path): {"users": [user.sub]} for item in docs} - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: # Simulate 1 running task cache.set("document-batch-indexer-throttle", 1) @@ -359,7 +359,7 @@ def test_models_documents_post_save_indexer_throttle(): assert [call.args[0] for call in mock_push.call_args_list] == [] - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: # No waiting task cache.delete("document-batch-indexer-throttle") @@ -389,7 +389,7 @@ def test_models_documents_access_post_save_indexer(): """Test indexation task on DocumentAccess update""" users = factories.UserFactory.create_batch(3) - with mock.patch.object(SearchIndexer, "push"): + with mock.patch.object(FindDocumentIndexer, "push"): with transaction.atomic(): doc = factories.DocumentFactory(users=users) doc_accesses = models.DocumentAccess.objects.filter(document=doc).order_by( @@ -398,7 +398,7 @@ def test_models_documents_access_post_save_indexer(): reset_batch_indexer_throttle() - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: with transaction.atomic(): for doc_access in doc_accesses: doc_access.save() @@ -426,7 +426,7 @@ def test_models_items_access_post_save_indexer_no_throttle(indexer_settings): reset_batch_indexer_throttle() - with mock.patch.object(SearchIndexer, "push") as mock_push: + with mock.patch.object(FindDocumentIndexer, "push") as mock_push: with transaction.atomic(): for doc_access in doc_accesses: doc_access.save() @@ -439,3 +439,70 @@ def test_models_items_access_post_save_indexer_no_throttle(indexer_settings): assert [len(d) for d in data] == [1] * 3 # the same document is indexed 3 times assert [d[0]["id"] for d in data] == [str(doc.pk)] * 3 + + +@mock.patch.object(FindDocumentIndexer, "search_query") +@pytest.mark.usefixtures("indexer_settings") +def test_find_document_indexer_search(mock_search_query): + """Test search function of FindDocumentIndexer returns formatted results""" + + # Mock API response from Find + hits = [ + { + "_id": "doc-123", + "_source": { + "title": "Test Document", + "content": "This is test content", + "updated_at": "2024-01-01T00:00:00Z", + "path": "/some/path/doc-123", + }, + }, + { + "_id": "doc-456", + "_source": { + "title.fr": "Document de test", + "content": "Contenu de test", + "updated_at": "2024-01-02T00:00:00Z", + }, + }, + ] + mock_search_query.return_value = hits + + q = "test" + token = "fake-token" + nb_results = 10 + path = "/some/path/" + visited = ["doc-123"] + results = FindDocumentIndexer().search( + q=q, token=token, nb_results=nb_results, path=path, visited=visited + ) + + mock_search_query.assert_called_once() + call_args = mock_search_query.call_args + assert call_args[1]["data"] == { + "q": q, + "visited": visited, + "services": ["docs"], + "nb_results": nb_results, + "order_by": "updated_at", + "order_direction": "desc", + "path": path, + } + + assert len(results) == 2 + assert results == [ + { + "id": hits[0]["_id"], + "title": hits[0]["_source"]["title"], + "content": hits[0]["_source"]["content"], + "updated_at": hits[0]["_source"]["updated_at"], + "path": hits[0]["_source"]["path"], + }, + { + "id": hits[1]["_id"], + "title": hits[1]["_source"]["title.fr"], + "title.fr": hits[1]["_source"]["title.fr"], # <- Find response artefact + "content": hits[1]["_source"]["content"], + "updated_at": hits[1]["_source"]["updated_at"], + }, + ] diff --git a/src/backend/core/tests/test_services_search_indexers.py b/src/backend/core/tests/test_services_search_indexers.py index 61488a921b..94406e8672 100644 --- a/src/backend/core/tests/test_services_search_indexers.py +++ b/src/backend/core/tests/test_services_search_indexers.py @@ -15,7 +15,7 @@ from core import factories, models, utils from core.services.search_indexers import ( BaseDocumentIndexer, - SearchIndexer, + FindDocumentIndexer, get_document_indexer, get_visited_document_ids_of, ) @@ -78,7 +78,7 @@ def test_services_search_indexer_is_configured(indexer_settings): # Valid class indexer_settings.SEARCH_INDEXER_CLASS = ( - "core.services.search_indexers.SearchIndexer" + "core.services.search_indexers.FindDocumentIndexer" ) get_document_indexer.cache_clear() @@ -98,7 +98,7 @@ def test_services_search_indexer_url_is_none(indexer_settings): indexer_settings.SEARCH_INDEXER_URL = None with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() assert "SEARCH_INDEXER_URL must be set in Django settings." in str(exc_info.value) @@ -110,7 +110,7 @@ def test_services_search_indexer_url_is_empty(indexer_settings): indexer_settings.SEARCH_INDEXER_URL = "" with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() assert "SEARCH_INDEXER_URL must be set in Django settings." in str(exc_info.value) @@ -122,7 +122,7 @@ def test_services_search_indexer_secret_is_none(indexer_settings): indexer_settings.SEARCH_INDEXER_SECRET = None with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() assert "SEARCH_INDEXER_SECRET must be set in Django settings." in str( exc_info.value @@ -136,7 +136,7 @@ def test_services_search_indexer_secret_is_empty(indexer_settings): indexer_settings.SEARCH_INDEXER_SECRET = "" with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() assert "SEARCH_INDEXER_SECRET must be set in Django settings." in str( exc_info.value @@ -150,7 +150,7 @@ def test_services_search_endpoint_is_none(indexer_settings): indexer_settings.SEARCH_INDEXER_QUERY_URL = None with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() assert "SEARCH_INDEXER_QUERY_URL must be set in Django settings." in str( exc_info.value @@ -164,7 +164,7 @@ def test_services_search_endpoint_is_empty(indexer_settings): indexer_settings.SEARCH_INDEXER_QUERY_URL = "" with pytest.raises(ImproperlyConfigured) as exc_info: - SearchIndexer() + FindDocumentIndexer() assert "SEARCH_INDEXER_QUERY_URL must be set in Django settings." in str( exc_info.value @@ -192,7 +192,7 @@ def test_services_search_indexers_serialize_document_returns_expected_json(): } } - indexer = SearchIndexer() + indexer = FindDocumentIndexer() result = indexer.serialize_document(document, accesses) assert set(result.pop("users")) == {str(user_a.sub), str(user_b.sub)} @@ -221,7 +221,7 @@ def test_services_search_indexers_serialize_document_deleted(): parent.soft_delete() document.refresh_from_db() - indexer = SearchIndexer() + indexer = FindDocumentIndexer() result = indexer.serialize_document(document, {}) assert result["is_active"] is False @@ -232,7 +232,7 @@ def test_services_search_indexers_serialize_document_empty(): """Empty documents returns empty content in the serialized json.""" document = factories.DocumentFactory(content="", title=None) - indexer = SearchIndexer() + indexer = FindDocumentIndexer() result = indexer.serialize_document(document, {}) assert result["content"] == "" @@ -256,10 +256,10 @@ def test_services_search_indexers_index_errors(indexer_settings): ) with pytest.raises(HTTPError): - SearchIndexer().index() + FindDocumentIndexer().index() -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") def test_services_search_indexers_batches_pass_only_batch_accesses( mock_push, indexer_settings ): @@ -276,7 +276,7 @@ def test_services_search_indexers_batches_pass_only_batch_accesses( access = factories.UserDocumentAccessFactory(document=document) expected_user_subs[str(document.id)] = str(access.user.sub) - assert SearchIndexer().index() == 5 + assert FindDocumentIndexer().index() == 5 # Should be 3 batches: 2 + 2 + 1 assert mock_push.call_count == 3 @@ -299,7 +299,7 @@ def test_services_search_indexers_batches_pass_only_batch_accesses( assert seen_doc_ids == {str(d.id) for d in documents} -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_batch_size_argument(mock_push): """ @@ -314,7 +314,7 @@ def test_services_search_indexers_batch_size_argument(mock_push): access = factories.UserDocumentAccessFactory(document=document) expected_user_subs[str(document.id)] = str(access.user.sub) - assert SearchIndexer().index(batch_size=2) == 5 + assert FindDocumentIndexer().index(batch_size=2) == 5 # Should be 3 batches: 2 + 2 + 1 assert mock_push.call_count == 3 @@ -337,7 +337,7 @@ def test_services_search_indexers_batch_size_argument(mock_push): assert seen_doc_ids == {str(d.id) for d in documents} -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_ignore_empty_documents(mock_push): """ @@ -349,7 +349,7 @@ def test_services_search_indexers_ignore_empty_documents(mock_push): empty_title = factories.DocumentFactory(title="") empty_content = factories.DocumentFactory(content="") - assert SearchIndexer().index() == 3 + assert FindDocumentIndexer().index() == 3 assert mock_push.call_count == 1 @@ -365,7 +365,7 @@ def test_services_search_indexers_ignore_empty_documents(mock_push): } -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") def test_services_search_indexers_skip_empty_batches(mock_push, indexer_settings): """ Documents indexing batch can be empty if all the docs are empty. @@ -377,14 +377,14 @@ def test_services_search_indexers_skip_empty_batches(mock_push, indexer_settings # Only empty docs factories.DocumentFactory.create_batch(5, content="", title="") - assert SearchIndexer().index() == 1 + assert FindDocumentIndexer().index() == 1 assert mock_push.call_count == 1 results = [doc["id"] for doc in mock_push.call_args[0][0]] assert results == [str(document.id)] -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_ancestors_link_reach(mock_push): """Document accesses and reach should take into account ancestors link reaches.""" @@ -395,7 +395,7 @@ def test_services_search_indexers_ancestors_link_reach(mock_push): parent = factories.DocumentFactory(parent=grand_parent, link_reach="public") document = factories.DocumentFactory(parent=parent, link_reach="restricted") - assert SearchIndexer().index() == 4 + assert FindDocumentIndexer().index() == 4 results = {doc["id"]: doc for doc in mock_push.call_args[0][0]} assert len(results) == 4 @@ -405,7 +405,7 @@ def test_services_search_indexers_ancestors_link_reach(mock_push): assert results[str(document.id)]["reach"] == "public" -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_ancestors_users(mock_push): """Document accesses and reach should include users from ancestors.""" @@ -415,7 +415,7 @@ def test_services_search_indexers_ancestors_users(mock_push): parent = factories.DocumentFactory(parent=grand_parent, users=[user_p]) document = factories.DocumentFactory(parent=parent, users=[user_d]) - assert SearchIndexer().index() == 3 + assert FindDocumentIndexer().index() == 3 results = {doc["id"]: doc for doc in mock_push.call_args[0][0]} assert len(results) == 3 @@ -428,7 +428,7 @@ def test_services_search_indexers_ancestors_users(mock_push): } -@patch.object(SearchIndexer, "push") +@patch.object(FindDocumentIndexer, "push") @pytest.mark.usefixtures("indexer_settings") def test_services_search_indexers_ancestors_teams(mock_push): """Document accesses and reach should include teams from ancestors.""" @@ -436,7 +436,7 @@ def test_services_search_indexers_ancestors_teams(mock_push): parent = factories.DocumentFactory(parent=grand_parent, teams=["team_p"]) document = factories.DocumentFactory(parent=parent, teams=["team_d"]) - assert SearchIndexer().index() == 3 + assert FindDocumentIndexer().index() == 3 results = {doc["id"]: doc for doc in mock_push.call_args[0][0]} assert len(results) == 3 @@ -453,7 +453,7 @@ def test_push_uses_correct_url_and_data(mock_post, indexer_settings): """ indexer_settings.SEARCH_INDEXER_URL = "http://example.com/index" - indexer = SearchIndexer() + indexer = FindDocumentIndexer() sample_data = [{"id": "123", "title": "Test"}] mock_response = mock_post.return_value @@ -554,7 +554,7 @@ def test_services_search_indexers_search_errors(indexer_settings): ) with pytest.raises(HTTPError): - SearchIndexer().search("alpha", token="mytoken") + FindDocumentIndexer().search("alpha", token="mytoken") @patch("requests.post") @@ -564,7 +564,7 @@ def test_services_search_indexers_search(mock_post, indexer_settings): document ids from linktraces. """ user = factories.UserFactory() - indexer = SearchIndexer() + indexer = FindDocumentIndexer() mock_response = mock_post.return_value mock_response.raise_for_status.return_value = None # No error @@ -605,7 +605,7 @@ def test_services_search_indexers_search_nb_results(mock_post, indexer_settings) indexer_settings.SEARCH_INDEXER_QUERY_LIMIT = 25 user = factories.UserFactory() - indexer = SearchIndexer() + indexer = FindDocumentIndexer() mock_response = mock_post.return_value mock_response.raise_for_status.return_value = None # No error @@ -633,3 +633,51 @@ def test_services_search_indexers_search_nb_results(mock_post, indexer_settings) assert args[0] == indexer_settings.SEARCH_INDEXER_QUERY_URL assert kwargs.get("json")["nb_results"] == 109 + + +def test_search_indexer_get_title_with_localized_field(): + """Test extracting title from localized title field.""" + source = {"title.extension": "Bonjour", "id": 1, "content": "test"} + result = FindDocumentIndexer.get_title(source) + + assert result == "Bonjour" + + +def test_search_indexer_get_title_with_multiple_localized_fields(): + """Test that first matching localized title is returned.""" + source = {"title.extension": "Bonjour", "title.en": "Hello", "id": 1} + result = FindDocumentIndexer.get_title(source) + + assert result in ["Bonjour", "Hello"] + + +def test_search_indexer_get_title_fallback_to_plain_title(): + """Test fallback to plain 'title' field when no localized field exists.""" + source = {"title": "Hello World", "id": 1} + result = FindDocumentIndexer.get_title(source) + + assert result == "Hello World" + + +def test_search_indexer_get_title_no_title_field(): + """Test that empty string is returned when no title field exists.""" + source = {"id": 1, "content": "test"} + result = FindDocumentIndexer.get_title(source) + + assert result == "" + + +def test_search_indexer_get_title_with_empty_localized_title(): + """Test that fallback works when localized title is empty.""" + source = {"title.extension": "", "title": "Fallback Title", "id": 1} + result = FindDocumentIndexer.get_title(source) + + assert result == "Fallback Title" + + +def test_search_indexer_get_title_with_multiple_extension(): + """Test extracting title from title field with multiple extensions.""" + source = {"title.extension_1.extension_2": "Bonjour", "id": 1, "content": "test"} + result = FindDocumentIndexer.get_title(source) + + assert result == "Bonjour" diff --git a/src/backend/core/tests/test_utils.py b/src/backend/core/tests/test_utils.py index 42d588c536..b7fa73a003 100644 --- a/src/backend/core/tests/test_utils.py +++ b/src/backend/core/tests/test_utils.py @@ -100,3 +100,38 @@ def test_utils_get_ancestor_to_descendants_map_multiple_paths(): "000100020005": {"000100020005"}, "00010003": {"00010003"}, } + + +def test_utils_get_value_by_pattern_matching_key(): + """Test extracting value from a dictionary with a matching key pattern.""" + data = {"title.extension": "Bonjour", "id": 1, "content": "test"} + result = utils.get_value_by_pattern(data, r"^title\.") + + assert set(result) == {"Bonjour"} + + +def test_utils_get_value_by_pattern_multiple_matches(): + """Test that all matching keys are returned.""" + data = {"title.extension_1": "Bonjour", "title.extension_2": "Hello", "id": 1} + result = utils.get_value_by_pattern(data, r"^title\.") + + assert set(result) == { + "Bonjour", + "Hello", + } + + +def test_utils_get_value_by_pattern_multiple_extensions(): + """Test that all matching keys are returned.""" + data = {"title.extension_1.extension_2": "Bonjour", "id": 1} + result = utils.get_value_by_pattern(data, r"^title\.") + + assert set(result) == {"Bonjour"} + + +def test_utils_get_value_by_pattern_no_match(): + """Test that empty list is returned when no key matches the pattern.""" + data = {"name": "Test", "id": 1} + result = utils.get_value_by_pattern(data, r"^title\.") + + assert result == [] diff --git a/src/backend/core/utils.py b/src/backend/core/utils.py index 357ede03c3..162ba58ba6 100644 --- a/src/backend/core/utils.py +++ b/src/backend/core/utils.py @@ -10,6 +10,27 @@ from core import enums +def get_value_by_pattern(data, pattern): + """ + Get all values from keys matching a regex pattern in a dictionary. + + Args: + data (dict): Source dictionary to search + pattern (str): Regex pattern to match against keys + + Returns: + list: List of values for all matching keys, empty list if no matches + + Example: + >>> get_value_by_pattern({"title.fr": "Bonjour", "id": 1}, r"^title\\.") + ["Bonjour"] + >>> get_value_by_pattern({"title.fr": "Bonjour", "title.en": "Hello"}, r"^title\\.") + ["Bonjour", "Hello"] + """ + regex = re.compile(pattern) + return [value for key, value in data.items() if regex.match(key)] + + def get_ancestor_to_descendants_map(paths, steplen): """ Given a list of document paths, return a mapping of ancestor_path -> set of descendant_paths. diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/SearchPage.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/SearchPage.tsx index 8027b2e45d..06eddbf434 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/SearchPage.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/SearchPage.tsx @@ -3,6 +3,7 @@ import { StyleSchema, } from '@blocknote/core'; import { useBlockNoteEditor } from '@blocknote/react'; +import { useTreeContext } from '@gouvfr-lasuite/ui-kit'; import type { KeyboardEvent } from 'react'; import { useEffect, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; @@ -26,12 +27,14 @@ import { import FoundPageIcon from '@/docs/doc-editor/assets/doc-found.svg'; import AddPageIcon from '@/docs/doc-editor/assets/doc-plus.svg'; import { + Doc, getEmojiAndTitle, useCreateChildDocTree, useDocStore, useTrans, } from '@/docs/doc-management'; -import { DocSearchSubPageContent, DocSearchTarget } from '@/docs/doc-search'; +import { DocSearchTarget } from '@/docs/doc-search'; +import { DocSearchContent } from '@/docs/doc-search/components/DocSearchContent'; import { useResponsiveStore } from '@/stores'; const inputStyle = css` @@ -87,7 +90,7 @@ export const SearchPage = ({ const { isDesktop } = useResponsiveStore(); const { untitledDocument } = useTrans(); const isEditable = editor.isEditable; - + const treeContext = useTreeContext(); /** * createReactInlineContentSpec add automatically the focus after * the inline content, so we need to set the focus on the input @@ -225,14 +228,14 @@ export const SearchPage = ({ `} $margin={{ top: '0.5rem' }} > - { if (!isEditable) { return; } - updateInlineContent({ type: 'interlinkingSearchInline', props: { diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/api/index.ts b/src/frontend/apps/impress/src/features/docs/doc-management/api/index.ts index 88c4b028bb..45a97857de 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/api/index.ts +++ b/src/frontend/apps/impress/src/features/docs/doc-management/api/index.ts @@ -8,5 +8,5 @@ export * from './useDocs'; export * from './useDocsFavorite'; export * from './useDuplicateDoc'; export * from './useRestoreDoc'; -export * from './useSubDocs'; export * from './useUpdateDoc'; +export * from './useSearchDocs'; diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/api/useDocs.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/api/useDocs.tsx index 90e1461e1e..911e52ba02 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/api/useDocs.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/api/useDocs.tsx @@ -15,7 +15,6 @@ export type DocsParams = { page: number; ordering?: DocsOrdering; is_creator_me?: boolean; - title?: string; is_favorite?: boolean; }; @@ -31,9 +30,6 @@ export const constructParams = (params: DocsParams): URLSearchParams => { if (params.is_creator_me !== undefined) { searchParams.set('is_creator_me', params.is_creator_me.toString()); } - if (params.title && params.title.length > 0) { - searchParams.set('title', params.title); - } if (params.is_favorite !== undefined) { searchParams.set('is_favorite', params.is_favorite.toString()); } diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/api/useSearchDocs.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/api/useSearchDocs.tsx new file mode 100644 index 0000000000..cd1cfc4a85 --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-management/api/useSearchDocs.tsx @@ -0,0 +1,67 @@ +import { useQuery } from '@tanstack/react-query'; + +import { APIError, errorCauses, fetchAPI, useAPIInfiniteQuery } from '@/api'; +import { DocSearchTarget } from '@/docs/doc-search'; + +import { DocsResponse, KEY_LIST_DOC } from './useDocs'; + +export type SearchDocsParams = { + page: number; + q: string; + target?: DocSearchTarget; + parentPath?: string; +}; + +const constructParams = ({ + q, + page, + target, + parentPath, +}: SearchDocsParams): URLSearchParams => { + const searchParams = new URLSearchParams(); + + searchParams.set('q', q); + if (target === DocSearchTarget.CURRENT && parentPath) { + searchParams.set('path', parentPath); + } + if (page) { + searchParams.set('page', page.toString()); + } + return searchParams; +}; + +const searchDocs = async ({ + q, + page, + target, + parentPath, +}: SearchDocsParams): Promise => { + const searchParams = constructParams({ q, page, target, parentPath }); + const response = await fetchAPI( + `documents/search/?${searchParams.toString()}`, + ); + + if (!response.ok) { + throw new APIError('Failed to get the docs', await errorCauses(response)); + } + + return response.json() as Promise; +}; + +export const useSearchDocs = ( + { q, page, target, parentPath }: SearchDocsParams, + queryConfig?: { enabled?: boolean }, +) => { + return useQuery({ + queryKey: [KEY_LIST_DOC, 'search', { q, page, target, parentPath }], + queryFn: () => searchDocs({ q, page, target, parentPath }), + ...queryConfig, + }); +}; + +export const useInfiniteSearchDocs = ( + params: SearchDocsParams, + queryConfig?: { enabled?: boolean }, +) => { + return useAPIInfiniteQuery(KEY_LIST_DOC, searchDocs, params, queryConfig); +}; diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/api/useSubDocs.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/api/useSubDocs.tsx deleted file mode 100644 index e76c8bc4e9..0000000000 --- a/src/frontend/apps/impress/src/features/docs/doc-management/api/useSubDocs.tsx +++ /dev/null @@ -1,62 +0,0 @@ -import { UseQueryOptions, useQuery } from '@tanstack/react-query'; - -import { - APIError, - InfiniteQueryConfig, - errorCauses, - fetchAPI, - useAPIInfiniteQuery, -} from '@/api'; - -import { DocsOrdering } from '../types'; - -import { DocsResponse, constructParams } from './useDocs'; - -export type SubDocsParams = { - page: number; - ordering?: DocsOrdering; - is_creator_me?: boolean; - title?: string; - is_favorite?: boolean; - parent_id: string; -}; - -export const getSubDocs = async ( - params: SubDocsParams, -): Promise => { - const searchParams = constructParams(params); - searchParams.set('parent_id', params.parent_id); - - const response: Response = await fetchAPI( - `documents/${params.parent_id}/descendants/?${searchParams.toString()}`, - ); - - if (!response.ok) { - throw new APIError( - 'Failed to get the sub docs', - await errorCauses(response), - ); - } - - return response.json() as Promise; -}; - -export const KEY_LIST_SUB_DOC = 'sub-docs'; - -export function useSubDocs( - params: SubDocsParams, - queryConfig?: UseQueryOptions, -) { - return useQuery({ - queryKey: [KEY_LIST_SUB_DOC, params], - queryFn: () => getSubDocs(params), - ...queryConfig, - }); -} - -export const useInfiniteSubDocs = ( - params: SubDocsParams, - queryConfig?: InfiniteQueryConfig, -) => { - return useAPIInfiniteQuery(KEY_LIST_SUB_DOC, getSubDocs, params, queryConfig); -}; diff --git a/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchContent.tsx b/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchContent.tsx index fc14d8bc63..bdee29c788 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchContent.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchContent.tsx @@ -3,24 +3,31 @@ import { useEffect, useMemo } from 'react'; import { InView } from 'react-intersection-observer'; import { QuickSearchData, QuickSearchGroup } from '@/components/quick-search'; +import { useInfiniteSearchDocs } from '@/docs/doc-management/api/useSearchDocs'; +import { DocSearchTarget } from '@/docs/doc-search'; -import { Doc, useInfiniteDocs } from '../../doc-management'; +import { Doc } from '../../doc-management'; -import { DocSearchFiltersValues } from './DocSearchFilters'; import { DocSearchItem } from './DocSearchItem'; type DocSearchContentProps = { search: string; - filters: DocSearchFiltersValues; onSelect: (doc: Doc) => void; onLoadingChange?: (loading: boolean) => void; + renderElement?: (doc: Doc) => React.ReactNode; + target?: DocSearchTarget; + parentPath?: string; + emptySearchText?: string; }; export const DocSearchContent = ({ search, - filters, onSelect, onLoadingChange, + renderElement = (doc) => , + target, + parentPath, + emptySearchText, }: DocSearchContentProps) => { const { data, @@ -29,21 +36,29 @@ export const DocSearchContent = ({ isLoading, fetchNextPage, hasNextPage, - } = useInfiniteDocs({ - page: 1, - title: search, - ...filters, - }); + } = useInfiniteSearchDocs( + { + q: search, + page: 1, + target, + parentPath, + }, + { + enabled: target !== DocSearchTarget.CURRENT || !!parentPath, + }, + ); const loading = isFetching || isRefetching || isLoading; const docsData: QuickSearchData = useMemo(() => { const docs = data?.pages.flatMap((page) => page.results) || []; + const defaultEmptyText = emptySearchText || t('No document found'); + const emptyText = search ? defaultEmptyText : t('Search by title'); return { groupName: docs.length > 0 ? t('Select a document') : '', elements: search ? docs : [], - emptyString: t('No document found'), + emptyString: target ? emptyText : defaultEmptyText, endActions: hasNextPage ? [ { @@ -52,7 +67,14 @@ export const DocSearchContent = ({ ] : [], }; - }, [search, data?.pages, fetchNextPage, hasNextPage]); + }, [ + search, + data?.pages, + fetchNextPage, + hasNextPage, + emptySearchText, + target, + ]); useEffect(() => { onLoadingChange?.(loading); @@ -62,7 +84,7 @@ export const DocSearchContent = ({ } + renderElement={renderElement} /> ); }; diff --git a/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchModal.tsx b/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchModal.tsx index 24a6d030a6..cfa679a15b 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchModal.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchModal.tsx @@ -1,4 +1,5 @@ import { Modal, ModalSize } from '@gouvfr-lasuite/cunningham-react'; +import { TreeContextType, useTreeContext } from '@gouvfr-lasuite/ui-kit'; import Image from 'next/image'; import { useRouter } from 'next/router'; import { useState } from 'react'; @@ -8,45 +9,40 @@ import { useDebouncedCallback } from 'use-debounce'; import { Box, ButtonCloseModal, Text } from '@/components'; import { QuickSearch } from '@/components/quick-search'; import { Doc, useDocUtils } from '@/docs/doc-management'; +import { + DocSearchFilters, + DocSearchFiltersValues, + DocSearchTarget, +} from '@/docs/doc-search'; import { useResponsiveStore } from '@/stores'; import EmptySearchIcon from '../assets/illustration-docs-empty.png'; import { DocSearchContent } from './DocSearchContent'; -import { - DocSearchFilters, - DocSearchFiltersValues, - DocSearchTarget, -} from './DocSearchFilters'; import { DocSearchItem } from './DocSearchItem'; -import { DocSearchSubPageContent } from './DocSearchSubPageContent'; type DocSearchModalGlobalProps = { onClose: () => void; isOpen: boolean; showFilters?: boolean; defaultFilters?: DocSearchFiltersValues; + treeContext?: TreeContextType | null; }; const DocSearchModalGlobal = ({ showFilters = false, defaultFilters, + treeContext, ...modalProps }: DocSearchModalGlobalProps) => { const { t } = useTranslation(); const [loading, setLoading] = useState(false); - const router = useRouter(); - const isDocPage = router.pathname === '/docs/[id]'; - const [search, setSearch] = useState(''); const [filters, setFilters] = useState( defaultFilters ?? {}, ); - - const target = filters.target ?? DocSearchTarget.ALL; const { isDesktop } = useResponsiveStore(); - const handleInputSearch = useDebouncedCallback(setSearch, 300); const handleSelect = (doc: Doc) => { @@ -120,25 +116,22 @@ const DocSearchModalGlobal = ({ )} {search && ( - <> - {target === DocSearchTarget.ALL && ( - - )} - {isDocPage && target === DocSearchTarget.CURRENT && ( - } - /> - )} - + } + target={ + filters.target === DocSearchTarget.CURRENT + ? DocSearchTarget.CURRENT + : DocSearchTarget.ALL + } + parentPath={ + filters.target === DocSearchTarget.CURRENT + ? treeContext?.root?.path + : undefined + } + /> )} @@ -157,6 +150,7 @@ const DocSearchModalDetail = ({ }: DocSearchModalDetailProps) => { const { hasChildren, isChild } = useDocUtils(doc); const isWithChildren = isChild || hasChildren; + const treeContext = useTreeContext(); let defaultFilters = DocSearchTarget.ALL; let showFilters = false; @@ -170,6 +164,7 @@ const DocSearchModalDetail = ({ {...modalProps} showFilters={showFilters} defaultFilters={{ target: defaultFilters }} + treeContext={treeContext} /> ); }; diff --git a/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchSubPageContent.tsx b/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchSubPageContent.tsx deleted file mode 100644 index 7d82005f63..0000000000 --- a/src/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchSubPageContent.tsx +++ /dev/null @@ -1,103 +0,0 @@ -import { useTreeContext } from '@gouvfr-lasuite/ui-kit'; -import { t } from 'i18next'; -import React, { useEffect, useState } from 'react'; -import { InView } from 'react-intersection-observer'; - -import { QuickSearchData, QuickSearchGroup } from '@/components/quick-search'; -import { Doc, useInfiniteSubDocs } from '@/docs/doc-management'; - -import { DocSearchFiltersValues } from './DocSearchFilters'; - -type DocSearchSubPageContentProps = { - search: string; - filters: DocSearchFiltersValues; - onSelect: (doc: Doc) => void; - onLoadingChange?: (loading: boolean) => void; - renderElement: (doc: Doc) => React.ReactNode; -}; - -export const DocSearchSubPageContent = ({ - search, - filters, - onSelect, - onLoadingChange, - renderElement, -}: DocSearchSubPageContentProps) => { - const treeContext = useTreeContext(); - - const { - data: subDocsData, - isFetching, - isRefetching, - isLoading, - fetchNextPage: subDocsFetchNextPage, - hasNextPage: subDocsHasNextPage, - } = useInfiniteSubDocs( - { - page: 1, - title: search, - ...filters, - parent_id: treeContext?.root?.id ?? '', - }, - { - enabled: !!treeContext?.root?.id, - }, - ); - const [docsData, setDocsData] = useState>({ - groupName: '', - elements: [], - emptyString: '', - }); - - const loading = isFetching || isRefetching || isLoading; - - useEffect(() => { - if (loading) { - return; - } - - const subDocs = subDocsData?.pages.flatMap((page) => page.results) || []; - - if (treeContext?.root) { - const isRootTitleIncludeSearch = treeContext.root?.title - ?.toLowerCase() - .includes(search.toLowerCase()); - - if (isRootTitleIncludeSearch) { - subDocs.unshift(treeContext.root); - } - } - - setDocsData({ - groupName: subDocs.length > 0 ? t('Select a doc') : '', - elements: search ? subDocs : [], - emptyString: search ? t('No document found') : t('Search by title'), - endActions: subDocsHasNextPage - ? [ - { - content: void subDocsFetchNextPage()} />, - }, - ] - : [], - }); - }, [ - loading, - search, - subDocsData?.pages, - subDocsFetchNextPage, - subDocsHasNextPage, - treeContext?.root, - ]); - - useEffect(() => { - onLoadingChange?.(loading); - }, [loading, onLoadingChange]); - - return ( - - ); -}; diff --git a/src/frontend/apps/impress/src/features/docs/doc-search/components/index.ts b/src/frontend/apps/impress/src/features/docs/doc-search/components/index.ts index d5d0e0c444..1a0889239d 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-search/components/index.ts +++ b/src/frontend/apps/impress/src/features/docs/doc-search/components/index.ts @@ -1,3 +1,2 @@ export * from './DocSearchModal'; export * from './DocSearchFilters'; -export * from './DocSearchSubPageContent';