From 2f63be0c05ade890a8063fb5fa38391fd13f7e12 Mon Sep 17 00:00:00 2001 From: Keith <148296621+keith991001@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:17:07 +0900 Subject: [PATCH] refactor(models): pass session into WorkflowAppLog accessors --- api/models/workflow.py | 10 +-- api/services/workflow_app_service.py | 16 +++- .../models/test_workflow_app_log_models.py | 80 +++++++++++++++++++ .../test_workflow_app_service_metadata.py | 27 ++++++- 4 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 api/tests/unit_tests/models/test_workflow_app_log_models.py diff --git a/api/models/workflow.py b/api/models/workflow.py index dd021307fd9a12..954f1787c28a9e 100644 --- a/api/models/workflow.py +++ b/api/models/workflow.py @@ -1393,17 +1393,15 @@ def workflow_run(self): return None - @property - def created_by_account(self): + def created_by_account(self, session: orm.Session) -> Account | None: created_by_role = CreatorUserRole(self.created_by_role) - return db.session.get(Account, self.created_by) if created_by_role == CreatorUserRole.ACCOUNT else None + return session.get(Account, self.created_by) if created_by_role == CreatorUserRole.ACCOUNT else None - @property - def created_by_end_user(self): + def created_by_end_user(self, session: orm.Session): from .model import EndUser created_by_role = CreatorUserRole(self.created_by_role) - return db.session.get(EndUser, self.created_by) if created_by_role == CreatorUserRole.END_USER else None + return session.get(EndUser, self.created_by) if created_by_role == CreatorUserRole.END_USER else None def to_dict(self) -> WorkflowAppLogDict: result: WorkflowAppLogDict = { diff --git a/api/services/workflow_app_service.py b/api/services/workflow_app_service.py index 56665fe698dd20..453c130e1aafe6 100644 --- a/api/services/workflow_app_service.py +++ b/api/services/workflow_app_service.py @@ -23,17 +23,27 @@ class LogView: """Lightweight wrapper for WorkflowAppLog with computed details. - Exposes `details_` for marshalling to `details` in API response + - Resolves the account/end-user accessors through the session it was built with - Proxies all other attributes to the underlying `WorkflowAppLog` """ - def __init__(self, log: WorkflowAppLog, details: LogViewDetails | None): + def __init__(self, log: WorkflowAppLog, details: LogViewDetails | None, session: Session): self.log = log self.details_ = details + self._session = session @property def details(self) -> LogViewDetails | None: return self.details_ + @property + def created_by_account(self) -> Account | None: + return self.log.created_by_account(self._session) + + @property + def created_by_end_user(self) -> EndUser | None: + return self.log.created_by_end_user(self._session) + def __getattr__(self, name): return getattr(self.log, name) @@ -171,11 +181,11 @@ def get_paginate_workflow_app_logs( if detail: rows = session.execute(offset_stmt).all() items = [ - LogView(log, {"trigger_metadata": self.handle_trigger_metadata(app_model.tenant_id, meta_val)}) + LogView(log, {"trigger_metadata": self.handle_trigger_metadata(app_model.tenant_id, meta_val)}, session) for log, meta_val in rows ] else: - items = [LogView(log, None) for log in session.scalars(offset_stmt).all()] + items = [LogView(log, None, session) for log in session.scalars(offset_stmt).all()] return { "page": page, "limit": limit, diff --git a/api/tests/unit_tests/models/test_workflow_app_log_models.py b/api/tests/unit_tests/models/test_workflow_app_log_models.py new file mode 100644 index 00000000000000..9e06e069190a24 --- /dev/null +++ b/api/tests/unit_tests/models/test_workflow_app_log_models.py @@ -0,0 +1,80 @@ +"""Regression coverage for ``models.workflow.WorkflowAppLog`` account accessors. + +Ensures the ``@property``→session-parameter refactor preserves the role-based dispatch: +``created_by_account`` looks up an Account only when role is ACCOUNT; ``created_by_end_user`` +looks up an EndUser only when role is END_USER. + +Both accessors are exercised against the real ``sqlite_session`` fixture (a genuine +SQLAlchemy ``Session`` bound to a pristine full-schema SQLite database) so the assertions +cover actual query behaviour rather than a mock's recorded call. +""" + +from sqlalchemy.orm import Session + +from models.account import Account +from models.enums import CreatorUserRole, EndUserType +from models.model import EndUser +from models.workflow import WorkflowAppLog, WorkflowAppLogCreatedFrom + + +def _log(role: CreatorUserRole, created_by: str) -> WorkflowAppLog: + """Construct a WorkflowAppLog without touching the database.""" + return WorkflowAppLog( + tenant_id="00000000-0000-0000-0000-000000000001", + app_id="00000000-0000-0000-0000-000000000002", + workflow_id="00000000-0000-0000-0000-000000000003", + workflow_run_id="00000000-0000-0000-0000-000000000004", + created_from=WorkflowAppLogCreatedFrom.WEB_APP, + created_by_role=role, + created_by=created_by, + ) + + +class TestCreatedByAccount: + def test_returns_account_lookup_when_role_is_account(self, sqlite_session: Session) -> None: + account = Account(name="Test Account", email="test@example.com") + sqlite_session.add(account) + sqlite_session.flush() + log = _log(CreatorUserRole.ACCOUNT, created_by=account.id) + + result = log.created_by_account(session=sqlite_session) + + assert result is not None + assert result.id == account.id + + def test_returns_none_when_role_is_end_user(self, sqlite_session: Session) -> None: + account = Account(name="Test Account", email="test@example.com") + sqlite_session.add(account) + sqlite_session.flush() + log = _log(CreatorUserRole.END_USER, created_by=account.id) + + assert log.created_by_account(session=sqlite_session) is None + + +class TestCreatedByEndUser: + def test_returns_end_user_lookup_when_role_is_end_user(self, sqlite_session: Session) -> None: + end_user = EndUser( + tenant_id="00000000-0000-0000-0000-000000000001", + type=EndUserType.BROWSER, + session_id="session-1", + ) + sqlite_session.add(end_user) + sqlite_session.flush() + log = _log(CreatorUserRole.END_USER, created_by=end_user.id) + + result = log.created_by_end_user(session=sqlite_session) + + assert result is not None + assert result.id == end_user.id + + def test_returns_none_when_role_is_account(self, sqlite_session: Session) -> None: + end_user = EndUser( + tenant_id="00000000-0000-0000-0000-000000000001", + type=EndUserType.BROWSER, + session_id="session-1", + ) + sqlite_session.add(end_user) + sqlite_session.flush() + log = _log(CreatorUserRole.ACCOUNT, created_by=end_user.id) + + assert log.created_by_end_user(session=sqlite_session) is None diff --git a/api/tests/unit_tests/services/test_workflow_app_service_metadata.py b/api/tests/unit_tests/services/test_workflow_app_service_metadata.py index ded4fab1c7a69c..c51de1abaa961a 100644 --- a/api/tests/unit_tests/services/test_workflow_app_service_metadata.py +++ b/api/tests/unit_tests/services/test_workflow_app_service_metadata.py @@ -2,10 +2,12 @@ import json import uuid -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest +from sqlalchemy.orm import Session +from models.account import Account from models.enums import AppTriggerType, CreatorUserRole from models.workflow import WorkflowAppLog, WorkflowAppLogCreatedFrom from services.workflow_app_service import LogView, WorkflowAppService @@ -24,11 +26,32 @@ def test_details_and_proxy_attributes(self) -> None: ) log.id = "log-1" - view = LogView(log=log, details={"trigger_metadata": {"type": "plugin"}}) + view = LogView(log=log, details={"trigger_metadata": {"type": "plugin"}}, session=MagicMock()) assert view.details == {"trigger_metadata": {"type": "plugin"}} assert view.id == "log-1" + def test_account_accessors_resolve_via_wrapped_session(self, sqlite_session: Session) -> None: + account = Account(name="Test Account", email="test@example.com") + sqlite_session.add(account) + sqlite_session.flush() + log = WorkflowAppLog( + tenant_id="tenant-1", + app_id="app-1", + workflow_id="workflow-1", + workflow_run_id="run-1", + created_from=WorkflowAppLogCreatedFrom.WEB_APP, + created_by_role=CreatorUserRole.ACCOUNT, + created_by=account.id, + ) + + view = LogView(log=log, details=None, session=sqlite_session) + + resolved = view.created_by_account + assert resolved is not None + assert resolved.id == account.id + assert view.created_by_end_user is None + class TestHandleTriggerMetadata: def test_returns_empty_dict_when_metadata_missing(self) -> None: