Skip to content
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

perf: optimization workflow interactive #2481

Merged
merged 1 commit into from
Mar 4, 2025
Merged

perf: optimization workflow interactive #2481

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
49 changes: 30 additions & 19 deletions ui/src/views/application-workflow/index.vue
View file
Original file line number Diff line number Diff line change
@@ -174,20 +174,25 @@ const isSave = ref(false)
const showHistory = ref(false)
const disablePublic = ref(false)
const currentVersion = ref<any>({})
const cloneWorkFlow = ref(null)

function back() {
MsgConfirm(t('common.tip'), t('views.applicationWorkflow.tip.saveMessage'), {
confirmButtonText: t('views.applicationWorkflow.setting.exitSave'),
cancelButtonText: t('views.applicationWorkflow.setting.exit'),
type: 'warning',
distinguishCancelAndClose: true
})
.then(() => {
saveApplication(true, true)
})
.catch((action: Action) => {
action === 'cancel' && router.push({ path: `/application/${id}/WORK_FLOW/overview` })
if (JSON.stringify(cloneWorkFlow.value) !== JSON.stringify(getGraphData())) {
MsgConfirm(t('common.tip'), t('views.applicationWorkflow.tip.saveMessage'), {
confirmButtonText: t('views.applicationWorkflow.setting.exitSave'),
cancelButtonText: t('views.applicationWorkflow.setting.exit'),
type: 'warning',
distinguishCancelAndClose: true
})
.then(() => {
saveApplication(true, true)
})
.catch((action: Action) => {
action === 'cancel' && router.push({ path: `/application/${id}/WORK_FLOW/overview` })
})
} else {
router.push({ path: `/application/${id}/WORK_FLOW/overview` })
}
}
function clickoutsideHistory() {
if (!disablePublic.value) {
@@ -357,6 +362,7 @@ function getDetail() {
workflowRef.value?.clearGraphData()
nextTick(() => {
workflowRef.value?.render(detail.value.work_flow)
cloneWorkFlow.value = getGraphData()
})
})
}
@@ -366,15 +372,20 @@ function saveApplication(bool?: boolean, back?: boolean) {
work_flow: getGraphData()
}
loading.value = back || false
application.asyncPutApplication(id, obj).then((res) => {
saveTime.value = new Date()
if (bool) {
MsgSuccess(t('common.saveSuccess'))
if (back) {
router.push({ path: `/application/${id}/WORK_FLOW/overview` })
application
.asyncPutApplication(id, obj)
.then((res) => {
saveTime.value = new Date()
if (bool) {
MsgSuccess(t('common.saveSuccess'))
if (back) {
router.push({ path: `/application/${id}/WORK_FLOW/overview` })
}
}
}
})
})
.catch(() => {
loading.value = false
})
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Code Review and Suggestions:

Irregularities and Potential Issues:

  1. Variable Naming: cloneWorkFlow appears to be intended to hold a copy of the graph data, but it's not assigned anywhere after initialization.

  2. Redundant Logic for Save Check:

    • The logic inside back() that checks changes against getGraphData() is redundant because saveApplication already handles updating only when there are changes.

  3. Synchronization Issue: In saveApplication, you're setting loading.value = back || false. If bool is true, loading should reflect this value correctly.

Optimization Suggestions:

  1. Use watchEffect() for Graph Data Changes:
    Instead of performing the save operation manually on each change, consider using Vue's reactive watchers to detect graph changes automatically.

    watchEffect(() => {
  if (JSON.stringify(graphData.value) !== JSON.stringify(getGraphData())) {
    // Perform save operations here
    saveApplication(true, true);
  }
});

    This approach ensures that saving occurs whenever the graph state changes without manual intervention.

  2. Remove Redundancy in Click Outside History Functionality:
    Remove unnecessary checks within clickoutsideHistory.

    function clickoutsideHistory() {
  router.push({ path: `/application/${id}/WORK_FLOW/overview` });
}

By making these optimizations, the code will become more efficient, readable, and maintainable. Additionally, using watchEffect() can simplify error handling related to network requests since Vue handles re-rendering and updates seamlessly during such conditions.

13 changes: 5 additions & 8 deletions ui/src/workflow/common/data.ts
View file
Original file line number Diff line number Diff line change
@@ -17,17 +17,14 @@ export const startNode = {
}
],
globalFields: [
{ label: t('views.applicationWorkflow.nodes.startNode.currentTime'), value: 'time' },
{
value: 'time',
label: t('views.applicationWorkflow.nodes.startNode.currentTime')
label: t('views.application.applicationForm.form.historyRecord.label'),
value: 'history_context'
},
{
value: 'history_context',
label: t('views.application.applicationForm.form.historyRecord.label')
},
{
value: 'chat_id',
label: t('chat.chatId')
label: t('chat.chatId'),
value: 'chat_id'
}
]
},
Copy link
Contributor

Choose a reason for hiding this comment

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

There are several areas in the code that could be improved:

  1. Unnecessary Comments and Empty Lines:

    • The empty lines between sections can be removed.
    • Remove unnecessary comments.

  2. Duplicate Values:

    • In globalFields, there is duplicate entry for both 'time' with two different labels. These should either have distinct labels or one labeled as appropriate.

  3. Variable Naming Convention:

    • Ensure variable names follow consistent naming conventions (e.g., camelCase vs PascalCase).

  4. Code Formatting:

    • Consider aligning multi-line strings to maintain consistency.

Here's refactored version of the code:

export const startNode = {
    conditions: [
        { name: 'current_date', fieldNames: ['date'] },
        { name: 'past_dates', fieldNames: ['period'] }
    ],
    globalFields: [
        { label: t('views.applicationWorkflow.nodes.startNode.currentTime'), value: 'time' }, // Use distinct labels if needed
        { label: t('chat.chatId'), value: 'chat_id' }                                                // Rename similar values consistently
    ]
};

This refactoring simplifies the structure while addressing potential duplication and clarity issues. Let me know if you need further adjustments!

20 changes: 11 additions & 9 deletions ui/src/workflow/nodes/variable-assign-node/index.vue
View file
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@
ref="replyNodeFormRef"
hide-required-asterisk
>
<el-scrollbar max-height="750" @wheel="wheel">
<template v-for="(item, index) in form_data.variable_list" :key="item.id">
<el-card shadow="never" class="card-never mb-8" style="--el-card-padding: 12px">
<el-form-item :label="$t('views.applicationWorkflow.variable.label')">
@@ -127,17 +128,18 @@
/>
</el-form-item>
</div>

<NodeCascader
v-else
ref="nodeCascaderRef2"
:nodeModel="nodeModel"
class="w-full"
:placeholder="$t('views.applicationWorkflow.variable.placeholder')"
v-model="item.reference"
/>
<el-form-item v-else>
<NodeCascader
ref="nodeCascaderRef2"
:nodeModel="nodeModel"
class="w-full"
:placeholder="$t('views.applicationWorkflow.variable.placeholder')"
v-model="item.reference"
/>
</el-form-item>
</el-card>
</template>
</el-scrollbar>
<el-button link type="primary" @click="addVariable">
<el-icon class="mr-4">
<Plus />
Copy link
Contributor

Choose a reason for hiding this comment

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

The code appears to be well-formed according to ESLint rules. Here are some minor optimizations and improvements:

  1. Consistent Indentation: Ensure consistent indentation throughout the code block.

  2. Remove Redundant Lines: The hide-required-asterisk attribute is repeated without a reason. Remove it if not needed.

  3. Correct Closing Braces: Ensure that all opening braces { have their corresponding closing braces } properly matched.

here's the optimized version of your code:

<template>
  <el-row :gutter="16" justify="space-around">
    <el-col
      v-for="(item, index) in formData.variableList"
      :key="item.id"
      :span="8"
    >
      <el-card shadow="never">
        <el-form-item label="$t('views.applicationWorkflow.variable.label')">
          <!-- Form item contents -->
        </el-form-item>
        <div v-if="... condition...">
          <!-- ... -->
        </div>
        <el-form-item v-else>
          <NodeCascader
            ref="nodeCascaderRef2"
            :nodeModel="nodeModel"
            class="w-full"
            :placeholder="$t('views.applicationWorkflow.variable.placeholder')"
            v-model="item.reference"
          />
        </el-form-item>
      </el-card>
    </el-col>
  </el-row>
</template>

<script setup>
import NodeCascader from '@/components/NodeCascader.vue';
import { Plus } from '@element-plus/icons-vue';

const formDa = reactive({
  variableList: [...],
});

function addVariable() {
  // Add logic here
}
</script>

<style lang="scss">
/* Your styles here */
</style>

Key Changes:

  • Removed unnecessary lines like ref="replyNodeFormRef" as it seems redundant with other attributes.
  • ensured proper spacing around operators where necessary (e.g., after semicolons).
  • Fixed any misaligned syntax errors within template loops or conditional statements.

This keeps the code clean and maintainable while adhering to standard coding practices and ESLint conventions.