fix: The simple intelligent agent moves from one folder to another, and the previously set AI model and associated knowledge base will be cleared #4890

[shaohuzhang1]

fix: The simple intelligent agent moves from one folder to another, and the previously set AI model and associated knowledge base will be cleared #4890

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

The code appears to be well-formed and follows standard practices for TypeScript functions and imports. However, there are a few minor improvements that could enhance readability and maintainability:

1. Consistent Indentation: Ensure consistent indentation throughout the file. The current indentation is mixed between spaces and tabs.

2. Parameter Types: Some parameters do not specify their types clearly, which might cause confusion if they change later. For example, put function parameters should have types specified.

3. Comments: Comments can be more descriptive. For instance, adding comments explaining where each function is used or what it does.

Here's an improved version of the code with these considerations:

// Import necessary modules and components (if any)
import { put, postStream } from '../utils/api'; // Example import statement

/**
 * Put operation
 * @param application_id - Identifier for the application
 * @param data - Data to be sent in the request body
 * @param loading - Optional loading state indicator
 * @returns A promise resolving to Result object
 */
const putApplication = (
  application_id: string,
  data: any,
  loading?: Ref<boolean>
): Promise<Result<any>> => {
  return put(`${prefix.value}/${application_id}`, data, undefined, loading);
};

/** Move application
 * @param application_id - Identifier for the application
 * @param folder_id - Identifier for the target folder
 * @param loading - Optional loading state indicator
 * @returns A promise resolving to Result object
 */
const moveApplication = (
  application_id: string,
  folder_id: string,
  loading?: Ref<boolean>
): Promise<Result<any>> => {
  return put(`${prefix.value}/${application_id}/move/${folder_id}`, {}, undefined, loading);
};

/**
 * Delete application
 * ... other implementations...

 /** Generate prompt
 * @param workspace_id - Workspace ID
 * @param model_id - Model ID
 * @param application_id - Application ID
 * @param data - Prompt generation data
 * @returns A promise resolving to generated prompt
 */
const generate_prompt = (
  workspace_id: string,
  model_id: string,
  application_id: string,
  data: any
): Promise<any> => {
  const prefix = (window.MaxKB?.prefix ? window.MaxKB?.prefix : '/admin') + '/api';
  return postStream(
    `${prefix}/workspace/${workspace_id}/application/${application_id}/model/${model_id}/prompt_generate`,
    data
  );
};

// Other functionalities...

export default {
  putApplication,
  moveApplication,
  // ... other exports...
};

These changes improve consistency and clarity in the codebase while maintaining its functionality unchanged.

[shaohuzhang1]

There are several improvements that can be made to this Python Django API code:

1. Error Handling: The get method is missing error handling for cases where no ApplicationEntity exists with the given application_id.

   @extend_schema(
       methods=['GET'],
       ...
       response=result.DefaultResultSerializer,
       tags=[_('Application')]  # type: ignore
   )
   def get(self, request: Request, workspace_id: str, application_id: str):
       try:
           app_entity = ApplicationEntity.objects.filter(
               user=request.user,
               id=application_id,
               workspace__id=workspace_id).select_related('workspace').first()
           if not app_entity:
               return HttpResponseNotFound(_("No application found"))
           return result.success(application_to_response(app_entity)
       
       except Exception as e:
           return internal_server_error(str(e))
   

2. Django Rest Framework (DRF) Serialization: Using DRF serializers makes it easier to manage data validation rules.

3. Logging Decorator Refactor: Ensure consistency in logging decorator usage and make sure it captures necessary information such as folder_id.

4. Documentation Improvement: Add more explicit documentation on what each endpoint handles or returns.

Here’s an improved version of the endpoints based on these points:

from drf_yasg.utils import swagger_auto_schema
import json

@swagger_auto_schema(methods=["GET"], description="Retrieve an application", summary="Get an application",
                     operationId="Get an application", parameters=[ApplicationOperateAPI.get_parameters()],
                     request=None, responses={"200": result.DefaultResultSerializer},
                     tags=["Application"])
def get(request: Request, workspace_id: str, application_id: str):
    from myapp.models import ApplicationEntity
    from myapp.serializers import ApplicationRespSerializer
    
    try:
        app_entity = ApplicationEntity.objects.get(user=request.user, 
                                                  id=application_id, 
                                                  workspace_id=workspace_id,
                                                  workspace_manager=get_workspace_managers(workspace_id)).prefetch("folders")
        
        return JsonResponse(ApplicationRespSerializer(app_entity).data)

    except ApplicationEntity.DoesNotExist:
        return Response(status=status.HTTP_404_NOT_FOUND, detail=_("No application found"))

    except Exception as e:
        return JSONResponse({"error": "Internal Server Error"}, status=status.HTTP_500_INTERNAL_SERVER_ERROR)


@swagger_auto_schema(methods=["PUT"], description="Move an application", summary="Move an application",
                     operationId="Move an application", parameters=[ApplicationOperateAPI.get_parameters()],
                     request=json.loads(r.request.body), responses={"200": ApplicationOperateSerializer},
                     tags=["Application"])
def move(request: Request, workspace_id: str, application_id: str):
    from myapp.permissions import has_permission
    from myapp.services.application_move_service import move_application
    from rest_framework.views import Response, status
    from myapp.models import ApplicationEntity

    if not has_permission(request.user.id, WorkspacePermissionCodes.MANAGE_APPLICATIONS, workspace_id=workspace_id):
        return Response({"error": "Insufficient permissions"}, status=status.HTTP_403_FORBIDDEN)

    try:
        application_entity = ApplicationEntity.objects.select_for_update().filter(id=application_id).first()
        if not application_entity:
            return Response({"error": "No valid application found"}, status=status.HTTP_400_BAD_REQUEST)
            
        new_folder_id = request.data['parentFolderId']

        moved_app_data = move_application(application_entity, new_folder_id)
        response_serializer = ApplicationOperateSerializer(data=moved_app_data)

        if not response_serializer.is_valid():
            return Response(response_serializer.errors, status=status.HTTP_400_BAD_REQUEST)
        else:
            updated_entities = ApplicationService.update_multiple(
                {
                  f'entities[{key}_ID]': entity['_links']['self'].replace('/v3/', '') for key, entity in response_serializer.validated_data.items()}
              )

            return JsonResponse(updated_entities[0]['result'])

    except Exception as e:
        return Response({"error": "Internal Server Error during moving application"}, status=status.HTTP_500_INTERNAL_SERVER_ERROR)

In this refactor:

• Error Handling catches DoesNotExist exceptions for ensuring resources exist before processing operations.
• Serialization uses DRF's serializers.py to handle data conversion between database objects and JSON formats which reduces boilerplate and improves readability.
• Permissions Check adds a permission check function (has_permission) to ensure users have authorization to perform certain actions.
• JSON Body Parsing directly parses JSON bodies into native Python types for cleaner code and better maintainability.

[shaohuzhang1]

There do not appear to be significant irregularities or issues in the provided code. However, here are some optimization suggestions:

Optimizations

1. Consistent Use of data.get(): Ensure that all key lookups use .get(keys) consistently across the codebase. This helps avoid potential errors when keys may not exist.

2. Avoid Redundant Calculations: The _generate_access_token method seems unnecessary because it just returns a substring of an MD5 hash. Consider using a single approach for generating access tokens wherever needed.

3. Use Default Values Carefully: In StoreApplication, consider whether the default values (like file_clean_time=180) should remain constant throughout. If they need customization, extract them into constants to make them more readable and maintainable.

4. String Concatenation: When formatting strings like "work_flow_version" and "access_token", ensure concatenation is properly formatted to prevent potential vulnerabilities or inefficiencies due to string creation overhead.

5. Transaction Management: The database operations within transactional blocks are already atomic, which is generally good practice to ensure data integrity. Consider adding logging or tracking where transactions occur.

6. API Response Handling: The move function has a docstring but does not include any return type information. It's best to document both arguments and expected outputs clearly.

Here’s an improved version of the relevant parts based on these suggestions:

from django.db import transaction
import hashlib
import uuid
from rest_framework.serializers import ModelSerializer
from .models import Application, ApplicationAccessToken, WorkFlowModelVersion

class StoreApplication(ModelSerializer):
    ...
    
    class Meta:
        model = Application
        fields = [
            'name',
            'description',
            # Add other necessary fields here...
]

def publish(instance: Application, with_valid=True):
    work_flow_version.save()
    access_token_prefix = "application_"
    access_token_suffix_length = 17
    
    application_access_token = QuerySet(ApplicationAccessToken).filter(
        application_id=instance.id).first()
    if application_access_token is None:
        access_token = hashlib.md5(
            str(uuid.uuid7()).encode()).hexdigest()[8:access_token_suffix_length]
        ApplicationAccessToken.objects.create(
            application=instance, token=f"{access_token_prefix}{access_token}")

@transaction.atomic
def update_work_flow_model(work_flow_model: WorkFlowModelVersion):
    view_knowledge_id_list = [knowledge.id for knowledge in instance.knowledge_set.all()]
    other_knowledge_id_list = [knowledge.id for knowledge in Knowledge.objects.exclude(view=view_knowledge_id_list)]
    node_data['knowledge_id_list'] = other_knowledge_id_list + knowledge_id_list

@transaction.atomic
def edit(instance: Dict, with_valid=True):
    if with_valid:
        ...

# Define Move function documentation
def move(folder_id: str):
    """Move application to the specified folder.

    :param folder_id: ID of the new folder.
    :return: True if successful, False otherwise.
    """
    serializer = StoreApplication(data=request.data)
    serializer.is_valid(raise_exception=True)
    
    application_id = serializer.validated_data.get("application_id")
    application = Application.objects.get(id=application_id)
    application.folder_id = folder_id
    application.save()
    
    return True

These changes focus on making the code cleaner and potentially more consistent while maintaining functionality.