diff --git a/.github/workflows/db-migration-test.yml b/.github/workflows/db-migration-test.yml index b1ccf496df..65f0149a74 100644 --- a/.github/workflows/db-migration-test.yml +++ b/.github/workflows/db-migration-test.yml @@ -110,6 +110,28 @@ jobs: sed -i 's/DB_PORT=5432/DB_PORT=3306/' .env sed -i 's/DB_USERNAME=postgres/DB_USERNAME=root/' .env + # hoverkraft-tech/compose-action@v2.6.0 only waits for `docker compose up -d` + # to return (container processes started); it does not wait on healthcheck + # status. mysql:8.0's first-time init takes 15-30s, so without an explicit + # wait the migration runs while InnoDB is still initialising and gets + # killed with "Lost connection during query". Poll a real SELECT until it + # succeeds. + - name: Wait for MySQL to accept queries + run: | + set +e + for i in $(seq 1 60); do + if docker run --rm --network host mysql:8.0 \ + mysql -h 127.0.0.1 -P 3306 -uroot -pdifyai123456 \ + -e 'SELECT 1' >/dev/null 2>&1; then + echo "MySQL ready after ${i}s" + exit 0 + fi + sleep 1 + done + echo "MySQL not ready after 60s; dumping container logs:" + docker compose -f docker/docker-compose.middleware.yaml --profile mysql logs --tail=200 db_mysql + exit 1 + - name: Run DB Migration env: DEBUG: true diff --git a/.github/workflows/web-e2e.yml b/.github/workflows/web-e2e.yml index a634830fef..bdc24887db 100644 --- a/.github/workflows/web-e2e.yml +++ b/.github/workflows/web-e2e.yml @@ -13,7 +13,7 @@ concurrency: jobs: test: name: Web Full-Stack E2E - runs-on: depot-ubuntu-24.04 + runs-on: depot-ubuntu-24.04-4 defaults: run: shell: bash diff --git a/api/controllers/service_api/dataset/document.py b/api/controllers/service_api/dataset/document.py index bc28ecb6b7..0b09facf58 100644 --- a/api/controllers/service_api/dataset/document.py +++ b/api/controllers/service_api/dataset/document.py @@ -468,15 +468,98 @@ class DocumentAddByFileApi(DatasetApiResource): return documents_and_batch_fields, 200 +def _update_document_by_file(tenant_id: str, dataset_id: UUID, document_id: UUID) -> tuple[Mapping[str, object], int]: + """Update a document from an uploaded file for canonical and deprecated routes.""" + dataset_id_str = str(dataset_id) + tenant_id_str = str(tenant_id) + dataset = db.session.scalar( + select(Dataset).where(Dataset.tenant_id == tenant_id_str, Dataset.id == dataset_id_str).limit(1) + ) + + if not dataset: + raise ValueError("Dataset does not exist.") + + if dataset.provider == "external": + raise ValueError("External datasets are not supported.") + + args: dict[str, object] = {} + if "data" in request.form: + args = json.loads(request.form["data"]) + if "doc_form" not in args: + args["doc_form"] = dataset.chunk_structure or "text_model" + if "doc_language" not in args: + args["doc_language"] = "English" + + # indexing_technique is already set in dataset since this is an update + args["indexing_technique"] = dataset.indexing_technique + + if "file" in request.files: + # save file info + file = request.files["file"] + + if len(request.files) > 1: + raise TooManyFilesError() + + if not file.filename: + raise FilenameNotExistsError + + if not current_user: + raise ValueError("current_user is required") + + try: + upload_file = FileService(db.engine).upload_file( + filename=file.filename, + content=file.read(), + mimetype=file.mimetype, + user=current_user, + source="datasets", + ) + except services.errors.file.FileTooLargeError as file_too_large_error: + raise FileTooLargeError(file_too_large_error.description) + except services.errors.file.UnsupportedFileTypeError: + raise UnsupportedFileTypeError() + data_source = { + "type": "upload_file", + "info_list": {"data_source_type": "upload_file", "file_info_list": {"file_ids": [upload_file.id]}}, + } + args["data_source"] = data_source + + # validate args + args["original_document_id"] = str(document_id) + + knowledge_config = KnowledgeConfig.model_validate(args) + DocumentService.document_create_args_validate(knowledge_config) + + try: + documents, _ = DocumentService.save_document_with_dataset_id( + dataset=dataset, + knowledge_config=knowledge_config, + account=dataset.created_by_account, + dataset_process_rule=dataset.latest_process_rule if "process_rule" not in args else None, + created_from="api", + ) + except ProviderTokenNotInitError as ex: + raise ProviderNotInitializeError(ex.description) + document = documents[0] + documents_and_batch_fields = {"document": marshal(document, document_fields), "batch": document.batch} + return documents_and_batch_fields, 200 + + @service_api_ns.route( "/datasets//documents//update_by_file", "/datasets//documents//update-by-file", ) -class DocumentUpdateByFileApi(DatasetApiResource): - """Resource for update documents.""" +class DeprecatedDocumentUpdateByFileApi(DatasetApiResource): + """Deprecated resource aliases for file document updates.""" - @service_api_ns.doc("update_document_by_file") - @service_api_ns.doc(description="Update an existing document by uploading a file") + @service_api_ns.doc("update_document_by_file_deprecated") + @service_api_ns.doc(deprecated=True) + @service_api_ns.doc( + description=( + "Deprecated legacy alias for updating an existing document by uploading a file. " + "Use PATCH /datasets/{dataset_id}/documents/{document_id} instead." + ) + ) @service_api_ns.doc(params={"dataset_id": "Dataset ID", "document_id": "Document ID"}) @service_api_ns.doc( responses={ @@ -487,82 +570,9 @@ class DocumentUpdateByFileApi(DatasetApiResource): ) @cloud_edition_billing_resource_check("vector_space", "dataset") @cloud_edition_billing_rate_limit_check("knowledge", "dataset") - def post(self, tenant_id, dataset_id, document_id): - """Update document by upload file.""" - dataset = db.session.scalar( - select(Dataset).where(Dataset.tenant_id == tenant_id, Dataset.id == dataset_id).limit(1) - ) - - if not dataset: - raise ValueError("Dataset does not exist.") - - if dataset.provider == "external": - raise ValueError("External datasets are not supported.") - - args = {} - if "data" in request.form: - args = json.loads(request.form["data"]) - if "doc_form" not in args: - args["doc_form"] = dataset.chunk_structure or "text_model" - if "doc_language" not in args: - args["doc_language"] = "English" - - # get dataset info - dataset_id = str(dataset_id) - tenant_id = str(tenant_id) - - # indexing_technique is already set in dataset since this is an update - args["indexing_technique"] = dataset.indexing_technique - - if "file" in request.files: - # save file info - file = request.files["file"] - - if len(request.files) > 1: - raise TooManyFilesError() - - if not file.filename: - raise FilenameNotExistsError - - if not current_user: - raise ValueError("current_user is required") - - try: - upload_file = FileService(db.engine).upload_file( - filename=file.filename, - content=file.read(), - mimetype=file.mimetype, - user=current_user, - source="datasets", - ) - except services.errors.file.FileTooLargeError as file_too_large_error: - raise FileTooLargeError(file_too_large_error.description) - except services.errors.file.UnsupportedFileTypeError: - raise UnsupportedFileTypeError() - data_source = { - "type": "upload_file", - "info_list": {"data_source_type": "upload_file", "file_info_list": {"file_ids": [upload_file.id]}}, - } - args["data_source"] = data_source - # validate args - args["original_document_id"] = str(document_id) - - knowledge_config = KnowledgeConfig.model_validate(args) - DocumentService.document_create_args_validate(knowledge_config) - - try: - documents, _ = DocumentService.save_document_with_dataset_id( - dataset=dataset, - knowledge_config=knowledge_config, - account=dataset.created_by_account, - dataset_process_rule=dataset.latest_process_rule if "process_rule" not in args else None, - created_from="api", - ) - except ProviderTokenNotInitError as ex: - raise ProviderNotInitializeError(ex.description) - document = documents[0] - documents_and_batch_fields = {"document": marshal(document, document_fields), "batch": document.batch} - return documents_and_batch_fields, 200 + def post(self, tenant_id: str, dataset_id: UUID, document_id: UUID): + """Update document by file through the deprecated file-update aliases.""" + return _update_document_by_file(tenant_id=tenant_id, dataset_id=dataset_id, document_id=document_id) @service_api_ns.route("/datasets//documents") @@ -876,6 +886,22 @@ class DocumentApi(DatasetApiResource): return response + @service_api_ns.doc("update_document_by_file") + @service_api_ns.doc(description="Update an existing document by uploading a file") + @service_api_ns.doc(params={"dataset_id": "Dataset ID", "document_id": "Document ID"}) + @service_api_ns.doc( + responses={ + 200: "Document updated successfully", + 401: "Unauthorized - invalid API token", + 404: "Document not found", + } + ) + @cloud_edition_billing_resource_check("vector_space", "dataset") + @cloud_edition_billing_rate_limit_check("knowledge", "dataset") + def patch(self, tenant_id: str, dataset_id: UUID, document_id: UUID): + """Update document by file on the canonical document resource.""" + return _update_document_by_file(tenant_id=tenant_id, dataset_id=dataset_id, document_id=document_id) + @service_api_ns.doc("delete_document") @service_api_ns.doc(description="Delete a document") @service_api_ns.doc(params={"dataset_id": "Dataset ID", "document_id": "Document ID"}) diff --git a/api/tests/unit_tests/controllers/service_api/dataset/test_document.py b/api/tests/unit_tests/controllers/service_api/dataset/test_document.py index 1b391e67ec..230c51161f 100644 --- a/api/tests/unit_tests/controllers/service_api/dataset/test_document.py +++ b/api/tests/unit_tests/controllers/service_api/dataset/test_document.py @@ -23,6 +23,7 @@ from werkzeug.exceptions import Forbidden, NotFound from controllers.service_api.dataset.document import ( DeprecatedDocumentAddByTextApi, + DeprecatedDocumentUpdateByFileApi, DeprecatedDocumentUpdateByTextApi, DocumentAddByFileApi, DocumentAddByTextApi, @@ -32,7 +33,6 @@ from controllers.service_api.dataset.document import ( DocumentListQuery, DocumentTextCreatePayload, DocumentTextUpdate, - DocumentUpdateByFileApi, DocumentUpdateByTextApi, InvalidMetadataError, ) @@ -1095,8 +1095,8 @@ class TestArchivedDocumentImmutableError: assert error.code == 403 -class TestDocumentTextRouteDeprecation: - """Test that legacy underscore text routes stay marked deprecated.""" +class TestDocumentRouteDeprecation: + """Test that legacy document routes stay marked deprecated.""" def test_create_by_text_legacy_alias_is_deprecated(self): """Ensure only the legacy create-by-text alias is marked deprecated.""" @@ -1108,10 +1108,15 @@ class TestDocumentTextRouteDeprecation: assert DeprecatedDocumentUpdateByTextApi.post.__apidoc__["deprecated"] is True assert DocumentUpdateByTextApi.post.__apidoc__.get("deprecated") is not True + def test_update_by_file_legacy_aliases_are_deprecated(self): + """Ensure only the legacy file-update aliases are marked deprecated.""" + assert DeprecatedDocumentUpdateByFileApi.post.__apidoc__["deprecated"] is True + assert DocumentApi.patch.__apidoc__.get("deprecated") is not True + # ============================================================================= # Endpoint tests for DocumentUpdateByTextApi, DocumentAddByFileApi, -# DocumentUpdateByFileApi. +# and the canonical/deprecated document file update routes. # # These controllers use ``@cloud_edition_billing_resource_check`` (does NOT # preserve ``__wrapped__``) and ``@cloud_edition_billing_rate_limit_check`` @@ -1359,13 +1364,52 @@ class TestDocumentAddByFileApiPost: api.post(tenant_id=mock_tenant.id, dataset_id=mock_dataset.id) -class TestDocumentUpdateByFileApiPost: - """Test suite for DocumentUpdateByFileApi.post() endpoint. +class TestDocumentUpdateByFileApiPatch: + """Test suite for the canonical document file update endpoint. - ``post`` is wrapped by ``@cloud_edition_billing_resource_check`` and + ``patch`` is wrapped by ``@cloud_edition_billing_resource_check`` and ``@cloud_edition_billing_rate_limit_check``. """ + @pytest.mark.parametrize("route_name", ["update_by_file", "update-by-file"]) + @patch("controllers.service_api.dataset.document._update_document_by_file") + @patch("controllers.service_api.wraps.FeatureService") + @patch("controllers.service_api.wraps.validate_and_get_api_token") + def test_update_by_file_deprecated_aliases_delegate_to_shared_handler( + self, + mock_validate_token, + mock_feature_svc, + mock_update_document_by_file, + route_name, + app, + mock_tenant, + mock_dataset, + ): + """Test legacy POST aliases still dispatch while marked deprecated.""" + _setup_billing_mocks(mock_validate_token, mock_feature_svc, mock_tenant.id) + mock_update_document_by_file.return_value = ({"document": {"id": "doc-1"}, "batch": "batch-1"}, 200) + + doc_id = str(uuid.uuid4()) + with app.test_request_context( + f"/datasets/{mock_dataset.id}/documents/{doc_id}/{route_name}", + method="POST", + headers={"Authorization": "Bearer test_token"}, + ): + api = DeprecatedDocumentUpdateByFileApi() + response, status = api.post( + tenant_id=mock_tenant.id, + dataset_id=mock_dataset.id, + document_id=doc_id, + ) + + assert status == 200 + assert response["batch"] == "batch-1" + mock_update_document_by_file.assert_called_once_with( + tenant_id=mock_tenant.id, + dataset_id=mock_dataset.id, + document_id=doc_id, + ) + @patch("controllers.service_api.dataset.document.db") @patch("controllers.service_api.wraps.FeatureService") @patch("controllers.service_api.wraps.validate_and_get_api_token") @@ -1387,15 +1431,15 @@ class TestDocumentUpdateByFileApiPost: doc_id = str(uuid.uuid4()) data = {"file": (BytesIO(b"content"), "test.pdf", "application/pdf")} with app.test_request_context( - f"/datasets/{mock_dataset.id}/documents/{doc_id}/update_by_file", - method="POST", + f"/datasets/{mock_dataset.id}/documents/{doc_id}", + method="PATCH", content_type="multipart/form-data", data=data, headers={"Authorization": "Bearer test_token"}, ): - api = DocumentUpdateByFileApi() + api = DocumentApi() with pytest.raises(ValueError, match="Dataset does not exist"): - api.post( + api.patch( tenant_id=mock_tenant.id, dataset_id=mock_dataset.id, document_id=doc_id, @@ -1423,15 +1467,15 @@ class TestDocumentUpdateByFileApiPost: doc_id = str(uuid.uuid4()) data = {"file": (BytesIO(b"content"), "test.pdf", "application/pdf")} with app.test_request_context( - f"/datasets/{mock_dataset.id}/documents/{doc_id}/update_by_file", - method="POST", + f"/datasets/{mock_dataset.id}/documents/{doc_id}", + method="PATCH", content_type="multipart/form-data", data=data, headers={"Authorization": "Bearer test_token"}, ): - api = DocumentUpdateByFileApi() + api = DocumentApi() with pytest.raises(ValueError, match="External datasets"): - api.post( + api.patch( tenant_id=mock_tenant.id, dataset_id=mock_dataset.id, document_id=doc_id, @@ -1482,14 +1526,14 @@ class TestDocumentUpdateByFileApiPost: doc_id = str(uuid.uuid4()) data = {"file": (BytesIO(b"file content"), "test.pdf", "application/pdf")} with app.test_request_context( - f"/datasets/{mock_dataset.id}/documents/{doc_id}/update_by_file", - method="POST", + f"/datasets/{mock_dataset.id}/documents/{doc_id}", + method="PATCH", content_type="multipart/form-data", data=data, headers={"Authorization": "Bearer test_token"}, ): - api = DocumentUpdateByFileApi() - response, status = api.post( + api = DocumentApi() + response, status = api.patch( tenant_id=mock_tenant.id, dataset_id=mock_dataset.id, document_id=doc_id, diff --git a/api/tests/unit_tests/oss/baidu_obs/test_baidu_obs.py b/api/tests/unit_tests/oss/baidu_obs/test_baidu_obs.py index 18ac762db8..053f811264 100644 --- a/api/tests/unit_tests/oss/baidu_obs/test_baidu_obs.py +++ b/api/tests/unit_tests/oss/baidu_obs/test_baidu_obs.py @@ -5,12 +5,13 @@ from baidubce.auth.bce_credentials import BceCredentials from baidubce.bce_client_configuration import BceClientConfiguration from extensions.storage.baidu_obs_storage import BaiduObsStorage -from tests.unit_tests.oss.__mock.baidu_obs import setup_baidu_obs_mock from tests.unit_tests.oss.__mock.base import ( BaseStorageTest, get_example_bucket, ) +pytest_plugins = ("tests.unit_tests.oss.__mock.baidu_obs",) + class TestBaiduObs(BaseStorageTest): @pytest.fixture(autouse=True) diff --git a/docker/docker-compose.middleware.yaml b/docker/docker-compose.middleware.yaml index 911da70a73..af3d54dfb3 100644 --- a/docker/docker-compose.middleware.yaml +++ b/docker/docker-compose.middleware.yaml @@ -59,19 +59,25 @@ services: - ${MYSQL_HOST_VOLUME:-./volumes/mysql/data}:/var/lib/mysql ports: - "${EXPOSE_MYSQL_PORT:-3306}:3306" + # mysqladmin ping passes during mysql:8.0's TCP-listening stage even while + # the server is still finalising init, leading to "Lost connection during + # query" on the first real query. Verify with a real SELECT instead. healthcheck: test: [ "CMD", - "mysqladmin", - "ping", - "-u", - "root", + "mysql", + "-h", + "127.0.0.1", + "-uroot", "-p${DB_PASSWORD:-difyai123456}", + "-e", + "SELECT 1", ] interval: 1s timeout: 3s retries: 30 + start_period: 20s # The redis cache. redis: diff --git a/e2e/features/apps/share-app.feature b/e2e/features/apps/share-app.feature index 22f89f7ebb..1c707306ef 100644 --- a/e2e/features/apps/share-app.feature +++ b/e2e/features/apps/share-app.feature @@ -17,3 +17,10 @@ Feature: Share app publicly Given a workflow app has been published and shared via API When I open the shared app URL Then the shared app page should be accessible + + @unauthenticated + Scenario: Run a shared workflow app without authentication + Given a workflow app has been published and shared via API + When I open the shared app URL + And I run the shared workflow app + Then the shared workflow run should succeed diff --git a/e2e/features/step-definitions/apps/share-app.steps.ts b/e2e/features/step-definitions/apps/share-app.steps.ts index 24da05baab..d5742bdaa8 100644 --- a/e2e/features/step-definitions/apps/share-app.steps.ts +++ b/e2e/features/step-definitions/apps/share-app.steps.ts @@ -37,3 +37,15 @@ Then('the shared app page should be accessible', async function (this: DifyWorld await expect(this.getPage()).toHaveURL(/\/(workflow|chat)\/[a-zA-Z0-9]+/, { timeout: 15_000 }) await expect(this.getPage().locator('body')).toBeVisible({ timeout: 10_000 }) }) + +When('I run the shared workflow app', async function (this: DifyWorld) { + const page = this.getPage() + const runButton = page.getByTestId('run-button') + + await expect(runButton).toBeEnabled({ timeout: 15_000 }) + await runButton.click() +}) + +Then('the shared workflow run should succeed', async function (this: DifyWorld) { + await expect(this.getPage().getByTestId('status-icon-success')).toBeVisible({ timeout: 55_000 }) +}) diff --git a/e2e/features/step-definitions/apps/workflow-run.steps.ts b/e2e/features/step-definitions/apps/workflow-run.steps.ts index 584a33e774..84c03bfa8f 100644 --- a/e2e/features/step-definitions/apps/workflow-run.steps.ts +++ b/e2e/features/step-definitions/apps/workflow-run.steps.ts @@ -12,8 +12,10 @@ Given('a minimal runnable workflow draft has been synced', async function (this: When('I run the workflow', async function (this: DifyWorld) { const page = this.getPage() - await page.getByText('Test Run').click() - await expect(page.getByText('Running').first()).toBeVisible({ timeout: 15_000 }) + const testRunButton = page.getByText('Test Run') + + await expect(testRunButton).toBeVisible({ timeout: 15_000 }) + await testRunButton.click() }) Then('the workflow run should succeed', async function (this: DifyWorld) { diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 3f3bd5f1f7..e1c8bda126 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -3506,11 +3506,6 @@ "count": 1 } }, - "web/app/components/plugins/reference-setting-modal/auto-update-setting/strategy-picker.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/components/plugins/reference-setting-modal/auto-update-setting/types.ts": { "erasable-syntax-only/enums": { "count": 2 diff --git a/web/app/components/base/chat/embedded-chatbot/__tests__/hooks.spec.tsx b/web/app/components/base/chat/embedded-chatbot/__tests__/hooks.spec.tsx index b4f3f01070..a6a35f4c7b 100644 --- a/web/app/components/base/chat/embedded-chatbot/__tests__/hooks.spec.tsx +++ b/web/app/components/base/chat/embedded-chatbot/__tests__/hooks.spec.tsx @@ -532,6 +532,7 @@ describe('useEmbeddedChatbot', () => { }) it('handleChangeConversation updates current conversation and refetches chat list', async () => { + mockStoreState.embeddedConversationId = null const { result } = await renderWithClient(() => useEmbeddedChatbot(AppSourceType.webApp)) act(() => { @@ -548,6 +549,39 @@ describe('useEmbeddedChatbot', () => { expect(result.current.clearChatList).toBe(false) }) + // Scenario: URL-provided conversation_id should take precedence over localStorage value. + it('should prioritize URL conversation_id over localStorage', async () => { + localStorage.setItem(CONVERSATION_ID_INFO, JSON.stringify({ + 'app-1': { 'embedded-user-1': 'stored-conv-id' }, + })) + mockStoreState.embeddedConversationId = 'url-conv-id' + mockGetProcessedSystemVariablesFromUrlParams.mockResolvedValue({ + user_id: 'embedded-user-1', + conversation_id: 'url-conv-id', + }) + + const { result } = await renderWithClient(() => useEmbeddedChatbot(AppSourceType.webApp)) + + await waitFor(() => { + expect(result.current.currentConversationId).toBe('url-conv-id') + }) + }) + + // Scenario: When no URL conversation_id is provided, fall back to localStorage. + it('should fall back to localStorage when no URL conversation_id is provided', async () => { + localStorage.setItem(CONVERSATION_ID_INFO, JSON.stringify({ + 'app-1': { DEFAULT: 'stored-conv-id' }, + })) + mockStoreState.embeddedConversationId = null + mockStoreState.embeddedUserId = null + + const { result } = await renderWithClient(() => useEmbeddedChatbot(AppSourceType.webApp)) + + await waitFor(() => { + expect(result.current.currentConversationId).toBe('stored-conv-id') + }) + }) + it('handleFeedback invokes updateFeedback service successfully', async () => { const { updateFeedback } = await import('@/service/share') const { result } = await renderWithClient(() => useEmbeddedChatbot(AppSourceType.webApp)) diff --git a/web/app/components/base/chat/embedded-chatbot/hooks.tsx b/web/app/components/base/chat/embedded-chatbot/hooks.tsx index 4ce6bc6318..39cbe66fbb 100644 --- a/web/app/components/base/chat/embedded-chatbot/hooks.tsx +++ b/web/app/components/base/chat/embedded-chatbot/hooks.tsx @@ -113,7 +113,7 @@ export const useEmbeddedChatbot = (appSourceType: AppSourceType, tryAppId?: stri }) }, [setConversationIdInfo]) const allowResetChat = !conversationId - const currentConversationId = useMemo(() => conversationIdInfo?.[appId || '']?.[userId || 'DEFAULT'] || conversationId || '', [appId, conversationIdInfo, userId, conversationId]) + const currentConversationId = useMemo(() => conversationId || conversationIdInfo?.[appId || '']?.[userId || 'DEFAULT'] || '', [appId, conversationIdInfo, userId, conversationId]) const handleConversationIdInfoChange = useCallback((changeConversationId: string) => { if (appId) { let prevValue = conversationIdInfo?.[appId || ''] diff --git a/web/app/components/base/date-and-time-picker/common/__tests__/option-list-item.spec.tsx b/web/app/components/base/date-and-time-picker/common/__tests__/option-list-item.spec.tsx index 8ccf8fab73..feca6d2e92 100644 --- a/web/app/components/base/date-and-time-picker/common/__tests__/option-list-item.spec.tsx +++ b/web/app/components/base/date-and-time-picker/common/__tests__/option-list-item.spec.tsx @@ -43,7 +43,7 @@ describe('OptionListItem', () => { , ) - const item = screen.getByRole('listitem') + const item = screen.getByRole('button') expect(item).toHaveClass('bg-components-button-ghost-bg-hover') }) @@ -54,7 +54,7 @@ describe('OptionListItem', () => { , ) - const item = screen.getByRole('listitem') + const item = screen.getByRole('button') expect(item).not.toHaveClass('bg-components-button-ghost-bg-hover') }) }) @@ -100,7 +100,7 @@ describe('OptionListItem', () => { Clickable , ) - fireEvent.click(screen.getByRole('listitem')) + fireEvent.click(screen.getByRole('button')) expect(handleClick).toHaveBeenCalledTimes(1) }) @@ -111,7 +111,7 @@ describe('OptionListItem', () => { Item , ) - fireEvent.click(screen.getByRole('listitem')) + fireEvent.click(screen.getByRole('button')) expect(Element.prototype.scrollIntoView).toHaveBeenCalledWith({ behavior: 'smooth' }) }) @@ -126,7 +126,7 @@ describe('OptionListItem', () => { , ) - const item = screen.getByRole('listitem') + const item = screen.getByRole('button') fireEvent.click(item) fireEvent.click(item) fireEvent.click(item) diff --git a/web/app/components/base/date-and-time-picker/common/__tests__/option-list.spec.tsx b/web/app/components/base/date-and-time-picker/common/__tests__/option-list.spec.tsx new file mode 100644 index 0000000000..9e6a8cace9 --- /dev/null +++ b/web/app/components/base/date-and-time-picker/common/__tests__/option-list.spec.tsx @@ -0,0 +1,28 @@ +import { render, screen } from '@testing-library/react' +import OptionList from '../option-list' + +describe('OptionList', () => { + it('should render a scrollable list with hidden scrollbar styles', () => { + render( + +
  • Item
    • +     , + ) + + const list = screen.getByRole('list') + + expect(list).toHaveClass('overflow-y-auto') + expect(list).toHaveClass('[scrollbar-width:none]') + expect(list).toHaveClass('[&::-webkit-scrollbar]:hidden') + }) + + it('should append caller className after default classes', () => { + render( + +
  • Item
    • +     , + ) + + expect(screen.getByRole('list')).toHaveClass('custom-list') + }) +}) diff --git a/web/app/components/base/date-and-time-picker/common/option-list-item.tsx b/web/app/components/base/date-and-time-picker/common/option-list-item.tsx index 31b303df1f..e1bfdde4be 100644 --- a/web/app/components/base/date-and-time-picker/common/option-list-item.tsx +++ b/web/app/components/base/date-and-time-picker/common/option-list-item.tsx @@ -1,4 +1,4 @@ -import type { FC } from 'react' +import type { FC, ReactNode } from 'react' import { cn } from '@langgenius/dify-ui/cn' import * as React from 'react' import { useEffect, useRef } from 'react' @@ -7,7 +7,8 @@ type OptionListItemProps = { isSelected: boolean onClick: () => void noAutoScroll?: boolean -} & React.LiHTMLAttributes + children: ReactNode +} const OptionListItem: FC = ({ isSelected, @@ -25,16 +26,21 @@ const OptionListItem: FC = ({ return (
  • { - listItemRef.current?.scrollIntoView({ behavior: 'smooth' }) - onClick() - }} > - {children} +
    • ) } diff --git a/web/app/components/base/date-and-time-picker/common/option-list.tsx b/web/app/components/base/date-and-time-picker/common/option-list.tsx new file mode 100644 index 0000000000..8fc2407089 --- /dev/null +++ b/web/app/components/base/date-and-time-picker/common/option-list.tsx @@ -0,0 +1,26 @@ +import type { HTMLAttributes, ReactNode } from 'react' +import { cn } from '@langgenius/dify-ui/cn' +import * as React from 'react' + +type OptionListProps = { + children: ReactNode +} & HTMLAttributes + +const optionListClassName = cn( + 'flex h-[208px] flex-col gap-y-0.5 overflow-y-auto pb-[184px]', + '[scrollbar-width:none] [&::-webkit-scrollbar]:hidden', +) + +const OptionList = ({ + children, + className, + ...props +}: OptionListProps) => { + return ( +
      + {children} +
    + ) +} + +export default React.memo(OptionList) diff --git a/web/app/components/base/date-and-time-picker/time-picker/__tests__/options.spec.tsx b/web/app/components/base/date-and-time-picker/time-picker/__tests__/options.spec.tsx index d7fa3be797..1bf3a52a8b 100644 --- a/web/app/components/base/date-and-time-picker/time-picker/__tests__/options.spec.tsx +++ b/web/app/components/base/date-and-time-picker/time-picker/__tests__/options.spec.tsx @@ -64,13 +64,13 @@ describe('TimePickerOptions', () => { it('should render selected hour in the list', () => { const props = createOptionsProps({ selectedTime: dayjs('2024-01-01 05:30:00') }) render() - const selectedHour = screen.getAllByRole('listitem').find(item => item.textContent === '05') + const selectedHour = screen.getAllByRole('button').find(item => item.textContent === '05') expect(selectedHour)!.toHaveClass('bg-components-button-ghost-bg-hover') }) it('should render selected minute in the list', () => { const props = createOptionsProps({ selectedTime: dayjs('2024-01-01 05:30:00') }) render() - const selectedMinute = screen.getAllByRole('listitem').find(item => item.textContent === '30') + const selectedMinute = screen.getAllByRole('button').find(item => item.textContent === '30') expect(selectedMinute)!.toHaveClass('bg-components-button-ghost-bg-hover') }) diff --git a/web/app/components/base/date-and-time-picker/time-picker/options.tsx b/web/app/components/base/date-and-time-picker/time-picker/options.tsx index 6f6e5e9c68..10e94f983d 100644 --- a/web/app/components/base/date-and-time-picker/time-picker/options.tsx +++ b/web/app/components/base/date-and-time-picker/time-picker/options.tsx @@ -1,6 +1,7 @@ import type { FC } from 'react' import type { TimeOptionsProps } from '../types' import * as React from 'react' +import OptionList from '../common/option-list' import OptionListItem from '../common/option-list-item' import { useTimeOptions } from '../hooks' @@ -16,7 +17,7 @@ const Options: FC = ({ return (
    {/* Hour */} -
      + { hourOptions.map((hour) => { const isSelected = selectedTime?.format('hh') === hour @@ -31,9 +32,9 @@ const Options: FC = ({ ) }) } -
    + {/* Minute */} -
      + { (minuteFilter ? minuteFilter(minuteOptions) : minuteOptions).map((minute) => { const isSelected = selectedTime?.format('mm') === minute @@ -48,9 +49,9 @@ const Options: FC = ({ ) }) } -
    + {/* Period */} -
      + { periodOptions.map((period) => { const isSelected = selectedTime?.format('A') === period @@ -66,7 +67,7 @@ const Options: FC = ({ ) }) } -
    +
    ) } diff --git a/web/app/components/base/date-and-time-picker/types.ts b/web/app/components/base/date-and-time-picker/types.ts index 2773fb7bc7..7dda1d013c 100644 --- a/web/app/components/base/date-and-time-picker/types.ts +++ b/web/app/components/base/date-and-time-picker/types.ts @@ -1,4 +1,4 @@ -import type { Placement } from '@floating-ui/react' +import type { Placement } from '@langgenius/dify-ui/popover' import type { Dayjs } from 'dayjs' export enum ViewType { diff --git a/web/app/components/base/date-and-time-picker/year-and-month-picker/options.tsx b/web/app/components/base/date-and-time-picker/year-and-month-picker/options.tsx index 2288925579..2e162472f4 100644 --- a/web/app/components/base/date-and-time-picker/year-and-month-picker/options.tsx +++ b/web/app/components/base/date-and-time-picker/year-and-month-picker/options.tsx @@ -1,6 +1,7 @@ import type { FC } from 'react' import type { YearAndMonthPickerOptionsProps } from '../types' import * as React from 'react' +import OptionList from '../common/option-list' import OptionListItem from '../common/option-list-item' import { useMonths, useYearOptions } from '../hooks' @@ -16,7 +17,7 @@ const Options: FC = ({ return (
    {/* Month Picker */} -
      + { months.map((month, index) => { const isSelected = selectedMonth === index @@ -31,9 +32,9 @@ const Options: FC = ({ ) }) } -
    + {/* Year Picker */} -
      + { yearOptions.map((year) => { const isSelected = selectedYear === year @@ -48,7 +49,7 @@ const Options: FC = ({ ) }) } -
    +
    ) } diff --git a/web/app/components/datasets/create/step-two/language-select/index.tsx b/web/app/components/datasets/create/step-two/language-select/index.tsx index fdef23ff27..bd1eee3df6 100644 --- a/web/app/components/datasets/create/step-two/language-select/index.tsx +++ b/web/app/components/datasets/create/step-two/language-select/index.tsx @@ -42,7 +42,6 @@ const LanguageSelect: FC = ({ placement="bottom-start" sideOffset={4} popupClassName="w-max" - listClassName="no-scrollbar" > {supportedLanguages.map(({ prompt_name }) => ( diff --git a/web/app/components/header/account-setting/model-provider-page/model-selector/__tests__/popup.spec.tsx b/web/app/components/header/account-setting/model-provider-page/model-selector/__tests__/popup.spec.tsx index 318b5bcd73..42232a71c0 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-selector/__tests__/popup.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-selector/__tests__/popup.spec.tsx @@ -55,7 +55,14 @@ vi.mock('../../hooks', async () => { }) vi.mock('../popup-item', () => ({ - default: ({ model }: { model: Model }) =>
    {model.provider}
    , + default: ({ model }: { model: Model }) => ( +
    + {model.provider} + {model.models.map(modelItem => ( + {modelItem.model} + ))} +
    + ), })) vi.mock('@/context/provider-context', () => ({ @@ -207,6 +214,156 @@ describe('Popup', () => { expect((input as HTMLInputElement).value).toBe('') }) + it('should show matching models when searching by model name', () => { + renderPopup( + , + ) + + fireEvent.change( + screen.getByPlaceholderText('datasetSettings.form.searchModel'), + { target: { value: 'claude' } }, + ) + + expect(screen.queryByText('openai')).not.toBeInTheDocument() + expect(screen.getByText('anthropic')).toBeInTheDocument() + expect(screen.getByText('claude-3')).toBeInTheDocument() + expect(screen.queryByText('gpt-4')).not.toBeInTheDocument() + expect(screen.queryByText('No model found for \u201Cclaude\u201D')).not.toBeInTheDocument() + }) + + it('should show empty search placeholder when no provider or model name matches', () => { + renderPopup( + , + ) + + fireEvent.change( + screen.getByPlaceholderText('datasetSettings.form.searchModel'), + { target: { value: 'mistral' } }, + ) + + expect(screen.getByText('No model found for \u201Cmistral\u201D'))!.toBeInTheDocument() + expect(screen.queryByText('openai')).not.toBeInTheDocument() + expect(screen.queryByText('gpt-4')).not.toBeInTheDocument() + }) + + it('should show all models of a provider when searching by provider label', () => { + renderPopup( + , + ) + + fireEvent.change( + screen.getByPlaceholderText('datasetSettings.form.searchModel'), + { target: { value: 'openai' } }, + ) + + expect(screen.getByText('openai'))!.toBeInTheDocument() + expect(screen.getByText('gpt-4'))!.toBeInTheDocument() + expect(screen.getByText('gpt-4o'))!.toBeInTheDocument() + expect(screen.queryByText('anthropic')).not.toBeInTheDocument() + expect(screen.queryByText('claude-3')).not.toBeInTheDocument() + }) + + it('should match by model provider key when model label does not contain the search text', () => { + renderPopup( + , + ) + + fireEvent.change( + screen.getByPlaceholderText('datasetSettings.form.searchModel'), + { target: { value: 'openai' } }, + ) + + expect(screen.getByText('azure_openai'))!.toBeInTheDocument() + expect(screen.getByText('gpt-4'))!.toBeInTheDocument() + }) + + it('should still apply scope features when matching by provider label', () => { + mockSupportFunctionCall.mockReturnValue(false) + + renderPopup( + , + ) + + fireEvent.change( + screen.getByPlaceholderText('datasetSettings.form.searchModel'), + { target: { value: 'openai' } }, + ) + + expect(screen.getByText('No model found for \u201Copenai\u201D'))!.toBeInTheDocument() + expect(screen.queryByText('gpt-4')).not.toBeInTheDocument() + expect(screen.queryByText('gpt-4-tool')).not.toBeInTheDocument() + }) + it('should not show compatible-only helper text when no scope features are applied', () => { renderPopup( { expect(screen.queryByText('common.modelProvider.selector.onlyCompatibleModelsShown')).not.toBeInTheDocument() }) - it('should show compatible-only helper banner when scope features are applied', () => { - const { container } = renderPopup( + it('should show compatible-only helper text when scope features are applied', () => { + renderPopup( { expect(screen.getByTestId('compatible-models-banner'))!.toBeInTheDocument() expect(screen.getByText('common.modelProvider.selector.onlyCompatibleModelsShown'))!.toBeInTheDocument() - expect(container.querySelector('.i-ri-information-2-fill'))!.toBeInTheDocument() + }) + + it('should keep search and footer outside the scrollable model list', () => { + renderPopup( + , + ) + + const scrollRegion = screen.getByRole('region', { name: 'common.modelProvider.models' }) + const searchInput = screen.getByPlaceholderText('datasetSettings.form.searchModel') + const settingsButton = screen.getByRole('button', { name: /common\.modelProvider\.selector\.modelProviderSettings/ }) + + expect(scrollRegion)!.toBeInTheDocument() + expect(scrollRegion).not.toContainElement(searchInput) + expect(scrollRegion).not.toContainElement(settingsButton) + expect(scrollRegion).toContainElement(screen.getByTestId('compatible-models-banner')) }) it('should filter by scope features including toolCall and non-toolCall checks', () => { diff --git a/web/app/components/header/account-setting/model-provider-page/model-selector/index.tsx b/web/app/components/header/account-setting/model-provider-page/model-selector/index.tsx index 9241c592f5..835821fd59 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-selector/index.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-selector/index.tsx @@ -88,7 +88,7 @@ const ModelSelector: FC = ({ placement="bottom-start" sideOffset={4} className={popupClassName} - popupClassName="overflow-hidden rounded-lg" + popupClassName="overflow-hidden rounded-xl" popupProps={{ style: { minWidth: '320px', width: 'var(--anchor-width, auto)' } }} > void + onInstallPlugin: (key: ModelProviderQuotaGetPaid) => void | Promise +} + +const MarketplaceSection: FC = ({ + marketplaceProviders, + marketplaceCollapsed, + installingProvider, + isMarketplacePluginsLoading, + theme, + onMarketplaceCollapsedChange, + onInstallPlugin, +}) => { + const { t } = useTranslation() + + if (marketplaceProviders.length === 0) + return null + + return ( + <> +
    +
    +
    +
    +
    +
    onMarketplaceCollapsedChange(!marketplaceCollapsed)} + > + {t('modelProvider.selector.fromMarketplace', { ns: 'common' })} + +
    +
    + {!marketplaceCollapsed && ( +
    + {marketplaceProviders.map((key) => { + const Icon = providerIconMap[key] + const isInstalling = installingProvider === key + return ( +
    +
    + + {modelNameMap[key]} +
    + +
    + ) + })} + + + {t('modelProvider.selector.discoverMoreInMarketplace', { ns: 'common' })} + + + +
    + )} +
    + + ) +} + +export default MarketplaceSection diff --git a/web/app/components/header/account-setting/model-provider-page/model-selector/popup-empty-state.tsx b/web/app/components/header/account-setting/model-provider-page/model-selector/popup-empty-state.tsx new file mode 100644 index 0000000000..dafd26387b --- /dev/null +++ b/web/app/components/header/account-setting/model-provider-page/model-selector/popup-empty-state.tsx @@ -0,0 +1,39 @@ +import type { FC } from 'react' +import { Button } from '@langgenius/dify-ui/button' +import { useTranslation } from 'react-i18next' + +type ModelSelectorEmptyStateProps = { + onConfigure: () => void +} + +const ModelSelectorEmptyState: FC = ({ + onConfigure, +}) => { + const { t } = useTranslation() + + return ( +
    +
    + +
    +
    +

    + {t('modelProvider.selector.noProviderConfigured', { ns: 'common' })} +

    +

    + {t('modelProvider.selector.noProviderConfiguredDesc', { ns: 'common' })} +

    +
    + +
    + ) +} + +export default ModelSelectorEmptyState diff --git a/web/app/components/header/account-setting/model-provider-page/model-selector/popup-item.tsx b/web/app/components/header/account-setting/model-provider-page/model-selector/popup-item.tsx index 72c52a9429..ff9e6575bb 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-selector/popup-item.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-selector/popup-item.tsx @@ -107,7 +107,8 @@ const PopupItem: FC = ({ return (
    -
    + {/* Keep the sticky provider header above model rows while the list scrolls. */} +
    setCollapsed(prev => !prev)} diff --git a/web/app/components/header/account-setting/model-provider-page/model-selector/popup-layout.tsx b/web/app/components/header/account-setting/model-provider-page/model-selector/popup-layout.tsx new file mode 100644 index 0000000000..50bd098af1 --- /dev/null +++ b/web/app/components/header/account-setting/model-provider-page/model-selector/popup-layout.tsx @@ -0,0 +1,130 @@ +import type { FC, ReactNode } from 'react' +import { + ScrollAreaContent, + ScrollAreaRoot, + ScrollAreaScrollbar, + ScrollAreaThumb, + ScrollAreaViewport, +} from '@langgenius/dify-ui/scroll-area' +import { useTranslation } from 'react-i18next' + +type ModelSelectorPopupFrameProps = { + children: ReactNode +} + +export const ModelSelectorPopupFrame: FC = ({ + children, +}) => { + return ( +
    + {children} +
    + ) +} + +type ModelSelectorSearchHeaderProps = { + searchText: string + onSearchTextChange: (value: string) => void +} + +export const ModelSelectorSearchHeader: FC = ({ + searchText, + onSearchTextChange, +}) => { + const { t } = useTranslation() + + return ( +
    +
    + + onSearchTextChange(e.target.value)} + /> + { + searchText && ( + onSearchTextChange('')} + /> + ) + } +
    +
    + ) +} + +type ModelSelectorScrollBodyProps = { + children: ReactNode + label: string +} + +export const ModelSelectorScrollBody: FC = ({ + children, + label, +}) => { + return ( + + + + {children} + + + {/* Keep the overlay scrollbar above sticky provider headers inside this scroll area. */} + + + + + ) +} + +export const CompatibleModelsNotice = () => { + const { t } = useTranslation() + + return ( +
    + {t('modelProvider.selector.onlyCompatibleModelsShown', { ns: 'common' })} +
    + ) +} + +type ModelProviderSettingsFooterProps = { + onOpenSettings: () => void +} + +export const ModelProviderSettingsFooter: FC = ({ + onOpenSettings, +}) => { + const { t } = useTranslation() + + return ( +
    + +
    + ) +} diff --git a/web/app/components/header/account-setting/model-provider-page/model-selector/popup.tsx b/web/app/components/header/account-setting/model-provider-page/model-selector/popup.tsx index 47ddb55b6c..86bac84310 100644 --- a/web/app/components/header/account-setting/model-provider-page/model-selector/popup.tsx +++ b/web/app/components/header/account-setting/model-provider-page/model-selector/popup.tsx @@ -5,8 +5,6 @@ import type { ModelItem, } from '../declarations' import type { ModelProviderQuotaGetPaid } from '@/types/model-provider' -import { Button } from '@langgenius/dify-ui/button' -import { cn } from '@langgenius/dify-ui/cn' import { useSuspenseQuery } from '@tanstack/react-query' import { useTheme } from 'next-themes' import { useCallback, useMemo, useState } from 'react' @@ -19,7 +17,6 @@ import { useProviderContext } from '@/context/provider-context' import { systemFeaturesQueryOptions } from '@/service/system-features' import { useInstallPackageFromMarketPlace } from '@/service/use-plugins' import { supportFunctionCall } from '@/utils/tool-call' -import { getMarketplaceUrl } from '@/utils/var' import { CustomConfigurationStatusEnum, ModelFeatureEnum, @@ -29,8 +26,17 @@ import { useLanguage, useMarketplaceAllPlugins } from '../hooks' import CreditsExhaustedAlert from '../provider-added-card/model-auth-dropdown/credits-exhausted-alert' import { useTrialCredits } from '../provider-added-card/use-trial-credits' import { providerSupportsCredits } from '../supports-credits' -import { MODEL_PROVIDER_QUOTA_GET_PAID, modelNameMap, providerIconMap, providerKeyToPluginId } from '../utils' +import { MODEL_PROVIDER_QUOTA_GET_PAID, providerKeyToPluginId } from '../utils' +import MarketplaceSection from './marketplace-section' +import ModelSelectorEmptyState from './popup-empty-state' import PopupItem from './popup-item' +import { + CompatibleModelsNotice, + ModelProviderSettingsFooter, + ModelSelectorPopupFrame, + ModelSelectorScrollBody, + ModelSelectorSearchHeader, +} from './popup-layout' type PopupProps = { defaultModel?: DefaultModel @@ -137,18 +143,26 @@ const Popup: FC = ({ }, [aiCreditVisibleProviders, installedProviderMap, modelList]) const filteredModelList = useMemo(() => { + const normalizedSearch = searchText.toLowerCase() + const matchesLabel = (label: Record) => { + if (label[language] !== undefined) + return label[language].toLowerCase().includes(normalizedSearch) + return Object.values(label).some(value => + value.toLowerCase().includes(normalizedSearch), + ) + } + const filtered = installedModelList.map((model) => { - const matchesProviderSearch = !searchText - || model.provider.toLowerCase().includes(searchText.toLowerCase()) - || Object.values(model.label).some(label => label.toLowerCase().includes(searchText.toLowerCase())) + const providerMatched = !!searchText && ( + matchesLabel(model.label) + || model.provider.toLowerCase().includes(normalizedSearch) + ) const filteredModels = model.models .filter((modelItem) => { - if (modelItem.label[language] !== undefined) - return modelItem.label[language].toLowerCase().includes(searchText.toLowerCase()) - return Object.values(modelItem.label).some(label => - label.toLowerCase().includes(searchText.toLowerCase()), - ) + if (!searchText || providerMatched) + return true + return matchesLabel(modelItem.label) }) .filter((modelItem) => { if (scopeFeatures.length === 0) @@ -159,8 +173,12 @@ const Popup: FC = ({ return modelItem.features?.includes(feature) ?? false }) }) - if (!matchesProviderSearch || (filteredModels.length === 0 && !aiCreditVisibleProviders.has(model.provider))) + if ( + (searchText && filteredModels.length === 0) + || (!searchText && filteredModels.length === 0 && !aiCreditVisibleProviders.has(model.provider)) + ) { return null + } return { ...model, models: filteredModels } }).filter((model): model is Model => model !== null) @@ -181,166 +199,59 @@ const Popup: FC = ({ return MODEL_PROVIDER_QUOTA_GET_PAID.filter(key => !installedProviders.has(key)) }, [modelProviders]) + const handleOpenSettings = useCallback(() => { + onHide() + setShowAccountSettingModal({ payload: ACCOUNT_SETTING_TAB.PROVIDER }) + }, [onHide, setShowAccountSettingModal]) + return ( -
    -
    -
    - - setSearchText(e.target.value)} - /> - { - searchText && ( - setSearchText('')} - /> - ) - } -
    - {scopeFeatures.length > 0 && ( -
    - -

    - {t('modelProvider.selector.onlyCompatibleModelsShown', { ns: 'common' })} -

    -
    - )} -
    + + {showCreditsExhaustedAlert && ( )} -
    - { - filteredModelList.map(model => ( - +
    + { + filteredModelList.map(model => ( + + )) + } + {!filteredModelList.length && !installedModelList.length && ( + - )) - } - {!filteredModelList.length && !installedModelList.length && ( -
    -
    - + )} + {!filteredModelList.length && installedModelList.length > 0 && ( +
    + {`No model found for \u201C${searchText}\u201D`}
    -
    -

    - {t('modelProvider.selector.noProviderConfigured', { ns: 'common' })} -

    -

    - {t('modelProvider.selector.noProviderConfiguredDesc', { ns: 'common' })} -

    -
    - -
    - )} - {!filteredModelList.length && installedModelList.length > 0 && ( -
    - {`No model found for \u201C${searchText}\u201D`} -
    - )} - {marketplaceProviders.length > 0 && ( - <> -
    -
    -
    -
    setMarketplaceCollapsed(prev => !prev)} - > - {t('modelProvider.selector.fromMarketplace', { ns: 'common' })} - -
    -
    - {!marketplaceCollapsed && ( - <> - {marketplaceProviders.map((key) => { - const Icon = providerIconMap[key] - const isInstalling = installingProvider === key - return ( -
    -
    - - {modelNameMap[key]} -
    - -
    - ) - })} - - - {t('modelProvider.selector.discoverMoreInMarketplace', { ns: 'common' })} - - - - - )} -
    - - )} -
    -
    { - onHide() - setShowAccountSettingModal({ payload: ACCOUNT_SETTING_TAB.PROVIDER }) - }} - > - - {t('modelProvider.selector.modelProviderSettings', { ns: 'common' })} -
    -
    + )} + {scopeFeatures.length > 0 && ( + + )} + +
    + + + ) } diff --git a/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-section.tsx b/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-section.tsx index 0d0c793741..7533d06d5f 100644 --- a/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-section.tsx +++ b/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-section.tsx @@ -1,6 +1,7 @@ import type { FC, ReactNode } from 'react' import type { PluginStatus } from '@/app/components/plugins/types' import type { Locale } from '@/i18n-config' +import { ScrollArea } from '@langgenius/dify-ui/scroll-area' import PluginItem from './plugin-item' type PluginSectionProps = { @@ -43,7 +44,14 @@ const PluginSection: FC = ({ ) {headerAction}
    -
    + {plugins.map(plugin => ( = ({ : undefined} /> ))} -
    + ) } diff --git a/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-task-list.tsx b/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-task-list.tsx index 24fcf85cde..cc2eed1dbb 100644 --- a/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-task-list.tsx +++ b/web/app/components/plugins/plugin-page/plugin-tasks/components/plugin-task-list.tsx @@ -1,6 +1,7 @@ import type { FC } from 'react' import type { PluginStatus } from '@/app/components/plugins/types' import { Button } from '@langgenius/dify-ui/button' +import { ScrollArea } from '@langgenius/dify-ui/scroll-area' import { useTranslation } from 'react-i18next' import { useGetLanguage } from '@/context/i18n' import ErrorPluginItem from './error-plugin-item' @@ -86,7 +87,14 @@ const PluginTaskList: FC = ({ {t('task.clearAll', { ns: 'plugin' })}
    -
    + {errorPlugins.map(plugin => ( = ({ onClear={() => onClearSingle(plugin.taskId, plugin.plugin_unique_identifier)} /> ))} -
    + )}
    diff --git a/web/app/components/plugins/plugin-page/plugin-tasks/index.tsx b/web/app/components/plugins/plugin-page/plugin-tasks/index.tsx index f3102c4909..00fcb7e072 100644 --- a/web/app/components/plugins/plugin-page/plugin-tasks/index.tsx +++ b/web/app/components/plugins/plugin-page/plugin-tasks/index.tsx @@ -117,7 +117,7 @@ const PluginTasks = () => { { }) describe('StrategyPicker (strategy-picker.tsx)', () => { - const defaultProps = { - value: AUTO_UPDATE_STRATEGY.disabled, - onChange: vi.fn(), + const i18nKeyByStrategy: Record = { + [AUTO_UPDATE_STRATEGY.disabled]: 'disabled', + [AUTO_UPDATE_STRATEGY.fixOnly]: 'fixOnly', + [AUTO_UPDATE_STRATEGY.latest]: 'latest', + } + + const triggerName = (strategy: AUTO_UPDATE_STRATEGY) => + new RegExp(`plugin\\.autoUpdate\\.strategy\\.${i18nKeyByStrategy[strategy]}\\.name`, 'i') + + const findOption = async (key: 'disabled' | 'fixOnly' | 'latest') => { + const options = await screen.findAllByRole('menuitemradio') + const option = options.find(item => + item.textContent?.includes(`plugin.autoUpdate.strategy.${key}.name`), + ) + if (!option) + throw new Error(`Strategy option "${key}" not found`) + return option } describe('Rendering', () => { it('should render trigger button with current strategy label', () => { - // Act - render() + render() - // Assert - expect(screen.getByRole('button', { name: /plugin\.autoUpdate\.strategy\.disabled\.name/i })).toBeInTheDocument() + expect(screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.disabled) })).toBeInTheDocument() }) it('should not render dropdown content when closed', () => { - // Act - render() + render() - // Assert - expect(screen.queryByTestId('portal-content')).not.toBeInTheDocument() + expect(screen.queryByRole('menu')).not.toBeInTheDocument() }) - it('should render all strategy options when open', () => { - // Arrange - mockPortalOpen = true + it('should render all strategy options when open', async () => { + const user = userEvent.setup() + render() - // Act - render() - fireEvent.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.disabled) })) - // Wait for portal to open - if (mockPortalOpen) { - // Assert all options visible (use getAllByText for strategy name as it appears in both trigger and dropdown) - expect(screen.getAllByText('plugin.autoUpdate.strategy.disabled.name').length).toBeGreaterThanOrEqual(1) - expect(screen.getByText('plugin.autoUpdate.strategy.fixOnly.name')).toBeInTheDocument() - expect(screen.getByText('plugin.autoUpdate.strategy.latest.name')).toBeInTheDocument() - } + const options = await screen.findAllByRole('menuitemradio') + expect(options).toHaveLength(3) + expect(options.some(o => o.textContent?.includes('plugin.autoUpdate.strategy.disabled.name'))).toBe(true) + expect(options.some(o => o.textContent?.includes('plugin.autoUpdate.strategy.fixOnly.name'))).toBe(true) + expect(options.some(o => o.textContent?.includes('plugin.autoUpdate.strategy.latest.name'))).toBe(true) }) }) describe('User Interactions', () => { - it('should toggle dropdown when trigger is clicked', () => { - // Act - render() - - // Assert - initially closed - expect(mockPortalOpen).toBe(false) - - // Act - click trigger - fireEvent.click(screen.getByTestId('portal-trigger')) - - // Assert - portal trigger element should still be in document - expect(screen.getByTestId('portal-trigger')).toBeInTheDocument() - }) - - it('should call onChange with fixOnly when Bug Fixes Only option is clicked', () => { - // Arrange - force portal content to be visible for testing option selection - forcePortalContentVisible = true - const onChange = vi.fn() - - // Act - render() - - // Find and click the "Bug Fixes Only" option - const fixOnlyOption = screen.getByText('plugin.autoUpdate.strategy.fixOnly.name').closest('div[class*="cursor-pointer"]') - expect(fixOnlyOption).toBeInTheDocument() - fireEvent.click(fixOnlyOption!) - - // Assert - expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.fixOnly) - }) - - it('should call onChange with latest when Latest Version option is clicked', () => { - // Arrange - force portal content to be visible for testing option selection - forcePortalContentVisible = true - const onChange = vi.fn() - - // Act - render() - - // Find and click the "Latest Version" option - const latestOption = screen.getByText('plugin.autoUpdate.strategy.latest.name').closest('div[class*="cursor-pointer"]') - expect(latestOption).toBeInTheDocument() - fireEvent.click(latestOption!) - - // Assert - expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.latest) - }) - - it('should call onChange with disabled when Disabled option is clicked', () => { - // Arrange - force portal content to be visible for testing option selection - forcePortalContentVisible = true - const onChange = vi.fn() - - // Act - render() - - // Find and click the "Disabled" option - need to find the one in the dropdown, not the button - const disabledOptions = screen.getAllByText('plugin.autoUpdate.strategy.disabled.name') - // The second one should be in the dropdown - const dropdownOption = disabledOptions.find(el => el.closest('div[class*="cursor-pointer"]')) - expect(dropdownOption).toBeInTheDocument() - fireEvent.click(dropdownOption!.closest('div[class*="cursor-pointer"]')!) - - // Assert - expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.disabled) - }) - - it('should stop event propagation when option is clicked', () => { - // Arrange - force portal content to be visible - forcePortalContentVisible = true - const onChange = vi.fn() - const parentClickHandler = vi.fn() - - // Act - render( -
    - -
    , - ) - - // Click an option - const fixOnlyOption = screen.getByText('plugin.autoUpdate.strategy.fixOnly.name').closest('div[class*="cursor-pointer"]') - fireEvent.click(fixOnlyOption!) - - // Assert - onChange is called but parent click handler should not propagate - expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.fixOnly) - }) - - it('should render check icon for currently selected option', () => { - // Arrange - force portal content to be visible - forcePortalContentVisible = true - - // Act - render with fixOnly selected - render() - - // Assert - RiCheckLine should be rendered (check icon) - // Find all "Bug Fixes Only" texts and get the one in the dropdown (has cursor-pointer parent) - const allFixOnlyTexts = screen.getAllByText('plugin.autoUpdate.strategy.fixOnly.name') - const dropdownOption = allFixOnlyTexts.find(el => el.closest('div[class*="cursor-pointer"]')) - const optionContainer = dropdownOption?.closest('div[class*="cursor-pointer"]') - expect(optionContainer).toBeInTheDocument() - // The check icon SVG should exist within the option - expect(optionContainer?.querySelector('svg')).toBeInTheDocument() - }) - - it('should not render check icon for non-selected options', () => { - // Arrange - force portal content to be visible - forcePortalContentVisible = true - - // Act - render with disabled selected + it('should open and close the menu when the trigger is clicked', async () => { + const user = userEvent.setup() render() - // Assert - check the Latest Version option should not have check icon - const latestOption = screen.getByText('plugin.autoUpdate.strategy.latest.name').closest('div[class*="cursor-pointer"]') - // The svg should only be in selected option, not in non-selected - const checkIconContainer = latestOption?.querySelector('div.mr-1') - // Non-selected option should have empty check icon container - expect(checkIconContainer?.querySelector('svg')).toBeNull() + const trigger = screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.disabled) }) + expect(screen.queryByRole('menu')).not.toBeInTheDocument() + + await user.click(trigger) + expect(await screen.findByRole('menu')).toBeInTheDocument() + }) + + it.each<[AUTO_UPDATE_STRATEGY, 'disabled' | 'fixOnly' | 'latest', AUTO_UPDATE_STRATEGY]>([ + [AUTO_UPDATE_STRATEGY.disabled, 'fixOnly', AUTO_UPDATE_STRATEGY.fixOnly], + [AUTO_UPDATE_STRATEGY.disabled, 'latest', AUTO_UPDATE_STRATEGY.latest], + [AUTO_UPDATE_STRATEGY.fixOnly, 'disabled', AUTO_UPDATE_STRATEGY.disabled], + ])('should call onChange with %s -> %s when option is selected', async (initial, optionKey, expected) => { + const user = userEvent.setup() + const onChange = vi.fn() + render() + + await user.click(screen.getByRole('button', { name: triggerName(initial) })) + await user.click(await findOption(optionKey)) + + expect(onChange).toHaveBeenCalledWith(expected) + }) + + it('should mark only the currently selected option with aria-checked', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.fixOnly) })) + + const options = await screen.findAllByRole('menuitemradio') + const checked = options.filter(o => o.getAttribute('aria-checked') === 'true') + + expect(checked).toHaveLength(1) + expect(checked[0]).toHaveTextContent('plugin.autoUpdate.strategy.fixOnly.name') + }) + + it('should render the check indicator inside the selected option only', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button', { name: triggerName(AUTO_UPDATE_STRATEGY.fixOnly) })) + + const fixOnlyOption = await findOption('fixOnly') + const latestOption = await findOption('latest') + + expect(fixOnlyOption.querySelector('.i-ri-check-line')).toBeInTheDocument() + expect(latestOption.querySelector('.i-ri-check-line')).toBeNull() }) }) }) @@ -1280,7 +1219,9 @@ describe('auto-update-setting', () => { render() // Assert - expect(screen.getByTestId('portal-elem')).toBeInTheDocument() + expect( + screen.getByRole('button', { name: /plugin\.autoUpdate\.strategy\.fixOnly\.name/i }), + ).toBeInTheDocument() }) it('should show time picker when strategy is not disabled', () => { @@ -1407,16 +1348,27 @@ describe('auto-update-setting', () => { }) describe('User Interactions', () => { - it('should call onChange with updated strategy when strategy changes', () => { + it('should call onChange with updated strategy when strategy changes', async () => { // Arrange + const user = userEvent.setup() const onChange = vi.fn() - const payload = createMockAutoUpdateConfig() + const payload = createMockAutoUpdateConfig({ strategy_setting: AUTO_UPDATE_STRATEGY.fixOnly }) // Act render() - // Assert - component renders with strategy picker - expect(screen.getByTestId('portal-elem')).toBeInTheDocument() + await user.click( + screen.getByRole('button', { name: /plugin\.autoUpdate\.strategy\.fixOnly\.name/i }), + ) + const latestOption = (await screen.findAllByRole('menuitemradio')).find(item => + item.textContent?.includes('plugin.autoUpdate.strategy.latest.name'), + )! + await user.click(latestOption) + + // Assert + expect(onChange).toHaveBeenCalledWith( + expect.objectContaining({ strategy_setting: AUTO_UPDATE_STRATEGY.latest }), + ) }) it('should call onChange with updated time when time changes', () => { diff --git a/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/strategy-picker.spec.tsx b/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/strategy-picker.spec.tsx index e287e48985..9fe089c34e 100644 --- a/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/strategy-picker.spec.tsx +++ b/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/strategy-picker.spec.tsx @@ -1,62 +1,12 @@ -import { fireEvent, render, screen } from '@testing-library/react' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { describe, expect, it, vi } from 'vitest' import StrategyPicker from '../strategy-picker' import { AUTO_UPDATE_STRATEGY } from '../types' -let portalOpen = false - -vi.mock('@langgenius/dify-ui/button', () => ({ - Button: ({ - children, - }: { - children: React.ReactNode - }) => {children}, -})) - -vi.mock('@/app/components/base/portal-to-follow-elem', async () => { - const _React = await import('react') - return { - PortalToFollowElem: ({ - open, - children, - }: { - open: boolean - children: React.ReactNode - }) => { - portalOpen = open - return
    {children}
    - }, - PortalToFollowElemTrigger: ({ - children, - onClick, - }: { - children: React.ReactNode - onClick: (event: { stopPropagation: () => void, nativeEvent: { stopImmediatePropagation: () => void } }) => void - }) => ( - - ), - PortalToFollowElemContent: ({ - children, - }: { - children: React.ReactNode - }) => portalOpen ?
    {children}
    : null, - } -}) +const triggerName = (key: string) => new RegExp(`plugin\\.autoUpdate\\.strategy\\.${key}\\.name`, 'i') describe('StrategyPicker', () => { - beforeEach(() => { - vi.clearAllMocks() - portalOpen = false - }) - it('renders the selected strategy label in the trigger', () => { render( { />, ) - expect(screen.getByTestId('trigger')).toHaveTextContent('plugin.autoUpdate.strategy.fixOnly.name') + expect(screen.getByRole('button', { name: triggerName('fixOnly') })).toBeInTheDocument() + expect(screen.queryByRole('menu')).not.toBeInTheDocument() }) - it('opens the option list when the trigger is clicked', () => { + it('opens the option list when the trigger is clicked', async () => { + const user = userEvent.setup() render( { />, ) - fireEvent.click(screen.getByTestId('trigger')) + await user.click(screen.getByRole('button', { name: triggerName('disabled') })) - expect(screen.getByTestId('portal-content')).toBeInTheDocument() - expect(screen.getByTestId('portal-content').querySelectorAll('svg')).toHaveLength(1) + const options = await screen.findAllByRole('menuitemradio') + expect(options).toHaveLength(3) expect(screen.getByText('plugin.autoUpdate.strategy.latest.description')).toBeInTheDocument() }) - it('calls onChange when a new strategy is selected', () => { + it('marks only the currently selected strategy as checked', async () => { + const user = userEvent.setup() + render( + , + ) + + await user.click(screen.getByRole('button', { name: triggerName('fixOnly') })) + + const checkedOptions = (await screen.findAllByRole('menuitemradio')) + .filter(item => item.getAttribute('aria-checked') === 'true') + + expect(checkedOptions).toHaveLength(1) + expect(checkedOptions[0]).toHaveTextContent('plugin.autoUpdate.strategy.fixOnly.name') + }) + + it('calls onChange and closes the menu when a new strategy is selected', async () => { + const user = userEvent.setup() const onChange = vi.fn() render( { />, ) - fireEvent.click(screen.getByTestId('trigger')) - fireEvent.click(screen.getByText('plugin.autoUpdate.strategy.latest.name')) + await user.click(screen.getByRole('button', { name: triggerName('disabled') })) + const latestOption = (await screen.findAllByRole('menuitemradio')) + .find(item => item.textContent?.includes('plugin.autoUpdate.strategy.latest.name'))! + await user.click(latestOption) expect(onChange).toHaveBeenCalledWith(AUTO_UPDATE_STRATEGY.latest) + expect(await screen.findByRole('button', { name: triggerName('disabled') })).toBeInTheDocument() }) }) diff --git a/web/app/components/plugins/reference-setting-modal/auto-update-setting/index.tsx b/web/app/components/plugins/reference-setting-modal/auto-update-setting/index.tsx index dc5d376eb3..d7d6fcd35f 100644 --- a/web/app/components/plugins/reference-setting-modal/auto-update-setting/index.tsx +++ b/web/app/components/plugins/reference-setting-modal/auto-update-setting/index.tsx @@ -105,7 +105,7 @@ const AutoUpdateSetting: FC = ({ const renderTimePickerTrigger = useCallback(({ inputElem, onClick, isOpen }: TriggerParams) => { return (
    @@ -137,7 +137,7 @@ const AutoUpdateSetting: FC = ({ <>