Conversation
|
@sourcery-ai review |
Reviewer's Guide将通知处理重构为一个集中式、支持国际化(i18n)的辅助工具,带可选图片支持;将其接入招募 SSR、崩溃以及每日任务完成等流程;新增 OneBot11 Provider 和 SMTP 图片附件支持;同时新增专门的 Notify 任务/调度器,并进行了一些清理修复。 统一 handle_notify 流程(包含 SSR 招募图片和 OneBot11)的时序图sequenceDiagram
actor User
participant DailyRecruit
participant NotifyHelper
participant NotifyI18N
participant DeployConfig
participant NotifyWinHandler
participant NotifyLinuxHandler
participant OnePushCore
participant OneBot11
User->>DailyRecruit: event_free_recruit()
DailyRecruit->>DailyRecruit: appear(RECRUIT_CONFIRM)
DailyRecruit->>DailyRecruit: appear(RECRUIT_NIKKE_SSR)
DailyRecruit->>DailyRecruit: save_drop_image(device.image,DailyRecruit_ScreenshotPath) image_path
DailyRecruit->>NotifyHelper: handle_notify(config,title_key=Recruit.title,content_key=Recruit.content,recruit_type_key=EventFree,image_path=image_path,always=Notification_WinOnePush)
NotifyHelper->>DeployConfig: Language
DeployConfig-->>NotifyHelper: lang
NotifyHelper->>NotifyI18N: get_text(RecruitType.EventFree,lang)
NotifyI18N-->>NotifyHelper: recruit_type
NotifyHelper->>NotifyI18N: get_text(Recruit.title,lang,config_name)
NotifyI18N-->>NotifyHelper: title
NotifyHelper->>NotifyI18N: get_text(Recruit.content,lang,config_name,recruit_type)
NotifyI18N-->>NotifyHelper: content
alt Windows platform
NotifyHelper->>NotifyWinHandler: handle_notify_win(title,content,image_path)
NotifyWinHandler-->>NotifyHelper: ok
alt always is True
NotifyHelper->>NotifyLinuxHandler: handle_notify_linux(Notification_OnePushConfig,title,content,image_path)
NotifyLinuxHandler->>OnePushCore: get_notifier(provider_name)
OnePushCore-->>NotifyLinuxHandler: OneBot11
NotifyLinuxHandler->>OneBot11: notify(endpoint,message_type,user_id,group_id,title,content,image_path)
OneBot11->>OneBot11: POST text message
OneBot11->>OneBot11: POST image message (base64)
OneBot11-->>NotifyLinuxHandler: Response(status_code)
NotifyLinuxHandler-->>NotifyHelper: success/fail
end
else NonWindows platform
NotifyHelper->>NotifyLinuxHandler: handle_notify_linux(Notification_OnePushConfig,title,content,image_path)
NotifyLinuxHandler->>OnePushCore: get_notifier(provider_name)
OnePushCore-->>NotifyLinuxHandler: OneBot11 or other Provider
NotifyLinuxHandler->>OneBot11: notify(...)
OneBot11->>OneBot11: send text and optional image
OneBot11-->>NotifyLinuxHandler: Response(status_code)
NotifyLinuxHandler-->>NotifyHelper: success/fail
end
NotifyHelper-->>DailyRecruit: done
DailyRecruit-->>User: SSR notification received
新通知流水线及 Provider 的类图classDiagram
class UI
class Notify{
+config
+device
+run()
}
class OnePushProvider{
<<interface>>
+params
+notify(**kwargs) Response
}
class OneBot11{
+name
+_params
+__init__()
+notify(**kwargs) Response
}
class NotifyI18N{
+I18N_NOTIFY
+get_text(key,lang,**kwargs) str
}
class NotifyHelper{
+handle_notify(config,title_key,content_key,**kwargs)
}
class NotifyWinHandler{
+handle_notify_win(**kwargs) bool
}
class NotifyLinuxHandler{
+handle_notify_linux(_config,**kwargs) bool
}
class SmtpImageParser{
+smtp_image_parser(self,subject,title,content,From,user,To,image_path,**kwargs) EmailMessage
}
class DeployConfig{
+Language
}
class NikkeConfig{
+config_name
+Notification_OnePushConfig
+Notification_WinOnePush
+Notification_WhenDailyTaskCompleted
+Notification_WhenDailyTaskCrashed
}
class DailyRecruit{
+config
+device
+save_drop_image(image,base_path) str
+notify_push(type,image=None)
+event_free_recruit(skip_first_screenshot=True)
+ordinary_150gem_recruit(skip_first_screenshot=True)
+social_point_recruit(skip_first_screenshot=True)
}
class MainRunner{
+config
+config_name
+device
+run(command,skip_first_screenshot=False)
+save_error_log() str
+_post_action()
+notify()
}
UI <|-- Notify
OnePushProvider <|-- OneBot11
Notify ..> NotifyHelper : uses
Notify ..> NikkeConfig : config
DailyRecruit ..> NotifyHelper : notify_push()
DailyRecruit ..> NikkeConfig : config
MainRunner ..> NotifyHelper : handle_notify()
MainRunner ..> NikkeConfig : config
NotifyHelper ..> NotifyWinHandler : calls
NotifyHelper ..> NotifyLinuxHandler : calls
NotifyHelper ..> NotifyI18N : get_text()
NotifyHelper ..> DeployConfig : read Language
NotifyHelper ..> NikkeConfig : Notification_OnePushConfig
NotifyLinuxHandler ..> OnePushProvider : get_notifier()
NotifyLinuxHandler ..> SmtpImageParser : smtp_image_parser()
OneBot11 ..|> OnePushProvider
SmtpImageParser ..> EmailMessage
文件级变更
Tips and commands与 Sourcery 交互
自定义你的体验访问你的 控制面板 来:
获取帮助Original review guide in EnglishReviewer's GuideRefactors notification handling into a centralized, i18n-aware helper with optional image support, wires it into recruit SSR, crash, and daily-completion flows, and adds a new OneBot11 provider plus SMTP image attachment support, alongside a dedicated Notify task/scheduler and some cleanup fixes. Sequence diagram for unified handle_notify flow with SSR recruit image and OneBot11sequenceDiagram
actor User
participant DailyRecruit
participant NotifyHelper
participant NotifyI18N
participant DeployConfig
participant NotifyWinHandler
participant NotifyLinuxHandler
participant OnePushCore
participant OneBot11
User->>DailyRecruit: event_free_recruit()
DailyRecruit->>DailyRecruit: appear(RECRUIT_CONFIRM)
DailyRecruit->>DailyRecruit: appear(RECRUIT_NIKKE_SSR)
DailyRecruit->>DailyRecruit: save_drop_image(device.image,DailyRecruit_ScreenshotPath) image_path
DailyRecruit->>NotifyHelper: handle_notify(config,title_key=Recruit.title,content_key=Recruit.content,recruit_type_key=EventFree,image_path=image_path,always=Notification_WinOnePush)
NotifyHelper->>DeployConfig: Language
DeployConfig-->>NotifyHelper: lang
NotifyHelper->>NotifyI18N: get_text(RecruitType.EventFree,lang)
NotifyI18N-->>NotifyHelper: recruit_type
NotifyHelper->>NotifyI18N: get_text(Recruit.title,lang,config_name)
NotifyI18N-->>NotifyHelper: title
NotifyHelper->>NotifyI18N: get_text(Recruit.content,lang,config_name,recruit_type)
NotifyI18N-->>NotifyHelper: content
alt Windows platform
NotifyHelper->>NotifyWinHandler: handle_notify_win(title,content,image_path)
NotifyWinHandler-->>NotifyHelper: ok
alt always is True
NotifyHelper->>NotifyLinuxHandler: handle_notify_linux(Notification_OnePushConfig,title,content,image_path)
NotifyLinuxHandler->>OnePushCore: get_notifier(provider_name)
OnePushCore-->>NotifyLinuxHandler: OneBot11
NotifyLinuxHandler->>OneBot11: notify(endpoint,message_type,user_id,group_id,title,content,image_path)
OneBot11->>OneBot11: POST text message
OneBot11->>OneBot11: POST image message (base64)
OneBot11-->>NotifyLinuxHandler: Response(status_code)
NotifyLinuxHandler-->>NotifyHelper: success/fail
end
else NonWindows platform
NotifyHelper->>NotifyLinuxHandler: handle_notify_linux(Notification_OnePushConfig,title,content,image_path)
NotifyLinuxHandler->>OnePushCore: get_notifier(provider_name)
OnePushCore-->>NotifyLinuxHandler: OneBot11 or other Provider
NotifyLinuxHandler->>OneBot11: notify(...)
OneBot11->>OneBot11: send text and optional image
OneBot11-->>NotifyLinuxHandler: Response(status_code)
NotifyLinuxHandler-->>NotifyHelper: success/fail
end
NotifyHelper-->>DailyRecruit: done
DailyRecruit-->>User: SSR notification received
Class diagram for the new notification pipeline and providersclassDiagram
class UI
class Notify{
+config
+device
+run()
}
class OnePushProvider{
<<interface>>
+params
+notify(**kwargs) Response
}
class OneBot11{
+name
+_params
+__init__()
+notify(**kwargs) Response
}
class NotifyI18N{
+I18N_NOTIFY
+get_text(key,lang,**kwargs) str
}
class NotifyHelper{
+handle_notify(config,title_key,content_key,**kwargs)
}
class NotifyWinHandler{
+handle_notify_win(**kwargs) bool
}
class NotifyLinuxHandler{
+handle_notify_linux(_config,**kwargs) bool
}
class SmtpImageParser{
+smtp_image_parser(self,subject,title,content,From,user,To,image_path,**kwargs) EmailMessage
}
class DeployConfig{
+Language
}
class NikkeConfig{
+config_name
+Notification_OnePushConfig
+Notification_WinOnePush
+Notification_WhenDailyTaskCompleted
+Notification_WhenDailyTaskCrashed
}
class DailyRecruit{
+config
+device
+save_drop_image(image,base_path) str
+notify_push(type,image=None)
+event_free_recruit(skip_first_screenshot=True)
+ordinary_150gem_recruit(skip_first_screenshot=True)
+social_point_recruit(skip_first_screenshot=True)
}
class MainRunner{
+config
+config_name
+device
+run(command,skip_first_screenshot=False)
+save_error_log() str
+_post_action()
+notify()
}
UI <|-- Notify
OnePushProvider <|-- OneBot11
Notify ..> NotifyHelper : uses
Notify ..> NikkeConfig : config
DailyRecruit ..> NotifyHelper : notify_push()
DailyRecruit ..> NikkeConfig : config
MainRunner ..> NotifyHelper : handle_notify()
MainRunner ..> NikkeConfig : config
NotifyHelper ..> NotifyWinHandler : calls
NotifyHelper ..> NotifyLinuxHandler : calls
NotifyHelper ..> NotifyI18N : get_text()
NotifyHelper ..> DeployConfig : read Language
NotifyHelper ..> NikkeConfig : Notification_OnePushConfig
NotifyLinuxHandler ..> OnePushProvider : get_notifier()
NotifyLinuxHandler ..> SmtpImageParser : smtp_image_parser()
OneBot11 ..|> OnePushProvider
SmtpImageParser ..> EmailMessage
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
OneBot11.notify, the missingendpointcase only logs an error but still proceeds and returns a Response with default status_code 0; consider short‑circuiting with a clear non‑200 status (e.g., 400) similar to the other validation branches so callers can reliably detect configuration errors. - The
_post_actionmethod now has a stray# 发送通知comment and extra blank lines without any related logic; consider removing or wiring this up to real notification behavior to keep the method’s intent clear.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `OneBot11.notify`, the missing `endpoint` case only logs an error but still proceeds and returns a Response with default status_code 0; consider short‑circuiting with a clear non‑200 status (e.g., 400) similar to the other validation branches so callers can reliably detect configuration errors.
- The `_post_action` method now has a stray `# 发送通知` comment and extra blank lines without any related logic; consider removing or wiring this up to real notification behavior to keep the method’s intent clear.
## Individual Comments
### Comment 1
<location path="module/notify/__init__.py" line_range="6-15" />
<code_context>
+def handle_notify(config, title_key: str, content_key, **kwargs):
</code_context>
<issue_to_address>
**issue (bug_risk):** Re-check whether not forwarding arbitrary kwargs to providers is acceptable or needs adjustment.
Previously, `handle_notify` passed all arbitrary kwargs to `handle_notify_linux`/providers, enabling provider-specific options (e.g., `subject`, custom headers, metadata). Now only `title`, `content`, and `image_path` are forwarded, while other kwargs are only used for i18n and then discarded.
If existing configs rely on those extra kwargs reaching providers, this becomes a breaking change. Either forward remaining non-internal kwargs as well (excluding things like `always`), or clearly document that only `title`, `content`, and `image_path` are passed through and update configs accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def handle_notify(config, title_key: str, content_key, **kwargs): | ||
| """ | ||
| 处理通知,支持多语言。 | ||
|
|
||
| Args: | ||
| config: NikkeConfig 实例,用于获取 onepush 配置和语言。 | ||
| title_key (str): 标题的 i18n key。 | ||
| content_key (str or list): 内容的 i18n key。 | ||
| **kwargs: | ||
| image_path (str): 图片路径。 |
There was a problem hiding this comment.
issue (bug_risk): Re-check whether not forwarding arbitrary kwargs to providers is acceptable or needs adjustment.
Previously, handle_notify passed all arbitrary kwargs to handle_notify_linux/providers, enabling provider-specific options (e.g., subject, custom headers, metadata). Now only title, content, and image_path are forwarded, while other kwargs are only used for i18n and then discarded.
If existing configs rely on those extra kwargs reaching providers, this becomes a breaking change. Either forward remaining non-internal kwargs as well (excluding things like always), or clearly document that only title, content, and image_path are passed through and update configs accordingly.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并留下了一些整体性的反馈:
- 在
OneBot11.notify中,当缺少endpoint时,你只记录了一条错误日志,但仍然继续构造api_url;建议在构造的 mockResponse上设置一个非 200 的状态码,并立即返回,以避免向无效 URL 发送请求。 - 新的
handle_notify包装函数丢弃了大部分原始的 kwargs,只把title、content和image_path传递给handle_notify_linux;如果你需要在每次调用时覆盖 provider 相关配置(例如自定义 endpoint/token/group_id),可以考虑在调用handle_notify_linux之前,显式允许这些参数并将它们合并进配置中。
给 AI 代理的提示
请根据下面的代码评审意见进行修改:
## 总体意见
- 在 `OneBot11.notify` 中,当缺少 `endpoint` 时,你只记录了一条错误日志,但仍然继续构造 `api_url`;建议在构造的 mock `Response` 上设置一个非 200 的状态码,并立即返回,以避免向无效 URL 发送请求。
- 新的 `handle_notify` 包装函数丢弃了大部分原始的 kwargs,只把 `title`、`content` 和 `image_path` 传递给 `handle_notify_linux`;如果你需要在每次调用时覆盖 provider 相关配置(例如自定义 endpoint/token/group_id),可以考虑在调用 `handle_notify_linux` 之前,显式允许这些参数并将它们合并进配置中。
## 逐条评论
### 评论 1
<location path="module/notify/onebot11.py" line_range="34-37" />
<code_context>
+ def notify(self, **kwargs) -> Response:
+ """重写 notify 方法,接管完整的推送逻辑"""
+ # 读取新的参数格式
+ endpoint = kwargs.get('endpoint', '').rstrip('/')
+ token = kwargs.get('token', '')
+ message_type = kwargs.get('message_type', '')
+ user_id = kwargs.get('user_id')
+ group_id = kwargs.get('group_id')
+
+ # 准备一个假的 Response 对象,用于兼容原 notify.py 的 status_code 校验逻辑
+ mock_resp = Response()
+
+ # 根据 message_type 校验必须的 ID 参数
+ if not endpoint:
+ logger.error("Notifier onebot11 require param 'endpoint'")
+ if message_type == 'private' and not user_id:
</code_context>
<issue_to_address>
**suggestion:** 当缺少 `endpoint` 时应尽早返回,而不是继续执行并最终触发失败的 HTTP 请求。
当前分支只是在 `endpoint` 缺失时记录一条日志,然后继续执行,构造出类似 `/send_group_msg` 的 URL,接着在 `requests.post` 中失败,并通过 `success = False` 变成一个笼统的 500。为了与 `user_id` / `group_id` / `message_type` 的校验逻辑保持一致,并提供更清晰的反馈,建议在缺少 `endpoint` 时设置 `mock_resp.status_code = 400` 并立即返回。
```suggestion
# 根据 message_type 校验必须的 ID 参数
if not endpoint:
logger.error("Notifier onebot11 require param 'endpoint'")
mock_resp.status_code = 400
return mock_resp
if message_type == 'private' and not user_id:
```
</issue_to_address>帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进评审质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
OneBot11.notify, whenendpointis missing you only log an error but still proceed to constructapi_url; consider setting a non-200 status on the mockResponseand returning immediately to avoid posting to an invalid URL. - The new
handle_notifywrapper drops most original kwargs and only forwardstitle,content, andimage_pathtohandle_notify_linux; if you need per-call overrides for provider options (e.g., custom endpoint/token/group_id), you may want to explicitly allow and merge those into the config before callinghandle_notify_linux.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `OneBot11.notify`, when `endpoint` is missing you only log an error but still proceed to construct `api_url`; consider setting a non-200 status on the mock `Response` and returning immediately to avoid posting to an invalid URL.
- The new `handle_notify` wrapper drops most original kwargs and only forwards `title`, `content`, and `image_path` to `handle_notify_linux`; if you need per-call overrides for provider options (e.g., custom endpoint/token/group_id), you may want to explicitly allow and merge those into the config before calling `handle_notify_linux`.
## Individual Comments
### Comment 1
<location path="module/notify/onebot11.py" line_range="34-37" />
<code_context>
+ def notify(self, **kwargs) -> Response:
+ """重写 notify 方法,接管完整的推送逻辑"""
+ # 读取新的参数格式
+ endpoint = kwargs.get('endpoint', '').rstrip('/')
+ token = kwargs.get('token', '')
+ message_type = kwargs.get('message_type', '')
+ user_id = kwargs.get('user_id')
+ group_id = kwargs.get('group_id')
+
+ # 准备一个假的 Response 对象,用于兼容原 notify.py 的 status_code 校验逻辑
+ mock_resp = Response()
+
+ # 根据 message_type 校验必须的 ID 参数
+ if not endpoint:
+ logger.error("Notifier onebot11 require param 'endpoint'")
+ if message_type == 'private' and not user_id:
</code_context>
<issue_to_address>
**suggestion:** Return early when `endpoint` is missing instead of falling through to a failing HTTP request.
This branch only logs the missing `endpoint` and then continues, building a URL like `/send_group_msg` and failing later in `requests.post`, which becomes a generic 500 via `success = False`. For consistency with the `user_id` / `group_id` / `message_type` validation and to give clearer feedback, set `mock_resp.status_code = 400` and return immediately when `endpoint` is missing.
```suggestion
# 根据 message_type 校验必须的 ID 参数
if not endpoint:
logger.error("Notifier onebot11 require param 'endpoint'")
mock_resp.status_code = 400
return mock_resp
if message_type == 'private' and not user_id:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
重构通知系统,使其支持国际化(i18n)、图片附件,并新增一个专门用于每日完成情况通知的任务,同时集成 OneBot11 和 SMTP 图片支持。
新功能:
增强改进:
Original summary in English
Summary by Sourcery
Revamp the notification system to be i18n-aware, support image attachments, and add a dedicated daily completion notify task while integrating OneBot11 and SMTP image support.
New Features:
Enhancements: