[shaohuzhang1]

fix: knowledge problem permission

[shaohuzhang1]

Here's my review of the code changes:

Issues & Suggestions

1. Significant flaw: knowledgeId.value usage in a plain function

return Boolean(perm.knowledge.workspace[method](knowledgeId.value))

• knowledgeId is referenced but not shown as defined in this diff. If it's a ref declared after this function (e.g., below in <script setup>), this works at call time due to hoisting/closures — but it's fragile and confusing.
• If knowledgeId is not a ref (e.g., a prop or computed differently), .value will be undefined. Verify its type.
• Suggestion: Move the function below the knowledgeId declaration, or pass knowledgeId as an explicit parameter for clarity.

2. Style: missing spaces after commas

import { isWorkspaceSharedResource,isSystemResource, isSystemSharedResource } from '@/utils/resource-context'

Inconsistent spacing (isWorkspaceSharedResource,isSystemResource). Run Prettier/ESLint.

3. Missing return fallback / exhaustive check

KnowledgeProblemPermissionMethod currently has only one method ('problemRelate'), so the fallthrough return for workspace is fine today. But:

• If more methods are added, be careful the perm.knowledge.* objects each expose the same keys.
• Consider a default guard or assertNever(method) pattern if this union grows.

4. v-if placement on <el-button>

Template:

<el-button plain :disabled="loading" v-if="knowledgeProblemPermission('problemRelate')" @click="emit('associate')">关联分段</el-button>

This is functionally OK, but:

• Attribute order: Conventionally put v-if first for readability: v-if="..." plain :disabled=....
• Performance/correctness caveat: knowledgeProblemPermission('problemRelate') is called on every render. Since it reads reactive refs (knowledgeId.value) and permission state, Vue will re-evaluate reliably, but if perm.knowledge.* calls have side effects or are expensive, consider wrapping in a computed:

  const canRelateProblem = computed(() => knowledgeProblemPermission('problemRelate'))

  Then use v-if="canRelateProblem".

5. isWorkspaceSharedResource() → false hardcoded

if (isWorkspaceSharedResource()) return false // workspace shared KB is read-only

Fine, but this bypasses any per-method logic. If later some methods should be allowed on workspace-shared resources, this becomes wrong. Document or extract into a map if it grows.

6. Naming: "问题" (problem) vs. variable problem

No functional issue, but knowledgeProblemPermission naming is consistent with the model name — good.

7. Redundant Boolean(...) casts

The Boolean() wrappers assume the perm.* methods may return non-boolean. That's defensive and OK, but if their signatures already guarantee boolean, the casts are noise. Keep only if you're unsure of the contract.

8. Order of checks

Current priority: system → system-shared → workspace-shared → workspace. Confirm that's intended precedence. A resource being both system and shared could match multiple predicates; make sure the priority matches business rules.

9. Import ordering

Imports are appended at the top-of-file region rather than sorted alongside existing @/ imports. Consistency with project import-order lint rule would fail CI if enforced.

Summary of recommended fixes

Priority	Fix
High	Confirm knowledgeId is a ref declared before use, or pass explicitly
Medium	Wrap permission check in computed for the template
Medium	Move v-if to be the first attribute
Low	Fix comma spacing, import ordering
Low	Add exhaustiveness handling if union grows

Otherwise the logic is sound and the feature (hiding "关联分段" when unauthorized) is implemented correctly.

[shaohuzhang1]

Here's my review of this diff:

Change summary: The permission check for rendering ProblemDetailDrawer was switched from 'problemRelate' to 'problemEdit'.

Potential issues / irregularities:

1. Behavioral change needs verification — This is a semantic change, not a cosmetic one. Users who had problemRelate but not problemEdit will lose access to the drawer. Confirm this is intentional and matches the intended UX (e.g., whether "relating" problems legitimately requires edit rights).

2. Permission granularity mismatch — Using problemEdit to control a "detail/relate" drawer couples read/relate operations to write permissions. If the drawer is primarily for viewing details, v-if on an edit-only permission may hide information users are otherwise allowed to see. Consider separating:

   • Viewing the drawer (problemRelate or a problemView)
   • Editing within it (problemEdit, checked inside the component or on action buttons)

3. Naming/semantics of knowledgeProblemPermission — Make sure this helper returns a boolean consistently (not undefined/truthy objects), since it's used directly in v-if.

4. Security note — Frontend v-if gating is UI-only; ensure the backend still enforces the same permission. A permission rename on the client without matching server checks can create inconsistent behavior.

5. Regression risk with the item above/below references — If hasPrevious, pagination, or navigation logic assumed the drawer would render under problemRelate, verify that flow still works when the user lacks problemEdit. Consider guarding related refs/methods (e.g., problemDetailDrawerRef) against being null.

6. No test update shown — Add/adjust a unit test asserting the drawer renders only with problemEdit (and does not for problemRelate-only users) to lock in the intent.

7. Minor optimization — Not applicable here; the expression is already short-circuited (detailProblem && ...). No performance concern.

Recommendation: If the goal is "only editors can open the detail drawer," the change is fine — just add tests and confirm backend parity. If the drawer serves viewers too, prefer keeping problemRelate for visibility and gate only mutating actions behind problemEdit.