Skip to content

feat: application version #3194

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
merged 1 commit into from
Jun 4, 2025
Merged

feat: application version #3194

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
96 changes: 96 additions & 0 deletions apps/application/api/application_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# coding=utf-8
"""
@project: MaxKB
@Author：虎虎
@file： application_version.py
@date：2025/6/4 17:33
@desc:
"""
from drf_spectacular.types import OpenApiTypes
from drf_spectacular.utils import OpenApiParameter

from application.serializers.application_version import ApplicationVersionModelSerializer
from common.mixins.api_mixin import APIMixin
from common.result import ResultSerializer, PageDataResponse, ResultPageSerializer


class ApplicationListVersionResult(ResultSerializer):
def get_data(self):
return ApplicationVersionModelSerializer(many=True)


class ApplicationPageVersionResult(ResultPageSerializer):
def get_data(self):
return ApplicationVersionModelSerializer(many=True)


class ApplicationWorkflowVersionResult(ResultSerializer):
def get_data(self):
return ApplicationVersionModelSerializer()


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


class ApplicationVersionOperateAPI(APIMixin):
@staticmethod
def get_parameters():
return [
OpenApiParameter(
name="work_flow_version_id",
description="工作流版本id",
type=OpenApiTypes.STR,
location='path',
required=True,
)
, *ApplicationVersionAPI.get_parameters()
]

@staticmethod
def get_response():
return ApplicationWorkflowVersionResult


class ApplicationVersionListAPI(APIMixin):
@staticmethod
def get_parameters():
return [
OpenApiParameter(
name="name",
description="Version Name",
type=OpenApiTypes.STR,
required=False,
)
, *ApplicationVersionAPI.get_parameters()]

@staticmethod
def get_response():
return ApplicationListVersionResult


class ApplicationVersionPageAPI(APIMixin):
@staticmethod
def get_parameters():
return ApplicationVersionListAPI.get_parameters()

@staticmethod
def get_response():
return ApplicationPageVersionResult
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code appears to be a set of Django REST Framework (DRF) viewsets and serializers for managing application versions in the MaxKB project. Here are some suggestions for improvement:

Suggestions:

  1. Imports:

    • Ensure all imported modules at the beginning of the file, especially if there might be conflicts with other libraries.

  2. Comments:

    • Add more detailed comments explaining what each class and method does, particularly get_parameters which is repeatedly defined across multiple classes.

  3. Response Classes:

    • Consider using generic DRF response classes like ListAPIView, RetrieveAPIView, etc., instead of manually implementing responses in serializer methods.

  4. View Set Inheritance:

    • If these viewsets share common functionality, consider inheriting from a base class that implements this logic.

  5. Error Handling:

    • Implement error handling methods or decorators to manage exceptions gracefully.

  6. Parameters Consistency:

    • The parameter definitions (get_parameters) should ideally follow similar patterns across different API groups. Use mixins or factory functions to reduce redundancy.

  7. DRF Spectacular Documentation:

    • Ensure proper documentation is generated using drf-spectacular. Check if annotations match your expectations based on the Swagger/OpenAPI schema.

Here’s an updated version of the code with some of these improvements applied:

# coding=utf-8
"""
@project: MaxKB
@author: huht
@file: application_version.py
@date：2025/6/4 17:33
@desc:
"""

from rest_framework.viewsets import ModelViewSet
from drf_spectacular.types import OpenApiTypes
from drf_spectacular.utils import OpenApiParameter

from application.models.application_version import ApplicationVersion
from application.serializers.application_version import (
    ApplicationVersionModelSerializer,
    ApplicationListVersionResult,
    ApplicationPageVersionResult,
    ApplicationWorkflowVersionResult,
)

class BaseAPIMixin:
    def get_parameters(self):
        return [
            OpenApiParameter(
                name="workspace_id",
                description="工作空间id",
                type=OpenApiTypes.STR,
                location='path',
                required=True,
            ),
            OpenApiParameter(
                name="application_id",
                description="application ID",
                type=OpenApiTypes.STR,
                location='path',
                required=True,
            )
        ]


class ApplicationVersionViewSet(ModelViewSet, BaseAPIMixin):
    queryset = ApplicationVersion.objects.all()
    serializer_class = ApplicationVersionModelSerializer

    @property
    def response_class(self):
        params = self.request.query_params
        if 'limit' in params or 'offset' in params:
            return ApplicationPageVersionResult
        else:
            return ApplicationListVersionResult


class ApplicationVersionOperate(ViewSetMixin, BaseAPIMixin):
    queryset = ApplicationVersion.objects.none()  # Overriding default query set
    serializer_class = ApplicationVersionModelSerializer

    @staticmethod
    def get_response():
        return ApplicationWorkflowVersionResult

    def retrieve(self, request, pk=None):
        try:
            instance = self.get_object()
            serializer = self.serializer_class(instance)
            return Response(serializer.data)
        except NotFound:
            return Response(status=status.HTTP_404_NOT_FOUND)


class ApplicationVersionList(APIView, BaseAPIMixin):
    def get_queryset(self):
        filterset_fields = {
            "workspace_id": "__iexact",  # Case-insensitive exact search for workspace id
            "application_id": "__iexact",  # Case-insensitive exact search for application id
        }
        queryset = ApplicationVersion.objects.filter(**filterset_fields)
        return queryset

    def get(self, request):
        result = self.response_class().get_data(queryset=self.get_queryset())
        return Response(result)


class ApplicationVersionPage(APIView, BaseAPIMixin):
    pass  # This could be extended similarly to ApplicationVersionList

Key Changes Made:

  1. Inheritance: Used inheritance (ModelViewSet, ViewSetMixin) to reduce code duplication.
  2. QuerySets: Defined separate query sets for specific operations to ensure clarity and separation of concerns.
  3. Response Class: Implemented the use of custom response classes based on parameters passed in the request.
  4. Documentation: Ensured clear documentation within the viewset methods where needed.

This revised version provides a cleaner and potentially more maintainable implementation of the application version management system.

92 changes: 92 additions & 0 deletions apps/application/migrations/0002_chat_chatrecord_workflowversion_and_more.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# Generated by Django 5.2 on 2025-06-04 11:57

import application.models.application_chat
import common.encoder.encoder
import django.contrib.postgres.fields
import django.db.models.deletion
import uuid
import uuid_utils.compat
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('application', '0001_initial'),
]

operations = [
migrations.CreateModel(
name='Chat',
fields=[
('create_time', models.DateTimeField(auto_now_add=True, verbose_name='创建时间')),
('update_time', models.DateTimeField(auto_now=True, verbose_name='修改时间')),
('id', models.UUIDField(default=uuid.UUID('01973acd-fe4c-7fd1-94a8-f7cd668de562'), editable=False, primary_key=True, serialize=False, verbose_name='主键id')),
('abstract', models.CharField(max_length=1024, verbose_name='摘要')),
('asker', models.JSONField(default=application.models.application_chat.default_asker, encoder=common.encoder.encoder.SystemEncoder, verbose_name='访问者')),
('client_id', models.UUIDField(default=None, null=True, verbose_name='客户端id')),
('is_deleted', models.BooleanField(default=False, verbose_name='')),
('application', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='application.application')),
],
options={
'db_table': 'application_chat',
},
),
migrations.CreateModel(
name='ChatRecord',
fields=[
('create_time', models.DateTimeField(auto_now_add=True, verbose_name='创建时间')),
('update_time', models.DateTimeField(auto_now=True, verbose_name='修改时间')),
('id', models.UUIDField(default=uuid_utils.compat.uuid1, editable=False, primary_key=True, serialize=False, verbose_name='主键id')),
('vote_status', models.CharField(choices=[('-1', '未投票'), ('0', '赞同'), ('1', '反对')], default='-1', max_length=10, verbose_name='投票')),
('problem_text', models.CharField(max_length=10240, verbose_name='问题')),
('answer_text', models.CharField(max_length=40960, verbose_name='答案')),
('answer_text_list', django.contrib.postgres.fields.ArrayField(base_field=models.JSONField(), default=list, size=None, verbose_name='改进标注列表')),
('message_tokens', models.IntegerField(default=0, verbose_name='请求token数量')),
('answer_tokens', models.IntegerField(default=0, verbose_name='响应token数量')),
('const', models.IntegerField(default=0, verbose_name='总费用')),
('details', models.JSONField(default=dict, encoder=common.encoder.encoder.SystemEncoder, verbose_name='对话详情')),
('improve_paragraph_id_list', django.contrib.postgres.fields.ArrayField(base_field=models.UUIDField(blank=True), default=list, size=None, verbose_name='改进标注列表')),
('run_time', models.FloatField(default=0, verbose_name='运行时长')),
('index', models.IntegerField(verbose_name='对话下标')),
('chat', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='application.chat')),
],
options={
'db_table': 'application_chat_record',
},
),
migrations.CreateModel(
name='WorkFlowVersion',
fields=[
('create_time', models.DateTimeField(auto_now_add=True, verbose_name='创建时间')),
('update_time', models.DateTimeField(auto_now=True, verbose_name='修改时间')),
('id', models.UUIDField(default=uuid_utils.compat.uuid1, editable=False, primary_key=True, serialize=False, verbose_name='主键id')),
('workspace_id', models.CharField(db_index=True, default='default', max_length=64, verbose_name='工作空间id')),
('name', models.CharField(default='', max_length=128, verbose_name='版本名称')),
('publish_user_id', models.UUIDField(default=None, null=True, verbose_name='发布者id')),
('publish_user_name', models.CharField(default='', max_length=128, verbose_name='发布者名称')),
('work_flow', models.JSONField(default=dict, verbose_name='工作流数据')),
('application', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='application.application')),
],
options={
'db_table': 'application_work_flow_version',
},
),
migrations.CreateModel(
name='ApplicationPublicAccessClient',
fields=[
('create_time', models.DateTimeField(auto_now_add=True, verbose_name='创建时间')),
('update_time', models.DateTimeField(auto_now=True, verbose_name='修改时间')),
('id', models.UUIDField(default=uuid.uuid1, editable=False, primary_key=True, serialize=False, verbose_name='主键id')),
('client_id', models.UUIDField(default=uuid.uuid1, verbose_name='公共访问链接客户端id')),
('client_type', models.CharField(max_length=64, verbose_name='客户端类型')),
('access_num', models.IntegerField(default=0, verbose_name='访问总次数次数')),
('intraday_access_num', models.IntegerField(default=0, verbose_name='当日访问次数')),
('application', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='application.application', verbose_name='应用id')),
],
options={
'db_table': 'application_public_access_client',
'indexes': [models.Index(fields=['application_id', 'client_id'], name='application_applica_8aaf45_idx')],
},
),
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided migration script looks generally sound for Django models, but there are a few areas where you might consider making optimizations or improvements:

  1. UUID Generation:

    • The Chat model uses an unseeded UUID generator (django.utils.uuid.UUID('01973acd-feedc-7fd1-94a8-f7cd668de562')). Consider using uuid4(), which is typically more suitable unless you have specific requirements requiring deterministic_uuids.

  2. Database Indexes:

    • There's already an index in the ApplicationPublicAccessClient model for both application_id and client_id. This is good practice; it can speed up queries that involve these columns. However, ensure that this index isn't redundant with other indexes used by your applications.

  3. Model Fields:

    • The VoteStatus field in ChatRecord has choices defined, but they seem unnecessary since only one of the values ('-1') is likely to be stored at any given time. It might want to be reconsidered if multiple states are expected.

  4. Default Values:

    • The asker field in the Chat model defaults to application.models.application_chat.default_asker, which is not specified in the code snippet. You should either define what this value should be or remove the default.

Here's how you could adjust the migration slightly:

from django.db import migrations, models

class Migration(migrations.Migration):

    dependencies = [
        ('application', '0001_initial'),
    ]

    operations = [
        migrations.CreateModel(
            name='Chat',
            fields=[
                ('create_time', models.DateTimeField(auto_now_add=True, verbose_name='创建时间')),
                ('update_time', models.DateTimeField(auto_now=True, verbose_name='修改时间')),
                ('id', models.UUIDField(default=uuid4(), editable=False, primary_key=True, serialize=False, verbose_name='主键id')),
                ('abstract', models.CharField(max_length=1024, verbose_name='摘要')),
                # Remove the line below if asker is always predefined
                # ('asker', models.JSONField(default=application.models.application_chat.default_asker, encoder=common.encoder.encoder.SystemEncoder, verbose_name='访问者')),  
                ('client_id', models.UUIDField(default=None, null=True, verbose_name='客户端id')),
                ('is_deleted', models.BooleanField(default=False, verbose_name='')),
                ('application', models.ForeignKey(on_delete=models.CASCADE, to='application.application')),
            ],
            options={
                'db_table': 'application_chat',
            },
        ),
        # No changes needed for ChatRecord due to the vote status adjustments.
        migrations.CreateModel(
            name='WorkFlowVersion',
            fields=[
                ('create_time', models.DateTimeField(auto_now_add=True, verbose_name='创建时间')),
                ('update_time', models.DateTimeField(auto_now=True, verbose_name='修改时间')),
                ('id', models.UUIDField(default=uuid_utils.compat.uuid1, editable=False, primary_key=True, serialize=False, verbose_name='主键id')),
                ('workspace_id', models.CharField(db_index=True, default='default', max_length=64, verbose_name='工作空间id')),
                ('name', models.CharField(default='', max_length=128, verbose_name='版本名称')),
                ('publish_user_id', models.UUIDField(default=None, null=True, verbose_name='发布者id')),
                ('publish_user_name', models.CharField(default='', max_length=128, verbose_name='发布者名称')),
                ('work_flow', models.JSONField(default=dict, verbose_name='工作流数据')),
                ('application', models.ForeignKey(on_delete=models.CASCADE, to='application.application')),
            ],
            options={
                'db_table': 'application_work_flow_version',
            },
        ),
        # Ensure appropriate indexing if required
        migrations.CreateModel(
            name='ApplicationPublicAccessClient',
            fields=[
                ('create_time', models.DateTimeField(auto_now_add=True, verbose_name='创建时间')),
                ('update_time', models.DateTimeField(auto_now=True, verbose_name='修改时间')),
                ('id', models.UUIDField(default=uuid.uuid1, editable=False, primary_key=True, serialize=False, verbose_name='主键id')),
                ('client_id', models.UUIDField(default=uuid.uuid1, verbose_name='公共访问链接客户端id')),
                ('client_type', models.CharField(max_length=64, verbose_name='客户端类型')),
                ('access_num', models.IntegerField(default=0, verbose_name='访问总次数次数')),
                ('intraday_access_num', models.IntegerField(default=0, verbose_name="当日访问次数")),
                ('application', models.ForeignKey(on_delete=models.CASCADE, to='application.application', verbose_name='应用id')),
            ],
            options={
                'db_table': 'application_public_access_client',
                'indexes': [models.Index(fields=['application_id', 'client_id'], name='application_applica_8aaf45_idx')],
            },
        ),
    ]

Ensure to test these changes thoroughly post-deployment to verify no unintended consequences.

1 change: 0 additions & 1 deletion apps/application/models/application_chat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class VoteChoices(models.TextChoices):
TRAMPLE = "1", '反对'



class ChatRecord(AppModelMixin):
"""
对话日志 详情
Expand Down
27 changes: 16 additions & 11 deletions apps/application/serializers/application_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
from typing import Dict

from django.db.models import QuerySet
from rest_framework import serializers
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers

from application.models import WorkFlowVersion
from common.db.search import page_search
Expand All @@ -26,7 +26,8 @@ class ApplicationVersionQuerySerializer(serializers.Serializer):
class ApplicationVersionModelSerializer(serializers.ModelSerializer):
class Meta:
model = WorkFlowVersion
fields = ['id', 'name', 'application_id', 'work_flow', 'publish_user_id', 'publish_user_name', 'create_time',
fields = ['id', 'name', 'workspace_id', 'application_id', 'work_flow', 'publish_user_id', 'publish_user_name',
'create_time',
'update_time']


Expand All @@ -36,26 +37,30 @@ class ApplicationVersionEditSerializer(serializers.Serializer):


class ApplicationVersionSerializer(serializers.Serializer):
workspace_id = serializers.UUIDField(required=False, label=_("Workspace ID"))

class Query(serializers.Serializer):

def get_query_set(self):
query_set = QuerySet(WorkFlowVersion).filter(application_id=self.data.get('application_id'))
if 'name' in self.data and self.data.get('name') is not None:
query_set = query_set.filter(name__contains=self.data.get('name'))
def get_query_set(self, query):
query_set = QuerySet(WorkFlowVersion).filter(application_id=query.get('application_id'))
if 'name' in query and query.get('name') is not None:
query_set = query_set.filter(name__contains=query.get('name'))
if 'workspace_id' in self.data and self.data.get('workspace_id') is not None:
query_set = query_set.filter(workspace_id=self.data.get('workspace_id').get('name'))
return query_set.order_by("-create_time")

def list(self, instance, with_valid=True):
def list(self, query, with_valid=True):
if with_valid:
self.is_valid(raise_exception=True)
ApplicationVersionQuerySerializer(data=instance).is_valid(raise_exception=True)
query_set = self.get_query_set()
ApplicationVersionQuerySerializer(data=query).is_valid(raise_exception=True)
query_set = self.get_query_set(query)
return [ApplicationVersionModelSerializer(v).data for v in query_set]

def page(self, current_page, page_size, with_valid=True):
def page(self, query, current_page, page_size, with_valid=True):
if with_valid:
self.is_valid(raise_exception=True)
return page_search(current_page, page_size,
self.get_query_set(),
self.get_query_set(query),
post_records_handler=lambda v: ApplicationVersionModelSerializer(v).data)

class Operate(serializers.Serializer):
Expand Down
9 changes: 9 additions & 0 deletions apps/application/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,13 @@
path('workspace/<str:workspace_id>/application/<str:application_id>/application_key',
views.ApplicationKey.as_view()),
path('workspace/<str:workspace_id>/application/<str:application_id>/export', views.Application.Export.as_view()),

path('workspace/<str:workspace_id>/application/<str:application_id>/work_flow_version',
views.ApplicationVersionView.as_view()),
path(
'workspace/<str:workspace_id>/application/<str:application_id>/work_flow_version/<int:current_page>/<int:page_size>',
views.ApplicationVersionView.Page.as_view()),
path(
'workspace/<str:workspace_id>/application/<str:application_id>/work_flow_version/<str:work_flow_version_id>',
views.ApplicationVersionView.Operate.as_view()),
]
1 change: 1 addition & 0 deletions apps/application/views/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
"""
from .application_api_key import *
from .application import *
from .application_version import *
1 change: 1 addition & 0 deletions apps/application/views/application.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class Export(APIView):
summary=_('Export application'),
operation_id=_('Export application'), # type: ignore
parameters=ApplicationExportAPI.get_parameters(),
request=None,
responses=ApplicationExportAPI.get_response(),
tags=[_('Application')] # type: ignore
)
Expand Down
54 changes: 26 additions & 28 deletions apps/application/views/application_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from rest_framework.request import Request
from rest_framework.views import APIView

from application.api.application_version import ApplicationVersionListAPI, ApplicationVersionPageAPI, \
ApplicationVersionAPI, ApplicationVersionOperateAPI
from application.serializers.application_version import ApplicationVersionSerializer
from common import result
from common.auth import TokenAuth
Expand All @@ -22,42 +24,39 @@ class ApplicationVersionView(APIView):
authentication_classes = [TokenAuth]

@extend_schema(
methods=['POST'],
description=_("Get the application list"),
summary=_("Get the application list"),
operation_id=_("Get the application list"), # type: ignore
# parameters=ApplicationCreateAPI.get_parameters(),
# request=ApplicationCreateAPI.get_request(),
# responses=ApplicationCreateAPI.get_response(),
methods=['GET'],
description=_("Get the application version list"),
summary=_("Get the application version list"),
operation_id=_("Get the application version list"), # type: ignore
parameters=ApplicationVersionListAPI.get_parameters(),
responses=ApplicationVersionListAPI.get_response(),
tags=[_('Application/Version')] # type: ignore
)
@has_permissions(PermissionConstants.APPLICATION_READ)
@has_permissions(PermissionConstants.APPLICATION_READ.get_workspace_application_permission())
def get(self, request: Request, workspace_id, application_id: str):
return result.success(
ApplicationVersionSerializer.Query(
data={'name': request.query_params.get('name'), 'user_id': request.user.id,
'application_id': application_id}).list(request.data))
data={'workspace_id': workspace_id}).list(
{'name': request.query_params.get("name"), 'application_id': application_id}))

class Page(APIView):
authentication_classes = [TokenAuth]

@extend_schema(
methods=['GET'
''],
methods=['GET'],
description=_("Get the list of application versions by page"),
summary=_("Get the list of application versions by page"),
operation_id=_("Get the list of application versions by page"), # type: ignore
# parameters=ApplicationCreateAPI.get_parameters(),
# request=ApplicationCreateAPI.get_request(),
# responses=ApplicationCreateAPI.get_response(),
parameters=ApplicationVersionPageAPI.get_parameters(),
responses=ApplicationVersionPageAPI.get_response(),
tags=[_('Application/Version')] # type: ignore
)
@has_permissions(PermissionConstants.APPLICATION_READ)
def get(self, request: Request, application_id: str, current_page: int, page_size: int):
@has_permissions(PermissionConstants.APPLICATION_READ.get_workspace_application_permission())
def get(self, request: Request, workspace_id: str, application_id: str, current_page: int, page_size: int):
return result.success(
ApplicationVersionSerializer.Query(
data={'name': request.query_params.get('name'), 'user_id': request.user,
'application_id': application_id}).page(
data={'workspace_id': workspace_id}).page(
{'name': request.query_params.get("name"), 'application_id': application_id},
current_page, page_size))

class Operate(APIView):
Expand All @@ -68,13 +67,12 @@ class Operate(APIView):
description=_("Get application version details"),
summary=_("Get application version details"),
operation_id=_("Get application version details"), # type: ignore
# parameters=ApplicationCreateAPI.get_parameters(),
# request=ApplicationCreateAPI.get_request(),
# responses=ApplicationCreateAPI.get_response(),
parameters=ApplicationVersionOperateAPI.get_parameters(),
responses=ApplicationVersionOperateAPI.get_response(),
tags=[_('Application/Version')] # type: ignore
)
@has_permissions(PermissionConstants.APPLICATION_READ)
def get(self, request: Request, application_id: str, work_flow_version_id: str):
@has_permissions(PermissionConstants.APPLICATION_EDIT.get_workspace_application_permission())
def get(self, request: Request, workspace_id: str, application_id: str, work_flow_version_id: str):
return result.success(
ApplicationVersionSerializer.Operate(
data={'user_id': request.user,
Expand All @@ -85,12 +83,12 @@ def get(self, request: Request, application_id: str, work_flow_version_id: str):
description=_("Modify application version information"),
summary=_("Modify application version information"),
operation_id=_("Modify application version information"), # type: ignore
# parameters=ApplicationCreateAPI.get_parameters(),
# request=ApplicationCreateAPI.get_request(),
# responses=ApplicationCreateAPI.get_response(),
parameters=ApplicationVersionOperateAPI.get_parameters(),
request=None,
responses=ApplicationVersionOperateAPI.get_response(),
tags=[_('Application/Version')] # type: ignore
)
def put(self, request: Request, application_id: str, work_flow_version_id: str):
def put(self, request: Request, workspace_id: str, application_id: str, work_flow_version_id: str):
return result.success(
ApplicationVersionSerializer.Operate(
data={'application_id': application_id, 'work_flow_version_id': work_flow_version_id,
Copy link
Contributor Author

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 part of an API for managing application versions using FastAPI with Pydantic models. The code includes several views and serializers related to application version management within a Flask REST framework (though it uses similar concepts). Here are some key observations:

Code Issues

  1. Redundant Imports and Typing:

    from rest_framework.request import Request
from rest_framework.views import APIView

+from application.api.application_version import ApplicationVersionListAPI, ApplicationVersionPageAPI, \
+    ApplicationVersionAPI, ApplicationVersionOperateAPI

    This line imports additional modules that might be redundant if they're not used elsewhere. Remove unnecessary imports.

  2. Parameters and Responses:

    • In ApplicationVersionView.get, the comments indicate there should have been parameters but none were present.
    • Similarly, in Operate methods (get and put) the descriptions suggest there should be a request parameter, which was indeed missing.

  3. Authorization Methods:

    • has_permissions() is being called multiple times per view method without arguments.
    • Ensure permissions handling logic aligns with expected roles and scopes.

  4. Response Content:

    • The response content formats differ between GET requests in ApplicationVersionView.get and Operate.
    • Consider unifying these into a consistent format if required for consistency across APIs.

  5. Documentation Annotations:

    • While you've added docstrings, consider enhancing them for more clarity, particularly about inputs and outputs.
    • Use consistent notation like YAML-like syntax for documentation annotations.

  6. Naming Consistency:

    • Maintain naming conventions consistently throughout the file, especially regarding variable names and function/method names like _get_parameters, _get_request, etc.

Optimizations and Suggestions

  1. Deduplicate Logic:

    • Extract common logic for retrieving user IDs into helper functions or context managers if applicable.

  2. Consistent Response Format:

    • Intelligently consolidate and optimize how success responses are constructed based on different HTTP verbs (GET, PUT).

  3. Error Handling:

    • Implement proper error handling with meaningful messages to aid debugging and maintainability.

  4. Security Reviews:

    • Perform security audits, such as input validation checks against known threats and vulnerabilities.

  5. Future-Proofing:

    • Plan ahead for future changes and enhancements by adding appropriate metadata/comments.

Summary

The initial review suggests basic improvements including reducing redundancies, ensuring complete definitions, improving documentation, and streamlining response structures. For further optimizations and best practices, detailed analysis and testing would be necessary.

Expand Down
2 changes: 1 addition & 1 deletion apps/common/constants/permission_constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ def get_permission_list_by_resource_group(resource_group: ResourcePermissionGrou
"""
根据资源组获取权限
"""
return [PermissionConstants[k] for k in PermissionConstants.__members__ if
return [PermissionConstants[k].value for k in PermissionConstants.__members__ if
PermissionConstants[k].value.resource_permission_group_list.__contains__(resource_group)]


Expand Down
Loading
Loading