Skip to content

feat: termbase edit #7764

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
fit2cloudwxx merged 1 commit into from
Nov 9, 2026
Merged

feat: termbase edit #7764

Show file tree
Hide file tree
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
5 changes: 5 additions & 0 deletions ui/src/api/admin/system/resource-management/knowledge/termbase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ const getTermbasePage = (knowledgeId: string, page: ParamsPage, query?: { conten
/** 删除单个知识库自定义分词条目。 */
const deleteTermbase = (knowledgeId: string, termbaseId: string) => del<undefined, boolean>(`${getPrefix(knowledgeId)}/${termbaseId}`)

/** 修改单个知识库自定义分词条目。 */
const putTermbase = (knowledgeId: string, termbaseId: string, payload: TermbaseUpdatePayload) =>
put<TermbaseUpdatePayload, void>(`${getPrefix(knowledgeId)}/${termbaseId}`, payload)

/** 批量删除知识库自定义分词条目。 */
const putBatchDeleteTermbases = (knowledgeId: string, termbaseIds: string[]) =>
put<string[], boolean>(`${getPrefix(knowledgeId)}/batch_delete`, termbaseIds)
Expand All @@ -26,6 +30,7 @@ export default {
postTermbases,
getTermbasePage,
deleteTermbase,
putTermbase,
putBatchDeleteTermbases,
postExportTermbases,
}

@shaohuzhang1 shaohuzhang1 Nov 9, 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.

Here is a review of the code snippet:

1. Naming Inconsistency (Critical for Readability)

  • Issue: The function name is putTermbase, but its neighboring functions use inconsistent verbs for HTTP methods:
    • getTerbbasePage (GET)
    • deleteTermbase (DELETE)
    • postTermbases/postExportTermbases (POST)
    • putBatchDeleteTermbases (PUT)
  • Suggestion: While putTermbase is technically correct for a PUT request, consider if this endpoint is actually for updating (which might be a PATCH or PUT) vs. replacing. More importantly, ensure the verb in the name matches the action. If putBatchDeleteTermbases exists, putTermbase implies a full replacement update. Ensure your team agrees on whether "update" maps to patchX or putX. Consistency is key.

2. Return Type Mismatch / API Contract Check

  • Issue: putTermbase returns void (put<TermbaseUpdatePayload, void>). However, deleteTermbase returns boolean, and putBatchDeleteTermbases also returns boolean.
  • Suggestion: Verify if the backend API truly returns no data (204 No Content or 200 with empty body). If it returns a boolean (like success/failure), change the generic type to boolean. If it returns 204, void is correct, but note that put from many HTTP clients resolves to undefined or the response object depending on configuration. Ensure your put helper handles 204 No Content gracefully when typed as void.

3. Lack of Input Validation

  • Issue: The payload: TermbaseUpdatePayload is passed directly. There is no check if TermbaseUpdatePayload is partially defined or empty.
  • Suggestion: If using TypeScript interfaces for TermbaseUpdatePayload, ensure it’s marked as Partial<Termbase> or explicitly defines optional fields to allow for PATCH-like behavior if intended. If it’s a full replacement (PUT), ensure all required fields are present in the type definition.

4. Export Consistency

  • Good Practice: Adding putTermbase to the default export object maintains consistency with other CRUD operations. This looks good.

5. Potential Optimization / Typo Check

  • Check: The comment says “修改...自定义分词条目” (Modify... custom termbase entry). Does “termbase” refer to a single term entry within a knowledge base? The name Termbase usually implies a whole database of terms. If this updates a single term, the naming might be misleading. Confirm if knowledgeId + termbaseId targets a single term or a sub-document.

Summary of Recommendations

✅ Code Structure: Good, follows existing patterns.
⚠️ Return Type: Confirm if the API returns void or boolean. Be consistent with deleteTermbase which returns boolean.
⚠️ Naming: Ensure put accurately reflects the HTTP method (PUT vs PATCH). If you intend partial updates, consider patchTermbase.
💡 Type Safety: Ensure TermbaseUpdatePayload is well-typed to prevent partial sends if the server expects a full object for PUT.

5 changes: 5 additions & 0 deletions ui/src/api/admin/system/shared-resources/knowledge/termbase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ const getTermbasePage = (knowledgeId: string, page: ParamsPage, query?: { conten
/** 删除单个知识库自定义分词条目。 */
const deleteTermbase = (knowledgeId: string, termbaseId: string) => del<undefined, boolean>(`${getPrefix(knowledgeId)}/${termbaseId}`)

/** 修改单个知识库自定义分词条目。 */
const putTermbase = (knowledgeId: string, termbaseId: string, payload: TermbaseUpdatePayload) =>
put<TermbaseUpdatePayload, void>(`${getPrefix(knowledgeId)}/${termbaseId}`, payload)

/** 批量删除知识库自定义分词条目。 */
const putBatchDeleteTermbases = (knowledgeId: string, termbaseIds: string[]) =>
put<string[], boolean>(`${getPrefix(knowledgeId)}/batch_delete`, termbaseIds)
Expand All @@ -26,6 +30,7 @@ export default {
postTermbases,
getTermbasePage,
deleteTermbase,
putTermbase,
putBatchDeleteTermbases,
postExportTermbases,
}

@shaohuzhang1 shaohuzhang1 Nov 9, 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.

Here is a review of your code snippet:

1. Inconsistency in Function Naming (Irregularity)

Issue: putBatchDeleteTermbases uses a PUT method but performs a delete operation logically (batch deletion). While technically valid if the API is designed that way, it is misleading.
Suggestion: Rename to deleteBatchTermbases or ensure the underlying HTTP method matches the semantic intent (usually DELETE for batch deletes, or POST if it's an action endpoint). If you must use PUT for this specific API contract, consider adding a comment explaining why PUT is used for deletion.

2. Redundant Payload Typing in putTermbase

Potential Issue: The generic type put<TermbaseUpdatePayload, void> sets the response type to void.
Suggestion: Confirm if the API actually returns no content (204 No Content or empty body). If it returns a confirmation object (like { success: true }), updating the return type would improve type safety. If it is strictly void, this is fine.

3. Missing Return Types / Async Handling

Observation: The functions appear to return Promises (based on common patterns like Axios or Fetch wrappers).
Suggestion: Explicitly add return types (e.g., Promise<void>) to ensure better developer experience and type checking when consuming these functions elsewhere.

// Better clarity
const putTermbase = async (...): Promise<void> => { ... }

4. Export Consistency

Good Practice: You correctly added putTermbase to the default export.
Suggestion: Consider whether you want to also export individual named exports along with the default object for more flexible imports (e.g., import { putTermbase } from './termbaseApi'). However, if your project standard is default exports only, this is fine as is.

Optimized & Cleaned Version Example

/** 修改单个知识库自定义分词条目。 */
const putTermbase = async (
  knowledgeId: string, 
  termbaseId: string, 
  payload: TermbaseUpdatePayload
): Promise<void> => {
  return put<TermbaseUpdatePayload, void>(
    `${getPrefix(knowledgeId)}/${termbaseId}`, 
    payload
  );
};

/** 
 * 批量删除知识库自定义分词条目。 
 * Note: Uses PUT due to backend API constraints.
 */
const putBatchDeleteTermbases = async (
  knowledgeId: string, 
  termbaseIds: string[]
): Promise<boolean> => {
  return put<string[], boolean>(
    `${getPrefix(knowledgeId)}/batch_delete`, 
    termbaseIds
  );
};

export default {
  postTermbases,
  getTermbasePage,
  deleteTermbase,
  putTermbase, // Added here
  putBatchDeleteTermbases,
  postExportTermbases,
};

Summary

  • ✅ Correct: New function added and exported properly.
  • ⚠️ Review: Check if putBatchDeleteTermbases naming/convention aligns with team standards.
  • ⚠️ Enhance: Add explicit async/return types for better TypeScript support.

5 changes: 5 additions & 0 deletions ui/src/api/admin/workspace/knowledge/termbase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ const getTermbasePage = (knowledgeId: string, page: ParamsPage, query?: { conten
/** 删除单个知识库自定义分词条目。 */
const deleteTermbase = (knowledgeId: string, termbaseId: string) => del<undefined, boolean>(`${getPrefix(knowledgeId)}/${termbaseId}`)

/** 修改单个知识库自定义分词条目。 */
const putTermbase = (knowledgeId: string, termbaseId: string, payload: TermbaseUpdatePayload) =>
put<TermbaseUpdatePayload, void>(`${getPrefix(knowledgeId)}/${termbaseId}`, payload)

/** 批量删除知识库自定义分词条目。 */
const putBatchDeleteTermbases = (knowledgeId: string, termbaseIds: string[]) =>
put<string[], boolean>(`${getPrefix(knowledgeId)}/batch_delete`, termbaseIds)
Expand All @@ -27,6 +31,7 @@ export default {
postTermbases,
getTermbasePage,
deleteTermbase,
putTermbase,
putBatchDeleteTermbases,
postExportTermbases,
}
47 changes: 41 additions & 6 deletions ui/src/views/knowledge-detail/termbase/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@
import { computed, onMounted, ref } from 'vue'
import { useRoute } from 'vue-router'
import type MkQuickCreate from '@/components/global/mk-table/MkQuickCreate.vue'
import MkEditName from '@/components/mk-edit-name/index.vue'
import type { ResourceDetailPageProps } from '@/layout/ResourceDetailLayout.vue'
import type { TermbaseItem } from '@/api/types'
import { datetimeFormat } from '@/utils/time'
import { MsgSuccess } from '@/utils/message'
import { useKnowledgeDetailContext } from '@/views/knowledge-detail/context'
import ButtonCreateTermbase from './components/ButtonCreateTermbase.vue'
import { isWorkspaceSharedResource,isSystemResource, isSystemSharedResource } from '@/utils/resource-context'
import { isWorkspaceSharedResource, isSystemResource, isSystemSharedResource } from '@/utils/resource-context'
import { perm } from '@/permission'

/**
* 知识库「词库」权限判断
*/
type KnowledgeTermbasePermissionMethod =
'termbaseCreate' | 'termbaseEdit' | 'termbaseDelete' | 'termbaseExport'
type KnowledgeTermbasePermissionMethod = 'termbaseCreate' | 'termbaseEdit' | 'termbaseDelete' | 'termbaseExport'

function knowledgeTermbasePermission(method: KnowledgeTermbasePermissionMethod): boolean {
if (isSystemResource()) return Boolean(perm.knowledge.system[method]())
Expand Down Expand Up @@ -81,6 +81,16 @@ function handleTermbasesCreated() {
return loadTermbases()
}

/* 词语原位编辑：保存接口不返回词条，刷新当前页同步更新时间。 */
function saveTermbaseContent(termbaseId: string, content: string): Promise<string> {
return (
resourceApi.termbase
.putTermbase?.(knowledgeId.value, termbaseId, { content })
.then(() => loadTermbases())
.then(() => content) ?? Promise.resolve(termbases.value.find(({ id }) => id === termbaseId)?.content ?? '')
)
}

/* 快捷创建与弹窗创建复用同一批量创建接口。 */
function handleQuickCreate(content: string) {
if (operationLoading.value) return
Expand Down Expand Up @@ -200,7 +210,18 @@ onMounted(() => {
/>
</template>
<el-table-column type="selection" width="40" />
<el-table-column prop="content" label="词语" show-overflow-tooltip />
<el-table-column prop="content" label="词语" show-overflow-tooltip>
<template #default="{ row }: { row: TermbaseItem }">
<!-- 编辑分词 -->
<MkEditName
:key="row.id"
v-model="row.content"
:maxlength="256"
:save="(content) => saveTermbaseContent(row.id, content)"
:disabled="isWorkspaceSharedResource() || !knowledgeTermbasePermission('termbaseEdit')"
/>
</template>
</el-table-column>
<el-table-column prop="create_time" label="创建时间" width="180">
<template #default="{ row }: { row: TermbaseItem }">
{{ datetimeFormat(row.create_time) }}
Expand All @@ -223,9 +244,23 @@ onMounted(() => {
</el-table-column>
<template #footer-batch-actions>
<!-- 批量导出词语 -->
<el-button v-if="knowledgeTermbasePermission('termbaseExport')" type="primary" plain :disabled="operationLoading" @click="handleExportTermbases">导出</el-button>
<el-button
v-if="knowledgeTermbasePermission('termbaseExport')"
type="primary"
plain
:disabled="operationLoading"
@click="handleExportTermbases"
>
导出
</el-button>
<!-- 批量删除词语 -->
<el-button v-if="!isWorkspaceSharedResource() && knowledgeTermbasePermission('termbaseDelete')" type="danger" plain :loading="operationLoading" @click="handleBatchDeleteTermbases">
<el-button
v-if="!isWorkspaceSharedResource() && knowledgeTermbasePermission('termbaseDelete')"
type="danger"
plain
:loading="operationLoading"
@click="handleBatchDeleteTermbases"
>
删除
</el-button>
</template>

@shaohuzhang1 shaohuzhang1 Nov 9, 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.

Review

1. saveTermbaseContent – error/edge-case handling (main concern)

function saveTermbaseContent(termbaseId: string, content: string): Promise<string> {
  return (
    resourceApi.termbase
      .putTermbase?.(knowledgeId.value, termbaseId, { content })
      .then(() => loadTermbases())
      .then(() => content) ?? Promise.resolve(...)
  )
}

Issues:

  • Optional chaining + ?? is fragile. If putTermbase is undefined, ?? kicks in and silently skips saving, then refetches and returns whatever is already in termbases. The edit "succeeds" invisibly — very confusing for users. Prefer an explicit guard that throws/returns a rejected promise if the API is unavailable.
  • Return value inconsistency. On success it returns content; on the fallback branch it returns the row's existing content. Since loadTermbases() was just called, termbases.value.find(...) may be a different object than before. Both callers of MkEditName's save should get the value actually persisted — ideally the server-returned content if putTermbase returns data, otherwise rely on the reloaded row.
  • No error handling / loading state. If putTermbase rejects, the chain rejects but MkEditName isn't guaranteed to revert the model. Consider catching, showing an error message, and rethrowing or returning the previous value so the UI reverts predictably.
  • v-model="row.content" mutates the row object directly before the server confirms. That's fine if MkEditName reverts on failure, but combined with the full loadTermbases() refetch you're double-writing the same field. Simplest fix: after reload, don't rely on local mutation — let the refreshed list be the source of truth.

Suggested shape:

async function saveTermbaseContent(termbaseId: string, content: string): Promise<string> {
  if (!resourceApi.termbase.putTermbase) {
    throw new Error('putTermbase API is not available')
  }
  await resourceApi.termbase.putTermbase(knowledgeId.value, termbaseId, { content })
  await loadTermbases()
  return termbases.value.find(({ id }) => id === termbaseId)?.content ?? content
}

(Wrapping loadTermbases in try/catch if you want the reload failure not to mask a successful save.)

2. Full-list reload on every single-word save

loadTermbases() after each inline edit re-fetches the whole table (and presumably loses selection/pagination position depending on implementation). For inline editing this is heavy. Better options:

  • Have putTermbase return the updated TermbaseItem and patch it into termbases.value locally.
  • Or update row.create_time/update_time in place from the response, avoiding a network round trip entirely.

If a reload is unavoidable (e.g. backend recomputes create_time), at least confirm loadTermbases preserves current page/filters.

3. Comment mentions create_time

The comment says "刷新当前页同步更新时间" (sync update time), but the table shows create_time. Ensure the column being displayed is actually what changes. Minor, but misleading comments age badly.

4. :key="row.id" on MkEditName

Good practice to force remount when rows are recycled, but note this also discards internal editing state if the list reorders. Make sure MkEditName doesn't hold half-committed state that would be lost on a background refresh.

5. Permission reactivity

:disabled="isWorkspaceSharedResource() || !knowledgeTermbasePermission('termbaseEdit')"

These are plain function calls, not computed, so they won't re-evaluate reactively unless something else triggers a re-render. This matches the existing pattern (v-if="knowledgeTermbasePermission(...)" is used the same way), so not a regression — but if permissions/resources can change without a full remount, wrap in computed.

6. maxlength="256" hard-coded

Consider extracting to a shared constant if the backend limit is enforced elsewhere; a mismatch will surface as a server-side error only.

7. Minor / style

  • Type alias reformatting and import-order fix (isWorkspaceSharedResource,isSystemResource → space) are fine.
  • Long <el-button> reformatting is cosmetic. No issues.
  • @click="handleExportTermbases" / handleBatchDeleteTermbases — fine as-is.
  • The added MkEditName import is alphabetically placed, good.

Summary of actionable items

  1. Replace ?. + ?? with an explicit availability check; don't silently no-op saves.
  2. Unify the returned value between success and fallback paths.
  3. Add failure handling so the UI reverts on save error.
  4. Avoid full loadTermbases() per inline edit if the API can return the updated item.
  5. Fix the "update time" comment to match the create_time column, or clarify.

Expand Down
Loading