EAGLE-1343: Graph edges can't be deleted after deleting ports

[james-strauss-uwa]

Moved removeFieldFromNodeById from Eagle.ts to LogicalGraph.ts so that it can work directly on the edges observableArray instead of working on a array copy provided by LogicalGraph.getEdges()

Minor update to the Utils.printLogicalGraphEdgesTable() to add some more information.

Summary by Sourcery

Enhancements:

• Move the removeFieldFromNodeById function from Eagle.ts to LogicalGraph.ts to operate directly on the edges observableArray, improving efficiency and maintainability.

[sourcery-ai]

Reviewer's Guide by Sourcery

The PR fixes an issue with graph edge deletion after port removal by moving the removeFieldFromNodeById method from Eagle.ts to LogicalGraph.ts. This allows direct manipulation of the edges observable array instead of working with a copy. The implementation also includes improvements to edge information display in the Utils class.

Sequence diagram for removing a field from a node

sequenceDiagram
    actor User
    participant LogicalGraph
    participant Node
    participant Edge
    participant Eagle

    User->>LogicalGraph: removeFieldFromNodeById(node, id)
    LogicalGraph->>Node: removeFieldById(id)
    LogicalGraph->>Edge: Check edges for id
    Edge-->>LogicalGraph: Return edges with id
    LogicalGraph->>Edge: Remove edges with id
    LogicalGraph->>Eagle: checkGraph()
    LogicalGraph->>Eagle: undo().pushSnapshot()
    LogicalGraph->>Eagle: flagActiveFileModified()
    LogicalGraph->>Eagle: selectedObjects.valueHasMutated()

Loading

Updated class diagram for LogicalGraph and Eagle classes

classDiagram
    class LogicalGraph {
        +removeFieldFromNodeById(node: Node, id: FieldId)
        +portIsLinked(nodeId: NodeId, portId: FieldId)
        ...
    }
    class Eagle {
        -removeFieldFromNodeById(node: Node, id: string)
        +nodeDropLogicalGraph(eagle: Eagle, event: JQuery.TriggeredEvent)
        ...
    }
    LogicalGraph --> Eagle : uses
    note for LogicalGraph "Moved removeFieldFromNodeById from Eagle to LogicalGraph"

Loading

File-Level Changes

Change	Details	Files
Relocated port removal functionality from Eagle to LogicalGraph class	Moved removeFieldFromNodeById method to LogicalGraph class Updated method to work directly with observable array instead of array copy Removed console.log statements from the original implementation Updated template to call the method from its new location	src/Eagle.ts src/LogicalGraph.ts templates/parameter_table.html
Enhanced edge information display in graph utility	Added node names to edge table data Added port display text to edge table data	src/Utils.ts

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time. You can also use
  this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey @james-strauss-uwa - I've reviewed your changes and they look great!

Here's what I looked at during the review

• 🟡 General issues: 1 issue found
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟡 Complexity: 1 issue found
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (performance): Consider caching the edges array value outside the loop to avoid repeated observable access

Instead of calling edges() multiple times in the loop, store the array once with const edgesArray = edges(); and use that in the loop. This will be more efficient and clearer to read.

        const edges: ko.ObservableArray<Edge> = this.edges;
        const edgesArray = edges();

[james-strauss-uwa]

No, I think this is fine, we want to cache the edges observableArray

[sourcery-ai]

Thank you for your feedback, we will generate fewer comments like this in the future according to the following instructions:

- Ensure the comment provides a clear and actionable suggestion for improving code performance.
- Verify that the comment includes a specific example or code snippet to illustrate the suggestion.
- Check that the comment is relevant to the code being reviewed and addresses a potential issue or improvement.
- Avoid comments that are overly prescriptive without considering the context or potential trade-offs.

[M-Wicenec]

looks good