Skip to content

feat: 插件有新版本时置顶显示(可开关)#7665

Merged
zouyonghe merged 7 commits intoAstrBotDevs:masterfrom
Blueteemo:fix/issue-7577-plugin-update-pin
Apr 21, 2026
Merged

feat: 插件有新版本时置顶显示(可开关)#7665
zouyonghe merged 7 commits intoAstrBotDevs:masterfrom
Blueteemo:fix/issue-7577-plugin-update-pin

Conversation

@Blueteemo
Copy link
Copy Markdown
Contributor

@Blueteemo Blueteemo commented Apr 19, 2026

问题描述

插件有新版本时,用户难以发现。当前需要手动切换到"更新状态"排序才能看到哪些插件有更新。

解决方案

添加 pinUpdatesOnTop 开关(默认开启),在任何排序方式下都优先显示有更新的插件。用户也可关闭此功能恢复原有排序行为。

额外想法

除解决 issue 外,额外添加了可开关的设置,让用户可选择是否启用此功能。

修改内容

  1. useExtensionPage.js:添加 pinUpdatesOnTop 状态和排序逻辑
  2. InstalledPluginsTab.vue:添加开关控件
  3. i18n 文件:中/英/俄语添加翻译

Related Issue

Fixes #7577

Summary by Sourcery

Add a preference-driven option to pin plugins with available updates to the top of the installed plugins list and centralize extension preference storage handling.

New Features:

  • Introduce a user-toggleable setting to always show plugins with available updates at the top of the installed plugins list, independent of the selected sort mode.

Enhancements:

  • Refactor extension preference persistence (show reserved plugins, list view mode, and update pinning) into a shared, storage-safe helper module with consistent boolean handling.

Tests:

  • Add unit tests covering the new extension preference storage helpers for reading and writing boolean flags.

@auto-assign auto-assign bot requested review from Fridemn and advent259141 April 19, 2026 04:09
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. area:webui The bug / feature is about webui(dashboard) of astrbot. feature:plugin The bug / feature is about AstrBot plugin system. labels Apr 19, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a "Pin Updates on Top" feature for the installed plugins list, allowing users to prioritize plugins with available updates. The changes include new internationalization keys across multiple locales, a UI toggle switch in the installed plugins tab, and updated sorting logic to handle the pinning behavior. The review feedback identifies a missing variable destructuring in the Vue component that would cause a runtime error, and provides suggestions for persisting the user's preference in local storage and adjusting UI constraints to prevent text truncation in different languages.

Comment thread dashboard/src/views/extension/InstalledPluginsTab.vue
Comment thread dashboard/src/views/extension/useExtensionPage.js Outdated
Comment thread dashboard/src/views/extension/InstalledPluginsTab.vue Outdated
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Apr 19, 2026
@Soulter Soulter force-pushed the master branch 2 times, most recently from faf411f to 0068960 Compare April 19, 2026 09:50
@zouyonghe
Copy link
Copy Markdown
Member

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The pinUpdatesOnTop localStorage key is hard-coded in multiple places; consider extracting it to a shared constant (and optionally namespacing it) to avoid typos and make future refactors safer.
  • Access to localStorage is guarded by a window check but not by a try/catch; in some environments (e.g., strict privacy modes) calls can still throw, so consider wrapping the get/set calls to fail gracefully without breaking the page.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `pinUpdatesOnTop` localStorage key is hard-coded in multiple places; consider extracting it to a shared constant (and optionally namespacing it) to avoid typos and make future refactors safer.
- Access to `localStorage` is guarded by a `window` check but not by a try/catch; in some environments (e.g., strict privacy modes) calls can still throw, so consider wrapping the get/set calls to fail gracefully without breaking the page.

## Individual Comments

### Comment 1
<location path="dashboard/src/views/extension/useExtensionPage.js" line_range="191-193" />
<code_context>
   const installedStatusFilter = ref("all");
   const installedSortBy = ref("default");
   const installedSortOrder = ref("desc");
+  const getInitialPinUpdatesOnTop = () => {
+    if (typeof window !== "undefined" && window.localStorage) {
+      const saved = localStorage.getItem("pinUpdatesOnTop");
+      return saved !== "false";
+    }
</code_context>
<issue_to_address>
**issue:** Accessing localStorage without try/catch can throw in some environments (e.g. Safari private mode), which can break hook initialization.

`localStorage.getItem`/`setItem` can still throw (e.g. `SecurityError`) even when `window` and `localStorage` exist. Please wrap these calls in try/catch for both reads and writes, and fall back to the default `true` when storage access fails so the hook doesn’t break in restricted environments.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread dashboard/src/views/extension/useExtensionPage.js Outdated
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Apr 20, 2026
@zouyonghe
Copy link
Copy Markdown
Member

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The pinUpdatesOnTop sorting block runs before any other sort criteria but ignores installedSortOrder; if users switch to ascending order they may still expect updated plugins to be grouped at the top but otherwise respect the chosen order, so it may be worth confirming or documenting this behavior in code (e.g., a short comment near the comparator).
  • In extensionPreferenceStorage.mjs, resolveStorage treats an explicitly passed null differently from undefined (the former bypasses the resolution logic, the latter triggers it); if consumers might pass null as “no storage,” consider handling that case explicitly or tightening the function’s JSDoc/type hints to avoid accidental misuse.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `pinUpdatesOnTop` sorting block runs before any other sort criteria but ignores `installedSortOrder`; if users switch to ascending order they may still expect updated plugins to be grouped at the top but otherwise respect the chosen order, so it may be worth confirming or documenting this behavior in code (e.g., a short comment near the comparator).
- In `extensionPreferenceStorage.mjs`, `resolveStorage` treats an explicitly passed `null` differently from `undefined` (the former bypasses the resolution logic, the latter triggers it); if consumers might pass `null` as “no storage,” consider handling that case explicitly or tightening the function’s JSDoc/type hints to avoid accidental misuse.

## Individual Comments

### Comment 1
<location path="dashboard/src/views/extension/useExtensionPage.js" line_range="448-449" />
<code_context>
         const fallbackResult =
           fallbackNameCompare !== 0 ? fallbackNameCompare : left.index - right.index;

+        if (
</code_context>
<issue_to_address>
**suggestion (performance):** Defer `fallbackResult` computation until after the `pinUpdatesOnTop` block to avoid unnecessary work on early returns.

With the new early return, `fallbackResult` can be computed and then discarded. Please move its computation down to just before the branches that use it to avoid extra work on this hot path.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread dashboard/src/views/extension/useExtensionPage.js Outdated
@zouyonghe
Copy link
Copy Markdown
Member

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • The sort comparator in useExtensionPage now allocates a getFallbackResult closure on every comparison; consider hoisting the fallback comparison logic (or precomputed values) out of the comparator or inlining it to avoid per-item function creation and improve readability.
  • In extensionPreferenceStorage.mjs, resolveStorage currently returns any non-nullish storage argument without validation; consider narrowing this to a Storage-like interface (or adding a simple shape check) so accidental non-storage values fail fast instead of causing subtle runtime issues when getItem/setItem are called.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `sort` comparator in `useExtensionPage` now allocates a `getFallbackResult` closure on every comparison; consider hoisting the fallback comparison logic (or precomputed values) out of the comparator or inlining it to avoid per-item function creation and improve readability.
- In `extensionPreferenceStorage.mjs`, `resolveStorage` currently returns any non-nullish `storage` argument without validation; consider narrowing this to a `Storage`-like interface (or adding a simple shape check) so accidental non-storage values fail fast instead of causing subtle runtime issues when `getItem`/`setItem` are called.

## Individual Comments

### Comment 1
<location path="dashboard/src/views/extension/useExtensionPage.js" line_range="443" />
<code_context>
         installedAtTimestamp: getInstalledAtTimestamp(plugin),
       }))
       .sort((left, right) => {
-        const fallbackNameCompare = compareInstalledPluginNames(
-          left.plugin,
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared comparator logic and update-pinning into small helper functions to simplify and clarify the sort comparator’s control flow.

You can simplify the comparator noticeably by pulling the shared pieces into small helpers and reusing them. That keeps the new feature, but makes the flow linear and easier to scan.

Also, the current condition:

```js
if (
  !pinUpdatesOnTop.value &&
  installedSortBy.value !== "update_status"
) {
  // ...
}
```

seems inverted compared to the comment “Pinning updates is a primary grouping…”. It likely should be `pinUpdatesOnTop.value && …` (without the leading `!`).

### 1. Extract shared fallback comparator

Instead of redefining `getFallbackResult` per call, move it out as a pure helper:

```js
const compareInstalledFallback = (left, right) => {
  const nameCompare = compareInstalledPluginNames(left.plugin, right.plugin);
  if (nameCompare !== 0) return nameCompare;
  return left.index - right.index;
};
```

Then in `sortInstalledPlugins`:

```js
.sort((left, right) => {
  // ... pinning logic ...

  if (installedSortBy.value === "install_time") {
    // ...
    return timeDiff !== 0 ? timeDiff : compareInstalledFallback(left, right);
  }

  if (installedSortBy.value === "author") {
    // ...
    return authorCompare !== 0
      ? (installedSortOrder.value === "desc" ? -authorCompare : authorCompare)
      : compareInstalledFallback(left, right);
  }

  if (installedSortBy.value === "update_status") {
    // ...
    return updateDiff !== 0 ? updateDiff : compareInstalledFallback(left, right);
  }

  return compareInstalledFallback(left, right);
});
```

This removes the inner closure, reduces visual noise, and makes the branches symmetrical.

### 2. Extract update‑pinning helper

You can keep the “pin updates” concern clearly separated from the rest of the comparator:

```js
const compareUpdatePinning = (left, right) => {
  const leftHasUpdate = left.plugin?.has_update ? 1 : 0;
  const rightHasUpdate = right.plugin?.has_update ? 1 : 0;
  if (leftHasUpdate === rightHasUpdate) return 0;
  return rightHasUpdate - leftHasUpdate; // updates first
};
```

Then at the top of the sort callback:

```js
.sort((left, right) => {
  if (pinUpdatesOnTop.value && installedSortBy.value !== "update_status") {
    const pinCompare = compareUpdatePinning(left, right);
    if (pinCompare !== 0) return pinCompare;
  }

  // existing installedSortBy branches using compareInstalledFallback(...)
});
```

This way the comparator reads as:

1. optional primary grouping by update pinning,
2. primary sort by chosen field,
3. shared fallback comparator,

without mixing those concerns inline.
</issue_to_address>

### Comment 2
<location path="dashboard/src/views/extension/extensionPreferenceStorage.mjs" line_range="9" />
<code_context>
+ * Resolve the storage backend for preference helpers.
+ * Pass `null` to explicitly disable storage access in callers/tests.
+ */
+const resolveStorage = (storage) => {
+  if (storage === null) {
+    return null;
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the storage helper API to a binary override-or-localStorage contract and let the boolean helpers focus solely on preference logic.

The main complexity comes from the tri‑state `resolveStorage` API (`undefined`/`null`/custom) and threading that through all helpers. You can keep all current functionality (including injection/override and safe fallback) with a simpler, binary contract:

- `storageOverride` provided → use it.
- Otherwise → try `window.localStorage`, fall back to `null` on error.

You can also drop the special meaning of `null` and handle “disable storage” purely in tests (by stubbing/mocking `window.localStorage`).

### Suggested refactor

Replace `resolveStorage` with a simpler helper and keep `readBooleanPreference` / `writeBooleanPreference` focused on boolean logic:

```ts
const getPreferenceStorage = (storageOverride?: Storage | null): Storage | null => {
  if (storageOverride) {
    return storageOverride;
  }
  if (typeof window === "undefined") {
    return null;
  }
  try {
    return window.localStorage ?? null;
  } catch {
    return null;
  }
};
```

Then update the callers:

```ts
export const readBooleanPreference = (
  key: string,
  fallback: boolean,
  storageOverride?: Storage | null,
): boolean => {
  const storage = getPreferenceStorage(storageOverride);
  if (!storage) return fallback;

  try {
    const saved = storage.getItem(key);
    if (saved === "true") return true;
    if (saved === "false") return false;
    return fallback;
  } catch {
    return fallback;
  }
};

export const writeBooleanPreference = (
  key: string,
  value: boolean,
  storageOverride?: Storage | null,
): void => {
  const storage = getPreferenceStorage(storageOverride);
  if (!storage) return;

  try {
    storage.setItem(key, String(value));
  } catch {
    // Ignore restricted storage environments.
  }
};
```

In tests:

- To disable storage entirely, stub `window.localStorage` to `null` or a throwing object instead of passing `null` into the API.
- To inject a fake storage, pass it as `storageOverride` (no special `null` semantics needed).

This reduces branching and mental load while preserving all current behaviors: safe access, injectable storage, and boolean parsing.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread dashboard/src/views/extension/useExtensionPage.js
Comment thread dashboard/src/views/extension/extensionPreferenceStorage.mjs Outdated
@zouyonghe
Copy link
Copy Markdown
Member

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="dashboard/src/views/extension/extensionPreferenceStorage.mjs" line_range="5" />
<code_context>
+export const PLUGIN_LIST_VIEW_MODE_STORAGE_KEY = "pluginListViewMode";
+export const PIN_UPDATES_ON_TOP_STORAGE_KEY = "pinUpdatesOnTop";
+
+const hasStorageMethod = (storage, methodName) =>
+  storage != null && typeof storage[methodName] === "function";
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the generic `resolveStorage`/`hasStorageMethod` abstraction with explicit read/write storage helpers to make the control flow clearer and easier to follow.

You can keep the same behaviors while simplifying the indirection around `methodName`/`hasStorageMethod` and making `readBooleanPreference` / `writeBooleanPreference` easier to follow.

### Simplify storage resolution into explicit read/write helpers

Instead of a generic `resolveStorage(storage, methodName)` plus `hasStorageMethod`, you can have two small helpers with the method checks inlined. This removes the string-based `methodName` plumbing and makes the control flow more direct, while preserving:

- `storage === null` → explicitly disable storage
- `storage !== undefined` → custom storage (tests/injection)
- `storage === undefined` → auto-detect `window.localStorage` where available
- Safe behavior in restricted environments

Example:

```js
const getStorageForRead = (storageOverride) => {
  if (storageOverride === null) {
    return null;
  }
  if (storageOverride !== undefined) {
    return typeof storageOverride?.getItem === "function" ? storageOverride : null;
  }
  if (typeof window === "undefined") {
    return null;
  }
  try {
    const localStorage = window.localStorage ?? null;
    return typeof localStorage?.getItem === "function" ? localStorage : null;
  } catch {
    return null;
  }
};

const getStorageForWrite = (storageOverride) => {
  if (storageOverride === null) {
    return null;
  }
  if (storageOverride !== undefined) {
    return typeof storageOverride?.setItem === "function" ? storageOverride : null;
  }
  if (typeof window === "undefined") {
    return null;
  }
  try {
    const localStorage = window.localStorage ?? null;
    return typeof localStorage?.setItem === "function" ? localStorage : null;
  } catch {
    return null;
  }
};
```

Then the preference helpers read more linearly, without the extra abstraction:

```js
export const readBooleanPreference = (key, fallback, storage) => {
  const targetStorage = getStorageForRead(storage);
  if (!targetStorage) {
    return fallback;
  }

  try {
    const saved = targetStorage.getItem(key);
    if (saved === "true") return true;
    if (saved === "false") return false;
    return fallback;
  } catch {
    return fallback;
  }
};

export const writeBooleanPreference = (key, value, storage) => {
  const targetStorage = getStorageForWrite(storage);
  if (!targetStorage) {
    return;
  }

  try {
    targetStorage.setItem(key, String(value));
  } catch {
    // Ignore restricted storage environments.
  }
};
```

This keeps all existing behaviors (including explicit disabling via `null`) but reduces cognitive load by avoiding generic `methodName` and `hasStorageMethod` indirection.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread dashboard/src/views/extension/extensionPreferenceStorage.mjs Outdated
@zouyonghe
Copy link
Copy Markdown
Member

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The compareInstalledUpdatePinning + update_status branch currently inverts arguments to flip sort order; consider passing a sort direction multiplier or boolean into a single comparison function to make the intent clearer and reduce the chance of subtle ordering bugs.
  • The getStorageForRead and getStorageForWrite helpers share a lot of structure; extracting a single parametrized helper (e.g., passing the required method name) would remove duplication and make future changes to storage resolution logic easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `compareInstalledUpdatePinning` + `update_status` branch currently inverts arguments to flip sort order; consider passing a sort direction multiplier or boolean into a single comparison function to make the intent clearer and reduce the chance of subtle ordering bugs.
- The `getStorageForRead` and `getStorageForWrite` helpers share a lot of structure; extracting a single parametrized helper (e.g., passing the required method name) would remove duplication and make future changes to storage resolution logic easier to maintain.

## Individual Comments

### Comment 1
<location path="dashboard/src/views/extension/extensionPreferenceStorage.mjs" line_range="9" />
<code_context>
+ * Resolve the storage backend for reading preferences.
+ * Pass `null` to explicitly disable storage access in callers/tests.
+ */
+const getStorageForRead = (storageOverride) => {
+  if (storageOverride === null) {
+    return null;
</code_context>
<issue_to_address>
**issue (complexity):** Consider unifying the storage resolver into a single function and moving read/write capability checks to the call sites to reduce duplication and narrow the API surface.

You can reduce duplication and surface area by collapsing `getStorageForRead`/`getStorageForWrite` into a single resolver and doing the capability checks at the call sites.

This keeps all current semantics intact, including:

- `storage === null` → explicitly disable storage
- `storage === undefined` → auto-detect `window.localStorage` if available
- `storage` object → use override (even if it only implements one of `getItem`/`setItem`)

Example refactor:

```ts
const getSafeLocalStorage = () => {
  if (typeof window === "undefined") return null;
  try {
    return window.localStorage ?? null;
  } catch {
    return null;
  }
};

const resolveStorage = (storageOverride?: Storage | null) => {
  if (storageOverride === null) return null;
  if (storageOverride !== undefined) return storageOverride;
  return getSafeLocalStorage();
};

export const readBooleanPreference = (
  key: string,
  fallback: boolean,
  storage?: Storage | null,
) => {
  const targetStorage = resolveStorage(storage);
  if (!targetStorage || typeof targetStorage.getItem !== "function") {
    return fallback;
  }

  try {
    const saved = targetStorage.getItem(key);
    if (saved === "true") return true;
    if (saved === "false") return false;
    return fallback;
  } catch {
    return fallback;
  }
};

export const writeBooleanPreference = (
  key: string,
  value: boolean,
  storage?: Storage | null,
) => {
  const targetStorage = resolveStorage(storage);
  if (!targetStorage || typeof targetStorage.setItem !== "function") {
    return;
  }

  try {
    targetStorage.setItem(key, String(value));
  } catch {
    // Ignore restricted storage environments.
  }
};
```

This:

- Removes the duplicated resolver logic and repeated `try/catch` blocks.
- Keeps the `null` override semantics and testability.
- Localizes the “can I read/write?” checks to the `readBooleanPreference`/`writeBooleanPreference` functions, aligning with the actual capability used.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread dashboard/src/views/extension/extensionPreferenceStorage.mjs
@zouyonghe zouyonghe merged commit fb16e12 into AstrBotDevs:master Apr 21, 2026
21 checks passed
@Blueteemo Blueteemo deleted the fix/issue-7577-plugin-update-pin branch April 21, 2026 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:webui The bug / feature is about webui(dashboard) of astrbot. feature:plugin The bug / feature is about AstrBot plugin system. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]版本更新

2 participants