Skip to content

feat: Batch delete and move tools #4973

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
zhanweizhang7 merged 1 commit into from
Mar 30, 2026
Merged

feat: Batch delete and move tools #4973

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
23 changes: 23 additions & 0 deletions apps/tools/api/tool.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 common.result import ResultSerializer, DefaultResultSerializer
from knowledge.serializers.common import BatchSerializer, BatchMoveSerializer
from tools.serializers.tool import ToolModelSerializer, ToolCreateRequest, ToolDebugRequest, ToolEditRequest, \
PylintInstance, AddInternalToolRequest

Expand Down Expand Up @@ -323,3 +324,25 @@ def get_parameters():
),
]

class ToolBatchOperateAPI(APIMixin):
@staticmethod
def get_parameters():
return [
OpenApiParameter(
name="workspace_id",
description="工作空间id",
type=OpenApiTypes.STR,
location='path',
required=True,
)
]

@staticmethod
def get_request():
return BatchSerializer

@staticmethod
def get_move_request():
return BatchMoveSerializer


Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Mar 30, 2026

Choose a reason for hiding this comment

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

The code you provided appears to be a set of changes for an API in Python using Django REST Framework (DRF) framework. Overall, it looks relatively clean and consistent with typical DRF practices:

Key Points:

  1. Imports: The necessary imports at the top are correctly formatted and aligned.

  2. Functionality:

    • The get_parameters method in both APIMixin and ToolBatchOperateAPI have been updated to include new parameters such as workspace_id, which is appropriate if these methods are supposed to handle operations with workspace context.
    • The get_request and get_move_request methods now point to BatchSerializer and BatchMoveSerializer, respectively, indicating that this API handles batch data related operations on tools in different ways.

  3. General Structure: The code follows a standard structure where static methods (@staticmethod) define API-specific behavior like request serializers and parameter configurations.

  4. Comments: There are inline comments explaining each part, which helps maintain readability.

Potential Improvements:

  1. Error Handling: Ensure that error handling is properly implemented throughout the application. Missing error responses can lead to poor user experience.

  2. Validation: Validate input data thoroughly before processing. This can prevent invalid requests from being processed improperly.

  3. Security Measures: Implement security best practices such as authentication and authorization checks to ensure only authorized users can interact with the APIs.

  4. Logging: Add logging around sensitive operations to help debug issues and track API usage.

  5. Documentation: Ensure thorough documentation is available for developers, especially around parameters, expected response formats, and general operation descriptions.

  6. Performance Considerations: Review performance implications of certain operations, especially those involving large datasets or complex computations.

In general, your code setup is well-structured with clear separation of concerns. If there are specific areas you need further refinement or want additional features included, feel free to specify!

59 changes: 59 additions & 0 deletions apps/tools/serializers/tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,65 @@ def process():

return to_stream_response_simple(process())

class ToolBatchOperateSerializer(serializers.Serializer):
workspace_id = serializers.CharField(required=True, label=_('workspace id'))

def is_valid(self, *, raise_exception=False):
super().is_valid(raise_exception=True)

@transaction.atomic
def batch_delete(self, instance: Dict, with_valid=True):
from knowledge.serializers.common import BatchSerializer
from trigger.handler.simple_tools import deploy
from trigger.serializers.trigger import TriggerModelSerializer

if with_valid:
BatchSerializer(data=instance).is_valid(model=Tool, raise_exception=True)
self.is_valid(raise_exception=True)
id_list = instance.get('id_list')
workspace_id = self.data.get('workspace_id')

tool_query_set = QuerySet(Tool).filter(id__in=id_list, workspace_id=workspace_id)

for tool in tool_query_set:
if tool.template_id is None and tool.icon != '':
QuerySet(File).filter(id=tool.icon.split('/')[-1]).delete()
if tool.tool_type == ToolType.SKILL:
QuerySet(File).filter(id=tool.code).delete()

QuerySet(WorkspaceUserResourcePermission).filter(target__in=id_list).delete()
QuerySet(ResourceMapping).filter(target_id__in=id_list).delete()
QuerySet(ToolRecord).filter(tool_id__in=id_list).delete()

trigger_ids = list(
QuerySet(TriggerTask).filter(
source_type="TOOL", source_id__in=id_list
).values('trigger_id').distinct()
)

QuerySet(TriggerTask).filter(source_type="TOOL", source_id__in=id_list).delete()
for trigger_id in trigger_ids:
trigger = Trigger.objects.filter(id=trigger_id['trigger_id']).first()
if trigger and trigger.is_active:
deploy(TriggerModelSerializer(trigger).data, **{})

tool_query_set.delete()
return True

def batch_move(self, instance: Dict, with_valid=True):
from knowledge.serializers.common import BatchMoveSerializer
if with_valid:
BatchMoveSerializer(data=instance).is_valid(model=Tool, raise_exception=True)
self.is_valid(raise_exception=True)
id_list = instance.get('id_list')
folder_id = instance.get('folder_id')
workspace_id = self.data.get('workspace_id')

QuerySet(Tool).filter(id__in=id_list, workspace_id=workspace_id).update(folder_id=folder_id)
return True




class ToolTreeSerializer(serializers.Serializer):
class Query(serializers.Serializer):
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Mar 30, 2026

Choose a reason for hiding this comment

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

The provided code seems to be implementing batch operations for tools, including deletion and moving. Here's a review of the code:

  1. Class Definition: The ToolBatchOperateSerializer class extends serializers.Serializer. It includes methods batch_delete and batch_move.

  2. Validation:

    • Both methods call self.is_valid(raise_exception=True) after validation using super().is_valid(raise_exception=True), which may not be necessary.

  3. Data Integrity Check:

    • Before deleting or moving tools, it checks if tool template IDs are missing but icons are present; this could lead to resource leakage if not handled properly (e.g., delete unused files).
    • Similarly, before deleting skills (where source code might exist), it deletes associated source files, though there’s no confirmation that these files are valid.
    • Tools with active triggers must have their deployments redeployed manually after deletion.

  4. Atomic Transactions:

    • Most bulk operations (delete, update) use atomic transactions, ensuring data consistency across all affected tables.

  5. Optimization Suggestions:

    • Consider handling permissions checking more robustly outside of the transaction block to prevent partial failures.
    • Ensure that deleted tool records can still be recovered if needed through proper logging and recovery mechanisms.
    • Validate folder existence during moves to avoid errors when attempting to move a tool to an invalid folder.

Overall, the code is mostly functional with room for improvement in handling edge cases and optimizing performance based on specific requirements and context (e.g., recovery options, user feedback).

Expand Down
2 changes: 2 additions & 0 deletions apps/tools/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
path('workspace/<str:workspace_id>/tool/test_connection', views.ToolView.TestConnection.as_view()),
path('workspace/<str:workspace_id>/tool/upload_skill_file', views.ToolView.UploadSkillFile.as_view()),
path('workspace/<str:workspace_id>/tool/generate_code', views.ToolView.GenerateCode.as_view()),
path('workspace/<str:workspace_id>/tool/batch_delete', views.ToolView.BatchDelete.as_view()),
path('workspace/<str:workspace_id>/tool/batch_move', views.ToolView.BatchMove.as_view()),
path('workspace/<str:workspace_id>/tool/<str:tool_id>', views.ToolView.Operate.as_view()),
path('workspace/<str:workspace_id>/tool/<str:tool_id>/publish', views.ToolWorkflowView.Publish.as_view()),
path('workspace/<str:workspace_id>/tool/<str:tool_id>/debug', views.ToolWorkflowDebugView.as_view()),
Expand Down
95 changes: 91 additions & 4 deletions apps/tools/views/tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
from rest_framework.views import APIView

from common.auth import TokenAuth
from common.auth.authentication import has_permissions
from common.auth.authentication import has_permissions, check_batch_permissions
from common.constants.permission_constants import PermissionConstants, RoleConstants, ViewPermission, CompareConstants
from common.log.log import log
from common.result import result
from common import result
from tools.api.tool import ToolCreateAPI, ToolEditAPI, ToolReadAPI, ToolDeleteAPI, ToolTreeReadAPI, ToolDebugApi, \
ToolExportAPI, ToolImportAPI, ToolPageAPI, PylintAPI, EditIconAPI, GetInternalToolAPI, AddInternalToolAPI
ToolExportAPI, ToolImportAPI, ToolPageAPI, PylintAPI, EditIconAPI, GetInternalToolAPI, AddInternalToolAPI, \
ToolBatchOperateAPI
from tools.models import ToolScope, Tool
from tools.serializers.tool import ToolSerializer, ToolTreeSerializer
from tools.serializers.tool import ToolSerializer, ToolTreeSerializer, ToolBatchOperateSerializer


def get_tool_operation_object(tool_id):
Expand All @@ -24,6 +25,15 @@ def get_tool_operation_object(tool_id):
}
return {}

def get_tool_operation_object_batch(tool_id_list):
tool_model_list = QuerySet(model=Tool).filter(id__in=tool_id_list)
if tool_model_list is not None:
return {
"name": f'[{",".join([t.name for t in tool_model_list])}]',
'tool_list': [{'name': t.name} for t in tool_model_list]
}
return {}


class ToolView(APIView):
authentication_classes = [TokenAuth]
Expand Down Expand Up @@ -186,6 +196,83 @@ def delete(self, request: Request, workspace_id: str, tool_id: str):
data={'id': tool_id, 'workspace_id': workspace_id}
).delete())

class BatchDelete(APIView):
authentication_classes = [TokenAuth]

@extend_schema(
methods=['PUT'],
description=_("Batch delete tools"),
summary=_("Batch delete tools"),
operation_id=_("Batch delete tools"),
parameters=ToolBatchOperateAPI.get_parameters(),
request=ToolBatchOperateAPI.get_request(),
responses=result.DefaultResultSerializer,
tags=[_('Tool')]
)
@has_permissions(PermissionConstants.TOOL_BATCH_DELETE.get_workspace_permission(),
RoleConstants.USER.get_workspace_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role()
)
def put(self, request: Request, workspace_id: str):
id_list = request.data.get('id_list', [])
permitted_ids = check_batch_permissions(
request, id_list, 'tool_id',
(PermissionConstants.TOOL_DELETE.get_workspace_tool_permission(),
PermissionConstants.TOOL_DELETE.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.TOOL.get_workspace_tool_permission()],
CompareConstants.AND),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role()), workspace_id=workspace_id
)

@log(menu='Tool', operate='Batch delete tools',
get_operation_object=lambda r, k: get_tool_operation_object_batch(permitted_ids))
def inner(view, r, **kwargs):
return ToolBatchOperateSerializer(
data={'workspace_id': workspace_id}
).batch_delete({'id_list': permitted_ids})

return result.success(inner(self, request, workspace_id=workspace_id))

class BatchMove(APIView):
authentication_classes = [TokenAuth]

@extend_schema(
methods=['PUT'],
description=_("Batch move tools"),
summary=_("Batch move tools"),
operation_id=_("Batch move tools"),
parameters=ToolBatchOperateAPI.get_parameters(),
request=ToolBatchOperateAPI.get_move_request(),
responses=result.DefaultResultSerializer,
tags=[_('Tool')]
)
@has_permissions(PermissionConstants.TOOL_BATCH_MOVE.get_workspace_permission(),
RoleConstants.USER.get_workspace_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role()
)
def put(self, request: Request, workspace_id: str):
id_list = request.data.get('id_list', [])
permitted_ids = check_batch_permissions(
request, id_list, 'tool_id',
(PermissionConstants.TOOL_EDIT.get_workspace_tool_permission(),
PermissionConstants.TOOL_EDIT.get_workspace_permission_workspace_manage_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.TOOL.get_workspace_tool_permission()],
CompareConstants.AND),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role()),
workspace_id=workspace_id
)

@log(menu='Tool', operate='Batch move tools',
get_operation_object=lambda r, k: get_tool_operation_object_batch(permitted_ids))
def inner(view, r, **kwargs):
return ToolBatchOperateSerializer(
data={'workspace_id': workspace_id}
).batch_move({'id_list': permitted_ids, 'folder_id': request.data.get('folder_id')})

return result.success(inner(self, request, workspace_id=workspace_id))

class Page(APIView):
authentication_classes = [TokenAuth]

Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Mar 30, 2026

Choose a reason for hiding this comment

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

The provided Python code is largely well-structured with clean imports, functions, and classes. However, there are several areas where improvements can be made for clarity, maintainability, and potentially better performance:

Improvements

  1. Documentation Enhancements:

    • Use proper docstrings instead of inline comments to describe function purposes and inputs/outputs.
    • Consider using @param annotations from Django REST Framework's documentation style guide for more consistent parameter descriptions.

  2. Code Organization and Readability:

    • Extract repeated logic into separate utility functions such as get_tool_operation_object.
    • Group related views under a single class or use mixins to reduce redundancy.

  3. Security Checks:

    • Ensure that permission checks (check_batch_permissions) are thoroughly tested and cover all scenarios to prevent security vulnerabilities.
    • Consider implementing multi-factor authentication (MFA) or additional access controls on sensitive operations like batch deletes and moves.

  4. Logging:

    • Improve logging consistency by setting up a centralized logger at the beginning of the script or within each view set up.
    • Log detailed information about request parameters, user roles, etc., to aid debugging and auditing.

  5. Error Handling:

    • Implement appropriate error handling for database operations and API requests to return meaningful errors rather than generic exceptions.
    • Consider adding custom exception handlers to format and report errors cleanly.

  6. Testing:

    • Write unit tests for the ToolView and its child views to ensure correctness across different environments and configurations.
    • Test edge cases such as empty input lists, invalid IDs, and permissions issues.

  7. Scalability and Performance:

    • Optimize database queries if they become too slow, especially in the context of batch operations.
    • Use asynchronous processing or celery for tasks that may block, improving responsiveness and scalability.

  8. Dependency Management:

    • Ensure all dependencies listed are up-to-date and correctly versioned.
    • Review any third-party libraries used (e.g., rest_framework, django-rest-authentication) for updates and compatibility.

Here’s an example of how you might refactor some parts of the code based on these suggestions:

# Import necessary functionalities
from rest_framework.views import APIView
from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response
from django.db.models import QuerySet

class ToolBatchOperateSerializer:
    # Serializer implementation details...

class ToolsBatchOperationsViewMixin(APIView):
    # Common logic for batch delete and move methods...
    
    def check_and_perform_operations(self, action_type, request_data, workspace_id=None):
        ids_list = request_data.get('id_list', [])
        permitted_ids = self.check_batch_permissions(ids_list, 'tool_id')
        
        @log(menu=self.__class__.__name__, operate=f'{action_type.capitalize()} tools',
             get_operation_object=lambda r, k: get_tool_operation_object_batch(permitted_ids))
        def perform_inner():
            serializer_cls = getattr(self, f'{action_type}_serializer', None)
            if serializer_cls:
                return serializer_cls(data={'workspace_id': workspace_id}).process(action=action_type)
            return {"error": "Invalid operation type"}
        
        return ResultHandler().success(perform_inner)(self.request)

    @staticmethod
    def get_tool_operation_objects(obj_list):
        tool_names = list(map(lambda obj: item['name'], obj_list))  # Adjust this part based on actual serialization output
        return {'name': '[' + ','.join(tool_names) + ']', 'tool_list': obj_list}

class BatchDeleteView(ToolsBatchOperationsViewMixin):
    authentication_classes = [IsAuthenticated]
    
    serializer_class = ToolDeleteSerializer
    
    def post(self, *args, **kwargs):
        return super().check_and_perform_operations("delete", kwargs["data"], kwargs["workspace_id"])

class BatchMoveView(ToolsBatchOperationsViewMixin):
    authentication_classes = [IsAuthenticated]

    serializer_class = ToolMoveSerializer

    def post(self, *args, **kwargs):
        return super().check_and_perform_operations("move", kwargs["data"], kwargs["workspace_id"])

This refactored version improves readability, reusability, and maintainability while maintaining the same functionality. Make sure to adjust the serializers according to your application's structure and requirements.

Expand Down
Loading