diff --git a/api/controllers/console/tag/tags.py b/api/controllers/console/tag/tags.py index f73e2da54e..b9e876c906 100644 --- a/api/controllers/console/tag/tags.py +++ b/api/controllers/console/tag/tags.py @@ -32,12 +32,7 @@ class TagBindingPayload(BaseModel): class TagBindingRemovePayload(BaseModel): - tag_id: str = Field(description="Tag ID to remove") - target_id: str = Field(description="Target ID to unbind tag from") - type: TagType = Field(description="Tag type") - - -class TagBindingItemDeletePayload(BaseModel): + tag_ids: list[str] = Field(description="Tag IDs to remove", min_length=1) target_id: str = Field(description="Target ID to unbind tag from") type: TagType = Field(description="Tag type") @@ -75,7 +70,6 @@ register_schema_models( TagBasePayload, TagBindingPayload, TagBindingRemovePayload, - TagBindingItemDeletePayload, TagListQueryParam, TagResponse, ) @@ -184,13 +178,13 @@ def _create_tag_bindings() -> tuple[dict[str, str], int]: return {"result": "success"}, 200 -def _remove_tag_binding() -> tuple[dict[str, str], int]: +def _remove_tag_bindings() -> tuple[dict[str, str], int]: _require_tag_binding_edit_permission() payload = TagBindingRemovePayload.model_validate(console_ns.payload or {}) TagService.delete_tag_binding( TagBindingDeletePayload( - tag_id=payload.tag_id, + tag_ids=payload.tag_ids, target_id=payload.target_id, type=payload.type, ) @@ -211,54 +205,15 @@ class TagBindingCollectionApi(Resource): return _create_tag_bindings() -@console_ns.route("/tag-bindings/") -class TagBindingItemApi(Resource): - """Canonical item resource for tag binding deletion.""" - - @console_ns.doc("delete_tag_binding") - @console_ns.doc(params={"id": "Tag ID"}) - @console_ns.expect(console_ns.models[TagBindingItemDeletePayload.__name__]) - @setup_required - @login_required - @account_initialization_required - def delete(self, id): - _require_tag_binding_edit_permission() - payload = TagBindingItemDeletePayload.model_validate(console_ns.payload or {}) - TagService.delete_tag_binding( - TagBindingDeletePayload( - tag_id=str(id), - target_id=payload.target_id, - type=payload.type, - ) - ) - return {"result": "success"}, 200 - - -@console_ns.route("/tag-bindings/create") -class DeprecatedTagBindingCreateApi(Resource): - """Deprecated verb-based alias for tag binding creation.""" - - @console_ns.doc("create_tag_binding_deprecated") - @console_ns.doc(deprecated=True) - @console_ns.doc(description="Deprecated legacy alias. Use POST /tag-bindings instead.") - @console_ns.expect(console_ns.models[TagBindingPayload.__name__]) - @setup_required - @login_required - @account_initialization_required - def post(self): - return _create_tag_bindings() - - @console_ns.route("/tag-bindings/remove") -class DeprecatedTagBindingRemoveApi(Resource): - """Deprecated verb-based alias for tag binding deletion.""" +class TagBindingRemoveApi(Resource): + """Batch resource for tag binding deletion.""" - @console_ns.doc("delete_tag_binding_deprecated") - @console_ns.doc(deprecated=True) - @console_ns.doc(description="Deprecated legacy alias. Use DELETE /tag-bindings/{id} instead.") + @console_ns.doc("remove_tag_bindings") + @console_ns.doc(description="Remove one or more tag bindings from a target.") @console_ns.expect(console_ns.models[TagBindingRemovePayload.__name__]) @setup_required @login_required @account_initialization_required def post(self): - return _remove_tag_binding() + return _remove_tag_bindings() diff --git a/api/controllers/service_api/dataset/dataset.py b/api/controllers/service_api/dataset/dataset.py index 76519cad0a..3eb773fa7c 100644 --- a/api/controllers/service_api/dataset/dataset.py +++ b/api/controllers/service_api/dataset/dataset.py @@ -2,7 +2,7 @@ from typing import Any, Literal, cast from flask import request from flask_restx import marshal -from pydantic import BaseModel, Field, TypeAdapter, field_validator +from pydantic import BaseModel, Field, TypeAdapter, field_validator, model_validator from werkzeug.exceptions import Forbidden, NotFound import services @@ -100,9 +100,27 @@ class TagBindingPayload(BaseModel): class TagUnbindingPayload(BaseModel): - tag_id: str + """Accept the legacy single-tag Service API payload while exposing a normalized tag_ids list internally.""" + + tag_ids: list[str] = Field(default_factory=list) + tag_id: str | None = None target_id: str + @model_validator(mode="before") + @classmethod + def normalize_legacy_tag_id(cls, data: object) -> object: + if not isinstance(data, dict): + return data + if not data.get("tag_ids") and data.get("tag_id"): + return {**data, "tag_ids": [data["tag_id"]]} + return data + + @model_validator(mode="after") + def validate_tag_ids(self) -> "TagUnbindingPayload": + if not self.tag_ids: + raise ValueError("Tag IDs is required.") + return self + class DatasetListQuery(BaseModel): page: int = Field(default=1, description="Page number") @@ -601,11 +619,11 @@ class DatasetTagBindingApi(DatasetApiResource): @service_api_ns.route("/datasets/tags/unbinding") class DatasetTagUnbindingApi(DatasetApiResource): @service_api_ns.expect(service_api_ns.models[TagUnbindingPayload.__name__]) - @service_api_ns.doc("unbind_dataset_tag") - @service_api_ns.doc(description="Unbind a tag from a dataset") + @service_api_ns.doc("unbind_dataset_tags") + @service_api_ns.doc(description="Unbind tags from a dataset") @service_api_ns.doc( responses={ - 204: "Tag unbound successfully", + 204: "Tags unbound successfully", 401: "Unauthorized - invalid API token", 403: "Forbidden - insufficient permissions", } @@ -618,7 +636,7 @@ class DatasetTagUnbindingApi(DatasetApiResource): payload = TagUnbindingPayload.model_validate(service_api_ns.payload or {}) TagService.delete_tag_binding( - TagBindingDeletePayload(tag_id=payload.tag_id, target_id=payload.target_id, type=TagType.KNOWLEDGE) + TagBindingDeletePayload(tag_ids=payload.tag_ids, target_id=payload.target_id, type=TagType.KNOWLEDGE) ) return "", 204 diff --git a/api/services/tag_service.py b/api/services/tag_service.py index 1882c855ea..8043a99be1 100644 --- a/api/services/tag_service.py +++ b/api/services/tag_service.py @@ -1,9 +1,11 @@ import uuid +from typing import cast import sqlalchemy as sa from flask_login import current_user from pydantic import BaseModel, Field -from sqlalchemy import func, select +from sqlalchemy import delete, func, select +from sqlalchemy.engine import CursorResult from werkzeug.exceptions import NotFound from extensions.ext_database import db @@ -29,7 +31,7 @@ class TagBindingCreatePayload(BaseModel): class TagBindingDeletePayload(BaseModel): - tag_id: str + tag_ids: list[str] = Field(min_length=1) target_id: str type: TagType @@ -178,13 +180,18 @@ class TagService: @staticmethod def delete_tag_binding(payload: TagBindingDeletePayload): TagService.check_target_exists(payload.type, payload.target_id) - tag_binding = db.session.scalar( - select(TagBinding) - .where(TagBinding.target_id == payload.target_id, TagBinding.tag_id == payload.tag_id) - .limit(1) + result = cast( + CursorResult, + db.session.execute( + delete(TagBinding).where( + TagBinding.target_id == payload.target_id, + TagBinding.tag_id.in_(payload.tag_ids), + TagBinding.tenant_id == current_user.current_tenant_id, + ) + ), ) - if tag_binding: - db.session.delete(tag_binding) + + if result.rowcount: db.session.commit() @staticmethod diff --git a/api/tests/test_containers_integration_tests/controllers/service_api/dataset/test_dataset.py b/api/tests/test_containers_integration_tests/controllers/service_api/dataset/test_dataset.py index 9b913d6d3d..437b199ec2 100644 --- a/api/tests/test_containers_integration_tests/controllers/service_api/dataset/test_dataset.py +++ b/api/tests/test_containers_integration_tests/controllers/service_api/dataset/test_dataset.py @@ -217,10 +217,20 @@ class TestTagUnbindingPayload: """Test suite for TagUnbindingPayload Pydantic model.""" def test_payload_with_valid_data(self): - payload = TagUnbindingPayload(tag_id="tag_123", target_id="dataset_456") - assert payload.tag_id == "tag_123" + payload = TagUnbindingPayload(tag_ids=["tag_123"], target_id="dataset_456") + assert payload.tag_ids == ["tag_123"] assert payload.target_id == "dataset_456" + def test_payload_normalizes_legacy_tag_id(self): + payload = TagUnbindingPayload(tag_id="tag_123", target_id="dataset_456") + assert payload.tag_ids == ["tag_123"] + assert payload.target_id == "dataset_456" + + def test_payload_rejects_empty_tag_ids(self): + with pytest.raises(ValueError) as exc_info: + TagUnbindingPayload(tag_ids=[], target_id="dataset_456") + assert "Tag IDs is required" in str(exc_info.value) + # --------------------------------------------------------------------------- # Helpers @@ -1012,6 +1022,36 @@ class TestDatasetTagUnbindingApiPost: mock_current_user.is_dataset_editor = True mock_tag_svc.delete_tag_binding.return_value = None + with app.test_request_context( + "/datasets/tags/unbinding", + method="POST", + json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, + ): + api = DatasetTagUnbindingApi() + result = api.post(_=None) + + assert result == ("", 204) + from services.tag_service import TagBindingDeletePayload + + mock_tag_svc.delete_tag_binding.assert_called_once_with( + TagBindingDeletePayload(tag_ids=["tag-1"], target_id="ds-1", type="knowledge") + ) + + @patch("controllers.service_api.dataset.dataset.TagService") + @patch("controllers.service_api.dataset.dataset.current_user") + def test_unbind_legacy_tag_id_success( + self, + mock_current_user, + mock_tag_svc, + app, + ): + from controllers.service_api.dataset.dataset import DatasetTagUnbindingApi + + mock_current_user.__class__ = Account + mock_current_user.has_edit_permission = True + mock_current_user.is_dataset_editor = True + mock_tag_svc.delete_tag_binding.return_value = None + with app.test_request_context( "/datasets/tags/unbinding", method="POST", @@ -1024,7 +1064,7 @@ class TestDatasetTagUnbindingApiPost: from services.tag_service import TagBindingDeletePayload mock_tag_svc.delete_tag_binding.assert_called_once_with( - TagBindingDeletePayload(tag_id="tag-1", target_id="ds-1", type="knowledge") + TagBindingDeletePayload(tag_ids=["tag-1"], target_id="ds-1", type="knowledge") ) @patch("controllers.service_api.dataset.dataset.current_user") @@ -1038,7 +1078,7 @@ class TestDatasetTagUnbindingApiPost: with app.test_request_context( "/datasets/tags/unbinding", method="POST", - json={"tag_id": "tag-1", "target_id": "ds-1"}, + json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, ): api = DatasetTagUnbindingApi() with pytest.raises(Forbidden): 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 5a6bf0466e..583b6128e6 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 @@ -1099,38 +1099,39 @@ class TestTagService: db_session_with_containers, mock_external_service_dependencies ) - # Create tag - tag = self._create_test_tags( - db_session_with_containers, mock_external_service_dependencies, tenant.id, "knowledge", 1 - )[0] + # Create tags + tags = self._create_test_tags( + db_session_with_containers, mock_external_service_dependencies, tenant.id, "knowledge", 2 + ) - # Create dataset and bind tag + # Create dataset and bind tags dataset = self._create_test_dataset(db_session_with_containers, mock_external_service_dependencies, tenant.id) self._create_test_tag_bindings( - db_session_with_containers, mock_external_service_dependencies, [tag], dataset.id, tenant.id + db_session_with_containers, mock_external_service_dependencies, tags, dataset.id, tenant.id ) - # Verify binding exists before deletion - - binding_before = ( + # Verify bindings exist before deletion + bindings_before = ( db_session_with_containers.query(TagBinding) - .where(TagBinding.tag_id == tag.id, TagBinding.target_id == dataset.id) - .first() + .where(TagBinding.tag_id.in_([tag.id for tag in tags]), TagBinding.target_id == dataset.id) + .all() ) - assert binding_before is not None + assert len(bindings_before) == 2 # Act: Execute the method under test - delete_payload = TagBindingDeletePayload(type="knowledge", target_id=dataset.id, tag_id=tag.id) + delete_payload = TagBindingDeletePayload( + type="knowledge", target_id=dataset.id, tag_ids=[tag.id for tag in tags] + ) TagService.delete_tag_binding(delete_payload) # Assert: Verify the expected outcomes - # Verify tag binding was deleted - binding_after = ( + # Verify tag bindings were deleted + bindings_after = ( db_session_with_containers.query(TagBinding) - .where(TagBinding.tag_id == tag.id, TagBinding.target_id == dataset.id) - .first() + .where(TagBinding.tag_id.in_([tag.id for tag in tags]), TagBinding.target_id == dataset.id) + .all() ) - assert binding_after is None + assert len(bindings_after) == 0 def test_delete_tag_binding_non_existent_binding( self, db_session_with_containers: Session, mock_external_service_dependencies @@ -1156,7 +1157,7 @@ class TestTagService: app = self._create_test_app(db_session_with_containers, mock_external_service_dependencies, tenant.id) # Act: Try to delete non-existent binding - delete_payload = TagBindingDeletePayload(type="app", target_id=app.id, tag_id=tag.id) + delete_payload = TagBindingDeletePayload(type="app", target_id=app.id, tag_ids=[tag.id]) TagService.delete_tag_binding(delete_payload) # Assert: Verify the expected outcomes diff --git a/api/tests/unit_tests/controllers/console/tag/test_tags.py b/api/tests/unit_tests/controllers/console/tag/test_tags.py index 6405558bb4..a26d171649 100644 --- a/api/tests/unit_tests/controllers/console/tag/test_tags.py +++ b/api/tests/unit_tests/controllers/console/tag/test_tags.py @@ -8,10 +8,8 @@ from werkzeug.exceptions import Forbidden import controllers.console.tag.tags as module from controllers.console import console_ns from controllers.console.tag.tags import ( - DeprecatedTagBindingCreateApi, - DeprecatedTagBindingRemoveApi, TagBindingCollectionApi, - TagBindingItemApi, + TagBindingRemoveApi, TagListApi, TagUpdateDeleteApi, ) @@ -249,39 +247,13 @@ class TestTagBindingCollectionApi: method(api) -class TestDeprecatedTagBindingCreateApi: - def test_create_success(self, app, admin_user, payload_patch): - api = DeprecatedTagBindingCreateApi() +class TestTagBindingRemoveApi: + def test_remove_success(self, app, admin_user, payload_patch): + api = TagBindingRemoveApi() method = unwrap(api.post) payload = { - "tag_ids": ["tag-1"], - "target_id": "target-1", - "type": "knowledge", - } - - with app.test_request_context("/", json=payload): - with ( - patch( - "controllers.console.tag.tags.current_account_with_tenant", - return_value=(admin_user, None), - ), - payload_patch(payload), - patch("controllers.console.tag.tags.TagService.save_tag_binding") as save_mock, - ): - result, status = method(api) - - save_mock.assert_called_once() - assert status == 200 - assert result["result"] == "success" - - -class TestTagBindingItemApi: - def test_delete_success(self, app, admin_user, payload_patch): - api = TagBindingItemApi() - method = unwrap(api.delete) - - payload = { + "tag_ids": ["tag-1", "tag-2"], "target_id": "target-1", "type": "knowledge", } @@ -295,57 +267,16 @@ class TestTagBindingItemApi: payload_patch(payload), patch("controllers.console.tag.tags.TagService.delete_tag_binding") as delete_mock, ): - result, status = method(api, "tag-1") + result, status = method(api) delete_mock.assert_called_once() delete_payload = delete_mock.call_args.args[0] - assert delete_payload.tag_id == "tag-1" - assert delete_payload.target_id == "target-1" - assert delete_payload.type == TagType.KNOWLEDGE - assert status == 200 - assert result["result"] == "success" - - def test_delete_forbidden(self, app, readonly_user): - api = TagBindingItemApi() - method = unwrap(api.delete) - - with app.test_request_context("/"): - with patch( - "controllers.console.tag.tags.current_account_with_tenant", - return_value=(readonly_user, None), - ): - with pytest.raises(Forbidden): - method(api, "tag-1") - - -class TestDeprecatedTagBindingRemoveApi: - def test_remove_success(self, app, admin_user, payload_patch): - api = DeprecatedTagBindingRemoveApi() - method = unwrap(api.post) - - payload = { - "tag_id": "tag-1", - "target_id": "target-1", - "type": "knowledge", - } - - with app.test_request_context("/", json=payload): - with ( - patch( - "controllers.console.tag.tags.current_account_with_tenant", - return_value=(admin_user, None), - ), - payload_patch(payload), - patch("controllers.console.tag.tags.TagService.delete_tag_binding") as delete_mock, - ): - result, status = method(api) - - delete_mock.assert_called_once() + assert delete_payload.tag_ids == ["tag-1", "tag-2"] assert status == 200 assert result["result"] == "success" def test_remove_forbidden(self, app, readonly_user, payload_patch): - api = DeprecatedTagBindingRemoveApi() + api = TagBindingRemoveApi() method = unwrap(api.post) with app.test_request_context("/", json={}): @@ -371,32 +302,30 @@ class TestTagResponseModel: class TestTagBindingRouteMetadata: - def test_legacy_write_routes_are_marked_deprecated(self): - assert DeprecatedTagBindingCreateApi.post.__apidoc__["deprecated"] is True - assert DeprecatedTagBindingRemoveApi.post.__apidoc__["deprecated"] is True + def test_write_routes_are_not_deprecated(self): assert TagBindingCollectionApi.post.__apidoc__.get("deprecated") is not True - assert TagBindingItemApi.delete.__apidoc__.get("deprecated") is not True + assert TagBindingRemoveApi.post.__apidoc__.get("deprecated") is not True def test_write_routes_have_stable_operation_ids(self): assert TagBindingCollectionApi.post.__apidoc__["id"] == "create_tag_binding" - assert TagBindingItemApi.delete.__apidoc__["id"] == "delete_tag_binding" - assert DeprecatedTagBindingCreateApi.post.__apidoc__["id"] == "create_tag_binding_deprecated" - assert DeprecatedTagBindingRemoveApi.post.__apidoc__["id"] == "delete_tag_binding_deprecated" + assert TagBindingRemoveApi.post.__apidoc__["id"] == "remove_tag_bindings" - def test_canonical_and_legacy_write_routes_are_registered(self): + def test_write_routes_are_registered(self): route_map = { resource.__name__: urls for resource, urls, _route_doc, _kwargs in console_ns.resources if resource.__name__ in { "TagBindingCollectionApi", - "TagBindingItemApi", - "DeprecatedTagBindingCreateApi", - "DeprecatedTagBindingRemoveApi", + "TagBindingRemoveApi", } } assert route_map["TagBindingCollectionApi"] == ("/tag-bindings",) - assert route_map["TagBindingItemApi"] == ("/tag-bindings/",) - assert route_map["DeprecatedTagBindingCreateApi"] == ("/tag-bindings/create",) - assert route_map["DeprecatedTagBindingRemoveApi"] == ("/tag-bindings/remove",) + assert route_map["TagBindingRemoveApi"] == ("/tag-bindings/remove",) + + def test_legacy_write_routes_are_not_registered(self): + urls = {url for _resource, resource_urls, _route_doc, _kwargs in console_ns.resources for url in resource_urls} + + assert "/tag-bindings/create" not in urls + assert "/tag-bindings/" not in urls diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 2d099669d1..2147bb95e8 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -155,9 +155,6 @@ } }, "web/app/account/(commonLayout)/account-page/email-change-modal.tsx": { - "erasable-syntax-only/enums": { - "count": 1 - }, "ts/no-explicit-any": { "count": 5 } @@ -1824,26 +1821,6 @@ "count": 1 } }, - "web/app/components/base/tag-management/__tests__/panel.spec.tsx": { - "ts/no-explicit-any": { - "count": 2 - } - }, - "web/app/components/base/tag-management/index.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, - "web/app/components/base/tag-management/tag-item-editor.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, - "web/app/components/base/tag-management/tag-remove-modal.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/base/text-generation/hooks.ts": { "ts/no-explicit-any": { "count": 1 @@ -2354,11 +2331,6 @@ "count": 1 } }, - "web/app/components/datasets/list/dataset-card/hooks/use-dataset-card-state.ts": { - "react/set-state-in-effect": { - "count": 1 - } - }, "web/app/components/datasets/metadata/edit-metadata-batch/input-combined.tsx": { "ts/no-explicit-any": { "count": 2 @@ -2464,17 +2436,6 @@ "count": 2 } }, - "web/app/components/explore/create-app-modal/index.tsx": { - "no-restricted-imports": { - "count": 1 - }, - "ts/no-explicit-any": { - "count": 1 - }, - "unicorn/prefer-number-properties": { - "count": 1 - } - }, "web/app/components/explore/item-operation/index.tsx": { "react/set-state-in-effect": { "count": 1 @@ -5368,11 +5329,6 @@ "count": 2 } }, - "web/service/knowledge/use-dataset.ts": { - "@tanstack/query/exhaustive-deps": { - "count": 1 - } - }, "web/service/share.ts": { "erasable-syntax-only/enums": { "count": 1 diff --git a/web/.storybook/main.ts b/web/.storybook/main.ts index 918860c786..e5bf0ee65e 100644 --- a/web/.storybook/main.ts +++ b/web/.storybook/main.ts @@ -1,7 +1,10 @@ import type { StorybookConfig } from '@storybook/nextjs-vite' const config: StorybookConfig = { - stories: ['../app/components/**/*.stories.@(js|jsx|mjs|ts|tsx)'], + stories: [ + '../app/components/**/*.stories.@(js|jsx|mjs|ts|tsx)', + '../features/**/*.stories.@(js|jsx|mjs|ts|tsx)', + ], addons: [ // Not working with Storybook Vite framework // '@storybook/addon-onboarding', diff --git a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/layout-main.tsx b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/layout-main.tsx index 8a1a6fd131..46d7f7833e 100644 --- a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/layout-main.tsx +++ b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/layout-main.tsx @@ -21,20 +21,14 @@ import { useShallow } from 'zustand/react/shallow' import AppSideBar from '@/app/components/app-sidebar' import { useStore } from '@/app/components/app/store' import Loading from '@/app/components/base/loading' -import { useStore as useTagStore } from '@/app/components/base/tag-management/store' import { useAppContext } from '@/context/app-context' import useBreakpoints, { MediaType } from '@/hooks/use-breakpoints' import useDocumentTitle from '@/hooks/use-document-title' -import dynamic from '@/next/dynamic' import { usePathname, useRouter } from '@/next/navigation' import { fetchAppDetailDirect } from '@/service/apps' import { AppModeEnum } from '@/types/app' import s from './style.module.css' -const TagManagementModal = dynamic(() => import('@/app/components/base/tag-management'), { - ssr: false, -}) - type IAppDetailLayoutProps = { children: React.ReactNode appId: string @@ -56,7 +50,6 @@ const AppDetailLayout: FC = (props) => { setAppDetail: state.setAppDetail, setAppSidebarExpand: state.setAppSidebarExpand, }))) - const showTagManagementModal = useTagStore(s => s.showTagManagementModal) const [isLoadingAppDetail, setIsLoadingAppDetail] = useState(false) const [appDetailRes, setAppDetailRes] = useState(null) const [navigation, setNavigation] = useState = (props) => {
{children}
- {showTagManagementModal && ( - - )} ) } diff --git a/web/app/account/(commonLayout)/account-page/email-change-modal.tsx b/web/app/account/(commonLayout)/account-page/email-change-modal.tsx index f3bb71c2d2..83bca6a8cb 100644 --- a/web/app/account/(commonLayout)/account-page/email-change-modal.tsx +++ b/web/app/account/(commonLayout)/account-page/email-change-modal.tsx @@ -3,8 +3,7 @@ import { Button } from '@langgenius/dify-ui/button' import { Dialog, DialogContent } from '@langgenius/dify-ui/dialog' import { toast } from '@langgenius/dify-ui/toast' import { RiCloseLine } from '@remixicon/react' -import * as React from 'react' -import { useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { Trans, useTranslation } from 'react-i18next' import Input from '@/app/components/base/input' import { useRouter } from '@/next/navigation' @@ -18,22 +17,23 @@ import { useLogout } from '@/service/use-common' import { asyncRunSafe } from '@/utils' type Props = { - show: boolean onClose: () => void email: string } -enum STEP { - start = 'start', - verifyOrigin = 'verifyOrigin', - newEmail = 'newEmail', - verifyNew = 'verifyNew', -} +const STEP = { + start: 'start', + verifyOrigin: 'verifyOrigin', + newEmail: 'newEmail', + verifyNew: 'verifyNew', +} as const -const EmailChangeModal = ({ onClose, email, show }: Props) => { +type Step = typeof STEP[keyof typeof STEP] + +const EmailChangeModal = ({ onClose, email }: Props) => { const { t } = useTranslation() const router = useRouter() - const [step, setStep] = useState(STEP.start) + const [step, setStep] = useState(STEP.start) const [code, setCode] = useState('') const [mail, setMail] = useState('') const [time, setTime] = useState(0) @@ -41,13 +41,25 @@ const EmailChangeModal = ({ onClose, email, show }: Props) => { const [newEmailExited, setNewEmailExited] = useState(false) const [unAvailableEmail, setUnAvailableEmail] = useState(false) const [isCheckingEmail, setIsCheckingEmail] = useState(false) + const timerRef = useRef | null>(null) + + const clearCountdown = useCallback(() => { + if (!timerRef.current) + return + + clearInterval(timerRef.current) + timerRef.current = null + }, []) + + useEffect(() => clearCountdown, [clearCountdown]) const startCount = () => { + clearCountdown() setTime(60) - const timer = setInterval(() => { + timerRef.current = setInterval(() => { setTime((prev) => { - if (prev <= 0) { - clearInterval(timer) + if (prev <= 1) { + clearCountdown() return 0 } return prev - 1 @@ -181,7 +193,7 @@ const EmailChangeModal = ({ onClose, email, show }: Props) => { } return ( - !open && onClose()}> + !open && onClose()}>
diff --git a/web/app/account/(commonLayout)/account-page/index.tsx b/web/app/account/(commonLayout)/account-page/index.tsx index 2a4ae86f84..0de33a2a71 100644 --- a/web/app/account/(commonLayout)/account-page/index.tsx +++ b/web/app/account/(commonLayout)/account-page/index.tsx @@ -332,11 +332,15 @@ export default function AccountPage() { /> ) } - setShowUpdateEmail(false)} - email={userProfile.email} - /> + {/* Use conditional JSX instead of a mounted controlled Dialog so closing destroys the email-change form session. */} + {showUpdateEmail + ? ( + setShowUpdateEmail(false)} + email={userProfile.email} + /> + ) + : null} ) } diff --git a/web/app/components/apps/__tests__/app-card.spec.tsx b/web/app/components/apps/__tests__/app-card.spec.tsx index 6a71dbac52..4edf5604da 100644 --- a/web/app/components/apps/__tests__/app-card.spec.tsx +++ b/web/app/components/apps/__tests__/app-card.spec.tsx @@ -301,9 +301,9 @@ vi.mock('@/app/components/base/tooltip', () => ({ default: ({ children, popupContent }: { children: React.ReactNode, popupContent: React.ReactNode }) => React.createElement('div', { title: popupContent }, children), })) -// TagSelector has API dependency (service/tag) - mock for isolated testing -vi.mock('@/app/components/base/tag-management/selector', () => ({ - default: ({ tags }: { tags?: { id: string, name: string }[] }) => { +// AppCardTags has tag API dependencies - mock for isolated testing +vi.mock('@/features/tag-management/components/app-card-tags', () => ({ + AppCardTags: ({ tags }: { tags?: { id: string, name: string }[] }) => { return React.createElement('div', { 'aria-label': 'tag-selector' }, tags?.map((tag: { id: string, name: string }) => React.createElement('span', { key: tag.id }, tag.name))) }, })) @@ -400,13 +400,30 @@ describe('AppCard', () => { it('should handle app with tags', () => { const appWithTags = { ...mockApp, - tags: [{ id: 'tag1', name: 'Tag 1', type: 'app', binding_count: 0 }], + tags: [{ id: 'tag1', name: 'Tag 1', type: 'app' as const, binding_count: 0 }], } render() // Verify the tag selector component renders expect(screen.getByLabelText('tag-selector')).toBeInTheDocument() }) + it('should display refreshed tag names from app props when tag ids stay the same', () => { + const firstApp = createMockApp({ + tags: [{ id: 'tag1', name: 'Old Tag', type: 'app' as const, binding_count: 0 }], + }) + const refreshedApp = createMockApp({ + tags: [{ id: 'tag1', name: 'New Tag', type: 'app' as const, binding_count: 0 }], + }) + + const { rerender } = render() + expect(screen.getByText('Old Tag')).toBeInTheDocument() + + rerender() + + expect(screen.getByText('New Tag')).toBeInTheDocument() + expect(screen.queryByText('Old Tag')).not.toBeInTheDocument() + }) + it('should render with onRefresh callback', () => { render() expect(screen.getByTitle('Test App')).toBeInTheDocument() @@ -1167,9 +1184,9 @@ describe('AppCard', () => { const multiTagApp = { ...mockApp, tags: [ - { id: 'tag1', name: 'Tag 1', type: 'app', binding_count: 0 }, - { id: 'tag2', name: 'Tag 2', type: 'app', binding_count: 0 }, - { id: 'tag3', name: 'Tag 3', type: 'app', binding_count: 0 }, + { id: 'tag1', name: 'Tag 1', type: 'app' as const, binding_count: 0 }, + { id: 'tag2', name: 'Tag 2', type: 'app' as const, binding_count: 0 }, + { id: 'tag3', name: 'Tag 3', type: 'app' as const, binding_count: 0 }, ], } render() @@ -1324,7 +1341,7 @@ describe('AppCard', () => { it('should stop propagation when clicking tag selector area', () => { const multiTagApp = createMockApp({ - tags: [{ id: 'tag1', name: 'Tag 1', type: 'app', binding_count: 0 }], + tags: [{ id: 'tag1', name: 'Tag 1', type: 'app' as const, binding_count: 0 }], }) render() diff --git a/web/app/components/apps/__tests__/list.spec.tsx b/web/app/components/apps/__tests__/list.spec.tsx index 9d1b39ef06..41d2ccbc80 100644 --- a/web/app/components/apps/__tests__/list.spec.tsx +++ b/web/app/components/apps/__tests__/list.spec.tsx @@ -1,7 +1,6 @@ import { act, fireEvent, screen } from '@testing-library/react' import * as React from 'react' import { createSystemFeaturesWrapper } from '@/__tests__/utils/mock-system-features' -import { useStore as useTagStore } from '@/app/components/base/tag-management/store' import { renderWithNuqs } from '@/test/nuqs-testing' import { AppModeEnum } from '@/types/app' @@ -29,6 +28,11 @@ vi.mock('@/service/client', () => ({ infiniteOptions: (options: unknown) => mockAppListInfiniteOptions(options), }, }, + tags: { + list: { + queryOptions: (options: unknown) => options, + }, + }, systemFeatures: { queryKey: () => ['console', 'systemFeatures'], }, @@ -139,10 +143,6 @@ vi.mock('@/service/use-apps', () => ({ }), })) -vi.mock('@/service/tag', () => ({ - fetchTagList: vi.fn().mockResolvedValue([{ id: 'tag-1', name: 'Test Tag', type: 'app' }]), -})) - vi.mock('@/config', async (importOriginal) => { const actual = await importOriginal() return { @@ -236,10 +236,6 @@ type AppListInfiniteOptions = { describe('List', () => { beforeEach(() => { vi.clearAllMocks() - useTagStore.setState({ - tagList: [{ id: 'tag-1', name: 'Test Tag', type: 'app', binding_count: 0 }], - showTagManagementModal: false, - }) mockIsCurrentWorkspaceEditor.mockReturnValue(true) mockIsCurrentWorkspaceDatasetOperator.mockReturnValue(false) mockDragging = false diff --git a/web/app/components/apps/app-card.tsx b/web/app/components/apps/app-card.tsx index 458c7578c7..06c5c8a9d8 100644 --- a/web/app/components/apps/app-card.tsx +++ b/web/app/components/apps/app-card.tsx @@ -1,7 +1,6 @@ 'use client' import type { DuplicateAppModalProps } from '@/app/components/app/duplicate-modal' -import type { Tag } from '@/app/components/base/tag-management/constant' import type { CreateAppModalProps } from '@/app/components/explore/create-app-modal' import type { EnvironmentVariable } from '@/app/components/workflow/types' import type { WorkflowOnlineUser } from '@/models/app' @@ -36,11 +35,11 @@ import { Trans, useTranslation } from 'react-i18next' import { AppTypeIcon } from '@/app/components/app/type-selector' import AppIcon from '@/app/components/base/app-icon' import Input from '@/app/components/base/input' -import TagSelector from '@/app/components/base/tag-management/selector' import { UserAvatarList } from '@/app/components/base/user-avatar-list' import { NEED_REFRESH_APP_LIST_KEY } from '@/config' import { useAppContext } from '@/context/app-context' import { useProviderContext } from '@/context/provider-context' +import { AppCardTags } from '@/features/tag-management/components/app-card-tags' import { useAsyncWindowOpen } from '@/hooks/use-async-window-open' import { AccessMode } from '@/models/access-control' import dynamic from '@/next/dynamic' @@ -77,6 +76,7 @@ type AppCardProps = { app: App onlineUsers?: WorkflowOnlineUser[] onRefresh?: () => void + onOpenTagManagement?: () => void } type AppCardOperationsMenuProps = { @@ -207,7 +207,7 @@ const AppCardOperationsMenuContent: React.FC ) } -const AppCard = ({ app, onlineUsers = [], onRefresh }: AppCardProps) => { +const AppCard = ({ app, onlineUsers = [], onRefresh, onOpenTagManagement = () => {} }: AppCardProps) => { const { t } = useTranslation() const deleteAppNameInputId = useId() const { data: systemFeatures } = useSuspenseQuery(systemFeaturesQueryOptions()) @@ -396,19 +396,6 @@ const AppCard = ({ app, onlineUsers = [], onRefresh }: AppCardProps) => { const shouldShowAccessControlOption = systemFeatures.webapp_auth.enabled && isCurrentWorkspaceEditor const operationsMenuWidthClassName = shouldShowSwitchOption ? 'w-[256px]' : 'w-[216px]' - const appTagsKey = useMemo(() => app.tags.map(tag => tag.id).join(','), [app.tags]) - const [tagState, setTagState] = useState<{ key: string, tags: Tag[] }>(() => ({ - key: appTagsKey, - tags: app.tags, - })) - const tags = tagState.key === appTagsKey ? tagState.tags : app.tags - const handleTagsUpdate = useCallback((nextTags: Tag[]) => { - setTagState({ - key: appTagsKey, - tags: nextTags, - }) - }, [appTagsKey]) - const EditTimeText = useMemo(() => { const timeText = formatTime({ date: (app.updated_at || app.created_at) * 1000, @@ -523,15 +510,12 @@ const AppCard = ({ app, onlineUsers = [], onRefresh }: AppCardProps) => { e.preventDefault() }} > -
- tag.id)} - selectedTags={tags} - onCacheUpdate={handleTagsUpdate} - onChange={onRefresh} +
+
diff --git a/web/app/components/apps/list.tsx b/web/app/components/apps/list.tsx index 728ef38ba5..0fd31dfb79 100644 --- a/web/app/components/apps/list.tsx +++ b/web/app/components/apps/list.tsx @@ -11,10 +11,9 @@ import { useTranslation } from 'react-i18next' import Checkbox from '@/app/components/base/checkbox' import Input from '@/app/components/base/input' import TabSliderNew from '@/app/components/base/tab-slider-new' -import TagFilter from '@/app/components/base/tag-management/filter' -import { useStore as useTagStore } from '@/app/components/base/tag-management/store' import { NEED_REFRESH_APP_LIST_KEY } from '@/config' import { useAppContext } from '@/context/app-context' +import { TagFilter } from '@/features/tag-management/components/tag-filter' import { CheckModal } from '@/hooks/use-pay' import dynamic from '@/next/dynamic' import { consoleQuery } from '@/service/client' @@ -24,12 +23,12 @@ import AppCard from './app-card' import { AppCardSkeleton } from './app-card-skeleton' import Empty from './empty' import Footer from './footer' -import useAppsQueryState from './hooks/use-apps-query-state' +import useAppsQueryStateHook from './hooks/use-apps-query-state' import { useDSLDragDrop } from './hooks/use-dsl-drag-drop' import { useWorkflowOnlineUsers } from './hooks/use-workflow-online-users' import NewAppCard from './new-app-card' -const TagManagementModal = dynamic(() => import('@/app/components/base/tag-management'), { +const TagManagementModal = dynamic(() => import('@/features/tag-management/components/tag-management-modal').then(mod => mod.TagManagementModal), { ssr: false, }) const CreateFromDSLModal = dynamic(() => import('@/app/components/app/create-from-dsl-modal'), { @@ -57,18 +56,20 @@ const List: FC = ({ const { t } = useTranslation() const { data: systemFeatures } = useSuspenseQuery(systemFeaturesQueryOptions()) const { isCurrentWorkspaceEditor, isCurrentWorkspaceDatasetOperator, isLoadingCurrentWorkspace } = useAppContext() - const showTagManagementModal = useTagStore(s => s.showTagManagementModal) const [activeTab, setActiveTab] = useQueryState( 'category', parseAsAppListCategory, ) - const { query: { tagIDs = [], keywords = '', isCreatedByMe: queryIsCreatedByMe = false }, setQuery } = useAppsQueryState() + // eslint-disable-next-line react/use-state -- custom URL query hook, not React.useState + const appsQuery = useAppsQueryStateHook() + const { query: { tagIDs = [], keywords = '', isCreatedByMe: queryIsCreatedByMe = false }, setQuery } = appsQuery const [isCreatedByMe, setIsCreatedByMe] = useState(queryIsCreatedByMe) const [tagFilterValue, setTagFilterValue] = useState(tagIDs) const [searchKeywords, setSearchKeywords] = useState(keywords) const newAppCardRef = useRef(null) const containerRef = useRef(null) + const [showTagManagementModal, setShowTagManagementModal] = useState(false) const [showCreateFromDSLModal, setShowCreateFromDSLModal] = useState(false) const [droppedDSLFile, setDroppedDSLFile] = useState() const setKeywords = useCallback((keywords: string) => { @@ -245,7 +246,7 @@ const List: FC = ({ {t('showMyCreatedAppsOnly', { ns: 'app' })}
- + setShowTagManagementModal(true)} /> = ({ app={app} onlineUsers={workflowOnlineUsersMap[app.id] ?? []} onRefresh={refetch} + onOpenTagManagement={() => setShowTagManagementModal(true)} /> )) : } @@ -302,9 +304,12 @@ const List: FC = ({ )}
- {showTagManagementModal && ( - - )} + setShowTagManagementModal(false)} + onTagsChange={refetch} + /> {showCreateFromDSLModal && ( diff --git a/web/app/components/base/tag-management/__tests__/tag-remove-modal.spec.tsx b/web/app/components/base/tag-management/__tests__/tag-remove-modal.spec.tsx deleted file mode 100644 index 943b7bc8ff..0000000000 --- a/web/app/components/base/tag-management/__tests__/tag-remove-modal.spec.tsx +++ /dev/null @@ -1,123 +0,0 @@ -import type { Tag } from '../constant' -import { render, screen } from '@testing-library/react' -import userEvent from '@testing-library/user-event' -import TagRemoveModal from '../tag-remove-modal' - -const mockTag: Tag = { - id: 'tag-1', - name: 'Frontend', - type: 'app', - binding_count: 3, -} - -describe('TagRemoveModal', () => { - beforeEach(() => { - vi.clearAllMocks() - }) - - // Rendering behavior and visibility control. - describe('Rendering', () => { - it('should render modal content when show is true', () => { - render( - , - ) - - expect(screen.getByText('common.tag.delete')).toBeInTheDocument() - expect(screen.getByText('"Frontend"')).toBeInTheDocument() - expect(screen.getByText('common.tag.deleteTip')).toBeInTheDocument() - expect(screen.getByText('common.operation.cancel')).toBeInTheDocument() - expect(screen.getByText('common.operation.delete')).toBeInTheDocument() - }) - - it('should not render modal content when show is false', () => { - render( - , - ) - - expect(screen.queryByText('common.tag.delete')).not.toBeInTheDocument() - expect(screen.queryByText('common.tag.deleteTip')).not.toBeInTheDocument() - }) - }) - - // User interactions for closing and confirming actions. - describe('User Interactions', () => { - it('should call onClose when top-right close icon is clicked', async () => { - const user = userEvent.setup() - const onClose = vi.fn() - render( - , - ) - - const closeIconButton = screen.getByTestId('tag-remove-modal-close-button') - expect(closeIconButton).toBeInTheDocument() - await user.click(closeIconButton) - - expect(onClose).toHaveBeenCalledTimes(1) - }) - - it('should call onClose when cancel button is clicked', async () => { - const user = userEvent.setup() - const onClose = vi.fn() - - render( - , - ) - - await user.click(screen.getByText('common.operation.cancel')) - expect(onClose).toHaveBeenCalledTimes(1) - }) - - it('should call onConfirm when delete button is clicked', async () => { - const user = userEvent.setup() - const onConfirm = vi.fn() - - render( - , - ) - - await user.click(screen.getByText('common.operation.delete')) - expect(onConfirm).toHaveBeenCalledTimes(1) - }) - }) - - // Edge case for unusual tag names in the title. - describe('Edge Cases', () => { - it('should render quoted empty tag name safely', () => { - render( - , - ) - - expect(screen.getByText('""')).toBeInTheDocument() - }) - }) -}) diff --git a/web/app/components/base/tag-management/constant.ts b/web/app/components/base/tag-management/constant.ts deleted file mode 100644 index 3c60041383..0000000000 --- a/web/app/components/base/tag-management/constant.ts +++ /dev/null @@ -1,6 +0,0 @@ -export type Tag = { - id: string - name: string - type: string - binding_count: number -} diff --git a/web/app/components/base/tag-management/index.tsx b/web/app/components/base/tag-management/index.tsx deleted file mode 100644 index 19fbfcb7c9..0000000000 --- a/web/app/components/base/tag-management/index.tsx +++ /dev/null @@ -1,62 +0,0 @@ -'use client' -import { toast } from '@langgenius/dify-ui/toast' -import { useEffect, useState } from 'react' -import { useTranslation } from 'react-i18next' -import Modal from '@/app/components/base/modal' -import { createTag, fetchTagList } from '@/service/tag' -import { useStore as useTagStore } from './store' -import TagItemEditor from './tag-item-editor' - -type TagManagementModalProps = { - type: 'knowledge' | 'app' - show: boolean -} -const TagManagementModal = ({ show, type }: TagManagementModalProps) => { - const { t } = useTranslation() - const tagList = useTagStore(s => s.tagList) - const setTagList = useTagStore(s => s.setTagList) - const setShowTagManagementModal = useTagStore(s => s.setShowTagManagementModal) - const getTagList = async (type: 'knowledge' | 'app') => { - const res = await fetchTagList(type) - setTagList(res) - } - const [pending, setPending] = useState(false) - const [name, setName] = useState('') - const createNewTag = async () => { - if (!name) - return - if (pending) - return - try { - setPending(true) - const newTag = await createTag(name, type) - toast.success(t('tag.created', { ns: 'common' })) - setTagList([ - newTag, - ...tagList, - ]) - setName('') - setPending(false) - } - catch { - toast.error(t('tag.failed', { ns: 'common' })) - setPending(false) - } - } - useEffect(() => { - getTagList(type) - }, [type]) - return ( - setShowTagManagementModal(false)}> -
{t('tag.manageTags', { ns: 'common' })}
-
setShowTagManagementModal(false)}> - -
-
- setName(e.target.value)} onKeyDown={e => e.key === 'Enter' && !e.nativeEvent.isComposing && createNewTag()} onBlur={createNewTag} /> - {tagList.map(tag => ())} -
- - ) -} -export default TagManagementModal diff --git a/web/app/components/base/tag-management/selector.tsx b/web/app/components/base/tag-management/selector.tsx deleted file mode 100644 index 0eb233ba4b..0000000000 --- a/web/app/components/base/tag-management/selector.tsx +++ /dev/null @@ -1,116 +0,0 @@ -import type { FC } from 'react' -import type { Tag } from '@/app/components/base/tag-management/constant' -import { cn } from '@langgenius/dify-ui/cn' -import { - Popover, - PopoverContent, - PopoverTrigger, -} from '@langgenius/dify-ui/popover' -import { useCallback, useMemo, useState } from 'react' -import { useTranslation } from 'react-i18next' -import { fetchTagList } from '@/service/tag' -import Panel from './panel' -import { useStore as useTagStore } from './store' -import Trigger from './trigger' - -export type TagSelectorProps = { - targetID: string - isPopover?: boolean - position?: 'bl' | 'br' - type: 'knowledge' | 'app' - value: string[] - selectedTags: Tag[] - onCacheUpdate: (tags: Tag[]) => void - onChange?: () => void - minWidth?: number | string -} - -const TagSelector: FC = ({ - targetID, - isPopover = true, - position, - type, - value, - selectedTags, - onCacheUpdate, - onChange, - minWidth, -}) => { - const { t } = useTranslation() - const tagList = useTagStore(s => s.tagList) - const setTagList = useTagStore(s => s.setTagList) - const [open, setOpen] = useState(false) - - const getTagList = useCallback(async () => { - const res = await fetchTagList(type) - setTagList(res) - }, [setTagList, type]) - - const tags = useMemo(() => { - if (selectedTags?.length) - return selectedTags.filter(selectedTag => tagList.find(tag => tag.id === selectedTag.id)).map(tag => tag.name) - return [] - }, [selectedTags, tagList]) - - const placement = useMemo(() => { - if (position === 'bl') - return 'bottom-start' as const - if (position === 'br') - return 'bottom-end' as const - return 'bottom' as const - }, [position]) - - const resolvedMinWidth = useMemo(() => { - if (minWidth == null) - return undefined - - return typeof minWidth === 'number' ? `${minWidth}px` : minWidth - }, [minWidth]) - - const triggerLabel = useMemo(() => { - if (tags.length) - return tags.join(', ') - - return t('tag.addTag', { ns: 'common' }) - }, [tags, t]) - - if (!isPopover) - return null - - return ( - - - - - - - - - ) -} - -export default TagSelector diff --git a/web/app/components/base/tag-management/store.ts b/web/app/components/base/tag-management/store.ts deleted file mode 100644 index 197d31ed7a..0000000000 --- a/web/app/components/base/tag-management/store.ts +++ /dev/null @@ -1,19 +0,0 @@ -import type { Tag } from './constant' -import { create } from 'zustand' - -type State = { - tagList: Tag[] - showTagManagementModal: boolean -} - -type Action = { - setTagList: (tagList?: Tag[]) => void - setShowTagManagementModal: (showTagManagementModal: boolean) => void -} - -export const useStore = create(set => ({ - tagList: [], - setTagList: tagList => set(() => ({ tagList })), - showTagManagementModal: false, - setShowTagManagementModal: showTagManagementModal => set(() => ({ showTagManagementModal })), -})) diff --git a/web/app/components/base/tag-management/tag-remove-modal.tsx b/web/app/components/base/tag-management/tag-remove-modal.tsx deleted file mode 100644 index 1088ca2043..0000000000 --- a/web/app/components/base/tag-management/tag-remove-modal.tsx +++ /dev/null @@ -1,48 +0,0 @@ -'use client' - -import type { Tag } from '@/app/components/base/tag-management/constant' -import { Button } from '@langgenius/dify-ui/button' -import { cn } from '@langgenius/dify-ui/cn' -import { noop } from 'es-toolkit/function' -import { useTranslation } from 'react-i18next' -import { AlertTriangle } from '@/app/components/base/icons/src/vender/solid/alertsAndFeedback' -import Modal from '@/app/components/base/modal' - -type TagRemoveModalProps = { - show: boolean - tag: Tag - onConfirm: () => void - onClose: () => void -} - -const TagRemoveModal = ({ show, tag, onConfirm, onClose }: TagRemoveModalProps) => { - const { t } = useTranslation() - - return ( - -
- -
-
- -
-
- {`${t('tag.delete', { ns: 'common' })} `} - {`"${tag.name}"`} -
-
- {t('tag.deleteTip', { ns: 'common' })} -
-
- - -
- - ) -} - -export default TagRemoveModal diff --git a/web/app/components/datasets/list/__tests__/datasets.spec.tsx b/web/app/components/datasets/list/__tests__/datasets.spec.tsx index 5b777e0b2e..f78622a9cd 100644 --- a/web/app/components/datasets/list/__tests__/datasets.spec.tsx +++ b/web/app/components/datasets/list/__tests__/datasets.spec.tsx @@ -56,8 +56,6 @@ vi.mock('@/context/app-context', () => ({ // Mock useDatasetCardState hook vi.mock('../dataset-card/hooks/use-dataset-card-state', () => ({ useDatasetCardState: () => ({ - tags: [], - setTags: vi.fn(), modalState: { showRenameModal: false, showConfirmDelete: false, @@ -77,6 +75,14 @@ vi.mock('../../rename-modal', () => ({ default: () => null, })) +vi.mock('../dataset-card', () => ({ + default: ({ dataset }: { dataset: DataSet }) => ( +
+ {dataset.name} +
+ ), +})) + function createMockDataset(overrides: Partial = {}): DataSet { return { id: 'dataset-1', diff --git a/web/app/components/datasets/list/__tests__/index.spec.tsx b/web/app/components/datasets/list/__tests__/index.spec.tsx index adc53debbd..7e46c46f1a 100644 --- a/web/app/components/datasets/list/__tests__/index.spec.tsx +++ b/web/app/components/datasets/list/__tests__/index.spec.tsx @@ -36,11 +36,6 @@ vi.mock('@/context/external-api-panel-context', () => ({ }), })) -// Mock tag management store -vi.mock('@/app/components/base/tag-management/store', () => ({ - useStore: () => false, -})) - // Mock useDocumentTitle hook vi.mock('@/hooks/use-document-title', () => ({ default: vi.fn(), @@ -108,15 +103,16 @@ vi.mock('@/app/components/develop/secret-key/secret-key-modal', () => ({ })) // Mock TagManagementModal -vi.mock('@/app/components/base/tag-management', () => ({ - default: () =>
, +vi.mock('@/features/tag-management/components/tag-management-modal', () => ({ + TagManagementModal: ({ show }: { show: boolean }) => show ?
: null, })) // Mock TagFilter -vi.mock('@/app/components/base/tag-management/filter', () => ({ - default: ({ onChange }: { value: string[], onChange: (val: string[]) => void }) => ( +vi.mock('@/features/tag-management/components/tag-filter', () => ({ + TagFilter: ({ onChange, onOpenTagManagement }: { value: string[], onChange: (val: string[]) => void, onOpenTagManagement: () => void }) => (
+
), })) @@ -226,7 +222,7 @@ describe('List', () => { it('should have correct container styling', () => { const { container } = render() const mainContainer = container.firstChild as HTMLElement - expect(mainContainer).toHaveClass('scroll-container', 'relative', 'flex', 'grow', 'flex-col') + expect(mainContainer).toHaveClass('relative', 'flex', 'grow', 'flex-col') }) }) @@ -312,15 +308,9 @@ describe('List', () => { expect(mockSetShowExternalApiPanel).toHaveBeenCalledWith(false) }) - it('should show TagManagementModal when showTagManagementModal is true', async () => { - vi.doMock('@/app/components/base/tag-management/store', () => ({ - useStore: () => true, // showTagManagementModal is true - })) - - vi.resetModules() - const { default: ListComponent } = await import('../index') - - render() + it('should show TagManagementModal when tag management is opened', () => { + render() + fireEvent.click(screen.getByText('Manage Tags')) expect(screen.getByTestId('tag-management-modal')).toBeInTheDocument() }) diff --git a/web/app/components/datasets/list/dataset-card/__tests__/index.spec.tsx b/web/app/components/datasets/list/dataset-card/__tests__/index.spec.tsx index f6c7e1e93d..55176faf47 100644 --- a/web/app/components/datasets/list/dataset-card/__tests__/index.spec.tsx +++ b/web/app/components/datasets/list/dataset-card/__tests__/index.spec.tsx @@ -30,8 +30,6 @@ vi.mock('@/context/app-context', () => ({ vi.mock('../hooks/use-dataset-card-state', () => ({ useDatasetCardState: () => ({ - tags: [], - setTags: vi.fn(), modalState: { showRenameModal: false, showConfirmDelete: false, @@ -55,8 +53,8 @@ vi.mock('../components/dataset-card-header', () => ({ vi.mock('../components/dataset-card-modals', () => ({ default: () =>
, })) -vi.mock('../components/tag-area', () => ({ - default: ({ onClick }: { onClick: (e: React.MouseEvent) => void, ref?: React.Ref }) => ( +vi.mock('@/features/tag-management/components/dataset-card-tags', () => ({ + DatasetCardTags: ({ onClick }: { onClick: (e: React.MouseEvent) => void }) => (
), })) diff --git a/web/app/components/datasets/list/dataset-card/components/__tests__/tag-area.spec.tsx b/web/app/components/datasets/list/dataset-card/components/__tests__/tag-area.spec.tsx deleted file mode 100644 index 2858469cdb..0000000000 --- a/web/app/components/datasets/list/dataset-card/components/__tests__/tag-area.spec.tsx +++ /dev/null @@ -1,198 +0,0 @@ -import type { Tag } from '@/app/components/base/tag-management/constant' -import type { DataSet } from '@/models/datasets' -import { fireEvent, render, screen } from '@testing-library/react' -import { useRef } from 'react' -import { describe, expect, it, vi } from 'vitest' -import { IndexingType } from '@/app/components/datasets/create/step-two' -import { ChunkingMode, DatasetPermission, DataSourceType } from '@/models/datasets' -import TagArea from '../tag-area' - -// Mock TagSelector as it's a complex component from base -vi.mock('@/app/components/base/tag-management/selector', () => ({ - default: ({ value, selectedTags, onCacheUpdate, onChange }: { - value: string[] - selectedTags: Tag[] - onCacheUpdate: (tags: Tag[]) => void - onChange?: () => void - }) => ( -
-
{value.join(',')}
-
- {selectedTags.length} - {' '} - tags -
- - -
- ), -})) - -describe('TagArea', () => { - const createMockDataset = (overrides: Partial = {}): DataSet => ({ - id: 'dataset-1', - name: 'Test Dataset', - description: 'Test description', - provider: 'vendor', - permission: DatasetPermission.allTeamMembers, - data_source_type: DataSourceType.FILE, - indexing_technique: IndexingType.QUALIFIED, - embedding_available: true, - app_count: 5, - document_count: 10, - word_count: 1000, - updated_at: 1609545600, - tags: [], - embedding_model: 'text-embedding-ada-002', - embedding_model_provider: 'openai', - created_by: 'user-1', - doc_form: ChunkingMode.text, - ...overrides, - } as DataSet) - - const mockTags: Tag[] = [ - { id: 'tag-1', name: 'Tag 1', type: 'knowledge', binding_count: 0 }, - { id: 'tag-2', name: 'Tag 2', type: 'knowledge', binding_count: 0 }, - ] - - const defaultProps = { - dataset: createMockDataset(), - tags: mockTags, - setTags: vi.fn(), - onSuccess: vi.fn(), - isHoveringTagSelector: false, - onClick: vi.fn(), - } - - beforeEach(() => { - vi.clearAllMocks() - }) - - describe('Rendering', () => { - it('should render without crashing', () => { - render() - expect(screen.getByTestId('tag-selector')).toBeInTheDocument() - }) - - it('should render TagSelector with correct value', () => { - render() - expect(screen.getByTestId('tag-values')).toHaveTextContent('tag-1,tag-2') - }) - - it('should display selected tags count', () => { - render() - expect(screen.getByTestId('selected-count')).toHaveTextContent('2 tags') - }) - }) - - describe('Props', () => { - it('should pass dataset id to TagSelector', () => { - const dataset = createMockDataset({ id: 'custom-dataset-id' }) - render() - expect(screen.getByTestId('tag-selector')).toBeInTheDocument() - }) - - it('should render with empty tags', () => { - render() - expect(screen.getByTestId('selected-count')).toHaveTextContent('0 tags') - }) - - it('should forward ref correctly', () => { - const TestComponent = () => { - const ref = useRef(null) - return - } - render() - expect(screen.getByTestId('tag-selector')).toBeInTheDocument() - }) - }) - - describe('User Interactions', () => { - it('should call onClick when container is clicked', () => { - const onClick = vi.fn() - const { container } = render() - - const wrapper = container.firstChild as HTMLElement - fireEvent.click(wrapper) - - expect(onClick).toHaveBeenCalledTimes(1) - }) - - it('should call setTags when tags are updated', () => { - const setTags = vi.fn() - render() - - fireEvent.click(screen.getByText('Update Tags')) - - expect(setTags).toHaveBeenCalledWith([{ id: 'new-tag', name: 'New Tag', type: 'knowledge', binding_count: 0 }]) - }) - - it('should call onSuccess when onChange is triggered', () => { - const onSuccess = vi.fn() - render() - - fireEvent.click(screen.getByText('Trigger Change')) - - expect(onSuccess).toHaveBeenCalledTimes(1) - }) - }) - - describe('Styles', () => { - it('should have opacity class when embedding is not available', () => { - const dataset = createMockDataset({ embedding_available: false }) - const { container } = render() - const wrapper = container.firstChild as HTMLElement - expect(wrapper).toHaveClass('opacity-30') - }) - - it('should not have opacity class when embedding is available', () => { - const dataset = createMockDataset({ embedding_available: true }) - const { container } = render() - const wrapper = container.firstChild as HTMLElement - expect(wrapper).not.toHaveClass('opacity-30') - }) - - it('should show mask when not hovering and has tags', () => { - const { container } = render() - const maskDiv = container.querySelector('.bg-tag-selector-mask-bg') - expect(maskDiv).toBeInTheDocument() - expect(maskDiv).not.toHaveClass('hidden') - }) - - it('should hide mask when hovering', () => { - const { container } = render() - // When hovering, the mask div should have 'hidden' class - const maskDiv = container.querySelector('.absolute.right-0.top-0') - expect(maskDiv).toHaveClass('hidden') - }) - - it('should make TagSelector visible when tags exist', () => { - const { container } = render() - const tagSelectorWrapper = container.querySelector('.visible') - expect(tagSelectorWrapper).toBeInTheDocument() - }) - }) - - describe('Edge Cases', () => { - it('should handle undefined onSuccess', () => { - render() - // Should not throw when clicking Trigger Change - expect(() => fireEvent.click(screen.getByText('Trigger Change'))).not.toThrow() - }) - - it('should handle many tags', () => { - const manyTags: Tag[] = Array.from({ length: 20 }, (_, i) => ({ - id: `tag-${i}`, - name: `Tag ${i}`, - type: 'knowledge' as const, - binding_count: 0, - })) - render() - expect(screen.getByTestId('selected-count')).toHaveTextContent('20 tags') - }) - }) -}) diff --git a/web/app/components/datasets/list/dataset-card/components/tag-area.tsx b/web/app/components/datasets/list/dataset-card/components/tag-area.tsx deleted file mode 100644 index 2c8d6aa73a..0000000000 --- a/web/app/components/datasets/list/dataset-card/components/tag-area.tsx +++ /dev/null @@ -1,55 +0,0 @@ -import type { Tag } from '@/app/components/base/tag-management/constant' -import type { DataSet } from '@/models/datasets' -import { cn } from '@langgenius/dify-ui/cn' -import * as React from 'react' -import TagSelector from '@/app/components/base/tag-management/selector' - -type TagAreaProps = { - dataset: DataSet - tags: Tag[] - setTags: (tags: Tag[]) => void - onSuccess?: () => void - isHoveringTagSelector: boolean - onClick: (e: React.MouseEvent) => void -} - -const TagArea = React.forwardRef(({ - dataset, - tags, - setTags, - onSuccess, - isHoveringTagSelector, - onClick, -}, ref) => ( -
-
0 && 'visible', - )} - > - tag.id)} - selectedTags={tags} - onCacheUpdate={setTags} - onChange={onSuccess} - /> -
-
-
-)) -TagArea.displayName = 'TagArea' - -export default TagArea diff --git a/web/app/components/datasets/list/dataset-card/hooks/__tests__/use-dataset-card-state.spec.ts b/web/app/components/datasets/list/dataset-card/hooks/__tests__/use-dataset-card-state.spec.ts index 7d07bcf9d0..fa4868f391 100644 --- a/web/app/components/datasets/list/dataset-card/hooks/__tests__/use-dataset-card-state.spec.ts +++ b/web/app/components/datasets/list/dataset-card/hooks/__tests__/use-dataset-card-state.spec.ts @@ -66,15 +66,6 @@ describe('useDatasetCardState', () => { }) describe('Initial State', () => { - it('should return tags from dataset', () => { - const dataset = createMockDataset() - const { result } = renderHook(() => - useDatasetCardState({ dataset, onSuccess: vi.fn() }), - ) - - expect(result.current.tags).toEqual(dataset.tags) - }) - it('should have initial modal state closed', () => { const dataset = createMockDataset() const { result } = renderHook(() => @@ -96,36 +87,6 @@ describe('useDatasetCardState', () => { }) }) - describe('Tags State', () => { - it('should update tags when setTags is called', () => { - const dataset = createMockDataset() - const { result } = renderHook(() => - useDatasetCardState({ dataset, onSuccess: vi.fn() }), - ) - - act(() => { - result.current.setTags([{ id: 'tag-2', name: 'Tag 2', type: 'knowledge', binding_count: 0 }]) - }) - - expect(result.current.tags).toEqual([{ id: 'tag-2', name: 'Tag 2', type: 'knowledge', binding_count: 0 }]) - }) - - it('should sync tags when dataset tags change', () => { - const dataset = createMockDataset() - const { result, rerender } = renderHook( - ({ dataset }) => useDatasetCardState({ dataset, onSuccess: vi.fn() }), - { initialProps: { dataset } }, - ) - - const newTags = [{ id: 'tag-3', name: 'Tag 3', type: 'knowledge', binding_count: 0 }] - const updatedDataset = createMockDataset({ tags: newTags }) - - rerender({ dataset: updatedDataset }) - - expect(result.current.tags).toEqual(newTags) - }) - }) - describe('Modal Handlers', () => { it('should open rename modal when openRenameModal is called', () => { const dataset = createMockDataset() @@ -279,15 +240,6 @@ describe('useDatasetCardState', () => { }) describe('Edge Cases', () => { - it('should handle empty tags array', () => { - const dataset = createMockDataset({ tags: [] }) - const { result } = renderHook(() => - useDatasetCardState({ dataset, onSuccess: vi.fn() }), - ) - - expect(result.current.tags).toEqual([]) - }) - it('should handle undefined onSuccess', async () => { const dataset = createMockDataset() const { result } = renderHook(() => diff --git a/web/app/components/datasets/list/dataset-card/hooks/use-dataset-card-state.ts b/web/app/components/datasets/list/dataset-card/hooks/use-dataset-card-state.ts index 6cffbb6828..88aa7b50ae 100644 --- a/web/app/components/datasets/list/dataset-card/hooks/use-dataset-card-state.ts +++ b/web/app/components/datasets/list/dataset-card/hooks/use-dataset-card-state.ts @@ -1,7 +1,6 @@ -import type { Tag } from '@/app/components/base/tag-management/constant' import type { DataSet } from '@/models/datasets' import { toast } from '@langgenius/dify-ui/toast' -import { useCallback, useEffect, useState } from 'react' +import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' import { useCheckDatasetUsage, useDeleteDataset } from '@/service/use-dataset-card' import { useExportPipelineDSL } from '@/service/use-pipeline' @@ -20,11 +19,6 @@ type UseDatasetCardStateOptions = { export const useDatasetCardState = ({ dataset, onSuccess }: UseDatasetCardStateOptions) => { const { t } = useTranslation() - const [tags, setTags] = useState(dataset.tags) - - useEffect(() => { - setTags(dataset.tags) - }, [dataset.tags]) // Modal state const [modalState, setModalState] = useState({ @@ -113,10 +107,6 @@ export const useDatasetCardState = ({ dataset, onSuccess }: UseDatasetCardStateO }, [dataset.id, deleteDatasetMutation, onSuccess, t, closeConfirmDelete]) return { - // Tag state - tags, - setTags, - // Modal state modalState, openRenameModal, diff --git a/web/app/components/datasets/list/dataset-card/index.tsx b/web/app/components/datasets/list/dataset-card/index.tsx index 5bd032d151..3fe4b6f7c0 100644 --- a/web/app/components/datasets/list/dataset-card/index.tsx +++ b/web/app/components/datasets/list/dataset-card/index.tsx @@ -1,8 +1,8 @@ 'use client' import type { DataSet } from '@/models/datasets' -import { useHover } from 'ahooks' -import { useMemo, useRef } from 'react' +import { useMemo } from 'react' import { useSelector as useAppContextWithSelector } from '@/context/app-context' +import { DatasetCardTags } from '@/features/tag-management/components/dataset-card-tags' import { useRouter } from '@/next/navigation' import CornerLabels from './components/corner-labels' import DatasetCardFooter from './components/dataset-card-footer' @@ -10,29 +10,27 @@ import DatasetCardHeader from './components/dataset-card-header' import DatasetCardModals from './components/dataset-card-modals' import Description from './components/description' import OperationsDropdown from './components/operations-dropdown' -import TagArea from './components/tag-area' -import { useDatasetCardState } from './hooks/use-dataset-card-state' +import { useDatasetCardState as useDatasetCardController } from './hooks/use-dataset-card-state' const EXTERNAL_PROVIDER = 'external' type DatasetCardProps = { dataset: DataSet onSuccess?: () => void + onOpenTagManagement?: () => void } const DatasetCard = ({ dataset, onSuccess, + onOpenTagManagement = () => {}, }: DatasetCardProps) => { const { push } = useRouter() const isCurrentWorkspaceDatasetOperator = useAppContextWithSelector(state => state.isCurrentWorkspaceDatasetOperator) - const tagSelectorRef = useRef(null) - const isHoveringTagSelector = useHover(tagSelectorRef) + const datasetCard = useDatasetCardController({ dataset, onSuccess }) const { - tags, - setTags, modalState, openRenameModal, closeRenameModal, @@ -40,7 +38,7 @@ const DatasetCard = ({ handleExportPipeline, detectIsUsedByApp, onConfirmDelete, - } = useDatasetCardState({ dataset, onSuccess }) + } = datasetCard const isExternalProvider = dataset.provider === EXTERNAL_PROVIDER const isPipelineUnpublished = useMemo(() => { @@ -72,14 +70,13 @@ const DatasetCard = ({ - void } const Datasets = ({ tags, keywords, includeAll, + onOpenTagManagement = () => {}, }: Props) => { const { t } = useTranslation() const isCurrentWorkspaceEditor = useAppContextWithSelector(state => state.isCurrentWorkspaceEditor) @@ -60,7 +62,7 @@ const Datasets = ({