feat(git): 支持订阅仓库议题变更推送 #11
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 新增议题订阅功能，支持 push 和 issue 事件 - 添加 issue.svg 图标资源 - 创建议题推送页面模板和样式文件 - 实现定时任务检测议题变更并推送 - 数据库新增 event 和 issue 表用于存储订阅信息 - 更新添加仓库命令，支持指定订阅事件类型 - 优化日期格式化工具函数 - 修复客户端代理配置逻辑错误 - 完善议题信息渲染模板 - 添加议题推送合集转发消息功能
Closed
4 tasks
审阅者指南此 PR 通过引入新的事件和议题数据库表和模型，扩展管理员命令以管理订阅类型，重构推送和议题任务以利用统一的事件模型，并添加用于渲染和分发通知的模板、实用程序和配置增强功能，实现了一个事件驱动的仓库推送和议题事件订阅系统。 AddRepo 命令与事件订阅的序列图sequenceDiagram
actor Admin
participant Bot
participant DB
participant Client
Admin->>Bot: #git添加[platform]订阅仓库owner/repo[:branch] [event]
Bot->>DB: GetRepo(botId, groupId, owner, repo)
alt 仓库不存在
Bot->>DB: AddRepo(...)
Bot->>DB: GetRepo(...)
end
Bot->>DB: GetRepo(platform, repoId, eventType)
alt 事件不存在
Bot->>DB: AddRepo(platform, repoId, eventType)
Bot->>DB: GetRepo(platform, repoId, eventType)
else 事件存在
Bot->>DB: UpdateEventType(platform, repoId, eventType)
end
alt eventType 包含 push
Bot->>Client: getRepoInfo(owner, repo)
Bot->>DB: GetRepo(eventId, branch)
alt pushRepo 不存在
Bot->>DB: AddRepo(eventId, branch)
end
end
alt eventType 包含 issue
Bot->>Client: getIssueList(owner, repo)
loop 针对每个议题
Bot->>DB: AddRepo(eventId, issue.number, hash(title), hash(body), state)
end
end
Bot->>Admin: 回复订阅结果
计划的议题轮询和通知推送的序列图sequenceDiagram
participant TaskScheduler
participant Bot
participant DB
participant Client
participant Group
TaskScheduler->>Bot: 定时任务触发 (议题轮询)
Bot->>DB: GetAll(platform, EventType.Issue)
loop 针对每个事件
Bot->>DB: GetRepo(event.repoId)
Bot->>Client: getIssueList(owner, repo)
loop 针对每个议题
Bot->>DB: GetRepo(event.id, issue.number)
alt 议题不存在
Bot->>DB: AddRepo(event.id, issue.number, hash(title), hash(body), state)
Bot->>Group: 推送新议题通知
else 议题已更改
Bot->>DB: UpdateState(event.id, issue.number, state)
Bot->>Group: 推送议题变更通知
end
end
end
新增和更新表的 ER 图：event, push, issue, repoerDiagram
REPO ||--o{ EVENT : "has"
EVENT ||--o{ PUSH : "has"
EVENT ||--o{ ISSUE : "has"
REPO {
id int PK
botId string
groupId string
owner string
repo string
createAt date
updateAt date
}
EVENT {
id int PK
repoId int FK
platform string
eventType string
createdAt date
updatedAt date
}
PUSH {
id int PK
eventId int FK
branch string
commitSha string
createdAt date
updatedAt date
}
ISSUE {
id int PK
eventId int FK
issueId string
title string
body string
state string
createdAt date
updatedAt date
}
新增和更新数据模型的类图：RepoInfo, EventRepo, PushRepo, IssueRepoclassDiagram
class RepoInfo {
+int id
+string botId
+string groupId
+string owner
+string repo
+Date createAt
+Date updateAt
}
class EventRepo {
+int id
+int repoId
+Platform platform
+EventType[] eventType
+Date createAt
+Date updateAt
}
class PushRepo {
+int eventId
+string branch
+string commitSha
+Date createAt
+Date updateAt
}
class IssueRepo {
+int eventId
+string issueId
+string title
+string body
+StateType state
+Date createAt
+Date updateAt
}
RepoInfo "1" --o "*" EventRepo : 拥有
EventRepo "1" --o "*" PushRepo : 拥有
EventRepo "1" --o "*" IssueRepo : 拥有
文件级更改
对关联议题的评估
可能关联的议题
提示和命令与 Sourcery 互动
自定义你的体验访问你的 仪表盘 以：
获取帮助Original review guide in EnglishReviewer's GuideThis PR implements an event-driven subscription system for repository push and issue events by introducing new event and issue database tables and models, extending admin commands to manage subscription types, refactoring push and issue tasks to leverage the unified event model, and adding templates, utilities, and configuration enhancements for rendering and dispatching notifications. Sequence diagram for AddRepo command with event subscriptionsequenceDiagram
actor Admin
participant Bot
participant DB
participant Client
Admin->>Bot: #git添加[platform]订阅仓库owner/repo[:branch] [event]
Bot->>DB: GetRepo(botId, groupId, owner, repo)
alt repo not exists
Bot->>DB: AddRepo(...)
Bot->>DB: GetRepo(...)
end
Bot->>DB: GetRepo(platform, repoId, eventType)
alt event not exists
Bot->>DB: AddRepo(platform, repoId, eventType)
Bot->>DB: GetRepo(platform, repoId, eventType)
else event exists
Bot->>DB: UpdateEventType(platform, repoId, eventType)
end
alt eventType includes push
Bot->>Client: getRepoInfo(owner, repo)
Bot->>DB: GetRepo(eventId, branch)
alt pushRepo not exists
Bot->>DB: AddRepo(eventId, branch)
end
end
alt eventType includes issue
Bot->>Client: getIssueList(owner, repo)
loop for each issue
Bot->>DB: AddRepo(eventId, issue.number, hash(title), hash(body), state)
end
end
Bot->>Admin: 回复订阅结果
Sequence diagram for scheduled issue polling and notification pushsequenceDiagram
participant TaskScheduler
participant Bot
participant DB
participant Client
participant Group
TaskScheduler->>Bot: 定时任务触发 (issue polling)
Bot->>DB: GetAll(platform, EventType.Issue)
loop for each event
Bot->>DB: GetRepo(event.repoId)
Bot->>Client: getIssueList(owner, repo)
loop for each issue
Bot->>DB: GetRepo(event.id, issue.number)
alt issue not exists
Bot->>DB: AddRepo(event.id, issue.number, hash(title), hash(body), state)
Bot->>Group: 推送新议题通知
else issue changed
Bot->>DB: UpdateState(event.id, issue.number, state)
Bot->>Group: 推送议题变更通知
end
end
end
ER diagram for new and updated tables: event, push, issue, repoerDiagram
REPO ||--o{ EVENT : "has"
EVENT ||--o{ PUSH : "has"
EVENT ||--o{ ISSUE : "has"
REPO {
id int PK
botId string
groupId string
owner string
repo string
createAt date
updateAt date
}
EVENT {
id int PK
repoId int FK
platform string
eventType string
createdAt date
updatedAt date
}
PUSH {
id int PK
eventId int FK
branch string
commitSha string
createdAt date
updatedAt date
}
ISSUE {
id int PK
eventId int FK
issueId string
title string
body string
state string
createdAt date
updatedAt date
}
Class diagram for new and updated data models: RepoInfo, EventRepo, PushRepo, IssueRepoclassDiagram
class RepoInfo {
+int id
+string botId
+string groupId
+string owner
+string repo
+Date createAt
+Date updateAt
}
class EventRepo {
+int id
+int repoId
+Platform platform
+EventType[] eventType
+Date createAt
+Date updateAt
}
class PushRepo {
+int eventId
+string branch
+string commitSha
+Date createAt
+Date updateAt
}
class IssueRepo {
+int eventId
+string issueId
+string title
+string body
+StateType state
+Date createAt
+Date updateAt
}
RepoInfo "1" --o "*" EventRepo : has
EventRepo "1" --o "*" PushRepo : has
EventRepo "1" --o "*" IssueRepo : has
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
大家好 - 我已经审阅了您的更改 - 以下是一些反馈：
- 在 AddRepo/RemoveRepo 命令中为事件参数添加输入验证和默认处理，以避免用户省略或错误输入事件类型时出现运行时错误。
- 将您的 TypeScript 模型字段名称 (createAt/updateAt) 与数据库列名称 (createdAt/updatedAt) 对齐，以避免映射不匹配。
- 将重复的平台到客户端映射逻辑提取到共享助手函数中，以减少命令和任务之间的重复。
AI 代理提示
请解决此代码审查中的评论：
## 总体评论
- 在 AddRepo/RemoveRepo 命令中为事件参数添加输入验证和默认处理，以避免用户省略或错误输入事件类型时出现运行时错误。
- 将您的 TypeScript 模型字段名称 (createAt/updateAt) 与数据库列名称 (createdAt/updatedAt) 对齐，以避免映射不匹配。
- 将重复的平台到客户端映射逻辑提取到共享助手函数中，以减少命令和任务之间的重复。
## 单独评论
### 评论 1
<location> `src/apps/admin.ts:44` </location>
<code_context>
+ let eventInfo = await db.event.GetRepo(platformName, repoInfo.id)
+ if (!eventInfo?.length) {
</code_context>
<issue_to_address>
**issue:** `eventInfo` 是数组及其用法之间可能存在的混淆。
由于 `eventInfo` 始终是一个数组，如果数组为空，检查 `!eventInfo` 可能无法按预期工作。请使用 `!eventInfo.length` 来确保检查准确无误。
</issue_to_address>
### 评论 2
<location> `src/apps/admin.ts:54` </location>
<code_context>
+
+ let msg = `添加订阅仓库成功, 平台: ${platformName}, 仓库: ${owner}/${repo}, 订阅类型: ${event}`
+
+ if (eventType.includes(EventType.Push)) {
+ const PushBranch = branch || 'main'
+ const pushRepo = await db.push.GetRepo(
</code_context>
<issue_to_address>
**issue (bug_risk):** 推送订阅逻辑硬编码了 Platform.GitHub。
在 `pushRepo` 查找中使用 `Platform.GitHub` 可能会导致非 GitHub 平台上的行为不正确。请使用 `platformName` 以保持一致性。
</issue_to_address>
### 评论 3
<location> `src/apps/push.ts:193` </location>
<code_context>
+ pushRepoInfo.repo,
+ pushRepo.branch,
+ )
+ if (!commitInfo || commitInfo.sha === pushRepo.commitSha) continue
+ const groupKey = `${pushRepoInfo.botId}-${pushRepoInfo.groupId}`
+ if (!groupMap.has(groupKey)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** 处理新提交后未更新 `commitSha`。
未更新 `commitSha` 可能会导致重复通知。请在处理后更新数据库中的 `commitSha`。
</issue_to_address>
### 评论 4
<location> `src/apps/issue.ts:22` </location>
<code_context>
+ if (isEmpty(token)) return logger.warn('未配置GitHub Token, 跳过任务')
+ try {
+ const client = Client.github()
+ handleRepoIssue(client, Platform.GitHub)
+ } catch (e) {
+ logger.error(e)
</code_context>
<issue_to_address>
**issue (bug_risk):** `handleRepoIssue` 缺少 `await` 可能会导致未处理的 Promise 拒绝。
由于 `handleRepoIssue` 是异步的，不等待它可能会导致执行不完整和遗漏错误。请添加 `await` 以确保正确处理。
</issue_to_address>
### 评论 5
<location> `src/apps/push.ts:159` </location>
<code_context>
)
const handleRepoPush = async (client: ClientType, platform: Platform) => {
- const all = await db.push.GetAll()
+ const all = await db.event.GetAll()
</code_context>
<issue_to_address>
**issue (complexity):** 考虑通过提取辅助函数来重构 `handleRepoPush`，用于获取、分组和处理推送项。
考虑将 `handleRepoPush` 分解为更小的步骤并提取分组逻辑。例如：
1) 提取所有“符合推送条件”的项
2) 按机器人+组进行分组
3) 按组渲染/发送
```ts
// helpers/push.ts
export async function fetchPushItems(
client: ClientType,
platform: Platform
): Promise<Array<{
pushRepo: PushRepo
pushRepoInfo: RepoInfo
commitInfo: CommitInfo
}>> {
const events = await db.event.GetAll()
const pushEvents = events.filter(
e => e.platform === platform && e.eventType.includes(EventType.Push)
)
const items: Array<{
pushRepo: PushRepo
pushRepoInfo: RepoInfo
commitInfo: CommitInfo
}> = []
for (const ev of pushEvents) {
const pushList = (await db.push.GetAll()).filter(p => p.repoId === ev.repoId)
for (const pushRepo of pushList) {
const info = await db.repo.GetRepo(pushRepo.repoId)
if (!info) continue
const commitInfo = await client.getCommitInfo(
info.owner, info.repo, pushRepo.branch
)
if (!commitInfo || commitInfo.sha === pushRepo.commitSha) continue
items.push({ pushRepo, pushRepoInfo: info, commitInfo })
}
}
return items
}
export function groupByBotGroup<T extends { pushRepoInfo: RepoInfo }>(
items: T[]
): Record<string, T[]> {
return items.reduce((map, item) => {
const key = `${item.pushRepoInfo.botId}-${item.pushRepoInfo.groupId}`
;(map[key] = map[key] || []).push(item)
return map
}, {} as Record<string, T[]>)
}
```
然后简化 `handleRepoPush`：
```ts
import { fetchPushItems, groupByBotGroup } from './helpers/push'
const handleRepoPush = async (client: ClientType, platform: Platform) => {
const items = await fetchPushItems(client, platform)
if (items.length === 0) return
const groups = groupByBotGroup(items)
for (const [key, groupItems] of Object.entries(groups)) {
const [botId, groupId] = key.split('-')
let image: ImageElement[] = []
for (const { pushRepo, pushRepoInfo, commitInfo } of groupItems) {
const parts = commitInfo.commit.message.split('\n')
// …build PushCommitInfo, Render, etc.
}
// …send to contactGroup(botId, groupId, image)
}
}
```
优点：
- 单一职责的助手函数
- 更扁平的循环（核心内部没有嵌套的“.filter”+“.map”）
- 更容易测试/维护每个部分而无需改变整体行为
</issue_to_address>
### 评论 6
<location> `src/models/db/issue.ts:7` </location>
<code_context>
+import { EventRepo, EventType, Platform } from '@/types'
+import { createClient } from './index'
+
+export async function AddRepo(
+ platform: Platform,
+ repoId: number,
</code_context>
<issue_to_address>
**issue (complexity):** 考虑将重载函数拆分为单一职责函数，并将数据库调用提取到辅助方法中，以阐明意图并减少分支。
```markdown
您可以通过以下方式显著降低认知负担：
1. 将每个重载拆分为两个命名清晰、职责单一的函数。
2. 将原始的 `client.run`/`client.all`/`client.get` 提取为返回 Promise 的小型辅助函数。
例如：
```ts
// db.ts
async function run(sql: string, params: any[]): Promise<void> {
const client = await createClient();
return new Promise((res, rej) =>
client.run(sql, params, err => (err ? rej(err) : res()))
);
}
async function all<T>(sql: string, params: any[]): Promise<T[]> {
const client = await createClient();
return new Promise((res, rej) =>
client.all(sql, params, (err, rows) => (err ? rej(err) : res(rows as T[])))
);
}
async function getOne<T>(sql: string, params: any[]): Promise<T | null> {
const client = await createClient();
return new Promise((res, rej) =>
client.get(sql, params, (err, row) => (err ? rej(err) : res(row as T | null)))
);
}
```
```ts
// issue.ts
export async function createRepo(platform: Platform, repoId: number) {
await run(
'INSERT INTO issue(platform, repoId) VALUES (?, ?)',
[platform, repoId]
);
}
export async function createIssue(
platform: Platform,
repoId: number,
issueId: string,
title: string,
body: string | null,
state: StateType
) {
await run(
'INSERT INTO issue(platform, repoId, issueId, title, body, state) VALUES (?, ?, ?, ?, ?, ?)',
[platform, repoId, issueId, title, body, state]
);
}
export async function getAllIssues(): Promise<IssueRepo[]> {
return all<IssueRepo>('SELECT * FROM issue', []);
}
export async function getRepoIssues(
platform: Platform,
repoId: number
): Promise<IssueRepo[]> {
return all<IssueRepo>(
'SELECT * FROM issue WHERE platform = ? AND repoId = ?',
[platform, repoId]
);
}
export async function getIssue(
platform: Platform,
repoId: number,
issueId: string
): Promise<IssueRepo | null> {
return getOne<IssueRepo>(
'SELECT * FROM issue WHERE platform = ? AND repoId = ? AND issueId = ?',
[platform, repoId, issueId]
);
}
export async function deleteRepo(platform: Platform, repoId: number) {
await run(
'DELETE FROM issue WHERE platform = ? AND repoId = ?',
[platform, repoId]
);
}
export async function deleteIssue(
platform: Platform,
repoId: number,
issueId: string
) {
await run(
'DELETE FROM issue WHERE platform = ? AND repoId = ? AND issueId = ?',
[platform, repoId, issueId]
);
}
export async function updateState(
platform: Platform,
repoId: number,
issueId: string,
state: StateType
) {
await run(
'UPDATE issue SET state = ? WHERE platform = ? AND repoId = ? AND issueId = ?',
[state, platform, repoId, issueId]
);
}
```
这将：
- 使每个函数目的和参数明确。
- 消除可选参数分支。
- 将 Promise 封装整合到**三个**小型辅助函数中。
```
</issue_to_address>
### 评论 7
<location> `src/apps/issue.ts:18` </location>
<code_context>
const token = Config.github.token
</code_context>
<issue_to_address>
**suggestion (code-quality):** 在访问和使用属性时，首选对象解构。([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {token} = Config.github
```
<br/><details><summary>解释</summary>对象解构通常可以消除不必要的临时引用，并使您的代码更简洁。
来自 [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### 评论 8
<location> `src/apps/issue.ts:33` </location>
<code_context>
const token = Config.gitee.token
</code_context>
<issue_to_address>
**suggestion (code-quality):** 在访问和使用属性时，首选对象解构。([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {token} = Config.gitee
```
<br/><details><summary>解释</summary>对象解构通常可以消除不必要的临时引用，并使您的代码更简洁。
来自 [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### 评论 9
<location> `src/apps/issue.ts:48` </location>
<code_context>
const token = Config.gitcode.token
</code_context>
<issue_to_address>
**suggestion (code-quality):** 在访问和使用属性时，首选对象解构。([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {token} = Config.gitcode
```
<br/><details><summary>解释</summary>对象解构通常可以消除不必要的临时引用，并使您的代码更简洁。
来自 [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### 10
<location> `src/apps/issue.ts:63` </location>
<code_context>
const token = Config.cnb.token
</code_context>
<issue_to_address>
**suggestion (code-quality):** 在访问和使用属性时，首选对象解构。([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {token} = Config.cnb
```
<br/><details><summary>解释</summary>对象解构通常可以消除不必要的临时引用，并使您的代码更简洁。
来自 [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### 评论 11
<location> `src/models/db/issue.ts:47-53` </location>
<code_context>
const result = await new Promise<IssueRepo[]>((resolve, reject) => {
client.all('SELECT * FROM issue', [], (err, rows) => {
if (err) reject(err)
else resolve(rows as IssueRepo[])
})
})
return result
</code_context>
<issue_to_address>
**suggestion (code-quality):** 内联立即返回的变量 ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return await new Promise<IssueRepo[]>((resolve, reject) => {
client.all('SELECT * FROM issue', [], (err, rows) => {
if (err) reject(err)
else resolve(rows as IssueRepo[])
})
});
```
<br/><details><summary>解释</summary>我们经常在人们的代码中看到将结果变量赋值然后立即返回的情况。
直接返回结果可以缩短代码并消除不必要的变量，从而减少阅读函数的认知负担。
中间变量的有用之处在于，如果它们被用作参数或条件，并且名称可以作为变量表示内容的注释。在您从函数返回它的情况下，函数名称会告诉您结果是什么，因此变量名称是不必要的。
</details>
</issue_to_address>
### 评论 12
<location> `src/models/db/issue.ts:74-84` </location>
<code_context>
const result = await new Promise<IssueRepo | null>((resolve, reject) => {
client.get(
'SELECT * FROM issue WHERE platform = ? AND repoId = ? AND issueId = ?',
[platform, repoId, issueId],
(err, rows) => {
if (err) reject(err)
else resolve(rows as unknown as IssueRepo | null)
},
)
})
return result
</code_context>
<issue_to_address>
**suggestion (code-quality):** 内联立即返回的变量 ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return await new Promise<IssueRepo | null>((resolve, reject) => {
client.get(
'SELECT * FROM issue WHERE platform = ? AND repoId = ? AND issueId = ?',
[platform, repoId, issueId],
(err, rows) => {
if (err) reject(err)
else resolve(rows as unknown as IssueRepo | null)
},
)
});
```
<br/><details><summary>解释</summary>我们经常在人们的代码中看到将结果变量赋值然后立即返回的情况。
直接返回结果可以缩短代码并消除不必要的变量，从而减少阅读函数的认知负担。
中间变量的有用之处在于，如果它们被用作参数或条件，并且名称可以作为变量表示内容的注释。在您从函数返回它的情况下，函数名称会告诉您结果是什么，因此变量名称是不必要的。
</details>
</issue_to_address>
### 评论 13
<location> `src/models/db/issue.ts:86-96` </location>
<code_context>
const result = await new Promise<IssueRepo[]>((resolve, reject) => {
client.all(
'SELECT * FROM issue WHERE platform = ? AND repoId = ?',
[platform, repoId],
(err, rows) => {
if (err) reject(err)
else resolve(rows as IssueRepo[])
},
)
})
return result
</code_context>
<issue_to_address>
**suggestion (code-quality):** 内联立即返回的变量 ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return await new Promise<IssueRepo[]>((resolve, reject) => {
client.all(
'SELECT * FROM issue WHERE platform = ? AND repoId = ?',
[platform, repoId],
(err, rows) => {
if (err) reject(err)
else resolve(rows as IssueRepo[])
},
)
});
```
<br/><details><summary>解释</summary>我们经常在人们的代码中看到将结果变量赋值然后立即返回的情况。
直接返回结果可以缩短代码并消除不必要的变量，从而减少阅读函数的认知负担。
中间变量的有用之处在于，如果它们被用作参数或条件，并且名称可以作为变量表示内容的注释。在您从函数返回它的情况下，函数名称会告诉您结果是什么，因此变量名称是不必要的。
</details>
</issue_to_address>帮助我更有用！请点击每个评论上的 👍 或 👎，我将使用反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- Add input validation and default handling for the event parameter in AddRepo/RemoveRepo commands to avoid runtime errors when the user omits or mistypes the event type.
- Align your TypeScript model field names (createAt/updateAt) with the database column names (createdAt/updatedAt) to avoid mapping mismatches.
- Extract the repeated platform–to–client mapping logic into a shared helper to reduce duplication across commands and tasks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add input validation and default handling for the event parameter in AddRepo/RemoveRepo commands to avoid runtime errors when the user omits or mistypes the event type.
- Align your TypeScript model field names (createAt/updateAt) with the database column names (createdAt/updatedAt) to avoid mapping mismatches.
- Extract the repeated platform–to–client mapping logic into a shared helper to reduce duplication across commands and tasks.
## Individual Comments
### Comment 1
<location> `src/apps/admin.ts:44` </location>
<code_context>
+ let eventInfo = await db.event.GetRepo(platformName, repoInfo.id)
+ if (!eventInfo?.length) {
</code_context>
<issue_to_address>
**issue:** Possible confusion between eventInfo being an array and its usage.
Since eventInfo is always an array, checking '!eventInfo' may not work as intended if the array is empty. Use '!eventInfo.length' to ensure the check is accurate.
</issue_to_address>
### Comment 2
<location> `src/apps/admin.ts:54` </location>
<code_context>
+
+ let msg = `添加订阅仓库成功, 平台: ${platformName}, 仓库: ${owner}/${repo}, 订阅类型: ${event}`
+
+ if (eventType.includes(EventType.Push)) {
+ const PushBranch = branch || 'main'
+ const pushRepo = await db.push.GetRepo(
</code_context>
<issue_to_address>
**issue (bug_risk):** Push subscription logic hardcodes Platform.GitHub.
Using Platform.GitHub in pushRepo lookup may lead to incorrect behavior on non-GitHub platforms. Use platformName instead for consistency.
</issue_to_address>
### Comment 3
<location> `src/apps/push.ts:193` </location>
<code_context>
+ pushRepoInfo.repo,
+ pushRepo.branch,
+ )
+ if (!commitInfo || commitInfo.sha === pushRepo.commitSha) continue
+ const groupKey = `${pushRepoInfo.botId}-${pushRepoInfo.groupId}`
+ if (!groupMap.has(groupKey)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** No update to commitSha after processing new commits.
Not updating commitSha may cause duplicate notifications. Please update commitSha in the database after processing.
</issue_to_address>
### Comment 4
<location> `src/apps/issue.ts:22` </location>
<code_context>
+ if (isEmpty(token)) return logger.warn('未配置GitHub Token, 跳过任务')
+ try {
+ const client = Client.github()
+ handleRepoIssue(client, Platform.GitHub)
+ } catch (e) {
+ logger.error(e)
</code_context>
<issue_to_address>
**issue (bug_risk):** Missing await for handleRepoIssue may cause unhandled promise rejections.
Since handleRepoIssue is async, not awaiting it may result in incomplete execution and missed errors. Add 'await' to ensure proper handling.
</issue_to_address>
### Comment 5
<location> `src/apps/push.ts:159` </location>
<code_context>
)
const handleRepoPush = async (client: ClientType, platform: Platform) => {
- const all = await db.push.GetAll()
+ const all = await db.event.GetAll()
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring `handleRepoPush` by extracting helper functions for fetching, grouping, and processing push items.
Consider breaking `handleRepoPush` into smaller steps and extracting the grouping logic. For example:
1) Extract fetching all “push‐eligible” items
2) Group them by bot+group
3) Render/send per group
```ts
// helpers/push.ts
export async function fetchPushItems(
client: ClientType,
platform: Platform
): Promise<Array<{
pushRepo: PushRepo
pushRepoInfo: RepoInfo
commitInfo: CommitInfo
}>> {
const events = await db.event.GetAll()
const pushEvents = events.filter(
e => e.platform === platform && e.eventType.includes(EventType.Push)
)
const items: Array<{
pushRepo: PushRepo
pushRepoInfo: RepoInfo
commitInfo: CommitInfo
}> = []
for (const ev of pushEvents) {
const pushList = (await db.push.GetAll()).filter(p => p.repoId === ev.repoId)
for (const pushRepo of pushList) {
const info = await db.repo.GetRepo(pushRepo.repoId)
if (!info) continue
const commitInfo = await client.getCommitInfo(
info.owner, info.repo, pushRepo.branch
)
if (!commitInfo || commitInfo.sha === pushRepo.commitSha) continue
items.push({ pushRepo, pushRepoInfo: info, commitInfo })
}
}
return items
}
export function groupByBotGroup<T extends { pushRepoInfo: RepoInfo }>(
items: T[]
): Record<string, T[]> {
return items.reduce((map, item) => {
const key = `${item.pushRepoInfo.botId}-${item.pushRepoInfo.groupId}`
;(map[key] = map[key] || []).push(item)
return map
}, {} as Record<string, T[]>)
}
```
Then simplify `handleRepoPush`:
```ts
import { fetchPushItems, groupByBotGroup } from './helpers/push'
const handleRepoPush = async (client: ClientType, platform: Platform) => {
const items = await fetchPushItems(client, platform)
if (items.length === 0) return
const groups = groupByBotGroup(items)
for (const [key, groupItems] of Object.entries(groups)) {
const [botId, groupId] = key.split('-')
let image: ImageElement[] = []
for (const { pushRepo, pushRepoInfo, commitInfo } of groupItems) {
const parts = commitInfo.commit.message.split('\n')
// …build PushCommitInfo, Render, etc.
}
// …send to contactGroup(botId, groupId, image)
}
}
```
Benefits:
- Single‐responsibility helpers
- Flatter loops (no nested “.filter” + “.map” inside core)
- Easier to test/maintain each piece without changing overall behavior
</issue_to_address>
### Comment 6
<location> `src/models/db/issue.ts:7` </location>
<code_context>
+import { EventRepo, EventType, Platform } from '@/types'
+import { createClient } from './index'
+
+export async function AddRepo(
+ platform: Platform,
+ repoId: number,
</code_context>
<issue_to_address>
**issue (complexity):** Consider splitting overloaded functions into single-responsibility functions and extracting database calls into helper methods to clarify intent and reduce branching.
```markdown
You can dramatically reduce cognitive overhead by
1. Splitting each overload into two clearly-named, single-responsibility functions.
2. Factoring out your raw `client.run`/`client.all`/`client.get` into small promise-returning helpers.
For example:
```ts
// db.ts
async function run(sql: string, params: any[]): Promise<void> {
const client = await createClient();
return new Promise((res, rej) =>
client.run(sql, params, err => (err ? rej(err) : res()))
);
}
async function all<T>(sql: string, params: any[]): Promise<T[]> {
const client = await createClient();
return new Promise((res, rej) =>
client.all(sql, params, (err, rows) => (err ? rej(err) : res(rows as T[])))
);
}
async function getOne<T>(sql: string, params: any[]): Promise<T | null> {
const client = await createClient();
return new Promise((res, rej) =>
client.get(sql, params, (err, row) => (err ? rej(err) : res(row as T | null)))
);
}
```
```ts
// issue.ts
export async function createRepo(platform: Platform, repoId: number) {
await run(
'INSERT INTO issue(platform, repoId) VALUES (?, ?)',
[platform, repoId]
);
}
export async function createIssue(
platform: Platform,
repoId: number,
issueId: string,
title: string,
body: string | null,
state: StateType
) {
await run(
'INSERT INTO issue(platform, repoId, issueId, title, body, state) VALUES (?, ?, ?, ?, ?, ?)',
[platform, repoId, issueId, title, body, state]
);
}
export async function getAllIssues(): Promise<IssueRepo[]> {
return all<IssueRepo>('SELECT * FROM issue', []);
}
export async function getRepoIssues(
platform: Platform,
repoId: number
): Promise<IssueRepo[]> {
return all<IssueRepo>(
'SELECT * FROM issue WHERE platform = ? AND repoId = ?',
[platform, repoId]
);
}
export async function getIssue(
platform: Platform,
repoId: number,
issueId: string
): Promise<IssueRepo | null> {
return getOne<IssueRepo>(
'SELECT * FROM issue WHERE platform = ? AND repoId = ? AND issueId = ?',
[platform, repoId, issueId]
);
}
export async function deleteRepo(platform: Platform, repoId: number) {
await run(
'DELETE FROM issue WHERE platform = ? AND repoId = ?',
[platform, repoId]
);
}
export async function deleteIssue(
platform: Platform,
repoId: number,
issueId: string
) {
await run(
'DELETE FROM issue WHERE platform = ? AND repoId = ? AND issueId = ?',
[platform, repoId, issueId]
);
}
export async function updateState(
platform: Platform,
repoId: number,
issueId: string,
state: StateType
) {
await run(
'UPDATE issue SET state = ? WHERE platform = ? AND repoId = ? AND issueId = ?',
[state, platform, repoId, issueId]
);
}
```
This:
- Makes each function’s purpose and parameters explicit.
- Removes optional-parameter branching.
- Consolidates promise-wrapping in **three** tiny helpers.
```
</issue_to_address>
### Comment 7
<location> `src/apps/issue.ts:18` </location>
<code_context>
const token = Config.github.token
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {token} = Config.github
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 8
<location> `src/apps/issue.ts:33` </location>
<code_context>
const token = Config.gitee.token
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {token} = Config.gitee
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 9
<location> `src/apps/issue.ts:48` </location>
<code_context>
const token = Config.gitcode.token
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {token} = Config.gitcode
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 10
<location> `src/apps/issue.ts:63` </location>
<code_context>
const token = Config.cnb.token
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {token} = Config.cnb
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 11
<location> `src/models/db/issue.ts:47-53` </location>
<code_context>
const result = await new Promise<IssueRepo[]>((resolve, reject) => {
client.all('SELECT * FROM issue', [], (err, rows) => {
if (err) reject(err)
else resolve(rows as IssueRepo[])
})
})
return result
</code_context>
<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return await new Promise<IssueRepo[]>((resolve, reject) => {
client.all('SELECT * FROM issue', [], (err, rows) => {
if (err) reject(err)
else resolve(rows as IssueRepo[])
})
});
```
<br/><details><summary>Explanation</summary>Something that we often see in people's code is assigning to a result variable
and then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
</details>
</issue_to_address>
### Comment 12
<location> `src/models/db/issue.ts:74-84` </location>
<code_context>
const result = await new Promise<IssueRepo | null>((resolve, reject) => {
client.get(
'SELECT * FROM issue WHERE platform = ? AND repoId = ? AND issueId = ?',
[platform, repoId, issueId],
(err, rows) => {
if (err) reject(err)
else resolve(rows as unknown as IssueRepo | null)
},
)
})
return result
</code_context>
<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return await new Promise<IssueRepo | null>((resolve, reject) => {
client.get(
'SELECT * FROM issue WHERE platform = ? AND repoId = ? AND issueId = ?',
[platform, repoId, issueId],
(err, rows) => {
if (err) reject(err)
else resolve(rows as unknown as IssueRepo | null)
},
)
});
```
<br/><details><summary>Explanation</summary>Something that we often see in people's code is assigning to a result variable
and then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
</details>
</issue_to_address>
### Comment 13
<location> `src/models/db/issue.ts:86-96` </location>
<code_context>
const result = await new Promise<IssueRepo[]>((resolve, reject) => {
client.all(
'SELECT * FROM issue WHERE platform = ? AND repoId = ?',
[platform, repoId],
(err, rows) => {
if (err) reject(err)
else resolve(rows as IssueRepo[])
},
)
})
return result
</code_context>
<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return await new Promise<IssueRepo[]>((resolve, reject) => {
client.all(
'SELECT * FROM issue WHERE platform = ? AND repoId = ?',
[platform, repoId],
(err, rows) => {
if (err) reject(err)
else resolve(rows as IssueRepo[])
},
)
});
```
<br/><details><summary>Explanation</summary>Something that we often see in people's code is assigning to a result variable
and then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Contributor
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
你好 - 我已经审查了你的更改，它们看起来很棒！
AI 代理提示
请解决此代码审查中的评论：
## 单独评论
### 评论 1
<location> `src/apps/admin.ts:69` </location>
<code_context>
+ }
}
+
+ if (eventType.includes(EventType.Issue)) {
+ const IssueRepo = await db.issue.GetRepo(Platform.GitHub, repoInfo.id)
+ if (!IssueRepo?.length) {
</code_context>
<issue_to_address>
**issue (bug_risk):** 问题订阅逻辑可能导致数据库条目重复。
对同一存储库重复执行 AddRepo 可能会插入重复的问题。在插入之前实施唯一性约束或检查现有问题。
</issue_to_address>
### 评论 2
<location> `src/apps/admin.ts:54` </location>
<code_context>
+
+ let msg = `添加订阅仓库成功, 平台: ${platformName}, 仓库: ${owner}/${repo}, 订阅类型: ${event}`
+
+ if (eventType.includes(EventType.Push)) {
+ const PushBranch = branch || 'main'
+ const pushRepo = await db.push.GetRepo(
</code_context>
<issue_to_address>
**issue (bug_risk):** db.push.GetRepo 中的硬编码平台可能导致不一致。
db.push.GetRepo 始终使用 Platform.GitHub，这可能导致非 GitHub 平台的行为不正确。请改用 platformName 以确保正确处理。
</issue_to_address>
### 评论 3
<location> `src/apps/admin.ts:6` </location>
<code_context>
+import { ClientType, EventType, Platform } from '@/types'
import karin from 'node-karin'
export const AddRepo = karin.command(
- /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?$/i,
+ /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?\s+([^/\s]+)?$/i,
</code_context>
<issue_to_address>
**issue (complexity):** 考虑重构命令逻辑，将平台解析、事件解析和事件订阅提取到辅助函数中，并将 Push/Issue 处理拆分为专用函数。
```markdown
我们可以通过提取三个辅助函数来大幅简化这两个命令：
1) `resolvePlatform` (平台名称 + 客户端)
2) `parseEventTypes`
3) `ensureEventSubscription`
然后将 Push/Issue 处理拆分为 `handlePush`/`handleIssue`。
--- helpers.ts ---
```ts
import { ClientType, EventType, Platform } from '@/types'
import { Client } from '@/common'
// 1) platform + client resolution
export function resolvePlatform(p?: string): {
platform: Platform,
client: ClientType
} {
switch (p?.toLowerCase()) {
case 'gitcode': return { platform: Platform.GitCode, client: Client.gitcode() }
case 'gitee': return { platform: Platform.Gitee, client: Client.gitee() }
case 'cnb': return { platform: Platform.Cnb, client: Client.cnb() }
default: return { platform: Platform.GitHub, client: Client.github() }
}
}
// 2) event parser
export function parseEventTypes(ev?: string): EventType[] {
if (!ev) return [EventType.Push]
return ev.toLowerCase()
.split(',')
.map(v => v.trim() === 'issue' ? EventType.Issue : EventType.Push)
}
// 3) event‐table upsert
export async function ensureEventSubscription(
platform: Platform, repoId: number, types: EventType[]
) {
const existing = await db.event.GetRepo(platform, repoId)
if (!existing?.length) {
await db.event.AddRepo(platform, repoId, types)
return db.event.GetRepo(platform, repoId)
} else {
await db.event.UpdateEventType(platform, repoId, types)
return existing
}
}
```
--- usage in AddRepo.ts ---
```ts
import { resolvePlatform, parseEventTypes, ensureEventSubscription } from './helpers'
export const AddRepo = karin.command(
/^...$/,
async e => {
const [, platStr, owner, name, branch, evStr] = e.msg.match(AddRepo.reg)!
const { platform, client } = resolvePlatform(platStr)
const eventTypes = parseEventTypes(evStr)
// repo ensure
let repoInfo = await db.repo.GetRepo(e.selfId, e.groupId, owner, name)
if (!repoInfo) {
await db.repo.AddRepo(e.selfId, e.groupId, owner, name)
repoInfo = await db.repo.GetRepo(e.selfId, e.groupId, owner, name)
}
if (!repoInfo) return e.reply('添加订阅仓库失败，请重试')
await ensureEventSubscription(platform, repoInfo.id, eventTypes)
let msg = `添加订阅仓库成功, 平台: ${platform}, 仓库: ${owner}/${name}, 订阅类型: ${evStr}`
if (eventTypes.includes(EventType.Push)) {
// ... call a small handlePush helper if you like
}
if (eventTypes.includes(EventType.Issue)) {
// ... call a small handleIssue helper
}
return e.reply(msg)
},
{ /* ... */ }
)
```
这会提取所有重复的分支，使每个步骤都集中，并保持您现有的行为不变。
</issue_to_address>
### 评论 4
<location> `src/apps/issue.ts:14` </location>
<code_context>
+ logger,
+} from 'node-karin'
+
+export const github = karin.task(
+ 'karin-plugin-git:issue:github',
+ Config.github.cron || '0 */5 * * * *',
</code_context>
<issue_to_address>
**issue (complexity):** 考虑通过将重复的任务定义提取到辅助函数中，并将主处理程序拆分为专注于功能的函数，以提高清晰度和可维护性来重构代码。
```suggestion
您可以通过以下方式显着减少重复和嵌套：
1. 将四个几乎相同的任务定义提取到 `createIssueTask` 辅助函数中。
2. 将 `handleRepoIssue` 拆分为三个专注的函数：
- `getRepoInfos(platform)`
- `groupNewOrUpdatedIssues(client, repos, platform)`
- `dispatchGroups(groupMap)`
这是一个最小示例：
```ts
// issue.ts
// 1. TASK FACTORY
function createIssueTask(
name: string,
platform: Platform,
cfg: { cron?: string; token?: string },
clientFactory: () => ClientType
) {
return karin.task(
`karin-plugin-git:issue:${name}`,
cfg.cron || '0 */5 * * * *',
async () => {
if (isEmpty(cfg.token)) return logger.warn(`未配置${name} Token, 跳过任务`);
try {
const client = clientFactory();
await handleRepoIssue(client, platform);
} catch (e) {
logger.error(e);
}
}
);
}
export const github = createIssueTask('github', Platform.GitHub, Config.github, Client.github);
export const gitee = createIssueTask('gitee', Platform.Gitee, Config.gitee, Client.gitee);
export const gitcode= createIssueTask('gitcode',Platform.GitCode, Config.gitcode,Client.gitcode);
export const cnb = createIssueTask('cnb', Platform.Cnb, Config.cnb, Client.cnb);
// 2. MAIN HANDLER
async function handleRepoIssue(client: ClientType, platform: Platform) {
const repos = await getRepoInfos(platform);
const groupMap = await groupNewOrUpdatedIssues(client, repos, platform);
await dispatchGroups(groupMap);
}
// 3a. FETCH REPO SUBSCRIPTIONS
async function getRepoInfos(platform: Platform) {
const subs = await db.event.GetAll();
return subs.filter(
s => s.platform === platform && s.eventType.includes(EventType.Issue)
);
}
// 3b. BUILD GROUP MAP
async function groupNewOrUpdatedIssues(
client: ClientType,
subs: Array<{ repoId: string }>,
platform: Platform
) {
const map = new Map<string, IssuePayload[]>();
for (const sub of subs) {
const repoInfo = await db.repo.GetRepo(sub.repoId);
if (!repoInfo) continue;
const key = `${repoInfo.botId}-${repoInfo.groupId}`;
const list = await client.getIssueList(repoInfo.owner, repoInfo.repo, { perPage: 100 });
for (const issue of list) {
const changed = await isIssueChanged(sub.repoId, platform, issue);
if (!changed) continue;
await saveIssueState(sub.repoId, platform, issue);
const entry = formatIssue(issue, repoInfo);
(map.get(key) ?? map.set(key, []).get(key)!).push(entry);
}
}
return map;
}
// 3c. DISPATCH MESSAGES
async function dispatchGroups(map: Map<string, IssuePayload[]>) {
for (const [key, items] of map) {
const [botId, groupId] = key.split('-');
const images = await Promise.all(items.map(i => Render.render('issue/index', { issue: i })));
if (images.length) {
await sendImage(botId, groupId, images);
}
}
}
// Helper examples:
async function isIssueChanged(repoId, platform, issue) { /* compare db.issue.GetRepo */ }
async function saveIssueState(repoId, platform, issue) { /* db.issue.AddRepo/UpdateState */ }
function formatIssue(issue, repoInfo) { /* returns IssuePayload */ }
```
这保留了所有现有行为，但：
- 移除了四个复制粘贴的任务。
- 将 `handleRepoIssue` 扁平化为清晰、可测试的步骤。
- 将数据库逻辑移动到小型辅助函数中 (`isIssueChanged`, `saveIssueState`, `formatIssue`)。
```
</issue_to_address>
### 评论 5
<location> `src/models/db/issue.ts:5` </location>
<code_context>
+import { createClient } from './index'
+import { Platform, IssueRepo } from '@/types'
+
+export async function AddRepo(platform: Platform, repoId: number): Promise<void>
+
+export async function AddRepo(
</code_context>
<issue_to_address>
**issue (complexity):** 考虑重构代码以使用通用的异步 SQL 辅助函数，并将重载折叠为每个 CRUD 操作的单个函数。
这里有一种方法可以消除所有重复的 Promise/SQL 样板代码，并将您的重载折叠成几个通用辅助函数。您保留了 100% 的相同功能，但：
• 将每个原始的 `new Promise` + 回调替换为简单的 `await run/get/all`
• 删除了每个 CRUD 方法的 3 个重载
```ts
// db.ts
import { createClient } from './index'
import { Database } from 'sqlite3'
import { promisify } from 'util'
type Client = Database & {
runAsync: (sql: string, params?: any[]) => Promise<void>
getAsync: <T>(sql: string, params?: any[]) => Promise<T | undefined>
allAsync: <T>(sql: string, params?: any[]) => Promise<T[]>
}
export async function getClient(): Promise<Client> {
const client: Database = await createClient()
return Object.assign(client, {
runAsync: promisify(client.run!.bind(client)),
getAsync: promisify(client.get!.bind(client)),
allAsync: promisify(client.all!.bind(client)),
})
}
```
```ts
// issueModel.ts
import { getClient } from './db'
import { Platform, IssueRepo } from '@/types'
import { StateType } from 'nipaw'
const TABLE = 'issue'
async function queryOne<T>(sql: string, params: any[]): Promise<T | null> {
const client = await getClient()
const row = await client.getAsync<T>(sql, params)
return row ?? null
}
async function queryAll<T>(sql: string, params: any[]): Promise<T[]> {
const client = await getClient()
return client.allAsync<T>(sql, params)
}
async function execute(sql: string, params: any[]): Promise<void> {
const client = await getClient()
await client.runAsync(sql, params)
}
export async function AddRepo(
platform: Platform,
repoId: number,
issueId?: string,
title?: string,
body?: string | null,
state?: StateType
): Promise<void> {
if (issueId && title != null && body != null && state != null) {
await execute(
`INSERT INTO ${TABLE}
(platform, repoId, issueId, title, body, state)
VALUES (?, ?, ?, ?, ?, ?)`,
[platform, repoId, issueId, title, body, state]
)
} else {
await execute(
`INSERT INTO ${TABLE} (platform, repoId) VALUES (?, ?)`,
[platform, repoId]
)
}
}
export async function GetAll(): Promise<IssueRepo[]> {
return queryAll<IssueRepo>(`SELECT * FROM ${TABLE}`, [])
}
export async function GetRepo(
platform: Platform,
repoId: number,
issueId?: string
): Promise<IssueRepo[] | IssueRepo | null> {
if (issueId) {
return queryOne<IssueRepo>(
`SELECT * FROM ${TABLE}
WHERE platform = ? AND repoId = ? AND issueId = ?`,
[platform, repoId, issueId]
)
}
return queryAll<IssueRepo>(
`SELECT * FROM ${TABLE} WHERE platform = ? AND repoId = ?`,
[platform, repoId]
)
}
export async function RemoveRepo(
platform: Platform,
repoId: number,
issueId?: string
): Promise<void> {
const sql = issueId
? `DELETE FROM ${TABLE} WHERE platform = ? AND repoId = ? AND issueId = ?`
: `DELETE FROM ${TABLE} WHERE platform = ? AND repoId = ?`
const params = issueId ? [platform, repoId, issueId] : [platform, repoId]
return execute(sql, params)
}
export async function UpdateState(
platform: Platform,
repoId: number,
issueId: string,
state: StateType
): Promise<void> {
return execute(
`UPDATE ${TABLE}
SET state = ?
WHERE platform = ? AND repoId = ? AND issueId = ?`,
[state, platform, repoId, issueId]
)
}
```
优点：
- 不再重复 `new Promise` + 回调
- 每次查询只调用一次 `getClient()`
- 所有重载都折叠成一个可选参数签名
- SQL 字符串与辅助函数相邻，因此仍然清楚正在运行的内容。
</issue_to_address>请帮助我更有用！请点击每个评论上的 👍 或 👎，我将使用反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/apps/admin.ts:69` </location>
<code_context>
+ }
}
+
+ if (eventType.includes(EventType.Issue)) {
+ const IssueRepo = await db.issue.GetRepo(Platform.GitHub, repoInfo.id)
+ if (!IssueRepo?.length) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Issue subscription logic may result in duplicate database entries.
Repeated AddRepo executions for the same repository may insert duplicate issues. Implement a uniqueness constraint or check for existing issues before inserting.
</issue_to_address>
### Comment 2
<location> `src/apps/admin.ts:54` </location>
<code_context>
+
+ let msg = `添加订阅仓库成功, 平台: ${platformName}, 仓库: ${owner}/${repo}, 订阅类型: ${event}`
+
+ if (eventType.includes(EventType.Push)) {
+ const PushBranch = branch || 'main'
+ const pushRepo = await db.push.GetRepo(
</code_context>
<issue_to_address>
**issue (bug_risk):** Hardcoded platform in db.push.GetRepo may cause inconsistency.
db.push.GetRepo always uses Platform.GitHub, which may lead to incorrect behavior for non-GitHub platforms. Use platformName instead to ensure proper handling.
</issue_to_address>
### Comment 3
<location> `src/apps/admin.ts:6` </location>
<code_context>
+import { ClientType, EventType, Platform } from '@/types'
import karin from 'node-karin'
export const AddRepo = karin.command(
- /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?$/i,
+ /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?\s+([^/\s]+)?$/i,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the command logic by extracting platform resolution, event parsing, and event subscription into helpers, and splitting Push/Issue handling into dedicated functions.
```markdown
We can drastically simplify both commands by pulling out three helpers:
1) `resolvePlatform` (platformName + client)
2) `parseEventTypes`
3) `ensureEventSubscription`
Then split Push/Issue handling into `handlePush`/`handleIssue`.
--- helpers.ts ---
```ts
import { ClientType, EventType, Platform } from '@/types'
import { Client } from '@/common'
// 1) platform + client resolution
export function resolvePlatform(p?: string): {
platform: Platform,
client: ClientType
} {
switch (p?.toLowerCase()) {
case 'gitcode': return { platform: Platform.GitCode, client: Client.gitcode() }
case 'gitee': return { platform: Platform.Gitee, client: Client.gitee() }
case 'cnb': return { platform: Platform.Cnb, client: Client.cnb() }
default: return { platform: Platform.GitHub, client: Client.github() }
}
}
// 2) event parser
export function parseEventTypes(ev?: string): EventType[] {
if (!ev) return [EventType.Push]
return ev.toLowerCase()
.split(',')
.map(v => v.trim() === 'issue' ? EventType.Issue : EventType.Push)
}
// 3) event‐table upsert
export async function ensureEventSubscription(
platform: Platform, repoId: number, types: EventType[]
) {
const existing = await db.event.GetRepo(platform, repoId)
if (!existing?.length) {
await db.event.AddRepo(platform, repoId, types)
return db.event.GetRepo(platform, repoId)
} else {
await db.event.UpdateEventType(platform, repoId, types)
return existing
}
}
```
--- usage in AddRepo.ts ---
```ts
import { resolvePlatform, parseEventTypes, ensureEventSubscription } from './helpers'
export const AddRepo = karin.command(
/^...$/,
async e => {
const [, platStr, owner, name, branch, evStr] = e.msg.match(AddRepo.reg)!
const { platform, client } = resolvePlatform(platStr)
const eventTypes = parseEventTypes(evStr)
// repo ensure
let repoInfo = await db.repo.GetRepo(e.selfId, e.groupId, owner, name)
if (!repoInfo) {
await db.repo.AddRepo(e.selfId, e.groupId, owner, name)
repoInfo = await db.repo.GetRepo(e.selfId, e.groupId, owner, name)
}
if (!repoInfo) return e.reply('添加订阅仓库失败，请重试')
await ensureEventSubscription(platform, repoInfo.id, eventTypes)
let msg = `添加订阅仓库成功, 平台: ${platform}, 仓库: ${owner}/${name}, 订阅类型: ${evStr}`
if (eventTypes.includes(EventType.Push)) {
// ... call a small handlePush helper if you like
}
if (eventTypes.includes(EventType.Issue)) {
// ... call a small handleIssue helper
}
return e.reply(msg)
},
{ /* ... */ }
)
```
This pulls out all repeated branching, makes each step focused, and keeps your existing behavior intact.
</issue_to_address>
### Comment 4
<location> `src/apps/issue.ts:14` </location>
<code_context>
+ logger,
+} from 'node-karin'
+
+export const github = karin.task(
+ 'karin-plugin-git:issue:github',
+ Config.github.cron || '0 */5 * * * *',
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the code by extracting repeated task definitions into a helper and splitting the main handler into focused functions for clarity and maintainability.
```suggestion
You can significantly reduce duplication and nesting by:
1. Extracting the four nearly‐identical task definitions into a `createIssueTask` helper.
2. Breaking `handleRepoIssue` into three focused functions:
- `getRepoInfos(platform)`
- `groupNewOrUpdatedIssues(client, repos, platform)`
- `dispatchGroups(groupMap)`
Here’s a minimal example:
```ts
// issue.ts
// 1. TASK FACTORY
function createIssueTask(
name: string,
platform: Platform,
cfg: { cron?: string; token?: string },
clientFactory: () => ClientType
) {
return karin.task(
`karin-plugin-git:issue:${name}`,
cfg.cron || '0 */5 * * * *',
async () => {
if (isEmpty(cfg.token)) return logger.warn(`未配置${name} Token, 跳过任务`);
try {
const client = clientFactory();
await handleRepoIssue(client, platform);
} catch (e) {
logger.error(e);
}
}
);
}
export const github = createIssueTask('github', Platform.GitHub, Config.github, Client.github);
export const gitee = createIssueTask('gitee', Platform.Gitee, Config.gitee, Client.gitee);
export const gitcode= createIssueTask('gitcode',Platform.GitCode, Config.gitcode,Client.gitcode);
export const cnb = createIssueTask('cnb', Platform.Cnb, Config.cnb, Client.cnb);
// 2. MAIN HANDLER
async function handleRepoIssue(client: ClientType, platform: Platform) {
const repos = await getRepoInfos(platform);
const groupMap = await groupNewOrUpdatedIssues(client, repos, platform);
await dispatchGroups(groupMap);
}
// 3a. FETCH REPO SUBSCRIPTIONS
async function getRepoInfos(platform: Platform) {
const subs = await db.event.GetAll();
return subs.filter(
s => s.platform === platform && s.eventType.includes(EventType.Issue)
);
}
// 3b. BUILD GROUP MAP
async function groupNewOrUpdatedIssues(
client: ClientType,
subs: Array<{ repoId: string }>,
platform: Platform
) {
const map = new Map<string, IssuePayload[]>();
for (const sub of subs) {
const repoInfo = await db.repo.GetRepo(sub.repoId);
if (!repoInfo) continue;
const key = `${repoInfo.botId}-${repoInfo.groupId}`;
const list = await client.getIssueList(repoInfo.owner, repoInfo.repo, { perPage: 100 });
for (const issue of list) {
const changed = await isIssueChanged(sub.repoId, platform, issue);
if (!changed) continue;
await saveIssueState(sub.repoId, platform, issue);
const entry = formatIssue(issue, repoInfo);
(map.get(key) ?? map.set(key, []).get(key)!).push(entry);
}
}
return map;
}
// 3c. DISPATCH MESSAGES
async function dispatchGroups(map: Map<string, IssuePayload[]>) {
for (const [key, items] of map) {
const [botId, groupId] = key.split('-');
const images = await Promise.all(items.map(i => Render.render('issue/index', { issue: i })));
if (images.length) {
await sendImage(botId, groupId, images);
}
}
}
// Helper examples:
async function isIssueChanged(repoId, platform, issue) { /* compare db.issue.GetRepo */ }
async function saveIssueState(repoId, platform, issue) { /* db.issue.AddRepo/UpdateState */ }
function formatIssue(issue, repoInfo) { /* returns IssuePayload */ }
```
This keeps all existing behavior but:
- Removes four copy‐pasted tasks.
- Flattens `handleRepoIssue` into clear, testable steps.
- Moves DB logic into small helpers (`isIssueChanged`, `saveIssueState`, `formatIssue`).
```
</issue_to_address>
### Comment 5
<location> `src/models/db/issue.ts:5` </location>
<code_context>
+import { createClient } from './index'
+import { Platform, IssueRepo } from '@/types'
+
+export async function AddRepo(platform: Platform, repoId: number): Promise<void>
+
+export async function AddRepo(
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the code to use generic async SQL helpers and collapse overloads into single functions for each CRUD operation.
Here’s one way to DRY‐up all the repeated Promise/SQL boilerplate and collapse your overloads into a couple of generic helpers. You keep 100% of the same functionality but:
• replace every raw `new Promise` + callback with a simple `await run/get/all`
• remove the 3 overloads per CRUD method
```ts
// db.ts
import { createClient } from './index'
import { Database } from 'sqlite3'
import { promisify } from 'util'
type Client = Database & {
runAsync: (sql: string, params?: any[]) => Promise<void>
getAsync: <T>(sql: string, params?: any[]) => Promise<T | undefined>
allAsync: <T>(sql: string, params?: any[]) => Promise<T[]>
}
export async function getClient(): Promise<Client> {
const client: Database = await createClient()
return Object.assign(client, {
runAsync: promisify(client.run!.bind(client)),
getAsync: promisify(client.get!.bind(client)),
allAsync: promisify(client.all!.bind(client)),
})
}
```
```ts
// issueModel.ts
import { getClient } from './db'
import { Platform, IssueRepo } from '@/types'
import { StateType } from 'nipaw'
const TABLE = 'issue'
async function queryOne<T>(sql: string, params: any[]): Promise<T | null> {
const client = await getClient()
const row = await client.getAsync<T>(sql, params)
return row ?? null
}
async function queryAll<T>(sql: string, params: any[]): Promise<T[]> {
const client = await getClient()
return client.allAsync<T>(sql, params)
}
async function execute(sql: string, params: any[]): Promise<void> {
const client = await getClient()
await client.runAsync(sql, params)
}
export async function AddRepo(
platform: Platform,
repoId: number,
issueId?: string,
title?: string,
body?: string | null,
state?: StateType
): Promise<void> {
if (issueId && title != null && body != null && state != null) {
await execute(
`INSERT INTO ${TABLE}
(platform, repoId, issueId, title, body, state)
VALUES (?, ?, ?, ?, ?, ?)`,
[platform, repoId, issueId, title, body, state]
)
} else {
await execute(
`INSERT INTO ${TABLE} (platform, repoId) VALUES (?, ?)`,
[platform, repoId]
)
}
}
export async function GetAll(): Promise<IssueRepo[]> {
return queryAll<IssueRepo>(`SELECT * FROM ${TABLE}`, [])
}
export async function GetRepo(
platform: Platform,
repoId: number,
issueId?: string
): Promise<IssueRepo[] | IssueRepo | null> {
if (issueId) {
return queryOne<IssueRepo>(
`SELECT * FROM ${TABLE}
WHERE platform = ? AND repoId = ? AND issueId = ?`,
[platform, repoId, issueId]
)
}
return queryAll<IssueRepo>(
`SELECT * FROM ${TABLE} WHERE platform = ? AND repoId = ?`,
[platform, repoId]
)
}
export async function RemoveRepo(
platform: Platform,
repoId: number,
issueId?: string
): Promise<void> {
const sql = issueId
? `DELETE FROM ${TABLE} WHERE platform = ? AND repoId = ? AND issueId = ?`
: `DELETE FROM ${TABLE} WHERE platform = ? AND repoId = ?`
const params = issueId ? [platform, repoId, issueId] : [platform, repoId]
return execute(sql, params)
}
export async function UpdateState(
platform: Platform,
repoId: number,
issueId: string,
state: StateType
): Promise<void> {
return execute(
`UPDATE ${TABLE}
SET state = ?
WHERE platform = ? AND repoId = ? AND issueId = ?`,
[state, platform, repoId, issueId]
)
}
```
Benefits:
- no more repeated `new Promise` + callbacks
- only one `getClient()` call per query
- all overloads collapse into a single optional‐param signature
- SQL strings live next to the helpers, so it’s still obvious what’s running.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
你好 - 我已审阅你的更改，它们看起来很棒！
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/apps/issue.ts:75` </location>
<code_context>
+)
+
+const handleRepoIssue = async (client: ClientType, platform: Platform) => {
+ const all = await db.event.GetAll(platform, EventType.Push)
+ const groupMap = new Map<
+ string,
</code_context>
<issue_to_address>
**issue (bug_risk):** 用于检索议题事件的 EventType.Push 可能不正确。
请考虑更新 handleRepoIssue 中的事件检索，以使用 EventType.Issue，确保处理相关的议题事件。
</issue_to_address>
### Comment 2
<location> `src/models/db/event.ts:32-33` </location>
<code_context>
+): Promise<Array<EventRepo>> {
+ let client = await createClient()
+ let result: Array<EventRepo>
+ if (platform) {
+ result = await new Promise<Array<EventRepo>>((resolve, reject) => {
+ client.all(
+ 'SELECT * FROM event WHERE platform = ?',
</code_context>
<issue_to_address>
**suggestion (bug_risk):** EventType 过滤逻辑可能无法按预期工作。
目前，当同时指定 platform 和 eventType 时，仅使用 platform 进行过滤。请更新逻辑以在提供这两个参数时支持按两者进行过滤。
</issue_to_address>
### Comment 3
<location> `src/models/db/event.ts:165` </location>
<code_context>
+ await new Promise<void>((resolve, reject) => {
+ client.run(
+ 'UPDATE event SET eventType = ? WHERE platform = ? AND repoId = ?',
+ [event, platform, repoId],
+ (err) => {
+ if (err) reject(err)
</code_context>
<issue_to_address>
**issue (bug_risk):** EventType 数组直接传递给 SQL，这可能会导致序列化错误。
在 UpdateEventType 中，请在将事件数组作为参数传递之前，将其连接成一个逗号分隔的字符串，以防止与 SQL 存储格式发生类型不匹配。
</issue_to_address>
### Comment 4
<location> `src/apps/admin.ts:12-14` </location>
<code_context>
- const [, platform, owner, repo, branch] = e.msg.match(AddRepo!.reg)!
+ const [, platform, owner, repo, branch, event] = e.msg.match(AddRepo!.reg)!
+
+ const eventType = event
+ ? (event.toLocaleLowerCase().split(',') as Array<EventType>)
+ : [EventType.Push]
</code_context>
<issue_to_address>
**suggestion:** 如果事件为空字符串，eventType 解析可能会产生空数组。
如果事件为空字符串，eventType 将是 ['']，这与任何有效的 EventType 都不匹配。请过滤掉空字符串，或者在事件为空时默认设置为 [EventType.Push]。
```suggestion
const eventType = event
? (
event
.toLocaleLowerCase()
.split(',')
.map(e => e.trim())
.filter(e => e) as Array<EventType>
)
: [EventType.Push]
// If eventType is empty after filtering, default to [EventType.Push]
if (!eventType.length) {
eventType.push(EventType.Push)
}
```
</issue_to_address>
### Comment 5
<location> `src/apps/admin.ts:139` </location>
<code_context>
}
- await db.push.RemoveRepo(platformName, repoInfo.id, PushBranch)
+ if (eventType.includes(EventType.Issue)) {
+ const issueRepo = await db.issue.GetRepo(repoInfo.id)
+ if (!issueRepo) {
+ return await e.reply('议题订阅不存在，删除失败')
</code_context>
<issue_to_address>
**question:** issueRepo 检索中 eventId 的使用不一致。
请验证此处使用 repoInfo.id 是否正确，并与其他 eventInfo.id 的使用保持一致。
</issue_to_address>
### Comment 6
<location> `src/apps/admin.ts:7` </location>
<code_context>
+import { isEmpty } from 'es-toolkit/compat'
import karin from 'node-karin'
export const AddRepo = karin.command(
- /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?$/i,
+ /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?\s+([^/\s]+)?$/i,
</code_context>
<issue_to_address>
**issue (complexity):** 考虑通过将平台/客户端解析、仓库/事件记录管理和事件订阅逻辑提取到辅助函数中来重构命令处理程序，以提高模块化。
```suggestion
`AddRepo` 和 `RemoveRepo` 都在一个函数中完成了相同的三件事：
1) 解析平台→客户端，
2) 确保仓库和事件记录，
3) 订阅/取消订阅每种事件类型。
你可以将这些任务分解成小的辅助函数——这样你的命令处理程序就只负责协调：
1. 提取平台 + 客户端解析
2. 提取“确保仓库”和“确保事件”逻辑
3. 提取推送/议题订阅 + 取消订阅
示例：
```ts
// 1) resolve platform + client
function getPlatformClient(p?: string): { platform: Platform, client: ClientType } {
switch (p?.toLowerCase()) {
case 'gitcode': return { platform: Platform.GitCode, client: Client.gitcode() }
case 'gitee': return { platform: Platform.Gitee, client: Client.gitee() }
case 'cnb': return { platform: Platform.Cnb, client: Client.cnb() }
default: return { platform: Platform.GitHub, client: Client.github() }
}
}
// 2) ensure repo + event records
async function ensureRepoEvent(
botId: number, groupId: number,
owner: string, repo: string,
platform: Platform, events: EventType[]
) {
let info = await db.repo.GetRepo(botId, groupId, owner, repo)
if (!info) {
await db.repo.AddRepo(botId, groupId, owner, repo)
info = await db.repo.GetRepo(botId, groupId, owner, repo)!
}
let ev = await db.event.GetRepo(platform, info.id, events)
if (!ev) {
await db.event.AddRepo(platform, info.id, events)
ev = await db.event.GetRepo(platform, info.id, events)!
} else {
await db.event.UpdateEventType(platform, info.id, events)
}
return { repoId: info.id, eventId: ev!.id }
}
// 3a) push subscription
async function subscribePush(eventId: number, branch: string, client: ClientType) {
const existing = await db.push.GetRepo(eventId, branch)
if (!existing) {
await db.push.AddRepo(eventId, branch)
return `分支: ${branch}`
} else {
return `推送订阅已存在，分支: ${branch}`
}
}
// 3b) issue subscription
async function subscribeIssue(eventId: number, owner: string, repo: string, client: ClientType) {
const existing = await db.issue.GetRepo(eventId)
if (existing.length === 0) {
const list = await client.getIssueList(owner, repo, { perPage: 100 })
for (const i of list) {
await db.issue.AddRepo(eventId, i.number, make_hash(i.title), i.body && make_hash(i.body), i.state)
}
return `议题 ${list.length} 条已订阅`
} else {
return `议题订阅已存在`
}
}
// In your command:
export const AddRepo = karin.command(/^…$/, async e => {
const [, platformArg, owner, repo, branchArg, evtArg] = e.msg.match(AddRepo!.reg)!
const events = evtArg?.toLowerCase().split(',') as EventType[] ?? [EventType.Push]
const { platform, client } = getPlatformClient(platformArg)
const { repoId, eventId } = await ensureRepoEvent(e.selfId, e.groupId, owner, repo, platform, events)
const parts: string[] = [`平台: ${platform}`, `仓库: ${owner}/${repo}`, `订阅类型: ${events.join(',')}`]
if (events.includes(EventType.Push)) {
const branch = branchArg || (await client.getRepoInfo(owner, repo)).defaultBranch
parts.push(await subscribePush(eventId, branch, client))
}
if (events.includes(EventType.Issue)) {
parts.push(await subscribeIssue(eventId, owner, repo, client))
}
await e.reply(`添加订阅仓库成功，` + parts.join('，'))
})
```
在 `RemoveRepo` 中应用相同的模式。这使得命令处理程序专注于协调，而辅助函数则负责繁重的工作。
</issue_to_address>
### Comment 7
<location> `src/apps/issue.ts:14` </location>
<code_context>
+ logger,
+} from 'node-karin'
+
+export const github = karin.task(
+ 'karin-plugin-git:issue:github',
+ Config.github.cron || '0 */5 * * * *',
</code_context>
<issue_to_address>
**issue (complexity):** 考虑重构代码，将任务注册和议题分组逻辑整合到可重用函数中，以提高可维护性。
```typescript
// 1) 消除任务注册中的重复代码 (DRY up task registration)
const ISSUE_TASKS = [
{ name: 'github', clientFn: Client.github, cfg: Config.github, platform: Platform.GitHub },
{ name: 'gitee', clientFn: Client.gitee, cfg: Config.gitee, platform: Platform.Gitee },
{ name: 'gitcode',clientFn: Client.gitcode,cfg: Config.gitcode,platform: Platform.GitCode},
{ name: 'cnb', clientFn: Client.cnb, cfg: Config.cnb, platform: Platform.Cnb },
] as const;
for (const { name, clientFn, cfg, platform } of ISSUE_TASKS) {
karin.task(
`karin-plugin-git:issue:${name}`,
cfg.cron || '0 */5 * * * *',
async () => {
if (isEmpty(cfg.token)) {
return logger.warn(`${name} token 未配置，跳过`);
}
try {
await handleRepoIssue(clientFn(), platform);
} catch (e) {
logger.error(e);
}
},
);
}
// 2) 将获取 + 分组提取到单独的函数中 (Extract fetching + grouping into its own function)
type IssuePayload = {
owner: string;
repo: string;
title: string;
body: string | null;
user: IssueUserInfo;
state: StateType;
issueDate: string;
};
async function collectIssueGroups(
events: Array<{ id: string; repoId: string }>,
client: ClientType,
): Promise<Map<string, IssuePayload[]>> {
const map = new Map<string, IssuePayload[]>();
for (const ev of events) {
const repo = await db.repo.GetRepo(ev.repoId);
if (!repo) continue;
const key = `${repo.groupId}-${repo.botId}`;
map.set(key, map.get(key) ?? []);
const list = await client.getIssueList(repo.owner, repo.repo, { perPage: 100 });
for (const issue of list) {
const current = await db.issue.GetRepo(ev.id, issue.number);
const hashTitle = make_hash(issue.title);
const hashBody = issue.body ? make_hash(issue.body) : null;
const changed =
!current ||
current.state !== issue.state ||
current.title !== hashTitle ||
current.body !== hashBody;
if (!current) {
await db.issue.AddRepo(ev.id, issue.number, hashTitle, hashBody, issue.state);
}
if (changed) {
map.get(key)!.push({
owner: repo.owner,
repo: repo.repo,
title: await Render.markdown(issue.title),
body: issue.body ? await Render.markdown(issue.body) : null,
user: issue.user,
state: issue.state,
issueDate: formatDate(issue.createdAt),
});
if (current) {
await db.issue.UpdateState(ev.id, issue.number, issue.state);
}
}
}
}
return map;
}
// 3) 简化 handleRepoIssue (Simplify handleRepoIssue)
async function handleRepoIssue(client: ClientType, platform: Platform) {
const events = await db.event.GetAll(platform, EventType.Push);
const groups = await collectIssueGroups(events, client);
for (const [key, issues] of groups) {
const [groupId, botId] = key.split('-');
if (issues.length === 0) continue;
const images = await Promise.all(
issues.map((issue) => Render.render('issue/index', { issue })),
);
await sendImage(botId, groupId, images);
}
}
```
这分解了：
- 任务注册循环
- `collectIssueGroups` 用于获取、哈希、数据库更新
- 一个精简的 `handleRepoIssue`，只负责连接各项功能。
</issue_to_address>
### Comment 8
<location> `src/models/db/event.ts:17` </location>
<code_context>
+ )
+}
+
+export async function GetAll(platform: Platform): Promise<Array<EventRepo>>
+
+export async function GetAll(
</code_context>
<issue_to_address>
**issue (complexity):** 考虑提取一个辅助函数用于查询和 eventType 解析，以简化 GetAll 和 GetRepo 中重复的逻辑并使其集中化。
```markdown
我建议提取一个单独的辅助函数来封装 `client.all` + `eventType` 分割，然后将 `GetAll`/`GetRepo` 简化为各几行代码：
```ts
// event.ts
async function queryEvents(
client: sqlite3.Database,
sql: string,
params: unknown[],
): Promise<EventRepo[]> {
const rows: any[] = await new Promise((res, rej) =>
client.all(sql, params, (err, rows) => (err ? rej(err) : res(rows))),
);
return rows.map(r => ({
...r,
eventType: (r.eventType as string).split(',') as EventType[],
}));
}
export async function GetAll(
platform?: Platform,
eventType?: EventType,
): Promise<EventRepo[]> {
const client = await createClient();
const clauses: string[] = [];
const params: unknown[] = [];
if (platform) {
clauses.push('platform = ?');
params.push(platform);
}
if (eventType) {
clauses.push('eventType = ?');
params.push(eventType);
}
const where = clauses.length ? ' WHERE ' + clauses.join(' AND ') : '';
return queryEvents(client, `SELECT * FROM event${where}`, params);
}
export async function GetRepo(
platform: Platform,
repoId: number,
eventType?: EventType | EventType[],
): Promise<EventRepo | EventRepo[] | null> {
const client = await createClient();
// add repoId clause first
const clauses = ['platform = ?', 'repoId = ?'];
const params: unknown[] = [platform, repoId];
if (eventType) {
const evts = Array.isArray(eventType) ? eventType : [eventType];
// for simplicity, do server‐side LIKE or IN if your DB supports it,
// otherwise fetch all and filter in‐memory:
const rows = await queryEvents(
client,
`SELECT * FROM event WHERE ${clauses.join(' AND ')}`,
params,
);
const filtered = rows.filter(r => evts.some(e => r.eventType.includes(e)));
return Array.isArray(eventType) ? filtered : filtered[0] || null;
}
// no eventType => return all matching repo
return queryEvents(
client,
`SELECT * FROM event WHERE ${clauses.join(' AND ')}`,
params,
);
}
```
- 移除了所有重复的 `new Promise` 样板代码
- 将 `eventType.split(',')` 集中在一处
- 将重载合并为可选参数 + 简单的保护子句
- 保持了相同的功能和 SQL 覆盖范围。
</issue_to_address>
### Comment 9
<location> `src/models/db/issue.ts:7` </location>
<code_context>
+import { EventRepo, EventType, Platform } from '@/types'
+import { createClient } from './index'
+
+export async function AddRepo(
+ platform: Platform,
+ repoId: number,
</code_context>
<issue_to_address>
**issue (complexity):** 考虑将重载函数重构为针对每个用例的单独、命名清晰的函数，以避免基于可选参数进行分支。
考虑将每个重载拆分为其自己的命名清晰的函数，这样就不会有基于可选参数的分支：
```ts
// issue.ts
export async function addEvent(eventId: number): Promise<void> {
const client = await createClient()
await client.run(
'INSERT INTO issue (eventId) VALUES (?)',
[eventId],
)
}
export async function addIssueDetail(
eventId: number,
issueId: string,
title: string,
body: string | null,
state: StateType,
): Promise<void> {
const client = await createClient()
await client.run(
'INSERT INTO issue (eventId, issueId, title, body, state) VALUES (?, ?, ?, ?, ?)',
[eventId, issueId, title, body, state],
)
}
```
```ts
// fetching
export async function getIssuesByEvent(eventId: number): Promise<IssueRepo[]> {
const client = await createClient()
return new Promise((res, rej) =>
client.all('SELECT * FROM issue WHERE eventId = ?', [eventId], (e, rows) =>
e ? rej(e) : res(rows as IssueRepo[]),
),
)
}
export async function getIssue(
eventId: number,
issueId: string,
): Promise<IssueRepo | null> {
const client = await createClient()
return new Promise((res, rej) =>
client.get(
'SELECT * FROM issue WHERE eventId = ? AND issueId = ?',
[eventId, issueId],
(e, row) => (e ? rej(e) : res(row as IssueRepo | null)),
),
)
}
```
```ts
// deleting
export async function removeIssuesByEvent(eventId: number): Promise<void> {
const client = await createClient()
await client.run('DELETE FROM issue WHERE eventId = ?', [eventId])
}
export async function removeIssue(
eventId: number,
issueId: string,
): Promise<void> {
const client = await createClient()
await client.run(
'DELETE FROM issue WHERE eventId = ? AND issueId = ?',
[eventId, issueId],
)
}
```
这消除了重载，并使每个函数都具有单一用途和自文档化。
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/apps/issue.ts:75` </location>
<code_context>
+)
+
+const handleRepoIssue = async (client: ClientType, platform: Platform) => {
+ const all = await db.event.GetAll(platform, EventType.Push)
+ const groupMap = new Map<
+ string,
</code_context>
<issue_to_address>
**issue (bug_risk):** EventType.Push used for issue event retrieval may be incorrect.
Consider updating the event retrieval in handleRepoIssue to use EventType.Issue to ensure relevant issue events are processed.
</issue_to_address>
### Comment 2
<location> `src/models/db/event.ts:32-33` </location>
<code_context>
+): Promise<Array<EventRepo>> {
+ let client = await createClient()
+ let result: Array<EventRepo>
+ if (platform) {
+ result = await new Promise<Array<EventRepo>>((resolve, reject) => {
+ client.all(
+ 'SELECT * FROM event WHERE platform = ?',
</code_context>
<issue_to_address>
**suggestion (bug_risk):** EventType filtering logic may not work as intended.
Currently, when both platform and eventType are specified, only platform is used for filtering. Please update the logic to support filtering by both parameters when provided.
</issue_to_address>
### Comment 3
<location> `src/models/db/event.ts:165` </location>
<code_context>
+ await new Promise<void>((resolve, reject) => {
+ client.run(
+ 'UPDATE event SET eventType = ? WHERE platform = ? AND repoId = ?',
+ [event, platform, repoId],
+ (err) => {
+ if (err) reject(err)
</code_context>
<issue_to_address>
**issue (bug_risk):** EventType array is passed directly to SQL, which may serialize incorrectly.
In UpdateEventType, join the event array into a comma-separated string before passing it as a parameter to prevent type mismatches with the SQL storage format.
</issue_to_address>
### Comment 4
<location> `src/apps/admin.ts:12-14` </location>
<code_context>
- const [, platform, owner, repo, branch] = e.msg.match(AddRepo!.reg)!
+ const [, platform, owner, repo, branch, event] = e.msg.match(AddRepo!.reg)!
+
+ const eventType = event
+ ? (event.toLocaleLowerCase().split(',') as Array<EventType>)
+ : [EventType.Push]
</code_context>
<issue_to_address>
**suggestion:** eventType parsing may produce empty array if event is empty string.
If event is an empty string, eventType will be [''], which won't match any valid EventType. Filter out empty strings or default to [EventType.Push] when event is empty.
```suggestion
const eventType = event
? (
event
.toLocaleLowerCase()
.split(',')
.map(e => e.trim())
.filter(e => e) as Array<EventType>
)
: [EventType.Push]
// If eventType is empty after filtering, default to [EventType.Push]
if (!eventType.length) {
eventType.push(EventType.Push)
}
```
</issue_to_address>
### Comment 5
<location> `src/apps/admin.ts:139` </location>
<code_context>
}
- await db.push.RemoveRepo(platformName, repoInfo.id, PushBranch)
+ if (eventType.includes(EventType.Issue)) {
+ const issueRepo = await db.issue.GetRepo(repoInfo.id)
+ if (!issueRepo) {
+ return await e.reply('议题订阅不存在，删除失败')
</code_context>
<issue_to_address>
**question:** Inconsistent eventId usage for issueRepo retrieval.
Please verify that using repoInfo.id here is correct and consistent with other usages of eventInfo.id.
</issue_to_address>
### Comment 6
<location> `src/apps/admin.ts:7` </location>
<code_context>
+import { isEmpty } from 'es-toolkit/compat'
import karin from 'node-karin'
export const AddRepo = karin.command(
- /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?$/i,
+ /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?\s+([^/\s]+)?$/i,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the command handlers by extracting platform/client resolution, repo/event record management, and event subscription logic into helper functions for better modularity.
```suggestion
Both `AddRepo` and `RemoveRepo` are doing the same three things in a single function:
1) resolve platform→client,
2) ensure repo & event records,
3) subscribe/unsubscribe each event type.
You can split each of those into small helpers—then your command handler is just orchestration:
1. Extract platform + client resolution
2. Extract “ensure repo” and “ensure event” logic
3. Extract push/issue subscribe + unsubscribe
Example:
```ts
// 1) resolve platform + client
function getPlatformClient(p?: string): { platform: Platform, client: ClientType } {
switch (p?.toLowerCase()) {
case 'gitcode': return { platform: Platform.GitCode, client: Client.gitcode() }
case 'gitee': return { platform: Platform.Gitee, client: Client.gitee() }
case 'cnb': return { platform: Platform.Cnb, client: Client.cnb() }
default: return { platform: Platform.GitHub, client: Client.github() }
}
}
// 2) ensure repo + event records
async function ensureRepoEvent(
botId: number, groupId: number,
owner: string, repo: string,
platform: Platform, events: EventType[]
) {
let info = await db.repo.GetRepo(botId, groupId, owner, repo)
if (!info) {
await db.repo.AddRepo(botId, groupId, owner, repo)
info = await db.repo.GetRepo(botId, groupId, owner, repo)!
}
let ev = await db.event.GetRepo(platform, info.id, events)
if (!ev) {
await db.event.AddRepo(platform, info.id, events)
ev = await db.event.GetRepo(platform, info.id, events)!
} else {
await db.event.UpdateEventType(platform, info.id, events)
}
return { repoId: info.id, eventId: ev!.id }
}
// 3a) push subscription
async function subscribePush(eventId: number, branch: string, client: ClientType) {
const existing = await db.push.GetRepo(eventId, branch)
if (!existing) {
await db.push.AddRepo(eventId, branch)
return `分支: ${branch}`
} else {
return `推送订阅已存在，分支: ${branch}`
}
}
// 3b) issue subscription
async function subscribeIssue(eventId: number, owner: string, repo: string, client: ClientType) {
const existing = await db.issue.GetRepo(eventId)
if (existing.length === 0) {
const list = await client.getIssueList(owner, repo, { perPage: 100 })
for (const i of list) {
await db.issue.AddRepo(eventId, i.number, make_hash(i.title), i.body && make_hash(i.body), i.state)
}
return `议题 ${list.length} 条已订阅`
} else {
return `议题订阅已存在`
}
}
// In your command:
export const AddRepo = karin.command(/^…$/, async e => {
const [, platformArg, owner, repo, branchArg, evtArg] = e.msg.match(AddRepo!.reg)!
const events = evtArg?.toLowerCase().split(',') as EventType[] ?? [EventType.Push]
const { platform, client } = getPlatformClient(platformArg)
const { repoId, eventId } = await ensureRepoEvent(e.selfId, e.groupId, owner, repo, platform, events)
const parts: string[] = [`平台: ${platform}`, `仓库: ${owner}/${repo}`, `订阅类型: ${events.join(',')}`]
if (events.includes(EventType.Push)) {
const branch = branchArg || (await client.getRepoInfo(owner, repo)).defaultBranch
parts.push(await subscribePush(eventId, branch, client))
}
if (events.includes(EventType.Issue)) {
parts.push(await subscribeIssue(eventId, owner, repo, client))
}
await e.reply(`添加订阅仓库成功，` + parts.join('，'))
})
```
Apply the same pattern in `RemoveRepo`. This keeps the command handler focused on orchestration while the helpers do the heavy lifting.
</issue_to_address>
### Comment 7
<location> `src/apps/issue.ts:14` </location>
<code_context>
+ logger,
+} from 'node-karin'
+
+export const github = karin.task(
+ 'karin-plugin-git:issue:github',
+ Config.github.cron || '0 */5 * * * *',
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the code to consolidate task registration and issue grouping logic into reusable functions for improved maintainability.
```typescript
// 1) DRY up task registration
const ISSUE_TASKS = [
{ name: 'github', clientFn: Client.github, cfg: Config.github, platform: Platform.GitHub },
{ name: 'gitee', clientFn: Client.gitee, cfg: Config.gitee, platform: Platform.Gitee },
{ name: 'gitcode',clientFn: Client.gitcode,cfg: Config.gitcode,platform: Platform.GitCode},
{ name: 'cnb', clientFn: Client.cnb, cfg: Config.cnb, platform: Platform.Cnb },
] as const;
for (const { name, clientFn, cfg, platform } of ISSUE_TASKS) {
karin.task(
`karin-plugin-git:issue:${name}`,
cfg.cron || '0 */5 * * * *',
async () => {
if (isEmpty(cfg.token)) {
return logger.warn(`${name} token not configured, skipping`);
}
try {
await handleRepoIssue(clientFn(), platform);
} catch (e) {
logger.error(e);
}
},
);
}
// 2) Extract fetching + grouping into its own function
type IssuePayload = {
owner: string;
repo: string;
title: string;
body: string | null;
user: IssueUserInfo;
state: StateType;
issueDate: string;
};
async function collectIssueGroups(
events: Array<{ id: string; repoId: string }>,
client: ClientType,
): Promise<Map<string, IssuePayload[]>> {
const map = new Map<string, IssuePayload[]>();
for (const ev of events) {
const repo = await db.repo.GetRepo(ev.repoId);
if (!repo) continue;
const key = `${repo.groupId}-${repo.botId}`;
map.set(key, map.get(key) ?? []);
const list = await client.getIssueList(repo.owner, repo.repo, { perPage: 100 });
for (const issue of list) {
const current = await db.issue.GetRepo(ev.id, issue.number);
const hashTitle = make_hash(issue.title);
const hashBody = issue.body ? make_hash(issue.body) : null;
const changed =
!current ||
current.state !== issue.state ||
current.title !== hashTitle ||
current.body !== hashBody;
if (!current) {
await db.issue.AddRepo(ev.id, issue.number, hashTitle, hashBody, issue.state);
}
if (changed) {
map.get(key)!.push({
owner: repo.owner,
repo: repo.repo,
title: await Render.markdown(issue.title),
body: issue.body ? await Render.markdown(issue.body) : null,
user: issue.user,
state: issue.state,
issueDate: formatDate(issue.createdAt),
});
if (current) {
await db.issue.UpdateState(ev.id, issue.number, issue.state);
}
}
}
}
return map;
}
// 3) Simplify handleRepoIssue
async function handleRepoIssue(client: ClientType, platform: Platform) {
const events = await db.event.GetAll(platform, EventType.Push);
const groups = await collectIssueGroups(events, client);
for (const [key, issues] of groups) {
const [groupId, botId] = key.split('-');
if (issues.length === 0) continue;
const images = await Promise.all(
issues.map((issue) => Render.render('issue/index', { issue })),
);
await sendImage(botId, groupId, images);
}
}
```
This factors out:
- task registration loop
- `collectIssueGroups` for fetching, hashing, DB updates
- a lean `handleRepoIssue` that just wires things up.
</issue_to_address>
### Comment 8
<location> `src/models/db/event.ts:17` </location>
<code_context>
+ )
+}
+
+export async function GetAll(platform: Platform): Promise<Array<EventRepo>>
+
+export async function GetAll(
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting a helper for querying and eventType parsing to simplify and centralize repeated logic in GetAll and GetRepo.
```markdown
I’d extract a single helper to wrap `client.all` + the `eventType` splitting, then reduce `GetAll`/`GetRepo` to a couple of lines each:
```ts
// event.ts
async function queryEvents(
client: sqlite3.Database,
sql: string,
params: unknown[],
): Promise<EventRepo[]> {
const rows: any[] = await new Promise((res, rej) =>
client.all(sql, params, (err, rows) => (err ? rej(err) : res(rows))),
);
return rows.map(r => ({
...r,
eventType: (r.eventType as string).split(',') as EventType[],
}));
}
export async function GetAll(
platform?: Platform,
eventType?: EventType,
): Promise<EventRepo[]> {
const client = await createClient();
const clauses: string[] = [];
const params: unknown[] = [];
if (platform) {
clauses.push('platform = ?');
params.push(platform);
}
if (eventType) {
clauses.push('eventType = ?');
params.push(eventType);
}
const where = clauses.length ? ' WHERE ' + clauses.join(' AND ') : '';
return queryEvents(client, `SELECT * FROM event${where}`, params);
}
export async function GetRepo(
platform: Platform,
repoId: number,
eventType?: EventType | EventType[],
): Promise<EventRepo | EventRepo[] | null> {
const client = await createClient();
// add repoId clause first
const clauses = ['platform = ?', 'repoId = ?'];
const params: unknown[] = [platform, repoId];
if (eventType) {
const evts = Array.isArray(eventType) ? eventType : [eventType];
// for simplicity, do server‐side LIKE or IN if your DB supports it,
// otherwise fetch all and filter in‐memory:
const rows = await queryEvents(
client,
`SELECT * FROM event WHERE ${clauses.join(' AND ')}`,
params,
);
const filtered = rows.filter(r => evts.some(e => r.eventType.includes(e)));
return Array.isArray(eventType) ? filtered : filtered[0] || null;
}
// no eventType => return all matching repo
return queryEvents(
client,
`SELECT * FROM event WHERE ${clauses.join(' AND ')}`,
params,
);
}
```
- Removes all the repeated `new Promise` boilerplate
- Centralizes `eventType.split(',')` in one place
- Collapses overloads into optional args + simple guard clauses
- Keeps identical functionality and SQL coverage.
</issue_to_address>
### Comment 9
<location> `src/models/db/issue.ts:7` </location>
<code_context>
+import { EventRepo, EventType, Platform } from '@/types'
+import { createClient } from './index'
+
+export async function AddRepo(
+ platform: Platform,
+ repoId: number,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring overloaded functions into separate, clearly-named functions for each use case to avoid branching on optional arguments.
Consider splitting each overload into its own clearly‐named function so there’s no branching on optional args:
```ts
// issue.ts
export async function addEvent(eventId: number): Promise<void> {
const client = await createClient()
await client.run(
'INSERT INTO issue (eventId) VALUES (?)',
[eventId],
)
}
export async function addIssueDetail(
eventId: number,
issueId: string,
title: string,
body: string | null,
state: StateType,
): Promise<void> {
const client = await createClient()
await client.run(
'INSERT INTO issue (eventId, issueId, title, body, state) VALUES (?, ?, ?, ?, ?)',
[eventId, issueId, title, body, state],
)
}
```
```ts
// fetching
export async function getIssuesByEvent(eventId: number): Promise<IssueRepo[]> {
const client = await createClient()
return new Promise((res, rej) =>
client.all('SELECT * FROM issue WHERE eventId = ?', [eventId], (e, rows) =>
e ? rej(e) : res(rows as IssueRepo[]),
),
)
}
export async function getIssue(
eventId: number,
issueId: string,
): Promise<IssueRepo | null> {
const client = await createClient()
return new Promise((res, rej) =>
client.get(
'SELECT * FROM issue WHERE eventId = ? AND issueId = ?',
[eventId, issueId],
(e, row) => (e ? rej(e) : res(row as IssueRepo | null)),
),
)
}
```
```ts
// deleting
export async function removeIssuesByEvent(eventId: number): Promise<void> {
const client = await createClient()
await client.run('DELETE FROM issue WHERE eventId = ?', [eventId])
}
export async function removeIssue(
eventId: number,
issueId: string,
): Promise<void> {
const client = await createClient()
await client.run(
'DELETE FROM issue WHERE eventId = ? AND issueId = ?',
[eventId, issueId],
)
}
```
This eliminates overloads and keeps each function single‐purpose and self‐documenting.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/models/db/event.ts
Outdated
Comment on lines
32
to
33
| if (platform) { | ||
| result = await new Promise<Array<EventRepo>>((resolve, reject) => { |
There was a problem hiding this comment.
suggestion (bug_risk): EventType 过滤逻辑可能无法按预期工作。
目前，当同时指定 platform 和 eventType 时，仅使用 platform 进行过滤。请更新逻辑以在提供这两个参数时支持按两者进行过滤。
Original comment in English
suggestion (bug_risk): EventType filtering logic may not work as intended.
Currently, when both platform and eventType are specified, only platform is used for filtering. Please update the logic to support filtering by both parameters when provided.
- 修改数据库表结构，将 push 和 issue 表关联到 event 表 - 更新事件类型处理逻辑，支持多事件订阅 - 重构添加/删除仓库订阅功能，提高代码可读性 - 增加对空仓库议题列表的处理 - 修复推送和议题订阅的查询逻辑 - 调整数据库索引以提升查询性能 - 修正命令参数默认值设置问题 - 完善错误提示信息，增强用户体验
Contributor
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The SQL filters in event.GetAll and event.GetRepo use exact matches on the comma‐separated eventType string, which will miss repos subscribed to multiple events—consider switching to LIKE or a normalized junction table to correctly include multi‐event subscriptions.
- The AddRepo/RemoveRepo regex and positional parsing of branch vs. event can get ambiguous when one or the other is omitted; consider switching to explicit flags (e.g.
--branchand--event) or reordering arguments to avoid misparses. - Push and issue task handlers share a lot of logic—extract common steps (e.g. grouping by bot/group, rendering, sending) into reusable helpers to reduce duplication and ease future changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The SQL filters in event.GetAll and event.GetRepo use exact matches on the comma‐separated eventType string, which will miss repos subscribed to multiple events—consider switching to LIKE or a normalized junction table to correctly include multi‐event subscriptions.
- The AddRepo/RemoveRepo regex and positional parsing of branch vs. event can get ambiguous when one or the other is omitted; consider switching to explicit flags (e.g. `--branch` and `--event`) or reordering arguments to avoid misparses.
- Push and issue task handlers share a lot of logic—extract common steps (e.g. grouping by bot/group, rendering, sending) into reusable helpers to reduce duplication and ease future changes.
## Individual Comments
### Comment 1
<location> `src/apps/admin.ts:60` </location>
<code_context>
+ return await e.reply('添加仓库订阅事件失败，请重试')
+ }
+
+ let msg = `添加订阅仓库成功, 平台: ${platformName}, 仓库: ${owner}/${repo}, 订阅类型: ${event}`
+
+ const PushEvent = eventType.includes(EventType.Push)
</code_context>
<issue_to_address>
**suggestion:** Message may not reflect actual eventType array.
Use eventType.join(',') instead of the raw 'event' string to ensure the message accurately reflects the normalized event types.
```suggestion
let msg = `添加订阅仓库成功, 平台: ${platformName}, 仓库: ${owner}/${repo}, 订阅类型: ${eventType.join(',')}`
```
</issue_to_address>
### Comment 2
<location> `src/apps/push.ts:84` </location>
<code_context>
const botId = e.selfId
const groupId = e.groupId
- const PushInfos = await db.push.GetAll()
+ const all = await db.event.GetAll()
- let client: ClientType
</code_context>
<issue_to_address>
**suggestion (performance):** No filtering by platform or event type in push command.
Querying all events without filters can lead to processing irrelevant data. Please add platform and event type filters to optimize efficiency.
Suggested implementation:
```typescript
// Filter events by platform and event type for efficiency
const platform = Platform.Cnb // or use the relevant platform variable
const eventType = 'push' // or use the relevant event type variable
const all = await db.event.GetAll({ platform, eventType })
```
Make sure that `db.event.GetAll` supports filtering by platform and event type via its parameters. If it does not, you will need to update its implementation to accept and use these filters.
</issue_to_address>
### Comment 3
<location> `src/apps/push.ts:111-120` </location>
<code_context>
+ for (const pushInfo of pushRepoList) {
</code_context>
<issue_to_address>
**suggestion:** No error handling for Render.render failures.
Add error handling to catch exceptions or validate the output of Render.render before adding images to the array.
</issue_to_address>
### Comment 4
<location> `src/apps/push.ts:145-142` </location>
<code_context>
+ for (const issue of issueRepoList) {
</code_context>
<issue_to_address>
**suggestion (performance):** Potential for unnecessary API calls for empty issues.
Filter out empty issues before making API requests to avoid unnecessary calls and reduce system load.
</issue_to_address>
### Comment 5
<location> `src/apps/push.ts:88-97` </location>
<code_context>
+ }>
+ >()
+
+ for (const event of all) {
+ const eventRepoInfo = await db.repo.GetRepo(event.repoId)
+ if (!eventRepoInfo) continue
</code_context>
<issue_to_address>
**issue (bug_risk):** Possible bug in pushRepoList fallback logic.
Ensure that the repo added when pushRepoList is empty uses the same identifier as the one used for fetching (event.repoId) to avoid inconsistencies.
</issue_to_address>
### Comment 6
<location> `src/models/db/push.ts:24-33` </location>
<code_context>
+ return result
+}
+
+export async function GetRepo(
+ platform: Platform,
+ repoId: number,
+): Promise<Array<EventRepo>>
+
+export async function GetRepo(
+ platform: Platform,
+ repoId: number,
+ eventType: Array<EventType> | EventType,
+): Promise<EventRepo | null>
+export async function GetRepo(
+ platform: Platform,
+ repoId: number,
+ eventType?: Array<EventType> | EventType,
+): Promise<Array<EventRepo> | EventRepo | null> {
+ let client = await createClient()
+
+ if (eventType) {
+ const result = await new Promise<Array<EventRepo>>((resolve, reject) => {
+ client.all(
+ 'SELECT * FROM event WHERE platform = ? AND repoId = ?',
+ [platform, repoId],
+ (err, rows) => {
+ if (err) reject(err)
+ else resolve(rows as Array<EventRepo>)
+ },
+ )
+ })
+
+ if (result && result.length > 0) {
</code_context>
<issue_to_address>
**suggestion:** Overloaded GetRepo signature may cause confusion.
Overloading GetRepo with different return types based on arguments can introduce ambiguity and runtime errors. Recommend using distinct function names or ensuring a consistent return type to improve clarity and safety.
</issue_to_address>
### Comment 7
<location> `src/apps/issue.ts:75` </location>
<code_context>
+)
+
+const handleRepoIssue = async (client: ClientType, platform: Platform) => {
+ const all = await db.event.GetAll(platform, EventType.Push)
+ const groupMap = new Map<
+ string,
</code_context>
<issue_to_address>
**issue:** EventType.Push used for issue handling may be incorrect.
Consider fetching events with EventType.Issue, or both Push and Issue, to ensure all relevant issue events are handled.
</issue_to_address>
### Comment 8
<location> `src/apps/admin.ts:7` </location>
<code_context>
+import { isEmpty } from 'es-toolkit/compat'
import karin from 'node-karin'
export const AddRepo = karin.command(
- /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?$/i,
+ /^#?git(?:添加|add)([^\s]+)?订阅仓库([^/\s]+)\/([^:\s]+)(?::([^/\s]+))?\s+([^/\s]+)?$/i,
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting repeated logic in the handlers into small helper functions to flatten and simplify the code structure.
```markdown
You can dramatically flatten both handlers by extracting the repeated bits into small helpers. For example:
1) helpers/platform.ts
```ts
import { Client, Config } from '@/common'
import { Platform, ClientType } from '@/types'
const PLATFORM_MAP: Record<string, { name: Platform; client: ClientType }> = {
gitcode: { name: Platform.GitCode, client: Client.gitcode() },
gitee: { name: Platform.Gitee, client: Client.gitee() },
cnb: { name: Platform.Cnb, client: Client.cnb() },
github: { name: Platform.GitHub, client: Client.github() },
}
export function parsePlatform(raw?: string) {
const key = raw?.toLowerCase() || 'github'
return PLATFORM_MAP[key] || PLATFORM_MAP['github']
}
```
2) helpers/events.ts
```ts
import { EventType } from '@/types'
export function parseEventTypes(raw?: string): EventType[] {
if (!raw) return [EventType.Push]
const events = raw
.toLowerCase()
.split(',')
.map(e => e.trim())
.filter(Boolean) as EventType[]
return events.length ? events : [EventType.Push]
}
```
3) helpers/repo.ts
```ts
import { db } from '@/models'
export async function getOrCreateRepo(
botId: number, groupId: number,
owner: string, repo: string
) {
let info = await db.repo.GetRepo(botId, groupId, owner, repo)
if (!info) {
await db.repo.AddRepo(botId, groupId, owner, repo)
info = await db.repo.GetRepo(botId, groupId, owner, repo)
}
if (!info) throw new Error('repo')
return info
}
```
4) In your `AddRepo` you then reduce nesting to:
```ts
import { parsePlatform } from './helpers/platform'
import { parseEventTypes } from './helpers/events'
import { getOrCreateRepo } from './helpers/repo'
export const AddRepo = karin.command(
/^…$/,
async e => {
const [, rawPlat, owner, repo, branch, rawEvt] = e.msg.match(AddRepo.reg)!
const { name: platformName, client } = parsePlatform(rawPlat)
const eventTypes = parseEventTypes(rawEvt)
const repoInfo = await getOrCreateRepo(e.selfId, e.groupId, owner, repo)
// … similarly extract event‐table upsert, pushSubscribe, issueSubscribe
// build `msg` and just `await e.reply(msg)`
},
{ … }
)
```
Repeat the same pattern in `RemoveRepo`. This keeps all logic, removes deep nesting and makes each step easy to read and test.
</issue_to_address>
### Comment 9
<location> `src/apps/push.ts:87` </location>
<code_context>
+
+ for (const [groupKey, issues] of groupMap) {
+ const [groupId, botId] = groupKey.split('-')
+ let image: ImageElement[] = []
+
+ for (const issue of issues) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring client selection and rendering logic into helper functions and batch asynchronous operations to simplify control flow.
- Extract client‐selection into a helper
- Pull “split & render” logic for commits/issues into small reusable functions
- Batch your renders with `Promise.all` and avoid inline loops/awaits
Example helpers you can introduce:
```ts
function getClientByPlatform(platform: Platform): ClientType {
switch (platform) {
case Platform.Gitee:
if (isEmpty(Config.gitee.token)) throw new Error('请先配置Gitee Token')
return Client.gitee()
case Platform.GitCode:
if (isEmpty(Config.gitcode.token)) throw new Error('请先配置GitCode Token')
return Client.gitcode()
case Platform.Cnb:
if (isEmpty(Config.cnb.token)) throw new Error('请先配置Cnb Token')
return Client.cnb()
default:
if (isEmpty(Config.github.token)) throw new Error('请先配置GitHub Token')
return Client.github()
}
}
```
```ts
async function renderCommitImage(
item: { commitInfo: CommitInfo; pushRepoInfo: RepoInfo; pushRepo: PushRepo }
): Promise<ImageElement> {
const parts = item.commitInfo.commit.message.split('\n')
const data: PushCommitInfo = {
...item.commitInfo,
owner: item.pushRepoInfo.owner,
repo: item.pushRepoInfo.repo,
branch: item.pushRepo.branch,
botId: item.pushRepoInfo.botId,
groupId: item.pushRepoInfo.groupId,
title: await Render.markdown(parts[0]),
body: await Render.markdown(parts.slice(1).join('\n')),
commitDate: formatDate(item.commitInfo.commit.committer.date),
}
return Render.render('commit/index', { commit: data })
}
async function renderIssueImage(
issue: IssueType, // replace with your Issue interface
repoInfo: RepoInfo
): Promise<ImageElement> {
const data = {
owner: repoInfo.owner,
repo: repoInfo.repo,
title: await Render.markdown(issue.title),
body: issue.body ? await Render.markdown(issue.body) : null,
user: issue.user,
state: issue.state,
issueDate: formatDate(issue.createdAt),
}
return Render.render('issue/index', { issue: data })
}
```
Then in your command you can write:
```ts
const client = getClientByPlatform(event.platform)
const [repos, issues] = await Promise.all([
db.push.GetRepo(event.id),
db.issue.GetRepo(event.id)
])
const [commitImages, issueImages] = await Promise.all([
Promise.all(repos.map(r => renderCommitImage({ commitInfo: await client.getCommitInfo(...), pushRepoInfo: repoInfo, pushRepo: r }))),
Promise.all(issues.map(i => renderIssueImage(await client.getIssueInfo(...), repoInfo)))
])
const images = [...commitImages, ...issueImages]
if (images.length) await sendImage(botId, groupId, images)
```
This removes nested loops, consolidates repeated DB and render calls, and keeps all original behavior.
</issue_to_address>
### Comment 10
<location> `src/models/db/event.ts:17` </location>
<code_context>
+ )
+}
+
+export async function GetAll(platform: Platform): Promise<Array<EventRepo>>
+
+export async function GetAll(
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the API to use a single event listing function with helper utilities for SQL and CSV handling to simplify the code.
Here’s one way to collapse all of those overloads and manual CSV handling into a single, very‐readable API—while keeping every codepath intact:
1) Introduce a single `listEvents(filter?)` instead of four overloads.
2) Push SQL‐building into a tiny helper.
3) Centralize CSV joins/splits into `encodeTypes()` / `decodeTypes()`.
```ts
type EventFilter = {
platform?: Platform
eventType?: EventType
}
function encodeTypes(ev: EventType[]) {
return ev.join(',')
}
function decodeTypes(csv: string): EventType[] {
return csv.split(',') as EventType[]
}
async function listEvents(filter: EventFilter = {}): Promise<EventRepo[]> {
const client = await createClient()
const parts: string[] = ['SELECT * FROM event']
const params: (Platform | EventType)[] = []
if (filter.platform) {
parts.push('platform = ?')
params.push(filter.platform)
}
if (filter.eventType) {
parts.push(filter.platform ? 'AND eventType = ?' : 'eventType = ?')
params.push(filter.eventType)
}
const sql = parts.join(' ')
const rows: any[] = await clientAll(client, sql, params)
return rows.map(row => ({
...row,
eventType: decodeTypes(row.eventType)
}))
}
// helper to promisify client.all
function clientAll(client, sql: string, params: any[]) {
return new Promise<any[]>((res, rej) =>
client.all(sql, params, (e, r) => (e ? rej(e) : res(r)))
)
}
```
You can then implement every other method in terms of that:
```ts
export const GetAll = listEvents
export async function GetAllByPlatform(p: Platform) {
return listEvents({ platform: p })
}
export async function GetAllByType(e: EventType) {
return listEvents({ eventType: e })
}
// get a single repo
export async function GetRepo(
platform: Platform,
repoId: number,
): Promise<EventRepo[]> {
const rows = await listEvents({ platform })
return rows.filter(r => r.repoId === repoId)
}
// Insert / Update / Delete can share encodeTypes() likewise
export async function AddRepo(
platform: Platform,
repoId: number,
events: EventType[],
) {
const client = await createClient()
await clientRun(
client,
'INSERT INTO event (platform,repoId,eventType) VALUES (?,?,?)',
[platform, repoId, encodeTypes(events)],
)
}
```
— this removes *all* overloads, keeps CSV logic in two tiny helpers and centralizes SQL building.
</issue_to_address>
### Comment 11
<location> `src/models/db/issue.ts:7` </location>
<code_context>
+import { EventRepo, EventType, Platform } from '@/types'
+import { createClient } from './index'
+
+export async function AddRepo(
+ platform: Platform,
+ repoId: number,
</code_context>
<issue_to_address>
**issue (complexity):** Consider splitting overloaded and multi-purpose functions into single-purpose functions with clear names, and extract repeated promise-wrapping logic into helpers.
```suggestion
Rather than packing all behaviors into overloads of `AddRepo`, `GetRepo`, and `RemoveRepo`, split them into single‐purpose functions with clear names. Also extract the common promise‐wrapping of sqlite methods into helpers.
1. Create helpers once:
```ts
// db.ts
import { createClient } from './index';
import { promisify } from 'util';
export async function getDb() {
const client = await createClient();
return {
run: promisify(client.run.bind(client)),
get: promisify(client.get.bind(client)),
all: promisify(client.all.bind(client)),
};
}
```
2. Split AddRepo overloads to two functions:
```ts
// issue.ts
import { getDb } from './db';
import { StateType, IssueRepo } from '@/types';
export async function createIssueRepo(eventId: number): Promise<void> {
const { run } = await getDb();
await run('INSERT INTO issue(eventId) VALUES(?)', [eventId]);
}
export async function addIssue(
eventId: number,
issueId: string,
title: string,
body: string | null,
state: StateType
): Promise<void> {
const { run } = await getDb();
await run(
'INSERT INTO issue(eventId, issueId, title, body, state) VALUES(?,?,?,?,?)',
[eventId, issueId, title, body, state]
);
}
```
3. Split GetRepo into list/get:
```ts
export async function listIssues(eventId: number): Promise<IssueRepo[]> {
const { all } = await getDb();
return all('SELECT * FROM issue WHERE eventId = ?', [eventId]);
}
export async function getIssue(
eventId: number,
issueId: string
): Promise<IssueRepo | null> {
const { get } = await getDb();
return get(
'SELECT * FROM issue WHERE eventId = ? AND issueId = ?',
[eventId, issueId]
);
}
```
4. Split RemoveRepo into deleteAll/deleteOne:
```ts
export async function deleteAllIssues(eventId: number): Promise<void> {
const { run } = await getDb();
await run('DELETE FROM issue WHERE eventId = ?', [eventId]);
}
export async function deleteIssue(
eventId: number,
issueId: string
): Promise<void> {
const { run } = await getDb();
await run(
'DELETE FROM issue WHERE eventId = ? AND issueId = ?',
[eventId, issueId]
);
}
```
5. Keep UpdateState as is, or use `run` helper:
```ts
export async function updateIssueState(
eventId: number,
issueId: string,
state: StateType
): Promise<void> {
const { run } = await getDb();
await run(
'UPDATE issue SET state = ? WHERE eventId = ? AND issueId = ?',
[state, eventId, issueId]
);
}
```
These changes eliminate overloads/optional‐parameter branching and make each function single‐purpose while preserving all existing functionality.
</issue_to_address>
### Comment 12
<location> `src/apps/issue.ts:74` </location>
<code_context>
+ },
+)
+
+const handleRepoIssue = async (client: ClientType, platform: Platform) => {
+ const all = await db.event.GetAll(platform, EventType.Push)
+ const groupMap = new Map<
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring handleRepoIssue by splitting its logic into smaller helper functions for fetching, detecting, rendering, and sending issues.
```markdown
It looks like `handleRepoIssue` is doing 4 distinct jobs inline (fetch events & repos, detect new/changed issues, render images, send messages). Splitting those responsibilities into small helpers will make it far easier to test and maintain:
1) Extract “fetch & group issues”
2) Extract “detect new/changed issues per event”
3) Extract “render images”
4) Extract “send images”
Here’s a sketch:
```ts
// services/issueLoader.ts
import { db } from '@/models'
import { ClientType, Platform } from '@/types'
export interface IssueData { /* title, body, user, state, date… */ }
export async function loadAllPushEvents(platform: Platform) {
return db.event.GetAll(platform, EventType.Push)
}
export async function detectIssuesForEvent(
event: Event,
client: ClientType
): Promise<IssueData[]> {
const repo = await db.repo.GetRepo(event.repoId)
if (!repo) return []
const rawList = await client.getIssueList(repo.owner, repo.repo, { perPage: 100 })
const results: IssueData[] = []
for (const raw of rawList) {
const prev = await db.issue.GetRepo(event.id, raw.number)
const hashBody = raw.body ? make_hash(raw.body) : null
// first‐time or changed?
if (!prev || prev.state !== raw.state || prev.title !== make_hash(raw.title) || prev.body !== hashBody) {
if (!prev) {
await db.issue.AddRepo(event.id, raw.number, make_hash(raw.title), hashBody, raw.state)
} else {
await db.issue.UpdateState(event.id, raw.number, raw.state)
}
results.push({
owner: repo.owner,
repo: repo.repo,
title: await Render.markdown(raw.title),
body: raw.body ? await Render.markdown(raw.body) : null,
user: raw.user,
state: raw.state,
issueDate: formatDate(raw.createdAt),
})
}
}
return results
}
```
```ts
// services/issueNotifier.ts
import { ImageElement, getBot, common, contactGroup } from 'node-karin'
export async function renderImages(issues: IssueData[]): Promise<ImageElement[]> {
return Promise.all(
issues.map(issue => Render.render('issue/index', { issue }))
)
}
export async function sendImages(
botId: string,
groupId: string,
images: ImageElement[]
) {
const bot = getBot(botId)
const contact = await contactGroup(groupId)
if (images.length > 1) {
const forward = await common.makeForward(images, botId, bot?.account.name)
await bot?.sendForwardMsg(contact, forward, { /* ... */ })
} else {
await bot?.sendMsg(contact, images[0])
}
}
```
```ts
// tasks/issueTask.ts
import { loadAllPushEvents, detectIssuesForEvent } from './services/issueLoader'
import { renderImages, sendImages } from './services/issueNotifier'
async function handleRepoIssue(client: ClientType, platform: Platform) {
const events = await loadAllPushEvents(platform)
const groupMap = new Map<string, IssueData[]>()
for (const ev of events) {
const changes = await detectIssuesForEvent(ev, client)
const key = `${ev.groupId}-${ev.botId}`
if (changes.length) {
groupMap.set(key, (groupMap.get(key) || []).concat(changes))
}
}
for (const [key, issues] of groupMap) {
const [groupId, botId] = key.split('-')
const imgs = await renderImages(issues)
await sendImages(botId, groupId, imgs)
}
}
```
This preserves all functionality but:
- each function is < 40 lines
- you can write unit‐tests for each piece
- `handleRepoIssue` reads top‐to‐bottom like a clear flow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const botId = e.selfId | ||
| const groupId = e.groupId | ||
| const PushInfos = await db.push.GetAll() | ||
| const all = await db.event.GetAll() |
There was a problem hiding this comment.
suggestion (performance): No filtering by platform or event type in push command.
Querying all events without filters can lead to processing irrelevant data. Please add platform and event type filters to optimize efficiency.
Suggested implementation:
// Filter events by platform and event type for efficiency
const platform = Platform.Cnb // or use the relevant platform variable
const eventType = 'push' // or use the relevant event type variable
const all = await db.event.GetAll({ platform, eventType })Make sure that db.event.GetAll supports filtering by platform and event type via its parameters. If it does not, you will need to update its implementation to accept and use these filters.
Comment on lines
+111
to
+120
| for (const pushInfo of pushRepoList) { | ||
| let commitInfo: CommitInfo | ||
| try { | ||
| commitInfo = await client.getCommitInfo( | ||
| RepoInfo.owner, | ||
| RepoInfo.repo, | ||
| pushInfo.branch, | ||
| ) | ||
| } catch (error) { | ||
| logger.warn( |
There was a problem hiding this comment.
suggestion: No error handling for Render.render failures.
Add error handling to catch exceptions or validate the output of Render.render before adding images to the array.
| const img = await Render.render('commit/index', { | ||
| commit: pushCommitInfo, | ||
| }) | ||
| image.push(img) |
There was a problem hiding this comment.
suggestion (performance): Potential for unnecessary API calls for empty issues.
Filter out empty issues before making API requests to avoid unnecessary calls and reduce system load.
Comment on lines
29
to
+33
| export async function GetRepo( | ||
| platform: Platform, | ||
| repoId: number, | ||
| eventId: number, | ||
| branch: string, | ||
| ): Promise<PushRepo | null> { | ||
| ): Promise<PushRepo | null> | ||
|
|
||
| export async function GetRepo(eventId: number): Promise<Array<PushRepo>> | ||
|
|
||
| export async function GetRepo( | ||
| eventId: number, | ||
| branch?: string, |
There was a problem hiding this comment.
suggestion: Overloaded GetRepo signature may cause confusion.
Overloading GetRepo with different return types based on arguments can introduce ambiguity and runtime errors. Recommend using distinct function names or ensuring a consistent return type to improve clarity and safety.
- 合并重复的数据库查询代码 - 使用动态查询构建减少条件分支 - 支持同时按平台和事件类型过滤 - 统一结果处理逻辑
Contributor
|
🎉 🎉 🎉 构建完成！ 你可以通过以下命令安装该版本： |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sourcery 总结
通过扩展命令、数据库 schema、模型、任务和模板，支持订阅和推送仓库问题变更以及推送事件。
新功能：
错误修复：
改进：
formatDate和make_hash工具函数，用于日期格式化和内容哈希文档：
event参数杂项：
nipaw依赖项版本提升至^1.8.0Original summary in English
Sourcery 总结
通过扩展命令、数据库 schema 和计划任务，支持订阅仓库 issue 事件以及 push 事件，并增强用于渲染通知的工具和模板。
新功能：
错误修复：
改进：
构建：
文档：
Original summary in English
Sourcery 总结
实现一个统一的仓库推送和议题事件订阅系统：引入新的数据库 schema 和模型，扩展管理命令以管理多种订阅类型，并添加带模板的计划任务来渲染和分发通知。
新功能：
错误修复：
改进：
构建：
文档：
Original summary in English
Sourcery 总结
通过添加新命令、数据库表、计划任务、模板和实用工具，实现了问题订阅功能，同时支持推送通知；重构了基于事件订阅的模式和任务逻辑；修复了代理配置并升级了依赖。
新功能：
错误修复：
改进：
构建：
文档：
Original summary in English
Summary by Sourcery
Implement issue subscription alongside push notifications by adding new commands, database tables, scheduled tasks, templates, and utility tools; refactor schema and task logic for event-based subscriptions; fix proxy configuration and bump dependency.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation: