Repository navigation
chore: update community identifiers and enhance serializers for user and tool APIs #7716
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -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( | ||
|
|
@@ -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( | ||
|
|
@@ -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( | ||
|
|
@@ -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( | ||
|
|
@@ -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( | ||
|
|
@@ -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( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code ReviewIssues
Suggestions
Overall: functionally correct, but refactor the duplication and fix the |
||
|
|
||
There was a problem hiding this comment.
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
Verify the return type of
get_response()TokensAggregationAPI.get_response()returns a serializer class orOpenApiResponse, because it is now nested under200.get_response()already returns aresponsesmapping/dict, then{200: <dict>}is likely invalid or produces an incorrect schema.Make sure the 400 response body matches
ResultStringSerializerValidationErrorresponse is usually{"field": ["error"]}or{"detail": "..."}, not a customresultenvelope.ResultStringSerializer, this is correct.The 400 description may be too specific
"Invalid or missing time range parameters"only covers time-range errors."Invalid query parameters", or keep endpoint-specific wording only if the validation really is time-range-only.Authentication/permission errors are not documented
TokenAuthandhas_permissions.401and/or403should also be documented.Duplication of the same 400 response definition
OpenApiResponseblock is repeated six times.Then use:
Import style is acceptable but slightly redundant
from common import resultandfrom common.result import ResultStringSerializerare fine.Pre-existing issue: operation ID with spaces
ApplicationMonitoring, the existingoperation_id="Dialogue-related statistical trends"contains spaces.homepage_dialogue_statistical_trends.Recommendation
The change is reasonable if:
get_response()returns a serializer class orOpenApiResponse, not a response dict.ResultStringSerializer.Otherwise, adjust the schema to match the real exception-handling and response structure.