fix: Loop node, MCP node cannot be dragged and added

[shaohuzhang1]

fix: Loop node, MCP node cannot be dragged and added

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

Your code snippet appears to have several issues that need addressing:

Issues Identified:

1. Duplicate McpServerInputDialog Component Imports:

   • There is a duplicate import line for McpServerInputDialog at lines 246 and 334.

   +import McpServerInputDialog fro "./component/McpServerInputDialog.vue";

   • This should be removed to avoid redundancy.

2. Potential Typo in Method Definition:

   • In the _getTools method, there might be confusion between loading and possibly another variable due to its usage in the loadSharedApi call.
   • Ensure consistency and correctness of variable names.

3. Regular Expression Usage:

   • The regular expression used in extractPlaceholders looks correct but can improve readability slightly:

     const re = /\{\{\s*([a-zA-Z_][\w.]*)\s*\}\}/g;

4. Function Parameter Consistency:

   • In the code snippet where you define functions like handleMcpVariables, ensure consistent naming conventions and parameters for clarity:

     function _getTools(mcp_servers: any) {
       // Function implementation here
     }
     
     function handleMcpVariables(vars: any) {
       // Function implementation here
     }

5. Logical Errors:

   • Ensure logical flow makes sense, especially with conditions and assignments related to node properties (node_model.properties.node_data).

General Recommendations:

• Avoid Redundant Code: Remove the second import statement for McpServerInputDialog.

• Check Loading Variable Usage: Review how loading is being used in loadSharedApi. It seems potentially redundant or incorrectly named based on the context provided.

• Improve Regex Readability: While not critical, breaking down complex regex patterns into smaller components could enhance readability.

• Consistent Naming: Keep parameter names and variables consistently named for better understanding and maintainability.

// Example updated version without duplicates
<template>
...
<McpServerInputDialog ref="mcpServerInputDialogRef" @refresh="handleMcpVariables" />
</template>

<script setup lang="ts">
import { cloneDeep, set } from 'lodash'
import { t } from '@/locales'
import { MsgError, MsgSuccess } from '@/utils/message'
import TooltipLabel from '@/components/dynamics-form/items/label/TooltipLabel.vue'
import NodeCascader from '@/workflow/common/NodeCascader.vue'
+import McpServerInputDialog from './component/McpServerInputDialog.vue'; // No longer imported twice

import { useRoute } from 'vue-router';
import { loadSharedApi } from '@/utils/dynamics-api/shared-api' 
...
</script>

If this information does not fully resolve your problem, please provide additional details or error messages for further assistance.

[shaohuzhang1]

The code looks mostly correct with some minor improvements:

1. The onBeforeUpdate lifecycle hook is missing a semicolon after its definition.
2. There's an unused variable named oldModel.
3. It's beneficial to add comments explaining each line of code that might help future refactors or developers who read this code.

Here’s the revised code:

<script setup lang="ts">
import { computed, defineComponent } from 'vue'
// Other imports...

defineExpose({
  // Expose your exposed properties, methods, etc.
})

const props = defineProps<{
  nodeModel: any // Adjust the type as per your needs
}>()

const mountedNodesCountRef = ref(0)
let oldModel

const validate = (newValue?: any): void | Promise<void> => {
  // Validation logic here...
}

onMounted(() => {
  const loopBodyNode = document.createElement('div') // Example element creation
  mountedNodesCountRef.value++

  if (!props.nodeModel.virtual) {
    mountLoopBodyNode() // Only call this if nodeModel.virtual is false
  }

  oldModel = JSON.parse(JSON.stringify(props.nodeModel)) // Clone the original object
})

function mountLoopBodyNode(): void {
  // Code to mount loop body nodes if needed
}
</script>

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

These changes should be straightforward and improve readability.

[shaohuzhang1]

There are no apparent irregularities or significant problems with the provided code. It looks well-written, clean, and maintains readability.

However, there is one area where it could be optimized slightly:

// No changes needed here

If you have any specific optimizations in mind or need any assistance with something else, feel free to ask!