diff --git a/CHANGELOG.md b/CHANGELOG.md index 34cb89ce9..6ec86b65c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to - 🚚(frontend) move Waffle to bottom left #2455 - ♿️(frontend) remove redundant aria-label on table of contents links #2459 - ♻️(core) fix typo in settings COLLABORATION_WS_NOT_CONNECTED_READY_ONLY #2481 +- ♻️(backend) scope document search by document id instead of path #2501 ### Fixed diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 4e32f84b4..0b465f135 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -1009,4 +1009,4 @@ class SearchQueryParamDocumentSerializer(serializers.Serializer): """Serializer for fulltext search requests through Find application""" q = serializers.CharField(required=True, allow_blank=True, trim_whitespace=True) - path = serializers.CharField(required=False, allow_blank=False) + document = serializers.UUIDField(required=False) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index a15fc6137..0b8e5f213 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -1542,15 +1542,23 @@ class DocumentViewSet( """ queryset = models.Document.objects.all() + # The indexer filters descendants by path prefix, so resolve the document + # id to its path before querying it. + path = None + document_id = params.validated_data.get("document") + if document_id: + try: + path = models.Document.objects.get(pk=document_id).values_list( + "path", flat=True + ) + except models.Document.DoesNotExist as exc: + raise drf.exceptions.NotFound("Document not found.") from exc + results = indexer.search( q=params.validated_data["q"], search_type=search_type, token=request.session.get("oidc_access_token"), - path=( - params.validated_data["path"] - if "path" in params.validated_data - else None - ), + path=path, visited=get_visited_document_ids_of(queryset, request.user), ) @@ -1618,7 +1626,7 @@ class DocumentViewSet( Only searches in the title field of documents. """ - if validated_data.get("path"): + if validated_data.get("document"): return self._list_descendants(request, validated_data) top_level_documents = self.get_queryset() @@ -1676,22 +1684,22 @@ class DocumentViewSet( def _list_descendants(self, request, validated_data): """ - 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. + List all documents descending from the document identified by the provided + document id. Includes the parent document itself. + Used internally by the search endpoint when document filtering is requested. """ # Get parent document without access filtering - parent_path = validated_data["path"] + document_id = validated_data["document"] user = request.user try: parent = ( models.Document.objects.annotate_user_roles(user) .annotate_is_favorite(user) .annotate_user_has_link_trace(user) - .get(path=parent_path) + .get(pk=document_id) ) except models.Document.DoesNotExist as exc: - raise drf.exceptions.NotFound("Document not found from path.") from exc + raise drf.exceptions.NotFound("Document not found.") from exc abilities = parent.get_abilities(user) if not abilities.get("search"): 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 f20b0b514..4ccc1f00b 100644 --- a/src/backend/core/tests/documents/test_api_documents_search.py +++ b/src/backend/core/tests/documents/test_api_documents_search.py @@ -547,7 +547,7 @@ def test_api_documents_search_indexer_crashes( parent = factories.DocumentFactory(title="parent", users=[user]) q = "alpha" response = client.get( - "/api/v1.0/documents/search/", data={"q": "alpha", "path": parent.path} + "/api/v1.0/documents/search/", data={"q": "alpha", "document": parent.id} ) # the search endpoint did not crash @@ -555,7 +555,9 @@ def test_api_documents_search_indexer_crashes( # fallback on title_search assert mock_search_using_database.call_count == 1 assert mock_search_using_database.call_args[0][0].GET.get("q") == q - assert mock_search_using_database.call_args[0][0].GET.get("path") == parent.path + assert mock_search_using_database.call_args[0][0].GET.get("document") == str( + parent.id + ) assert response.json() == mocked_response diff --git a/src/backend/core/tests/documents/test_api_documents_search_descendants.py b/src/backend/core/tests/documents/test_api_documents_search_descendants.py index 8abf2984c..a542e4658 100644 --- a/src/backend/core/tests/documents/test_api_documents_search_descendants.py +++ b/src/backend/core/tests/documents/test_api_documents_search_descendants.py @@ -1,6 +1,6 @@ """ Tests for search API endpoint in impress's core app when indexer is not -available and a path param is given. +available and a document param is given. """ # pylint: disable=too-many-lines @@ -34,7 +34,7 @@ def test_api_documents_search_descendants_list_anonymous_public_standalone(): factories.UserDocumentAccessFactory(document=child1) response = APIClient().get( - "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "doc", "document": document.id} ) assert response.status_code == 200 @@ -248,7 +248,7 @@ def test_api_documents_search_descendants_list_anonymous_public_parent(): factories.UserDocumentAccessFactory(document=child1) response = APIClient().get( - "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "doc", "document": document.id} ) assert response.status_code == 200 @@ -448,7 +448,7 @@ def test_api_documents_search_descendants_list_anonymous_restricted_or_authentic _grand_child = factories.DocumentFactory(title="grand child", parent=child) response = APIClient().get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 403 @@ -478,7 +478,7 @@ def test_api_documents_search_descendants_list_authenticated_unrelated_public_or factories.UserDocumentAccessFactory(document=child1) response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 200 @@ -669,7 +669,7 @@ def test_api_documents_search_descendants_list_authenticated_public_or_authentic factories.UserDocumentAccessFactory(document=child1) response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 200 @@ -850,7 +850,7 @@ def test_api_documents_search_descendants_list_authenticated_unrelated_restricte factories.UserDocumentAccessFactory(document=child1) response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 403 @@ -881,7 +881,7 @@ def test_api_documents_search_descendants_list_authenticated_related_direct(): grand_child = factories.DocumentFactory(parent=child1, title="grand child") response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 200 assert response.json() == { @@ -1073,7 +1073,7 @@ def test_api_documents_search_descendants_list_authenticated_related_parent(): grand_child = factories.DocumentFactory(parent=child1, title="grand child") response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 200 assert response.json() == { @@ -1252,7 +1252,7 @@ def test_api_documents_search_descendants_list_authenticated_related_child(): factories.UserDocumentAccessFactory(document=document) response = client.get( - "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "doc", "document": document.id} ) assert response.status_code == 403 assert response.json() == { @@ -1279,7 +1279,7 @@ def test_api_documents_search_descendants_list_authenticated_related_team_none( factories.TeamDocumentAccessFactory(document=document, team="myteam") response = client.get( - "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "doc", "document": document.id} ) assert response.status_code == 403 @@ -1310,7 +1310,7 @@ def test_api_documents_search_descendants_list_authenticated_related_team_member access = factories.TeamDocumentAccessFactory(document=document, team="myteam") response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) # pylint: disable=R0801 @@ -1509,7 +1509,7 @@ def test_api_documents_search_descendants_search_on_title(query, nb_results): # Perform the search query response = client.get( - "/api/v1.0/documents/search/", data={"q": query, "path": parent.path} + "/api/v1.0/documents/search/", data={"q": query, "document": parent.id} ) assert response.status_code == 200