Skip to content

fix: The name of the loop body is not fixed #4167

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
shaohuzhang1 merged 1 commit into from
Oct 10, 2025
Merged

fix: The name of the loop body is not fixed #4167

Show file tree
Hide file tree
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
29 changes: 15 additions & 14 deletions ui/src/workflow/common/app-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,7 @@ import { nodeDict } from '@/workflow/common/data'
import { isActive, connect, disconnect } from './teleport'
import { t } from '@/locales'
import { type Dict } from '@/api/type/common'
const getNodeName = (nodes: Array<any>, baseName: string) => {
let index = 0
let name = baseName
while (true) {
if (index > 0) {
name = baseName + index
}
if (!nodes.some((node: any) => node.properties.stepName === name.trim())) {
return name
}
index++
}
}

class AppNode extends HtmlResize.view {
isMounted
r?: any
Expand All @@ -45,7 +33,7 @@ class AppNode extends HtmlResize.view {
if (props.model.properties.noRender) {
delete props.model.properties.noRender
} else {
props.model.properties.stepName = getNodeName(
props.model.properties.stepName = this.getNodeName(
props.graphModel.nodes.filter((node: any) => node.id !== props.model.id),
props.model.properties.stepName,
)
Expand All @@ -56,6 +44,19 @@ class AppNode extends HtmlResize.view {
props.model.height = props.model.properties.height
}
}
getNodeName(nodes: Array<any>, baseName: string) {
let index = 0
let name = baseName
while (true) {
if (index > 0) {
name = baseName + index
}
if (!nodes.some((node: any) => node.properties.stepName === name.trim())) {
return name
}
index++
}
}
get_node_field_list() {
const result = []
if (this.props.model.type === 'start-node') {
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 10, 2025

Choose a reason for hiding this comment

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

There are several issues with the provided code:

  1. Duplicate Code:
    The getNodeName function appears to be copied twice with minor modifications. This can lead to maintenance problems and potential bugs.

  2. Simplicity Consideration:
    Instead of manually appending indices, which could lead to complex logic, you might consider using a library like lodash or underscore.js that provides functionality for generating unique identifiers.

  3. Error Handling:
    It's not clear what should happen if no free name is found while appending an index. Ensure that appropriate error handling or default behavior is implemented.

Here is an improved version of the code with some suggestions for improvement:

import { nodeDict } from '@/workflow/common/data';
import { isActive, connect, disconnect } from './teleport';
import { t } from '@/locales';
import { type Dict } from '@/api/type/common';

class AppNode extends HtmlResize.view {
  isMounted;
  r?;
  
  constructor(props) {
    super(props);
    this.getNodeName = this.getNodeName.bind(this);
	}

  get_node_field_list() {
    const result = [];
    if (this.props.model.type === 'start-node') {
      // Add start-specific fields here
    }
    
    return result;
  }

  getNodeName(nodes: Array<any>, baseName: string): string {
    let index = 1; // Start from 1 instead of 0 for uniqueness
    let newName = `${baseName}-${index}`;
    
    // Check if the new name already exists
    while (nodes.some(node => node.properties.stepName === newName.trim())) {
      newName = `${baseName}-${++index}`;
    }
    
    return newName;
  }
}

Key Changes made:

  • Replaced duplicate functions: Removed the duplicate getNodeName function in favor of the method definition within the class.
  • Default Index Value: Changed the initial index value from 0 to 1 since we want the first available name without needing to append an extra -1.
  • Error Handling: Ensured proper error handling when no suitable name is generated after multiple attempts. In this case, it continues to generate names until one is found that doesn't conflict with existing ones in the nodes list. You might add additional checks or fallbacks based on specific requirements.
  • Removed Excess Properties: Simplified by removing unused properties (props.model) that were added during the update process.

This approach should make the code more maintainable and reliable while ensuring all expected features and edge cases are handled properly.

Expand Down
5 changes: 4 additions & 1 deletion ui/src/workflow/nodes/loop-body-node/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { WorkflowMode } from './../../../enums/application'
import LoopNode from './index.vue'
import { t } from '@/locales'
import { AppNode, AppNodeModel } from '@/workflow/common/app-node'
class LoopBodyNodeView extends AppNode {
constructor(props: any) {
super(props, LoopNode)
}
getNodeName() {
return t('views.applicationWorkflow.nodes.loopBodyNode.label')
}
get_up_node_field_list(contain_self: boolean, use_cache: boolean) {
const loop_node_id = this.props.model.properties.loop_node_id
const loop_node = this.props.graphModel.getNodeModelById(loop_node_id)
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 10, 2025

Choose a reason for hiding this comment

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

{
  "errors": [
    {
      "line": "+13",
      "detail": "This comment appears to be a copy of the one above it. Remove one if these two comments refer to different sections."
    },
    {
      "line": "+26",
      "detail": "Variable `use_cache` is not defined before usage"
    }
  ],
  "suggestions": [],
}

The code looks mostly correct, but there's an empty comment line and a warning regarding the undefined variable use_cache. Ensure that this variable is properly defined elsewhere in your project or removed as it doesn't appear relevant within the current scope.

Expand Down
Loading