mirror of https://github.com/langgenius/dify.git
fix: fix omitted app icon_type updates (#33988)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
7fe25f1365
commit
1674f8c2fb
|
|
@ -95,7 +95,7 @@ class CreateAppPayload(BaseModel):
|
|||
name: str = Field(..., min_length=1, description="App name")
|
||||
description: str | None = Field(default=None, description="App description (max 400 chars)", max_length=400)
|
||||
mode: Literal["chat", "agent-chat", "advanced-chat", "workflow", "completion"] = Field(..., description="App mode")
|
||||
icon_type: str | None = Field(default=None, description="Icon type")
|
||||
icon_type: IconType | None = Field(default=None, description="Icon type")
|
||||
icon: str | None = Field(default=None, description="Icon")
|
||||
icon_background: str | None = Field(default=None, description="Icon background color")
|
||||
|
||||
|
|
@ -103,7 +103,7 @@ class CreateAppPayload(BaseModel):
|
|||
class UpdateAppPayload(BaseModel):
|
||||
name: str = Field(..., min_length=1, description="App name")
|
||||
description: str | None = Field(default=None, description="App description (max 400 chars)", max_length=400)
|
||||
icon_type: str | None = Field(default=None, description="Icon type")
|
||||
icon_type: IconType | None = Field(default=None, description="Icon type")
|
||||
icon: str | None = Field(default=None, description="Icon")
|
||||
icon_background: str | None = Field(default=None, description="Icon background color")
|
||||
use_icon_as_answer_icon: bool | None = Field(default=None, description="Use icon as answer icon")
|
||||
|
|
@ -113,7 +113,7 @@ class UpdateAppPayload(BaseModel):
|
|||
class CopyAppPayload(BaseModel):
|
||||
name: str | None = Field(default=None, description="Name for the copied app")
|
||||
description: str | None = Field(default=None, description="Description for the copied app", max_length=400)
|
||||
icon_type: str | None = Field(default=None, description="Icon type")
|
||||
icon_type: IconType | None = Field(default=None, description="Icon type")
|
||||
icon: str | None = Field(default=None, description="Icon")
|
||||
icon_background: str | None = Field(default=None, description="Icon background color")
|
||||
|
||||
|
|
@ -594,7 +594,7 @@ class AppApi(Resource):
|
|||
args_dict: AppService.ArgsDict = {
|
||||
"name": args.name,
|
||||
"description": args.description or "",
|
||||
"icon_type": args.icon_type or "",
|
||||
"icon_type": args.icon_type,
|
||||
"icon": args.icon or "",
|
||||
"icon_background": args.icon_background or "",
|
||||
"use_icon_as_answer_icon": args.use_icon_as_answer_icon or False,
|
||||
|
|
|
|||
|
|
@ -241,7 +241,7 @@ class AppService:
|
|||
class ArgsDict(TypedDict):
|
||||
name: str
|
||||
description: str
|
||||
icon_type: str
|
||||
icon_type: IconType | str | None
|
||||
icon: str
|
||||
icon_background: str
|
||||
use_icon_as_answer_icon: bool
|
||||
|
|
@ -257,7 +257,13 @@ class AppService:
|
|||
assert current_user is not None
|
||||
app.name = args["name"]
|
||||
app.description = args["description"]
|
||||
app.icon_type = IconType(args["icon_type"]) if args["icon_type"] else None
|
||||
icon_type = args.get("icon_type")
|
||||
if icon_type is None:
|
||||
resolved_icon_type = app.icon_type
|
||||
else:
|
||||
resolved_icon_type = IconType(icon_type)
|
||||
|
||||
app.icon_type = resolved_icon_type
|
||||
app.icon = args["icon"]
|
||||
app.icon_background = args["icon_background"]
|
||||
app.use_icon_as_answer_icon = args.get("use_icon_as_answer_icon", False)
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ from sqlalchemy.orm import Session
|
|||
|
||||
from constants.model_template import default_app_templates
|
||||
from models import Account
|
||||
from models.model import App, Site
|
||||
from models.model import App, IconType, Site
|
||||
from services.account_service import AccountService, TenantService
|
||||
from tests.test_containers_integration_tests.helpers import generate_valid_password
|
||||
|
||||
|
|
@ -463,6 +463,109 @@ class TestAppService:
|
|||
assert updated_app.tenant_id == app.tenant_id
|
||||
assert updated_app.created_by == app.created_by
|
||||
|
||||
def test_update_app_should_preserve_icon_type_when_omitted(
|
||||
self, db_session_with_containers: Session, mock_external_service_dependencies
|
||||
):
|
||||
"""
|
||||
Test update_app keeps the persisted icon_type when the update payload omits it.
|
||||
"""
|
||||
fake = Faker()
|
||||
|
||||
account = AccountService.create_account(
|
||||
email=fake.email(),
|
||||
name=fake.name(),
|
||||
interface_language="en-US",
|
||||
password=generate_valid_password(fake),
|
||||
)
|
||||
TenantService.create_owner_tenant_if_not_exist(account, name=fake.company())
|
||||
tenant = account.current_tenant
|
||||
|
||||
from services.app_service import AppService
|
||||
|
||||
app_service = AppService()
|
||||
app = app_service.create_app(
|
||||
tenant.id,
|
||||
{
|
||||
"name": fake.company(),
|
||||
"description": fake.text(max_nb_chars=100),
|
||||
"mode": "chat",
|
||||
"icon_type": "emoji",
|
||||
"icon": "🎯",
|
||||
"icon_background": "#45B7D1",
|
||||
},
|
||||
account,
|
||||
)
|
||||
|
||||
mock_current_user = create_autospec(Account, instance=True)
|
||||
mock_current_user.id = account.id
|
||||
mock_current_user.current_tenant_id = account.current_tenant_id
|
||||
|
||||
with patch("services.app_service.current_user", mock_current_user):
|
||||
updated_app = app_service.update_app(
|
||||
app,
|
||||
{
|
||||
"name": "Updated App Name",
|
||||
"description": "Updated app description",
|
||||
"icon_type": None,
|
||||
"icon": "🔄",
|
||||
"icon_background": "#FF8C42",
|
||||
"use_icon_as_answer_icon": True,
|
||||
},
|
||||
)
|
||||
|
||||
assert updated_app.icon_type == IconType.EMOJI
|
||||
|
||||
def test_update_app_should_reject_empty_icon_type(
|
||||
self, db_session_with_containers: Session, mock_external_service_dependencies
|
||||
):
|
||||
"""
|
||||
Test update_app rejects an explicit empty icon_type.
|
||||
"""
|
||||
fake = Faker()
|
||||
|
||||
account = AccountService.create_account(
|
||||
email=fake.email(),
|
||||
name=fake.name(),
|
||||
interface_language="en-US",
|
||||
password=generate_valid_password(fake),
|
||||
)
|
||||
TenantService.create_owner_tenant_if_not_exist(account, name=fake.company())
|
||||
tenant = account.current_tenant
|
||||
|
||||
from services.app_service import AppService
|
||||
|
||||
app_service = AppService()
|
||||
app = app_service.create_app(
|
||||
tenant.id,
|
||||
{
|
||||
"name": fake.company(),
|
||||
"description": fake.text(max_nb_chars=100),
|
||||
"mode": "chat",
|
||||
"icon_type": "emoji",
|
||||
"icon": "🎯",
|
||||
"icon_background": "#45B7D1",
|
||||
},
|
||||
account,
|
||||
)
|
||||
|
||||
mock_current_user = create_autospec(Account, instance=True)
|
||||
mock_current_user.id = account.id
|
||||
mock_current_user.current_tenant_id = account.current_tenant_id
|
||||
|
||||
with patch("services.app_service.current_user", mock_current_user):
|
||||
with pytest.raises(ValueError):
|
||||
app_service.update_app(
|
||||
app,
|
||||
{
|
||||
"name": "Updated App Name",
|
||||
"description": "Updated app description",
|
||||
"icon_type": "",
|
||||
"icon": "🔄",
|
||||
"icon_background": "#FF8C42",
|
||||
"use_icon_as_answer_icon": True,
|
||||
},
|
||||
)
|
||||
|
||||
def test_update_app_name_success(self, db_session_with_containers: Session, mock_external_service_dependencies):
|
||||
"""
|
||||
Test successful app name update.
|
||||
|
|
|
|||
|
|
@ -7,14 +7,19 @@ from __future__ import annotations
|
|||
|
||||
import uuid
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
from werkzeug.exceptions import BadRequest, NotFound
|
||||
|
||||
from controllers.console import console_ns
|
||||
from controllers.console.app import (
|
||||
annotation as annotation_module,
|
||||
)
|
||||
from controllers.console.app import (
|
||||
app as app_module,
|
||||
)
|
||||
from controllers.console.app import (
|
||||
completion as completion_module,
|
||||
)
|
||||
|
|
@ -203,6 +208,48 @@ class TestCompletionEndpoints:
|
|||
method(app_model=MagicMock(id="app-1"))
|
||||
|
||||
|
||||
class TestAppEndpoints:
|
||||
"""Tests for app endpoints."""
|
||||
|
||||
def test_app_put_should_preserve_icon_type_when_payload_omits_it(self, app, monkeypatch):
|
||||
api = app_module.AppApi()
|
||||
method = _unwrap(api.put)
|
||||
payload = {
|
||||
"name": "Updated App",
|
||||
"description": "Updated description",
|
||||
"icon": "🤖",
|
||||
"icon_background": "#FFFFFF",
|
||||
}
|
||||
app_service = MagicMock()
|
||||
app_service.update_app.return_value = SimpleNamespace()
|
||||
response_model = MagicMock()
|
||||
response_model.model_dump.return_value = {"id": "app-1"}
|
||||
|
||||
monkeypatch.setattr(app_module, "AppService", lambda: app_service)
|
||||
monkeypatch.setattr(app_module.AppDetailWithSite, "model_validate", MagicMock(return_value=response_model))
|
||||
|
||||
with (
|
||||
app.test_request_context("/console/api/apps/app-1", method="PUT", json=payload),
|
||||
patch.object(type(console_ns), "payload", payload),
|
||||
):
|
||||
response = method(app_model=SimpleNamespace(icon_type=app_module.IconType.EMOJI))
|
||||
|
||||
assert response == {"id": "app-1"}
|
||||
assert app_service.update_app.call_args.args[1]["icon_type"] is None
|
||||
|
||||
def test_update_app_payload_should_reject_empty_icon_type(self):
|
||||
with pytest.raises(ValidationError):
|
||||
app_module.UpdateAppPayload.model_validate(
|
||||
{
|
||||
"name": "Updated App",
|
||||
"description": "Updated description",
|
||||
"icon_type": "",
|
||||
"icon": "🤖",
|
||||
"icon_background": "#FFFFFF",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
# ========== OpsTrace Tests ==========
|
||||
class TestOpsTraceEndpoints:
|
||||
"""Tests for ops_trace endpoint."""
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ import pytest
|
|||
|
||||
from core.errors.error import ProviderTokenNotInitError
|
||||
from models import Account, Tenant
|
||||
from models.model import App, AppMode
|
||||
from models.model import App, AppMode, IconType
|
||||
from services.app_service import AppService
|
||||
|
||||
|
||||
|
|
@ -411,6 +411,7 @@ class TestAppServiceGetAndUpdate:
|
|||
|
||||
# Assert
|
||||
assert updated is app
|
||||
assert updated.icon_type == IconType.IMAGE
|
||||
assert renamed is app
|
||||
assert iconed is app
|
||||
assert site_same is app
|
||||
|
|
@ -419,6 +420,79 @@ class TestAppServiceGetAndUpdate:
|
|||
assert api_changed is app
|
||||
assert mock_db.session.commit.call_count >= 5
|
||||
|
||||
def test_update_app_should_preserve_icon_type_when_not_provided(self, service: AppService) -> None:
|
||||
"""Test update_app keeps the existing icon_type when the payload omits it."""
|
||||
# Arrange
|
||||
app = cast(
|
||||
App,
|
||||
SimpleNamespace(
|
||||
name="old",
|
||||
description="old",
|
||||
icon_type=IconType.EMOJI,
|
||||
icon="a",
|
||||
icon_background="#111",
|
||||
use_icon_as_answer_icon=False,
|
||||
max_active_requests=1,
|
||||
),
|
||||
)
|
||||
args = {
|
||||
"name": "new",
|
||||
"description": "new-desc",
|
||||
"icon_type": None,
|
||||
"icon": "new-icon",
|
||||
"icon_background": "#222",
|
||||
"use_icon_as_answer_icon": True,
|
||||
"max_active_requests": 5,
|
||||
}
|
||||
user = SimpleNamespace(id="user-1")
|
||||
|
||||
with (
|
||||
patch("services.app_service.current_user", user),
|
||||
patch("services.app_service.db") as mock_db,
|
||||
patch("services.app_service.naive_utc_now", return_value="now"),
|
||||
):
|
||||
# Act
|
||||
updated = service.update_app(app, args)
|
||||
|
||||
# Assert
|
||||
assert updated is app
|
||||
assert updated.icon_type == IconType.EMOJI
|
||||
mock_db.session.commit.assert_called_once()
|
||||
|
||||
def test_update_app_should_reject_empty_icon_type(self, service: AppService) -> None:
|
||||
"""Test update_app rejects an explicit empty icon_type."""
|
||||
app = cast(
|
||||
App,
|
||||
SimpleNamespace(
|
||||
name="old",
|
||||
description="old",
|
||||
icon_type=IconType.EMOJI,
|
||||
icon="a",
|
||||
icon_background="#111",
|
||||
use_icon_as_answer_icon=False,
|
||||
max_active_requests=1,
|
||||
),
|
||||
)
|
||||
args = {
|
||||
"name": "new",
|
||||
"description": "new-desc",
|
||||
"icon_type": "",
|
||||
"icon": "new-icon",
|
||||
"icon_background": "#222",
|
||||
"use_icon_as_answer_icon": True,
|
||||
"max_active_requests": 5,
|
||||
}
|
||||
user = SimpleNamespace(id="user-1")
|
||||
|
||||
with (
|
||||
patch("services.app_service.current_user", user),
|
||||
patch("services.app_service.db") as mock_db,
|
||||
):
|
||||
with pytest.raises(ValueError):
|
||||
service.update_app(app, args)
|
||||
|
||||
mock_db.session.commit.assert_not_called()
|
||||
|
||||
|
||||
class TestAppServiceDeleteAndMeta:
|
||||
"""Test suite for delete and metadata methods."""
|
||||
|
|
|
|||
Loading…
Reference in New Issue