-
Notifications
You must be signed in to change notification settings - Fork 610
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
重构webui适配 #1801
重构webui适配 #1801
Conversation
Sourcery 审查指南此拉取请求重构了 WebUI 以提高兼容性。 ApiDataSource 和相关模型的类图classDiagram
class ApiDataSource {
+get_base_info(bot_id: str) list[BaseInfo]
+get_all_chat_count(bot_id: str) QueryCount
+get_all_call_count(bot_id: str) QueryCount
+get_active_group(date_type: QueryDateType, bot_id: str) list[ActiveGroup]
+get_hot_plugin(date_type: QueryDateType, bot_id: str) list[HotPlugin]
+get_bot_block_module(bot_id: str) BotBlockModule
-__build_bot_info(bot: Bot) TemplateBaseInfo
-__get_bot_version() str
-__init_bot_base_data(select_bot: TemplateBaseInfo)
-__get_query(base_query, date_type, bot_id)
}
class PlatformUtils {
+get_platform(bot: Bot) str
+get_group_list(bot: Bot) tuple[list, str]
+get_friend_list(bot: Bot) tuple[list, str]
+send_message(bot: Bot, user_id: str, group_id: str, message: str)
}
class BaseInfo {
+self_id: str
+nickname: str
+ava_url: str
+received_messages: int
+group_count: int
+friend_count: int
+day_call: int
+version: str
+connect_time: int
+connect_date: str
+connect_count: int
+status: bool
+is_select: bool
}
ApiDataSource ..> PlatformUtils : uses
ApiDataSource ..> BaseInfo : creates
文件级别更改
提示和命令与 Sourcery 互动
自定义您的体验访问您的仪表板以:
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request refactors the WebUI for better compatibility. Class diagram for ApiDataSource and related modelsclassDiagram
class ApiDataSource {
+get_base_info(bot_id: str) list[BaseInfo]
+get_all_chat_count(bot_id: str) QueryCount
+get_all_call_count(bot_id: str) QueryCount
+get_active_group(date_type: QueryDateType, bot_id: str) list[ActiveGroup]
+get_hot_plugin(date_type: QueryDateType, bot_id: str) list[HotPlugin]
+get_bot_block_module(bot_id: str) BotBlockModule
-__build_bot_info(bot: Bot) TemplateBaseInfo
-__get_bot_version() str
-__init_bot_base_data(select_bot: TemplateBaseInfo)
-__get_query(base_query, date_type, bot_id)
}
class PlatformUtils {
+get_platform(bot: Bot) str
+get_group_list(bot: Bot) tuple[list, str]
+get_friend_list(bot: Bot) tuple[list, str]
+send_message(bot: Bot, user_id: str, group_id: str, message: str)
}
class BaseInfo {
+self_id: str
+nickname: str
+ava_url: str
+received_messages: int
+group_count: int
+friend_count: int
+day_call: int
+version: str
+connect_time: int
+connect_date: str
+connect_count: int
+status: bool
+is_select: bool
}
ApiDataSource ..> PlatformUtils : uses
ApiDataSource ..> BaseInfo : creates
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
嘿 @HibiKier - 我已经审查了你的更改,它们看起来很棒!
这是我在审查期间查看的内容
- 🟡 一般问题:发现2个问题
- 🟢 安全性:一切看起来都很好
- 🟢 测试:一切看起来都很好
- 🟢 复杂性:一切看起来都很好
- 🟢 文档:一切看起来都很好
帮助我变得更有用!请在每条评论上点击👍或👎,我将使用反馈来改进你的审查。
Original comment in English
Hey @HibiKier - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -28,15 +28,15 @@ | |||
|
|||
@driver.on_shutdown | |||
async def _(): | |||
if ws_conn: | |||
if ws_conn and ws_conn.client_state == WebSocketState.CONNECTED: | |||
await ws_conn.close() | |||
|
|||
|
|||
@ws_router.websocket("/chat") | |||
async def _(websocket: WebSocket): | |||
global ws_conn |
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.
建议 (bug_risk): 考虑使用连接管理器而不是全局变量来管理WebSocket连接
使用全局变量来管理WebSocket连接可能会导致多个客户端连接时出现竞争条件和连接问题。考虑实现一个可以处理多个同时连接的适当连接管理器。
建议的实现:
- 确保更新代码中之前使用
ws_conn
全局变量的任何其他部分,以使用适当的连接管理器方法 - 如果需要广播消息给所有连接,您可能需要为ConnectionManager类添加方法
- 考虑为连接失败添加错误处理
Original comment in English
suggestion (bug_risk): Consider using a connection manager instead of a global variable for websocket connections
Using a global variable for websocket connections could lead to race conditions and connection issues when multiple clients connect. Consider implementing a proper connection manager that can handle multiple simultaneous connections.
Suggested implementation:
from zhenxun.models.group_member_info import GroupInfoUser
from typing import List
class ConnectionManager:
def __init__(self):
self.active_connections: List[WebSocket] = []
async def connect(self, websocket: WebSocket):
await websocket.accept()
self.active_connections.append(websocket)
def disconnect(self, websocket: WebSocket):
if websocket in self.active_connections:
self.active_connections.remove(websocket)
async def close_all(self):
for connection in self.active_connections:
if connection.client_state == WebSocketState.CONNECTED:
await connection.close()
self.active_connections.clear()
manager = ConnectionManager()
@driver.on_shutdown
async def _():
await manager.close_all()
async def _(websocket: WebSocket):
await manager.connect(websocket)
try:
while websocket.client_state == WebSocketState.CONNECTED:
- Make sure to update any other parts of the code that were previously using the
ws_conn
global variable to use the appropriate connection manager methods instead - You may want to add methods to the ConnectionManager class for broadcasting messages to all connections if that functionality is needed
- Consider adding error handling for connection failures
<a href="https://nonebot.dev/"> | ||
<img src="https://img.shields.io/badge/nonebot-v2.1.3-EA5252" alt="nonebot"> | ||
</a> | ||
<a href="https://onebot.dev/"> |
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.
问题: QQ群链接不正确
QQ群的链接不正确。应该是https://qm.qq.com/q/mRNtLSl6uc
。
Original comment in English
issue: Incorrect QQ group link
The link for the QQ group is incorrect. It should be https://qm.qq.com/q/mRNtLSl6uc
.
else None, | ||
) | ||
for member in members | ||
] | ||
return [] | ||
|
||
@classmethod | ||
async def get_user( |
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.
问题 (代码质量): 我们发现了这些问题:
- 添加保护子句 (
last-if-guard
) - 用if表达式替换if语句 (
assign-if-exp
)
Original comment in English
issue (code-quality): We've found these issues:
- Add guard clause (
last-if-guard
) - Replace if statement with if expression (
assign-if-exp
)
@@ -411,69 +344,43 @@ | |||
return "unknown" | |||
|
|||
@classmethod | |||
async def get_group_list(cls, bot: Bot) -> tuple[list[GroupConsole], str]: | |||
async def get_group_list( |
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.
问题 (代码质量): 我们发现了这些问题:
- 添加保护子句 (
last-if-guard
) - 用列表扩展替换for附加循环 (
for-append-to-extend
)
Original comment in English
issue (code-quality): We've found these issues:
- Add guard clause (
last-if-guard
) - Replace a for append loop with list extend (
for-append-to-extend
)
Summary by Sourcery
文档:
Original summary in English
Summary by Sourcery
Documentation: