From d1391c2ed525279f92525a0455b257bc7fa1552e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9D=9E=E6=B3=95=E6=93=8D=E4=BD=9C?= Date: Wed, 23 Sep 2026 09:24:22 +0000 Subject: [PATCH] refactor(api): isolate installed app management (#41957) --- api/.importlinter | 14 + api/controllers/console/explore/error.py | 34 + .../console/explore/installed_app.py | 220 +-- .../explore/installed_app_admission.py | 34 +- api/controllers/console/wraps.py | 4 + api/extensions/ext_application_services.py | 67 +- api/openapi/markdown/console-openapi.md | 39 - api/repositories/installed_app_repository.py | 138 +- .../workspace_query_repository.py | 11 + api/services/installed_app_access_service.py | 35 +- api/services/installed_app_service.py | 235 ++- api/services/webapp_access_query_service.py | 35 +- .../console/explore/test_completion.py | 9 +- .../console/explore/test_installed_app.py | 1281 ++++++++++------- .../explore/test_installed_app_admission.py | 51 +- .../explore/test_installed_app_completion.py | 13 +- .../console/explore/test_workflow.py | 9 +- .../test_ext_application_services.py | 366 ++++- .../test_installed_app_repository.py | 260 +++- .../test_installed_app_access_service.py | 176 +++ .../test_webapp_access_query_service.py | 120 ++ .../api/console/installed-apps/orpc.gen.ts | 29 - .../api/console/installed-apps/types.gen.ts | 39 - .../api/console/installed-apps/zod.gen.ts | 32 - .../apps/__tests__/app-card.spec.tsx | 1 - .../__tests__/hooks.spec.tsx | 1 - .../__tests__/app-nav-item.spec.tsx | 35 - .../installed-app-navigation/app-nav-item.tsx | 5 - .../installed-app/__tests__/index.spec.tsx | 1 - .../item-operation/__tests__/index.spec.tsx | 70 +- .../explore/item-operation/index.tsx | 28 - .../explore/item-operation/style.module.css | 4 - .../main-nav/__tests__/index.spec.tsx | 26 +- .../web-apps-section.browser.spec.tsx | 4 - .../__tests__/web-apps-section.spec.tsx | 79 +- .../main-nav/components/web-apps-section.tsx | 77 +- web/i18n/locales/ar-TN/explore.json | 2 - web/i18n/locales/az-AZ/explore.json | 2 - web/i18n/locales/de-DE/explore.json | 2 - web/i18n/locales/en-US/explore.json | 2 - web/i18n/locales/es-ES/explore.json | 2 - web/i18n/locales/fa-IR/explore.json | 2 - web/i18n/locales/fr-FR/explore.json | 2 - web/i18n/locales/hi-IN/explore.json | 2 - web/i18n/locales/id-ID/explore.json | 2 - web/i18n/locales/it-IT/explore.json | 2 - web/i18n/locales/ja-JP/explore.json | 2 - web/i18n/locales/ko-KR/explore.json | 2 - web/i18n/locales/lo-LA/explore.json | 2 - web/i18n/locales/nl-NL/explore.json | 2 - web/i18n/locales/pl-PL/explore.json | 2 - web/i18n/locales/pt-BR/explore.json | 2 - web/i18n/locales/ro-RO/explore.json | 2 - web/i18n/locales/ru-RU/explore.json | 2 - web/i18n/locales/sl-SI/explore.json | 2 - web/i18n/locales/th-TH/explore.json | 2 - web/i18n/locales/tr-TR/explore.json | 2 - web/i18n/locales/uk-UA/explore.json | 2 - web/i18n/locales/vi-VN/explore.json | 2 - web/i18n/locales/zh-Hans/explore.json | 2 - web/i18n/locales/zh-Hant/explore.json | 2 - web/service/console/query-policies.ts | 16 - 62 files changed, 2280 insertions(+), 1368 deletions(-) diff --git a/api/.importlinter b/api/.importlinter index 00605db67a4..dfc2451770f 100644 --- a/api/.importlinter +++ b/api/.importlinter @@ -210,6 +210,20 @@ forbidden_modules = sqlalchemy werkzeug +[importlinter:contract:installed-app-management-boundary] +name = Installed app management service is framework and persistence neutral +type = forbidden +source_modules = + services.installed_app_service +forbidden_modules = + controllers + extensions + flask + models + repositories + sqlalchemy + werkzeug + [importlinter:contract:installed-app-generation-boundary] name = Installed app generation service is framework and persistence neutral type = forbidden diff --git a/api/controllers/console/explore/error.py b/api/controllers/console/explore/error.py index c8a52d640b7..d33e301e175 100644 --- a/api/controllers/console/explore/error.py +++ b/api/controllers/console/explore/error.py @@ -1,6 +1,40 @@ +from core.logging.context import get_request_id from libs.exception import BaseHTTPException +class InstalledAppHTTPError(BaseHTTPException): + """A Console error with safe details and a request ID for troubleshooting.""" + + def __init__(self) -> None: + super().__init__() + details: dict[str, object] = {"request_id": get_request_id()} + self.data = {"code": self.error_code, "message": self.description, "status": self.code, "details": details} + + +class InstalledAppNotFoundHTTPError(InstalledAppHTTPError): + error_code = "installed_app_not_found" + description = "The app was not found in this workspace." + code = 404 + + +class InstalledAppUnavailableHTTPError(InstalledAppHTTPError): + error_code = "installed_app_unavailable" + description = "The app is not available in this app library." + code = 404 + + +class InstalledAppInvalidCursorError(InstalledAppHTTPError): + error_code = "invalid_cursor" + description = "The app list cursor is invalid. Refresh the list and try again." + code = 400 + + +class WebAppAccessUnavailableHTTPError(InstalledAppHTTPError): + error_code = "web_app_access_unavailable" + description = "The app access service is unavailable. Try again later." + code = 503 + + class NotCompletionAppError(BaseHTTPException): error_code = "not_completion_app" description = "Not Completion App" diff --git a/api/controllers/console/explore/installed_app.py b/api/controllers/console/explore/installed_app.py index e1b8295a983..d1ab49867a1 100644 --- a/api/controllers/console/explore/installed_app.py +++ b/api/controllers/console/explore/installed_app.py @@ -2,37 +2,34 @@ import base64 import binascii import logging -from flask import request from flask_restx import Resource from pydantic import BaseModel, Field, computed_field -from sqlalchemy import and_, select -from werkzeug.exceptions import BadRequest, Forbidden, NotFound -from controllers.common.fields import SimpleMessageResponse, SimpleResultMessageResponse +from controllers.common.fields import SimpleResultMessageResponse from controllers.common.schema import query_params_from_model, register_response_schema_models, register_schema_models from controllers.console import console_ns -from controllers.console.explore.wraps import InstalledAppResource -from controllers.console.wraps import ( - account_initialization_required, - cloud_edition_billing_resource_check, - model_validate, - with_current_tenant_id, - with_current_user, +from controllers.console.explore.error import ( + InstalledAppInvalidCursorError, + InstalledAppNotFoundHTTPError, + InstalledAppUnavailableHTTPError, + WebAppAccessUnavailableHTTPError, ) -from extensions.ext_database import db +from controllers.console.explore.installed_app_admission import get_installed_app +from controllers.console.flask_admission import console_account_admission +from controllers.console.wraps import model_validate +from extensions.ext_application_services import application_services from fields.base import ResponseModel from graphon.file import helpers as file_helpers -from libs.datetime_utils import naive_utc_now from libs.helper import dump_response, to_timestamp -from libs.login import login_required -from models import Account, App, InstalledApp, RecommendedApp +from machinery.context import RequestContext from models.model import AppMode, IconType -from services.account_service import TenantService -from services.installed_app_service import InstalledAppCursor, InstalledAppService - - -class InstalledAppCreatePayload(BaseModel): - app_id: str +from services.installed_app_access_service import InstalledAppNotFoundError, InstalledAppRef +from services.installed_app_service import ( + InstalledAppCursor, + InstalledAppRecord, + InstalledAppUnavailableError, +) +from services.webapp_access_query_service import WebAppAccessUnavailableError class InstalledAppUpdatePayload(BaseModel): @@ -75,8 +72,8 @@ def _decode_installed_app_cursor(cursor: str | None) -> InstalledAppCursor | Non padded_cursor = cursor + "=" * (-len(cursor) % 4) payload = base64.b64decode(padded_cursor, altchars=b"-_", validate=True) return InstalledAppCursor.model_validate_json(payload) - except (binascii.Error, UnicodeDecodeError, ValueError): - raise BadRequest("Invalid cursor") from None + except (binascii.Error, UnicodeDecodeError, ValueError) as error: + raise InstalledAppInvalidCursorError() from error class InstalledAppInfoResponse(ResponseModel): @@ -102,7 +99,6 @@ class InstalledAppResponse(ResponseModel): is_pinned: bool last_used_at: int | None editable: bool - uninstallable: bool class InstalledAppListResponse(ResponseModel): @@ -112,26 +108,22 @@ class InstalledAppListResponse(ResponseModel): def _installed_app_response_data( - installed_app: InstalledApp, - app_model: App, + installed_app: InstalledAppRecord, *, - current_tenant_id: str, - current_user: Account, + editable: bool, ) -> InstalledAppResponse: return InstalledAppResponse( id=installed_app.id, - app=InstalledAppInfoResponse.model_validate(app_model), + app=InstalledAppInfoResponse.model_validate(installed_app.app, from_attributes=True), app_owner_tenant_id=installed_app.app_owner_tenant_id, is_pinned=installed_app.is_pinned, last_used_at=to_timestamp(installed_app.last_used_at), - editable=current_user.role in {"owner", "admin"}, - uninstallable=current_tenant_id == installed_app.app_owner_tenant_id, + editable=editable, ) register_schema_models( console_ns, - InstalledAppCreatePayload, InstalledAppUpdatePayload, InstalledAppsListQuery, ) @@ -140,155 +132,93 @@ register_response_schema_models( InstalledAppInfoResponse, InstalledAppResponse, InstalledAppListResponse, - SimpleMessageResponse, SimpleResultMessageResponse, ) @console_ns.route("/installed-apps") class InstalledAppsListApi(Resource): - @login_required - @account_initialization_required @console_ns.doc(params=query_params_from_model(InstalledAppsListQuery)) @console_ns.response(200, "Success", console_ns.models[InstalledAppListResponse.__name__]) - @with_current_user - @with_current_tenant_id - def get(self, current_tenant_id: str, current_user: Account): - query = InstalledAppsListQuery.model_validate(request.args.to_dict()) + @console_account_admission() + @model_validate(InstalledAppsListQuery) + def get(self, query: InstalledAppsListQuery, request_context: RequestContext) -> dict[str, object]: cursor = _decode_installed_app_cursor(query.cursor) - if current_user.current_tenant is None: - raise ValueError("current_user.current_tenant must not be None") - - installed_apps, has_more, next_cursor = InstalledAppService.get_visible_page( - tenant_id=current_tenant_id, - user_id=str(current_user.id), - cursor=cursor, - limit=query.limit, - app_id=query.app_id, - name=query.name, - session=db.session, - ) - - current_user.role = TenantService.get_user_role(current_user, current_user.current_tenant, session=db.session()) + try: + page = application_services().installed_apps.get_visible_page( + tenant_id=request_context.active_workspace_id, + user_id=request_context.account_id, + cursor=cursor, + limit=query.limit, + app_id=query.app_id, + name=query.name, + ) + except WebAppAccessUnavailableError as error: + logger.exception( + "Installed app list access check failed: workspace_id=%s account_id=%s request_id=%s", + request_context.active_workspace_id, + request_context.account_id, + request_context.request_id, + ) + raise WebAppAccessUnavailableHTTPError() from error installed_app_list = [ _installed_app_response_data( installed_app, - app_model, - current_tenant_id=current_tenant_id, - current_user=current_user, + editable=page.editable, ) - for installed_app, app_model in installed_apps + for installed_app in page.data ] - logger.debug("installed_app_list: %s, user_id: %s", installed_app_list, current_user.id) + logger.debug("installed_app_list: %s, user_id: %s", installed_app_list, request_context.account_id) return dump_response( InstalledAppListResponse, { "installed_apps": installed_app_list, - "has_more": has_more, - "next_cursor": _encode_installed_app_cursor(next_cursor) if next_cursor else None, + "has_more": page.has_more, + "next_cursor": _encode_installed_app_cursor(page.next_cursor) if page.next_cursor else None, }, ) - @login_required - @account_initialization_required - @cloud_edition_billing_resource_check("apps") - @console_ns.expect(console_ns.models[InstalledAppCreatePayload.__name__]) - @console_ns.response(200, "Success", console_ns.models[SimpleMessageResponse.__name__]) - @with_current_tenant_id - @model_validate(InstalledAppCreatePayload) - def post(self, req_data: InstalledAppCreatePayload, current_tenant_id: str): - recommended_app = db.session.scalar( - select(RecommendedApp).where(RecommendedApp.app_id == req_data.app_id).limit(1) - ) - if recommended_app is None: - raise NotFound("Recommended app not found") - - app = db.session.get(App, req_data.app_id) - - if app is None: - raise NotFound("App entity not found") - - if not app.is_public: - raise Forbidden("You can't install a non-public app") - - installed_app = db.session.scalar( - select(InstalledApp) - .where(and_(InstalledApp.app_id == req_data.app_id, InstalledApp.tenant_id == current_tenant_id)) - .limit(1) - ) - - if installed_app is None: - # todo: position - recommended_app.install_count += 1 - - new_installed_app = InstalledApp( - app_id=req_data.app_id, - tenant_id=current_tenant_id, - app_owner_tenant_id=app.tenant_id, - is_pinned=False, - last_used_at=naive_utc_now(), - ) - db.session.add(new_installed_app) - db.session.commit() - - return {"message": "App installed successfully"} - @console_ns.route("/installed-apps/") -class InstalledAppApi(InstalledAppResource): - """ - get, update, and delete an installed app - use InstalledAppResource to apply default decorators and get installed_app - """ +class InstalledAppApi(Resource): + """Read or update an admitted workspace installation.""" @console_ns.response(200, "Success", console_ns.models[InstalledAppResponse.__name__]) - @with_current_user - @with_current_tenant_id + @console_account_admission() + @get_installed_app def get( self, - current_tenant_id: str, - current_user: Account, - installed_app: InstalledApp, - ): - app_model = InstalledAppService.get_published_app(installed_app.app_id, session=db.session) - if app_model is None: - raise NotFound("Installed app not found") - if current_user.current_tenant is None: - raise ValueError("current_user.current_tenant must not be None") - - current_user.role = TenantService.get_user_role(current_user, current_user.current_tenant, session=db.session()) + request_context: RequestContext, + installed_app: InstalledAppRef, + ) -> dict[str, object]: + try: + detail = application_services().installed_apps.get_detail( + installed_app=installed_app, account_id=request_context.account_id + ) + except InstalledAppUnavailableError as error: + raise InstalledAppUnavailableHTTPError() from error + except InstalledAppNotFoundError as error: + raise InstalledAppNotFoundHTTPError() from error return dump_response( InstalledAppResponse, _installed_app_response_data( - installed_app, - app_model, - current_tenant_id=current_tenant_id, - current_user=current_user, + detail.installation, + editable=detail.editable, ), ) - @console_ns.response(204, "App uninstalled successfully") - @with_current_tenant_id - def delete(self, current_tenant_id: str, installed_app: InstalledApp): - if installed_app.app_owner_tenant_id == current_tenant_id: - raise BadRequest("You can't uninstall an app owned by the current tenant") - - db.session.delete(installed_app) - db.session.commit() - - return "", 204 - @console_ns.response(200, "Success", console_ns.models[SimpleResultMessageResponse.__name__]) @console_ns.expect(console_ns.models[InstalledAppUpdatePayload.__name__]) + @console_account_admission() + @get_installed_app @model_validate(InstalledAppUpdatePayload) - def patch(self, req_data: InstalledAppUpdatePayload, installed_app: InstalledApp): - commit_args = False - if req_data.is_pinned is not None: - installed_app.is_pinned = req_data.is_pinned - commit_args = True - - if commit_args: - db.session.commit() + def patch( + self, req_data: InstalledAppUpdatePayload, request_context: RequestContext, installed_app: InstalledAppRef + ) -> dict[str, str]: + try: + application_services().installed_apps.set_pinned(installed_app=installed_app, is_pinned=req_data.is_pinned) + except InstalledAppNotFoundError as error: + raise InstalledAppNotFoundHTTPError() from error return {"result": "success", "message": "App info updated successfully"} diff --git a/api/controllers/console/explore/installed_app_admission.py b/api/controllers/console/explore/installed_app_admission.py index f0f70193045..1934350e974 100644 --- a/api/controllers/console/explore/installed_app_admission.py +++ b/api/controllers/console/explore/installed_app_admission.py @@ -1,13 +1,15 @@ """Installed-app admission for handlers using a pure installation reference.""" +import logging from collections.abc import Callable from functools import wraps from typing import Concatenate -from werkzeug.exceptions import NotFound - -from controllers.console.explore.error import AppAccessDeniedError -from controllers.web.error import WebAppAccessServiceUnavailableError +from controllers.console.explore.error import ( + AppAccessDeniedError, + InstalledAppNotFoundHTTPError, + WebAppAccessUnavailableHTTPError, +) from extensions.ext_application_services import application_services from machinery.context import RequestContext from services.installed_app_access_service import ( @@ -17,6 +19,8 @@ from services.installed_app_access_service import ( ) from services.webapp_access_query_service import WebAppAccessUnavailableError +logger = logging.getLogger(__name__) + def get_installed_app[T, **P, R]( view: Callable[Concatenate[T, RequestContext, InstalledAppRef, P], R], @@ -27,19 +31,27 @@ def get_installed_app[T, **P, R]( def decorated(self: T, request_context: RequestContext, /, *args: P.args, **kwargs: P.kwargs) -> R: installed_app_id = kwargs.pop("installed_app_id", None) if installed_app_id is None: - raise ValueError("installed_app_id is required") + raise RuntimeError("The installed-app admission route must provide installed_app_id") try: installed_app = application_services().installed_app_access.get_access( installed_app_id=str(installed_app_id), tenant_id=request_context.active_workspace_id, account_id=request_context.account_id, ) - except InstalledAppNotFoundError: - raise NotFound("Installed app not found") from None - except InstalledAppAccessDeniedError: - raise AppAccessDeniedError() from None - except WebAppAccessUnavailableError: - raise WebAppAccessServiceUnavailableError() from None + except InstalledAppNotFoundError as error: + raise InstalledAppNotFoundHTTPError() from error + except InstalledAppAccessDeniedError as error: + raise AppAccessDeniedError() from error + except WebAppAccessUnavailableError as error: + logger.exception( + "Installed app admission access check failed: installed_app_id=%s workspace_id=%s " + "account_id=%s request_id=%s", + installed_app_id, + request_context.active_workspace_id, + request_context.account_id, + request_context.request_id, + ) + raise WebAppAccessUnavailableHTTPError() from error return view(self, request_context, installed_app, *args, **kwargs) return decorated diff --git a/api/controllers/console/wraps.py b/api/controllers/console/wraps.py index 813c8a75a41..b9420e56cca 100644 --- a/api/controllers/console/wraps.py +++ b/api/controllers/console/wraps.py @@ -651,6 +651,10 @@ def with_current_user_id[T, **P, R]( def validate_request[M: BaseModel](model: type[M]) -> M: """Parse and validate the current request without exposing submitted values.""" + # TODO: Standardize request parsing and structured validation errors after + # auditing callers. Malformed or falsy JSON currently becomes {}, allowing + # optional payloads to succeed as no-ops; validation details currently travel + # as a JSON string in the error message. if request.method == "GET": raw = request.args.to_dict(flat=True) elif request.method == "DELETE": diff --git a/api/extensions/ext_application_services.py b/api/extensions/ext_application_services.py index 67f8caa1f45..10bbcad023b 100644 --- a/api/extensions/ext_application_services.py +++ b/api/extensions/ext_application_services.py @@ -1,8 +1,9 @@ """Composition root for application services used by transport adapters.""" import json +import logging import time -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from dataclasses import dataclass from datetime import UTC, datetime from functools import partial @@ -183,6 +184,7 @@ from services.inner_mail_service import InnerMailService from services.installed_app_access_service import InstalledAppAccessService from services.installed_app_generation_adapters import AppGenerateServiceRuntime as InstalledAppGenerateServiceRuntime from services.installed_app_generation_service import InstalledAppGenerationService +from services.installed_app_service import InstalledAppService from services.knowledge.api_key_service import DatasetApiKeyService from services.message_file_preview_service import MessageFilePreviewService from services.message_suggested_questions_adapters import MessageSuggestedQuestionsRuntime @@ -252,9 +254,20 @@ from services.workspace_plan_gateway import DeploymentWorkspacePlanGateway from services.workspace_query_service import WorkspaceQueryService from tasks.mail_inner_task import enqueue_inner_mail +logger = logging.getLogger(__name__) + _EXTENSION_KEY = "application_services" +# TODO: Normalize EnterpriseService.WebAppAuth result/error contracts in the SDK, +# migrate its callers, then inject its methods directly and remove these adapters. +# Define SDK errors for timeouts, transport failures, upstream status and invalid +# responses before adding finer HTTP mappings; these adapters report unavailability. +# Validate required fields and real booleans there, replacing legacy permission +# truthiness conversion. Missing fields currently become False, {} or a default mode. +# Replace response-shape ValueError/KeyError/AttributeError with typed SDK errors; +# ordinary ValueError can still reach the global 400 invalid_param handler. The +# lost field information cannot be recovered by translating exceptions here. def _get_enterprise_webapp_access_mode(app_id: str) -> WebAppAccessMode: try: settings = EnterpriseService.WebAppAuth.get_app_access_mode_by_id(app_id) @@ -266,13 +279,39 @@ def _get_enterprise_webapp_access_mode(app_id: str) -> WebAppAccessMode: raise WebAppAccessUnavailableError from e -def _is_user_allowed_to_access_webapp(user_id: str, app_id: str) -> bool: +def _is_enterprise_webapp_user_allowed(user_id: str, app_id: str) -> bool: try: return EnterpriseService.WebAppAuth.is_user_allowed_to_access_webapp(user_id, app_id) except (EnterpriseServiceError, httpx.RequestError, json.JSONDecodeError, UnicodeDecodeError) as e: raise WebAppAccessUnavailableError from e +def _batch_get_enterprise_webapp_access_modes(*, app_ids: Sequence[str]) -> Mapping[str, WebAppAccessMode]: + try: + settings = EnterpriseService.WebAppAuth.batch_get_app_access_mode_by_id(list(app_ids)) + except (EnterpriseServiceError, httpx.RequestError, json.JSONDecodeError, UnicodeDecodeError, ValidationError) as e: + raise WebAppAccessUnavailableError from e + access_modes: dict[str, WebAppAccessMode] = {} + for app_id, setting in settings.items(): + try: + access_mode = WebAppAccessMode(setting.access_mode) + except ValueError: + logger.warning("Skipping invalid web app access mode %r for app %s", setting.access_mode, app_id) + continue + access_modes[app_id] = access_mode + return access_modes + + +def _batch_get_enterprise_webapp_user_permissions(*, user_id: str, app_ids: Sequence[str]) -> Mapping[str, bool]: + try: + permissions = EnterpriseService.WebAppAuth.batch_is_user_allowed_to_access_webapps( + user_id=user_id, app_ids=list(app_ids) + ) + except (EnterpriseServiceError, httpx.RequestError, json.JSONDecodeError, UnicodeDecodeError) as e: + raise WebAppAccessUnavailableError from e + return {app_id: bool(allowed) for app_id, allowed in permissions.items()} + + @dataclass(frozen=True, slots=True) class AccountServices: access: AccountAccessService @@ -335,6 +374,7 @@ class ApplicationServices: init_validation: InitValidationService installed_app_access: InstalledAppAccessService installed_app_generation: InstalledAppGenerationService + installed_apps: InstalledAppService notifications: NotificationService step_by_step_tour: StepByStepTourService partner_tenant_bindings: PartnerTenantBindingService @@ -520,11 +560,20 @@ def build_application_services( dify_config.CONSOLE_API_URL + "/console/api/workspaces/current/tool-provider/builtin/" ), ) + webapp_auth_enabled = SystemFeatureService.is_webapp_auth_enabled(deployment_edition=deployment_edition) webapp_access = WebAppAccessQueryService( access=WebAppAccessQueryRepository(session_factory=database_client), - webapp_auth_enabled=SystemFeatureService.is_webapp_auth_enabled(deployment_edition=deployment_edition), + webapp_auth_enabled=webapp_auth_enabled, access_mode_for_app=_get_enterprise_webapp_access_mode, - is_user_allowed_for_app=_is_user_allowed_to_access_webapp, + is_user_allowed_for_app=_is_enterprise_webapp_user_allowed, + get_access_modes=_batch_get_enterprise_webapp_access_modes, + get_user_permissions=_batch_get_enterprise_webapp_user_permissions, + ) + installed_app_access = InstalledAppAccessService( + installed_apps=installed_apps, + is_user_allowed=webapp_access.is_user_allowed, + get_access_modes=webapp_access.batch_get_access_modes, + get_user_permissions=webapp_access.batch_get_user_permissions, ) app_preview_repository = AppPreviewQueryRepository(session_factory=database_client) feature_gateway = FeatureServiceGateway() @@ -755,15 +804,17 @@ def build_application_services( queries=AppScopedEndUserQueryService(end_users=app_scoped_end_user_repository), ), webapp_access=webapp_access, - installed_app_access=InstalledAppAccessService( - installed_apps=installed_apps, - is_user_allowed=webapp_access.is_user_allowed, - ), + installed_app_access=installed_app_access, installed_app_generation=InstalledAppGenerationService( app_definitions=app_definitions, usage=installed_apps, runtime=InstalledAppGenerateServiceRuntime(session_factory=database_client), ), + installed_apps=InstalledAppService( + installed_apps=installed_apps, + get_workspace_role=workspace_query_repository.get_account_role, + get_visible_app_ids=installed_app_access.get_visible_app_ids if webapp_auth_enabled else None, + ), web_app_runtime=WebAppRuntimeQueryService( runtime=app_definition_repository, file_service=file_service, diff --git a/api/openapi/markdown/console-openapi.md b/api/openapi/markdown/console-openapi.md index d484d62fa21..2a5ba8d2779 100644 --- a/api/openapi/markdown/console-openapi.md +++ b/api/openapi/markdown/console-openapi.md @@ -6770,32 +6770,6 @@ Request body: | ---- | ----------- | ------ | | 200 | Success | **application/json**: [InstalledAppListResponse](#installedapplistresponse)
| -### [POST] /installed-apps -#### Request Body - -| Required | Schema | -| -------- | ------ | -| Yes | **application/json**: [InstalledAppCreatePayload](#installedappcreatepayload)
| - -#### Responses - -| Code | Description | Schema | -| ---- | ----------- | ------ | -| 200 | Success | **application/json**: [SimpleMessageResponse](#simplemessageresponse)
| - -### [DELETE] /installed-apps/{installed_app_id} -#### Parameters - -| Name | Located in | Description | Required | Schema | -| ---- | ---------- | ----------- | -------- | ------ | -| installed_app_id | path | | Yes | string (uuid) | - -#### Responses - -| Code | Description | -| ---- | ----------- | -| 204 | App uninstalled successfully | - ### [GET] /installed-apps/{installed_app_id} #### Parameters @@ -19179,12 +19153,6 @@ Input field definition for snippet parameters. | required | boolean | | No | | type | string | | No | -#### InstalledAppCreatePayload - -| Name | Type | Description | Required | -| ---- | ---- | ----------- | -------- | -| app_id | string | | Yes | - #### InstalledAppInfoResponse | Name | Type | Description | Required | @@ -19217,7 +19185,6 @@ Input field definition for snippet parameters. | id | string | | Yes | | is_pinned | boolean | | Yes | | last_used_at | integer | | Yes | -| uninstallable | boolean | | Yes | #### InstalledAppUpdatePayload @@ -22235,12 +22202,6 @@ Resource types understood by access policies. | message | string | | Yes | | query | string | | Yes | -#### SimpleMessageResponse - -| Name | Type | Description | Required | -| ---- | ---- | ----------- | -------- | -| message | string | | Yes | - #### SimpleModelConfig | Name | Type | Description | Required | diff --git a/api/repositories/installed_app_repository.py b/api/repositories/installed_app_repository.py index e16a0e82632..eba9af7f13a 100644 --- a/api/repositories/installed_app_repository.py +++ b/api/repositories/installed_app_repository.py @@ -1,18 +1,93 @@ """Persist workspace installations with independently scoped operations.""" from datetime import datetime +from itertools import starmap from typing import cast, override -from sqlalchemy import select, update +from sqlalchemy import ColumnElement, UnaryExpression, and_, exists, or_, select, update from sqlalchemy.engine import CursorResult from sqlalchemy.orm import Session, sessionmaker -from models.model import App, InstalledApp +from libs.helper import escape_like_pattern +from models.model import App, AppMode, AppModelConfig, InstalledApp +from models.workflow import Workflow from services.installed_app_access_service import InstalledAppAccessStore, InstalledAppNotFoundError, InstalledAppRef from services.installed_app_generation_service import InstalledAppUsageRecorder +from services.installed_app_service import ( + InstalledAppCursor, + InstalledAppInfo, + InstalledAppRecord, + InstalledAppStore, +) -class SQLAlchemyInstalledAppRepository(InstalledAppAccessStore, InstalledAppUsageRecorder): +def _published_app_filter() -> ColumnElement[bool]: + """Match the model configuration used by installed-app parameters. + + Keep the existing publication reference policy: workflow version, public + visibility and site settings do not decide whether an installation is listed. + """ + workflow_app_modes = (AppMode.ADVANCED_CHAT, AppMode.WORKFLOW) + has_published_workflow = exists(select(Workflow.id).where(Workflow.id == App.workflow_id)) + has_published_model_config = exists(select(AppModelConfig.id).where(AppModelConfig.id == App.app_model_config_id)) + return and_( + App.mode != AppMode.AGENT, + or_( + and_(App.mode.in_(workflow_app_modes), App.workflow_id.isnot(None), has_published_workflow), + and_(~App.mode.in_(workflow_app_modes), App.app_model_config_id.isnot(None), has_published_model_config), + ), + ) + + +def _installed_app_cursor_filter(cursor: InstalledAppCursor) -> ColumnElement[bool]: + same_pin_group = InstalledApp.is_pinned == cursor.is_pinned + if cursor.last_used_at is None: + later_in_pin_group = and_( + InstalledApp.last_used_at.is_(None), + InstalledApp.id > cursor.installed_app_id, + ) + else: + later_in_pin_group = or_( + InstalledApp.last_used_at < cursor.last_used_at, + InstalledApp.last_used_at.is_(None), + and_( + InstalledApp.last_used_at == cursor.last_used_at, + InstalledApp.id > cursor.installed_app_id, + ), + ) + if cursor.is_pinned: + return or_(InstalledApp.is_pinned.is_(False), and_(same_pin_group, later_in_pin_group)) + return and_(same_pin_group, later_in_pin_group) + + +def _installed_app_order_by() -> tuple[UnaryExpression[bool], UnaryExpression[datetime | None], UnaryExpression[str]]: + return ( + InstalledApp.is_pinned.desc(), + InstalledApp.last_used_at.desc().nulls_last(), + InstalledApp.id.asc(), + ) + + +def _to_record(installed_app: InstalledApp, app: App) -> InstalledAppRecord: + return InstalledAppRecord( + id=installed_app.id, + app=InstalledAppInfo( + id=app.id, + name=app.name, + description=app.description, + mode=app.mode, + icon_type=app.icon_type, + icon=app.icon, + icon_background=app.icon_background, + use_icon_as_answer_icon=app.use_icon_as_answer_icon, + ), + app_owner_tenant_id=installed_app.app_owner_tenant_id, + is_pinned=installed_app.is_pinned, + last_used_at=installed_app.last_used_at, + ) + + +class SQLAlchemyInstalledAppRepository(InstalledAppAccessStore, InstalledAppUsageRecorder, InstalledAppStore): def __init__(self, *, session_factory: sessionmaker[Session]) -> None: self._session_factory: sessionmaker[Session] = session_factory @@ -52,3 +127,60 @@ class SQLAlchemyInstalledAppRepository(InstalledAppAccessStore, InstalledAppUsag ) if cast(CursorResult, result).rowcount == 0: raise InstalledAppNotFoundError(f"Installed app {installed_app.id} no longer exists") + + @override + def get_candidates( + self, + *, + tenant_id: str, + cursor: InstalledAppCursor | None, + limit: int, + app_id: str | None, + name: str | None, + ) -> tuple[InstalledAppRecord, ...]: + statement = ( + select(InstalledApp, App) + .join(App, App.id == InstalledApp.app_id) + .where(InstalledApp.tenant_id == tenant_id, _published_app_filter()) + ) + if app_id: + statement = statement.where(InstalledApp.app_id == app_id) + if name and (normalized_name := name.strip()): + escaped_name = escape_like_pattern(normalized_name) + statement = statement.where(App.name.ilike(f"%{escaped_name}%", escape="\\")) + if cursor is not None: + statement = statement.where(_installed_app_cursor_filter(cursor)) + with self._session_factory() as session: + rows = session.execute(statement.order_by(*_installed_app_order_by()).limit(limit)) + return tuple(starmap(_to_record, rows)) + + @override + def get_published(self, *, installed_app: InstalledAppRef) -> InstalledAppRecord | None: + with self._session_factory() as session: + row = session.execute( + select(InstalledApp, App) + .join(App, App.id == InstalledApp.app_id) + .where( + InstalledApp.id == installed_app.id, + InstalledApp.tenant_id == installed_app.tenant_id, + InstalledApp.app_id == installed_app.app_id, + _published_app_filter(), + ) + .limit(1) + ).first() + return None if row is None else _to_record(row[0], row[1]) + + @override + def set_pinned(self, *, installed_app: InstalledAppRef, is_pinned: bool) -> None: + with self._session_factory.begin() as session: + result = session.execute( + update(InstalledApp) + .where( + InstalledApp.id == installed_app.id, + InstalledApp.tenant_id == installed_app.tenant_id, + InstalledApp.app_id == installed_app.app_id, + ) + .values(is_pinned=is_pinned) + ) + if cast(CursorResult, result).rowcount == 0: + raise InstalledAppNotFoundError(f"Installed app {installed_app.id} no longer exists") diff --git a/api/repositories/workspace_query_repository.py b/api/repositories/workspace_query_repository.py index eba63eef4f1..12de7805b23 100644 --- a/api/repositories/workspace_query_repository.py +++ b/api/repositories/workspace_query_repository.py @@ -24,6 +24,17 @@ class WorkspaceQueryRepository( def __init__(self, session_factory: sessionmaker[Session]) -> None: self._session_factory = session_factory + def get_account_role(self, *, account_id: str, tenant_id: str) -> str | None: + """Read the current membership role without loading or mutating an Account.""" + stmt = ( + select(TenantAccountJoin.role) + .where(TenantAccountJoin.account_id == account_id, TenantAccountJoin.tenant_id == tenant_id) + .limit(1) + ) + with self._session_factory() as session: + role = session.scalar(stmt) + return role.value if role is not None else None + @override def list_for_account(self, account_id: str) -> tuple[WorkspaceRecord, ...]: stmt = ( diff --git a/api/services/installed_app_access_service.py b/api/services/installed_app_access_service.py index 19c43d8e6c7..3f01b0dd1f7 100644 --- a/api/services/installed_app_access_service.py +++ b/api/services/installed_app_access_service.py @@ -1,8 +1,12 @@ -"""Application admission for an app installed in a workspace.""" +"""Resolve installed-app admission and account-specific list visibility.""" +from collections.abc import Sequence from dataclasses import dataclass from typing import Protocol +from enums import WebAppAccessMode +from services.webapp_access_query_service import WebAppAccessModesQuery, WebAppUserPermissionsQuery + @dataclass(frozen=True, slots=True) class InstalledAppRef: @@ -33,9 +37,13 @@ class InstalledAppAccessService: *, installed_apps: InstalledAppAccessStore, is_user_allowed: WebAppUserAccessCheck, + get_access_modes: WebAppAccessModesQuery, + get_user_permissions: WebAppUserPermissionsQuery, ) -> None: - self._installed_apps = installed_apps - self._is_user_allowed = is_user_allowed + self._installed_apps: InstalledAppAccessStore = installed_apps + self._is_user_allowed: WebAppUserAccessCheck = is_user_allowed + self._get_access_modes: WebAppAccessModesQuery = get_access_modes + self._get_user_permissions: WebAppUserPermissionsQuery = get_user_permissions def get_access(self, *, installed_app_id: str, tenant_id: str, account_id: str) -> InstalledAppRef: installed_app = self._installed_apps.resolve(installed_app_id=installed_app_id, tenant_id=tenant_id) @@ -44,3 +52,24 @@ class InstalledAppAccessService: if not self._is_user_allowed(user_id=account_id, app_id=installed_app.app_id): raise InstalledAppAccessDeniedError("Access to installed app denied") return installed_app + + def get_visible_app_ids(self, *, user_id: str, app_ids: Sequence[str]) -> frozenset[str]: + """Apply list visibility rules without changing single-app admission. + + Missing access settings and SSO-only apps are omitted from the list. + Preserve candidate order for the subsequent batch permission query. + """ + if not app_ids: + return frozenset(app_ids) + + access_modes = self._get_access_modes(app_ids=app_ids) + candidates = [ + app_id + for app_id in app_ids + if app_id in access_modes and access_modes[app_id] != WebAppAccessMode.SSO_VERIFIED + ] + if not candidates: + return frozenset() + + permissions = self._get_user_permissions(user_id=user_id, app_ids=candidates) + return frozenset(app_id for app_id in candidates if permissions.get(app_id)) diff --git a/api/services/installed_app_service.py b/api/services/installed_app_service.py index ba879e4bfbf..d9de2045f65 100644 --- a/api/services/installed_app_service.py +++ b/api/services/installed_app_service.py @@ -1,14 +1,13 @@ +"""Manage workspace installations and paginate apps visible to an account.""" + +from collections.abc import Sequence, Set +from dataclasses import dataclass from datetime import datetime +from typing import Protocol from pydantic import BaseModel -from sqlalchemy import and_, exists, or_, select -from sqlalchemy.orm import Session, scoped_session -from libs.helper import escape_like_pattern -from models import App, AppModelConfig, InstalledApp, Workflow -from models.model import AppMode -from services.enterprise.enterprise_service import EnterpriseService -from services.system_feature_service import SystemFeatureService +from services.installed_app_access_service import InstalledAppRef class InstalledAppCursor(BaseModel): @@ -17,86 +16,90 @@ class InstalledAppCursor(BaseModel): installed_app_id: str -def _published_app_filter(): - """Return the SQL predicate for installed-app web API availability. - - The installed-app parameters endpoint reads the published workflow for - workflow-style apps and the published app model config for easy UI apps. - Keep the list endpoint aligned in SQL so it does not return entries that - will immediately fail with app_unavailable when opened. - """ - workflow_app_modes = (AppMode.ADVANCED_CHAT, AppMode.WORKFLOW) - has_published_workflow = exists(select(Workflow.id).where(Workflow.id == App.workflow_id)) - has_published_model_config = exists(select(AppModelConfig.id).where(AppModelConfig.id == App.app_model_config_id)) - - return and_( - App.mode != AppMode.AGENT, - or_( - and_(App.mode.in_(workflow_app_modes), App.workflow_id.isnot(None), has_published_workflow), - and_(~App.mode.in_(workflow_app_modes), App.app_model_config_id.isnot(None), has_published_model_config), - ), - ) +@dataclass(frozen=True, slots=True) +class InstalledAppInfo: + id: str + name: str + description: str + mode: str + icon_type: str | None + icon: str | None + icon_background: str | None + use_icon_as_answer_icon: bool -def _installed_app_cursor_filter(cursor: InstalledAppCursor): - same_pin_group = InstalledApp.is_pinned == cursor.is_pinned - if cursor.last_used_at is None: - later_in_pin_group = and_( - InstalledApp.last_used_at.is_(None), - InstalledApp.id > cursor.installed_app_id, - ) - else: - later_in_pin_group = or_( - InstalledApp.last_used_at < cursor.last_used_at, - InstalledApp.last_used_at.is_(None), - and_( - InstalledApp.last_used_at == cursor.last_used_at, - InstalledApp.id > cursor.installed_app_id, - ), +@dataclass(frozen=True, slots=True) +class InstalledAppRecord: + id: str + app: InstalledAppInfo + app_owner_tenant_id: str + is_pinned: bool + last_used_at: datetime | None + + def cursor(self) -> InstalledAppCursor: + return InstalledAppCursor( + is_pinned=self.is_pinned, + last_used_at=self.last_used_at, + installed_app_id=self.id, ) - if cursor.is_pinned: - return or_( - InstalledApp.is_pinned.is_(False), - and_(same_pin_group, later_in_pin_group), - ) - return and_(same_pin_group, later_in_pin_group) + +@dataclass(frozen=True, slots=True) +class InstalledAppPage: + data: tuple[InstalledAppRecord, ...] + has_more: bool + next_cursor: InstalledAppCursor | None + editable: bool -def _installed_app_order_by(): - return ( - InstalledApp.is_pinned.desc(), - InstalledApp.last_used_at.desc().nulls_last(), - InstalledApp.id.asc(), - ) +@dataclass(frozen=True, slots=True) +class InstalledAppDetail: + installation: InstalledAppRecord + editable: bool -def _filter_rows_by_webapp_auth( - rows: list[tuple[InstalledApp, App]], - *, - user_id: str, -) -> list[tuple[InstalledApp, App]]: - if not rows: - return [] +class InstalledAppUnavailableError(RuntimeError): + """The admitted installation cannot be returned as a published library app.""" - app_ids = [app.id for _, app in rows] - webapp_settings = EnterpriseService.WebAppAuth.batch_get_app_access_mode_by_id(app_ids) - candidates = [ - (installed_app, app) - for installed_app, app in rows - if (setting := webapp_settings.get(app.id)) is not None and setting.access_mode != "sso_verified" - ] - permissions = EnterpriseService.WebAppAuth.batch_is_user_allowed_to_access_webapps( - user_id=user_id, - app_ids=[app.id for _, app in candidates], - ) - return [(installed_app, app) for installed_app, app in candidates if permissions.get(app.id)] + +class InstalledAppStore(Protocol): + def get_candidates( + self, + *, + tenant_id: str, + cursor: InstalledAppCursor | None, + limit: int, + app_id: str | None, + name: str | None, + ) -> tuple[InstalledAppRecord, ...]: ... + + def get_published(self, *, installed_app: InstalledAppRef) -> InstalledAppRecord | None: ... + + def set_pinned(self, *, installed_app: InstalledAppRef, is_pinned: bool) -> None: ... + + +class InstalledAppVisibilityQuery(Protocol): + def __call__(self, *, user_id: str, app_ids: Sequence[str]) -> Set[str]: ... + + +class WorkspaceRoleLookup(Protocol): + def __call__(self, *, account_id: str, tenant_id: str) -> str | None: ... class InstalledAppService: - @classmethod + def __init__( + self, + *, + installed_apps: InstalledAppStore, + get_workspace_role: WorkspaceRoleLookup, + get_visible_app_ids: InstalledAppVisibilityQuery | None, + ) -> None: + self._installed_apps: InstalledAppStore = installed_apps + self._get_workspace_role: WorkspaceRoleLookup = get_workspace_role + self._get_visible_app_ids: InstalledAppVisibilityQuery | None = get_visible_app_ids + def get_visible_page( - cls, + self, *, tenant_id: str, user_id: str, @@ -104,74 +107,62 @@ class InstalledAppService: limit: int, app_id: str | None, name: str | None, - session: Session | scoped_session, - ) -> tuple[list[tuple[InstalledApp, App]], bool, InstalledAppCursor | None]: + ) -> InstalledAppPage: """Scan ordered candidates until one page of authorized apps is complete.""" - stmt = ( - select(InstalledApp, App) - .join(App, App.id == InstalledApp.app_id) - .where(InstalledApp.tenant_id == tenant_id, _published_app_filter()) - ) - if app_id: - stmt = stmt.where(InstalledApp.app_id == app_id) - if name and (normalized_name := name.strip()): - escaped_name = escape_like_pattern(normalized_name) - stmt = stmt.where(App.name.ilike(f"%{escaped_name}%", escape="\\")) - - webapp_auth_enabled = SystemFeatureService.is_webapp_auth_enabled() - scan_size = limit * 2 if webapp_auth_enabled else limit + 1 - visible_rows: list[tuple[InstalledApp, App]] = [] + scan_size = limit * 2 if self._get_visible_app_ids is not None else limit + 1 + visible_rows: list[InstalledAppRecord] = [] scan_cursor = cursor has_more = False - last_consumed_app: InstalledApp | None = None + last_consumed_app: InstalledAppRecord | None = None while True: - page_stmt = stmt - if scan_cursor is not None: - page_stmt = page_stmt.where(_installed_app_cursor_filter(scan_cursor)) - candidate_result = session.execute(page_stmt.order_by(*_installed_app_order_by()).limit(scan_size)).all() - candidate_rows = [(installed_app, app) for installed_app, app in candidate_result] + # Candidate records are detached and the repository Session is closed + # before the optional Enterprise request. + candidate_rows = self._installed_apps.get_candidates( + tenant_id=tenant_id, + cursor=scan_cursor, + limit=scan_size, + app_id=app_id, + name=name, + ) if not candidate_rows: break - authorized_rows = candidate_rows - if webapp_auth_enabled: - authorized_rows = _filter_rows_by_webapp_auth(candidate_rows, user_id=user_id) + authorized_app_ids: Set[str] = {row.app.id for row in candidate_rows} + if self._get_visible_app_ids is not None: + authorized_app_ids = self._get_visible_app_ids( + user_id=user_id, app_ids=[row.app.id for row in candidate_rows] + ) - authorized_installed_app_ids = {installed_app.id for installed_app, _ in authorized_rows} for row in candidate_rows: - installed_app = row[0] - if installed_app.id not in authorized_installed_app_ids: - last_consumed_app = installed_app + if row.app.id not in authorized_app_ids: + last_consumed_app = row continue if len(visible_rows) == limit: has_more = True break visible_rows.append(row) - last_consumed_app = installed_app + last_consumed_app = row if has_more: break if len(candidate_rows) < scan_size: break - last_scanned_app = candidate_rows[-1][0] - scan_cursor = InstalledAppCursor( - is_pinned=last_scanned_app.is_pinned, - last_used_at=last_scanned_app.last_used_at, - installed_app_id=last_scanned_app.id, - ) + scan_cursor = candidate_rows[-1].cursor() - next_cursor = ( - InstalledAppCursor( - is_pinned=last_consumed_app.is_pinned, - last_used_at=last_consumed_app.last_used_at, - installed_app_id=last_consumed_app.id, - ) - if has_more and last_consumed_app - else None + next_cursor = last_consumed_app.cursor() if has_more and last_consumed_app is not None else None + role = self._get_workspace_role(account_id=user_id, tenant_id=tenant_id) + return InstalledAppPage( + data=tuple(visible_rows), has_more=has_more, next_cursor=next_cursor, editable=role in {"owner", "admin"} ) - return visible_rows, has_more, next_cursor - @staticmethod - def get_published_app(app_id: str, *, session: Session | scoped_session) -> App | None: - return session.scalar(select(App).where(App.id == app_id, _published_app_filter()).limit(1)) + def get_detail(self, *, installed_app: InstalledAppRef, account_id: str) -> InstalledAppDetail: + installation = self._installed_apps.get_published(installed_app=installed_app) + if installation is None: + raise InstalledAppUnavailableError(f"Installed app {installed_app.id} is not available as a published app") + role = self._get_workspace_role(account_id=account_id, tenant_id=installed_app.tenant_id) + return InstalledAppDetail(installation=installation, editable=role in {"owner", "admin"}) + + def set_pinned(self, *, installed_app: InstalledAppRef, is_pinned: bool | None) -> None: + if is_pinned is not None: + self._installed_apps.set_pinned(installed_app=installed_app, is_pinned=is_pinned) diff --git a/api/services/webapp_access_query_service.py b/api/services/webapp_access_query_service.py index 1228d0affbd..0adfc8e3fdc 100644 --- a/api/services/webapp_access_query_service.py +++ b/api/services/webapp_access_query_service.py @@ -1,6 +1,6 @@ """Application service for resolving web-app access.""" -from collections.abc import Callable +from collections.abc import Callable, Mapping, Sequence from typing import Protocol from enums import WebAppAccessMode @@ -12,6 +12,14 @@ class WebAppAccessQuery(Protocol): def find_app_id_by_code(self, app_code: str) -> str | None: ... +class WebAppAccessModesQuery(Protocol): + def __call__(self, *, app_ids: Sequence[str]) -> Mapping[str, WebAppAccessMode]: ... + + +class WebAppUserPermissionsQuery(Protocol): + def __call__(self, *, user_id: str, app_ids: Sequence[str]) -> Mapping[str, bool]: ... + + class WebAppAccessReferenceRequiredError(ValueError): """Raised when neither an app ID nor an app code was provided.""" @@ -32,11 +40,15 @@ class WebAppAccessQueryService: webapp_auth_enabled: bool, access_mode_for_app: Callable[[str], WebAppAccessMode], is_user_allowed_for_app: Callable[[str, str], bool], + get_access_modes: WebAppAccessModesQuery, + get_user_permissions: WebAppUserPermissionsQuery, ) -> None: - self._access = access - self._webapp_auth_enabled = webapp_auth_enabled - self._access_mode_for_app = access_mode_for_app - self._is_user_allowed_for_app = is_user_allowed_for_app + self._access: WebAppAccessQuery = access + self._webapp_auth_enabled: bool = webapp_auth_enabled + self._access_mode_for_app: Callable[[str], WebAppAccessMode] = access_mode_for_app + self._is_user_allowed_for_app: Callable[[str, str], bool] = is_user_allowed_for_app + self._get_access_modes: WebAppAccessModesQuery = get_access_modes + self._get_user_permissions: WebAppUserPermissionsQuery = get_user_permissions def get_access_mode(self, *, app_id: str | None, app_code: str | None) -> WebAppAccessMode: if not self._webapp_auth_enabled: @@ -60,3 +72,16 @@ class WebAppAccessQueryService: return True return self._is_user_allowed_for_app(user_id, app_id) + + def batch_get_access_modes(self, *, app_ids: Sequence[str]) -> Mapping[str, WebAppAccessMode]: + """Return successfully parsed modes; missing IDs do not grant access.""" + if not self._webapp_auth_enabled or not app_ids: + return dict.fromkeys(app_ids, WebAppAccessMode.PUBLIC) + + return self._get_access_modes(app_ids=app_ids) + + def batch_get_user_permissions(self, *, user_id: str, app_ids: Sequence[str]) -> Mapping[str, bool]: + if not self._webapp_auth_enabled or not app_ids: + return dict.fromkeys(app_ids, True) + + return self._get_user_permissions(user_id=user_id, app_ids=app_ids) diff --git a/api/tests/unit_tests/controllers/console/explore/test_completion.py b/api/tests/unit_tests/controllers/console/explore/test_completion.py index 9d952539277..7860c8127ac 100644 --- a/api/tests/unit_tests/controllers/console/explore/test_completion.py +++ b/api/tests/unit_tests/controllers/console/explore/test_completion.py @@ -301,7 +301,14 @@ def test_chat_enforces_admission_before_payload_validation( ) else: _assert_json_response( - response, status=404, body={"code": "not_found", "message": "Installed app not found", "status": 404} + response, + status=404, + body={ + "code": "installed_app_not_found", + "message": "The app was not found in this workspace.", + "status": 404, + "details": {"request_id": "request-1"}, + }, ) assert chat_runtime.calls == [] assert _last_used_at(harness, sqlite_session_factory) is None diff --git a/api/tests/unit_tests/controllers/console/explore/test_installed_app.py b/api/tests/unit_tests/controllers/console/explore/test_installed_app.py index e66a461833b..2f4d8e80f0b 100644 --- a/api/tests/unit_tests/controllers/console/explore/test_installed_app.py +++ b/api/tests/unit_tests/controllers/console/explore/test_installed_app.py @@ -1,576 +1,797 @@ -from collections.abc import Generator -from contextlib import contextmanager +"""HTTP contracts for InstalledApp management with real admission and persistence.""" + +import json +from collections.abc import Mapping, Sequence +from dataclasses import dataclass, field from datetime import datetime -from inspect import unwrap -from types import SimpleNamespace -from unittest.mock import patch +from uuid import UUID, uuid4 import pytest -from flask import Flask -from sqlalchemy import Engine, select -from sqlalchemy.orm import Session, scoped_session, sessionmaker -from werkzeug.exceptions import BadRequest, Forbidden, NotFound +from flask import Flask, got_request_exception +from flask.testing import FlaskClient +from sqlalchemy import Connection, delete, event, select +from sqlalchemy.orm import Session, SessionTransaction, sessionmaker import controllers.console.explore.installed_app as module -import services.installed_app_service as service_module -from models import Account, Tenant, TenantAccountJoin -from models.account import AccountStatus, TenantAccountRole -from models.model import App, AppMode, AppModelConfig, IconType, InstalledApp, RecommendedApp +from enums import WebAppAccessMode +from models import Account, App, InstalledApp, Tenant, TenantAccountJoin +from models.account import TenantAccountRole +from models.model import AppMode, AppModelConfig, IconType from models.workflow import Workflow, WorkflowKind, WorkflowType +from repositories.installed_app_repository import SQLAlchemyInstalledAppRepository +from repositories.workspace_query_repository import WorkspaceQueryRepository +from services.installed_app_access_service import InstalledAppAccessService +from services.installed_app_service import InstalledAppService +from services.webapp_access_query_service import WebAppAccessUnavailableError +from tests.unit_tests.controllers.console.explore.test_installed_app_admission import ( + _assert_json_response, + _Harness, + harness, +) + +__all__ = ["harness"] + +_USED_AT = datetime(2024, 1, 1) + + +@dataclass +class _Services: + installed_apps: InstalledAppService + + +@dataclass +class _Management: + harness: _Harness + session_factory: sessionmaker[Session] + services: _Services + sessions: list[Session] + denied_app_ids: set[str] = field(default_factory=set) + visibility_calls: list[tuple[str, tuple[str, ...]]] = field(default_factory=list) + signed_files: list[str] = field(default_factory=list) + visibility_error: WebAppAccessUnavailableError | None = None + + @property + def client(self) -> FlaskClient: + return self.harness.app.test_client() + + def url(self, installed_app_id: str | None = None) -> str: + return f"/installed-apps/{installed_app_id or self.harness.installed_app.id}" + + def assert_sessions_closed(self) -> None: + assert all(not session.in_transaction() and not session.identity_map for session in self.sessions) + + def get_access_modes(self, *, app_ids: Sequence[str]) -> Mapping[str, WebAppAccessMode]: + self.assert_sessions_closed() + if self.visibility_error is not None: + raise self.visibility_error + return dict.fromkeys(app_ids, WebAppAccessMode.PUBLIC) + + def get_user_permissions(self, *, user_id: str, app_ids: Sequence[str]) -> Mapping[str, bool]: + self.assert_sessions_closed() + self.visibility_calls.append((user_id, tuple(app_ids))) + return {app_id: app_id not in self.denied_app_ids for app_id in app_ids} @pytest.fixture -def tenant_id() -> str: - return "tenant-1" +def management( + harness: _Harness, + monkeypatch: pytest.MonkeyPatch, + sqlite_session_factory: sessionmaker[Session], +) -> _Management: + factory = sessionmaker(bind=sqlite_session_factory.kw["bind"], expire_on_commit=False) + tenant = harness.account.current_tenant + assert tenant is not None + harness.account.role = TenantAccountRole.ADMIN + with factory.begin() as session: + session.add_all( + [ + harness.account, + tenant, + TenantAccountJoin( + tenant_id=tenant.id, + account_id=harness.account.id, + current=True, + role=TenantAccountRole.OWNER, + ), + ] + ) + app = session.get(App, harness.target_app.id) + assert app is not None + app.description = "Description" + app.icon_type = IconType.EMOJI + app.icon = "robot" + app.icon_background = "#FFFFFF" + app.use_icon_as_answer_icon = False + _publish(session, app) + sessions: list[Session] = [] + @event.listens_for(factory, "after_begin") + def track_session(session: Session, _transaction: SessionTransaction, _connection: Connection) -> None: + sessions.append(session) -@pytest.fixture -def database_session(sqlite_engine: Engine) -> Generator[scoped_session[Session]]: - session_registry = scoped_session(sessionmaker(bind=sqlite_engine, expire_on_commit=False)) - try: - yield session_registry - finally: - session_registry.remove() - - -@pytest.fixture -def current_user(database_session: scoped_session[Session], tenant_id: str) -> Account: - tenant = Tenant(name="Current tenant") - tenant.id = tenant_id - account = Account(name="Current user", email="user@example.com", status=AccountStatus.ACTIVE) - account.id = "user-1" - membership = TenantAccountJoin( - tenant_id=tenant.id, - account_id=account.id, - current=True, - role=TenantAccountRole.OWNER, + services = _Services( + installed_apps=InstalledAppService( + installed_apps=SQLAlchemyInstalledAppRepository(session_factory=factory), + get_workspace_role=WorkspaceQueryRepository(factory).get_account_role, + get_visible_app_ids=None, + ) ) - database_session.add_all([tenant, account, membership]) - database_session.commit() - account._current_tenant = tenant - account.role = TenantAccountRole.OWNER - return account + management = _Management(harness, factory, services, sessions) + + def sign_icon(file_id: str) -> str: + management.assert_sessions_closed() + management.signed_files.append(file_id) + return f"/signed-files/{file_id}" + + monkeypatch.setattr(module, "application_services", lambda: services) + monkeypatch.setattr(module.file_helpers, "get_signed_file_url", sign_icon) + harness.api.add_resource(module.InstalledAppsListApi, "/installed-apps") + harness.api.add_resource(module.InstalledAppApi, "/installed-apps/") + return management -def _persist_app( - session: Session | scoped_session[Session], - *, - app_id: str, - tenant_id: str = "owner-tenant", - name: str | None = None, - mode: AppMode = AppMode.CHAT, - public: bool = True, - published: bool = True, -) -> App: - app = App( - id=app_id, - tenant_id=tenant_id, - name=name or f"App {app_id}", - description="Description", - mode=mode, - icon_type=IconType.EMOJI, - icon="robot", - icon_background="#FFFFFF", - enable_site=True, - enable_api=True, - is_public=public, - max_active_requests=None, - ) - session.add(app) - session.flush() - if published and mode in {AppMode.WORKFLOW, AppMode.ADVANCED_CHAT}: +def _publish(session: Session, app: App) -> None: + if app.mode in {AppMode.WORKFLOW, AppMode.ADVANCED_CHAT}: workflow = Workflow( - id=f"workflow-{app_id}", - tenant_id=tenant_id, - app_id=app_id, + tenant_id=app.tenant_id, + app_id=app.id, type=WorkflowType.WORKFLOW, kind=WorkflowKind.STANDARD, version="1", graph='{"nodes":[],"edges":[]}', features="{}", - created_by="user-1", + created_by=str(uuid4()), environment_variables=[], conversation_variables=[], rag_pipeline_variables=[], ) session.add(workflow) - app.workflow_id = workflow.id - elif published: - model_config = AppModelConfig(app_id=app_id) - session.add(model_config) session.flush() - app.app_model_config_id = model_config.id - session.commit() - return app + app.workflow_id = workflow.id + else: + config = AppModelConfig(app_id=app.id) + session.add(config) + session.flush() + app.app_model_config_id = config.id -def _persist_installed_app( - session: Session | scoped_session[Session], - app: App, +def _installation( + management: _Management, *, - tenant_id: str, - installed_app_id: str, + name: str = "Another app", + mode: AppMode = AppMode.CHAT, + published: bool = True, + tenant_id: str | None = None, + app_owner_tenant_id: str | None = None, + installed_app_id: str | None = None, pinned: bool = False, - last_used_at: datetime | None = datetime(2024, 1, 1), -) -> InstalledApp: - installed_app = InstalledApp( - app_id=app.id, - tenant_id=tenant_id, - app_owner_tenant_id=app.tenant_id, - is_pinned=pinned, - last_used_at=last_used_at, + last_used_at: datetime | None = _USED_AT, +) -> tuple[InstalledApp, App]: + with management.session_factory.begin() as session: + app = App( + tenant_id=app_owner_tenant_id or str(uuid4()), + name=name, + description="Description", + mode=mode, + icon_type=IconType.EMOJI, + icon="robot", + icon_background="#FFFFFF", + enable_site=True, + enable_api=True, + is_public=True, + ) + session.add(app) + session.flush() + if published: + _publish(session, app) + installed = InstalledApp( + app_id=app.id, + tenant_id=tenant_id or management.harness.installed_app.tenant_id, + app_owner_tenant_id=app.tenant_id, + is_pinned=pinned, + last_used_at=last_used_at, + ) + if installed_app_id is not None: + installed.id = installed_app_id + session.add(installed) + return installed, app + + +def _clear_installations(management: _Management) -> None: + with management.session_factory.begin() as session: + session.execute(delete(InstalledApp)) + + +def _enable_visibility(management: _Management) -> None: + def unexpected_single_permission(*, user_id: str, app_id: str) -> bool: + pytest.fail(f"Listing should use batch permissions: {user_id=}, {app_id=}") + + repository = SQLAlchemyInstalledAppRepository(session_factory=management.session_factory) + access = InstalledAppAccessService( + installed_apps=repository, + is_user_allowed=unexpected_single_permission, + get_access_modes=management.get_access_modes, + get_user_permissions=management.get_user_permissions, + ) + management.services.installed_apps = InstalledAppService( + installed_apps=repository, + get_workspace_role=WorkspaceQueryRepository(management.session_factory).get_account_role, + get_visible_app_ids=access.get_visible_app_ids, ) - installed_app.id = installed_app_id - session.add(installed_app) - session.commit() - return installed_app -@contextmanager -def _controller_context( - database_session: scoped_session[Session], - *, - role: TenantAccountRole = TenantAccountRole.OWNER, - auth_enabled: bool = False, -) -> Generator[None]: - with ( - patch.object(module.db, "session", database_session), - patch.object(module.TenantService, "get_user_role", return_value=role), - patch.object( - service_module.SystemFeatureService, - "is_webapp_auth_enabled", - return_value=auth_enabled, +def test_response_schema_preserves_domain_types_and_default_page_size() -> None: + assert module.InstalledAppsListQuery().limit == 20 + app_schema = module.InstalledAppInfoResponse.model_json_schema(mode="serialization") + assert set(app_schema["properties"]) == { + "id", + "name", + "description", + "mode", + "icon_type", + "icon", + "icon_background", + "use_icon_as_answer_icon", + "icon_url", + } + assert app_schema["properties"]["mode"]["$ref"].endswith("/AppMode") + assert any(item.get("$ref", "").endswith("/IconType") for item in app_schema["properties"]["icon_type"]["anyOf"]) + + +def test_collection_rejects_post_and_keeps_get_available(management: _Management) -> None: + response = management.client.post("/installed-apps", json={"app_id": management.harness.target_app.id}) + + _assert_json_response( + response, + status=405, + body={ + "code": "method_not_allowed", + "message": "The method is not allowed for the requested URL.", + "status": 405, + }, + ) + options = management.client.options("/installed-apps") + assert options.status_code == 200 + allowed_methods = {method.strip() for method in options.headers["Allow"].split(",")} + assert "POST" not in allowed_methods + assert "GET" in allowed_methods + assert management.sessions == [] + + response = management.client.get("/installed-apps") + + assert response.status_code == 200 + assert [item["id"] for item in response.get_json()["installed_apps"]] == [management.harness.installed_app.id] + + +@pytest.mark.parametrize("endpoint", ["list", "detail"]) +@pytest.mark.parametrize("icon_type", [IconType.EMOJI, IconType.IMAGE, None]) +@pytest.mark.parametrize("last_used_at", [None, _USED_AT]) +def test_read_responses_preserve_exact_fields_types_headers_and_signed_icons( + management: _Management, endpoint: str, icon_type: IconType | None, last_used_at: datetime | None +) -> None: + harness = management.harness + with management.session_factory.begin() as session: + app = session.get(App, harness.target_app.id) + assert app is not None + app.icon_type = icon_type + installed = session.get(InstalledApp, harness.installed_app.id) + assert installed is not None + installed.last_used_at = last_used_at + expected: dict[str, object] = { + "id": harness.installed_app.id, + "app": { + "id": harness.target_app.id, + "name": "Shared app", + "description": "Description", + "mode": "completion", + "icon_type": icon_type.value if icon_type is not None else None, + "icon": "robot", + "icon_background": "#FFFFFF", + "use_icon_as_answer_icon": False, + "icon_url": "/signed-files/robot" if icon_type == IconType.IMAGE else None, + }, + "app_owner_tenant_id": harness.target_app.tenant_id, + "is_pinned": False, + "last_used_at": int(last_used_at.timestamp()) if last_used_at is not None else None, + "editable": True, + } + + response = management.client.get("/installed-apps" if endpoint == "list" else management.url()) + + body: dict[str, object] = ( + {"installed_apps": [expected], "has_more": False, "next_cursor": None} if endpoint == "list" else expected + ) + _assert_json_response(response, status=200, body=body) + assert dict(response.headers) == {"Content-Type": "application/json", "Content-Length": str(len(response.data))} + assert management.signed_files == (["robot"] if icon_type == IconType.IMAGE else []) + management.assert_sessions_closed() + + +@pytest.mark.parametrize("role", [TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.NORMAL, None]) +@pytest.mark.parametrize("endpoint", ["list", "detail"]) +def test_read_uses_fresh_workspace_role_without_mutating_cached_account( + management: _Management, role: TenantAccountRole | None, endpoint: str +) -> None: + harness = management.harness + with management.session_factory.begin() as session: + membership = session.scalar(select(TenantAccountJoin).where(TenantAccountJoin.account_id == harness.account.id)) + assert membership is not None + if role is None: + session.delete(membership) + else: + membership.role = role + + response = management.client.get("/installed-apps" if endpoint == "list" else management.url()) + + assert response.status_code == 200 + item = response.get_json()["installed_apps"][0] if endpoint == "list" else response.get_json() + assert item["editable"] is (role in {TenantAccountRole.OWNER, TenantAccountRole.ADMIN}) + assert harness.account.role == TenantAccountRole.ADMIN + + +@pytest.mark.parametrize("endpoint", ["list", "detail"]) +@pytest.mark.parametrize("different_owner", ["account", "workspace"]) +def test_read_does_not_use_privileged_membership_from_another_account_or_workspace( + management: _Management, endpoint: str, different_owner: str +) -> None: + harness = management.harness + account_id = harness.account.id + tenant_id = harness.installed_app.tenant_id + with management.session_factory.begin() as session: + session.execute( + delete(TenantAccountJoin).where( + TenantAccountJoin.account_id == account_id, + TenantAccountJoin.tenant_id == tenant_id, + ) + ) + if different_owner == "account": + account = Account(name="Another account", email="other@example.com") + session.add(account) + session.flush() + account_id = account.id + else: + tenant = Tenant(name="Another workspace") + session.add(tenant) + session.flush() + tenant_id = tenant.id + session.add(TenantAccountJoin(account_id=account_id, tenant_id=tenant_id, role=TenantAccountRole.OWNER)) + + response = management.client.get("/installed-apps" if endpoint == "list" else management.url()) + + assert response.status_code == 200 + item = response.get_json()["installed_apps"][0] if endpoint == "list" else response.get_json() + assert item["editable"] is False + assert harness.account.role == TenantAccountRole.ADMIN + + +def test_list_filters_tenant_mode_and_actual_publication_targets(management: _Management) -> None: + workflow, _ = _installation(management, mode=AppMode.WORKFLOW) + advanced, _ = _installation(management, mode=AppMode.ADVANCED_CHAT) + _installation(management, mode=AppMode.AGENT) + _installation(management, published=False) + _installation(management, mode=AppMode.WORKFLOW, published=False) + _installation(management, tenant_id=str(uuid4())) + missing_config, config_app = _installation(management) + missing_workflow, workflow_app = _installation(management, mode=AppMode.WORKFLOW) + with management.session_factory.begin() as session: + session.execute(delete(AppModelConfig).where(AppModelConfig.id == config_app.app_model_config_id)) + session.execute(delete(Workflow).where(Workflow.id == workflow_app.workflow_id)) + # Existing installations remain visible even if their app is private. + app = session.get(App, management.harness.target_app.id) + assert app is not None + app.is_public = False + + response = management.client.get("/installed-apps") + + assert response.status_code == 200 + ids = {item["id"] for item in response.get_json()["installed_apps"]} + assert ids == {management.harness.installed_app.id, workflow.id, advanced.id} + assert missing_config.id not in ids + assert missing_workflow.id not in ids + filtered = management.client.get("/installed-apps", query_string={"app_id": workflow.app_id}) + assert [item["id"] for item in filtered.get_json()["installed_apps"]] == [workflow.id] + + +def test_list_name_search_trims_and_escapes_wildcards(management: _Management) -> None: + exact, _ = _installation(management, name="Literal %_\\ app") + _installation(management, name="Literal wildcard app") + + response = management.client.get("/installed-apps", query_string={"name": " %_\\ "}) + + assert response.status_code == 200 + assert [item["id"] for item in response.get_json()["installed_apps"]] == [exact.id] + assert len(management.client.get("/installed-apps", query_string={"name": " "}).get_json()["installed_apps"]) == 3 + + +def test_list_pages_pin_time_null_and_id_order_without_duplicates(management: _Management) -> None: + _clear_installations(management) + expected: list[str] = [] + for number, pinned, used_at in [ + (1, True, datetime(2024, 2, 1)), + (2, True, _USED_AT), + (3, True, _USED_AT), + (4, True, None), + (5, False, datetime(2024, 3, 1)), + (6, False, None), + (7, False, None), + ]: + installed, _ = _installation( + management, installed_app_id=str(UUID(int=number)), pinned=pinned, last_used_at=used_at + ) + expected.append(installed.id) + received: list[str] = [] + cursor: str | None = None + for _ in range(4): + query = {"limit": "2"} + if cursor is not None: + query["cursor"] = cursor + response = management.client.get("/installed-apps", query_string=query) + assert response.status_code == 200 + page = response.get_json() + received.extend(item["id"] for item in page["installed_apps"]) + cursor = page["next_cursor"] + assert page["has_more"] is (cursor is not None) + assert received == expected + assert cursor is None + + +def test_visible_page_scans_denied_batches_and_cursor_tracks_last_consumed_candidate(management: _Management) -> None: + _clear_installations(management) + _enable_visibility(management) + rows = [_installation(management, installed_app_id=str(UUID(int=index + 1))) for index in range(5)] + management.denied_app_ids.update(app.id for _, app in rows[1:4]) + + response = management.client.get("/installed-apps", query_string={"limit": 1}) + + assert response.status_code == 200 + page = response.get_json() + assert [item["id"] for item in page["installed_apps"]] == [rows[0][0].id] + assert page["has_more"] is True + cursor = module._decode_installed_app_cursor(page["next_cursor"]) + assert cursor is not None + assert cursor.installed_app_id == rows[3][0].id + assert management.visibility_calls == [ + (management.harness.account.id, tuple(app.id for _, app in rows[start : start + 2])) for start in (0, 2, 4) + ] + response = management.client.get("/installed-apps", query_string={"limit": 1, "cursor": page["next_cursor"]}) + assert [item["id"] for item in response.get_json()["installed_apps"]] == [rows[4][0].id] + assert response.get_json()["has_more"] is False + assert response.get_json()["next_cursor"] is None + + +@pytest.mark.parametrize("empty", ["database", "visibility"]) +def test_list_empty_result_has_no_cursor(management: _Management, empty: str) -> None: + if empty == "database": + _clear_installations(management) + else: + _enable_visibility(management) + management.denied_app_ids.add(management.harness.target_app.id) + response = management.client.get("/installed-apps") + _assert_json_response(response, status=200, body={"installed_apps": [], "has_more": False, "next_cursor": None}) + + +@pytest.mark.parametrize("cursor", ["not-a-cursor", "!!!", "e30", ""]) +def test_list_invalid_cursor_returns_specific_error(management: _Management, cursor: str) -> None: + response = management.client.get("/installed-apps", query_string={"cursor": cursor}) + _assert_json_response( + response, + status=400, + body={ + "code": "invalid_cursor", + "message": "The app list cursor is invalid. Refresh the list and try again.", + "status": 400, + "details": {"request_id": "request-1"}, + }, + ) + assert management.sessions == [] + + +@pytest.mark.parametrize( + ("query", "error"), + [ + ( + {"limit": "private-input"}, + { + "type": "int_parsing", + "loc": ["limit"], + "msg": "Input should be a valid integer, unable to parse string as an integer", + }, ), - ): - yield + ( + {"limit": "0"}, + {"type": "greater_than_equal", "loc": ["limit"], "msg": "Input should be greater than or equal to 1"}, + ), + ( + {"limit": "101"}, + {"type": "less_than_equal", "loc": ["limit"], "msg": "Input should be less than or equal to 100"}, + ), + ( + {"name": "private-input" * 10}, + {"type": "string_too_long", "loc": ["name"], "msg": "String should have at most 100 characters"}, + ), + ], +) +def test_list_validation_identifies_query_field_without_echoing_input( + management: _Management, query: dict[str, str], error: dict[str, object] +) -> None: + response = management.client.get("/installed-apps", query_string=query) + + _assert_json_response( + response, + status=422, + body={"code": "unprocessable_entity", "message": json.dumps([error]), "status": 422}, + ) + assert "private-input" not in response.get_data(as_text=True) + assert management.sessions == [] -class TestInstalledAppsListApi: - def test_list_query_defaults_to_20(self) -> None: - assert module.InstalledAppsListQuery().limit == 20 +@pytest.mark.parametrize( + ("payload", "error"), + [ + ( + {"is_pinned": "private-input"}, + { + "type": "bool_parsing", + "loc": ["is_pinned"], + "msg": "Input should be a valid boolean, unable to interpret input", + }, + ), + ( + ["private-input"], + { + "type": "model_type", + "loc": [], + "msg": "Input should be a valid dictionary or instance of InstalledAppUpdatePayload", + }, + ), + ], +) +def test_patch_validation_identifies_body_field_without_echoing_input( + management: _Management, payload: object, error: dict[str, object] +) -> None: + response = management.client.patch(management.url(), data=json.dumps(payload), content_type="application/json") - def test_response_schema_preserves_installed_app_domain_types(self) -> None: - app_schema = module.InstalledAppInfoResponse.model_json_schema(mode="serialization") - list_schema = module.InstalledAppListResponse.model_json_schema(mode="serialization") - - assert { - "id", - "name", - "description", - "mode", - "icon_type", - "icon", - "icon_background", - "use_icon_as_answer_icon", - "icon_url", - } <= set(app_schema["required"]) - assert set(app_schema["$defs"]["AppMode"]["enum"]) == {mode.value for mode in AppMode} - assert set(app_schema["$defs"]["IconType"]["enum"]) == {icon_type.value for icon_type in IconType} - assert "next_cursor" in list_schema["required"] - - def test_published_app_filter_checks_publish_targets(self) -> None: - compiled_filter = str(service_module._published_app_filter().compile(compile_kwargs={"literal_binds": True})) - - assert "workflows" in compiled_filter - assert "app_model_configs" in compiled_filter - assert "workflow_id" in compiled_filter - assert "app_model_config_id" in compiled_filter - assert "apps.mode != 'agent'" in compiled_filter - - def test_get_filters_tenant_publication_mode_and_app_id( - self, - app: Flask, - current_user: Account, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - chat = _persist_app(database_session, app_id="chat") - workflow = _persist_app(database_session, app_id="workflow", mode=AppMode.WORKFLOW) - unpublished = _persist_app(database_session, app_id="unpublished", published=False) - agent = _persist_app(database_session, app_id="agent", mode=AppMode.AGENT) - foreign = _persist_app(database_session, app_id="foreign") - for index, (app_model, installed_tenant) in enumerate( - ( - (chat, tenant_id), - (workflow, tenant_id), - (unpublished, tenant_id), - (agent, tenant_id), - (foreign, "other-tenant"), - ) - ): - _persist_installed_app( - database_session, - app_model, - tenant_id=installed_tenant, - installed_app_id=f"installed-{index}", - ) - - api = module.InstalledAppsListApi() - method = unwrap(api.get) - with app.test_request_context("/"), _controller_context(database_session): - result = method(api, tenant_id, current_user) - - assert {item["app"]["id"] for item in result["installed_apps"]} == {"chat", "workflow"} - assert all(item["editable"] is True for item in result["installed_apps"]) - assert all(item["uninstallable"] is False for item in result["installed_apps"]) - assert result["has_more"] is False - assert result["next_cursor"] is None - - with ( - app.test_request_context("/?app_id=workflow"), - _controller_context(database_session, role=TenantAccountRole.NORMAL), - ): - filtered = method(api, tenant_id, current_user) - assert [item["app"]["id"] for item in filtered["installed_apps"]] == ["workflow"] - assert filtered["installed_apps"][0]["editable"] is False - - def test_get_name_search_escapes_like_wildcards( - self, - app: Flask, - current_user: Account, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - exact = _persist_app(database_session, app_id="exact", name="Sales%_Q3") - wildcard_decoy = _persist_app(database_session, app_id="decoy", name="SalesZZQ3") - for index, app_model in enumerate((exact, wildcard_decoy)): - _persist_installed_app( - database_session, - app_model, - tenant_id=tenant_id, - installed_app_id=f"installed-{index}", - ) - - with app.test_request_context("/?name=Sales%25_Q3"), _controller_context(database_session): - result = unwrap(module.InstalledAppsListApi().get)(module.InstalledAppsListApi(), tenant_id, current_user) - - assert [item["app"]["id"] for item in result["installed_apps"]] == ["exact"] - - def test_get_orders_and_paginates_with_cursor( - self, - app: Flask, - current_user: Account, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - app_models = [_persist_app(database_session, app_id=f"app-{index}") for index in range(4)] - rows = ( - (app_models[0], "installed-0", True, datetime(2024, 1, 1)), - (app_models[1], "installed-1", False, datetime(2024, 1, 3)), - (app_models[2], "installed-2", False, datetime(2024, 1, 2)), - (app_models[3], "installed-3", False, None), - ) - for app_model, installed_id, pinned, last_used_at in rows: - _persist_installed_app( - database_session, - app_model, - tenant_id=tenant_id, - installed_app_id=installed_id, - pinned=pinned, - last_used_at=last_used_at, - ) - - api = module.InstalledAppsListApi() - method = unwrap(api.get) - with app.test_request_context("/?limit=2"), _controller_context(database_session): - first_page = method(api, tenant_id, current_user) - - assert [item["id"] for item in first_page["installed_apps"]] == ["installed-0", "installed-1"] - assert first_page["has_more"] is True - cursor = module._decode_installed_app_cursor(first_page["next_cursor"]) - assert cursor is not None - assert cursor.installed_app_id == "installed-1" - - with ( - app.test_request_context(f"/?limit=2&cursor={first_page['next_cursor']}"), - _controller_context(database_session), - ): - second_page = method(api, tenant_id, current_user) - assert [item["id"] for item in second_page["installed_apps"]] == ["installed-2", "installed-3"] - assert second_page["has_more"] is False - - def test_get_scans_past_denied_candidate_batch( - self, - app: Flask, - current_user: Account, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - allowed_first = _persist_app(database_session, app_id="allowed-first") - denied = _persist_app(database_session, app_id="denied") - allowed_later = _persist_app(database_session, app_id="allowed-later") - _persist_installed_app( - database_session, - allowed_first, - tenant_id=tenant_id, - installed_app_id="installed-allowed-first", - last_used_at=datetime(2024, 1, 3), - ) - _persist_installed_app( - database_session, - denied, - tenant_id=tenant_id, - installed_app_id="installed-denied", - last_used_at=datetime(2024, 1, 2), - ) - _persist_installed_app( - database_session, - allowed_later, - tenant_id=tenant_id, - installed_app_id="installed-allowed-later", - last_used_at=datetime(2024, 1, 1), - ) - settings = { - app_id: SimpleNamespace(access_mode="restricted") for app_id in ("allowed-first", "denied", "allowed-later") - } - - def permission_state(*, user_id: str, app_ids: list[str]) -> dict[str, bool]: - assert user_id == current_user.id - return {app_id: app_id != "denied" for app_id in app_ids} - - with ( - app.test_request_context("/?limit=1"), - _controller_context(database_session, role=TenantAccountRole.NORMAL, auth_enabled=True), - patch.object( - service_module.EnterpriseService.WebAppAuth, - "batch_get_app_access_mode_by_id", - side_effect=lambda app_ids: {app_id: settings[app_id] for app_id in app_ids}, - ), - patch.object( - service_module.EnterpriseService.WebAppAuth, - "batch_is_user_allowed_to_access_webapps", - side_effect=permission_state, - ), - ): - result = unwrap(module.InstalledAppsListApi().get)(module.InstalledAppsListApi(), tenant_id, current_user) - - assert [item["id"] for item in result["installed_apps"]] == ["installed-allowed-first"] - assert result["has_more"] is True - cursor = module._decode_installed_app_cursor(result["next_cursor"]) - assert cursor is not None - assert cursor.installed_app_id == "installed-denied" - - def test_get_applies_web_auth_permission_and_sso_state( - self, - app: Flask, - current_user: Account, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - apps = {app_id: _persist_app(database_session, app_id=app_id) for app_id in ("allowed", "denied", "sso")} - for index, app_model in enumerate(apps.values()): - _persist_installed_app( - database_session, - app_model, - tenant_id=tenant_id, - installed_app_id=f"installed-{index}", - ) - settings = { - "allowed": SimpleNamespace(access_mode="restricted"), - "denied": SimpleNamespace(access_mode="restricted"), - "sso": SimpleNamespace(access_mode="sso_verified"), - } - - with ( - app.test_request_context("/"), - _controller_context(database_session, auth_enabled=True), - patch.object( - service_module.EnterpriseService.WebAppAuth, - "batch_get_app_access_mode_by_id", - return_value=settings, - ), - patch.object( - service_module.EnterpriseService.WebAppAuth, - "batch_is_user_allowed_to_access_webapps", - return_value={"allowed": True, "denied": False}, - ), - ): - result = unwrap(module.InstalledAppsListApi().get)(module.InstalledAppsListApi(), tenant_id, current_user) - - assert [item["app"]["id"] for item in result["installed_apps"]] == ["allowed"] - - def test_get_rejects_invalid_cursor(self, app: Flask, current_user: Account, tenant_id: str) -> None: - with app.test_request_context("/?cursor=not-a-cursor"), pytest.raises(BadRequest, match="Invalid cursor"): - unwrap(module.InstalledAppsListApi().get)(module.InstalledAppsListApi(), tenant_id, current_user) - - def test_get_rejects_user_without_current_tenant(self, app: Flask, tenant_id: str) -> None: - current_user = Account(name="No tenant", email="no-tenant@example.com", status=AccountStatus.ACTIVE) - current_user.id = "user-without-tenant" - - with ( - app.test_request_context("/"), - pytest.raises(ValueError, match="current_user.current_tenant must not be None"), - ): - unwrap(module.InstalledAppsListApi().get)(module.InstalledAppsListApi(), tenant_id, current_user) + _assert_json_response( + response, + status=422, + body={"code": "unprocessable_entity", "message": json.dumps([error]), "status": 422}, + ) + assert "private-input" not in response.get_data(as_text=True) + assert management.sessions == [] + with management.session_factory() as session: + installed = session.get(InstalledApp, management.harness.installed_app.id) + assert installed is not None + assert installed.is_pinned is False -class TestInstalledAppsCreateApi: - def test_post_installs_public_recommended_app_once( - self, - app: Flask, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - app_model = _persist_app(database_session, app_id="recommended", public=True) - recommended = RecommendedApp( - app_id=app_model.id, - description={"en-US": "recommended"}, - copyright="copyright", - privacy_policy="https://example.com/privacy", - category="productivity", - ) - database_session.add(recommended) - database_session.commit() - recommended_id = recommended.id - bind = database_session.bind - request_data = module.InstalledAppCreatePayload(app_id=app_model.id) +@pytest.mark.parametrize( + ("body", "content_type"), + [ + pytest.param('{"is_pinned": private-input}', "application/json", id="malformed-json"), + pytest.param("null", "application/json", id="null"), + pytest.param("false", "application/json", id="false"), + pytest.param("[]", "application/json", id="empty-list"), + pytest.param("", "application/json", id="empty-body"), + pytest.param('{"is_pinned": false}', "text/plain", id="non-json-media"), + ], +) +def test_patch_shared_parser_normalizes_empty_or_unreadable_body_to_noop( + management: _Management, body: str, content_type: str +) -> None: + installed, _ = _installation(management, pinned=True) + management.sessions.clear() - for _ in range(2): - with ( - app.test_request_context("/", json={"app_id": app_model.id}), - patch.object(module.db, "session", database_session), - ): - assert unwrap(module.InstalledAppsListApi().post)( - module.InstalledAppsListApi(), request_data, tenant_id - ) == {"message": "App installed successfully"} + response = management.client.patch(management.url(installed.id), data=body, content_type=content_type) - database_session.remove() - with Session(bind) as observer: - installed = observer.scalars(select(InstalledApp)).all() - assert len(installed) == 1 - assert installed[0].tenant_id == tenant_id - assert installed[0].app_owner_tenant_id == app_model.tenant_id - persisted_recommendation = observer.get(RecommendedApp, recommended_id) - assert persisted_recommendation is not None - assert persisted_recommendation.install_count == 1 - - def test_post_enforces_recommendation_and_public_state( - self, - app: Flask, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - api = module.InstalledAppsListApi() - with ( - app.test_request_context("/", json={"app_id": "missing"}), - patch.object(module.db, "session", database_session), - pytest.raises(NotFound, match="Recommended app not found"), - ): - unwrap(api.post)(api, module.InstalledAppCreatePayload(app_id="missing"), tenant_id) - - private_app = _persist_app(database_session, app_id="private", public=False) - database_session.add( - RecommendedApp( - app_id=private_app.id, - description={}, - copyright="copyright", - privacy_policy="privacy", - category="category", - ) - ) - database_session.commit() - with ( - app.test_request_context("/", json={"app_id": private_app.id}), - patch.object(module.db, "session", database_session), - pytest.raises(Forbidden, match="non-public app"), - ): - unwrap(api.post)(api, module.InstalledAppCreatePayload(app_id=private_app.id), tenant_id) + _assert_json_response( + response, + status=200, + body={"result": "success", "message": "App info updated successfully"}, + ) + assert management.sessions == [] + with management.session_factory() as session: + persisted = session.get(InstalledApp, installed.id) + assert persisted is not None + assert persisted.is_pinned is True + assert persisted.last_used_at == _USED_AT -class TestInstalledAppApi: - def test_get_returns_published_app_and_rejects_unpublished_app( - self, - app: Flask, - current_user: Account, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - published = _persist_app(database_session, app_id="published") - installed = _persist_installed_app( - database_session, - published, - tenant_id=tenant_id, - installed_app_id="installed-published", - ) - api = module.InstalledAppApi() - with app.test_request_context("/"), _controller_context(database_session): - result = unwrap(api.get)(api, tenant_id, current_user, installed) - - assert result["id"] == installed.id - assert result["app"]["id"] == published.id - assert result["app"]["mode"] == AppMode.CHAT - assert result["app"]["icon_type"] == IconType.EMOJI - assert result["editable"] is True - - unpublished = _persist_app(database_session, app_id="unpublished", published=False) - unpublished_install = _persist_installed_app( - database_session, - unpublished, - tenant_id=tenant_id, - installed_app_id="installed-unpublished", - ) - with ( - app.test_request_context("/"), - patch.object(module.db, "session", database_session), - pytest.raises(NotFound, match="Installed app not found"), - ): - unwrap(api.get)(api, tenant_id, current_user, unpublished_install) - - def test_delete_removes_foreign_app_and_rejects_owned_app( - self, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - foreign_app = _persist_app(database_session, app_id="foreign") - installed = _persist_installed_app( - database_session, - foreign_app, - tenant_id=tenant_id, - installed_app_id="installed-foreign", - ) - api = module.InstalledAppApi() - with patch.object(module.db, "session", database_session): - response, status = unwrap(api.delete)(api, tenant_id, installed) - assert (response, status) == ("", 204) - database_session.expire_all() - assert database_session.get(InstalledApp, installed.id) is None - - owned_app = _persist_app(database_session, app_id="owned", tenant_id=tenant_id) - owned_install = _persist_installed_app( - database_session, - owned_app, - tenant_id=tenant_id, - installed_app_id="installed-owned", - ) - with pytest.raises(BadRequest, match="owned by the current tenant"): - unwrap(api.delete)(api, tenant_id, owned_install) - assert database_session.get(InstalledApp, owned_install.id) is not None - - def test_patch_persists_pin_and_accepts_noop_payload( - self, - app: Flask, - tenant_id: str, - database_session: scoped_session[Session], - ) -> None: - app_model = _persist_app(database_session, app_id="pin") - installed = _persist_installed_app( - database_session, - app_model, - tenant_id=tenant_id, - installed_app_id="installed-pin", - ) - api = module.InstalledAppApi() - with ( - app.test_request_context("/", json={"is_pinned": True}), - patch.object(module.db, "session", database_session), - ): - result = unwrap(api.patch)(api, module.InstalledAppUpdatePayload(is_pinned=True), installed) - assert result["result"] == "success" - database_session.expire_all() - persisted = database_session.get(InstalledApp, installed.id) +@pytest.mark.parametrize( + ("mode", "published"), [(AppMode.CHAT, False), (AppMode.WORKFLOW, False), (AppMode.AGENT, True)] +) +def test_unavailable_installation_rejects_detail_but_can_be_pinned( + management: _Management, mode: AppMode, published: bool +) -> None: + installed, _ = _installation(management, mode=mode, published=published) + url = management.url(installed.id) + _assert_json_response( + management.client.get(url), + status=404, + body={ + "code": "installed_app_unavailable", + "message": "The app is not available in this app library.", + "status": 404, + "details": {"request_id": "request-1"}, + }, + ) + _assert_json_response( + management.client.patch(url, json={"is_pinned": True}), + status=200, + body={"result": "success", "message": "App info updated successfully"}, + ) + with management.session_factory() as session: + persisted = session.get(InstalledApp, installed.id) assert persisted is not None assert persisted.is_pinned is True - with app.test_request_context("/", json={}), patch.object(module.db, "session", database_session): - result = unwrap(api.patch)(api, module.InstalledAppUpdatePayload(), installed) - assert result["result"] == "success" + +@pytest.mark.parametrize("owned", [False, True]) +def test_item_rejects_delete_and_keeps_get_and_patch_available(management: _Management, owned: bool) -> None: + owner_id = management.harness.installed_app.tenant_id if owned else str(uuid4()) + installed, _ = _installation(management, app_owner_tenant_id=owner_id) + management.sessions.clear() + url = management.url(installed.id) + + _assert_json_response( + management.client.delete(url), + status=405, + body={ + "code": "method_not_allowed", + "message": "The method is not allowed for the requested URL.", + "status": 405, + }, + ) + options = management.client.options(url) + assert options.status_code == 200 + allowed_methods = {method.strip() for method in options.headers["Allow"].split(",")} + assert "DELETE" not in allowed_methods + assert {"GET", "PATCH"} <= allowed_methods + assert management.sessions == [] + assert management.harness.state.permission_calls == [] + + response = management.client.get(url) + assert response.status_code == 200 + assert response.get_json()["id"] == installed.id + assert "uninstallable" not in response.get_json() + _assert_json_response( + management.client.patch(url, json={"is_pinned": True}), + status=200, + body={"result": "success", "message": "App info updated successfully"}, + ) + with management.session_factory() as session: + persisted = session.get(InstalledApp, installed.id) + assert persisted is not None + assert persisted.is_pinned is True + + +@pytest.mark.parametrize("payload", [{}, {"is_pinned": None}, {"is_pinned": False}, {"is_pinned": True}]) +def test_patch_preserves_pin_values_and_noop_payloads(management: _Management, payload: dict[str, object]) -> None: + installed, _ = _installation(management, pinned=True) + management.sessions.clear() + response = management.client.patch(management.url(installed.id), json=payload) + _assert_json_response(response, status=200, body={"result": "success", "message": "App info updated successfully"}) + if payload.get("is_pinned") is None: + assert management.sessions == [] + with management.session_factory() as session: + persisted = session.get(InstalledApp, installed.id) + assert persisted is not None + assert persisted.is_pinned is (payload["is_pinned"] if payload.get("is_pinned") is not None else True) + assert persisted.last_used_at == _USED_AT + + +@pytest.mark.parametrize("method", ["GET", "PATCH"]) +@pytest.mark.parametrize("failure", ["tenant", "missing", "orphan", "permission"]) +def test_item_admission_precedes_management_actions_and_preserves_errors( + management: _Management, method: str, failure: str +) -> None: + harness = management.harness + url = management.url() + if failure == "permission": + harness.state.allowed = False + elif failure == "missing": + url = management.url(str(uuid4())) + else: + with management.session_factory.begin() as session: + if failure == "tenant": + installed = session.get(InstalledApp, harness.installed_app.id) + assert installed is not None + installed.tenant_id = str(uuid4()) + else: + app = session.get(App, harness.target_app.id) + assert app is not None + session.delete(app) + management.sessions.clear() + response = management.client.open(url, method=method, json={"is_pinned": "invalid"}) + if failure == "permission": + _assert_json_response( + response, status=403, body={"code": "access_denied", "message": "App access denied.", "status": 403} + ) + else: + _assert_json_response( + response, + status=404, + body={ + "code": "installed_app_not_found", + "message": "The app was not found in this workspace.", + "status": 404, + "details": {"request_id": "request-1"}, + }, + ) + assert management.sessions == [] + with management.session_factory() as session: + installed = session.get(InstalledApp, harness.installed_app.id) + assert (installed is None) is (failure == "orphan") + if installed is not None: + assert installed.is_pinned is False + + +@pytest.mark.parametrize(("method", "item"), [("GET", False), ("GET", True), ("PATCH", True)]) +def test_management_dependency_failure_preserves_cause_and_request_context( + management: _Management, + caplog: pytest.LogCaptureFixture, + method: str, + item: bool, +) -> None: + failure = WebAppAccessUnavailableError("private upstream response containing credentials") + if item: + management.harness.state.permission_error = failure + else: + _enable_visibility(management) + management.visibility_error = failure + exceptions: list[Exception] = [] + + def capture_exception(_sender: Flask, exception: Exception) -> None: + exceptions.append(exception) + + with got_request_exception.connected_to(capture_exception): + response = management.client.open( + management.url() if item else "/installed-apps", method=method, json={"is_pinned": True} + ) + + _assert_json_response( + response, + status=503, + body={ + "code": "web_app_access_unavailable", + "message": "The app access service is unavailable. Try again later.", + "status": 503, + "details": {"request_id": "request-1"}, + }, + ) + assert "WWW-Authenticate" not in response.headers + assert "private upstream" not in response.get_data(as_text=True) + assert len(exceptions) == 1 + assert exceptions[0].__cause__ is failure + management.assert_sessions_closed() + records = [record for record in caplog.records if record.name.startswith("controllers.console.explore.")] + assert len(records) == 1 + assert records[0].exc_info is not None + assert records[0].exc_info[1] is failure + assert management.harness.installed_app.tenant_id in records[0].getMessage() + assert management.harness.account.id in records[0].getMessage() + assert "request-1" in records[0].getMessage() + with management.session_factory() as session: + installed = session.get(InstalledApp, management.harness.installed_app.id) + assert installed is not None + assert installed.is_pinned is False + + +@pytest.mark.parametrize(("method", "item"), [("GET", False), ("GET", True), ("PATCH", True)]) +def test_all_management_handlers_apply_setup_before_business_operations( + management: _Management, method: str, item: bool +) -> None: + management.harness.state.setup_completed = False + response = management.client.open(management.url() if item else "/installed-apps", method=method, json={}) + _assert_json_response( + response, + status=401, + body={ + "code": "not_setup", + "message": "Dify has not been initialized and installed yet. " + "Please proceed with the initialization and installation process first.", + "status": 401, + }, + ) + assert response.headers["WWW-Authenticate"] == 'Bearer realm="api"' + assert management.sessions == [] + assert management.harness.state.permission_calls == [] diff --git a/api/tests/unit_tests/controllers/console/explore/test_installed_app_admission.py b/api/tests/unit_tests/controllers/console/explore/test_installed_app_admission.py index ac38975761e..371efba6780 100644 --- a/api/tests/unit_tests/controllers/console/explore/test_installed_app_admission.py +++ b/api/tests/unit_tests/controllers/console/explore/test_installed_app_admission.py @@ -1,5 +1,5 @@ import json -from collections.abc import Callable, Generator +from collections.abc import Callable, Generator, Mapping, Sequence from dataclasses import dataclass, field from operator import itemgetter from uuid import UUID, uuid4 @@ -14,6 +14,7 @@ from werkzeug.exceptions import Unauthorized from werkzeug.test import TestResponse import controllers.console.explore.completion as completion_module +import controllers.console.explore.error as explore_error_module import controllers.console.explore.installed_app_admission as admission_module import controllers.console.explore.parameter as parameter_module import controllers.console.explore.saved_message as saved_message_module @@ -25,7 +26,7 @@ import libs.login as login_module import services.app_task_service as app_task_module from controllers.console.explore.installed_app_admission import get_installed_app from controllers.console.flask_admission import console_account_admission -from enums import DeploymentEdition +from enums import DeploymentEdition, WebAppAccessMode from extensions.ext_login import DifyLoginManager, unauthorized_handler from libs.external_api import ExternalApi from machinery.context import RequestContext @@ -124,10 +125,18 @@ def harness( state.permission_action() return state.allowed + def unexpected_access_modes(*, app_ids: Sequence[str]) -> Mapping[str, WebAppAccessMode]: + pytest.fail(f"Single-app admission should not query batch access modes: {app_ids=}") + + def unexpected_user_permissions(*, user_id: str, app_ids: Sequence[str]) -> Mapping[str, bool]: + pytest.fail(f"Single-app admission should not query batch permissions: {user_id=}, {app_ids=}") + services = _ApplicationServices( installed_app_access=InstalledAppAccessService( installed_apps=SQLAlchemyInstalledAppRepository(session_factory=repository_factory), is_user_allowed=permission_check, + get_access_modes=unexpected_access_modes, + get_user_permissions=unexpected_user_permissions, ) ) @@ -144,6 +153,7 @@ def harness( monkeypatch.setattr(login_module, "check_csrf_token", check_csrf) monkeypatch.setattr(console_wraps, "_is_setup_completed", setup_completed) monkeypatch.setattr(console_admission, "get_request_id", lambda: "request-1") + monkeypatch.setattr(explore_error_module, "get_request_id", lambda: "request-1") monkeypatch.setattr(console_admission, "get_trace_id", lambda: None) app = Flask(__name__) @@ -237,7 +247,12 @@ def test_missing_or_foreign_installation_returns_404_before_permission( _assert_json_response( response, status=404, - body={"code": "not_found", "message": "Installed app not found", "status": 404}, + body={ + "code": "installed_app_not_found", + "message": "The app was not found in this workspace.", + "status": 404, + "details": {"request_id": "request-1"}, + }, ) assert harness.state.permission_calls == [] assert harness.state.events == ["setup", "csrf"] @@ -310,20 +325,42 @@ def test_account_admission_precedes_installed_app_lookup( assert response.headers["WWW-Authenticate"] == 'Bearer realm="api"' -def test_permission_dependency_unavailable_returns_503_without_calling_handler(harness: _Harness) -> None: - harness.state.permission_error = WebAppAccessUnavailableError() +def test_permission_dependency_failure_preserves_cause_without_leaking_details( + harness: _Harness, + caplog: pytest.LogCaptureFixture, +) -> None: + failure = WebAppAccessUnavailableError("private upstream response containing credentials") + harness.state.permission_error = failure + exceptions: list[Exception] = [] - response = harness.app.test_client().get(harness.url()) + def capture_exception(_sender: Flask, exception: Exception) -> None: + exceptions.append(exception) + + with got_request_exception.connected_to(capture_exception): + response = harness.app.test_client().get(harness.url()) _assert_json_response( response, status=503, body={ "code": "web_app_access_unavailable", - "message": "Web app access service is unavailable.", + "message": "The app access service is unavailable. Try again later.", "status": 503, + "details": {"request_id": "request-1"}, }, ) + assert "private upstream" not in response.get_data(as_text=True) + assert len(exceptions) == 1 + assert exceptions[0].__cause__ is failure + records = [record for record in caplog.records if record.name == admission_module.__name__] + assert len(records) == 1 + assert records[0].exc_info is not None + assert records[0].exc_info[1] is failure + assert harness.installed_app.id in records[0].getMessage() + assert harness.installed_app.tenant_id in records[0].getMessage() + assert harness.account.id in records[0].getMessage() + assert "request-1" in records[0].getMessage() + assert "WWW-Authenticate" not in response.headers assert harness.state.events == ["setup", "csrf", "permission"] assert harness.state.permission_calls == [(harness.account.id, harness.target_app.id)] diff --git a/api/tests/unit_tests/controllers/console/explore/test_installed_app_completion.py b/api/tests/unit_tests/controllers/console/explore/test_installed_app_completion.py index 808ffee6351..d59bc90605f 100644 --- a/api/tests/unit_tests/controllers/console/explore/test_installed_app_completion.py +++ b/api/tests/unit_tests/controllers/console/explore/test_installed_app_completion.py @@ -348,7 +348,14 @@ def test_completion_requires_installed_app_admission_before_payload_validation( ) else: _assert_json_response( - response, status=404, body={"code": "not_found", "message": "Installed app not found", "status": 404} + response, + status=404, + body={ + "code": "installed_app_not_found", + "message": "The app was not found in this workspace.", + "status": 404, + "details": {"request_id": "request-1"}, + }, ) assert runtime.calls == [] if rejection != "missing": @@ -388,6 +395,8 @@ def test_resource_removed_after_admission_does_not_start_runtime( assert _last_used_at(harness, sqlite_session_factory) is None else: _assert_json_response( - response, status=404, body={"code": "not_found", "message": "Installed app not found", "status": 404} + response, + status=404, + body={"code": "not_found", "message": "Installed app not found", "status": 404}, ) assert runtime.calls == [] diff --git a/api/tests/unit_tests/controllers/console/explore/test_workflow.py b/api/tests/unit_tests/controllers/console/explore/test_workflow.py index 303f925ec2f..c967556193e 100644 --- a/api/tests/unit_tests/controllers/console/explore/test_workflow.py +++ b/api/tests/unit_tests/controllers/console/explore/test_workflow.py @@ -342,7 +342,14 @@ def test_workflow_handlers_require_admission_before_payload_or_task_actions( ) else: _assert_json_response( - response, status=404, body={"code": "not_found", "message": "Installed app not found", "status": 404} + response, + status=404, + body={ + "code": "installed_app_not_found", + "message": "The app was not found in this workspace.", + "status": 404, + "details": {"request_id": "request-1"}, + }, ) assert runtime.calls == [] assert stop_redis.operations == [] diff --git a/api/tests/unit_tests/extensions/test_ext_application_services.py b/api/tests/unit_tests/extensions/test_ext_application_services.py index fa23943ef55..8460a941f57 100644 --- a/api/tests/unit_tests/extensions/test_ext_application_services.py +++ b/api/tests/unit_tests/extensions/test_ext_application_services.py @@ -1,6 +1,8 @@ """Tests for application-service dependency wiring.""" import json +import logging +from collections.abc import Mapping from io import BytesIO from types import SimpleNamespace from typing import cast @@ -10,8 +12,7 @@ from uuid import uuid4 import httpx import pytest from flask import Flask -from pydantic import ValidationError -from sqlalchemy import select +from sqlalchemy import event, select from sqlalchemy.orm import Session, sessionmaker from core.tools.tool_file_manager import ToolFileManager @@ -19,7 +20,7 @@ from enums import DeploymentEdition, WebAppAccessMode from extensions import ext_application_services from extensions.ext_redis import RedisClientWrapper from machinery.context import RequestContext -from models.account import Account +from models.account import Account, Tenant, TenantAccountJoin, TenantAccountRole from models.enums import CustomizeTokenStrategy from models.model import AccountTrialAppRecord, App, AppMode, AppModelConfig, DifySetup, InstalledApp, Site, TrialApp from repositories.account_activation_repository import SQLAlchemyAccountActivationRepository @@ -80,8 +81,7 @@ from services.auth.data_source_api_key_auth_service import DataSourceApiKeyAuthS from services.billing_portal_service import BillingPortalService from services.billing_service import BillingService from services.compliance_download_service import ComplianceDownloadService -from services.enterprise.enterprise_service import WebAppSettings -from services.errors.enterprise import EnterpriseAPIError, EnterpriseAPINotFoundError +from services.errors.enterprise import EnterpriseAPIError, EnterpriseAPINotFoundError, EnterpriseServiceError from services.file_service import FileService from services.human_input_file_upload_service import HumanInputFileUploadService from services.init_validation_service import InvalidInitializationPasswordError @@ -99,7 +99,7 @@ from services.retention.workflow_run.archive_log_service import WorkflowRunArchi from services.tag_application_service import TagApplicationService from services.tool_file_download_service import ToolFileDownloadService from services.upload_file_delivery_service import UploadFileDeliveryService -from services.webapp_access_query_service import WebAppAccessUnavailableError +from services.webapp_access_query_service import WebAppAccessQueryService, WebAppAccessUnavailableError from services.workflow_app_log_query_service import WorkflowAppLogQueryService from services.workflow_run_service import WorkflowRunService from services.workflow_statistic_query_service import WorkflowStatisticQueryService @@ -1064,87 +1064,87 @@ def test_build_application_services_adapts_enterprise_webapp_access_mode( get_access_mode.assert_called_once_with("app-1") +def _query_webapp_access( + service: WebAppAccessQueryService, query_kind: str +) -> WebAppAccessMode | bool | Mapping[str, WebAppAccessMode] | Mapping[str, bool]: + if query_kind == "single-mode": + return service.get_access_mode(app_id="app-1", app_code=None) + if query_kind == "single-permission": + return service.is_user_allowed(user_id="viewer", app_id="app-1") + if query_kind == "batch-modes": + return service.batch_get_access_modes(app_ids=("app-1",)) + assert query_kind == "batch-permissions" + return service.batch_get_user_permissions(user_id="viewer", app_ids=("app-1",)) + + +@pytest.mark.parametrize("query_kind", ["single-mode", "single-permission", "batch-modes", "batch-permissions"]) @pytest.mark.parametrize( "enterprise_error", [ - pytest.param(EnterpriseAPINotFoundError(), id="not-found"), - pytest.param(EnterpriseAPIError(), id="api-error"), - pytest.param(httpx.ConnectError("connection failed"), id="transport"), + pytest.param(httpx.ReadTimeout("Enterprise timed out"), id="timeout"), + pytest.param(httpx.ConnectError("connection failed"), id="connection"), + pytest.param(EnterpriseServiceError("upstream failure"), id="upstream"), pytest.param(json.JSONDecodeError("invalid", "", 0), id="invalid-json"), pytest.param(UnicodeDecodeError("utf-8", b"\xff", 0, 1, "invalid"), id="invalid-encoding"), - pytest.param( - ValidationError.from_exception_data(WebAppSettings.__name__, []), - id="invalid-response", - ), ], ) -def test_build_application_services_maps_known_enterprise_errors( +def test_webapp_access_queries_map_known_enterprise_errors_to_unavailable( sqlite_session_factory: sessionmaker[Session], + query_kind: str, enterprise_error: Exception, ) -> None: - with ( - patch("extensions.ext_application_services.SystemFeatureService.is_webapp_auth_enabled", return_value=True), - patch( - "extensions.ext_application_services.EnterpriseService.WebAppAuth.get_app_access_mode_by_id", - side_effect=enterprise_error, - ), - ): + with patch( + "services.enterprise.enterprise_service.EnterpriseRequest.send_request", side_effect=enterprise_error + ) as enterprise_request: services = ext_application_services.build_application_services( database_client=sqlite_session_factory, - deployment_edition=DeploymentEdition.COMMUNITY, + deployment_edition=DeploymentEdition.ENTERPRISE, initialization_password="", redis=MagicMock(spec=RedisClientWrapper), ) - with pytest.raises(WebAppAccessUnavailableError) as raised: - services.webapp_access.get_access_mode(app_id="app-1", app_code=None) + _query_webapp_access(services.webapp_access, query_kind) + assert type(raised.value) is WebAppAccessUnavailableError assert raised.value.__cause__ is enterprise_error + assert enterprise_request.call_count == 1 -def test_build_application_services_maps_invalid_access_mode_to_unavailable( - sqlite_session_factory: sessionmaker[Session], +@pytest.mark.parametrize("access_mode", ["invalid", 123]) +def test_single_webapp_mode_maps_invalid_enum_or_field_value_to_unavailable( + sqlite_session_factory: sessionmaker[Session], access_mode: str | int ) -> None: - with ( - patch("extensions.ext_application_services.SystemFeatureService.is_webapp_auth_enabled", return_value=True), - patch( - "extensions.ext_application_services.EnterpriseService.WebAppAuth.get_app_access_mode_by_id", - return_value=SimpleNamespace(access_mode="invalid"), - ), + with patch( + "services.enterprise.enterprise_service.EnterpriseRequest.send_request", + return_value={"accessMode": access_mode}, ): services = ext_application_services.build_application_services( database_client=sqlite_session_factory, - deployment_edition=DeploymentEdition.COMMUNITY, + deployment_edition=DeploymentEdition.ENTERPRISE, initialization_password="", redis=MagicMock(spec=RedisClientWrapper), ) - with pytest.raises(WebAppAccessUnavailableError) as raised: services.webapp_access.get_access_mode(app_id="app-1", app_code=None) + assert type(raised.value) is WebAppAccessUnavailableError assert isinstance(raised.value.__cause__, ValueError) -def test_build_application_services_does_not_hide_unknown_enterprise_errors( - sqlite_session_factory: sessionmaker[Session], +@pytest.mark.parametrize("query_kind", ["single-mode", "single-permission", "batch-modes", "batch-permissions"]) +@pytest.mark.parametrize("failure", [TypeError("adapter bug"), ValueError("unexpected programming error")]) +def test_webapp_access_queries_do_not_hide_unknown_programming_errors( + sqlite_session_factory: sessionmaker[Session], query_kind: str, failure: Exception ) -> None: - failure = TypeError("adapter bug") - with ( - patch("extensions.ext_application_services.SystemFeatureService.is_webapp_auth_enabled", return_value=True), - patch( - "extensions.ext_application_services.EnterpriseService.WebAppAuth.get_app_access_mode_by_id", - side_effect=failure, - ), - ): + with patch("services.enterprise.enterprise_service.EnterpriseRequest.send_request", side_effect=failure): services = ext_application_services.build_application_services( database_client=sqlite_session_factory, - deployment_edition=DeploymentEdition.COMMUNITY, + deployment_edition=DeploymentEdition.ENTERPRISE, initialization_password="", redis=MagicMock(spec=RedisClientWrapper), ) - - with pytest.raises(TypeError) as raised: - services.webapp_access.get_access_mode(app_id="app-1", app_code=None) + with pytest.raises(type(failure)) as raised: + _query_webapp_access(services.webapp_access, query_kind) assert raised.value is failure @@ -1195,7 +1195,7 @@ def test_webapp_permission_adapter_maps_connection_failure() -> None: ), pytest.raises(WebAppAccessUnavailableError) as raised, ): - ext_application_services._is_user_allowed_to_access_webapp("user-1", "app-1") + ext_application_services._is_enterprise_webapp_user_allowed("user-1", "app-1") assert raised.value.__cause__ is failure @@ -1233,3 +1233,271 @@ def test_build_application_services_wires_dynamic_recommended_catalog( services.recommended_app_queries.list_recommended( language="en-US", ) + + +@pytest.mark.parametrize( + ("deployment_edition", "permission_result"), + [ + pytest.param(DeploymentEdition.COMMUNITY, False, id="community-skips-enterprise"), + pytest.param(DeploymentEdition.CLOUD, False, id="cloud-skips-enterprise"), + pytest.param(DeploymentEdition.ENTERPRISE, True, id="enterprise-allowed"), + pytest.param(DeploymentEdition.ENTERPRISE, False, id="enterprise-denied"), + ], +) +def test_installed_app_management_composition_reads_real_installations_and_current_workspace_role( + sqlite_session_factory: sessionmaker[Session], + installed_app_ref: InstalledAppRef, + deployment_edition: DeploymentEdition, + permission_result: bool, +) -> None: + account_id = str(uuid4()) + with sqlite_session_factory.begin() as session: + account = Account(name="Viewer", email="management@example.com") + account.id = account_id + tenant = Tenant(name="Viewer workspace") + tenant.id = installed_app_ref.tenant_id + session.add_all([account, tenant]) + membership = TenantAccountJoin( + tenant_id=installed_app_ref.tenant_id, account_id=account_id, role=TenantAccountRole.OWNER + ) + configuration = AppModelConfig(app_id=installed_app_ref.app_id) + session.add_all([membership, configuration]) + session.flush() + app = session.get(App, installed_app_ref.app_id) + assert app is not None + app.app_model_config_id = configuration.id + membership_id = membership.id + + active_connections = 0 + engine = sqlite_session_factory.kw["bind"] + + @event.listens_for(engine, "checkout") + def connection_checked_out(_connection: object, _record: object, _proxy: object) -> None: + nonlocal active_connections + active_connections += 1 + + @event.listens_for(engine, "checkin") + def connection_checked_in(_connection: object, _record: object) -> None: + nonlocal active_connections + active_connections -= 1 + + def enterprise_response(method: str, path: str, *, json: dict[str, object]) -> dict[str, object]: + assert deployment_edition == DeploymentEdition.ENTERPRISE + # The candidate read must release its DB connection before either network call. + assert active_connections == 0 + assert method == "POST" + if path == "/webapp/access-mode/batch/id": + assert json == {"appIds": [installed_app_ref.app_id]} + return {"accessModes": {installed_app_ref.app_id: "private"}} + assert path == "/webapp/permission/batch" + assert json == {"userId": account_id, "appIds": [installed_app_ref.app_id]} + return {"permissions": {installed_app_ref.app_id: permission_result}} + + with patch( + "services.enterprise.enterprise_service.EnterpriseRequest.send_request", side_effect=enterprise_response + ) as enterprise_request: + services = ext_application_services.build_application_services( + database_client=sqlite_session_factory, + deployment_edition=deployment_edition, + initialization_password="", + redis=MagicMock(spec=RedisClientWrapper), + ) + if deployment_edition != DeploymentEdition.ENTERPRISE: + assert services.webapp_access.batch_get_access_modes(app_ids=(installed_app_ref.app_id,)) == { + installed_app_ref.app_id: WebAppAccessMode.PUBLIC + } + assert services.webapp_access.batch_get_user_permissions( + user_id=account_id, app_ids=(installed_app_ref.app_id,) + ) == {installed_app_ref.app_id: True} + assert services.installed_app_access.get_visible_app_ids( + user_id=account_id, app_ids=(installed_app_ref.app_id,) + ) == frozenset({installed_app_ref.app_id}) + page = services.installed_apps.get_visible_page( + tenant_id=installed_app_ref.tenant_id, + user_id=account_id, + cursor=None, + limit=1, + app_id=None, + name=None, + ) + assert page.editable is True + assert page.has_more is False + assert page.next_cursor is None + expected_ids: list[str] = ( + [installed_app_ref.id] if deployment_edition != DeploymentEdition.ENTERPRISE or permission_result else [] + ) + assert [installation.id for installation in page.data] == expected_ids + + with sqlite_session_factory.begin() as session: + stored_membership = session.get(TenantAccountJoin, membership_id) + assert stored_membership is not None + stored_membership.role = TenantAccountRole.NORMAL + services.installed_apps.set_pinned(installed_app=installed_app_ref, is_pinned=True) + detail = services.installed_apps.get_detail(installed_app=installed_app_ref, account_id=account_id) + assert detail.editable is False + assert detail.installation.is_pinned is True + assert detail.installation.id == installed_app_ref.id + assert detail.installation.app.id == installed_app_ref.app_id + assert active_connections == 0 + + if deployment_edition == DeploymentEdition.ENTERPRISE: + assert enterprise_request.call_args_list == [ + call("POST", "/webapp/access-mode/batch/id", json={"appIds": [installed_app_ref.app_id]}), + call( + "POST", + "/webapp/permission/batch", + json={"userId": account_id, "appIds": [installed_app_ref.app_id]}, + ), + ] + else: + enterprise_request.assert_not_called() + + +def test_installed_app_visibility_batches_settings_before_permissions_and_preserves_truthiness_filter( + sqlite_session_factory: sessionmaker[Session], +) -> None: + app_ids = ( + "allowed", + "truthy-permission", + "missing-setting", + "sso", + "denied", + "zero", + "empty", + "null", + "missing-permission", + ) + permission_candidates = ["allowed", "truthy-permission", "denied", "zero", "empty", "null", "missing-permission"] + with patch( + "services.enterprise.enterprise_service.EnterpriseRequest.send_request", + side_effect=[ + { + "accessModes": { + "allowed": "private", + "truthy-permission": "private_all", + "sso": "sso_verified", + "denied": "public", + "zero": "private_all", + "empty": "public", + "null": "private", + "missing-permission": "private", + } + }, + { + "permissions": { + "allowed": True, + "truthy-permission": 1, + "missing-setting": True, + "sso": True, + "denied": False, + "zero": 0, + "empty": "", + "null": None, + } + }, + ], + ) as enterprise_request: + services = ext_application_services.build_application_services( + database_client=sqlite_session_factory, + deployment_edition=DeploymentEdition.ENTERPRISE, + initialization_password="", + redis=MagicMock(spec=RedisClientWrapper), + ) + visible = services.installed_app_access.get_visible_app_ids(user_id="viewer", app_ids=app_ids) + + assert visible == frozenset({"allowed", "truthy-permission"}) + assert enterprise_request.call_args_list == [ + call("POST", "/webapp/access-mode/batch/id", json={"appIds": list(app_ids)}), + call("POST", "/webapp/permission/batch", json={"userId": "viewer", "appIds": permission_candidates}), + ] + + +@pytest.mark.parametrize("include_valid_apps", [True, False], ids=["mixed-modes", "all-invalid"]) +def test_installed_app_visibility_skips_and_logs_each_invalid_access_mode( + sqlite_session_factory: sessionmaker[Session], caplog: pytest.LogCaptureFixture, include_valid_apps: bool +) -> None: + modes = {"invalid-empty": "", "invalid-unknown": "future-mode"} + valid_ids: list[str] = ["valid-before", "valid-after"] if include_valid_apps else [] + if include_valid_apps: + modes = {"valid-before": "private", **modes, "valid-after": "public"} + app_ids = tuple(modes) + responses: list[object] = [{"accessModes": modes}] + if include_valid_apps: + # Even an over-inclusive permission response must not restore invalid apps. + responses.append({"permissions": dict.fromkeys(app_ids, True)}) + with patch( + "services.enterprise.enterprise_service.EnterpriseRequest.send_request", side_effect=responses + ) as enterprise_request: + services = ext_application_services.build_application_services( + database_client=sqlite_session_factory, + deployment_edition=DeploymentEdition.ENTERPRISE, + initialization_password="", + redis=MagicMock(spec=RedisClientWrapper), + ) + visible = services.installed_app_access.get_visible_app_ids(user_id="viewer", app_ids=app_ids) + + assert visible == frozenset(valid_ids) + expected_calls = [call("POST", "/webapp/access-mode/batch/id", json={"appIds": list(app_ids)})] + if include_valid_apps: + expected_calls.append(call("POST", "/webapp/permission/batch", json={"userId": "viewer", "appIds": valid_ids})) + assert enterprise_request.call_args_list == expected_calls + warnings = [ + message + for logger_name, level, message in caplog.record_tuples + if logger_name == ext_application_services.__name__ and level == logging.WARNING + ] + assert len(warnings) == 2 + assert any("invalid-empty" in message and repr("") in message for message in warnings) + assert any("invalid-unknown" in message and repr("future-mode") in message for message in warnings) + + +@pytest.mark.parametrize("app_ids", [(), ("sso", "missing")]) +def test_installed_app_visibility_skips_unnecessary_enterprise_requests( + sqlite_session_factory: sessionmaker[Session], app_ids: tuple[str, ...] +) -> None: + with patch( + "services.enterprise.enterprise_service.EnterpriseRequest.send_request", + return_value={"accessModes": {"sso": "sso_verified"}}, + ) as enterprise_request: + services = ext_application_services.build_application_services( + database_client=sqlite_session_factory, + deployment_edition=DeploymentEdition.ENTERPRISE, + initialization_password="", + redis=MagicMock(spec=RedisClientWrapper), + ) + visible = services.installed_app_access.get_visible_app_ids(user_id="viewer", app_ids=app_ids) + + assert visible == frozenset() + if app_ids: + enterprise_request.assert_called_once_with( + "POST", "/webapp/access-mode/batch/id", json={"appIds": list(app_ids)} + ) + else: + enterprise_request.assert_not_called() + + +@pytest.mark.parametrize("failure_stage", ["settings", "permissions"]) +def test_installed_app_visibility_propagates_access_unavailable_with_original_cause( + sqlite_session_factory: sessionmaker[Session], + failure_stage: str, +) -> None: + enterprise_error = EnterpriseAPIError("batch unavailable") + responses: list[object] = [] + if failure_stage == "permissions": + responses.append({"accessModes": {"app-1": "private"}}) + responses.append(enterprise_error) + with patch( + "services.enterprise.enterprise_service.EnterpriseRequest.send_request", side_effect=responses + ) as enterprise_request: + services = ext_application_services.build_application_services( + database_client=sqlite_session_factory, + deployment_edition=DeploymentEdition.ENTERPRISE, + initialization_password="", + redis=MagicMock(spec=RedisClientWrapper), + ) + with pytest.raises(WebAppAccessUnavailableError) as caught: + services.installed_app_access.get_visible_app_ids(user_id="viewer", app_ids=("app-1",)) + + assert type(caught.value) is WebAppAccessUnavailableError + assert caught.value.__cause__ is enterprise_error + assert enterprise_request.call_count == (2 if failure_stage == "permissions" else 1) diff --git a/api/tests/unit_tests/repositories/test_installed_app_repository.py b/api/tests/unit_tests/repositories/test_installed_app_repository.py index 99b7599d0c7..c0fd7fd7035 100644 --- a/api/tests/unit_tests/repositories/test_installed_app_repository.py +++ b/api/tests/unit_tests/repositories/test_installed_app_repository.py @@ -1,14 +1,16 @@ from dataclasses import replace -from datetime import datetime +from datetime import datetime, timedelta from uuid import uuid4 import pytest from sqlalchemy import event, inspect from sqlalchemy.orm import Session, sessionmaker -from models.model import App, AppMode, InstalledApp +from models.model import App, AppMode, AppModelConfig, IconType, InstalledApp +from models.workflow import Workflow, WorkflowKind, WorkflowType from repositories.installed_app_repository import SQLAlchemyInstalledAppRepository from services.installed_app_access_service import InstalledAppNotFoundError, InstalledAppRef +from services.installed_app_service import InstalledAppInfo, InstalledAppRecord _USED_AT = datetime(2026, 9, 6, 12, 30, 45) _VIEWER_TENANT_ID = "11111111-1111-4111-8111-111111111111" @@ -199,3 +201,257 @@ def test_record_rolls_back_and_preserves_commit_error( stored = session.get(InstalledApp, installed_app.id) assert stored is not None assert stored.last_used_at is None + + +def _app_with_publication( + session: Session, + *, + name: str = "Shared app", + mode: AppMode = AppMode.COMPLETION, + published: bool = True, +) -> App: + app = App( + tenant_id=_OWNER_TENANT_ID, + name=name, + description="", + mode=mode, + icon_type=IconType.EMOJI, + icon="robot", + icon_background=None, + use_icon_as_answer_icon=False, + is_public=False, + enable_site=False, + enable_api=False, + ) + session.add(app) + session.flush() + if published and mode in (AppMode.WORKFLOW, AppMode.ADVANCED_CHAT): + # Preserve the existing reference-existence check, including draft versions. + workflow = Workflow( + id=str(uuid4()), + tenant_id=app.tenant_id, + app_id=app.id, + type=WorkflowType.WORKFLOW, + kind=WorkflowKind.STANDARD, + version="draft", + graph='{"nodes":[],"edges":[]}', + features="{}", + created_by=str(uuid4()), + environment_variables=[], + conversation_variables=[], + rag_pipeline_variables=[], + ) + session.add(workflow) + app.workflow_id = workflow.id + elif published: + configuration = AppModelConfig(app_id=app.id) + session.add(configuration) + session.flush() + app.app_model_config_id = configuration.id + return app + + +@pytest.mark.parametrize("mode", list(AppMode)) +@pytest.mark.parametrize("publication", ["present", "missing", "dangling"]) +def test_candidate_and_detail_publication_filters_preserve_mode_and_reference_rules( + sqlite_session_factory: sessionmaker[Session], mode: AppMode, publication: str +) -> None: + with sqlite_session_factory.begin() as session: + app = _app_with_publication(session, mode=mode, published=publication == "present") + if publication == "dangling": + if mode in (AppMode.ADVANCED_CHAT, AppMode.WORKFLOW): + app.workflow_id = str(uuid4()) + else: + app.app_model_config_id = str(uuid4()) + installation = _installation(app_id=app.id) + session.add(installation) + session.flush() + reference = _reference(installation) + repository = SQLAlchemyInstalledAppRepository(session_factory=sqlite_session_factory) + + candidates = repository.get_candidates(tenant_id=_VIEWER_TENANT_ID, cursor=None, limit=10, app_id=None, name=None) + detail = repository.get_published(installed_app=reference) + + if publication == "present" and mode != AppMode.AGENT: + assert len(candidates) == 1 + assert candidates[0] == detail + assert candidates[0].app.mode == mode + else: + assert candidates == () + assert detail is None + + +def test_candidates_return_detached_nested_data_and_stored_installation_owner( + sqlite_session_factory: sessionmaker[Session], +) -> None: + stored_owner_id = str(uuid4()) + with sqlite_session_factory.begin() as session: + app = _app_with_publication(session) + installation = _installation(app_id=app.id) + installation.app_owner_tenant_id = stored_owner_id + installation.last_used_at = _USED_AT + session.add(installation) + session.flush() + installation_id, app_id = installation.id, app.id + repository = SQLAlchemyInstalledAppRepository(session_factory=sqlite_session_factory) + + result = repository.get_candidates(tenant_id=_VIEWER_TENANT_ID, cursor=None, limit=10, app_id=None, name=None) + + assert result == ( + InstalledAppRecord( + id=installation_id, + app=InstalledAppInfo( + id=app_id, + name="Shared app", + description="", + mode="completion", + icon_type="emoji", + icon="robot", + icon_background=None, + use_icon_as_answer_icon=False, + ), + app_owner_tenant_id=stored_owner_id, + is_pinned=False, + last_used_at=_USED_AT, + ), + ) + assert inspect(result[0], raiseerr=False) is None + assert inspect(result[0].app, raiseerr=False) is None + + +def test_candidates_order_and_cursor_cover_pin_groups_ties_and_null_dates( + sqlite_session_factory: sessionmaker[Session], +) -> None: + ordered: list[str] = [] + rows = [ + (True, _USED_AT), + (True, _USED_AT), + (True, _USED_AT - timedelta(days=1)), + (True, None), + (True, None), + (False, _USED_AT + timedelta(days=1)), + (False, _USED_AT), + (False, _USED_AT), + (False, None), + (False, None), + ] + with sqlite_session_factory.begin() as session: + for number, (is_pinned, last_used_at) in enumerate(rows, start=1): + app = _app_with_publication(session) + installation = _installation(app_id=app.id) + installation.id = f"00000000-0000-4000-8000-{number:012d}" + installation.is_pinned = is_pinned + installation.last_used_at = last_used_at + session.add(installation) + ordered.append(installation.id) + repository = SQLAlchemyInstalledAppRepository(session_factory=sqlite_session_factory) + + all_rows = repository.get_candidates(tenant_id=_VIEWER_TENANT_ID, cursor=None, limit=20, app_id=None, name=None) + + assert [row.id for row in all_rows] == ordered + for index, row in enumerate(all_rows): + after = repository.get_candidates( + tenant_id=_VIEWER_TENANT_ID, cursor=row.cursor(), limit=20, app_id=None, name=None + ) + assert [candidate.id for candidate in after] == ordered[index + 1 :] + assert repository.get_candidates(tenant_id=_VIEWER_TENANT_ID, cursor=None, limit=0, app_id=None, name=None) == () + assert ( + repository.get_candidates(tenant_id=_VIEWER_TENANT_ID, cursor=None, limit=2, app_id=None, name=None) + == all_rows[:2] + ) + + +@pytest.mark.parametrize("search", [" 50%_\\ ", "50%_\\", "literal"]) +def test_candidates_filter_literal_case_insensitive_name_and_app_with_workspace_scope( + sqlite_session_factory: sessionmaker[Session], search: str +) -> None: + with sqlite_session_factory.begin() as session: + matching = _app_with_publication(session, name="LITERAL 50%_\\ suffix") + another = _app_with_publication(session, name="Literal 5000 suffix") + foreign = _app_with_publication(session, name=matching.name) + session.add_all([_installation(app_id=matching.id), _installation(app_id=another.id)]) + foreign_installation = _installation(app_id=foreign.id) + foreign_installation.tenant_id = _OWNER_TENANT_ID + session.add(foreign_installation) + repository = SQLAlchemyInstalledAppRepository(session_factory=sqlite_session_factory) + + candidates = repository.get_candidates( + tenant_id=_VIEWER_TENANT_ID, cursor=None, limit=10, app_id=matching.id, name=search + ) + + assert [row.app.id for row in candidates] == [matching.id] + if search != "literal": + by_name = repository.get_candidates( + tenant_id=_VIEWER_TENANT_ID, cursor=None, limit=10, app_id=None, name=search + ) + assert [row.app.id for row in by_name] == [matching.id] + assert len(repository.get_candidates(tenant_id=_VIEWER_TENANT_ID, cursor=None, limit=10, app_id="", name=" ")) == 2 + assert ( + repository.get_candidates(tenant_id=_VIEWER_TENANT_ID, cursor=None, limit=10, app_id=foreign.id, name=None) + == () + ) + + +@pytest.mark.parametrize("field", ["id", "tenant_id", "app_id"]) +@pytest.mark.parametrize("operation", ["get_published", "set_pinned"]) +def test_management_operations_require_all_admitted_reference_fields( + sqlite_session_factory: sessionmaker[Session], field: str, operation: str +) -> None: + with sqlite_session_factory.begin() as session: + app = _app_with_publication(session) + installation = _installation(app_id=app.id) + session.add(installation) + session.flush() + repository = SQLAlchemyInstalledAppRepository(session_factory=sqlite_session_factory) + reference = replace(_reference(installation), **{field: str(uuid4())}) + + if operation == "get_published": + assert repository.get_published(installed_app=reference) is None + else: + with pytest.raises(InstalledAppNotFoundError, match=reference.id): + repository.set_pinned(installed_app=reference, is_pinned=True) + + with sqlite_session_factory() as session: + stored = session.get(InstalledApp, installation.id) + assert stored is not None + assert stored.is_pinned is False + + +@pytest.mark.parametrize("is_pinned", [True, False]) +def test_pin_updates_only_pin_state_including_unpublished_apps( + sqlite_session_factory: sessionmaker[Session], installed_app: InstalledApp, is_pinned: bool +) -> None: + repository = SQLAlchemyInstalledAppRepository(session_factory=sqlite_session_factory) + + repository.set_pinned(installed_app=_reference(installed_app), is_pinned=is_pinned) + repository.set_pinned(installed_app=_reference(installed_app), is_pinned=is_pinned) + + with sqlite_session_factory() as session: + stored = session.get(InstalledApp, installed_app.id) + assert stored is not None + assert stored.is_pinned is is_pinned + assert stored.last_used_at is None + assert stored.position == 7 + assert stored.app_owner_tenant_id == installed_app.app_owner_tenant_id + + +def test_pin_rolls_back_and_preserves_database_failure( + sqlite_session_factory: sessionmaker[Session], installed_app: InstalledApp +) -> None: + failure = RuntimeError("installation commit unavailable") + repository_factory = sessionmaker(bind=sqlite_session_factory.kw["bind"], expire_on_commit=False) + + @event.listens_for(repository_factory, "before_commit") + def fail_commit(session: Session) -> None: + session.flush() + raise failure + + repository = SQLAlchemyInstalledAppRepository(session_factory=repository_factory) + with pytest.raises(RuntimeError, match="installation commit unavailable") as caught: + repository.set_pinned(installed_app=_reference(installed_app), is_pinned=False) + + assert caught.value is failure + with sqlite_session_factory() as session: + stored = session.get(InstalledApp, installed_app.id) + assert stored is not None + assert stored.is_pinned is True diff --git a/api/tests/unit_tests/services/test_installed_app_access_service.py b/api/tests/unit_tests/services/test_installed_app_access_service.py index f547a212abc..eabb473b1a9 100644 --- a/api/tests/unit_tests/services/test_installed_app_access_service.py +++ b/api/tests/unit_tests/services/test_installed_app_access_service.py @@ -1,5 +1,9 @@ +from collections.abc import Mapping, Sequence +from dataclasses import dataclass, field + import pytest +from enums import WebAppAccessMode from services.installed_app_access_service import ( InstalledAppAccessDeniedError, InstalledAppAccessService, @@ -29,6 +33,39 @@ class _Store: return self._result +def _unexpected_access_modes(*, app_ids: Sequence[str]) -> Mapping[str, WebAppAccessMode]: + pytest.fail(f"Unexpected batch access-mode query: {app_ids=}") + + +def _unexpected_user_permissions(*, user_id: str, app_ids: Sequence[str]) -> Mapping[str, bool]: + pytest.fail(f"Unexpected batch permission query: {user_id=}, {app_ids=}") + + +def _unexpected_single_permission(*, user_id: str, app_id: str) -> bool: + pytest.fail(f"Visibility must use batch permission checks: {user_id=}, {app_id=}") + + +@dataclass +class _BatchAccess: + modes: Mapping[str, WebAppAccessMode] = field(default_factory=dict) + permissions: Mapping[str, bool] = field(default_factory=dict) + events: list[tuple[str, ...]] = field(default_factory=list) + modes_error: Exception | None = None + permissions_error: Exception | None = None + + def get_access_modes(self, *, app_ids: Sequence[str]) -> Mapping[str, WebAppAccessMode]: + self.events.append(("modes", *app_ids)) + if self.modes_error is not None: + raise self.modes_error + return self.modes + + def get_user_permissions(self, *, user_id: str, app_ids: Sequence[str]) -> Mapping[str, bool]: + self.events.append(("permissions", user_id, *app_ids)) + if self.permissions_error is not None: + raise self.permissions_error + return self.permissions + + @pytest.mark.parametrize("allowed", [True, False]) def test_access_resolves_workspace_installation_before_checking_target_app(allowed: bool) -> None: events: list[tuple[str, ...]] = [] @@ -40,6 +77,8 @@ def test_access_resolves_workspace_installation_before_checking_target_app(allow service = InstalledAppAccessService( installed_apps=_Store(result=_REF, events=events), is_user_allowed=is_user_allowed, + get_access_modes=_unexpected_access_modes, + get_user_permissions=_unexpected_user_permissions, ) if allowed: @@ -60,6 +99,8 @@ def test_missing_installation_does_not_check_permission() -> None: service = InstalledAppAccessService( installed_apps=_Store(result=None, events=events), is_user_allowed=unexpected_permission_check, + get_access_modes=_unexpected_access_modes, + get_user_permissions=_unexpected_user_permissions, ) with pytest.raises(InstalledAppNotFoundError, match="^Installed app not found$"): @@ -80,6 +121,8 @@ def test_dependency_errors_propagate_unchanged(failure_source: str) -> None: service = InstalledAppAccessService( installed_apps=_Store(result=_REF, events=events, error=failure if failure_source == "store" else None), is_user_allowed=is_user_allowed, + get_access_modes=_unexpected_access_modes, + get_user_permissions=_unexpected_user_permissions, ) with pytest.raises(RuntimeError) as raised: @@ -87,3 +130,136 @@ def test_dependency_errors_propagate_unchanged(failure_source: str) -> None: assert raised.value is failure assert len(events) == (1 if failure_source == "store" else 2) + + +def test_visibility_skips_external_queries_when_empty() -> None: + events: list[tuple[str, ...]] = [] + service = InstalledAppAccessService( + installed_apps=_Store(result=_REF, events=events, error=AssertionError("Visibility must not resolve an app")), + is_user_allowed=_unexpected_single_permission, + get_access_modes=_unexpected_access_modes, + get_user_permissions=_unexpected_user_permissions, + ) + + visible = service.get_visible_app_ids(user_id="account-1", app_ids=[]) + + assert visible == frozenset() + assert isinstance(visible, frozenset) + assert events == [] + + +def test_visibility_filters_modes_then_permissions_preserving_batch_order_and_duplicates() -> None: + app_ids = ["public", "sso", "missing", "private", "private-all", "public", "also-public", "denied", "no-permission"] + batches = _BatchAccess( + modes={ + "public": WebAppAccessMode.PUBLIC, + "sso": WebAppAccessMode.SSO_VERIFIED, + "private": WebAppAccessMode.PRIVATE, + "private-all": WebAppAccessMode.PRIVATE_ALL, + "also-public": WebAppAccessMode.PUBLIC, + "denied": WebAppAccessMode.PRIVATE, + "no-permission": WebAppAccessMode.PUBLIC, + }, + permissions={ + "public": True, + "private": True, + "private-all": True, + "also-public": True, + "denied": False, + "sso": True, + "missing": True, + "not-requested": True, + }, + ) + events: list[tuple[str, ...]] = [] + service = InstalledAppAccessService( + installed_apps=_Store(result=_REF, events=events, error=AssertionError("Visibility must not resolve an app")), + is_user_allowed=_unexpected_single_permission, + get_access_modes=batches.get_access_modes, + get_user_permissions=batches.get_user_permissions, + ) + + visible = service.get_visible_app_ids(user_id="account-1", app_ids=app_ids) + + assert visible == frozenset({"public", "private", "private-all", "also-public"}) + assert isinstance(visible, frozenset) + assert batches.events == [ + ("modes", *app_ids), + ( + "permissions", + "account-1", + "public", + "private", + "private-all", + "public", + "also-public", + "denied", + "no-permission", + ), + ] + assert events == [] + + +@pytest.mark.parametrize("app_ids", [["missing"], ["sso", "missing", "sso"]]) +def test_visibility_skips_permission_query_when_no_mode_is_eligible(app_ids: list[str]) -> None: + batches = _BatchAccess( + modes={"sso": WebAppAccessMode.SSO_VERIFIED}, + permissions_error=AssertionError("Empty candidates must not reach the permission provider"), + ) + service = InstalledAppAccessService( + installed_apps=_Store(result=_REF, events=[], error=AssertionError("Visibility must not resolve an app")), + is_user_allowed=_unexpected_single_permission, + get_access_modes=batches.get_access_modes, + get_user_permissions=batches.get_user_permissions, + ) + + assert service.get_visible_app_ids(user_id="account-1", app_ids=app_ids) == frozenset() + assert batches.events == [("modes", *app_ids)] + + +@pytest.mark.parametrize("failure_source", ["modes", "permissions"]) +def test_visibility_preserves_dependency_exception_and_query_order(failure_source: str) -> None: + failure = RuntimeError("Enterprise request failed") + batches = _BatchAccess(modes={"app-1": WebAppAccessMode.PUBLIC}) + if failure_source == "modes": + batches.modes_error = failure + else: + batches.permissions_error = failure + service = InstalledAppAccessService( + installed_apps=_Store(result=_REF, events=[], error=AssertionError("Visibility must not resolve an app")), + is_user_allowed=_unexpected_single_permission, + get_access_modes=batches.get_access_modes, + get_user_permissions=batches.get_user_permissions, + ) + + with pytest.raises(RuntimeError) as raised: + service.get_visible_app_ids(user_id="account-1", app_ids=["app-1"]) + + assert raised.value is failure + assert batches.events == ( + [("modes", "app-1")] + if failure_source == "modes" + else [("modes", "app-1"), ("permissions", "account-1", "app-1")] + ) + + +def test_single_app_admission_does_not_apply_list_sso_filter() -> None: + events: list[tuple[str, ...]] = [] + batches = _BatchAccess(modes={_REF.app_id: WebAppAccessMode.SSO_VERIFIED}, permissions={_REF.app_id: False}) + + def allowed(*, user_id: str, app_id: str) -> bool: + events.append(("authorize", user_id, app_id)) + return True + + service = InstalledAppAccessService( + installed_apps=_Store(result=_REF, events=events), + is_user_allowed=allowed, + get_access_modes=batches.get_access_modes, + get_user_permissions=batches.get_user_permissions, + ) + + admitted = service.get_access(installed_app_id=_REF.id, tenant_id=_REF.tenant_id, account_id="account-1") + + assert admitted is _REF + assert batches.events == [] + assert events == [("resolve", _REF.id, _REF.tenant_id), ("authorize", "account-1", _REF.app_id)] diff --git a/api/tests/unit_tests/services/test_webapp_access_query_service.py b/api/tests/unit_tests/services/test_webapp_access_query_service.py index 36007fccaeb..096168d7fd4 100644 --- a/api/tests/unit_tests/services/test_webapp_access_query_service.py +++ b/api/tests/unit_tests/services/test_webapp_access_query_service.py @@ -1,3 +1,5 @@ +from collections.abc import Mapping, Sequence +from dataclasses import dataclass, field from unittest.mock import MagicMock, create_autospec import pytest @@ -11,6 +13,14 @@ from services.webapp_access_query_service import ( ) +def _unexpected_access_modes(*, app_ids: Sequence[str]) -> Mapping[str, WebAppAccessMode]: + raise AssertionError(f"Unexpected batch access mode query: {app_ids}") + + +def _unexpected_user_permissions(*, user_id: str, app_ids: Sequence[str]) -> Mapping[str, bool]: + raise AssertionError(f"Unexpected batch permission query: {user_id}, {app_ids}") + + def _service( *, access: MagicMock, @@ -26,6 +36,8 @@ def _service( webapp_auth_enabled=enabled, access_mode_for_app=access_mode_for_app, is_user_allowed_for_app=is_user_allowed_for_app, + get_access_modes=_unexpected_access_modes, + get_user_permissions=_unexpected_user_permissions, ), access_mode_for_app, is_user_allowed_for_app, @@ -148,3 +160,111 @@ def test_enabled_auth_delegates_user_permission() -> None: assert service.is_user_allowed(user_id="user-1", app_id="app-1") is False is_user_allowed_for_app.assert_called_once_with("user-1", "app-1") + + +@dataclass +class _BatchQueries: + access_modes: Mapping[str, WebAppAccessMode] = field(default_factory=dict) + user_permissions: Mapping[str, bool] = field(default_factory=dict) + mode_requests: list[tuple[str, ...]] = field(default_factory=list) + permission_requests: list[tuple[str, tuple[str, ...]]] = field(default_factory=list) + mode_error: Exception | None = None + permission_error: Exception | None = None + + def get_access_modes(self, *, app_ids: Sequence[str]) -> Mapping[str, WebAppAccessMode]: + self.mode_requests.append(tuple(app_ids)) + if self.mode_error is not None: + raise self.mode_error + return self.access_modes + + def get_user_permissions(self, *, user_id: str, app_ids: Sequence[str]) -> Mapping[str, bool]: + self.permission_requests.append((user_id, tuple(app_ids))) + if self.permission_error is not None: + raise self.permission_error + return self.user_permissions + + def find_app_id_by_code(self, app_code: str) -> str | None: + raise AssertionError(f"Batch queries must not resolve app codes: {app_code}") + + def get_access_mode(self, app_id: str) -> WebAppAccessMode: + raise AssertionError(f"Batch queries must not use single-app lookups: {app_id}") + + def is_user_allowed(self, user_id: str, app_id: str) -> bool: + raise AssertionError(f"Batch queries must not use single-app permissions: {user_id}, {app_id}") + + +def _batch_service(queries: _BatchQueries, *, enabled: bool = True) -> WebAppAccessQueryService: + return WebAppAccessQueryService( + access=queries, + webapp_auth_enabled=enabled, + access_mode_for_app=queries.get_access_mode, + is_user_allowed_for_app=queries.is_user_allowed, + get_access_modes=queries.get_access_modes, + get_user_permissions=queries.get_user_permissions, + ) + + +@pytest.mark.parametrize( + ("enabled", "app_ids"), + [ + pytest.param(False, ("app-b", "app-a", "app-b"), id="disabled-auth"), + pytest.param(False, (), id="disabled-empty"), + pytest.param(True, (), id="enabled-empty"), + ], +) +def test_disabled_or_empty_batch_queries_return_public_access_without_external_calls( + enabled: bool, app_ids: tuple[str, ...] +) -> None: + queries = _BatchQueries( + mode_error=AssertionError("Must not query modes"), + permission_error=AssertionError("Must not query permissions"), + ) + service = _batch_service(queries, enabled=enabled) + + assert service.batch_get_access_modes(app_ids=app_ids) == dict.fromkeys(app_ids, WebAppAccessMode.PUBLIC) + assert service.batch_get_user_permissions(user_id="viewer", app_ids=app_ids) == dict.fromkeys(app_ids, True) + assert queries.mode_requests == [] + assert queries.permission_requests == [] + + +def test_enabled_batch_queries_preserve_order_duplicates_missing_results_and_sso_mode() -> None: + app_ids = ("private", "sso", "public", "private", "missing") + queries = _BatchQueries( + access_modes={ + "private": WebAppAccessMode.PRIVATE, + "sso": WebAppAccessMode.SSO_VERIFIED, + "public": WebAppAccessMode.PUBLIC, + }, + user_permissions={"private": False, "sso": True, "public": True}, + ) + service = _batch_service(queries) + + modes = service.batch_get_access_modes(app_ids=app_ids) + permissions = service.batch_get_user_permissions(user_id="viewer", app_ids=app_ids) + + assert modes == queries.access_modes + assert modes["sso"] is WebAppAccessMode.SSO_VERIFIED + assert "missing" not in modes + assert permissions == queries.user_permissions + assert "missing" not in permissions + assert queries.mode_requests == [app_ids] + assert queries.permission_requests == [("viewer", app_ids)] + + +@pytest.mark.parametrize("failure_stage", ["modes", "permissions"]) +@pytest.mark.parametrize("failure", [ConnectionError("batch unavailable"), ValueError("invalid batch response")]) +def test_batch_callback_errors_propagate_without_wrapping(failure_stage: str, failure: Exception) -> None: + queries = _BatchQueries(mode_error=failure, permission_error=failure) + service = _batch_service(queries) + + if failure_stage == "modes": + with pytest.raises(type(failure)) as raised: + service.batch_get_access_modes(app_ids=("app-1",)) + assert queries.mode_requests == [("app-1",)] + assert queries.permission_requests == [] + else: + with pytest.raises(type(failure)) as raised: + service.batch_get_user_permissions(user_id="viewer", app_ids=("app-1",)) + assert queries.mode_requests == [] + assert queries.permission_requests == [("viewer", ("app-1",))] + assert raised.value is failure diff --git a/packages/contracts/generated/api/console/installed-apps/orpc.gen.ts b/packages/contracts/generated/api/console/installed-apps/orpc.gen.ts index 1bd3bc08ccf..c3e97542601 100644 --- a/packages/contracts/generated/api/console/installed-apps/orpc.gen.ts +++ b/packages/contracts/generated/api/console/installed-apps/orpc.gen.ts @@ -5,8 +5,6 @@ import * as z from 'zod' import { zDeleteInstalledAppsByInstalledAppIdConversationsByCIdPath, zDeleteInstalledAppsByInstalledAppIdConversationsByCIdResponse, - zDeleteInstalledAppsByInstalledAppIdPath, - zDeleteInstalledAppsByInstalledAppIdResponse, zDeleteInstalledAppsByInstalledAppIdSavedMessagesByMessageIdPath, zDeleteInstalledAppsByInstalledAppIdSavedMessagesByMessageIdResponse, zGetInstalledAppsByInstalledAppIdConversationsPath, @@ -38,7 +36,6 @@ import { zPatchInstalledAppsByInstalledAppIdConversationsByCIdUnpinResponse, zPatchInstalledAppsByInstalledAppIdPath, zPatchInstalledAppsByInstalledAppIdResponse, - zPostInstalledAppsBody, zPostInstalledAppsByInstalledAppIdAudioToTextPath, zPostInstalledAppsByInstalledAppIdAudioToTextResponse, zPostInstalledAppsByInstalledAppIdChatMessagesBody, @@ -68,7 +65,6 @@ import { zPostInstalledAppsByInstalledAppIdWorkflowsRunResponse, zPostInstalledAppsByInstalledAppIdWorkflowsTasksByTaskIdStopPath, zPostInstalledAppsByInstalledAppIdWorkflowsTasksByTaskIdStopResponse, - zPostInstalledAppsResponse, } from './zod.gen.ts' export const post = oc @@ -510,18 +506,6 @@ export const workflows = { tasks, } -export const delete3 = oc - .route({ - inputStructure: 'detailed', - method: 'DELETE', - operationId: 'deleteInstalledAppsByInstalledAppId', - path: '/installed-apps/{installed_app_id}', - successStatus: 204, - tags: ['console'], - }) - .input(z.object({ params: zDeleteInstalledAppsByInstalledAppIdPath })) - .output(zDeleteInstalledAppsByInstalledAppIdResponse) - export const get8 = oc .route({ inputStructure: 'detailed', @@ -550,7 +534,6 @@ export const patch3 = oc .output(zPatchInstalledAppsByInstalledAppIdResponse) export const byInstalledAppId = { - delete: delete3, get: get8, patch: patch3, audioToText, @@ -576,20 +559,8 @@ export const get9 = oc .input(z.object({ query: zGetInstalledAppsQuery.optional() })) .output(zGetInstalledAppsResponse) -export const post12 = oc - .route({ - inputStructure: 'detailed', - method: 'POST', - operationId: 'postInstalledApps', - path: '/installed-apps', - tags: ['console'], - }) - .input(z.object({ body: zPostInstalledAppsBody })) - .output(zPostInstalledAppsResponse) - export const installedApps = { get: get9, - post: post12, byInstalledAppId, } diff --git a/packages/contracts/generated/api/console/installed-apps/types.gen.ts b/packages/contracts/generated/api/console/installed-apps/types.gen.ts index 0c87032f3f2..37d345e93bc 100644 --- a/packages/contracts/generated/api/console/installed-apps/types.gen.ts +++ b/packages/contracts/generated/api/console/installed-apps/types.gen.ts @@ -10,14 +10,6 @@ export type InstalledAppListResponse = { next_cursor: string | null } -export type InstalledAppCreatePayload = { - app_id: string -} - -export type SimpleMessageResponse = { - message: string -} - export type InstalledAppResponse = { app: InstalledAppInfoResponse app_owner_tenant_id: string @@ -25,7 +17,6 @@ export type InstalledAppResponse = { id: string is_pinned: boolean last_used_at: number | null - uninstallable: boolean } export type InstalledAppUpdatePayload = { @@ -768,7 +759,6 @@ export type InstalledAppResponseWritable = { id: string is_pinned: boolean last_used_at: number | null - uninstallable: boolean } export type ExploreMessageInfiniteScrollPaginationWritable = { @@ -831,35 +821,6 @@ export type GetInstalledAppsResponses = { export type GetInstalledAppsResponse = GetInstalledAppsResponses[keyof GetInstalledAppsResponses] -export type PostInstalledAppsData = { - body: InstalledAppCreatePayload - path?: never - query?: never - url: '/installed-apps' -} - -export type PostInstalledAppsResponses = { - 200: SimpleMessageResponse -} - -export type PostInstalledAppsResponse = PostInstalledAppsResponses[keyof PostInstalledAppsResponses] - -export type DeleteInstalledAppsByInstalledAppIdData = { - body?: never - path: { - installed_app_id: string - } - query?: never - url: '/installed-apps/{installed_app_id}' -} - -export type DeleteInstalledAppsByInstalledAppIdResponses = { - 204: void -} - -export type DeleteInstalledAppsByInstalledAppIdResponse = - DeleteInstalledAppsByInstalledAppIdResponses[keyof DeleteInstalledAppsByInstalledAppIdResponses] - export type GetInstalledAppsByInstalledAppIdData = { body?: never path: { diff --git a/packages/contracts/generated/api/console/installed-apps/zod.gen.ts b/packages/contracts/generated/api/console/installed-apps/zod.gen.ts index fdc49e41844..93bb6ab4e52 100644 --- a/packages/contracts/generated/api/console/installed-apps/zod.gen.ts +++ b/packages/contracts/generated/api/console/installed-apps/zod.gen.ts @@ -2,20 +2,6 @@ import * as z from 'zod' -/** - * InstalledAppCreatePayload - */ -export const zInstalledAppCreatePayload = z.object({ - app_id: z.string(), -}) - -/** - * SimpleMessageResponse - */ -export const zSimpleMessageResponse = z.object({ - message: z.string(), -}) - /** * InstalledAppUpdatePayload */ @@ -619,7 +605,6 @@ export const zInstalledAppResponse = z.object({ id: z.string(), is_pinned: z.boolean(), last_used_at: z.int().nullable(), - uninstallable: z.boolean(), }) /** @@ -941,7 +926,6 @@ export const zInstalledAppResponseWritable = z.object({ id: z.string(), is_pinned: z.boolean(), last_used_at: z.int().nullable(), - uninstallable: z.boolean(), }) /** @@ -1003,22 +987,6 @@ export const zGetInstalledAppsQuery = z.object({ */ export const zGetInstalledAppsResponse = zInstalledAppListResponse -export const zPostInstalledAppsBody = zInstalledAppCreatePayload - -/** - * Success - */ -export const zPostInstalledAppsResponse = zSimpleMessageResponse - -export const zDeleteInstalledAppsByInstalledAppIdPath = z.object({ - installed_app_id: z.uuid(), -}) - -/** - * App uninstalled successfully - */ -export const zDeleteInstalledAppsByInstalledAppIdResponse = z.void() - export const zGetInstalledAppsByInstalledAppIdPath = z.object({ installed_app_id: z.uuid(), }) diff --git a/web/app/components/apps/__tests__/app-card.spec.tsx b/web/app/components/apps/__tests__/app-card.spec.tsx index acadece4488..63361a94a35 100644 --- a/web/app/components/apps/__tests__/app-card.spec.tsx +++ b/web/app/components/apps/__tests__/app-card.spec.tsx @@ -451,7 +451,6 @@ describe('AppCard', () => { editable: false, is_pinned: false, last_used_at: null, - uninstallable: true, app: { id: mockApp.id, name: mockApp.name, diff --git a/web/app/components/base/chat/chat-with-history/__tests__/hooks.spec.tsx b/web/app/components/base/chat/chat-with-history/__tests__/hooks.spec.tsx index d3bc4c2f809..f47064aca93 100644 --- a/web/app/components/base/chat/chat-with-history/__tests__/hooks.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/__tests__/hooks.spec.tsx @@ -1234,7 +1234,6 @@ describe('useChatWithHistory', () => { editable: true, is_pinned: false, last_used_at: null, - uninstallable: true, app: { id: 'app-id', name: 'Installed App', diff --git a/web/app/components/explore/installed-app-navigation/__tests__/app-nav-item.spec.tsx b/web/app/components/explore/installed-app-navigation/__tests__/app-nav-item.spec.tsx index bc3db596c1f..54d9e81b94a 100644 --- a/web/app/components/explore/installed-app-navigation/__tests__/app-nav-item.spec.tsx +++ b/web/app/components/explore/installed-app-navigation/__tests__/app-nav-item.spec.tsx @@ -28,7 +28,6 @@ const baseProps = { editable: true, is_pinned: false, last_used_at: null, - uninstallable: false, app: { id: 'source-app-123', name: 'My App', @@ -43,7 +42,6 @@ const baseProps = { } satisfies InstalledAppResponse, isSelected: false, onTogglePin: vi.fn(), - onDelete: vi.fn(), } describe('AppNavItem', () => { @@ -108,18 +106,6 @@ describe('AppNavItem', () => { expect(link).toHaveAttribute('aria-current', 'page') }) - it('should call onDelete with app id when delete action is clicked', async () => { - const user = userEvent.setup() - render() - - await user.click( - screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), - ) - await user.click(await screen.findByText('explore.sidebar.action.delete')) - - expect(baseProps.onDelete).toHaveBeenCalledWith('app-123') - }) - it('should request the next pin state', async () => { const user = userEvent.setup() render() @@ -132,25 +118,4 @@ describe('AppNavItem', () => { expect(baseProps.onTogglePin).toHaveBeenCalledWith('app-123', true) }) }) - - describe('Edge Cases', () => { - it('should not render delete action when app is uninstallable', async () => { - const user = userEvent.setup() - render( - , - ) - - await user.click( - screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), - ) - - expect(screen.queryByText('explore.sidebar.action.delete')).not.toBeInTheDocument() - }) - }) }) diff --git a/web/app/components/explore/installed-app-navigation/app-nav-item.tsx b/web/app/components/explore/installed-app-navigation/app-nav-item.tsx index 9a10a3c05d1..5cfdc5c3c3a 100644 --- a/web/app/components/explore/installed-app-navigation/app-nav-item.tsx +++ b/web/app/components/explore/installed-app-navigation/app-nav-item.tsx @@ -10,20 +10,17 @@ type IAppNavItemProps = { app: InstalledAppResponse isSelected: boolean onTogglePin: (id: string, isPinned: boolean) => void - onDelete: (id: string) => void } export default function AppNavItem({ app: installedApp, isSelected, onTogglePin, - onDelete, }: IAppNavItemProps) { const [isPrefetchEnabled, setIsPrefetchEnabled] = useState(false) const { id, is_pinned: isPinned, - uninstallable, app: { name, icon_type, icon, icon_background, icon_url }, } = installedApp const url = buildInstalledAppPath(id) @@ -59,8 +56,6 @@ export default function AppNavItem({ itemName={name} isPinned={isPinned} togglePin={() => onTogglePin(id, !isPinned)} - isShowDelete={!uninstallable && !isSelected} - onDelete={() => onDelete(id)} /> diff --git a/web/app/components/explore/installed-app/__tests__/index.spec.tsx b/web/app/components/explore/installed-app/__tests__/index.spec.tsx index 5c3de1785db..1eb5b2fc023 100644 --- a/web/app/components/explore/installed-app/__tests__/index.spec.tsx +++ b/web/app/components/explore/installed-app/__tests__/index.spec.tsx @@ -92,7 +92,6 @@ const createInstalledApp = (mode: AppMode = 'chat'): InstalledAppResponse => ({ editable: true, is_pinned: false, last_used_at: null, - uninstallable: true, app: { id: 'app-123', name: 'Test App', diff --git a/web/app/components/explore/item-operation/__tests__/index.spec.tsx b/web/app/components/explore/item-operation/__tests__/index.spec.tsx index 5ff9b34493e..f21b8cf9d6e 100644 --- a/web/app/components/explore/item-operation/__tests__/index.spec.tsx +++ b/web/app/components/explore/item-operation/__tests__/index.spec.tsx @@ -1,4 +1,5 @@ -import { fireEvent, render, screen } from '@testing-library/react' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import * as React from 'react' import ItemOperation from '../index' @@ -11,9 +12,7 @@ describe('ItemOperation', () => { const props: React.ComponentProps = { itemName: 'My App', isPinned: false, - isShowDelete: true, togglePin: vi.fn(), - onDelete: vi.fn(), ...overrides, } return { @@ -26,20 +25,8 @@ describe('ItemOperation', () => { it('should distinguish operation triggers by item name', () => { render( <> - - + + , ) @@ -51,23 +38,24 @@ describe('ItemOperation', () => { ).toBeInTheDocument() }) - it('should render pin and delete actions when menu is open', async () => { + it('should render pin action when menu is open', async () => { + const user = userEvent.setup() renderComponent() - fireEvent.click( + await user.click( screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), ) expect(await screen.findByText('explore.sidebar.action.pin')).toBeInTheDocument() - expect(screen.getByText('explore.sidebar.action.delete')).toBeInTheDocument() }) }) describe('Props', () => { it('should render rename action when isShowRenameConversation is true', async () => { + const user = userEvent.setup() renderComponent({ isShowRenameConversation: true }) - fireEvent.click( + await user.click( screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), ) @@ -75,9 +63,10 @@ describe('ItemOperation', () => { }) it('should render unpin label when isPinned is true', async () => { + const user = userEvent.setup() renderComponent({ isPinned: true }) - fireEvent.click( + await user.click( screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), ) @@ -87,38 +76,29 @@ describe('ItemOperation', () => { describe('User Interactions', () => { it('should call togglePin when clicking pin action', async () => { + const user = userEvent.setup() const { props } = renderComponent() - fireEvent.click( + await user.click( screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), ) - fireEvent.click(await screen.findByText('explore.sidebar.action.pin')) + await user.click(await screen.findByText('explore.sidebar.action.pin')) expect(props.togglePin).toHaveBeenCalledTimes(1) }) - it('should call onDelete when clicking delete action', async () => { - const { props } = renderComponent() - - fireEvent.click( - screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), - ) - fireEvent.click(await screen.findByText('explore.sidebar.action.delete')) - - expect(props.onDelete).toHaveBeenCalledTimes(1) - }) - it('should call onRenameConversation when clicking rename action', async () => { + const user = userEvent.setup() const onRenameConversation = vi.fn() renderComponent({ isShowRenameConversation: true, onRenameConversation, }) - fireEvent.click( + await user.click( screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), ) - fireEvent.click(await screen.findByText('explore.sidebar.action.rename')) + await user.click(await screen.findByText('explore.sidebar.action.rename')) expect(onRenameConversation).toHaveBeenCalledTimes(1) }) @@ -126,8 +106,9 @@ describe('ItemOperation', () => { describe('Edge Cases', () => { it('should keep the menu open after rerender', async () => { + const user = userEvent.setup() const { props, rerender } = renderComponent() - fireEvent.click( + await user.click( screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), ) await screen.findByText('explore.sidebar.action.pin') @@ -138,25 +119,20 @@ describe('ItemOperation', () => { }) it('should stop propagation when clicking menu actions', async () => { + const user = userEvent.setup() const onParentClick = vi.fn() const togglePin = vi.fn() render(
- +
, ) - fireEvent.click( + await user.click( screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*My App/ }), ) - fireEvent.click(await screen.findByText('explore.sidebar.action.pin')) + await user.click(await screen.findByText('explore.sidebar.action.pin')) expect(togglePin).toHaveBeenCalledTimes(1) expect(onParentClick).not.toHaveBeenCalled() diff --git a/web/app/components/explore/item-operation/index.tsx b/web/app/components/explore/item-operation/index.tsx index 0ca1c6fb24a..5349ac1857d 100644 --- a/web/app/components/explore/item-operation/index.tsx +++ b/web/app/components/explore/item-operation/index.tsx @@ -16,9 +16,7 @@ type IItemOperationProps = { isPinned: boolean isShowRenameConversation?: boolean onRenameConversation?: () => void - isShowDelete: boolean togglePin: () => void - onDelete: () => void } function ItemOperation({ @@ -28,8 +26,6 @@ function ItemOperation({ togglePin, isShowRenameConversation, onRenameConversation, - isShowDelete, - onDelete, }: IItemOperationProps) { const { t } = useTranslation(['explore']) const { t: tCommon } = useTranslation(['common']) @@ -79,30 +75,6 @@ function ItemOperation({ {t(($) => $['sidebar.action.rename'])} )} - {isShowDelete && ( - { - e.stopPropagation() - onDelete() - }} - > - - - {t(($) => $['sidebar.action.delete'])} - - - )} ) diff --git a/web/app/components/explore/item-operation/style.module.css b/web/app/components/explore/item-operation/style.module.css index 78d270db2d3..0a60212baf7 100644 --- a/web/app/components/explore/item-operation/style.module.css +++ b/web/app/components/explore/item-operation/style.module.css @@ -7,7 +7,3 @@ .actionName { @apply text-sm text-text-secondary; } - -.deleteActionItem:hover .deleteActionItemChild { - @apply text-red-500; -} diff --git a/web/app/components/main-nav/__tests__/index.spec.tsx b/web/app/components/main-nav/__tests__/index.spec.tsx index 4c65bb7b146..46e189d1616 100644 --- a/web/app/components/main-nav/__tests__/index.spec.tsx +++ b/web/app/components/main-nav/__tests__/index.spec.tsx @@ -53,7 +53,6 @@ const { mockIsAgentV2Enabled, mockSwitchWorkspace, mockToastSuccess, - mockUninstall, mockUpdatePinStatus, } = vi.hoisted(() => ({ mockFetchNextInstalledAppsPage: vi.fn(), @@ -61,7 +60,6 @@ const { mockIsAgentV2Enabled: vi.fn(() => true), mockSwitchWorkspace: vi.fn(), mockToastSuccess: vi.fn(), - mockUninstall: vi.fn(), mockUpdatePinStatus: vi.fn(), })) const mockStepByStepTour = vi.hoisted(() => { @@ -340,11 +338,6 @@ vi.mock('@/service/console', async (importOriginal) => { }), }, byInstalledAppId: { - delete: { - mutationOptions: () => ({ - mutationFn: (input: unknown) => mockUninstall(input), - }), - }, patch: { mutationOptions: () => ({ mutationFn: (input: unknown) => mockUpdatePinStatus(input), @@ -474,7 +467,6 @@ const createInstalledApp = ( app_owner_tenant_id: overrides.app_owner_tenant_id ?? 'tenant-1', editable: overrides.editable ?? true, last_used_at: overrides.last_used_at ?? null, - uninstallable: overrides.uninstallable ?? false, is_pinned: overrides.is_pinned ?? false, app: { id: overrides.app?.id ?? 'app-1', @@ -676,7 +668,6 @@ describe('MainNav', () => { } }, ) - mockUninstall.mockResolvedValue(undefined) mockUpdatePinStatus.mockResolvedValue({ result: 'success', message: 'updated' }) mockSwitchWorkspace.mockReturnValue(new Promise(() => {})) }) @@ -1694,10 +1685,9 @@ describe('MainNav', () => { expect(screen.getByText('Alpha App')).toBeInTheDocument() }) - it('updates pin status and reuses the existing delete confirmation for installed web apps', async () => { + it('updates pin status for installed web apps', async () => { const user = userEvent.setup() mockInstalledApps = [createInstalledApp()] - mockUninstall.mockResolvedValue(undefined) mockUpdatePinStatus.mockResolvedValue(undefined) renderMainNav() @@ -1714,19 +1704,5 @@ describe('MainNav', () => { body: { is_pinned: true }, }) }) - - await user.hover(screen.getByText('Alpha App')) - await user.click( - screen.getByRole('button', { name: /common\.operation\.moreActionsFor.*Alpha App/ }), - ) - await user.click(await screen.findByText('explore.sidebar.action.delete')) - await user.click(await screen.findByText('common.operation.confirm')) - - await waitFor(() => { - expect(mockUninstall).toHaveBeenCalledWith({ - params: { installed_app_id: 'installed-1' }, - }) - expect(mockToastSuccess).toHaveBeenCalledWith('common.api.remove') - }) }) }) diff --git a/web/app/components/main-nav/components/__tests__/web-apps-section.browser.spec.tsx b/web/app/components/main-nav/components/__tests__/web-apps-section.browser.spec.tsx index b9e4cab5b3e..477efb4f308 100644 --- a/web/app/components/main-nav/components/__tests__/web-apps-section.browser.spec.tsx +++ b/web/app/components/main-nav/components/__tests__/web-apps-section.browser.spec.tsx @@ -10,7 +10,6 @@ import WebAppsSection from '../web-apps-section' const service = vi.hoisted(() => ({ list: vi.fn<() => Promise>(), - remove: vi.fn<() => Promise>(), pin: vi.fn<() => Promise>(), })) @@ -32,7 +31,6 @@ vi.mock('@/service/console', () => ({ }), }, byInstalledAppId: { - delete: { mutationOptions: () => ({ mutationFn: service.remove }) }, patch: { mutationOptions: () => ({ mutationFn: service.pin }) }, }, }, @@ -45,7 +43,6 @@ function createApp(index: number): InstalledAppResponse { app_owner_tenant_id: 'tenant-1', editable: true, last_used_at: null, - uninstallable: false, is_pinned: index === 0, app: { id: `app-${index}`, @@ -93,7 +90,6 @@ async function renderSection(count = 40) { beforeEach(() => { vi.clearAllMocks() - service.remove.mockResolvedValue(undefined) service.pin.mockResolvedValue(undefined) }) diff --git a/web/app/components/main-nav/components/__tests__/web-apps-section.spec.tsx b/web/app/components/main-nav/components/__tests__/web-apps-section.spec.tsx index 3c366756328..e0284337db8 100644 --- a/web/app/components/main-nav/components/__tests__/web-apps-section.spec.tsx +++ b/web/app/components/main-nav/components/__tests__/web-apps-section.spec.tsx @@ -13,7 +13,6 @@ vi.mock('@tanstack/react-virtual') const service = vi.hoisted(() => ({ list: vi.fn<() => Promise>(), - remove: vi.fn<() => Promise>(), pin: vi.fn<() => Promise>(), })) @@ -35,7 +34,6 @@ vi.mock('@/service/console', () => ({ }), }, byInstalledAppId: { - delete: { mutationOptions: () => ({ mutationFn: service.remove }) }, patch: { mutationOptions: () => ({ mutationFn: service.pin }) }, }, }, @@ -48,7 +46,6 @@ function createApp(index: number): InstalledAppResponse { app_owner_tenant_id: 'tenant-1', editable: true, last_used_at: null, - uninstallable: false, is_pinned: index === 0, app: { id: `app-${index}`, @@ -84,7 +81,6 @@ async function renderSection(count = 2) { beforeEach(() => { vi.clearAllMocks() - service.remove.mockResolvedValue(undefined) service.pin.mockResolvedValue(undefined) }) @@ -107,37 +103,39 @@ it('leaves only the web apps fallback on the server without running its client p expect(onBrowserBailout).toHaveBeenCalledOnce() }) -it('restores the menu trigger on cancel and blocks Escape during deletion', async () => { - const { user } = await renderSection() - const trigger = screen.getByRole('button', { - name: 'common.operation.moreActionsFor:{"name":"App 0"}', - }) - await user.click(trigger) - await user.click(await screen.findByRole('menuitem', { name: 'explore.sidebar.action.delete' })) - await user.click(await screen.findByRole('button', { name: 'common.operation.cancel' })) - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()) - expect(trigger).toHaveFocus() - - let finish!: () => void - service.remove.mockImplementation( - () => - new Promise((resolve) => { - finish = resolve +it.each([ + { index: 0, action: 'unpin', nextPinned: false }, + { index: 1, action: 'pin', nextPinned: true }, +])( + 'offers $action without a delete action for installed web apps', + async ({ index, action, nextPinned }) => { + const { user } = await renderSection() + await user.click( + screen.getByRole('button', { + name: `common.operation.moreActionsFor:{"name":"App ${index}"}`, }), - ) - await user.click(trigger) - await user.click(await screen.findByRole('menuitem', { name: 'explore.sidebar.action.delete' })) - await user.click(await screen.findByRole('button', { name: 'common.operation.confirm' })) - try { - await user.keyboard('{Escape}') - expect(screen.getByRole('alertdialog')).toBeInTheDocument() - } finally { - await act(async () => { - finish() + ) + const pinAction = await screen.findByRole('menuitem', { + name: `explore.sidebar.action.${action}`, }) - } - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()) -}) + expect( + screen.queryByRole('menuitem', { name: 'explore.sidebar.action.delete' }), + ).not.toBeInTheDocument() + + await user.click(pinAction) + + await waitFor(() => + expect(service.pin).toHaveBeenCalledWith( + { + params: { installed_app_id: `installed-${index}` }, + body: { is_pinned: nextPinned }, + }, + expect.anything(), + ), + ) + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument() + }, +) it('keeps the search input focused when a refresh removes all apps', async () => { const { queryClient, user } = await renderSection(1) @@ -154,21 +152,6 @@ it('keeps the search input focused when a refresh removes all apps', async () => expect(search).toHaveFocus() }) -it('returns focus to the section control after deleting the final app', async () => { - const { queryClient, user } = await renderSection(1) - service.remove.mockImplementation(async () => { - service.list.mockResolvedValue({ installed_apps: [], has_more: false, next_cursor: null }) - await queryClient.invalidateQueries({ queryKey: ['installed-apps'] }) - }) - await user.click( - screen.getByRole('button', { name: 'common.operation.moreActionsFor:{"name":"App 0"}' }), - ) - await user.click(await screen.findByRole('menuitem', { name: 'explore.sidebar.action.delete' })) - await user.click(await screen.findByRole('button', { name: 'common.operation.confirm' })) - await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()) - expect(screen.getByRole('button', { name: 'explore.sidebar.webApps' })).toHaveFocus() -}) - it('keeps focus when clearing empty search results after the unfiltered cache expires', async () => { const { queryClient, user } = await renderSection() const toggle = screen.getByRole('button', { name: 'common.operation.search' }) diff --git a/web/app/components/main-nav/components/web-apps-section.tsx b/web/app/components/main-nav/components/web-apps-section.tsx index a731ea91bd8..ab816f2ff2e 100644 --- a/web/app/components/main-nav/components/web-apps-section.tsx +++ b/web/app/components/main-nav/components/web-apps-section.tsx @@ -5,15 +5,6 @@ import type { InstalledAppResponse, } from '@dify/contracts/api/console/installed-apps/types.gen' import type { InfiniteData } from '@tanstack/react-query' -import { - AlertDialog, - AlertDialogActions, - AlertDialogCancelButton, - AlertDialogConfirmButton, - AlertDialogContent, - AlertDialogDescription, - AlertDialogTitle, -} from '@langgenius/dify-ui/alert-dialog' import { Button } from '@langgenius/dify-ui/button' import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from '@langgenius/dify-ui/collapsible' import { IconButton } from '@langgenius/dify-ui/icon-button' @@ -70,14 +61,12 @@ const WebAppsSectionContent = () => { const { t } = useTranslation(['common', 'explore']) const pathname = usePathname() const scrollRef = useRef(null) - const sectionToggleRef = useRef(null) const searchFocusRequestedRef = useRef(false) const sectionLabelId = useId() const [lastFocusedAppId, setLastFocusedAppId] = useState(null) const [appsExpanded, setAppsExpanded] = useState(true) const [searchVisible, setSearchVisible] = useState(false) const [searchText, setSearchText] = useState('') - const [uninstallDialogAppId, setUninstallDialogAppId] = useState(null) const normalizedSearchText = searchText.trim() const installedAppsQuery = useInfiniteQuery( @@ -97,9 +86,6 @@ const WebAppsSectionContent = () => { }), ) const installedApps = installedAppsQuery.data ?? emptyInstalledApps - const uninstallAppMutation = useMutation( - consoleQuery.installedApps.byInstalledAppId.delete.mutationOptions(), - ) const updatePinStatusMutation = useMutation( consoleQuery.installedApps.byInstalledAppId.patch.mutationOptions(), ) @@ -176,22 +162,6 @@ const WebAppsSectionContent = () => { input.focus() }, []) - const handleDelete = () => { - if (!uninstallDialogAppId) return - - uninstallAppMutation.mutate( - { - params: { installed_app_id: uninstallDialogAppId }, - }, - { - onSuccess: () => { - setUninstallDialogAppId(null) - toast.success(t(($) => $['api.remove'], { ns: 'common' })) - }, - }, - ) - } - const handleUpdatePinStatus = (id: string, isPinned: boolean) => { updatePinStatusMutation.mutate( { @@ -211,9 +181,7 @@ const WebAppsSectionContent = () => { !installedAppsQuery.isPlaceholderData && installedApps.length === 0 && !normalizedSearchText && - !searchVisible && - uninstallDialogAppId === null && - !uninstallAppMutation.isSuccess + !searchVisible ) return null @@ -223,10 +191,7 @@ const WebAppsSectionContent = () => { onOpenChange={setAppsExpanded} className="grid min-h-0 flex-1 grid-cols-[minmax(0,1fr)_auto] grid-rows-[auto_auto_minmax(0,1fr)]" > - + {t(($) => $['sidebar.webApps'], { ns: 'explore' })} @@ -332,10 +297,6 @@ const WebAppsSectionContent = () => { app={row.app} isSelected={isInstalledAppPath(pathname, row.app.id)} onTogglePin={handleUpdatePinStatus} - onDelete={(id) => { - uninstallAppMutation.reset() - setUninstallDialogAppId(id) - }} /> )} @@ -383,40 +344,6 @@ const WebAppsSectionContent = () => { - { - if (uninstallAppMutation.isPending) { - details.cancel() - return - } - if (!open) setUninstallDialogAppId(null) - }} - > - (uninstallAppMutation.isSuccess ? sectionToggleRef.current : true)} - > -
- - {t(($) => $['sidebar.delete.title'], { ns: 'explore' })} - - - {t(($) => $['sidebar.delete.content'], { ns: 'explore' })} - -
- - - {t(($) => $['operation.cancel'], { ns: 'common' })} - - - {t(($) => $['operation.confirm'], { ns: 'common' })} - - - - ) } diff --git a/web/i18n/locales/ar-TN/explore.json b/web/i18n/locales/ar-TN/explore.json index c858bf73212..ad167fd6791 100644 --- a/web/i18n/locales/ar-TN/explore.json +++ b/web/i18n/locales/ar-TN/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "تثبيت", "sidebar.action.rename": "إعادة تسمية", "sidebar.action.unpin": "إلغاء التثبيت", - "sidebar.delete.content": "هل أنت متأكد أنك تريد حذف هذا التطبيق؟", - "sidebar.delete.title": "حذف التطبيق", "sidebar.webApps": "تطبيقات الويب", "tryApp.category": "الفئة", "tryApp.createFromSampleApp": "إنشاء من هذا التطبيق النموذجي", diff --git a/web/i18n/locales/az-AZ/explore.json b/web/i18n/locales/az-AZ/explore.json index 8ec0bc1c3bd..33ce0a30332 100644 --- a/web/i18n/locales/az-AZ/explore.json +++ b/web/i18n/locales/az-AZ/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Sancaqla", "sidebar.action.rename": "Adını dəyiş", "sidebar.action.unpin": "Sancaqdan çıxar", - "sidebar.delete.content": "Bu tətbiqi silmək istədiyinizə əminsiniz?", - "sidebar.delete.title": "Tətbiqin silinməsi", "sidebar.webApps": "Veb tətbiqləri", "tryApp.category": "Kateqoriya", "tryApp.createFromSampleApp": "Bu nümunə tətbiqdən yarat", diff --git a/web/i18n/locales/de-DE/explore.json b/web/i18n/locales/de-DE/explore.json index b8b40532f4c..a9a290e2bef 100644 --- a/web/i18n/locales/de-DE/explore.json +++ b/web/i18n/locales/de-DE/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Anheften", "sidebar.action.rename": "Umbenennen", "sidebar.action.unpin": "Lösen", - "sidebar.delete.content": "Sind Sie sicher, dass Sie diese App löschen möchten?", - "sidebar.delete.title": "App löschen", "sidebar.webApps": "Web-Apps", "tryApp.category": "Kategorie", "tryApp.createFromSampleApp": "Aus dieser Beispiel-App erstellen", diff --git a/web/i18n/locales/en-US/explore.json b/web/i18n/locales/en-US/explore.json index da28d9dfc97..6c183f56c1b 100644 --- a/web/i18n/locales/en-US/explore.json +++ b/web/i18n/locales/en-US/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Pin", "sidebar.action.rename": "Rename", "sidebar.action.unpin": "Unpin", - "sidebar.delete.content": "Are you sure you want to delete this app?", - "sidebar.delete.title": "Delete app", "sidebar.webApps": "Web apps", "tryApp.category": "Category", "tryApp.createFromSampleApp": "Create from this sample app", diff --git a/web/i18n/locales/es-ES/explore.json b/web/i18n/locales/es-ES/explore.json index c9308865770..3936fa96ff0 100644 --- a/web/i18n/locales/es-ES/explore.json +++ b/web/i18n/locales/es-ES/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Anclar", "sidebar.action.rename": "Renombrar", "sidebar.action.unpin": "Desanclar", - "sidebar.delete.content": "¿Estás seguro de que quieres eliminar esta aplicación?", - "sidebar.delete.title": "Eliminar aplicación", "sidebar.webApps": "Aplicaciones web", "tryApp.category": "Categoría", "tryApp.createFromSampleApp": "Crear desde esta aplicación de ejemplo", diff --git a/web/i18n/locales/fa-IR/explore.json b/web/i18n/locales/fa-IR/explore.json index a3665afe775..28f045edeae 100644 --- a/web/i18n/locales/fa-IR/explore.json +++ b/web/i18n/locales/fa-IR/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "سنجاق کردن", "sidebar.action.rename": "تغییر نام", "sidebar.action.unpin": "برداشتن سنجاق", - "sidebar.delete.content": "آیا مطمئن هستید که می‌خواهید این برنامه را حذف کنید؟", - "sidebar.delete.title": "حذف برنامه", "sidebar.webApps": "برنامه‌های وب", "tryApp.category": "دسته‌بندی", "tryApp.createFromSampleApp": "از این برنامه نمونه ایجاد کنید", diff --git a/web/i18n/locales/fr-FR/explore.json b/web/i18n/locales/fr-FR/explore.json index d903eec41aa..5e4d843cf8a 100644 --- a/web/i18n/locales/fr-FR/explore.json +++ b/web/i18n/locales/fr-FR/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Épingle", "sidebar.action.rename": "Renommer", "sidebar.action.unpin": "Détacher", - "sidebar.delete.content": "Êtes-vous sûr de vouloir supprimer cette application ?", - "sidebar.delete.title": "Supprimer l'application", "sidebar.webApps": "Applications web", "tryApp.category": "Catégorie", "tryApp.createFromSampleApp": "Créer à partir de cette application exemple", diff --git a/web/i18n/locales/hi-IN/explore.json b/web/i18n/locales/hi-IN/explore.json index abfe96d361c..033471e3989 100644 --- a/web/i18n/locales/hi-IN/explore.json +++ b/web/i18n/locales/hi-IN/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "पिन करें", "sidebar.action.rename": "नाम बदलें", "sidebar.action.unpin": "पिन हटाएँ", - "sidebar.delete.content": "क्या आप वाकई इस ऐप को हटाना चाहते हैं?", - "sidebar.delete.title": "ऐप हटाएं", "sidebar.webApps": "वेब ऐप्स", "tryApp.category": "श्रेणी", "tryApp.createFromSampleApp": "इस नमूना ऐप से बनाएं", diff --git a/web/i18n/locales/id-ID/explore.json b/web/i18n/locales/id-ID/explore.json index 0135e7d93f4..1c782d6d275 100644 --- a/web/i18n/locales/id-ID/explore.json +++ b/web/i18n/locales/id-ID/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Sematkan", "sidebar.action.rename": "Ganti nama", "sidebar.action.unpin": "Lepaskan sematan", - "sidebar.delete.content": "Apakah Anda yakin ingin menghapus aplikasi ini?", - "sidebar.delete.title": "Hapus aplikasi", "sidebar.webApps": "Aplikasi web", "tryApp.category": "Kategori", "tryApp.createFromSampleApp": "Buat dari aplikasi contoh ini", diff --git a/web/i18n/locales/it-IT/explore.json b/web/i18n/locales/it-IT/explore.json index 53cb5194e98..d7cd2dfd74f 100644 --- a/web/i18n/locales/it-IT/explore.json +++ b/web/i18n/locales/it-IT/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Fissa", "sidebar.action.rename": "Rinomina", "sidebar.action.unpin": "Sblocca", - "sidebar.delete.content": "Sei sicuro di voler eliminare questa app?", - "sidebar.delete.title": "Elimina app", "sidebar.webApps": "App web", "tryApp.category": "Categoria", "tryApp.createFromSampleApp": "Crea da questa app di esempio", diff --git a/web/i18n/locales/ja-JP/explore.json b/web/i18n/locales/ja-JP/explore.json index ab2d1214d7d..8406691242e 100644 --- a/web/i18n/locales/ja-JP/explore.json +++ b/web/i18n/locales/ja-JP/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "ピン留め", "sidebar.action.rename": "名前変更", "sidebar.action.unpin": "ピン留め解除", - "sidebar.delete.content": "このアプリを削除してもよろしいですか？", - "sidebar.delete.title": "アプリを削除", "sidebar.webApps": "Webアプリ", "tryApp.category": "カテゴリー", "tryApp.createFromSampleApp": "テンプレートから作成", diff --git a/web/i18n/locales/ko-KR/explore.json b/web/i18n/locales/ko-KR/explore.json index 9cd4076cacf..8cf1c6ab3e3 100644 --- a/web/i18n/locales/ko-KR/explore.json +++ b/web/i18n/locales/ko-KR/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "고정", "sidebar.action.rename": "이름 변경", "sidebar.action.unpin": "고정 해제", - "sidebar.delete.content": "이 앱을 삭제해도 괜찮습니까?", - "sidebar.delete.title": "앱 삭제", "sidebar.webApps": "웹 앱", "tryApp.category": "카테고리", "tryApp.createFromSampleApp": "이 샘플 앱에서 만들기", diff --git a/web/i18n/locales/lo-LA/explore.json b/web/i18n/locales/lo-LA/explore.json index 12190b06027..c43244b50c2 100644 --- a/web/i18n/locales/lo-LA/explore.json +++ b/web/i18n/locales/lo-LA/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "ປັກໝຸດ", "sidebar.action.rename": "ປ່ຽນຊື່", "sidebar.action.unpin": "ຖອນປັກໝຸດ", - "sidebar.delete.content": "ເຈົ້າແນ່ໃຈບໍວ່າຕ້ອງການລຶບແອັບນີ້?", - "sidebar.delete.title": "ລຶບແອັບ", "sidebar.webApps": "ແອັບເວັບ", "tryApp.category": "ໝວດໝູ່", "tryApp.createFromSampleApp": "ສ້າງຈາກແອັບຕົວຢ່າງນີ້", diff --git a/web/i18n/locales/nl-NL/explore.json b/web/i18n/locales/nl-NL/explore.json index db3f074778f..a7edcf1d679 100644 --- a/web/i18n/locales/nl-NL/explore.json +++ b/web/i18n/locales/nl-NL/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Pin", "sidebar.action.rename": "Rename", "sidebar.action.unpin": "Unpin", - "sidebar.delete.content": "Are you sure you want to delete this app?", - "sidebar.delete.title": "Delete app", "sidebar.webApps": "Web apps", "tryApp.category": "Category", "tryApp.createFromSampleApp": "Create from this sample app", diff --git a/web/i18n/locales/pl-PL/explore.json b/web/i18n/locales/pl-PL/explore.json index 8079637c5cc..efe60284760 100644 --- a/web/i18n/locales/pl-PL/explore.json +++ b/web/i18n/locales/pl-PL/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Przypnij", "sidebar.action.rename": "Zmień nazwę", "sidebar.action.unpin": "Odepnij", - "sidebar.delete.content": "Czy na pewno chcesz usunąć tę aplikację?", - "sidebar.delete.title": "Usuń aplikację", "sidebar.webApps": "Aplikacje internetowe", "tryApp.category": "Kategoria", "tryApp.createFromSampleApp": "Utwórz z tej przykładowej aplikacji", diff --git a/web/i18n/locales/pt-BR/explore.json b/web/i18n/locales/pt-BR/explore.json index b71094f6b78..741bbfcdcbe 100644 --- a/web/i18n/locales/pt-BR/explore.json +++ b/web/i18n/locales/pt-BR/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Fixar", "sidebar.action.rename": "Renomear", "sidebar.action.unpin": "Desafixar", - "sidebar.delete.content": "Tem certeza de que deseja excluir este aplicativo?", - "sidebar.delete.title": "Excluir aplicativo", "sidebar.webApps": "Aplicativos web", "tryApp.category": "Categoria", "tryApp.createFromSampleApp": "Criar a partir deste aplicativo de exemplo", diff --git a/web/i18n/locales/ro-RO/explore.json b/web/i18n/locales/ro-RO/explore.json index 1202d2b05b2..3dcba233f2d 100644 --- a/web/i18n/locales/ro-RO/explore.json +++ b/web/i18n/locales/ro-RO/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Fixați", "sidebar.action.rename": "Redenumire", "sidebar.action.unpin": "Deblocați", - "sidebar.delete.content": "Sunteți sigur că doriți să ștergeți această aplicație?", - "sidebar.delete.title": "Ștergeți aplicația", "sidebar.webApps": "Aplicații web", "tryApp.category": "Categorie", "tryApp.createFromSampleApp": "Creați din această aplicație exemplu", diff --git a/web/i18n/locales/ru-RU/explore.json b/web/i18n/locales/ru-RU/explore.json index f166f8116a7..1ffe3806de8 100644 --- a/web/i18n/locales/ru-RU/explore.json +++ b/web/i18n/locales/ru-RU/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Закрепить", "sidebar.action.rename": "Переименовать", "sidebar.action.unpin": "Открепить", - "sidebar.delete.content": "Вы уверены, что хотите удалить это приложение?", - "sidebar.delete.title": "Удалить приложение", "sidebar.webApps": "Веб-приложения", "tryApp.category": "Категория", "tryApp.createFromSampleApp": "Создать из этого примера приложения", diff --git a/web/i18n/locales/sl-SI/explore.json b/web/i18n/locales/sl-SI/explore.json index 23bbfc42d05..6d659a513a8 100644 --- a/web/i18n/locales/sl-SI/explore.json +++ b/web/i18n/locales/sl-SI/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Pripni", "sidebar.action.rename": "Preimenuj", "sidebar.action.unpin": "Odpni", - "sidebar.delete.content": "Ali ste prepričani, da želite izbrisati to aplikacijo?", - "sidebar.delete.title": "Izbriši aplikacijo", "sidebar.webApps": "Spletne aplikacije", "tryApp.category": "Kategorija", "tryApp.createFromSampleApp": "Ustvari iz te vzorčne aplikacije", diff --git a/web/i18n/locales/th-TH/explore.json b/web/i18n/locales/th-TH/explore.json index 7fe2daf80a4..7ceab65d60d 100644 --- a/web/i18n/locales/th-TH/explore.json +++ b/web/i18n/locales/th-TH/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "เข็มกลัด", "sidebar.action.rename": "ตั้งชื่อใหม่", "sidebar.action.unpin": "ปลดหมุด", - "sidebar.delete.content": "คุณแน่ใจหรือไม่ว่าต้องการลบแอปนี้?", - "sidebar.delete.title": "ลบแอป", "sidebar.webApps": "แอปเว็บ", "tryApp.category": "หมวดหมู่", "tryApp.createFromSampleApp": "สร้างจากแอปตัวอย่างนี้", diff --git a/web/i18n/locales/tr-TR/explore.json b/web/i18n/locales/tr-TR/explore.json index 1a4901be1e3..b4fc6a86120 100644 --- a/web/i18n/locales/tr-TR/explore.json +++ b/web/i18n/locales/tr-TR/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Sabitle", "sidebar.action.rename": "Yeniden Adlandır", "sidebar.action.unpin": "Sabitlemeyi Kaldır", - "sidebar.delete.content": "Bu uygulamayı silmek istediğinizden emin misiniz?", - "sidebar.delete.title": "Uygulamayı sil", "sidebar.webApps": "Web uygulamaları", "tryApp.category": "Kategori", "tryApp.createFromSampleApp": "Bu örnek uygulamadan oluştur", diff --git a/web/i18n/locales/uk-UA/explore.json b/web/i18n/locales/uk-UA/explore.json index eb859861b61..227fe71422c 100644 --- a/web/i18n/locales/uk-UA/explore.json +++ b/web/i18n/locales/uk-UA/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Закріпити", "sidebar.action.rename": "Перейменувати", "sidebar.action.unpin": "Відкріпити", - "sidebar.delete.content": "Ви впевнені, що хочете видалити цю програму?", - "sidebar.delete.title": "Видалити додаток", "sidebar.webApps": "Веб-додатки", "tryApp.category": "Категорія", "tryApp.createFromSampleApp": "Створити з цього зразка додатку", diff --git a/web/i18n/locales/vi-VN/explore.json b/web/i18n/locales/vi-VN/explore.json index 7b06b51d70a..8e55fdb5f4b 100644 --- a/web/i18n/locales/vi-VN/explore.json +++ b/web/i18n/locales/vi-VN/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "Ghim", "sidebar.action.rename": "Đổi tên", "sidebar.action.unpin": "Bỏ ghim", - "sidebar.delete.content": "Bạn có chắc chắn muốn xóa ứng dụng này không?", - "sidebar.delete.title": "Xóa ứng dụng", "sidebar.webApps": "Ứng dụng web", "tryApp.category": "Danh mục", "tryApp.createFromSampleApp": "Tạo từ ứng dụng mẫu này", diff --git a/web/i18n/locales/zh-Hans/explore.json b/web/i18n/locales/zh-Hans/explore.json index b30fdcb0b43..67d8191cc91 100644 --- a/web/i18n/locales/zh-Hans/explore.json +++ b/web/i18n/locales/zh-Hans/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "置顶", "sidebar.action.rename": "重命名", "sidebar.action.unpin": "取消置顶", - "sidebar.delete.content": "您确定要删除此程序吗？", - "sidebar.delete.title": "删除程序", "sidebar.webApps": "WEB APPS", "tryApp.category": "分类", "tryApp.createFromSampleApp": "从此模板创建应用", diff --git a/web/i18n/locales/zh-Hant/explore.json b/web/i18n/locales/zh-Hant/explore.json index 398f1b01e50..f57c5636561 100644 --- a/web/i18n/locales/zh-Hant/explore.json +++ b/web/i18n/locales/zh-Hant/explore.json @@ -33,8 +33,6 @@ "sidebar.action.pin": "置頂", "sidebar.action.rename": "重新命名", "sidebar.action.unpin": "取消置頂", - "sidebar.delete.content": "您確定要刪除此程式嗎？", - "sidebar.delete.title": "刪除程式", "sidebar.webApps": "WEB APPS", "tryApp.category": "分類", "tryApp.createFromSampleApp": "從此範本建立應用", diff --git a/web/service/console/query-policies.ts b/web/service/console/query-policies.ts index 0d2b1e1de25..cc326e75679 100644 --- a/web/service/console/query-policies.ts +++ b/web/service/console/query-policies.ts @@ -717,22 +717,6 @@ export function createConsoleQuery(consoleClient: ConsoleClient) { }, }, }, - delete: { - mutationOptions: { - onSuccess: (_response, variables, _onMutateResult, context) => { - context.client.removeQueries({ - queryKey: consoleQuery.installedApps.byInstalledAppId.get.queryKey({ - input: { - params: variables.params, - }, - }), - }) - context.client.invalidateQueries({ - queryKey: consoleQuery.installedApps.get.key(), - }) - }, - }, - }, patch: { mutationOptions: { onSuccess: (_response, variables, _onMutateResult, context) => {