Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 11 additions & 2 deletions apps/chat/views/v2/chat.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
from common.constants.cache_version import Cache_Version
from common.exception.app_exception import AppAuthenticationFailed, AppApiException
from common.log.log import _get_ip_address, log
from common.result import DefaultResultSerializer, result
from common.result import DefaultResultSerializer, ResultStringSerializer, result
from common.utils.rsa_util import decrypt
from knowledge.models import FileSourceType
from maxkb.const import CONFIG
Expand Down Expand Up @@ -188,7 +188,16 @@ class AuthProfile(APIView):
summary=_("Get application authentication information"),
operation_id=_("Get application authentication information"), # type: ignore
parameters=ChatAuthenticationProfileAPIV2.get_parameters(),
responses=None,
responses={
200: OpenApiResponse(
response=DefaultResultSerializer,
description=_("Success"),
),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("access_token is required"),
),
},
tags=[_("Chat")], # type: ignore
)
def get(self, request: Request):
Expand Down
6 changes: 3 additions & 3 deletions apps/chat/views/v2/chat_record.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
)
from common import result
from common.auth import ChatTokenAuth
from common.result import DefaultResultSerializer
from common.result import ResultStringSerializer


class VoteView(APIView):
Expand Down Expand Up @@ -69,7 +69,7 @@ class HistoricalConversationView(APIView):
responses={
200: HistoricalConversationAPI.get_response(),
401: OpenApiResponse(
response=DefaultResultSerializer,
response=ResultStringSerializer,
description=_("Authentication required"),
),
},
Expand Down Expand Up @@ -163,7 +163,7 @@ class PageView(APIView):
responses={
200: PageHistoricalConversationAPI.get_response(),
401: OpenApiResponse(
response=DefaultResultSerializer,
response=ResultStringSerializer,
description=_("Authentication required"),
),
},
Expand Down
15 changes: 12 additions & 3 deletions apps/chat/views/v3/chat.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
from common.constants.cache_version import Cache_Version
from common.exception.app_exception import AppApiException
from common.log.log import _get_ip_address, log, record_log
from common.result import DefaultResultSerializer, result
from common.result import DefaultResultSerializer, ResultStringSerializer, result
from common.utils.rsa_util import decrypt
from knowledge.models import FileSourceType
from maxkb.const import CONFIG
Expand Down Expand Up @@ -194,7 +194,16 @@ class ChatAuthProfile(APIView):
summary=_("Get application authentication information"),
operation_id=_("V3 Get application authentication information"), # type: ignore
parameters=ChatAuthenticationProfileAPI.get_parameters(),
responses=None,
responses={
200: OpenApiResponse(
response=DefaultResultSerializer,
description=_("Success"),
),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("access_token is required"),
),
},
tags=[_("V3 Chat")], # type: ignore
)
def get(self, request: Request):
Expand Down Expand Up @@ -472,7 +481,7 @@ class ChatUserProfileView(APIView):
responses={
200: UserProfileAPI.get_response(),
401: OpenApiResponse(
response=DefaultResultSerializer,
response=ResultStringSerializer,
description=_("Authentication required"),
),
},
Expand Down
10 changes: 10 additions & 0 deletions apps/common/result/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ class DefaultResultSerializer(serializers.Serializer):
data = serializers.BooleanField(required=False, default=True)


class ResultStringSerializer(serializers.Serializer):
code = serializers.IntegerField(required=True, help_text=_("response code"), label=_("response code"))
message = serializers.CharField(
required=False, default="success", help_text=_("error prompt"), label=_("error prompt")
)
data = serializers.CharField(
required=False, allow_null=True, help_text=_("response data"), label=_("response data")
)


class ResultSerializer(serializers.Serializer):
"""
响应结果
Expand Down
51 changes: 44 additions & 7 deletions apps/homepage/views/homepage.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
@desc:
"""

from drf_spectacular.utils import extend_schema
from drf_spectacular.utils import OpenApiResponse, extend_schema
from rest_framework.request import Request
from rest_framework.views import APIView

from common import result
from common.result import ResultStringSerializer
from common.auth import TokenAuth
from common.auth.authentication import has_permissions
from common.auth.constants.permission_constants import PermissionConstants
Expand Down Expand Up @@ -44,7 +45,13 @@ class ChatRecordAggregation(APIView):
summary=_("Chat record aggregation"),
operation_id="homepage_chat_count_aggregation",
parameters=TokensAggregationAPI.get_parameters(),
responses=TokensAggregationAPI.get_response(),
responses={
200: TokensAggregationAPI.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("Invalid or missing time range parameters"),
),
},
tags=[_("Home page")], # type: ignore
)
@has_permissions(
Expand Down Expand Up @@ -73,7 +80,13 @@ class TokensAggregation(APIView):
summary=_("Tokens data aggregation"),
operation_id="homepage_tokens_aggregation",
parameters=TokensAggregationAPI.get_parameters(),
responses=TokensAggregationAPI.get_response(),
responses={
200: TokensAggregationAPI.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("Invalid or missing time range parameters"),
),
},
tags=[_("Home page")], # type: ignore
)
@has_permissions(
Expand Down Expand Up @@ -102,7 +115,13 @@ class ApplicationTokensRankingExport(APIView):
summary=_("Top applications by token consumption export"),
operation_id="homepage_application_tokens_ranking_export",
parameters=RankingBaseExportAPI.get_parameters(),
responses=RankingBaseExportAPI.get_response(),
responses={
200: RankingBaseExportAPI.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("Invalid or missing time range parameters"),
),
},
tags=[_("Home page")], # type: ignore
)
@has_permissions(
Expand Down Expand Up @@ -160,7 +179,13 @@ class ApplicationQuestionRankingExport(APIView):
summary=_("Top applications by question count export"),
operation_id="homepage_application_question_ranking_export",
parameters=RankingBaseExportAPI.get_parameters(),
responses=RankingBaseExportAPI.get_response(),
responses={
200: RankingBaseExportAPI.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("Invalid or missing time range parameters"),
),
},
tags=[_("Home page")], # type: ignore
)
@has_permissions(
Expand Down Expand Up @@ -218,7 +243,13 @@ class UserTokensRankingExport(APIView):
summary=_("Top users by token consumption export"),
operation_id="homepage_user_tokens_ranking_export",
parameters=RankingBaseExportAPI.get_parameters(),
responses=RankingBaseExportAPI.get_response(),
responses={
200: RankingBaseExportAPI.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("Invalid or missing time range parameters"),
),
},
tags=[_("Home page")], # type: ignore
)
@has_permissions(
Expand Down Expand Up @@ -276,7 +307,13 @@ class ApplicationMonitoring(APIView):
summary=_("Dialogue-related statistical trends"),
operation_id="Dialogue-related statistical trends", # type: ignore
parameters=ApplicationMonitoringAPI.get_parameters(),
responses=ApplicationMonitoringAPI.get_response(),
responses={
200: ApplicationMonitoringAPI.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("Invalid or missing time range parameters"),
),
},
tags=[_("Home page")], # type: ignore
)
@has_permissions(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Overall, this is a schema/documentation-only change and does not alter runtime behavior. I do not see a clear functional bug, but there are a few schema-correctness and maintainability points to verify.

Potential issues

  1. Verify the return type of get_response()

    • The new code assumes TokensAggregationAPI.get_response() returns a serializer class or OpenApiResponse, because it is now nested under 200.
    • If get_response() already returns a responses mapping/dict, then {200: <dict>} is likely invalid or produces an incorrect schema.
    • If it returns a serializer class, this change is correct.
  2. Make sure the 400 response body matches ResultStringSerializer

    • DRF’s default ValidationError response is usually {"field": ["error"]} or {"detail": "..."}, not a custom result envelope.
    • If the project has a custom exception handler that wraps errors in ResultStringSerializer, this is correct.
    • Otherwise, the documented 400 schema will not match the actual response.
  3. The 400 description may be too specific

    • "Invalid or missing time range parameters" only covers time-range errors.
    • These endpoints may also return 400 for invalid pagination, filters, or other query parameters.
    • Consider a more general description such as "Invalid query parameters", or keep endpoint-specific wording only if the validation really is time-range-only.
  4. Authentication/permission errors are not documented

    • These views use TokenAuth and has_permissions.
    • If you are adding explicit error documentation, consider whether 401 and/or 403 should also be documented.
    • This may already be handled globally by drf-spectacular, but it is worth confirming.
  5. Duplication of the same 400 response definition

    • The same OpenApiResponse block is repeated six times.
    • This is not a bug, but it increases the chance of inconsistency later.
    • Consider a shared constant/helper, for example:
    COMMON_400_RESPONSE = OpenApiResponse(
        response=ResultStringSerializer,
        description=_("Invalid query parameters"),
    )
    
    def common_responses(success_response):
        return {
            200: success_response,
            400: COMMON_400_RESPONSE,
        }

    Then use:

    responses=common_responses(TokensAggregationAPI.get_response())
  6. Import style is acceptable but slightly redundant

    • from common import result and from common.result import ResultStringSerializer are fine.
    • No action is required unless the project has stricter import grouping rules.
  7. Pre-existing issue: operation ID with spaces

    • In ApplicationMonitoring, the existing operation_id="Dialogue-related statistical trends" contains spaces.
    • OpenAPI operation IDs should preferably be stable, machine-friendly identifiers, e.g. homepage_dialogue_statistical_trends.
    • This is not caused by the diff, but it is an existing schema irregularity.

Recommendation

The change is reasonable if:

  • get_response() returns a serializer class or OpenApiResponse, not a response dict.
  • The actual 400 payload is indeed ResultStringSerializer.
  • The 400 description is accurate for all validation failures on these endpoints.

Otherwise, adjust the schema to match the real exception-handling and response structure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Code Review

Issues

  1. Repeated code (DRY violation): The same OpenApiResponse(response=ResultStringSerializer, description=_("Invalid or missing time range parameters")) block is duplicated 6 times. Extract it once:

    INVALID_TIME_RANGE_RESPONSE = OpenApiResponse(
        response=ResultStringSerializer,
        description=_("Invalid or missing time range parameters"),
    )

    Then use responses={200: ..., 400: INVALID_TIME_RANGE_RESPONSE}.

  2. operation_id inconsistency: ApplicationMonitoring uses "Dialogue-related statistical trends" — an English human-readable string with spaces/uppercase as an operation_id. This breaks the typical snake_case convention and may cause invalid OpenAPI operationId values. Use e.g. homepage_application_monitoring.

  3. Import style: from common import result plus from common.result import ResultStringSerializer — if only ResultStringSerializer is needed and result module import is unused elsewhere in this diff, prefer a single consistent import.

Suggestions

  1. Verify accuracy of the 400 doc: Make sure these endpoints actually return 400 (not 200-wrapped error bodies) for invalid time ranges; otherwise the schema will mislead clients. If errors are returned via your Result wrapper with HTTP 200, document that instead.

  2. Consider reusable serializer-level responses: If most views follow the {200: ..., 400: ...} pattern, consider defining a small helper or mixin to build the responses dict, reducing boilerplate further.

  3. Minor: keep # type: ignore comments justified; if tags lists are valid, they may be removable.

Overall: functionally correct, but refactor the duplication and fix the operation_id naming.

Expand Down
2 changes: 1 addition & 1 deletion apps/models_provider/api/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class ModelDetailSerializer(serializers.Serializer):
status = serializers.CharField(required=True, label=_("status"))
meta = serializers.DictField(required=False, allow_null=True, label=_("model meta"))
credential = serializers.DictField(required=False, label=_("certification information"))
workspace_id = serializers.CharField(required=True, label=_("workspace id"))
workspace_id = serializers.CharField(required=False, label=_("workspace id"))
nick_name = serializers.CharField(required=False, allow_blank=True, label=_("nick name"))
username = serializers.CharField(required=False, allow_blank=True, label=_("username"))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@
from models_provider.impl.local_model_provider.model.reranker import LocalReranker

embedding_text2vec_base_chinese = ModelInfo(
"shibing624/text2vec-base-chinese", "", ModelTypeConst.EMBEDDING, LocalEmbeddingCredential(), LocalEmbedding
"/opt/maxkb-app/model/embedding/shibing624_text2vec-base-chinese",
"",
ModelTypeConst.EMBEDDING,
LocalEmbeddingCredential(),
LocalEmbedding,
)
bge_reranker_v2_m3 = ModelInfo(
"BAAI/bge-reranker-v2-m3", "", ModelTypeConst.RERANKER, LocalRerankerCredential(), LocalReranker
Expand Down
35 changes: 30 additions & 5 deletions apps/models_provider/views/provide.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@


from common import result
from common.result import ResultStringSerializer
from common.auth import TokenAuth
from django.utils.translation import gettext_lazy as _
from drf_spectacular.utils import extend_schema
from drf_spectacular.utils import OpenApiResponse, extend_schema
from models_provider.api.provide import ProvideApi
from models_provider.constants.model_provider_constants import ModelProvideConstants
from models_provider.serializers.model_serializer import get_default_model_params_setting
Expand Down Expand Up @@ -70,7 +71,13 @@ class ModelTypeList(APIView):
description=_("Get a list of model types"),
operation_id=_("Get a list of model types"), # type: ignore
parameters=ProvideApi.ModelTypeList.get_query_params_api(),
responses=ProvideApi.ModelTypeList.get_response(),
responses={
200: ProvideApi.ModelTypeList.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("provider is required"),
),
},
tags=[_("Model")], # type: ignore
)
def get(self, request: Request):
Expand All @@ -86,7 +93,13 @@ class ModelList(APIView):
description=_("Example of obtaining model list"),
operation_id=_("Example of obtaining model list"), # type: ignore
parameters=ProvideApi.ModelList.get_query_params_api(),
responses=ProvideApi.ModelList.get_response(),
responses={
200: ProvideApi.ModelList.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("provider is required"),
),
},
tags=[_("Model")], # type: ignore
)
@staticmethod
Expand Down Expand Up @@ -181,7 +194,13 @@ class ModelParamsForm(APIView):
description=_("Get model default parameters"),
operation_id=_("Get model default parameters"), # type: ignore
parameters=ProvideApi.ModelParamsForm.get_query_params_api(),
responses=ProvideApi.ModelParamsForm.get_response(),
responses={
200: ProvideApi.ModelParamsForm.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("provider is required"),
),
},
tags=[_("Model")], # type: ignore
)
def get(self, request: Request):
Expand All @@ -202,7 +221,13 @@ class ModelForm(APIView):
description=_("Get the model creation form"),
operation_id=_("Get the model creation form"), # type: ignore
parameters=ProvideApi.ModelParamsForm.get_query_params_api(),
responses=ProvideApi.ModelParamsForm.get_response(),
responses={
200: ProvideApi.ModelParamsForm.get_response(),
400: OpenApiResponse(
response=ResultStringSerializer,
description=_("provider is required"),
),
},
tags=[_("Model")], # type: ignore
)
def get(self, request: Request):
Expand Down
71 changes: 68 additions & 3 deletions apps/system_manage/api/chat_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from drf_spectacular.utils import OpenApiParameter

from common.mixins.api_mixin import APIMixin
from common.result import ResultSerializer
from common.result import ResultPageSerializer, ResultSerializer
from users.serializers.user import CreateUserSerializer
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers
Expand Down Expand Up @@ -101,16 +101,81 @@ class ChatUserListResponse(serializers.Serializer):
id = serializers.CharField(required=True, label=_("ID"))
username = serializers.CharField(required=True, label=_("Username"))
nick_name = serializers.CharField(required=True, label=_("Nickname"))
email = serializers.EmailField(required=False, allow_blank=True, label=_("Email"))
phone = serializers.CharField(required=False, allow_blank=True, label=_("Phone"))
email = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Email"))
phone = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Phone"))
is_active = serializers.BooleanField(required=False, default=True, label=_("Is Active"))
create_time = serializers.DateTimeField(required=False, label=_("Create Time"))
update_time = serializers.DateTimeField(required=False, label=_("Update Time"))
source = serializers.CharField(required=False, allow_blank=True, label=_("Source"))


class ChatUsersListResponse(ResultSerializer):
def get_data(self):
return ChatUserListResponse(many=True)


class ChatUserDetailSerializer(serializers.Serializer):
id = serializers.CharField(required=True, label=_("ID"))
username = serializers.CharField(required=True, label=_("Username"))
nick_name = serializers.CharField(required=True, label=_("Nickname"))
email = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Email"))
phone = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Phone"))
is_active = serializers.BooleanField(required=False, default=True, label=_("Is Active"))
create_time = serializers.DateTimeField(required=False, label=_("Create Time"))
update_time = serializers.DateTimeField(required=False, label=_("Update Time"))
source = serializers.CharField(required=False, allow_blank=True, label=_("Source"))
user_group_ids = serializers.ListField(
required=False, child=serializers.CharField(required=False), label=_("User Group IDs")
)


class ChatUserDetailResponse(ResultSerializer):
def get_data(self):
return ChatUserDetailSerializer()


class ChatUserPageUserListSerializer(serializers.Serializer):
id = serializers.CharField(required=True, label=_("ID"))
username = serializers.CharField(required=True, label=_("Username"))
nick_name = serializers.CharField(required=True, label=_("Nickname"))
email = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Email"))
phone = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Phone"))
is_active = serializers.BooleanField(required=False, default=True, label=_("Is Active"))
create_time = serializers.DateTimeField(required=False, label=_("Create Time"))
update_time = serializers.DateTimeField(required=False, label=_("Update Time"))
source = serializers.CharField(required=False, allow_blank=True, label=_("Source"))
user_group_ids = serializers.ListField(
required=False, child=serializers.CharField(required=False), label=_("User Group IDs")
)
user_group_names = serializers.ListField(
required=False, child=serializers.CharField(required=False), label=_("User Group Names")
)
token_quota = serializers.DictField(required=False, allow_null=True, label=_("Token Quota"))


class ChatUserPageUserListResponse(ResultPageSerializer):
def get_data(self):
return ChatUserPageUserListSerializer(many=True)


class ChatUserGroupPageUserListSerializer(serializers.Serializer):
id = serializers.CharField(required=True, label=_("ID"))
username = serializers.CharField(required=True, label=_("Username"))
nick_name = serializers.CharField(required=True, label=_("Nickname"))
email = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Email"))
phone = serializers.CharField(required=False, allow_blank=True, allow_null=True, label=_("Phone"))
is_active = serializers.BooleanField(required=False, default=True, label=_("Is Active"))
create_time = serializers.DateTimeField(required=False, label=_("Create Time"))
update_time = serializers.DateTimeField(required=False, label=_("Update Time"))
source = serializers.CharField(required=False, allow_blank=True, label=_("Source"))
user_group_relation_id = serializers.CharField(required=False, label=_("User Group Relation ID"))


class ChatUserGroupPageUserListResponse(ResultPageSerializer):
def get_data(self):
return ChatUserGroupPageUserListSerializer(many=True)


class ChatUserPageApi(APIMixin):
@staticmethod
def get_parameters():
Expand Down
Loading
Loading