From 2dd4a886800790bf2b1d026ae308061ef792f2b2 Mon Sep 17 00:00:00 2001 From: yyh Date: Mon, 11 May 2026 18:22:28 +0800 Subject: [PATCH] refactor(tag-management): use tag names for console filters --- api/controllers/console/app/app.py | 28 ++++----- api/controllers/console/datasets/datasets.py | 33 ++++++---- api/services/app_service.py | 6 +- api/services/dataset_service.py | 27 +++++++- api/services/tag_service.py | 17 ++++++ .../services/test_app_service.py | 10 +-- .../test_dataset_service_retrieval.py | 18 +++--- .../services/test_tag_service.py | 31 +++++----- .../console/app/test_app_response_models.py | 61 ++++++++++--------- .../console/datasets/test_datasets.py | 8 ++- .../components/apps/__tests__/list.spec.tsx | 12 ++-- .../__tests__/use-apps-query-state.spec.tsx | 24 ++++---- .../apps/hooks/use-apps-query-state.ts | 16 +++-- web/app/components/apps/list.tsx | 15 +++-- .../datasets/list/__tests__/datasets.spec.tsx | 12 ++-- .../datasets/list/__tests__/index.spec.tsx | 29 ++++++--- web/app/components/datasets/list/datasets.tsx | 6 +- web/app/components/datasets/list/index.tsx | 39 ++++++------ web/contract/console/apps.ts | 2 +- .../__tests__/tag-filter.spec.tsx | 22 +++---- .../tag-management/components/tag-filter.tsx | 19 +++--- web/models/datasets.ts | 4 +- web/service/knowledge/use-dataset.ts | 10 +-- 23 files changed, 251 insertions(+), 198 deletions(-) diff --git a/api/controllers/console/app/app.py b/api/controllers/console/app/app.py index 4429039d79f..871a05de89b 100644 --- a/api/controllers/console/app/app.py +++ b/api/controllers/console/app/app.py @@ -1,6 +1,5 @@ import logging import re -import uuid from datetime import datetime from typing import Any, Literal from uuid import UUID @@ -61,7 +60,7 @@ ALLOW_CREATE_APP_MODES = ["chat", "agent-chat", "advanced-chat", "workflow", "co register_enum_models(console_ns, IconType) _logger = logging.getLogger(__name__) -_TAG_IDS_BRACKET_PATTERN = re.compile(r"^tag_ids\[(\d+)\]$") +_TAG_NAMES_BRACKET_PATTERN = re.compile(r"^tag_names\[(\d+)\]$") class AppListQuery(BaseModel): @@ -71,44 +70,41 @@ class AppListQuery(BaseModel): default="all", description="App mode filter" ) name: str | None = Field(default=None, description="Filter by app name") - tag_ids: list[str] | None = Field(default=None, description="Filter by tag IDs") + tag_names: list[str] | None = Field(default=None, description="Filter by tag names") is_created_by_me: bool | None = Field(default=None, description="Filter by creator") - @field_validator("tag_ids", mode="before") + @field_validator("tag_names", mode="before") @classmethod - def validate_tag_ids(cls, value: list[str] | None) -> list[str] | None: + def validate_tag_names(cls, value: list[str] | None) -> list[str] | None: if not value: return None if not isinstance(value, list): - raise ValueError("Unsupported tag_ids type.") + raise ValueError("Unsupported tag_names type.") items = [str(item).strip() for item in value if item and str(item).strip()] if not items: return None - try: - return [str(uuid.UUID(item)) for item in items] - except ValueError as exc: - raise ValueError("Invalid UUID format in tag_ids.") from exc + return items def _normalize_app_list_query_args(query_args: MultiDict[str, str]) -> dict[str, str | list[str]]: normalized: dict[str, str | list[str]] = {} - indexed_tag_ids: list[tuple[int, str]] = [] + indexed_tag_names: list[tuple[int, str]] = [] for key in query_args: - match = _TAG_IDS_BRACKET_PATTERN.fullmatch(key) + match = _TAG_NAMES_BRACKET_PATTERN.fullmatch(key) if match: - indexed_tag_ids.extend((int(match.group(1)), value) for value in query_args.getlist(key)) + indexed_tag_names.extend((int(match.group(1)), value) for value in query_args.getlist(key)) continue value = query_args.get(key) if value is not None: normalized[key] = value - if indexed_tag_ids: - normalized["tag_ids"] = [value for _, value in sorted(indexed_tag_ids)] + if indexed_tag_names: + normalized["tag_names"] = [value for _, value in sorted(indexed_tag_names)] return normalized @@ -483,7 +479,7 @@ class AppListApi(Resource): limit=args.limit, mode=args.mode, name=args.name, - tag_ids=args.tag_ids, + tag_names=args.tag_names, is_created_by_me=args.is_created_by_me, ) diff --git a/api/controllers/console/datasets/datasets.py b/api/controllers/console/datasets/datasets.py index 0e91779b2cc..58350457cfc 100644 --- a/api/controllers/console/datasets/datasets.py +++ b/api/controllers/console/datasets/datasets.py @@ -202,7 +202,16 @@ class ConsoleDatasetListQuery(BaseModel): keyword: str | None = Field(default=None, description="Search keyword") include_all: bool = Field(default=False, description="Include all datasets") ids: list[str] = Field(default_factory=list, description="Filter by dataset IDs") - tag_ids: list[str] = Field(default_factory=list, description="Filter by tag IDs") + tag_names: list[str] = Field(default_factory=list, description="Filter by tag names") + + @field_validator("tag_names", mode="before") + @classmethod + def validate_tag_names(cls, value: list[str] | None) -> list[str]: + if not value: + return [] + if not isinstance(value, list): + raise ValueError("Unsupported tag_names type.") + return [str(item).strip() for item in value if item and str(item).strip()] register_schema_models( @@ -296,7 +305,7 @@ class DatasetListApi(Resource): "limit": "Number of items per page (default: 20)", "ids": "Filter by dataset IDs (list)", "keyword": "Search keyword", - "tag_ids": "Filter by tag IDs (list)", + "tag_names": "Filter by tag names (list)", "include_all": "Include all datasets (default: false)", } ) @@ -309,24 +318,24 @@ class DatasetListApi(Resource): current_user, current_tenant_id = current_account_with_tenant() # Convert query parameters to dict, handling list parameters correctly query_params: dict[str, str | list[str]] = dict(request.args.to_dict()) - # Handle ids and tag_ids as lists (Flask request.args.getlist returns list even for single value) + # Handle ids and tag_names as lists (Flask request.args.getlist returns list even for single value) if "ids" in request.args: query_params["ids"] = request.args.getlist("ids") - if "tag_ids" in request.args: - query_params["tag_ids"] = request.args.getlist("tag_ids") + if "tag_names" in request.args: + query_params["tag_names"] = request.args.getlist("tag_names") query = ConsoleDatasetListQuery.model_validate(query_params) # provider = request.args.get("provider", default="vendor") if query.ids: datasets, total = DatasetService.get_datasets_by_ids(query.ids, current_tenant_id) else: datasets, total = DatasetService.get_datasets( - query.page, - query.limit, - current_tenant_id, - current_user, - query.keyword, - query.tag_ids, - query.include_all, + page=query.page, + per_page=query.limit, + tenant_id=current_tenant_id, + user=current_user, + search=query.keyword, + include_all=query.include_all, + tag_names=query.tag_names, ) # check embedding setting diff --git a/api/services/app_service.py b/api/services/app_service.py index 6716833f6c6..aa8a7e4a785 100644 --- a/api/services/app_service.py +++ b/api/services/app_service.py @@ -37,7 +37,7 @@ class AppListParams(BaseModel): limit: int = Field(default=20, ge=1, le=100) mode: Literal["completion", "chat", "advanced-chat", "workflow", "agent-chat", "channel", "all"] = "all" name: str | None = None - tag_ids: list[str] | None = None + tag_names: list[str] | None = None is_created_by_me: bool | None = None @@ -83,8 +83,8 @@ class AppService: name = params.name[:30] escaped_name = escape_like_pattern(name) filters.append(App.name.ilike(f"%{escaped_name}%", escape="\\")) - if params.tag_ids and len(params.tag_ids) > 0: - target_ids = TagService.get_target_ids_by_tag_ids("app", tenant_id, params.tag_ids) + if params.tag_names and len(params.tag_names) > 0: + target_ids = TagService.get_target_ids_by_tag_names("app", tenant_id, params.tag_names) if target_ids and len(target_ids) > 0: filters.append(App.id.in_(target_ids)) else: diff --git a/api/services/dataset_service.py b/api/services/dataset_service.py index 383474f4f62..74bf8d3904f 100644 --- a/api/services/dataset_service.py +++ b/api/services/dataset_service.py @@ -119,7 +119,16 @@ class AutoDisableLogsDict(TypedDict): class DatasetService: @staticmethod - def get_datasets(page, per_page, tenant_id=None, user=None, search=None, tag_ids=None, include_all=False): + def get_datasets( + page, + per_page, + tenant_id=None, + user=None, + search=None, + tag_ids=None, + include_all=False, + tag_names=None, + ): query = select(Dataset).where(Dataset.tenant_id == tenant_id).order_by(Dataset.created_at.desc(), Dataset.id) if user: @@ -172,8 +181,20 @@ class DatasetService: escaped_search = helper.escape_like_pattern(search) query = query.where(Dataset.name.ilike(f"%{escaped_search}%", escape="\\")) - # Check if tag_ids is not empty to avoid WHERE false condition - if tag_ids and len(tag_ids) > 0: + if tag_names and len(tag_names) > 0: + if tenant_id is not None: + target_ids = TagService.get_target_ids_by_tag_names( + "knowledge", + tenant_id, + tag_names, + ) + else: + target_ids = [] + if target_ids and len(target_ids) > 0: + query = query.where(Dataset.id.in_(target_ids)) + else: + return [], 0 + elif tag_ids and len(tag_ids) > 0: if tenant_id is not None: target_ids = TagService.get_target_ids_by_tag_ids( "knowledge", diff --git a/api/services/tag_service.py b/api/services/tag_service.py index 8043a99be13..e54e8e1757e 100644 --- a/api/services/tag_service.py +++ b/api/services/tag_service.py @@ -74,6 +74,23 @@ class TagService: ).all() return tag_bindings + @staticmethod + def get_target_ids_by_tag_names(tag_type: str, current_tenant_id: str, tag_names: list[str]): + if not tag_names: + return [] + tags = db.session.scalars( + select(Tag).where(Tag.name.in_(tag_names), Tag.tenant_id == current_tenant_id, Tag.type == tag_type) + ).all() + if not tags: + return [] + tag_ids = [tag.id for tag in tags] + tag_bindings = db.session.scalars( + select(TagBinding.target_id).where( + TagBinding.tag_id.in_(tag_ids), TagBinding.tenant_id == current_tenant_id + ) + ).all() + return tag_bindings + @staticmethod def get_tag_by_tag_name(tag_type: str, current_tenant_id: str, tag_name: str): if not tag_type or not tag_name: diff --git a/api/tests/test_containers_integration_tests/services/test_app_service.py b/api/tests/test_containers_integration_tests/services/test_app_service.py index c37fce296f5..8f3d48343a2 100644 --- a/api/tests/test_containers_integration_tests/services/test_app_service.py +++ b/api/tests/test_containers_integration_tests/services/test_app_service.py @@ -342,16 +342,16 @@ class TestAppService: app = app_service.create_app(tenant.id, app_params, account) # Mock TagService to return the app ID for tag filtering - with patch("services.app_service.TagService.get_target_ids_by_tag_ids") as mock_tag_service: + with patch("services.app_service.TagService.get_target_ids_by_tag_names") as mock_tag_service: mock_tag_service.return_value = [app.id] # Test with tag filter - params = AppListParams(page=1, limit=10, mode="chat", tag_ids=["tag1", "tag2"]) + params = AppListParams(page=1, limit=10, mode="chat", tag_names=["Finance", "Support"]) paginated_apps = app_service.get_paginate_apps(account.id, tenant.id, params) # Verify tag service was called - mock_tag_service.assert_called_once_with("app", tenant.id, ["tag1", "tag2"]) + mock_tag_service.assert_called_once_with("app", tenant.id, ["Finance", "Support"]) # Verify results assert paginated_apps is not None @@ -359,10 +359,10 @@ class TestAppService: assert paginated_apps.items[0].id == app.id # Test with tag filter that returns no results - with patch("services.app_service.TagService.get_target_ids_by_tag_ids") as mock_tag_service: + with patch("services.app_service.TagService.get_target_ids_by_tag_names") as mock_tag_service: mock_tag_service.return_value = [] - params = AppListParams(page=1, limit=10, mode="chat", tag_ids=["nonexistent_tag"]) + params = AppListParams(page=1, limit=10, mode="chat", tag_names=["Missing"]) paginated_apps = app_service.get_paginate_apps(account.id, tenant.id, params) diff --git a/api/tests/test_containers_integration_tests/services/test_dataset_service_retrieval.py b/api/tests/test_containers_integration_tests/services/test_dataset_service_retrieval.py index 0c610311bb3..72fb10c5373 100644 --- a/api/tests/test_containers_integration_tests/services/test_dataset_service_retrieval.py +++ b/api/tests/test_containers_integration_tests/services/test_dataset_service_retrieval.py @@ -264,7 +264,7 @@ class TestDatasetServiceGetDatasets: assert total == 1 def test_get_datasets_with_tag_filtering(self, db_session_with_containers: Session): - """Test get_datasets with tag_ids filtering.""" + """Test get_datasets with tag_names filtering.""" # Arrange account, tenant = DatasetRetrievalTestDataFactory.create_account_with_tenant(db_session_with_containers) page = 1 @@ -289,22 +289,22 @@ class TestDatasetServiceGetDatasets: tag_2 = DatasetRetrievalTestDataFactory.create_tag_binding( db_session_with_containers, tenant.id, account.id, dataset_2.id ) - tag_ids = [tag_1.id, tag_2.id] + tag_names = [tag_1.name, tag_2.name] # Act - datasets, total = DatasetService.get_datasets(page, per_page, tenant_id=tenant.id, tag_ids=tag_ids) + datasets, total = DatasetService.get_datasets(page, per_page, tenant_id=tenant.id, tag_names=tag_names) # Assert - assert len(datasets) == 2 + assert {dataset.id for dataset in datasets} == {dataset_1.id, dataset_2.id} assert total == 2 - def test_get_datasets_with_empty_tag_ids(self, db_session_with_containers: Session): - """Test get_datasets with empty tag_ids skips tag filtering and returns all matching datasets.""" + def test_get_datasets_with_empty_tag_names(self, db_session_with_containers: Session): + """Test get_datasets with empty tag_names skips tag filtering and returns all matching datasets.""" # Arrange account, tenant = DatasetRetrievalTestDataFactory.create_account_with_tenant(db_session_with_containers) page = 1 per_page = 20 - tag_ids = [] + tag_names = [] for i in range(3): DatasetRetrievalTestDataFactory.create_dataset( @@ -316,10 +316,10 @@ class TestDatasetServiceGetDatasets: ) # Act - datasets, total = DatasetService.get_datasets(page, per_page, tenant_id=tenant.id, tag_ids=tag_ids) + datasets, total = DatasetService.get_datasets(page, per_page, tenant_id=tenant.id, tag_names=tag_names) # Assert - # When tag_ids is empty, tag filtering is skipped, so normal query results are returned + # When tag_names is empty, tag filtering is skipped, so normal query results are returned assert len(datasets) == 3 assert total == 3 diff --git a/api/tests/test_containers_integration_tests/services/test_tag_service.py b/api/tests/test_containers_integration_tests/services/test_tag_service.py index 583b6128e67..7f9ae0410cd 100644 --- a/api/tests/test_containers_integration_tests/services/test_tag_service.py +++ b/api/tests/test_containers_integration_tests/services/test_tag_service.py @@ -412,14 +412,14 @@ class TestTagService: assert len(result) == 0 assert isinstance(result, list) - def test_get_target_ids_by_tag_ids_success( + def test_get_target_ids_by_tag_names_success( self, db_session_with_containers: Session, mock_external_service_dependencies ): """ - Test successful retrieval of target IDs by tag IDs. + Test successful retrieval of target IDs by tag names. This test verifies: - - Proper target ID retrieval for valid tag IDs + - Proper target ID retrieval for valid tag names - Correct filtering by tag type and tenant - Proper handling of tag bindings """ @@ -448,8 +448,8 @@ class TestTagService: ) # Act: Execute the method under test - tag_ids = [tag.id for tag in tags] - result = TagService.get_target_ids_by_tag_ids("knowledge", tenant.id, tag_ids) + tag_names = [tag.name for tag in tags] + result = TagService.get_target_ids_by_tag_names("knowledge", tenant.id, tag_names) # Assert: Verify the expected outcomes assert result is not None @@ -468,14 +468,14 @@ class TestTagService: second_dataset_count = result.count(datasets[1].id) assert second_dataset_count == 1 - def test_get_target_ids_by_tag_ids_empty_tag_ids( + def test_get_target_ids_by_tag_names_empty_tag_names( self, db_session_with_containers: Session, mock_external_service_dependencies ): """ - Test target ID retrieval with empty tag IDs list. + Test target ID retrieval with empty tag names list. This test verifies: - - Proper handling of empty tag IDs + - Proper handling of empty tag names - Correct return value for empty input """ # Arrange: Create test data @@ -484,22 +484,22 @@ class TestTagService: db_session_with_containers, mock_external_service_dependencies ) - # Act: Execute the method under test with empty tag IDs - result = TagService.get_target_ids_by_tag_ids("knowledge", tenant.id, []) + # Act: Execute the method under test with empty tag names + result = TagService.get_target_ids_by_tag_names("knowledge", tenant.id, []) # Assert: Verify the expected outcomes assert result is not None assert len(result) == 0 assert isinstance(result, list) - def test_get_target_ids_by_tag_ids_no_matching_tags( + def test_get_target_ids_by_tag_names_no_matching_tags( self, db_session_with_containers: Session, mock_external_service_dependencies ): """ Test target ID retrieval when no tags match the criteria. This test verifies: - - Proper handling of non-existent tag IDs + - Proper handling of non-existent tag names - Correct return value for no matches """ # Arrange: Create test data @@ -508,13 +508,10 @@ class TestTagService: db_session_with_containers, mock_external_service_dependencies ) - # Create non-existent tag IDs - import uuid - - non_existent_tag_ids = [str(uuid.uuid4()), str(uuid.uuid4())] + non_existent_tag_names = ["missing-tag-1", "missing-tag-2"] # Act: Execute the method under test - result = TagService.get_target_ids_by_tag_ids("knowledge", tenant.id, non_existent_tag_ids) + result = TagService.get_target_ids_by_tag_names("knowledge", tenant.id, non_existent_tag_names) # Assert: Verify the expected outcomes assert result is not None diff --git a/api/tests/unit_tests/controllers/console/app/test_app_response_models.py b/api/tests/unit_tests/controllers/console/app/test_app_response_models.py index 80e7c41a9ef..bdd76151f6b 100644 --- a/api/tests/unit_tests/controllers/console/app/test_app_response_models.py +++ b/api/tests/unit_tests/controllers/console/app/test_app_response_models.py @@ -10,7 +10,6 @@ from typing import Any import pytest from flask.views import MethodView -from pydantic import ValidationError from werkzeug.datastructures import MultiDict # kombu references MethodView as a global when importing celery/kombu pools. @@ -176,22 +175,22 @@ def _dummy_workflow(): ) -def test_app_list_query_normalizes_orpc_bracket_tag_ids(app_module): - first_tag_id = "8c4ef3d1-58a1-4d94-8a1c-1c171d889e08" - second_tag_id = "3c39395b-6d1f-4030-8b17-eaa7cc85221c" +def test_app_list_query_normalizes_orpc_bracket_tag_names(app_module): + first_tag_name = "Finance" + second_tag_name = "Support" query_args = MultiDict( [ ("page", "1"), ("limit", "30"), - ("tag_ids[1]", second_tag_id), - ("tag_ids[0]", first_tag_id), + ("tag_names[1]", second_tag_name), + ("tag_names[0]", first_tag_name), ] ) normalized = app_module._normalize_app_list_query_args(query_args) query = app_module.AppListQuery.model_validate(normalized) - assert query.tag_ids == [first_tag_id, second_tag_id] + assert query.tag_names == [first_tag_name, second_tag_name] def test_app_list_query_preserves_regular_query_params(app_module): @@ -220,55 +219,57 @@ def test_app_list_query_preserves_regular_query_params(app_module): assert query.mode == "chat" assert query.name == "Sales Copilot" assert query.is_created_by_me is True - assert query.tag_ids is None + assert query.tag_names is None -def test_app_list_query_normalizes_empty_bracket_tag_ids_to_none(app_module): +def test_app_list_query_normalizes_empty_bracket_tag_names_to_none(app_module): query_args = MultiDict( [ - ("tag_ids[0]", ""), - ("tag_ids[1]", " "), + ("tag_names[0]", ""), + ("tag_names[1]", " "), ] ) normalized = app_module._normalize_app_list_query_args(query_args) query = app_module.AppListQuery.model_validate(normalized) - assert normalized == {"tag_ids": ["", " "]} - assert query.tag_ids is None + assert normalized == {"tag_names": ["", " "]} + assert query.tag_names is None -def test_app_list_query_rejects_invalid_bracket_tag_id(app_module): - normalized = app_module._normalize_app_list_query_args(MultiDict([("tag_ids[0]", "not-a-uuid")])) +def test_app_list_query_accepts_non_uuid_bracket_tag_name(app_module): + normalized = app_module._normalize_app_list_query_args(MultiDict([("tag_names[0]", "not-a-uuid")])) - with pytest.raises(ValidationError): - app_module.AppListQuery.model_validate(normalized) + query = app_module.AppListQuery.model_validate(normalized) + + assert query.tag_names == ["not-a-uuid"] -def test_app_list_query_sorts_bracket_tag_ids_by_index(app_module): - first_tag_id = "8c4ef3d1-58a1-4d94-8a1c-1c171d889e08" - second_tag_id = "3c39395b-6d1f-4030-8b17-eaa7cc85221c" - third_tag_id = "9d5ec0f7-4f2b-4e7f-9c13-1e7a034d0eb1" +def test_app_list_query_sorts_bracket_tag_names_by_index(app_module): + first_tag_name = "Finance" + second_tag_name = "Support" + third_tag_name = "Marketing" query_args = MultiDict( [ - ("tag_ids[2]", third_tag_id), - ("tag_ids[1]", second_tag_id), - ("tag_ids[0]", first_tag_id), + ("tag_names[2]", third_tag_name), + ("tag_names[1]", second_tag_name), + ("tag_names[0]", first_tag_name), ] ) normalized = app_module._normalize_app_list_query_args(query_args) query = app_module.AppListQuery.model_validate(normalized) - assert query.tag_ids == [first_tag_id, second_tag_id, third_tag_id] + assert query.tag_names == [first_tag_name, second_tag_name, third_tag_name] -def test_app_list_query_rejects_flat_tag_ids(app_module): - tag_id = "8c4ef3d1-58a1-4d94-8a1c-1c171d889e08" - normalized = app_module._normalize_app_list_query_args(MultiDict([("tag_ids", tag_id)])) +def test_app_list_query_does_not_map_old_tag_ids_to_tag_names(app_module): + normalized = app_module._normalize_app_list_query_args( + MultiDict([("tag_ids", "8c4ef3d1-58a1-4d94-8a1c-1c171d889e08")]) + ) + query = app_module.AppListQuery.model_validate(normalized) - with pytest.raises(ValidationError): - app_module.AppListQuery.model_validate(normalized) + assert query.tag_names is None def test_app_partial_serialization_uses_aliases(app_models): diff --git a/api/tests/unit_tests/controllers/console/datasets/test_datasets.py b/api/tests/unit_tests/controllers/console/datasets/test_datasets.py index e28d68ee5a7..e5e4f907947 100644 --- a/api/tests/unit_tests/controllers/console/datasets/test_datasets.py +++ b/api/tests/unit_tests/controllers/console/datasets/test_datasets.py @@ -129,7 +129,7 @@ class TestDatasetList: assert status == 200 assert resp["total"] == 2 - def test_get_with_tag_ids(self, app: Flask): + def test_get_with_tag_names(self, app: Flask): api = DatasetListApi() method = unwrap(api.get) @@ -137,7 +137,7 @@ class TestDatasetList: datasets = [MagicMock()] marshaled = [self._mock_dataset_dict()] - with app.test_request_context("/datasets?tag_ids=tag1"): + with app.test_request_context("/datasets?tag_names=Finance&tag_names=Support"): with ( patch( "controllers.console.datasets.datasets.current_account_with_tenant", @@ -147,7 +147,7 @@ class TestDatasetList: DatasetService, "get_datasets", return_value=(datasets, 1), - ), + ) as get_datasets_mock, patch( "controllers.console.datasets.datasets.marshal", return_value=marshaled, @@ -160,6 +160,8 @@ class TestDatasetList: ): resp, status = method(api) + get_datasets_mock.assert_called_once() + assert get_datasets_mock.call_args.kwargs["tag_names"] == ["Finance", "Support"] assert status == 200 def test_embedding_available_false(self, app: Flask): diff --git a/web/app/components/apps/__tests__/list.spec.tsx b/web/app/components/apps/__tests__/list.spec.tsx index 0c6f1702d71..b90aa74bb9c 100644 --- a/web/app/components/apps/__tests__/list.spec.tsx +++ b/web/app/components/apps/__tests__/list.spec.tsx @@ -49,12 +49,12 @@ vi.mock('@/context/app-context', () => ({ })) const mockSetKeywords = vi.fn() -const mockSetTagIDs = vi.fn() +const mockSetTagNames = vi.fn() const mockSetIsCreatedByMe = vi.fn() const mockSetCategory = vi.fn() const mockQueryState = { category: 'all', - tagIDs: [] as string[], + tagNames: [] as string[], keywords: '', isCreatedByMe: false, } @@ -64,7 +64,7 @@ vi.mock('../hooks/use-apps-query-state', () => ({ query: mockQueryState, setCategory: mockSetCategory, setKeywords: mockSetKeywords, - setTagIDs: mockSetTagIDs, + setTagNames: mockSetTagNames, setIsCreatedByMe: mockSetIsCreatedByMe, }), })) @@ -253,7 +253,7 @@ describe('List', () => { mockServiceState.isLoading = false mockServiceState.isFetchingNextPage = false mockQueryState.category = 'all' - mockQueryState.tagIDs = [] + mockQueryState.tagNames = [] mockQueryState.keywords = '' mockQueryState.isCreatedByMe = false mockUseWorkflowOnlineUsers.mockClear() @@ -375,7 +375,7 @@ describe('List', () => { describe('App List Query', () => { it('should build paged query input from active filters', () => { - mockQueryState.tagIDs = ['tag-1'] + mockQueryState.tagNames = ['Frontend'] mockQueryState.keywords = 'sales' mockQueryState.isCreatedByMe = true mockQueryState.category = AppModeEnum.WORKFLOW @@ -389,7 +389,7 @@ describe('List', () => { page: 2, limit: 30, name: 'sales', - tag_ids: ['tag-1'], + tag_names: ['Frontend'], is_created_by_me: true, mode: AppModeEnum.WORKFLOW, }, diff --git a/web/app/components/apps/hooks/__tests__/use-apps-query-state.spec.tsx b/web/app/components/apps/hooks/__tests__/use-apps-query-state.spec.tsx index 782f6ec3534..4cde9798344 100644 --- a/web/app/components/apps/hooks/__tests__/use-apps-query-state.spec.tsx +++ b/web/app/components/apps/hooks/__tests__/use-apps-query-state.spec.tsx @@ -18,24 +18,24 @@ describe('useAppsQueryState', () => { expect(result.current.query).toEqual({ category: 'all', - tagIDs: [], + tagNames: [], keywords: '', isCreatedByMe: false, }) expect(typeof result.current.setCategory).toBe('function') expect(typeof result.current.setKeywords).toBe('function') - expect(typeof result.current.setTagIDs).toBe('function') + expect(typeof result.current.setTagNames).toBe('function') expect(typeof result.current.setIsCreatedByMe).toBe('function') }) it('should parse app list filters from URL', () => { const { result } = renderWithAdapter( - '?category=workflow&tagIDs=tag1;tag2&keywords=search+term&isCreatedByMe=true', + '?category=workflow&tags=Frontend;Backend&keywords=search+term&isCreatedByMe=true', ) expect(result.current.query).toEqual({ category: AppModeEnum.WORKFLOW, - tagIDs: ['tag1', 'tag2'], + tagNames: ['Frontend', 'Backend'], keywords: 'search term', isCreatedByMe: true, }) @@ -121,27 +121,27 @@ describe('useAppsQueryState', () => { const { result, onUrlUpdate } = renderWithAdapter() act(() => { - result.current.setTagIDs(['tag1', 'tag2']) + result.current.setTagNames(['Frontend', 'Backend']) }) await waitFor(() => expect(onUrlUpdate).toHaveBeenCalled()) const update = onUrlUpdate.mock.calls.at(-1)![0] - expect(result.current.query.tagIDs).toEqual(['tag1', 'tag2']) - expect(update.searchParams.get('tagIDs')).toBe('tag1;tag2') + expect(result.current.query.tagNames).toEqual(['Frontend', 'Backend']) + expect(update.searchParams.get('tags')).toBe('Frontend;Backend') expect(update.options.history).toBe('push') }) - it('should remove tagIDs from URL when empty', async () => { - const { result, onUrlUpdate } = renderWithAdapter('?tagIDs=tag1;tag2') + it('should remove tags from URL when empty', async () => { + const { result, onUrlUpdate } = renderWithAdapter('?tags=Frontend;Backend') act(() => { - result.current.setTagIDs([]) + result.current.setTagNames([]) }) await waitFor(() => expect(onUrlUpdate).toHaveBeenCalled()) const update = onUrlUpdate.mock.calls.at(-1)![0] - expect(result.current.query.tagIDs).toEqual([]) - expect(update.searchParams.has('tagIDs')).toBe(false) + expect(result.current.query.tagNames).toEqual([]) + expect(update.searchParams.has('tags')).toBe(false) }) it('should update created-by-me URL state', async () => { diff --git a/web/app/components/apps/hooks/use-apps-query-state.ts b/web/app/components/apps/hooks/use-apps-query-state.ts index a0109eb0618..19fc1010dc9 100644 --- a/web/app/components/apps/hooks/use-apps-query-state.ts +++ b/web/app/components/apps/hooks/use-apps-query-state.ts @@ -16,7 +16,7 @@ const appListQueryParsers = { category: parseAsStringLiteral(APP_LIST_CATEGORY_VALUES) .withDefault('all') .withOptions({ history: 'push' }), - tagIDs: parseAsArrayOf(parseAsString, ';') + tagNames: parseAsArrayOf(parseAsString, ';') .withDefault([]) .withOptions({ history: 'push' }), keywords: parseAsString.withDefault('').withOptions({ @@ -28,7 +28,11 @@ const appListQueryParsers = { } export function useAppsQueryState() { - const [query, setQuery] = useQueryStates(appListQueryParsers) + const [query, setQuery] = useQueryStates(appListQueryParsers, { + urlKeys: { + tagNames: 'tags', + }, + }) const setCategory = useCallback((category: AppListCategory) => { setQuery({ category }) @@ -38,8 +42,8 @@ export function useAppsQueryState() { setQuery({ keywords }) }, [setQuery]) - const setTagIDs = useCallback((tagIDs: string[]) => { - setQuery({ tagIDs }) + const setTagNames = useCallback((tagNames: string[]) => { + setQuery({ tagNames }) }, [setQuery]) const setIsCreatedByMe = useCallback((isCreatedByMe: boolean) => { @@ -50,7 +54,7 @@ export function useAppsQueryState() { query, setCategory, setKeywords, - setTagIDs, + setTagNames, setIsCreatedByMe, - }), [query, setCategory, setKeywords, setTagIDs, setIsCreatedByMe]) + }), [query, setCategory, setKeywords, setTagNames, setIsCreatedByMe]) } diff --git a/web/app/components/apps/list.tsx b/web/app/components/apps/list.tsx index e2e8e737fcb..c21fb1e9de0 100644 --- a/web/app/components/apps/list.tsx +++ b/web/app/components/apps/list.tsx @@ -1,6 +1,5 @@ 'use client' -import type { FC } from 'react' import type { AppListQuery } from '@/contract/console/apps' import { cn } from '@langgenius/dify-ui/cn' import { keepPreviousData, useInfiniteQuery, useSuspenseQuery } from '@tanstack/react-query' @@ -38,19 +37,19 @@ const CreateFromDSLModal = dynamic(() => import('@/app/components/app/create-fro type Props = { controlRefreshList?: number } -const List: FC = ({ +function List({ controlRefreshList = 0, -}) => { +}: Props) { const { t } = useTranslation() const { data: systemFeatures } = useSuspenseQuery(systemFeaturesQueryOptions()) const { isCurrentWorkspaceEditor, isCurrentWorkspaceDatasetOperator, isLoadingCurrentWorkspace } = useAppContext() // eslint-disable-next-line react/use-state -- custom URL query hook, not React.useState const { - query: { category, tagIDs, keywords, isCreatedByMe }, + query: { category, tagNames, keywords, isCreatedByMe }, setCategory, setKeywords, - setTagIDs, + setTagNames, setIsCreatedByMe, } = useAppsQueryState() const debouncedKeywords = useDebounce(keywords, { wait: APP_LIST_SEARCH_DEBOUNCE_MS }) @@ -75,10 +74,10 @@ const List: FC = ({ page: 1, limit: 30, name: debouncedKeywords, - ...(tagIDs.length ? { tag_ids: tagIDs } : {}), + ...(tagNames.length ? { tag_names: tagNames } : {}), ...(isCreatedByMe ? { is_created_by_me: isCreatedByMe } : {}), ...(category !== 'all' ? { mode: category } : {}), - }), [category, debouncedKeywords, isCreatedByMe, tagIDs]) + }), [category, debouncedKeywords, isCreatedByMe, tagNames]) const { data, @@ -209,7 +208,7 @@ const List: FC = ({ {t('showMyCreatedAppsOnly', { ns: 'app' })} - setShowTagManagementModal(true)} /> + setShowTagManagementModal(true)} /> { const defaultProps = { - tags: [], + tagNames: [], keywords: '', includeAll: false, } @@ -197,12 +197,12 @@ describe('Datasets', () => { }) describe('Props', () => { - it('should pass tags to useDatasetList', async () => { + it('should pass tag names to useDatasetList', async () => { const { useDatasetList } = await import('@/service/knowledge/use-dataset') - render() + render() expect(useDatasetList).toHaveBeenCalledWith( expect.objectContaining({ - tag_ids: ['tag-1', 'tag-2'], + tag_names: ['Finance', 'Support'], }), ) }) @@ -448,8 +448,8 @@ describe('Datasets', () => { }) describe('Edge Cases', () => { - it('should handle empty tags array', () => { - render() + it('should handle empty tag names array', () => { + render() expect(screen.getByRole('navigation')).toBeInTheDocument() }) diff --git a/web/app/components/datasets/list/__tests__/index.spec.tsx b/web/app/components/datasets/list/__tests__/index.spec.tsx index 7e46c46f1a3..35c19bef439 100644 --- a/web/app/components/datasets/list/__tests__/index.spec.tsx +++ b/web/app/components/datasets/list/__tests__/index.spec.tsx @@ -1,13 +1,17 @@ import type { ReactElement } from 'react' import { fireEvent, screen, waitFor } from '@testing-library/react' +import { NuqsTestingAdapter } from 'nuqs/adapters/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' import { renderWithSystemFeatures } from '@/__tests__/utils/mock-system-features' import List from '../index' let mockBrandingEnabled = false -const render = (ui: ReactElement) => renderWithSystemFeatures(ui, { - systemFeatures: { branding: { enabled: mockBrandingEnabled } }, -}) +const render = (ui: ReactElement, searchParams = '') => renderWithSystemFeatures( + {ui}, + { + systemFeatures: { branding: { enabled: mockBrandingEnabled } }, + }, +) const mockPush = vi.fn() const mockReplace = vi.fn() @@ -71,9 +75,9 @@ vi.mock('@/service/knowledge/use-dataset', () => ({ // Mock Datasets component vi.mock('../datasets', () => ({ - default: ({ tags, keywords, includeAll }: { tags: string[], keywords: string, includeAll: boolean }) => ( + default: ({ tagNames, keywords, includeAll }: { tagNames: string[], keywords: string, includeAll: boolean }) => (
- {tags.join(',')} + {tagNames.join(',')} {keywords} {includeAll ? 'true' : 'false'}
@@ -111,7 +115,7 @@ vi.mock('@/features/tag-management/components/tag-management-modal', () => ({ vi.mock('@/features/tag-management/components/tag-filter', () => ({ TagFilter: ({ onChange, onOpenTagManagement }: { value: string[], onChange: (val: string[]) => void, onOpenTagManagement: () => void }) => (
- +
), @@ -180,6 +184,11 @@ describe('List', () => { render() expect(screen.getByTestId('tags')).toHaveTextContent('') }) + + it('should read tag names from URL', () => { + render(, '?tags=Finance;Support') + expect(screen.getByTestId('tags')).toHaveTextContent('Finance,Support') + }) }) describe('User Interactions', () => { @@ -201,13 +210,15 @@ describe('List', () => { expect(input).toHaveValue('test search') }) - it('should trigger tag filter change', () => { + it('should pass selected tag names to Datasets', async () => { render() - // Tag filter is rendered and interactive const selectTagsBtn = screen.getByText('Select Tags') expect(selectTagsBtn).toBeInTheDocument() fireEvent.click(selectTagsBtn) - // The onChange callback was triggered (debounced) + + await waitFor(() => { + expect(screen.getByTestId('tags')).toHaveTextContent('Finance,Support') + }) }) }) diff --git a/web/app/components/datasets/list/datasets.tsx b/web/app/components/datasets/list/datasets.tsx index 1e5c268636a..8da72f8571a 100644 --- a/web/app/components/datasets/list/datasets.tsx +++ b/web/app/components/datasets/list/datasets.tsx @@ -9,14 +9,14 @@ import DatasetCard from './dataset-card' import NewDatasetCard from './new-dataset-card' type Props = { - tags: string[] + tagNames: string[] keywords: string includeAll: boolean onOpenTagManagement?: () => void } const Datasets = ({ - tags, + tagNames, keywords, includeAll, onOpenTagManagement = () => {}, @@ -31,7 +31,7 @@ const Datasets = ({ isFetchingNextPage, } = useDatasetList({ initialPage: 1, - tag_ids: tags, + tag_names: tagNames, limit: 30, include_all: includeAll, keyword: keywords, diff --git a/web/app/components/datasets/list/index.tsx b/web/app/components/datasets/list/index.tsx index 3d405de95d7..b302e0b3d93 100644 --- a/web/app/components/datasets/list/index.tsx +++ b/web/app/components/datasets/list/index.tsx @@ -2,7 +2,8 @@ import { Button } from '@langgenius/dify-ui/button' import { useSuspenseQuery } from '@tanstack/react-query' -import { useBoolean, useDebounceFn } from 'ahooks' +import { useBoolean, useDebounce } from 'ahooks' +import { parseAsArrayOf, parseAsString, useQueryStates } from 'nuqs' // Libraries import { useState } from 'react' @@ -33,22 +34,18 @@ const List = () => { useDocumentTitle(t('knowledge', { ns: 'dataset' })) const [keywords, setKeywords] = useState('') - const [searchKeywords, setSearchKeywords] = useState('') - const { run: handleSearch } = useDebounceFn(() => { - setSearchKeywords(keywords) - }, { wait: 500 }) - const handleKeywordsChange = (value: string) => { - setKeywords(value) - handleSearch() - } - const [tagFilterValue, setTagFilterValue] = useState([]) - const [tagIDs, setTagIDs] = useState([]) - const { run: handleTagsUpdate } = useDebounceFn(() => { - setTagIDs(tagFilterValue) - }, { wait: 500 }) - const handleTagsChange = (value: string[]) => { - setTagFilterValue(value) - handleTagsUpdate() + const searchKeywords = useDebounce(keywords, { wait: 500 }) + const [{ tagNames }, setListQuery] = useQueryStates({ + tagNames: parseAsArrayOf(parseAsString, ';') + .withDefault([]) + .withOptions({ history: 'push' }), + }, { + urlKeys: { + tagNames: 'tags', + }, + }) + const setTagNames = (tagNames: string[]) => { + setListQuery({ tagNames }) } const isCurrentWorkspaceManager = useAppContextSelector(state => state.isCurrentWorkspaceManager) @@ -68,14 +65,14 @@ const List = () => { tooltip={t('allKnowledgeDescription', { ns: 'dataset' }) as string} /> )} - setShowTagManagementModal(true)} /> + setShowTagManagementModal(true)} /> handleKeywordsChange(e.target.value)} - onClear={() => handleKeywordsChange('')} + onChange={e => setKeywords(e.target.value)} + onClear={() => setKeywords('')} /> { isCurrentWorkspaceManager && ( @@ -92,7 +89,7 @@ const List = () => { - setShowTagManagementModal(true)} /> + setShowTagManagementModal(true)} /> {!systemFeatures.branding.enabled && } { }) it('should display the first selected tag name when tags are selected', () => { - render() + render() expect(screen.getByText('Frontend')).toBeInTheDocument() }) it('should display the count badge when multiple tags are selected', () => { - render() + render() expect(screen.getByText('Frontend')).toBeInTheDocument() expect(screen.getByText('+1')).toBeInTheDocument() }) it('should display correct count badge for three selected tags', () => { - render() + render() expect(screen.getByText('+2')).toBeInTheDocument() }) it('should not show placeholder when tags are selected', () => { - render() + render() expect(screen.queryByText(i18n.placeholder)).not.toBeInTheDocument() }) }) @@ -99,7 +99,7 @@ describe('TagFilter', () => { await user.click(screen.getByText(i18n.placeholder)) await user.click(screen.getByText('Frontend')) - expect(onChange).toHaveBeenCalledWith(['tag-1']) + expect(onChange).toHaveBeenCalledWith(['Frontend']) }) it('should select the highlighted tag with keyboard navigation', async () => { @@ -112,13 +112,13 @@ describe('TagFilter', () => { await user.keyboard('{ArrowDown}') await user.keyboard('{Enter}') - expect(onChange).toHaveBeenCalledWith(['tag-2']) + expect(onChange).toHaveBeenCalledWith(['Backend']) }) it('should call onChange to deselect when an already-selected tag is clicked', async () => { const user = userEvent.setup() const onChange = vi.fn() - render() + render() // Open dropdown — trigger shows the tag name "Frontend" await user.click(screen.getByText('Frontend')) @@ -157,17 +157,17 @@ describe('TagFilter', () => { it('should add a tag to the selection', async () => { const user = userEvent.setup() const onChange = vi.fn() - render() + render() await user.click(screen.getByText('Frontend')) await user.click(screen.getByTitle('Backend')) - expect(onChange).toHaveBeenCalledWith(['tag-1', 'tag-2']) + expect(onChange).toHaveBeenCalledWith(['Frontend', 'Backend']) }) it('should show check icon for selected tags in dropdown', async () => { const user = userEvent.setup() - render() + render() await user.click(screen.getByText('Frontend')) @@ -180,7 +180,7 @@ describe('TagFilter', () => { it('should clear all selected tags when clear button is clicked', async () => { const user = userEvent.setup() const onChange = vi.fn() - render() + render() const clearButton = screen.getByRole('button', { name: i18n.operationClear }) expect(clearButton).toBeInTheDocument() diff --git a/web/features/tag-management/components/tag-filter.tsx b/web/features/tag-management/components/tag-filter.tsx index 2c6938dc4d5..41eaf88c66d 100644 --- a/web/features/tag-management/components/tag-filter.tsx +++ b/web/features/tag-management/components/tag-filter.tsx @@ -38,22 +38,21 @@ export const TagFilter = ({ }, })) - const tagById = useMemo(() => new Map(tagList.map(tag => [tag.id, tag])), [tagList]) + const tagByName = useMemo(() => new Map(tagList.map(tag => [tag.name, tag])), [tagList]) const items = useMemo(() => tagList.filter(tag => tag.type === type), [tagList, type]) const selectedTags = useMemo(() => { - return value.flatMap((tagId) => { - const tag = tagById.get(tagId) + return value.flatMap((tagName) => { + const tag = tagByName.get(tagName) return tag ? [tag] : [] }) - }, [tagById, value]) + }, [tagByName, value]) - const firstTagId = value[0] - const currentTagName = firstTagId ? tagById.get(firstTagId)?.name : undefined - const triggerLabel = selectedTags.length ? selectedTags.map(tag => tag.name).join(', ') : t('tag.placeholder', { ns: 'common' }) + const firstTagName = value[0] + const currentTagName = firstTagName + const triggerLabel = value.length ? value.join(', ') : t('tag.placeholder', { ns: 'common' }) const handleValueChange = useCallback((nextTags: Tag[]) => { - const unknownTagIds = value.filter(tagId => !tagById.has(tagId)) - onChange([...unknownTagIds, ...nextTags.map(tag => tag.id)]) - }, [onChange, tagById, value]) + onChange(nextTags.map(tag => tag.name)) + }, [onChange]) return ( page = 1, limit, ids, - tag_ids, + tag_names, include_all, keyword, } = params @@ -43,7 +43,7 @@ const normalizeDatasetsParams = (params: Partial page, ...(limit ? { limit } : {}), ...(ids?.length ? { ids } : {}), - ...(tag_ids?.length ? { tag_ids } : {}), + ...(tag_names?.length ? { tag_names } : {}), ...(include_all !== undefined ? { include_all } : {}), ...(keyword ? { keyword } : {}), } @@ -81,12 +81,12 @@ export const useInfiniteDatasets = ( } export const useDatasetList = (params: DatasetListRequest) => { - const { initialPage, tag_ids, limit, include_all, keyword } = params + const { initialPage, tag_names, limit, include_all, keyword } = params return useInfiniteQuery({ - queryKey: [...datasetListQueryKey, initialPage, tag_ids, limit, include_all, keyword], + queryKey: [...datasetListQueryKey, initialPage, tag_names, limit, include_all, keyword], queryFn: ({ pageParam = 1 }) => { const urlParams = qs.stringify({ - tag_ids, + tag_names, limit, include_all, keyword,