feat: add ConfigRouteManagerDialog component for managing routing configurations#5597
feat: add ConfigRouteManagerDialog component for managing routing configurations#5597
Conversation
…figurations - Implemented a new dialog component for managing routes associated with configurations, allowing users to view, delete, and save routes. - Enhanced the ProviderSelector component with improved styling for better readability. - Updated English and Chinese localization files to include new strings for the route manager and profile sidebar. - Refactored ConfigPage.vue to integrate the new route management dialog and improve layout responsiveness. - Added methods for handling route management, including fetching, saving, and removing routes.
dosubot
bot
added
the
size:XXL
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the configuration management capabilities by introducing dedicated components for managing routing configurations and improving the overall user interface for configuration profiles. It streamlines the process of associating platform sessions with specific configurations, making the system more intuitive and robust for users to define how different platforms interact with their bot's settings. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
dosubot
bot
added
the
area:webui
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并留下了一些整体上的反馈：
- 加载和映射路线/平台数据的逻辑在
openRouteManageDialog（构建typeMap）以及refreshConfigBindings/buildConfigBindingMap中被重复实现；建议提取一个共享的辅助函数，或者直接复用buildConfigBindingMap，这样既能保持行为一致，也有机会集中 API 调用。 onConfigSelect方法里有嵌套分支，重复执行this.selectedConfigID = value; this.getConfig(value);；可以重构为：先统一处理一次“未保存更改”的流程，然后在唯一的一个位置执行切换逻辑，这会让控制流更易理解，也更不容易出错。
面向 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- 加载和映射路线/平台数据的逻辑在 `openRouteManageDialog`（构建 `typeMap`）以及 `refreshConfigBindings`/`buildConfigBindingMap` 中被重复实现；建议提取一个共享的辅助函数，或者直接复用 `buildConfigBindingMap`，这样既能保持行为一致，也有机会集中 API 调用。
- `onConfigSelect` 方法里有嵌套分支，重复执行 `this.selectedConfigID = value; this.getConfig(value);`；可以重构为：先统一处理一次“未保存更改”的流程，然后在唯一的一个位置执行切换逻辑，这会让控制流更易理解，也更不容易出错。
## Individual Comments
### Comment 1
<location path="dashboard/src/views/ConfigPage.vue" line_range="604-613" />
<code_context>
+ nonTargetUmopSet.add(umop);
+ });
+
+ const targetEntries = [];
+ for (const item of this.routeManageItems) {
+ const umop = String(item.umop || '').trim();
+ if (!umop) {
+ continue;
+ }
+ if (nonTargetUmopSet.has(umop)) {
+ this.save_message = this.tm('routeManager.routeOccupied', { umop });
+ this.save_message_snack = true;
+ this.save_message_success = "error";
+ this.routeManageSaving = false;
+ return;
+ }
+ targetEntries.push([umop, this.routeManageConfigId]);
+ }
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 校验同一条路线列表中的重复 UMoP，避免静默覆盖。
`nonTargetUmopSet` 可以防止与 `nonTargetEntries` 的冲突，但仍然允许 `this.routeManageItems` 内部存在重复项。如果同一个 `umop` 被添加了两次，`Object.fromEntries(mergedEntries)` 只会保留最后一个。请对 `targetEntries` 做去重（例如通过 Set），或者在 `routeManageItems` 包含重复 UMoP 时抛出错误。
建议实现：
```
nonTargetEntries.push([umop, confId]);
nonTargetUmopSet.add(umop);
});
const targetEntries = [];
const targetUmopSet = new Set();
for (const item of this.routeManageItems) {
const umop = String(item.umop || '').trim();
if (!umop) {
continue;
}
if (nonTargetUmopSet.has(umop)) {
this.save_message = this.tm('routeManager.routeOccupied', { umop });
this.save_message_snack = true;
this.save_message_success = "error";
this.routeManageSaving = false;
return;
}
if (targetUmopSet.has(umop)) {
this.save_message = this.tm('routeManager.duplicateUmop', { umop });
this.save_message_snack = true;
this.save_message_success = "error";
this.routeManageSaving = false;
return;
}
targetUmopSet.add(umop);
targetEntries.push([umop, this.routeManageConfigId]);
}
```
1. 在 i18n 文案文件中为新的错误 key `routeManager.duplicateUmop` 添加本地化字符串（例如：`"routeManager.duplicateUmop": "UMoP {umop} is listed more than once in this route."`），并与现有翻译风格保持一致。
2. 如果你的项目对 i18n key 的命名规范或文案措辞有不同偏好，请在组件和语言文件中相应调整该 key 和消息内容。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The logic for loading and mapping route/platform data is duplicated between
openRouteManageDialog(buildingtypeMap) andrefreshConfigBindings/buildConfigBindingMap; consider extracting a shared helper or reusingbuildConfigBindingMapso the behavior stays consistent and the API calls can potentially be centralized. - The
onConfigSelectmethod has nested branches that repeatthis.selectedConfigID = value; this.getConfig(value);; refactoring to handle the "unsaved changes" flow once and then perform the switch in a single place would make the control flow easier to follow and less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic for loading and mapping route/platform data is duplicated between `openRouteManageDialog` (building `typeMap`) and `refreshConfigBindings`/`buildConfigBindingMap`; consider extracting a shared helper or reusing `buildConfigBindingMap` so the behavior stays consistent and the API calls can potentially be centralized.
- The `onConfigSelect` method has nested branches that repeat `this.selectedConfigID = value; this.getConfig(value);`; refactoring to handle the "unsaved changes" flow once and then perform the switch in a single place would make the control flow easier to follow and less error-prone.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/ConfigPage.vue" line_range="604-613" />
<code_context>
+ nonTargetUmopSet.add(umop);
+ });
+
+ const targetEntries = [];
+ for (const item of this.routeManageItems) {
+ const umop = String(item.umop || '').trim();
+ if (!umop) {
+ continue;
+ }
+ if (nonTargetUmopSet.has(umop)) {
+ this.save_message = this.tm('routeManager.routeOccupied', { umop });
+ this.save_message_snack = true;
+ this.save_message_success = "error";
+ this.routeManageSaving = false;
+ return;
+ }
+ targetEntries.push([umop, this.routeManageConfigId]);
+ }
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Validate duplicate UMoPs within the same route list to avoid silent overwrites.
`nonTargetUmopSet` prevents conflicts with `nonTargetEntries`, but duplicates within `this.routeManageItems` are still allowed. If the same `umop` is added twice, `Object.fromEntries(mergedEntries)` will drop all but the last. Please either deduplicate `targetEntries` (e.g., via a Set) or surface an error when `routeManageItems` contains duplicate UMoPs.
Suggested implementation:
```
nonTargetEntries.push([umop, confId]);
nonTargetUmopSet.add(umop);
});
const targetEntries = [];
const targetUmopSet = new Set();
for (const item of this.routeManageItems) {
const umop = String(item.umop || '').trim();
if (!umop) {
continue;
}
if (nonTargetUmopSet.has(umop)) {
this.save_message = this.tm('routeManager.routeOccupied', { umop });
this.save_message_snack = true;
this.save_message_success = "error";
this.routeManageSaving = false;
return;
}
if (targetUmopSet.has(umop)) {
this.save_message = this.tm('routeManager.duplicateUmop', { umop });
this.save_message_snack = true;
this.save_message_success = "error";
this.routeManageSaving = false;
return;
}
targetUmopSet.add(umop);
targetEntries.push([umop, this.routeManageConfigId]);
}
```
1. Add a localized string for the new error key `routeManager.duplicateUmop` in your i18n message files (e.g., something like `"routeManager.duplicateUmop": "UMoP {umop} is listed more than once in this route."`), matching your existing translation patterns.
2. If your project prefers a different naming convention for i18n keys or message wording, adjust the key and message accordingly in both this component and the locale files.
</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 targetEntries = []; | ||
| for (const item of this.routeManageItems) { | ||
| const umop = String(item.umop || '').trim(); | ||
| if (!umop) { | ||
| continue; | ||
| } | ||
| if (nonTargetUmopSet.has(umop)) { | ||
| this.save_message = this.tm('routeManager.routeOccupied', { umop }); | ||
| this.save_message_snack = true; | ||
| this.save_message_success = "error"; |
There was a problem hiding this comment.
suggestion (bug_risk): 校验同一条路线列表中的重复 UMoP，避免静默覆盖。
nonTargetUmopSet 可以防止与 nonTargetEntries 的冲突，但仍然允许 this.routeManageItems 内部存在重复项。如果同一个 umop 被添加了两次，Object.fromEntries(mergedEntries) 只会保留最后一个。请对 targetEntries 做去重（例如通过 Set），或者在 routeManageItems 包含重复 UMoP 时抛出错误。
建议实现：
nonTargetEntries.push([umop, confId]);
nonTargetUmopSet.add(umop);
});
const targetEntries = [];
const targetUmopSet = new Set();
for (const item of this.routeManageItems) {
const umop = String(item.umop || '').trim();
if (!umop) {
continue;
}
if (nonTargetUmopSet.has(umop)) {
this.save_message = this.tm('routeManager.routeOccupied', { umop });
this.save_message_snack = true;
this.save_message_success = "error";
this.routeManageSaving = false;
return;
}
if (targetUmopSet.has(umop)) {
this.save_message = this.tm('routeManager.duplicateUmop', { umop });
this.save_message_snack = true;
this.save_message_success = "error";
this.routeManageSaving = false;
return;
}
targetUmopSet.add(umop);
targetEntries.push([umop, this.routeManageConfigId]);
}
- 在 i18n 文案文件中为新的错误 key
routeManager.duplicateUmop添加本地化字符串（例如："routeManager.duplicateUmop": "UMoP {umop} is listed more than once in this route."），并与现有翻译风格保持一致。 - 如果你的项目对 i18n key 的命名规范或文案措辞有不同偏好，请在组件和语言文件中相应调整该 key 和消息内容。
Original comment in English
suggestion (bug_risk): Validate duplicate UMoPs within the same route list to avoid silent overwrites.
nonTargetUmopSet prevents conflicts with nonTargetEntries, but duplicates within this.routeManageItems are still allowed. If the same umop is added twice, Object.fromEntries(mergedEntries) will drop all but the last. Please either deduplicate targetEntries (e.g., via a Set) or surface an error when routeManageItems contains duplicate UMoPs.
Suggested implementation:
nonTargetEntries.push([umop, confId]);
nonTargetUmopSet.add(umop);
});
const targetEntries = [];
const targetUmopSet = new Set();
for (const item of this.routeManageItems) {
const umop = String(item.umop || '').trim();
if (!umop) {
continue;
}
if (nonTargetUmopSet.has(umop)) {
this.save_message = this.tm('routeManager.routeOccupied', { umop });
this.save_message_snack = true;
this.save_message_success = "error";
this.routeManageSaving = false;
return;
}
if (targetUmopSet.has(umop)) {
this.save_message = this.tm('routeManager.duplicateUmop', { umop });
this.save_message_snack = true;
this.save_message_success = "error";
this.routeManageSaving = false;
return;
}
targetUmopSet.add(umop);
targetEntries.push([umop, this.routeManageConfigId]);
}
- Add a localized string for the new error key
routeManager.duplicateUmopin your i18n message files (e.g., something like"routeManager.duplicateUmop": "UMoP {umop} is listed more than once in this route."), matching your existing translation patterns. - If your project prefers a different naming convention for i18n keys or message wording, adjust the key and message accordingly in both this component and the locale files.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed feature for managing routing configurations through a new UI. The refactoring of ConfigPage.vue into a more organized workbench layout with a responsive design is a great improvement to the user experience. The new components, ConfigProfileSidebar and ConfigRouteManagerDialog, are well-implemented. My feedback includes a few suggestions to improve code style, user experience in the route manager, and code conciseness.
| <v-icon size="24" class="mr-2">mdi-file-outline</v-icon> | ||
| {{ config.name }} | ||
| </div> | ||
| <div class="mt-3 d-flex" style="align-items: start; justify-content: center;"> |
There was a problem hiding this comment.
For better code organization and adherence to best practices, it's recommended to avoid inline styles. Please move the styling for this div into a dedicated CSS class in the <style> block. This improves maintainability and separates concerns. You can then add the corresponding styles to a new .profile-card__routes class in the <style> block.
<div class="mt-3 d-flex profile-card__routes">
| routes: (() => { | ||
| const sortedRoutes = routes.sort((a, b) => a.umop.localeCompare(b.umop)); | ||
| const allSessionsRoute = sortedRoutes.find((route) => isAllSessionsRoute(route.umop)); | ||
| if (allSessionsRoute) { | ||
| return [allSessionsRoute]; | ||
| } | ||
| return sortedRoutes; | ||
| })() |
There was a problem hiding this comment.
The current logic in groupedRoutes hides specific routes if a wildcard "all sessions" route (:*:*) exists for a platform. This might be confusing for users, as they won't see the complete picture of all configured routes for that platform. I suggest displaying all routes, with the "all sessions" route sorted to the top for clarity. This will provide a more transparent view of the configuration.
routes: routes.sort((a, b) => {
const aIsAll = isAllSessionsRoute(a.umop);
const bIsAll = isAllSessionsRoute(b.umop);
if (aIsAll !== bIsAll) {
return aIsAll ? -1 : 1;
}
return a.umop.localeCompare(b.umop);
})
| this.configInfoList = [...infoList].sort((a, b) => { | ||
| if (a.id === 'default' && b.id !== 'default') { | ||
| return -1; | ||
| } | ||
| if (a.id !== 'default' && b.id === 'default') { | ||
| return 1; | ||
| } | ||
| return 0; | ||
| }); |
There was a problem hiding this comment.
The sorting logic to place the 'default' configuration first is a bit verbose. It can be simplified for better readability and conciseness by using boolean-to-number conversion. This achieves the same result in a more compact way.
this.configInfoList = [...infoList].sort((a, b) => {
// Puts 'default' config at the top of the list.
return (b.id === 'default') - (a.id === 'default');
});
1 participant
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
为配置配置文件引入路由管理界面，并改进配置页面布局和用户体验（UX）。
新功能：
ConfigRouteManagerDialog组件，用于查看和管理与配置配置文件关联的路由条目。ConfigProfileSidebar组件，用于列出配置配置文件及其平台绑定，并提供快速访问路由管理的入口。增强：
ConfigPage布局重构为双栏工作台，包含固定的配置文件侧边栏、自适应工具栏，并改进未保存更改和加载状态的处理。ProviderSelector中已选提供方文本的可读性。AstrBotMessage的平台消息模型文档，使用更清晰的英文字段描述。日常维护（Chores）：
Original summary in English
Summary by Sourcery
Introduce a routing management UI for configuration profiles and improve the config page layout and UX.
New Features:
Enhancements:
Chores: