diff --git a/api/controllers/console/explore/installed_app.py b/api/controllers/console/explore/installed_app.py index 86b36e3c92..0ab7a96f11 100644 --- a/api/controllers/console/explore/installed_app.py +++ b/api/controllers/console/explore/installed_app.py @@ -5,7 +5,7 @@ from typing import Any from flask import request from flask_restx import Resource from pydantic import BaseModel, Field, computed_field, field_validator -from sqlalchemy import and_, select +from sqlalchemy import and_, exists, or_, select from werkzeug.exceptions import BadRequest, Forbidden, NotFound from controllers.common.fields import SimpleMessageResponse, SimpleResultMessageResponse @@ -24,8 +24,8 @@ from graphon.file import helpers as file_helpers from libs.datetime_utils import naive_utc_now from libs.helper import to_timestamp from libs.login import login_required -from models import Account, App, InstalledApp, RecommendedApp -from models.model import IconType +from models import Account, App, AppModelConfig, InstalledApp, RecommendedApp, Workflow +from models.model import AppMode, IconType from services.account_service import TenantService from services.enterprise.enterprise_service import EnterpriseService from services.feature_service import FeatureService @@ -61,6 +61,24 @@ def _safe_primitive(value: Any) -> Any: return None +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 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), + ) + + class InstalledAppInfoResponse(ResponseModel): id: str name: str | None = None @@ -141,33 +159,32 @@ class InstalledAppsListApi(Resource): def get(self, current_tenant_id: str, current_user: Account): query = InstalledAppsListQuery.model_validate(request.args.to_dict()) + stmt = ( + select(InstalledApp, App) + .join(App, App.id == InstalledApp.app_id) + .where(InstalledApp.tenant_id == current_tenant_id, _published_app_filter()) + ) if query.app_id: - installed_apps = db.session.scalars( - select(InstalledApp).where( - and_(InstalledApp.tenant_id == current_tenant_id, InstalledApp.app_id == query.app_id) - ) - ).all() - else: - installed_apps = db.session.scalars( - select(InstalledApp).where(InstalledApp.tenant_id == current_tenant_id) - ).all() + stmt = stmt.where(InstalledApp.app_id == query.app_id) + + installed_apps = db.session.execute(stmt).all() 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) - installed_app_list: list[dict[str, Any]] = [ - { - "id": installed_app.id, - "app": installed_app.app, - "app_owner_tenant_id": installed_app.app_owner_tenant_id, - "is_pinned": installed_app.is_pinned, - "last_used_at": installed_app.last_used_at, - "editable": current_user.role in {"owner", "admin"}, - "uninstallable": current_tenant_id == installed_app.app_owner_tenant_id, - } - for installed_app in installed_apps - if installed_app.app is not None - ] + installed_app_list: list[dict[str, Any]] = [] + for installed_app, app_model in installed_apps: + installed_app_list.append( + { + "id": installed_app.id, + "app": app_model, + "app_owner_tenant_id": installed_app.app_owner_tenant_id, + "is_pinned": installed_app.is_pinned, + "last_used_at": installed_app.last_used_at, + "editable": current_user.role in {"owner", "admin"}, + "uninstallable": current_tenant_id == installed_app.app_owner_tenant_id, + } + ) # filter out apps that user doesn't have access to if FeatureService.get_system_features().webapp_auth.enabled: 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 d7ec808efa..be6275b5cb 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 @@ -57,6 +57,14 @@ def payload_patch() -> PayloadPatch: class TestInstalledAppsListApi: + def test_published_app_filter_checks_publish_targets(self) -> None: + compiled_filter = str(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 + def test_get_installed_apps( self, app: Flask, current_user: MagicMock, tenant_id: str, installed_app: MagicMock ) -> None: @@ -64,7 +72,7 @@ class TestInstalledAppsListApi: method = unwrap(api.get) session = MagicMock() - session.scalars.return_value.all.return_value = [installed_app] + session.execute.return_value.all.return_value = [(installed_app, installed_app.app)] with ( app.test_request_context("/"), @@ -87,7 +95,7 @@ class TestInstalledAppsListApi: method = unwrap(api.get) session = MagicMock() - session.scalars.return_value.all.return_value = [] + session.execute.return_value.all.return_value = [] with ( app.test_request_context("/?app_id=a1"), @@ -111,7 +119,7 @@ class TestInstalledAppsListApi: method = unwrap(api.get) session = MagicMock() - session.scalars.return_value.all.return_value = [installed_app] + session.execute.return_value.all.return_value = [(installed_app, installed_app.app)] mock_webapp_setting = MagicMock() mock_webapp_setting.access_mode = "restricted" @@ -148,7 +156,7 @@ class TestInstalledAppsListApi: method = unwrap(api.get) session = MagicMock() - session.scalars.return_value.all.return_value = [installed_app] + session.execute.return_value.all.return_value = [(installed_app, installed_app.app)] mock_webapp_setting = MagicMock() mock_webapp_setting.access_mode = "restricted" @@ -185,7 +193,7 @@ class TestInstalledAppsListApi: method = unwrap(api.get) session = MagicMock() - session.scalars.return_value.all.return_value = [installed_app] + session.execute.return_value.all.return_value = [(installed_app, installed_app.app)] mock_webapp_setting = MagicMock() mock_webapp_setting.access_mode = "sso_verified" @@ -214,11 +222,54 @@ class TestInstalledAppsListApi: api = module.InstalledAppsListApi() method = unwrap(api.get) - installed_app_with_null = MagicMock() - installed_app_with_null.app = None + session = MagicMock() + session.execute.return_value.all.return_value = [] + + with ( + app.test_request_context("/"), + patch.object(module.db, "session", session), + patch.object(module.TenantService, "get_user_role", return_value="owner"), + patch.object( + module.FeatureService, + "get_system_features", + return_value=MagicMock(webapp_auth=MagicMock(enabled=False)), + ), + ): + result = method(api, tenant_id, current_user) + + assert result["installed_apps"] == [] + + def test_get_installed_apps_filters_unpublished_chat_apps( + self, app: Flask, current_user: MagicMock, tenant_id: str + ) -> None: + api = module.InstalledAppsListApi() + method = unwrap(api.get) session = MagicMock() - session.scalars.return_value.all.return_value = [installed_app_with_null] + session.execute.return_value.all.return_value = [] + + with ( + app.test_request_context("/"), + patch.object(module.db, "session", session), + patch.object(module.TenantService, "get_user_role", return_value="owner"), + patch.object( + module.FeatureService, + "get_system_features", + return_value=MagicMock(webapp_auth=MagicMock(enabled=False)), + ), + ): + result = method(api, tenant_id, current_user) + + assert result["installed_apps"] == [] + + def test_get_installed_apps_filters_unpublished_workflow_apps( + self, app: Flask, current_user: MagicMock, tenant_id: str + ) -> None: + api = module.InstalledAppsListApi() + method = unwrap(api.get) + + session = MagicMock() + session.execute.return_value.all.return_value = [] with ( app.test_request_context("/"), @@ -243,7 +294,7 @@ class TestInstalledAppsListApi: current_user.current_tenant = None session = MagicMock() - session.scalars.return_value.all.return_value = [installed_app] + session.execute.return_value.all.return_value = [(installed_app, installed_app.app)] with ( app.test_request_context("/"),