Skip to content

fix: Knowledge Base Workflow Execution List #4437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shaohuzhang1 merged 1 commit into from
Dec 4, 2025
Merged

fix: Knowledge Base Workflow Execution List #4437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions apps/knowledge/api/knowledge_workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from common.mixins.api_mixin import APIMixin
from knowledge.serializers.knowledge_workflow import KnowledgeWorkflowActionRequestSerializer
from knowledge.serializers.knowledge_workflow import KnowledgeWorkflowActionListQuerySerializer


class KnowledgeWorkflowApi(APIMixin):
Expand All @@ -14,6 +15,12 @@ class KnowledgeWorkflowVersionApi(APIMixin):
pass


class KnowledgeWorkflowActionPageApi(APIMixin):
@staticmethod
def get_request():
return KnowledgeWorkflowActionListQuerySerializer


class KnowledgeWorkflowActionApi(APIMixin):
@staticmethod
def get_request():
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code snippet is mostly clear and follows Python conventions. However, there are several areas that could be improved or optimized:

  1. Imports: The imports are fine but consider using an alias if APIMixin becomes longer.

  2. Docstrings: Ensure that all classes have a docstring to describe their purpose, methods should also have concise descriptions.

  3. Static Methods: Using static methods when they don't need access to instance attributes can simplify the code structure. Consider making these into regular methods without @staticmethod.

  4. Class Names and Variable Names: Use meaningful names for variables and ensure consistency across the class hierarchy (e.g., camelCase instead of snake_case).

Here's a revised version with some minor improvements:

from common.mixins.api_mixin import APIMixin
from knowledge.serializers.knowledge_workflow import (
    KnowledgeWorkflowActionRequestSerializer,
    KnowledgeWorkflowActionListQuerySerializer,
)
import logging

logger = logging.getLogger(__name__)

class KnowledgeWorkflowApi(APIMixin):
    ...

class KnowledgeWorkflowVersionApi(APIMixin):
    ...

class KnowledgeWorkflowActionPageApi(APIMixin):
    @classmethod
    def get_request(cls):
        logger.info("Retrieving KnowledgeWorkflowActionListQuerySerializer")
        return cls.KNOWLEDGE_WORKFLOW_ACTION_LIST_QUERY_SERIALIZER


class KnowledgeWorkflowActionApi(APIMixin):
    @classmethod
    def get_request(cls):
        logger.info("Retrieving KnowledgeWorkflowActionRequestSerializer")
        return cls.KNOWLEDGE_WORKFLOW_ACTION_REQUEST_SERIALIZER

Key Changes:

  • Moved static method logic to regular methods within each class to improve modularity and clarity.
  • Added a logger for debugging purposes, which can help during development.
  • Ensured correct attribute references (cls.KNOWLEDGE_WORKFLOW_ACTION_*) inside get_request methods. Make sure this constant is defined somewhere in one of these classes or elsewhere in your project. If not, define it properly at global scope.

Expand Down
4 changes: 2 additions & 2 deletions apps/knowledge/serializers/knowledge_workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ def list(self, instance: Dict, is_valid=True):
self.is_valid(raise_exception=True)
KnowledgeWorkflowActionListQuerySerializer(data=instance).is_valid(raise_exception=True)
return [{'id': a.id, 'knowledge_id': a.knowledge_id, 'state': a.state,
'details': a.details, 'meta': a.meta, 'run_time': a.run_time} for a in self.get_query_set(instance)]
'meta': a.meta, 'run_time': a.run_time} for a in self.get_query_set(instance)]

def page(self, current_page, page_size, instance: Dict, is_valid=True):
if is_valid:
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code looks mostly clean and well-formatted. However, there are a couple of minor improvements and suggestions:

  1. Function Naming Consistency: Ensure that all function names are consistent throughout your codebase. For example, get_query_set could be renamed to something more descriptive like fetch_actions.

  2. Empty List Handling: In case self.get_query_set(instance) returns an empty list, you might want to handle this gracefully in your method, perhaps by returning an empty list or raising a custom exception.

  3. Comments and Documentation: Adding comments to clarify the purpose of each part can enhance readability. You also mentioned potential security concerns about exposing sensitive information (details, meta). It might be helpful to document these fields and consider how they should be handled securely, such as logging them but not displaying them publicly.

  4. Performance Considerations: If self.get_query_set(instance) becomes time-consuming,consider caching results temporarily based on input parameters.

Here's a slightly improved version of your code with some of these considerations:

@@ -61,15 +61,18 @@ def list(self, instance: Dict, is_valid=True) -> List[Dict]:
             self.is_valid(raise_exception=True)
             KnowledgeWorkflowActionListQuerySerializer(data=instance).is_valid(raise_exception=True)
         return [
-            {'id': a.id, 'knowledge_id': a.knowledge_id, 'state': a.state,
+            {
                 'id': a.id,
                 'knowledge_id': a.knowledge_id,
                 'state': a.state,
-                'details': a.details, 'meta': a.meta, 'run_time': a.run_time}
+                'meta': a.meta,
+                'run_time': a.run_time
+            }
             for a in self.fetch_actions(instance)
         ]

     def fetch_actions(self, instance: Dict) -> Iterable[KnowledgeWorkflowAction]:
         """Fetches actions from the database."""
         # Example implementation using Django ORM
         return (
             KnowledgeWorkflowAction.objects.filter(knowledge_id=instance['knowledge_id'])
                                     .order_by('created_at')
                                 )

@@ -77,7 +80,7 @@ def page(self, current_page: int, page_size: int, instance: Dict, is_valid=True) \
                             raise CustomException("Invalid data provided")
                         result = page_search(
                             current_page=current_page,
                             page_size=page_size,
                             items=self.fetch_actions(instance),
                             item_serializer=lambda a: {
                                 'id': a.id,
                                 'knowledge_id': a.knowledge_id,
                                 'state': a.state,
-                                'details': a.details,
+                                'metadata': a.metadata,
                                 'meta': a.meta,
                                 'run_time': a.run_time
                             }
@@ -102,6 +105,7 @@ def action(self, instance: Dict, user, with_valid=True):
                     if with_valid:
                         self.validate_action_data(instance)

+# Assuming CustomException is defined somewhere else in your project
class CustomException(Exception):
    pass

These modifications improve clarity and potentially add robustness, though without additional context about your application and its requirements, further optimizations may be necessary specific to your use-case.

self.is_valid(raise_exception=True)
KnowledgeWorkflowActionListQuerySerializer(data=instance).is_valid(raise_exception=True)
return page_search(current_page, page_size, self.get_query_set(instance),
lambda a: {'id': a.id, 'knowledge_id': a.knowledge_id, 'state': a.state,
'details': a.details, 'meta': a.meta, 'run_time': a.run_time})
'meta': a.meta, 'run_time': a.run_time})

def action(self, instance: Dict, user, with_valid=True):
if with_valid:
Expand Down
5 changes: 3 additions & 2 deletions apps/knowledge/views/knowledge_workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
from common.constants.permission_constants import PermissionConstants, RoleConstants, ViewPermission, CompareConstants
from common.log.log import log
from common.result import result, DefaultResultSerializer
from knowledge.api.knowledge_workflow import KnowledgeWorkflowApi, KnowledgeWorkflowActionApi
from knowledge.api.knowledge_workflow import KnowledgeWorkflowApi, KnowledgeWorkflowActionApi, \
KnowledgeWorkflowActionPageApi
from knowledge.serializers.common import get_knowledge_operation_object
from knowledge.serializers.knowledge_workflow import KnowledgeWorkflowSerializer, KnowledgeWorkflowActionSerializer, \
KnowledgeWorkflowMcpSerializer
Expand Down Expand Up @@ -118,7 +119,7 @@ class Page(APIView):
summary=_('Page Knowledge workflow action'),
operation_id=_('Page Knowledge workflow action'), # type: ignore
parameters=KnowledgeWorkflowActionApi.get_parameters(),
request=KnowledgeWorkflowActionApi.get_request(),
request=KnowledgeWorkflowActionPageApi.get_request(),
responses=KnowledgeWorkflowActionApi.get_response(),
tags=[_('Knowledge Base')] # type: ignore
)
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No significant errors or critical issues were found in the modified code snippet. Here is a brief overview of what was checked and updated:

  1. Imports: No major discrepancies found in imports.

  2. Methods:

    • The page view remains functional as per its previous definitions.
    • The operation_type, summary, path, description, tags, and other properties remain unchanged.

  3. Response:

    • The response dictionary remained essentially the same, with no new keys adding ambiguity to the API description.

Overall, the update seems straightforward without introducing bugs or significant performance optimizations. If there are ongoing maintenance tasks that need attention, this might be a good place for future modifications based on feedback or changes in requirements.

Expand Down
Loading