-
-
Notifications
You must be signed in to change notification settings - Fork 968
feat: support options to delete plugins config and data #3280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support options to delete plugins config and data #3280
Conversation
- 二次确认时可选删除插件配置和持久化数据 - 添加对应的i18n支持
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
大家好 - 我已经审阅了你的更改 - 这里有一些反馈:
- 在 ExtensionPage.vue 中移除未使用的 inject('$confirm') 导入和相关的死代码,因为现在由自定义的 UninstallConfirmDialog 处理确认。
- ExtensionCard.vue 和 ExtensionPage.vue 中的确认逻辑非常相似——考虑将其提取到一个共享的组合式函数或混入中,以避免重复的显示/确认处理程序代码。
- 在 star_manager.uninstall_plugin 中,删除配置和数据的代码块可以提取到辅助方法中,以简化主方法并提高可读性。
AI 代理提示
请处理此代码审查中的评论:
## 总体评论
- 在 ExtensionPage.vue 中移除未使用的 inject('$confirm') 导入和相关的死代码,因为现在由自定义的 UninstallConfirmDialog 处理确认。
- ExtensionCard.vue 和 ExtensionPage.vue 中的确认逻辑非常相似——考虑将其提取到一个共享的组合式函数或混入中,以避免重复的显示/确认处理程序代码。
- 在 star_manager.uninstall_plugin 中,删除配置和数据的代码块可以提取到辅助方法中,以简化主方法并提高可读性。
## 单独评论
### 评论 1
<location> `dashboard/src/views/ExtensionPage.vue:19` </location>
<code_context>
const commonStore = useCommonStore();
const { t } = useI18n();
const { tm } = useModuleI18n('features/extension');
+const $confirm = inject('$confirm');
const fileInput = ref(null);
const activeTab = ref('installed');
</code_context>
<issue_to_address>
**suggestion:** 未使用的 $confirm 注入可以移除。
由于 UninstallConfirmDialog 替换了之前的用法,$confirm 可以安全移除以提高代码清晰度。
```suggestion
```
</issue_to_address>
### 评论 2
<location> `astrbot/core/star/star_manager.py:732` </location>
<code_context>
f"移除插件成功,但是删除插件文件夹失败: {e!s}。您可以手动删除该文件夹,位于 addons/plugins/ 下。",
)
+ # 删除插件配置文件
+ if delete_config and root_dir_name:
+ config_file = os.path.join(
</code_context>
<issue_to_address>
**issue (complexity):** 考虑将重复的删除逻辑重构为一个私有辅助方法,并使用循环处理数据目录,以提高代码清晰度并减少重复。
以下是一个思路:把「判断存在 → 调用删除 → 统一捕获并打日志」这段重复逻辑抽成一个私有方法,然后对 config 与两个 data 目录只做一次循环即可。示例:
```python
# 在类里新增一个私有方法
def _try_remove(
self,
path: str,
remove_fn: Callable[[str], None],
success_msg: str,
error_msg: str,
):
if not os.path.exists(path):
return
try:
remove_fn(path)
logger.info(success_msg)
except Exception as e:
logger.warning(f"{error_msg}: {e!s}")
```
然后把原来的三段重复逻辑替换成:
```python
# 删除插件配置
if delete_config and root_dir_name:
cfg_path = os.path.join(
self.plugin_config_path, f"{root_dir_name}_config.json"
)
self._try_remove(
cfg_path,
os.remove,
f"已删除插件 {plugin_name} 的配置文件",
"删除插件配置文件失败",
)
# 删除插件持久化数据
if delete_data and root_dir_name:
base = os.path.dirname(ppath)
for subdir in ("plugin_data", "plugins_data"):
data_path = os.path.join(base, subdir, root_dir_name)
self._try_remove(
data_path,
remove_dir,
f"已删除插件 {plugin_name} 的持久化数据 ({subdir})",
f"删除插件持久化数据失败 ({subdir})",
)
```
这样一来:
- 删除逻辑合并在一个方法里, try/except 只写一次
- 遍历两种 data 目录,去掉重复代码块
- 功能与原来完全一致,且可读性更高。
</issue_to_address>帮助我更有用!请点击每个评论上的 👍 或 👎,我将使用反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- Remove the unused inject('$confirm') import and related dead code in ExtensionPage.vue since the custom UninstallConfirmDialog is now handling confirmation.
- The confirmation logic in ExtensionCard.vue and ExtensionPage.vue is very similar – consider extracting it into a shared composable or mixin to avoid duplicated show/confirm handler code.
- In star_manager.uninstall_plugin, the blocks for deleting config and data could be extracted into helper methods to simplify the main method and improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Remove the unused inject('$confirm') import and related dead code in ExtensionPage.vue since the custom UninstallConfirmDialog is now handling confirmation.
- The confirmation logic in ExtensionCard.vue and ExtensionPage.vue is very similar – consider extracting it into a shared composable or mixin to avoid duplicated show/confirm handler code.
- In star_manager.uninstall_plugin, the blocks for deleting config and data could be extracted into helper methods to simplify the main method and improve readability.
## Individual Comments
### Comment 1
<location> `dashboard/src/views/ExtensionPage.vue:19` </location>
<code_context>
const commonStore = useCommonStore();
const { t } = useI18n();
const { tm } = useModuleI18n('features/extension');
+const $confirm = inject('$confirm');
const fileInput = ref(null);
const activeTab = ref('installed');
</code_context>
<issue_to_address>
**suggestion:** Unused $confirm injection can be removed.
Since UninstallConfirmDialog replaces the previous usage, $confirm can be safely removed to improve code clarity.
```suggestion
```
</issue_to_address>
### Comment 2
<location> `astrbot/core/star/star_manager.py:732` </location>
<code_context>
f"移除插件成功,但是删除插件文件夹失败: {e!s}。您可以手动删除该文件夹,位于 addons/plugins/ 下。",
)
+ # 删除插件配置文件
+ if delete_config and root_dir_name:
+ config_file = os.path.join(
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the repeated deletion logic into a private helper method and using a loop for data directories to improve code clarity and reduce duplication.
以下是一个思路:把「判断存在 → 调用删除 → 统一捕获并打日志」这段重复逻辑抽成一个私有方法,然后对 config 与两个 data 目录只做一次循环即可。示例:
```python
# 在类里新增一个私有方法
def _try_remove(
self,
path: str,
remove_fn: Callable[[str], None],
success_msg: str,
error_msg: str,
):
if not os.path.exists(path):
return
try:
remove_fn(path)
logger.info(success_msg)
except Exception as e:
logger.warning(f"{error_msg}: {e!s}")
```
然后把原来的三段重复逻辑替换成:
```python
# 删除插件配置
if delete_config and root_dir_name:
cfg_path = os.path.join(
self.plugin_config_path, f"{root_dir_name}_config.json"
)
self._try_remove(
cfg_path,
os.remove,
f"已删除插件 {plugin_name} 的配置文件",
"删除插件配置文件失败",
)
# 删除插件持久化数据
if delete_data and root_dir_name:
base = os.path.dirname(ppath)
for subdir in ("plugin_data", "plugins_data"):
data_path = os.path.join(base, subdir, root_dir_name)
self._try_remove(
data_path,
remove_dir,
f"已删除插件 {plugin_name} 的持久化数据 ({subdir})",
f"删除插件持久化数据失败 ({subdir})",
)
```
这样一来:
- 删除逻辑合并在一个方法里, try/except 只写一次
- 遍历两种 data 目录,去掉重复代码块
- 功能与原来完全一致,且可读性更高。
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the plugin uninstallation feature by adding options to delete plugin configuration files and persistent data during uninstallation. Previously, uninstalling a plugin only removed the plugin code but left configuration and data files intact.
Key changes:
- Added a new
UninstallConfirmDialogcomponent with checkboxes for config and data deletion - Extended the backend
uninstall_pluginAPI to acceptdelete_configanddelete_dataparameters - Updated the plugin manager to handle deletion of config files and data directories
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| dashboard/src/components/shared/UninstallConfirmDialog.vue | New dialog component with options to delete config/data during uninstall |
| dashboard/src/components/shared/ExtensionCard.vue | Updated to use new dialog and emit options with uninstall event |
| dashboard/src/views/ExtensionPage.vue | Modified uninstall handler to support new options parameter |
| dashboard/src/i18n/locales/zh-CN/features/extension.json | Added Chinese translations for new uninstall options |
| dashboard/src/i18n/locales/en-US/features/extension.json | Added English translations for new uninstall options |
| dashboard/src/i18n/locales/zh-CN/messages/validation.json | Added warning message for irreversible operations |
| dashboard/src/i18n/locales/en-US/messages/validation.json | Added warning message for irreversible operations |
| astrbot/dashboard/routes/plugin.py | Updated API endpoint to accept config/data deletion flags |
| astrbot/core/star/star_manager.py | Implemented logic to delete config files and data directories |
Comments suppressed due to low confidence (1)
dashboard/src/components/shared/ExtensionCard.vue:53
- The
$confirminjection is no longer used after replacing the confirmation dialog withUninstallConfirmDialog. This unused variable should be removed to improve code maintainability.
const $confirm = inject("$confirm");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
总体上没什么问题,可以解决一下 Copilot 给出的建议。
|
已移除const $confirm = inject('$confirm'); |
* Chore: Dockerfile (#3266) * fix: Dockerfile python main.py 改为uv run main.py * fix(dockerfile): 减少重复安装 * fix: 修复一些细节问题 * fix(.dockerignore): 需要git文件夹以获取astrbot版本(带git commit hash后缀) * fix(.dockerignore): uv run之前会uv sync * Replace insecure random with secrets module in cryptographic contexts (#3248) * Initial plan * Security fixes: Replace insecure random with secrets module and improve SSL context Co-authored-by: LIghtJUNction <[email protected]> * Address code review feedback: fix POST method and add named constants Co-authored-by: LIghtJUNction <[email protected]> * Improve documentation for random number generation constants Co-authored-by: LIghtJUNction <[email protected]> * Update astrbot/core/utils/io.py Co-authored-by: Copilot <[email protected]> * Update astrbot/core/platform/sources/wecom_ai_bot/WXBizJsonMsgCrypt.py Co-authored-by: Copilot <[email protected]> * Update tests/test_security_fixes.py Co-authored-by: Copilot <[email protected]> * Update astrbot/core/utils/io.py Co-authored-by: Copilot <[email protected]> * Update astrbot/core/utils/io.py Co-authored-by: Copilot <[email protected]> * Fix: Handle path parameter in SSL fallback for download_image_by_url Co-authored-by: LIghtJUNction <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: LIghtJUNction <[email protected]> Co-authored-by: LIghtJUNction <[email protected]> Co-authored-by: Copilot <[email protected]> * chore: nodejs in Dockerfile * fix: typing error (#3267) * fix: 修复一些小错误。 修复aiocqhttp和slack中部分逻辑缺失的await。修复discord中错误的异常捕获类型。 * fix(core.platform): 修复discord适配器中错误的message_chain赋值 * fix(aiocqhttp): 更新convert_message方法的返回类型为AstrBotMessage | None --------- Co-authored-by: Soulter <[email protected]> * feat: support options to delete plugins config and data (#3280) * - 为插件管理页面中,删除插件提供一致的二次确认(原本只有卡片视图有二次确认) - 二次确认时可选删除插件配置和持久化数据 - 添加对应的i18n支持 * ruff * 移除未使用的 const $confirm = inject('$confirm'); --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Soulter <[email protected]> Co-authored-by: Dt8333 <[email protected]> Co-authored-by: Soulter <[email protected]> Co-authored-by: Misaka Mikoto <[email protected]>
* Chore: Dockerfile (#3266) * fix: Dockerfile python main.py 改为uv run main.py * fix(dockerfile): 减少重复安装 * fix: 修复一些细节问题 * fix(.dockerignore): 需要git文件夹以获取astrbot版本(带git commit hash后缀) * fix(.dockerignore): uv run之前会uv sync * Replace insecure random with secrets module in cryptographic contexts (#3248) * Initial plan * Security fixes: Replace insecure random with secrets module and improve SSL context Co-authored-by: LIghtJUNction <[email protected]> * Address code review feedback: fix POST method and add named constants Co-authored-by: LIghtJUNction <[email protected]> * Improve documentation for random number generation constants Co-authored-by: LIghtJUNction <[email protected]> * Update astrbot/core/utils/io.py Co-authored-by: Copilot <[email protected]> * Update astrbot/core/platform/sources/wecom_ai_bot/WXBizJsonMsgCrypt.py Co-authored-by: Copilot <[email protected]> * Update tests/test_security_fixes.py Co-authored-by: Copilot <[email protected]> * Update astrbot/core/utils/io.py Co-authored-by: Copilot <[email protected]> * Update astrbot/core/utils/io.py Co-authored-by: Copilot <[email protected]> * Fix: Handle path parameter in SSL fallback for download_image_by_url Co-authored-by: LIghtJUNction <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: LIghtJUNction <[email protected]> Co-authored-by: LIghtJUNction <[email protected]> Co-authored-by: Copilot <[email protected]> * chore: nodejs in Dockerfile * fix: typing error (#3267) * fix: 修复一些小错误。 修复aiocqhttp和slack中部分逻辑缺失的await。修复discord中错误的异常捕获类型。 * fix(core.platform): 修复discord适配器中错误的message_chain赋值 * fix(aiocqhttp): 更新convert_message方法的返回类型为AstrBotMessage | None --------- Co-authored-by: Soulter <[email protected]> * feat: support options to delete plugins config and data (#3280) * - 为插件管理页面中,删除插件提供一致的二次确认(原本只有卡片视图有二次确认) - 二次确认时可选删除插件配置和持久化数据 - 添加对应的i18n支持 * ruff * 移除未使用的 const $confirm = inject('$confirm'); --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Soulter <[email protected]> Co-authored-by: Dt8333 <[email protected]> Co-authored-by: Soulter <[email protected]> Co-authored-by: Misaka Mikoto <[email protected]>
Motivation / 动机
Modifications / 改动点
具体改动
Verification Steps / 验证步骤
Screenshots or Test Results / 运行截图或测试结果
经测试
均正常,未出现漏删和误删
Compatibility & Breaking Changes / 兼容性与破坏性变更
经测试,使用旧版面板文件,可以正常调用修改后的插件删除函数
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.Sourcery 总结
添加一个统一的两步插件卸载流程,并提供删除配置和持久化数据的选项
新功能:
改进:
Original summary in English
Summary by Sourcery
Add a unified two-step plugin uninstall flow with options to remove configuration and persistent data
New Features:
Enhancements: