Skip to content

fix: switch for the knowledge image list #7892

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.

Sign up for GitHub

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

Merged
peslyn merged 1 commit into from
Nov 13, 2026
+1 −1
Merged

fix: switch for the knowledge image list #7892

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ui/src/views/knowledge-detail/library/image/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ onBeforeUnmount(() => {
<el-switch
:model-value="row.is_active"
size="small"
:disabled="operationLoading && !canImage('edit')"
:disabled="operationLoading || !canImage('edit')"
:before-change="() => handleChangeActive(row)"
/>

@shaohuzhang1 shaohuzhang1 Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The change from && to || is a logic bug and likely incorrect.

Analysis of the original (&&):

  • Switch is disabled only when data is loading AND the user lacks edit permission.
  • If not loading, or if user has permission, the switch stays enabled.
  • This allows a user with edit rights to toggle even during loading (which can be risky), but lets users see/toggle when idle.

Analysis of the new (||):

  • Switch is disabled when data is loading OR user lacks edit permission.
  • More importantly: if the user lacks edit permission, it's always disabled — good.
  • But during loading, it's disabled for everyone — also reasonable.
  • The real question is intent: does canImage('edit') mean "user CAN edit"? If yes, then !canImage('edit') = "cannot edit" → || = "disabled if cannot edit OR loading" — this is arguably the more correct/intuitive behavior.

Potential issues to verify:

  1. Semantics of canImage('edit'): Confirm it returns true when the user has permission. If it actually returns whether an image editing operation is allowed (naming is ambiguous), negating it may behave unexpectedly.

  2. operationLoading scope: Is it per-row or global? If global, one row's save disables switches on all rows. Consider a per-row loading flag (e.g., row._loading) for better UX.

  3. before-change vs disabled redundancy: handleChangeActive(row) presumably performs the API call. With :disabled, the switch won't fire before-change when disabled — so make sure permission checks in handleChangeActive are still present as defense-in-depth (server-side authorization must exist regardless).

  4. Read-only display: When disabled due to lack of permission, consider showing a tooltip explaining why (better UX than a silently inert control).

Optimization suggestion:

:disabled="isRowBusy(row) || !canImage('edit')"

Create a helper isRowBusy(row) that tracks per-row operations rather than a shared operationLoading, avoiding cross-row lock-outs.

Verdict: The || change is defensible and probably intentional (fixing cases where unauthorized users could interact), but verify: (a) the meaning of canImage('edit'), and (b) whether operationLoading is global — if global, prefer per-row loading to avoid disabling unrelated rows.

Expand Down
Loading