[shaohuzhang1]

【【资源授权】角色（继承工作空间管理员）授权知识库「自定义分词」编辑权限，登陆该账号自定义分词没有编辑按钮出现，无法编辑】
https://www.tapd.cn/tapd_fe/45245355/bug/detail/1145245355001078987

[shaohuzhang1]

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.

[shaohuzhang1]

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.

[shaohuzhang1]

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.