refactor:优化Dashboard 审查页与设置页，并支持 AstrBot 插件内嵌页面#174
Closed
YumemiDream wants to merge 7 commits into
Closed
Conversation
- Add a dedicated review workspace layout with sticky sidebar containing 6 category buttons: persona / style / jargon / persona-state / reviewed / batches - Each button shows live count badges; counts update whenever persona, style, jargon, persona-state, backup or batch data is loaded - Add setReviewTab(tab) / updateReviewNavCounts(counts) helpers and bind button clicks + page-revisit restore (state.reviews.tab) - Add new integration test test_dashboard_review_queue_uses_sidebar_categories to lock in the new structure
Merge the Dashboard settings page refactor together with AstrBot Plugin Pages support. Includes: - categorized settings sidebar and denser right-pane controls - improved dependency install controls and scrollbars - compact Dashboard header and removal of misleading jargon occurrence sorting - AstrBot Plugin Page entry for the Dashboard using bridge API + iframe - WebUI dashboard_url bridge endpoint and regression tests
Contributor
Reviewer's GuideRefactors the Self Learning Dashboard review and settings pages with sidebar-driven layouts and boolean switch controls, and adds an AstrBot Plugin Page that embeds the dashboard via a new dashboard_url Web API in WebUIManager, covered by integration and unit tests. Sequence diagram for AstrBot Plugin Page loading the dashboard via dashboard_urlsequenceDiagram
actor AstrBotUser
participant PluginPage as dashboard/index.html
participant AstrBotPluginPage
participant WebUIManager
AstrBotUser->>PluginPage: Open plugin details page
PluginPage->>AstrBotPluginPage: ready()
AstrBotPluginPage-->>PluginPage: resolved
PluginPage->>AstrBotPluginPage: apiGet('dashboard_url')
AstrBotPluginPage->>WebUIManager: GET dashboard_url
WebUIManager->>WebUIManager: _public_webui_base_url()
WebUIManager-->>AstrBotPluginPage: { url: /static/html/dashboard.html }
AstrBotPluginPage-->>PluginPage: payload with url
PluginPage->>PluginPage: setDashboardUrl(payload.url)
PluginPage->>DashboardFrame: load iframe src
DashboardFrame-->>AstrBotUser: Render Self Learning Dashboard
File-Level Changes
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}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The dashboard URL helpers (
_public_webui_base_urland the AstrBot Plugin PagefallbackDashboardUrl) always usehttp://, which can cause mixed-content issues if AstrBot or the WebUI are served over HTTPS; consider deriving the scheme fromwindow.location.protocolon the frontend and from config or request on the backend. - In
applyConfigSearch, the dependency panel visibility relies on a hardcoded search string ('Dependency_Install_Guide 依赖安装 手动安装依赖 dependencies'), which can drift from the actual schema; consider deriving this label/search text from the config schema or a shared constant to avoid future mismatches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The dashboard URL helpers (`_public_webui_base_url` and the AstrBot Plugin Page `fallbackDashboardUrl`) always use `http://`, which can cause mixed-content issues if AstrBot or the WebUI are served over HTTPS; consider deriving the scheme from `window.location.protocol` on the frontend and from config or request on the backend.
- In `applyConfigSearch`, the dependency panel visibility relies on a hardcoded search string (`'Dependency_Install_Guide 依赖安装 手动安装依赖 dependencies'`), which can drift from the actual schema; consider deriving this label/search text from the config schema or a shared constant to avoid future mismatches.
## Individual Comments
### Comment 1
<location path="pages/dashboard/index.html" line_range="136-139" />
<code_context>
+ const statusTextEl = document.getElementById('statusText');
+ const frameEl = document.getElementById('dashboardFrame');
+
+ function fallbackDashboardUrl() {
+ const host = window.location.hostname || '127.0.0.1';
+ const safeHost = host.includes(':') && !host.startsWith('[') ? `[${host}]` : host;
+ return `http://${safeHost}:7833/static/html/dashboard.html`;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider deriving the fallback URL scheme from the current page to avoid mixed-content issues.
`fallbackDashboardUrl` always uses `http://`. When AstrBot runs over HTTPS, this will trigger mixed-content blocking and the iframe won’t load. Please derive the scheme from `window.location.protocol` (or the current origin) so the dashboard URL matches the page’s protocol.
For example:
```js
const scheme = window.location.protocol === 'https:' ? 'https' : 'http';
return `${scheme}://${safeHost}:7833/static/html/dashboard.html`;
```
</issue_to_address>
### Comment 2
<location path="webui/manager.py" line_range="57-70" />
<code_context>
# 创建
+ def _public_webui_base_url(self) -> str:
+ host = str(getattr(self._config, "web_interface_host", "127.0.0.1") or "127.0.0.1")
+ port = int(getattr(self._config, "web_interface_port", 7833) or 7833)
+ if host in {"0.0.0.0", "::", "[::]"}:
+ try:
+ from quart import request
+
+ host_header = request.host.split(":", 1)[0]
+ except (ImportError, RuntimeError):
+ host_header = ""
+ host = host_header or "127.0.0.1"
+ if ":" in host and not host.startswith("["):
+ host = f"[{host}]"
+ return f"http://{host}:{port}"
+
+ def _register_plugin_pages_api(self) -> None:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Deriving the WebUI base URL purely as http may cause mixed-content or reverse-proxy issues.
Building the URL as `http://{host}:{port}` will be wrong when AstrBot is served over HTTPS or behind a TLS-terminating proxy that rewrites scheme/port, causing mixed-content issues or broken links from the plugin page.
Where possible, derive scheme/host/port from the incoming request (e.g. `request.scheme` and `request.host`) or rely on the front-end’s origin instead of hardcoding `http`. This ensures the dashboard URL exposed via the plugin pages API matches the actual public endpoint.
```suggestion
def _public_webui_base_url(self) -> str:
"""
Derive the public WebUI base URL.
Prefer the incoming request's scheme/host (including proxy headers) when available
to avoid mixed-content and reverse-proxy issues. Fall back to configuration if
there is no active request context.
"""
# Try to derive from the current HTTP request (respecting common proxy headers)
try:
from quart import request # type: ignore
# Accessing request.* without a context raises RuntimeError; keep this in the try.
scheme = request.headers.get("X-Forwarded-Proto", request.scheme)
# X-Forwarded-Host may contain host[:port]; if absent, request.host already does.
host = request.headers.get("X-Forwarded-Host", request.host)
if host:
# Normalise IPv6 literal hosts if needed
if ":" in host and not host.startswith("["):
# host may already contain a port (e.g. "example.com:8443" or "[::1]:8443"),
# so we only wrap the bare host portion when it's an IPv6 literal.
host_part, *port_part = host.rsplit(":", 1)
if ":" in host_part and not host_part.startswith("["):
host_part = f"[{host_part}]"
host = ":".join([host_part] + port_part) if port_part else host_part
return f"{scheme}://{host}"
except (ImportError, RuntimeError):
# No Quart available or no active request context; fall back to config
pass
# Fallback: derive from configured bind host/port, using http as a reasonable default.
host = str(getattr(self._config, "web_interface_host", "127.0.0.1") or "127.0.0.1")
port = int(getattr(self._config, "web_interface_port", 7833) or 7833)
# When bound to all interfaces, pick a more specific host for generating URLs.
if host in {"0.0.0.0", "::", "[::]"}:
host = "127.0.0.1"
# Normalise IPv6 literal host
if ":" in host and not host.startswith("["):
host = f"[{host}]"
return f"http://{host}:{port}"
```
</issue_to_address>
### Comment 3
<location path="tests/integration/test_webui_static_assets.py" line_range="108-131" />
<code_context>
+ assert "updateReviewNavCounts" in text
+
+
+def test_dashboard_settings_uses_sidebar_categories():
+ text = (PLUGIN_ROOT / "web_res" / "static" / "html" / "dashboard.html").read_text(encoding="utf-8")
+
+ assert "settings-sidebar" in text
+ assert "settings-workspace" in text
+ assert "settingsSidebarSummary" in text
+ assert "settingsNav" in text
+ assert 'data-config-group="all"' in text
+ assert "dependencyInstallPanel" in text
+ assert "renderSettingsNav" in text
+ assert "setConfigGroup" in text
+ assert "configGroupIcon" in text
+ assert "SETTINGS_NAV_SECTIONS" in text
+ assert "常用入口" in text
+ assert "学习链路" in text
+ assert "人格与关系" in text
+ assert "数据与运行" in text
+ assert "settings-nav::-webkit-scrollbar" in text
+ assert "scrollbar-width: thin" in text
+ assert "Dependency_Install_Guide: 'extension'" in text
+ assert "deployed_code" not in text
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Avoid over-coupling dashboard settings tests to exact copy and raw substrings
These assertions validate the new sidebar structure and key labels, but relying on long exact strings (full Chinese labels, `settings-nav::-webkit-scrollbar`, exact CSS declarations) and negative substring checks (like ensuring `"deployed_code"` is absent) makes the test brittle to harmless copy or CSS changes. Prefer asserting on stable structural markers (ids, data attributes, function names), and loosen or narrow text/style checks so they confirm the intended layout/behavior without depending on exact CSS serialization or full label text.
```suggestion
assert "updateReviewNavCounts" in text
+
+
+def test_dashboard_settings_uses_sidebar_categories():
+ text = (PLUGIN_ROOT / "web_res" / "static" / "html" / "dashboard.html").read_text(encoding="utf-8")
+
+ # Structural hooks for the settings sidebar and workspace layout
+ assert "settings-sidebar" in text
+ assert "settings-workspace" in text
+ assert "settingsSidebarSummary" in text
+ assert "settingsNav" in text
+
+ # Config grouping and navigation rendering hooks
+ assert 'data-config-group="all"' in text
+ assert "dependencyInstallPanel" in text
+ assert "renderSettingsNav" in text
+ assert "setConfigGroup" in text
+ assert "configGroupIcon" in text
+ assert "SETTINGS_NAV_SECTIONS" in text
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+136
to
+139
| function fallbackDashboardUrl() { | ||
| const host = window.location.hostname || '127.0.0.1'; | ||
| const safeHost = host.includes(':') && !host.startsWith('[') ? `[${host}]` : host; | ||
| return `http://${safeHost}:7833/static/html/dashboard.html`; |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Consider deriving the fallback URL scheme from the current page to avoid mixed-content issues.
fallbackDashboardUrl always uses http://. When AstrBot runs over HTTPS, this will trigger mixed-content blocking and the iframe won’t load. Please derive the scheme from window.location.protocol (or the current origin) so the dashboard URL matches the page’s protocol.
For example:
const scheme = window.location.protocol === 'https:' ? 'https' : 'http';
return `${scheme}://${safeHost}:7833/static/html/dashboard.html`;
Comment on lines
+57
to
+70
| def _public_webui_base_url(self) -> str: | ||
| host = str(getattr(self._config, "web_interface_host", "127.0.0.1") or "127.0.0.1") | ||
| port = int(getattr(self._config, "web_interface_port", 7833) or 7833) | ||
| if host in {"0.0.0.0", "::", "[::]"}: | ||
| try: | ||
| from quart import request | ||
|
|
||
| host_header = request.host.split(":", 1)[0] | ||
| except (ImportError, RuntimeError): | ||
| host_header = "" | ||
| host = host_header or "127.0.0.1" | ||
| if ":" in host and not host.startswith("["): | ||
| host = f"[{host}]" | ||
| return f"http://{host}:{port}" |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Deriving the WebUI base URL purely as http may cause mixed-content or reverse-proxy issues.
Building the URL as http://{host}:{port} will be wrong when AstrBot is served over HTTPS or behind a TLS-terminating proxy that rewrites scheme/port, causing mixed-content issues or broken links from the plugin page.
Where possible, derive scheme/host/port from the incoming request (e.g. request.scheme and request.host) or rely on the front-end’s origin instead of hardcoding http. This ensures the dashboard URL exposed via the plugin pages API matches the actual public endpoint.
Suggested change
| def _public_webui_base_url(self) -> str: | |
| host = str(getattr(self._config, "web_interface_host", "127.0.0.1") or "127.0.0.1") | |
| port = int(getattr(self._config, "web_interface_port", 7833) or 7833) | |
| if host in {"0.0.0.0", "::", "[::]"}: | |
| try: | |
| from quart import request | |
| host_header = request.host.split(":", 1)[0] | |
| except (ImportError, RuntimeError): | |
| host_header = "" | |
| host = host_header or "127.0.0.1" | |
| if ":" in host and not host.startswith("["): | |
| host = f"[{host}]" | |
| return f"http://{host}:{port}" | |
| def _public_webui_base_url(self) -> str: | |
| """ | |
| Derive the public WebUI base URL. | |
| Prefer the incoming request's scheme/host (including proxy headers) when available | |
| to avoid mixed-content and reverse-proxy issues. Fall back to configuration if | |
| there is no active request context. | |
| """ | |
| # Try to derive from the current HTTP request (respecting common proxy headers) | |
| try: | |
| from quart import request # type: ignore | |
| # Accessing request.* without a context raises RuntimeError; keep this in the try. | |
| scheme = request.headers.get("X-Forwarded-Proto", request.scheme) | |
| # X-Forwarded-Host may contain host[:port]; if absent, request.host already does. | |
| host = request.headers.get("X-Forwarded-Host", request.host) | |
| if host: | |
| # Normalise IPv6 literal hosts if needed | |
| if ":" in host and not host.startswith("["): | |
| # host may already contain a port (e.g. "example.com:8443" or "[::1]:8443"), | |
| # so we only wrap the bare host portion when it's an IPv6 literal. | |
| host_part, *port_part = host.rsplit(":", 1) | |
| if ":" in host_part and not host_part.startswith("["): | |
| host_part = f"[{host_part}]" | |
| host = ":".join([host_part] + port_part) if port_part else host_part | |
| return f"{scheme}://{host}" | |
| except (ImportError, RuntimeError): | |
| # No Quart available or no active request context; fall back to config | |
| pass | |
| # Fallback: derive from configured bind host/port, using http as a reasonable default. | |
| host = str(getattr(self._config, "web_interface_host", "127.0.0.1") or "127.0.0.1") | |
| port = int(getattr(self._config, "web_interface_port", 7833) or 7833) | |
| # When bound to all interfaces, pick a more specific host for generating URLs. | |
| if host in {"0.0.0.0", "::", "[::]"}: | |
| host = "127.0.0.1" | |
| # Normalise IPv6 literal host | |
| if ":" in host and not host.startswith("["): | |
| host = f"[{host}]" | |
| return f"http://{host}:{port}" |
Comment on lines
+108
to
+131
| assert "updateReviewNavCounts" in text | ||
|
|
||
|
|
||
| def test_dashboard_settings_uses_sidebar_categories(): | ||
| text = (PLUGIN_ROOT / "web_res" / "static" / "html" / "dashboard.html").read_text(encoding="utf-8") | ||
|
|
||
| assert "settings-sidebar" in text | ||
| assert "settings-workspace" in text | ||
| assert "settingsSidebarSummary" in text | ||
| assert "settingsNav" in text | ||
| assert 'data-config-group="all"' in text | ||
| assert "dependencyInstallPanel" in text | ||
| assert "renderSettingsNav" in text | ||
| assert "setConfigGroup" in text | ||
| assert "configGroupIcon" in text | ||
| assert "SETTINGS_NAV_SECTIONS" in text | ||
| assert "常用入口" in text | ||
| assert "学习链路" in text | ||
| assert "人格与关系" in text | ||
| assert "数据与运行" in text | ||
| assert "settings-nav::-webkit-scrollbar" in text | ||
| assert "scrollbar-width: thin" in text | ||
| assert "Dependency_Install_Guide: 'extension'" in text | ||
| assert "deployed_code" not in text |
Contributor
There was a problem hiding this comment.
suggestion (testing): Avoid over-coupling dashboard settings tests to exact copy and raw substrings
These assertions validate the new sidebar structure and key labels, but relying on long exact strings (full Chinese labels, settings-nav::-webkit-scrollbar, exact CSS declarations) and negative substring checks (like ensuring "deployed_code" is absent) makes the test brittle to harmless copy or CSS changes. Prefer asserting on stable structural markers (ids, data attributes, function names), and loosen or narrow text/style checks so they confirm the intended layout/behavior without depending on exact CSS serialization or full label text.
Suggested change
| assert "updateReviewNavCounts" in text | |
| def test_dashboard_settings_uses_sidebar_categories(): | |
| text = (PLUGIN_ROOT / "web_res" / "static" / "html" / "dashboard.html").read_text(encoding="utf-8") | |
| assert "settings-sidebar" in text | |
| assert "settings-workspace" in text | |
| assert "settingsSidebarSummary" in text | |
| assert "settingsNav" in text | |
| assert 'data-config-group="all"' in text | |
| assert "dependencyInstallPanel" in text | |
| assert "renderSettingsNav" in text | |
| assert "setConfigGroup" in text | |
| assert "configGroupIcon" in text | |
| assert "SETTINGS_NAV_SECTIONS" in text | |
| assert "常用入口" in text | |
| assert "学习链路" in text | |
| assert "人格与关系" in text | |
| assert "数据与运行" in text | |
| assert "settings-nav::-webkit-scrollbar" in text | |
| assert "scrollbar-width: thin" in text | |
| assert "Dependency_Install_Guide: 'extension'" in text | |
| assert "deployed_code" not in text | |
| assert "updateReviewNavCounts" in text | |
| + | |
| + | |
| +def test_dashboard_settings_uses_sidebar_categories(): | |
| + text = (PLUGIN_ROOT / "web_res" / "static" / "html" / "dashboard.html").read_text(encoding="utf-8") | |
| + | |
| + # Structural hooks for the settings sidebar and workspace layout | |
| + assert "settings-sidebar" in text | |
| + assert "settings-workspace" in text | |
| + assert "settingsSidebarSummary" in text | |
| + assert "settingsNav" in text | |
| + | |
| + # Config grouping and navigation rendering hooks | |
| + assert 'data-config-group="all"' in text | |
| + assert "dependencyInstallPanel" in text | |
| + assert "renderSettingsNav" in text | |
| + assert "setConfigGroup" in text | |
| + assert "configGroupIcon" in text | |
| + assert "SETTINGS_NAV_SECTIONS" in text |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
改动概览
主要优化 Self Learning Dashboard 的前端体验，并新增 AstrBot Plugin Pages 支持，使用户可以直接从 AstrBot WebUI 的插件详情页进入 Dashboard。
Dashboard 设置页重构
Dashboard 体验优化
AstrBot Plugin Pages 支持
pages/dashboard/index.html，支持在 AstrBot 插件详情页中打开 Dashboard。.astrbot-plugin/i18n/zh-CN.json和.astrbot-plugin/i18n/en-US.json，注册 Dashboard 页面标题。WebUIManager中注册dashboard_urlWeb API，用于返回当前 WebUI Dashboard 地址。回归测试
dashboard_url调用存在。dashboard_urlAPI 会被注册。验证
已执行：
python -m py_compile tests\integration\test_webui_static_assets.py webui\manager.py tests\unit\test_webui_manager.pypages/dashboard/index.html与web_res/static/html/dashboard.htmlgit diff --checkSummary by Sourcery
Refine the Self Learning dashboard UI and settings layout while adding AstrBot Plugin Page support for opening the dashboard from AstrBot WebUI, with corresponding tests and Web API registration.
Enhancements:
Tests: