Skip to content

fix: application export skills #7788

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 10, 2026
Merged

fix: application export skills #7788

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
8 changes: 5 additions & 3 deletions ui/src/views/application/application-card/action-dropdown/ExportApplicationAction.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ const isSystem = computed(() => !('putMoveApplication' in props.api))
const canExportApplication = computed(() =>
isSystem.value ? perm.application.system.export() : perm.application.workspace.export(props.application.id),
)
const canExportSkills = computed(() =>
isSystem.value ? perm.application.system.exportSkills() : perm.application.workspace.exportSkills(props.application.id),
const canExportSkills = computed(
() =>
props.application.is_publish &&
(isSystem.value ? perm.application.system.exportSkills() : perm.application.workspace.exportSkills(props.application.id)),
)

// 二级菜单留在所属一级浮层内，避免焦点移入时关闭一级菜单，并避开滚动容器裁剪。
Expand Down Expand Up @@ -86,7 +88,7 @@ function handleExportApplication() {
</script>

<template>
<MkDropdownItem @click.prevent.stop class="p-0!">
<MkDropdownItem v-if="canExportApplication || canExportSkills" @click.prevent.stop class="p-0!">
<MkDropdown class="w-full" trigger="hover" placement="right-start" :append-to="exportMenuContainer">
<!-- 展开智能体导出菜单 -->
<div ref="exportTriggerRef" class="flex-between w-full gap-2 p-2">

@shaohuzhang1 shaohuzhang1 Nov 10, 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 an analysis of the provided code snippet, focusing on irregularities, potential issues, and optimizations.

1. Potential Logic Issue: canExportSkills Condition

Issue:
In the canExportSkills computed property, you added a check for props.application.is_publish.

const canExportSkills = computed(
  () =>
    props.application.is_publish && // <--- New condition
    (isSystem.value ? perm.application.system.exportSkills() : perm.application.workspace.exportSkills(props.application.id)),
)

Risk:

  • If props.application is undefined or null, accessing .is_publish will throw a runtime error. Ensure props.application is guaranteed to exist elsewhere or add optional chaining (props.application?.is_publish).
  • Business Logic: Does un-published applications really not allow exporting skills? If this is intended behavior, it is fine. If "exporting skills" should still be possible for non-published apps, this logic blocks it unnecessarily.

2. Template Logic Inconsistency

Issue:
In the template, the dropdown toggle visibility is conditioned by:

<MkDropdownItem v-if="canExportApplication || canExportSkills" ...>

However, inside the dropdown, there are likely separate options for "Export Application" and "Export Skills."

Problem:
If canExportApplication is false but canExportSkills is true, the dropdown opens. The user might see a menu with only one item ("Export Skills"), or worse, if the inner components don't individually check permissions, they might render disabled/broken buttons for Export Application.

Suggestion:
Ensure that the individual options inside the <MkDropdown> also respect their respective permissions. For example:

<!-- Inside MkDropdown -->
<MenuItem v-if="canExportApplication" @click="handleExportApplication">Export App</MenuItem>
<MenuItem v-if="canExportSkills" @click="handleExportSkills">Export Skills</MenuItem>

Without internal v-ifs, showing a parent menu based on || could lead to UX confusion if one permission fails while the other passes.

3. Code Style & Readability

Suggestion:
The canExportSkills computed property spans multiple lines awkwardly. While prettier might format it this way, it can be made cleaner for readability.

Optimized Version:

const canExportSkills = computed(() => {
  // Early return if not published (clearer intent than long boolean chain)
  if (!props.application?.is_publish) {
    return false;
  }

  return isSystem.value
    ? perm.application.system.exportSkills()
    : perm.application.workspace.exportSkills(props.application.id);
});

4. Dependency Safety (props.application.id)

Issue:
In canExportApplication:

perm.application.workspace.export(props.application.id)

If isSystem is false, we assume a workspace context. If props.application is undefined here, it crashes.

Best Practice:
Ideally, canExportApplication and canExportSkills should share a common safety check or rely on a precomputed safe state object for the application data.

Summary of Recommendations

  1. Add Optional Chaining/Checks: Ensure props.application exists before accessing .is_publish or .id.
  2. Internal Menu Filtering: Verify that child elements inside MkDropdown are conditionally rendered based on their specific permissions (canExportApplication vs canExportSkills), not just the combined flag. This prevents displaying empty/disabled states incorrectly.
  3. Readability: Use early returns in canExportSkills to handle the is_publish gate clearly.

Corrected/Cleaned Code Example

// Script section
const canExportApplication = computed(() => {
  if (!props.application) return false; // Safety guard
  return isSystem.value 
    ? perm.application.system.export() 
    : perm.application.workspace.export(props.application.id);
});

const canExportSkills = computed(() => {
  // Ensure app exists and is published
  if (!props.application?.is_publish) return false;
  
  return isSystem.value 
    ? perm.application.system.exportSkills() 
    : perm.application.workspace.exportSkills(props.application.id);
});
<!-- Template section -->
<template>
  <!-- Parent visibility relies on at least one action being available -->
  <MkDropdownItem 
    v-if="canExportApplication || canExportSkills" 
    @click.prevent.stop 
    class="p-0!"
  >
    <MkDropdown 
      class="w-full" 
      trigger="hover" 
      placement="right-start" 
      :append-to="exportMenuContainer"
    >
      <!-- Option 1: Export Application -->
      <div 
        v-if="canExportApplication" 
        class="flex-between w-full gap-2 p-2"
        @click="handleExportApplication"
      >
        <!-- Label/icons -->
      </div>

      <!-- Option 2: Export Skills -->
      <div 
        v-if="canExportSkills" 
        class="flex-between w-full gap-2 p-2"
        @click="handleExportSkills"
      >
        <!-- Label/icons -->
      </div>
      
      <!-- Note: Adjust structure based on actual component hierarchy -->
    </MkDropdown>
  </MkDropdownItem>
</template>

Expand Down
Loading