-
Notifications
You must be signed in to change notification settings - Fork 2.6k
refactor: move chat share endpoints to chat API routes and remove workspace_id from share-link flow #4817
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
refactor: move chat share endpoints to chat API routes and remove workspace_id from share-link flow #4817
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 |
|---|---|---|
|
|
@@ -30,9 +30,8 @@ class ChatRecordLinkView(APIView): | |
| tags=[_("Chat record link")] # type: ignore | ||
| ) | ||
|
|
||
| def post(self, request: Request, workspace_id: str, application_id: str, chat_id: str): | ||
| def post(self, request: Request, application_id: str, chat_id: str): | ||
| return result.success(ChatRecordShareLinkSerializer(data={ | ||
| "workspace_id": workspace_id, | ||
| "application_id": application_id, | ||
| "chat_id": chat_id, | ||
| "user_id": request.auth.chat_user_id | ||
|
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. The given code has a few potential issues that need to be addressed:
Here's an optimized version of the code with these improvements: class ChatRecordLinkView(APIView):
tags = [_("Chat record link")]
def post(self, request: Request, application_id: str, chat_id: str) -> Response:
return result.success(
{
"application_id": application_id,
"chat_id": chat_id,
"user_id": request.auth.chat_user_id,
}
)Explanation:
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| from django.urls import path | ||
|
|
||
| from application.views import ChatRecordDetailView, ChatRecordLinkView | ||
| from chat.views.mcp import mcp_view | ||
| from . import views | ||
|
|
||
|
|
@@ -24,5 +25,7 @@ | |
| path('historical_conversation/clear',views.HistoricalConversationView.BatchDelete.as_view(), name='historical_conversation_clear'), | ||
| path('historical_conversation/<str:chat_id>',views.HistoricalConversationView.Operate.as_view(), name='historical_conversation_operate'), | ||
| path('historical_conversation_record/<str:chat_id>', views.HistoricalConversationRecordView.as_view(), name='historical_conversation_record'), | ||
| path('historical_conversation_record/<str:chat_id>/<int:current_page>/<int:page_size>', views.HistoricalConversationRecordView.PageView.as_view(), name='historical_conversation_record') | ||
| path('historical_conversation_record/<str:chat_id>/<int:current_page>/<int:page_size>', views.HistoricalConversationRecordView.PageView.as_view(), name='historical_conversation_record'), | ||
| path('share/<str:link>', ChatRecordDetailView.as_view()), | ||
| path('<str:application_id>/chat/<str:chat_id>/share_chat', ChatRecordLinkView.as_view()), | ||
| ] | ||
|
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. Here are some recommendations and optimizations:
Here’s an optimized version with improved naming conventions and clarity: from django.urls import include, re_path
app_name = 'history'
urlpatterns = [
# Other existing paths here...
# Simplified historical conversation record endpoint
path('historical_conversations/<str:chat_id>',
views.HistoricalConversationListView.as_view(),
name='historical_conversations'),
# Add documentation comments
"""
Path for displaying detailed information about a chat.
Args:
chat_id (str): The unique identifier for the chat.
Returns:
JsonResponse: Detailed view of the chat record.
"""
path("share/<int:pk>/", ChatRecordDetailView.as_view(), name="shared-chat-detail"),
"""
Endpoint for creating links for sharing chats.
Args:
application_id (str): ID of the application associated with the chat.
chat_id (str): Unique identifier of the chat being shared.
Returns:
HttpResponse: Redirect response with link creation success status messages.
"""
path("<str:application_id>/chat/<str:chat_id>/create-link/",
create_share_link, # Assuming this function exists elsewhere
name="create-share-link"),
] |
||
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.
The provided code snippet is mostly correct but contains a redundant section regarding the
workspace_idparameter. Since the parameter is defined within the methodget request(), which suggests it's intended to be dynamic and not static, removing this section should resolve any redundancy. Here are the changes:No further optimization suggestions are needed based on the information provided. If you plan to include more parameters dynamically in future versions of the function, make sure they are added here as well.