Skip to content

DYN-10313 - Python Port Copy Paste + Undo Redo Fixes#17021

Merged
zeusongit merged 11 commits intoDynamoDS:masterfrom
johnpierson:dyn10313_PythonNodePortCopy
Apr 7, 2026
Merged

DYN-10313 - Python Port Copy Paste + Undo Redo Fixes#17021
zeusongit merged 11 commits intoDynamoDS:masterfrom
johnpierson:dyn10313_PythonNodePortCopy

Conversation

@johnpierson
Copy link
Copy Markdown
Member

@johnpierson johnpierson commented Apr 6, 2026

Purpose

Fixes two related bugs with custom port names on Python nodes (tracked in DYN-10313 ):

  1. Copy/paste drops custom port names — when copying a Python node (Ctrl+C / Ctrl+V), custom input and output port
    names were reset to defaults (IN[0], IN[1], OUT). The XML serialization path used for copy and undo had no mechanism
    to round-trip port names or tooltips.
  2. Undo has no effect on port property changes — editing a port's name or description via the Edit Port Properties
    dialog could not be undone. The mutations were applied directly to PortModel without recording the owning node in the
    undo stack, so Ctrl+Z had no effect. Additionally, after undo did restore a port name, the UI would not refresh
    because PortModel.Name raised no PropertyChanged notification, and PortViewModel.PortPropertyChanged was listening for
    "PortName" — a property name PortModel never raised (dead code).

Changes:

  • PythonNodeBase.SerializeCore / DeserializeCore (new overrides) — round-trip custom port names and tooltips for both
    input (PortInfo portName/portToolTip attributes) and output (OutPortInfo elements) ports. Applies to both PythonNode
    and PythonStringNode via the shared base class.
  • PortModel.Name — converted from auto-property to backing field; now raises RaisePropertyChanged(nameof(Name)) on
    change, consistent with ToolTip.
  • InPortViewModel.EditPortProperties / OutPortViewModel.EditPortProperties — wrapped mutations in
    UndoRecorder.BeginActionGroup() with WorkspaceModel.RecordModelForModification before changing port.Name and
    port.ToolTip. Also sets HasUnsavedChanges = true (previously missing).
  • PortViewModel.PortPropertyChanged — fixed dead switch case: case nameof(PortName) (= "PortName") → case
    nameof(PortModel.Name) (= "Name"), so UI refreshes when undo restores a port name.
  • Three new tests in PythonEditTests: custom port names and tooltips survive SaveContext.Copy for PythonNode,
    SaveContext.Undo for PythonNode, and SaveContext.Copy for PythonStringNode (covering the shared PythonNodeBase path).

20260406-pythonFixes

Declarations

Release Notes

Custom port names and descriptions on Python Script nodes are now preserved when copying a node and when undoing port property edits. Undo also correctly refreshes the port label in the UI.

Reviewers

@DynamoDS/eidos

FYIs

johnpierson and others added 3 commits April 6, 2026 09:33
  Custom input/output port names on PythonNode and PythonStringNode were
  reset to defaults (IN[n], OUT) on copy/paste and undo/redo because the
  XML serialization path (SerializeCore/DeserializeCore) had no mechanism
  to round-trip port names.

  Add SerializeCore/DeserializeCore overrides on PythonNodeBase that write
  portName into existing PortInfo elements (inputs) and new OutPortInfo
  elements (outputs), then re-apply them after base deserialization
  recreates ports. Covers both SaveContext.Copy and SaveContext.Undo.

  Add two tests in PythonEditTests verifying copy and undo round-trips
  preserve custom port names.
 Port name and description edits via EditPortProperties were applied
  directly to PortModel without recording the node for undo, so Ctrl+Z
  had no effect. Additionally, PortModel.Name had no PropertyChanged
  notification, so the UI would not refresh after undo restored an old
  port name.

  Record the owning node for modification inside a UndoRecorder action
  group before mutating port.Name and port.ToolTip in both InPortViewModel
  and OutPortViewModel. Mark HasUnsavedChanges after the edit (previously
  missing). Convert PortModel.Name to a backing field that raises
  PropertyChanged so the UI updates on undo. Extend PythonNodeBase
  SerializeCore/DeserializeCore to also round-trip port tooltips so the
  full port state is captured in the undo snapshot.
PortViewModel.PortPropertyChanged listened for "PortName" from
  PortModel, but PortModel.Name raises "Name". The case was dead code —
  PortModel never raised "PortName" — so name changes (including undo
  restores) never propagated to the UI. Description worked because
  PortModel.ToolTip already raised "ToolTip" with a matching case.

  Change the switch case from nameof(PortName) to nameof(PortModel.Name)
  so the ViewModel re-evaluates PortName and the UI updates correctly
  when undo restores a previous port name.

Co-Authored-By: Claude <claude@users.noreply.github.com>
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-10313

Co-Authored-By: Claude <claude@users.noreply.github.com>
@johnpierson johnpierson marked this pull request as ready for review April 6, 2026 17:41
Copilot AI review requested due to automatic review settings April 6, 2026 17:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes Dynamo Python node custom port metadata not surviving copy/paste and undo/redo, and ensures UI updates correctly when port names are restored.

Changes:

  • Add XML round-tripping for Python node input/output port names and tooltips during copy/undo deserialization.
  • Make PortModel.Name raise PropertyChanged, and update PortViewModel to listen for the correct property name.
  • Wrap port property edits in undo action groups and mark the workspace dirty; add regression tests for copy/undo.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/Libraries/DynamoPythonTests/PythonEditTests.cs Adds tests verifying Python node custom port names survive Copy and Undo XML round-trips.
src/Libraries/PythonNodeModels/PythonNode.cs Adds PythonNodeBase XML serialization overrides to persist/restore port names/tooltips for copy/undo.
src/DynamoCore/Graph/Nodes/PortModel.cs Converts Name to a backing field and raises PropertyChanged when it changes.
src/DynamoCoreWpf/ViewModels/Core/PortViewModel.cs Fixes port property change handling to respond to PortModel.Name notifications.
src/DynamoCoreWpf/ViewModels/Core/InPortViewModel.cs Records node modifications in the undo stack when editing port properties and sets HasUnsavedChanges.
src/DynamoCoreWpf/ViewModels/Core/OutPortViewModel.cs Records node modifications in the undo stack when editing port properties and sets HasUnsavedChanges.

johnpierson and others added 3 commits April 6, 2026 11:53
EditPortProperties, EditPortPropertiesCommand, and ListOutportNames were
  duplicated verbatim across InPortViewModel and OutPortViewModel, with one
  subtle difference: which port collection was passed for sibling-name
  deduplication. InPortViewModel also had a latent bug — PortType was
  hardcoded to PortType.Output instead of reflecting the actual port type.

  Move the unified implementation into PortViewModel. Use port.PortType to
  select node.InPorts or node.OutPorts for sibling names, and pass
  port.PortType to the dialog (fixing the bug). Remove the now-redundant
  field, command, method, and helper from both subclasses, along with the
  unused usings they pulled in.

Co-Authored-By: Claude <claude@users.noreply.github.com>
XmlElement.GetAttribute returns "" for both a missing attribute and a
  present-but-empty one. The IsNullOrEmpty guard on portToolTip therefore
  silently dropped intentionally cleared descriptions. Switch to
  HasAttribute so an empty string is restored when the attribute exists.
  portName keeps the IsNullOrEmpty guard since an empty name is invalid.

Co-Authored-By: Claude <claude@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

johnpierson and others added 2 commits April 6, 2026 15:03
  - Use HasAttribute("portName") instead of !IsNullOrEmpty in DeserializeCore
    for both input and output ports so intentionally-empty names round-trip correctly
  - Add index >= 0 / outIndex >= 0 guards against malformed/corrupt XML
  - Fix ShowHideFlags.Show → ShowHideFlags.Hide in EditPortProperties to
    actually close the port context menu instead of re-opening it
  - Fix misleading InternalsVisibleTo comment in PythonEditTests
  - Extend copy/undo tests to assert ToolTip values alongside Name
  - Add WhenPythonStringNodeCopied test to cover the PythonStringNode path
    through the shared PythonNodeBase serialization

Co-Authored-By: Claude <claude@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

- Add index >= 0 guard in SerializeCore to match DeserializeCore,
  preventing negative index from malformed XML causing IndexOutOfRange
- Update SerializeCore XML doc comment to reflect that portName/portToolTip
  are written for all SaveContext values, not just Copy and Undo

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 7, 2026

@johnpierson johnpierson requested a review from a team April 7, 2026 14:59
// Re-apply any custom values that were serialized.
foreach (XmlNode child in nodeElement.ChildNodes)
{
if (child.Name == "PortInfo" && child is XmlElement portInfo)
Copy link
Copy Markdown
Contributor

@aparajit-pratap aparajit-pratap Apr 7, 2026

Choose a reason for hiding this comment

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

Hmm, so this is not InPortInfo - not symmetrically named like OutPortInfo? Not a comment on this PR but just an observation about confusing naming.

@zeusongit zeusongit merged commit ed026f7 into DynamoDS:master Apr 7, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants