diff --git a/src/DynamoCore/Core/CustomNodeManager.cs b/src/DynamoCore/Core/CustomNodeManager.cs index 0369bd8e7ac..d4b9d6a194c 100644 --- a/src/DynamoCore/Core/CustomNodeManager.cs +++ b/src/DynamoCore/Core/CustomNodeManager.cs @@ -984,7 +984,7 @@ private bool InitializeCustomNode( newWorkspace.Category = workspaceInfo.Category; // Mark the custom node workspace as having no changes - when we set the category on the above line // this marks the workspace as changed. - newWorkspace.HasUnsavedChanges = false; + newWorkspace.MarkAsSaved(); } } @@ -1549,7 +1549,7 @@ from output in outputs IsVisibleInDynamoLibrary = true }); - newWorkspace.HasUnsavedChanges = true; + newWorkspace.MarkAsIndependentlyModified(); RegisterCustomNodeWorkspace(newWorkspace); diff --git a/src/DynamoCore/Core/UndoRedoRecorder.cs b/src/DynamoCore/Core/UndoRedoRecorder.cs index 6d4edf32af5..6886440cde0 100644 --- a/src/DynamoCore/Core/UndoRedoRecorder.cs +++ b/src/DynamoCore/Core/UndoRedoRecorder.cs @@ -60,6 +60,15 @@ internal interface IUndoRedoRecorderClient /// Notifies the UI that the undo/redo state has changed so that undo/redo buttons can be enabled/disabled. /// void UpdateUndoRedoStack(); + + /// + /// UndoRedoRecorder calls this method right after recording a fresh user + /// modification (e.g. a node/note/group being moved or resized) so the + /// client can mark itself as having unsaved changes. Not called during + /// undo/redo replay itself, which goes through DeleteModel/CreateModel/ + /// ReloadModel instead. + /// + void MarkAsModified(); } internal class UndoRedoRecorder : LogSourceBase @@ -75,6 +84,7 @@ public enum UserAction private const string UserActionAttrib = "UserAction"; private const string ActionGroup = "ActionGroup"; + private const string AffectsSavedStateAttrib = "AffectsSavedState"; private readonly IUndoRedoRecorderClient undoClient; private readonly XmlDocument document = new XmlDocument(); @@ -201,6 +211,7 @@ public void RecordCreationForUndo(ModelBase model) RecordActionInternal(currentActionGroup, model, UserAction.Creation); + currentActionGroup.SetAttribute(AffectsSavedStateAttrib, bool.TrueString); redoStack.Clear(); // Wipe out the redo-stack. } @@ -216,22 +227,35 @@ public void RecordDeletionForUndo(ModelBase model) RecordActionInternal(currentActionGroup, model, UserAction.Deletion); + currentActionGroup.SetAttribute(AffectsSavedStateAttrib, bool.TrueString); redoStack.Clear(); // Wipe out the redo-stack. } /// /// Record the given model right before it is modified. This results - /// in a modification action to be recorded under the current action + /// in a modification action to be recorded under the current action /// group. Undoing this action will result in the model being reverted /// to the states that it was in before the modification took place. /// /// The model to be recorded. - public void RecordModificationForUndo(ModelBase model) + /// + /// Whether this recording represents a real content change that should mark the + /// workspace dirty (e.g. a node/note/group drag or resize). Pass false for recordings + /// that exist purely to make an incidental action (like a selection change) undoable, + /// since that state is never written to the saved file (DYN-10717). + /// + public void RecordModificationForUndo(ModelBase model, bool markAsModified = true) { RecordActionInternal(currentActionGroup, model, UserAction.Modification); redoStack.Clear(); // Wipe out the redo-stack. + + if (markAsModified) + { + currentActionGroup.SetAttribute(AffectsSavedStateAttrib, bool.TrueString); + undoClient.MarkAsModified(); + } } /// @@ -270,6 +294,22 @@ public XmlElement PopFromUndoGroup() public bool CanUndo { get { return undoStack.Count > 0; } } public bool CanRedo { get { return redoStack.Count > 0; } } + /// + /// The number of action groups currently on the undo stack that actually affect + /// what would be written to the saved file (i.e. were recorded via + /// RecordCreationForUndo/RecordDeletionForUndo, or RecordModificationForUndo with + /// markAsModified: true) -- action groups recorded purely to make an incidental, + /// non-persisted action undoable (e.g. a selection change) are excluded. Used by + /// the owning workspace to detect when Undo/Redo has returned to the exact content + /// position it was at when last saved, so it can correctly report having no unsaved + /// changes again (DYN-10717) instead of a one-way "dirty" flag that Undo never + /// clears, without being fooled by undoing/redoing a non-content action group. + /// + internal int SavedStateAffectingUndoDepth + { + get { return undoStack.Count(group => group.GetAttribute(AffectsSavedStateAttrib) == bool.TrueString); } + } + #endregion #region Private Class Helper Methods @@ -421,6 +461,9 @@ private void UndoActionGroup(XmlElement actionGroup) } } + if (actionGroup.GetAttribute(AffectsSavedStateAttrib) == bool.TrueString) + newGroup.SetAttribute(AffectsSavedStateAttrib, bool.TrueString); + redoStack.Push(newGroup); // Place the states on the redo-stack. } @@ -467,6 +510,9 @@ private void RedoActionGroup(XmlElement actionGroup) } } + if (actionGroup.GetAttribute(AffectsSavedStateAttrib) == bool.TrueString) + newGroup.SetAttribute(AffectsSavedStateAttrib, bool.TrueString); + undoStack.Push(newGroup); } diff --git a/src/DynamoCore/Graph/Workspaces/CustomNodeWorkspaceModel.cs b/src/DynamoCore/Graph/Workspaces/CustomNodeWorkspaceModel.cs index add23d7f767..1d740046c31 100644 --- a/src/DynamoCore/Graph/Workspaces/CustomNodeWorkspaceModel.cs +++ b/src/DynamoCore/Graph/Workspaces/CustomNodeWorkspaceModel.cs @@ -83,7 +83,7 @@ public CustomNodeWorkspaceModel( { Debug.WriteLine("Creating a custom node workspace..."); - HasUnsavedChanges = false; + MarkAsSaved(); CustomNodeId = Guid.Parse(info.ID); Category = info.Category; @@ -101,7 +101,7 @@ private void OnPropertyChanged(object sender, PropertyChangedEventArgs args) if (args.PropertyName == "Category" || args.PropertyName == "Description") { - HasUnsavedChanges = true; + MarkAsIndependentlyModified(); OnInfoChanged(); } } diff --git a/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs b/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs index c5bb1cb5dac..90941a495d8 100644 --- a/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs +++ b/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs @@ -512,6 +512,84 @@ protected override void DisposeNode(NodeModel node) base.DisposeNode(node); } + /// + /// Computes the external file references by inspecting evaluated output values from + /// this workspace's running engine (only possible for a home workspace, not e.g. a + /// custom node workspace, hence this override). + /// + private protected override List ComputeExternalFileReferences() + { + var externalFiles = new Dictionary(); + + // If an execution is in progress we'll have to wait for it to be done before we can gather the + // external file references as this implementation relies on the output values of each node. + // instead just bail to avoid blocking the UI. + if (RunSettings.RunEnabled && !RunSettings.ForceBlockRun) + { + foreach (var node in Nodes) + { + CollectExternalFileReferencesForNode(node, externalFiles); + } + } + + return externalFiles.Values.ToList(); + } + + /// + /// Checks each output port of the given node for a file path value, recording any + /// found as an external file reference. + /// + private void CollectExternalFileReferencesForNode(NodeModel node, Dictionary externalFiles) + { + externalFilesDictionary.TryGetValue(node.GUID, out var serializedDependencyInfo); + + // Check for the file path string value at each of the output ports of all nodes in the workspace. + var outputIdentifierNames = node.OutPorts.Select(port => node.GetAstIdentifierForOutputIndex(port.Index)?.Name); + foreach (var id in outputIdentifierNames) + { + var mirror = EngineController.GetMirror(id); + var data = mirror?.GetData().Data; + + if (data is string dataString && dataString.Contains(@"\")) + { + RecordExternalFileReference(node, dataString, serializedDependencyInfo, externalFiles); + } + } + } + + /// + /// Records the given output value as an external file reference, either because it + /// exists on disk, or -- if not -- because it matches a previously serialized + /// dependency for this node. + /// + private void RecordExternalFileReference(NodeModel node, string dataString, DependencyInfo serializedDependencyInfo, Dictionary externalFiles) + { + // Check if the value exists on disk + DynamoUtilities.PathHelper.FileInfoAtPath(dataString, out bool fileExists, out string fileSize); + if (fileExists) + { + var externalFilePath = System.IO.Path.GetFullPath(dataString); + var externalFileName = System.IO.Path.GetFileName(dataString); + + if (!externalFiles.ContainsKey(externalFilePath)) + { + externalFiles[externalFilePath] = new DependencyInfo(externalFileName, dataString, ReferenceType.External); + } + + externalFiles[externalFilePath].AddDependent(node.GUID); + externalFiles[externalFilePath].Size = fileSize; + } + // Read the serialized value for that node. + else if (serializedDependencyInfo != null && dataString.Contains(serializedDependencyInfo.Name)) + { + if (!externalFiles.ContainsKey(serializedDependencyInfo.Name)) + { + externalFiles[serializedDependencyInfo.Name] = new DependencyInfo(serializedDependencyInfo.Name, ReferenceType.External); + } + externalFiles[serializedDependencyInfo.Name].AddDependent(node.GUID); + } + } + /// /// Called when the RequestSilenceNodeModifiedEvents event is emitted from a Node /// @@ -925,10 +1003,10 @@ internal void ReCompileCodeBlockNodesForFunctionDefinitions() cbn.ProcessCodeDirect(cbn.RecompileCodeBlockAST); } // This method is intended to be called only during opening of an existing workspace - // and therefore if the workspace is set as dirty on account of CBN precompilation, + // and therefore if the workspace is set as dirty on account of CBN precompilation, // the workspace should be reverted to a clean state as it's undesirable to have unsaved // changes for a workspace that is newly opened. - HasUnsavedChanges = false; + MarkAsSaved(); } internal bool TryGetMatchingWorkspaceData(string uniqueId, out Dictionary data) diff --git a/src/DynamoCore/Graph/Workspaces/UndoRedo.cs b/src/DynamoCore/Graph/Workspaces/UndoRedo.cs index 5cee05546c7..0adfe3cbd80 100644 --- a/src/DynamoCore/Graph/Workspaces/UndoRedo.cs +++ b/src/DynamoCore/Graph/Workspaces/UndoRedo.cs @@ -60,6 +60,7 @@ internal void Undo() if (null != undoRecorder) { undoRecorder.Undo(); + UpdateHasUnsavedChangesFromSavedStateAffectingDepth(); // http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-7883 // Request run for every undo action @@ -72,6 +73,7 @@ internal void Redo() if (null != undoRecorder) { undoRecorder.Redo(); + UpdateHasUnsavedChangesFromSavedStateAffectingDepth(); // http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-7883 // Request run for every redo action @@ -83,6 +85,31 @@ internal void ClearUndoRecorder() { if (null != undoRecorder) undoRecorder.Clear(); + + savedUndoDepth = 0; + } + + /// + /// Marks this workspace as having no unsaved changes, and remembers the current + /// position in the undo history so that Undo/Redo back to this exact point can + /// correctly report the workspace as clean again, rather than leaving Undo unable + /// to ever clear the one-way dirty flag (DYN-10717). + /// + internal void MarkAsSaved() + { + // A save clears both undo-tracked and independently-flagged (administrative) + // dirty state. independentDirtyFlag must be reset explicitly here + independentDirtyFlag = false; + HasUnsavedChanges = false; + savedUndoDepth = undoRecorder?.SavedStateAffectingUndoDepth ?? 0; + } + + private void UpdateHasUnsavedChangesFromSavedStateAffectingDepth() + { + // OR in independentDirtyFlag rather than overwriting: the undo stack being back + // at its saved position only means undo-tracked content is clean again + bool undoDepthIndicatesUnsaved = undoRecorder.SavedStateAffectingUndoDepth != savedUndoDepth; + HasUnsavedChanges = undoDepthIndicatesUnsaved || independentDirtyFlag; } // See RecordModelsForModification below for more details. @@ -106,7 +133,12 @@ internal static void RecordModelForModification(ModelBase model, UndoRedoRecorde /// /// The models to be recorded for undo. /// - internal static void RecordModelsForModification(List models, UndoRedoRecorder recorder) + /// + /// Whether this recording represents a real content change that should mark the + /// workspace dirty. Pass false for recordings that exist purely to make an incidental + /// action (like a selection change) undoable (DYN-10717). + /// + internal static void RecordModelsForModification(List models, UndoRedoRecorder recorder, bool markAsModified = true) { if (null == recorder) return; @@ -116,7 +148,7 @@ internal static void RecordModelsForModification(List models, UndoRed using (recorder.BeginActionGroup()) { foreach (var model in models) - recorder.RecordModificationForUndo(model); + recorder.RecordModificationForUndo(model, markAsModified); } } @@ -852,6 +884,15 @@ public void UpdateUndoRedoStack() { RaisePropertyChanged("CanUndoRedoCommand"); } + + // Explicit implementation (rather than matching the public/implicit style of + // the other IUndoRedoRecorderClient members above) so this stays an internal + // implementation detail of the undo/dirty-flag wiring, not new public API. + void IUndoRedoRecorderClient.MarkAsModified() + { + HasUnsavedChanges = true; + } + /// /// Returns model by GUID /// diff --git a/src/DynamoCore/Graph/Workspaces/WorkspaceModel.cs b/src/DynamoCore/Graph/Workspaces/WorkspaceModel.cs index 9e38a457bcd..c4682cb0f55 100644 --- a/src/DynamoCore/Graph/Workspaces/WorkspaceModel.cs +++ b/src/DynamoCore/Graph/Workspaces/WorkspaceModel.cs @@ -286,12 +286,25 @@ internal int CurrentPasteOffset private string author = "None provided"; private string description; private bool hasUnsavedChanges; + + /// + /// Tracks whether something marked this workspace dirty independently of the + /// undo-stack depth tracking (e.g. workspace-level/administrative state such as a + /// unit-conversion node's selected units, geometry scale factor, active linter, or + /// custom graph metadata -- none of which go through the tagged undo-recording + /// system). UpdateHasUnsavedChangesFromSavedStateAffectingDepth() ORs this in rather + /// than letting undo-depth comparisons blindly overwrite it, so an unrelated + /// undo/redo can never silently discard a real, independently-flagged unsaved change + /// (DYN-10717). + /// + private bool independentDirtyFlag; private bool isReadOnly; private readonly List nodes; private readonly List notes; private readonly List annotations; internal readonly List presets; private readonly UndoRedoRecorder undoRecorder; + private int savedUndoDepth; private static List savedModels = null; private double scaleFactor = 1.0; private bool hasNodeInSyncWithDefinition; @@ -306,7 +319,7 @@ internal int CurrentPasteOffset private List externalFileReferences; private Dictionary nodePackageDictionary = new Dictionary(); private Dictionary localDefinitionsDictionary = new Dictionary(); - private Dictionary externalFilesDictionary = new Dictionary(); + private protected readonly Dictionary externalFilesDictionary = new Dictionary(); private readonly string customNodeExtension = ".dyf"; /// @@ -430,7 +443,7 @@ internal virtual void OnCurrentOffsetChanged(object sender, PointEventArgs e) internal virtual void OnSaved() { LastSaved = DateTime.Now; - HasUnsavedChanges = false; + MarkAsSaved(); if (Saved != null) Saved(); @@ -991,61 +1004,15 @@ private List ComputeNodeLocalDefinitions() } /// - /// Computes the external file references if the Workspace Model is a HomeWorkspaceModel and graph is not running. + /// Computes the external file references for this workspace. The base implementation + /// returns an empty list; only overrides this, since + /// computing references relies on inspecting evaluated output values from a running + /// engine, which other workspace types (e.g. custom nodes) don't have. /// /// - private List ComputeExternalFileReferences() + private protected virtual List ComputeExternalFileReferences() { - var externalFiles = new Dictionary(); - - // If an execution is in progress we'll have to wait for it to be done before we can gather the - // external file references as this implementation relies on the output values of each node. - //instead just bail to avoid blocking the UI. - if (this is HomeWorkspaceModel homeWorkspaceModel && homeWorkspaceModel.RunSettings.RunEnabled && !homeWorkspaceModel.RunSettings.ForceBlockRun) - { - foreach (var node in nodes) - { - externalFilesDictionary.TryGetValue(node.GUID, out var serializedDependencyInfo); - - // Check for the file path string value at each of the output ports of all nodes in the workspace. - foreach (var port in node.OutPorts) - { - var id = node.GetAstIdentifierForOutputIndex(port.Index)?.Name; - var mirror = homeWorkspaceModel.EngineController.GetMirror(id); - var data = mirror?.GetData().Data; - - if (data is string dataString && dataString.Contains(@"\")) - { - // Check if the value exists on disk - PathHelper.FileInfoAtPath(dataString, out bool fileExists, out string fileSize); - if (fileExists) - { - var externalFilePath = Path.GetFullPath(dataString); - var externalFileName = Path.GetFileName(dataString); - - if (!externalFiles.ContainsKey(externalFilePath)) - { - externalFiles[externalFilePath] = new DependencyInfo(externalFileName, dataString, ReferenceType.External); - } - - externalFiles[externalFilePath].AddDependent(node.GUID); - externalFiles[externalFilePath].Size = fileSize; - } - // Read the serialized value for that node. - else if (serializedDependencyInfo != null && dataString.Contains(serializedDependencyInfo.Name)) - { - if (!externalFiles.ContainsKey(serializedDependencyInfo.Name)) - { - externalFiles[serializedDependencyInfo.Name] = new DependencyInfo(serializedDependencyInfo.Name, ReferenceType.External); - } - externalFiles[serializedDependencyInfo.Name].AddDependent(node.GUID); - } - } - } - } - } - - return externalFiles.Values.ToList(); + return new List(); } /// @@ -1098,6 +1065,7 @@ public bool HasUnsavedChanges if (!File.Exists(this.FileName)) // but the filename is invalid { this.fileName = string.Empty; + independentDirtyFlag = true; hasUnsavedChanges = true; } } @@ -1111,6 +1079,19 @@ public bool HasUnsavedChanges } } + /// + /// Marks this workspace as having unsaved changes due to a workspace-level or + /// administrative change that is not tracked by the undo/redo system. Unlike + /// setting directly, this ensures the flag cannot be + /// silently cleared by an unrelated Undo/Redo operation that returns the undo stack + /// to its last-saved position (DYN-10717). + /// + public void MarkAsIndependentlyModified() + { + independentDirtyFlag = true; + HasUnsavedChanges = true; + } + /// /// Returns if current workspace is readonly. /// @@ -1451,7 +1432,7 @@ protected WorkspaceModel( FileName = info.FileName; Zoom = info.Zoom; - HasUnsavedChanges = false; + MarkAsSaved(); IsReadOnly = DynamoUtilities.PathHelper.IsReadOnlyPath(fileName); LastSaved = DateTime.Now; diff --git a/src/DynamoCore/Models/DynamoModel.cs b/src/DynamoCore/Models/DynamoModel.cs index 81e01a6759f..db4fd7e69e5 100644 --- a/src/DynamoCore/Models/DynamoModel.cs +++ b/src/DynamoCore/Models/DynamoModel.cs @@ -2636,7 +2636,7 @@ internal bool OpenJsonFile( if (string.IsNullOrEmpty(workspace.FileName)) { - workspace.HasUnsavedChanges = true; + workspace.MarkAsIndependentlyModified(); } RunType runType = RunType.Manual; @@ -2727,7 +2727,7 @@ private void ReloadDummyNodes() if (resolvedDummyNode) { - currentWorkspace.HasUnsavedChanges = false; + currentWorkspace.MarkAsSaved(); // Once all the dummy nodes are reloaded, the DummyNodesReloaded event is invoked and // the Dependency table is regenerated in the WorkspaceDependencyView extension. currentWorkspace.OnDummyNodesReloaded(); @@ -3601,7 +3601,7 @@ public void ClearCurrentWorkspace() //don't save the file path CurrentWorkspace.FileName = ""; - CurrentWorkspace.HasUnsavedChanges = false; + CurrentWorkspace.MarkAsSaved(); CurrentWorkspace.Name = ""; // Clear workspace metadata properties when creating new workspace diff --git a/src/DynamoCore/Models/DynamoModelCommands.cs b/src/DynamoCore/Models/DynamoModelCommands.cs index 9d6e9a511e5..b8ee42d0f93 100644 --- a/src/DynamoCore/Models/DynamoModelCommands.cs +++ b/src/DynamoCore/Models/DynamoModelCommands.cs @@ -305,7 +305,10 @@ private void AddSelectionAndRecordUndo(ModelBase model) { try { - WorkspaceModel.RecordModelsForModification(new List() { model }, CurrentWorkspace.UndoRecorder); + // markAsModified: false -- selection is undo-tracked for UX (Ctrl+Z restores + // it) but is transient UI state, never written to the saved file, so it must + // not dirty the workspace (DYN-10717). + WorkspaceModel.RecordModelsForModification(new List() { model }, CurrentWorkspace.UndoRecorder, markAsModified: false); DynamoSelection.Instance.Selection.AddUnique(model); } catch (Exception ex) @@ -324,7 +327,7 @@ private void ClearSelectionAndRecordUndo() models.Add(modelBase); } - WorkspaceModel.RecordModelsForModification(models, CurrentWorkspace.UndoRecorder); + WorkspaceModel.RecordModelsForModification(models, CurrentWorkspace.UndoRecorder, markAsModified: false); DynamoSelection.Instance.ClearSelection(); } diff --git a/src/DynamoCore/PublicAPI.Unshipped.txt b/src/DynamoCore/PublicAPI.Unshipped.txt index 2f374cf2ec3..530472017ae 100644 --- a/src/DynamoCore/PublicAPI.Unshipped.txt +++ b/src/DynamoCore/PublicAPI.Unshipped.txt @@ -6,3 +6,4 @@ Dynamo.Models.DynamoModel.DefaultStartConfiguration.EnableUnTrustedLocationsNoti Dynamo.Models.DynamoModel.IStartConfiguration.EnableUnTrustedLocationsNotifications.get -> bool Dynamo.Models.DynamoModel.OpenFileCommand.OpenFileCommand(System.String filePath, System.Boolean forceManualExecutionMode, System.Boolean isTemplate, System.Boolean forceBlockRun) -> void Dynamo.Models.DynamoModel.InsertFileCommand.InsertFileCommand(System.String filePath, System.Boolean forceManualExecutionMode, System.Boolean forceBlockRun) -> void +Dynamo.Graph.Workspaces.WorkspaceModel.MarkAsIndependentlyModified() -> void diff --git a/src/DynamoCoreWpf/Controls/ShortcutToolbar.xaml.cs b/src/DynamoCoreWpf/Controls/ShortcutToolbar.xaml.cs index 5c154ffe08a..59278bd64bd 100644 --- a/src/DynamoCoreWpf/Controls/ShortcutToolbar.xaml.cs +++ b/src/DynamoCoreWpf/Controls/ShortcutToolbar.xaml.cs @@ -77,7 +77,6 @@ public ShortcutToolbar(DynamoViewModel dynamoViewModel) private void ShortcutToolbar_Loaded(object sender, RoutedEventArgs e) { - IsSaveButtonEnabled = false; IsExportMenuEnabled = false; IsLoginMenuEnabled = !DynamoViewModel.Model.NoNetworkMode; DynamoViewModel.OnRequestShorcutToolbarLoaded(RightMenu.ActualWidth); @@ -245,19 +244,6 @@ internal bool IsOpenButtonEnabled } } - internal bool IsSaveButtonEnabled - { - set - { - Button saveButton = GetButton("SAVE"); - if (saveButton != null) - { - saveButton.IsEnabled = value; - saveButton.Opacity = value ? 1 : 0.5; - } - } - } - internal bool IsLoginMenuEnabled { set diff --git a/src/DynamoCoreWpf/PublicAPI.Unshipped.txt b/src/DynamoCoreWpf/PublicAPI.Unshipped.txt index 90409bffdaf..b91b7c9ba73 100644 --- a/src/DynamoCoreWpf/PublicAPI.Unshipped.txt +++ b/src/DynamoCoreWpf/PublicAPI.Unshipped.txt @@ -22,6 +22,8 @@ Dynamo.PackageManager.ViewModels.PackageManagerSearchElementViewModel.IsUninstal Dynamo.UI.Controls.StartPageViewModel.TemplateFiles.get -> System.Collections.ObjectModel.ObservableCollection Dynamo.UI.Views.ScriptObject.ResetSettings() -> void Dynamo.UI.Views.ScriptObject.ScriptObject(System.Action requestLaunchDynamo, System.Action requestImportSettings, System.Func requestSignIn, System.Func requestSignOut, System.Action requestResetSettings) -> void +Dynamo.ViewModels.DynamoViewModel.CanSaveWorkspace.get -> bool +Dynamo.ViewModels.DynamoViewModel.CanSaveWorkspaceAs.get -> bool Dynamo.ViewModels.DynamoViewModel.NoNetworkMode.get -> bool Dynamo.ViewModels.DynamoViewModel.OnlineAccess.get -> bool Dynamo.ViewModels.DynamoViewModel.PythonEngineUpgradeToastRequested -> System.Action diff --git a/src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs b/src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs index 0df3307b296..80e76b7a95a 100644 --- a/src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs +++ b/src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs @@ -1817,7 +1817,7 @@ internal void UpdateGroupStyle(GroupStyleItem itemEntryParameter) FontSize = (double)itemEntryParameter.FontSize; GroupStyleId = itemEntryParameter.GroupStyleId; - WorkspaceViewModel.HasUnsavedChanges = true; + WorkspaceViewModel.Model.MarkAsIndependentlyModified(); } /// @@ -2123,7 +2123,7 @@ internal void ToggleIsVisibleGroup(object parameters) this.AnnotationModel.IsVisible = !this.AnnotationModel.IsVisible; WorkspaceViewModel.DynamoViewModel.Model.ExecuteCommand(command); WorkspaceViewModel.DynamoViewModel.RaiseCanExecuteUndoRedo(); - WorkspaceViewModel.HasUnsavedChanges = true; + WorkspaceViewModel.Model.MarkAsIndependentlyModified(); Analytics.TrackEvent(Actions.Preview, Categories.GroupOperations, this.AnnotationModel.IsVisible.ToString()); } @@ -2162,7 +2162,7 @@ internal void ToggleIsFrozenGroup(object parameters) WorkspaceViewModel.DynamoViewModel.Model.ExecuteCommand(command); WorkspaceViewModel.DynamoViewModel.RaiseCanExecuteUndoRedo(); - WorkspaceViewModel.HasUnsavedChanges = true; + WorkspaceViewModel.Model.MarkAsIndependentlyModified(); Analytics.TrackEvent(Actions.Freeze, Categories.GroupOperations, newFrozenState.ToString()); } diff --git a/src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs b/src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs index 46459a29fb1..38f85da98d2 100644 --- a/src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs +++ b/src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs @@ -72,6 +72,7 @@ public partial class DynamoViewModel : ViewModelBase, IDynamoViewModel private readonly DynamoModel model; private Point transformOrigin; private bool showStartPage = false; + private WorkspaceModel saveCommandsTrackedWorkspace; private PreferencesViewModel preferencesViewModel; private string dynamoMLDataPath = string.Empty; private const string dynamoMLDataFileName = "DynamoMLDataPipeline.json"; @@ -428,6 +429,8 @@ public bool ShowStartPage if(ShowInsertDialogAndInsertResultCommand != null) ShowInsertDialogAndInsertResultCommand.RaiseCanExecuteChanged(); + + NotifySaveCommandsChanged(); } } @@ -898,6 +901,9 @@ protected DynamoViewModel(StartConfiguration startConfiguration) SubscribeModelUiEvents(); SubscribeModelChangedHandlers(); SubscribeModelBackupFileSaveEvent(); + TrackWorkspaceForSaveCommands(model.CurrentWorkspace); + GuideFlowEvents.GuidedTourStart += OnGuidedTourStateChanged; + GuideFlowEvents.GuidedTourFinish += OnGuidedTourStateChanged; InitializeAutomationSettings(startConfiguration.CommandFilePath); @@ -1304,6 +1310,34 @@ private void UnsubscribeModelChangedEvents() model.PropertyChanged -= _model_PropertyChanged; model.WorkspaceCleared -= ModelWorkspaceCleared; model.RequestCancelActiveStateForNode -= this.CancelActiveState; + TrackWorkspaceForSaveCommands(null); + GuideFlowEvents.GuidedTourStart -= OnGuidedTourStateChanged; + GuideFlowEvents.GuidedTourFinish -= OnGuidedTourStateChanged; + } + + /// + /// Keeps the Save command's CanExecute in sync with the current workspace's dirty + /// flag: unsubscribes from the previously tracked workspace and subscribes to the + /// new one, then re-evaluates CanExecute immediately (the new workspace may already + /// differ in HasUnsavedChanges from the old one). + /// + private void TrackWorkspaceForSaveCommands(WorkspaceModel workspace) + { + if (saveCommandsTrackedWorkspace != null) + saveCommandsTrackedWorkspace.PropertyChanged -= SaveCommandsTrackedWorkspace_PropertyChanged; + + saveCommandsTrackedWorkspace = workspace; + + if (saveCommandsTrackedWorkspace != null) + saveCommandsTrackedWorkspace.PropertyChanged += SaveCommandsTrackedWorkspace_PropertyChanged; + + NotifySaveCommandsChanged(); + } + + private void SaveCommandsTrackedWorkspace_PropertyChanged(object sender, PropertyChangedEventArgs e) + { + if (e.PropertyName == nameof(WorkspaceModel.HasUnsavedChanges)) + NotifySaveCommandsChanged(); } private void SubscribeDispatcherHandlers() @@ -1482,6 +1516,7 @@ void _model_PropertyChanged(object sender, PropertyChangedEventArgs e) RaisePropertyChanged("ViewingHomespace"); if (this.PublishCurrentWorkspaceCommand != null) this.PublishCurrentWorkspaceCommand.RaiseCanExecuteChanged(); + TrackWorkspaceForSaveCommands(model.CurrentWorkspace); RaisePropertyChanged("IsPanning"); RaisePropertyChanged("IsOrbiting"); //RaisePropertyChanged("RunEnabled"); @@ -3245,9 +3280,52 @@ public void ShowSaveDialogIfNeededAndSaveResult(object parameter) } } + /// + /// "Save" is only meaningful when there is something new to persist, so it is also + /// gated on the current workspace's dirty flag (unlike "Save As", which can always + /// save a copy regardless of whether anything changed). + /// internal bool CanShowSaveDialogIfNeededAndSaveResultCommand(object parameter) { - return true; + return !GuideFlowEvents.IsAnyGuideActive && !ShowStartPage && (Model.CurrentWorkspace?.HasUnsavedChanges ?? false); + } + + /// + /// Whether the current workspace can be saved via the "Save" menu item, hotkey, and + /// toolbar button. Bound directly (rather than relying solely on the command's + /// CanExecute-driven IsEnabled coercion) because WPF's MenuItem does not reliably + /// coerce IsEnabled when the very first CanExecute evaluation is false. + /// + public bool CanSaveWorkspace => CanShowSaveDialogIfNeededAndSaveResultCommand(null); + + /// + /// Whether the current workspace can be saved via the "Save As" menu item and hotkey. + /// Bound directly for the same reason as . + /// + public bool CanSaveWorkspaceAs => CanShowSaveDialogAndSaveResult(null); + + /// + /// Keeps the Save/Save As commands (menu items, shortcut bar, and Ctrl+S/Ctrl+Shift+S) + /// in sync with the guided tour state. Unlike ShowStartPage, this is not tied to + /// workspace-creation flows, so it can safely gate CanExecute without resurrecting + /// DYN-10717 (Save/Save As stuck disabled on a fresh workspace). + /// + private void OnGuidedTourStateChanged(GuidedTourStateEventArgs args) + { + NotifySaveCommandsChanged(); + } + + /// + /// Raises change notifications for the Save/Save As commands and their bound + /// CanSaveWorkspace(As) properties, keeping the menu items, hotkeys, and toolbar + /// button in sync. + /// + private void NotifySaveCommandsChanged() + { + ShowSaveDialogIfNeededAndSaveResultCommand?.RaiseCanExecuteChanged(); + ShowSaveDialogAndSaveResultCommand?.RaiseCanExecuteChanged(); + RaisePropertyChanged(nameof(CanSaveWorkspace)); + RaisePropertyChanged(nameof(CanSaveWorkspaceAs)); } public void ShowSaveDialogAndSaveResult(object parameter) @@ -3371,7 +3449,7 @@ private bool ShowWarningDialogOnSaveWithUnresolvedIssues() internal bool CanShowSaveDialogAndSaveResult(object parameter) { - return true; + return !GuideFlowEvents.IsAnyGuideActive && !ShowStartPage; } public void ToggleFullscreenWatchShowing(object parameter) diff --git a/src/DynamoCoreWpf/Views/Core/DynamoView.xaml b/src/DynamoCoreWpf/Views/Core/DynamoView.xaml index 28bd90a97ed..709a191f79b 100644 --- a/src/DynamoCoreWpf/Views/Core/DynamoView.xaml +++ b/src/DynamoCoreWpf/Views/Core/DynamoView.xaml @@ -355,12 +355,12 @@ Command="{Binding ShowSaveDialogIfNeededAndSaveResultCommand}" Header="{x:Static p:Resources.DynamoViewFileMenuSave}" InputGestureText="Ctrl + S" - IsEnabled="False" /> + IsEnabled="{Binding CanSaveWorkspace}" /> + IsEnabled="{Binding CanSaveWorkspaceAs}" /> s.UniqueId == viewExtension.UniqueId); - // Create default settings if they do not currently exist - if (settings == null) - { - settings = new ViewExtensionSettings() - { - Name = viewExtension.Name, - UniqueId = viewExtension.UniqueId, - DisplayMode = ViewExtensionDisplayMode.DockRight - }; - this.dynamoViewModel.PreferenceSettings.ViewExtensionSettings.Add(settings); - } + CreateExtensionControl(viewExtension, content); + } + else + { + FocusExtensionControl(window, tab); + } - if (this.dynamoViewModel.PreferenceSettings.EnablePersistExtensions) - { - settings.IsOpen = true; - } + return addExtensionControl ? ExtensionControlResult.Added : ExtensionControlResult.AlreadyPresent; + } - if (settings.DisplayMode == ViewExtensionDisplayMode.FloatingWindow) - { - window = AddExtensionWindow(viewExtension, content, settings.WindowSettings); - } - else + /// + /// Creates a new extension control (as a floating window or a tab, per its settings) + /// for a view extension that isn't currently open. + /// + private void CreateExtensionControl(IViewExtension viewExtension, UIElement content) + { + var settings = this.dynamoViewModel.PreferenceSettings.ViewExtensionSettings.Find(s => s.UniqueId == viewExtension.UniqueId); + // Create default settings if they do not currently exist + if (settings == null) + { + settings = new ViewExtensionSettings() { - tab = AddExtensionTab(viewExtension, content); - } + Name = viewExtension.Name, + UniqueId = viewExtension.UniqueId, + DisplayMode = ViewExtensionDisplayMode.DockRight + }; + this.dynamoViewModel.PreferenceSettings.ViewExtensionSettings.Add(settings); + } + + if (this.dynamoViewModel.PreferenceSettings.EnablePersistExtensions) + { + settings.IsOpen = true; + } + + if (settings.DisplayMode == ViewExtensionDisplayMode.FloatingWindow) + { + AddExtensionWindow(viewExtension, content, settings.WindowSettings); } else { - // Set focus on the existing control - if (window != null) + AddExtensionTab(viewExtension, content); + } + } + + /// + /// Sets focus on an already-open extension control, whether it's a floating window + /// or a tab (making sure the extension bar is visible first, for the tab case). + /// + private void FocusExtensionControl(ExtensionWindow window, TabItem tab) + { + if (window != null) + { + window.Focus(); + } + else if (tab != null) + { + // Make sure the extension bar is visible + if (ExtensionsCollapsed) { - window.Focus(); + ToggleExtensionBarCollapseStatus(); } - else if (tab != null) - { - // Make sure the extension bar is visible - if (ExtensionsCollapsed) - { - ToggleExtensionBarCollapseStatus(); - } - tabDynamic.SelectedItem = tab; - } + tabDynamic.SelectedItem = tab; } - - return addExtensionControl ? ExtensionControlResult.Added : ExtensionControlResult.AlreadyPresent; } - private ExtensionWindow AddExtensionWindow(IViewExtension viewExtension, UIElement content, WindowSettings windowSettings) + private void AddExtensionWindow(IViewExtension viewExtension, UIElement content, WindowSettings windowSettings) { ExtensionWindow window; if (windowSettings == null) @@ -651,8 +655,6 @@ private ExtensionWindow AddExtensionWindow(IViewExtension viewExtension, UIEleme window.Show(); ExtensionWindows.Add(viewExtension.Name, window); - - return window; } private void ExtensionWindow_Closing(object sender, CancelEventArgs e) @@ -679,7 +681,7 @@ private void SaveExtensionWindowSettings(ExtensionWindow window) } } - private TabItem AddExtensionTab(IViewExtension viewExtension, UIElement content) + private void AddExtensionTab(IViewExtension viewExtension, UIElement content) { // creates a new tab item var tab = new TabItem(); @@ -702,8 +704,6 @@ private TabItem AddExtensionTab(IViewExtension viewExtension, UIElement content) dynamoViewModel.SideBarTabItems.Insert(dynamoViewModel.SideBarTabItems.Count, tab); tabDynamic.SelectedItem = tab; - - return tab; } private void UpdateNodeIcons_Click(object sender, RoutedEventArgs e) { diff --git a/src/DynamoCoreWpf/Views/Core/GeometryScalingPopup.xaml.cs b/src/DynamoCoreWpf/Views/Core/GeometryScalingPopup.xaml.cs index 562c681d8a0..34f5cc46a3c 100644 --- a/src/DynamoCoreWpf/Views/Core/GeometryScalingPopup.xaml.cs +++ b/src/DynamoCoreWpf/Views/Core/GeometryScalingPopup.xaml.cs @@ -69,7 +69,7 @@ private void RunGraphWhenScaleFactorUpdated() if (dynamoViewModel.ScaleFactorLog != viewModel.ScaleValue) { dynamoViewModel.ScaleFactorLog = (int)viewModel.ScaleValue; - dynamoViewModel.CurrentSpace.HasUnsavedChanges = true; + dynamoViewModel.CurrentSpace.MarkAsIndependentlyModified(); //Due that binding are done before the constructor of this class we need to execute the Log only if the viewModel was assigned previously if (viewModel != null) diff --git a/src/GraphMetadataViewExtension/GraphMetadataViewModel.cs b/src/GraphMetadataViewExtension/GraphMetadataViewModel.cs index c0a6b7f7aae..a46346f4689 100644 --- a/src/GraphMetadataViewExtension/GraphMetadataViewModel.cs +++ b/src/GraphMetadataViewExtension/GraphMetadataViewModel.cs @@ -327,10 +327,10 @@ private void HandleDeleteRequest(object sender, EventArgs e) } private void MarkCurrentWorkspaceModified() - { + { if (currentWorkspace != null && !string.IsNullOrEmpty(currentWorkspace.FileName)) { - currentWorkspace.HasUnsavedChanges = true; + currentWorkspace.MarkAsIndependentlyModified(); } } diff --git a/src/Libraries/UnitsUI/NodeViewCustomizations.cs b/src/Libraries/UnitsUI/NodeViewCustomizations.cs index 015f273f6a8..6b120b90b43 100644 --- a/src/Libraries/UnitsUI/NodeViewCustomizations.cs +++ b/src/Libraries/UnitsUI/NodeViewCustomizations.cs @@ -304,17 +304,17 @@ public void CustomizeView(DynamoUnitConvert model, NodeView nodeView) private void SelectConversionQuantity_PreviewMouseUp(object sender, System.Windows.Input.MouseButtonEventArgs e) { - nodeViewModel.WorkspaceViewModel.HasUnsavedChanges = true; + nodeViewModel.WorkspaceViewModel.Model.MarkAsIndependentlyModified(); } private void SelectConversionFrom_PreviewMouseUp(object sender, System.Windows.Input.MouseButtonEventArgs e) { - nodeViewModel.WorkspaceViewModel.HasUnsavedChanges = true; + nodeViewModel.WorkspaceViewModel.Model.MarkAsIndependentlyModified(); } private void SelectConversionTo_MouseLeftButtonDown(object sender, System.Windows.Input.MouseButtonEventArgs e) { - nodeViewModel.WorkspaceViewModel.HasUnsavedChanges = true; + nodeViewModel.WorkspaceViewModel.Model.MarkAsIndependentlyModified(); } public void Dispose() diff --git a/src/Libraries/UnitsUI/ViewModelCustomization.cs b/src/Libraries/UnitsUI/ViewModelCustomization.cs index 29f75f2c220..669af74cd77 100644 --- a/src/Libraries/UnitsUI/ViewModelCustomization.cs +++ b/src/Libraries/UnitsUI/ViewModelCustomization.cs @@ -148,7 +148,7 @@ internal void model_PropertyChanged(object sender, System.ComponentModel.Propert private void OnSwitchUnitsButtonClick(object obj) { dynamoConvertModel.SwitchUnitsDropdownValues(); - nodeViewModel.WorkspaceViewModel.HasUnsavedChanges = true; + nodeViewModel.WorkspaceViewModel.Model.MarkAsIndependentlyModified(); } private bool CanSwitchUnitsButton(object obj) diff --git a/src/LintingViewExtension/LinterViewModel.cs b/src/LintingViewExtension/LinterViewModel.cs index 91e73bdbdde..06cb789eb53 100644 --- a/src/LintingViewExtension/LinterViewModel.cs +++ b/src/LintingViewExtension/LinterViewModel.cs @@ -63,7 +63,7 @@ public LinterExtensionDescriptor ActiveLinter if (viewLoadedParams.CurrentWorkspaceModel is HomeWorkspaceModel currentWorkspace) { - currentWorkspace.HasUnsavedChanges = true; + currentWorkspace.MarkAsIndependentlyModified(); } } } diff --git a/src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml.cs b/src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml.cs index 135e2678076..d8cc8f7c254 100644 --- a/src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml.cs +++ b/src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml.cs @@ -237,8 +237,8 @@ internal void UpdateWorkspaceToUseInstalledPackage(PackageDependencyInfo info) info.Version = new Version(targetInfo.VersionName); info.State = PackageDependencyState.Loaded; info.Path = targetInfo.RootDirectory; - // Mark the current workspace dirty for save - currentWorkspace.HasUnsavedChanges = true; + // Mark the current workspace dirty for save. + currentWorkspace.MarkAsIndependentlyModified(); dependencyViewExtension.DependencyRegen(currentWorkspace); } } diff --git a/test/DynamoCoreTests/CoreTests.cs b/test/DynamoCoreTests/CoreTests.cs index b1cc3b162c2..79e1f66eb5b 100644 --- a/test/DynamoCoreTests/CoreTests.cs +++ b/test/DynamoCoreTests/CoreTests.cs @@ -854,6 +854,111 @@ public void TestFileDirtyOnLacingChange() Assert.AreEqual(true, CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); } + /// + /// Regression test for DYN-10717: dragging/resizing a node, note, or group changes + /// what would be written to the saved file (X/Y/Width/Height), but previously never + /// marked the workspace dirty. RecordModelsForModification is the shared entry point + /// every drag-completion/resize code path funnels through (StateMachine's node drag, + /// AnnotationViewModel's group/note resize, etc.), so exercising it directly here + /// covers all of them without needing WPF-level drag simulation. + /// + [Test] + public void TestFileDirtyOnNodeModification() + { + string openPath = Path.Combine(TestDirectory, "core", "LacingTest.dyn"); + OpenModel(openPath); + + Assert.IsFalse(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + + var node = CurrentDynamoModel.CurrentWorkspace.Nodes.First(); + + // Passed as ModelBase[] (not List) to unambiguously call the public + // RecordModelsForModification(IEnumerable) overload, which opens its + // own action group -- the internal List overload is an exact-type + // match that C# would otherwise prefer, and it assumes a group is already open. + CurrentDynamoModel.CurrentWorkspace.RecordModelsForModification(new ModelBase[] { node }); + + Assert.IsTrue(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + } + + /// + /// Regression test for DYN-10717: undoing the single change made since the file was + /// opened/saved must clear HasUnsavedChanges again -- Undo previously never reset + /// this one-way flag at all, so Save stayed enabled forever after the first edit even + /// if the user undid it back to the exact saved state. Redoing that change should + /// mark it dirty again. + /// + [Test] + public void TestUndoRedoRestoresHasUnsavedChangesToSavedState() + { + string openPath = Path.Combine(TestDirectory, "core", "LacingTest.dyn"); + OpenModel(openPath); + + Assert.IsFalse(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + + var node = CurrentDynamoModel.CurrentWorkspace.Nodes.First(); + var newPosition = $"{node.X + 100};{node.Y}"; + CurrentDynamoModel.ExecuteCommand(new DynCmd.UpdateModelValueCommand(Guid.Empty, node.GUID, nameof(NodeModel.Position), newPosition)); + Assert.IsTrue(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + + // Undo the only change made since open -- back to the saved state, so clean again. + CurrentDynamoModel.ExecuteCommand(new DynCmd.UndoRedoCommand(DynCmd.UndoRedoCommand.Operation.Undo)); + Assert.IsFalse(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + + // Redo re-applies the change -- dirty again. + CurrentDynamoModel.ExecuteCommand(new DynCmd.UndoRedoCommand(DynCmd.UndoRedoCommand.Operation.Redo)); + Assert.IsTrue(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + } + + /// + /// Regression test for DYN-10717: undoing only one of two changes must not clear + /// HasUnsavedChanges, since the workspace is still not back at the exact undo-stack + /// depth it was at when last saved/opened. + /// + [Test] + public void TestUndoingOneOfTwoChangesStaysDirty() + { + string openPath = Path.Combine(TestDirectory, "core", "LacingTest.dyn"); + OpenModel(openPath); + + var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes.Take(2).ToList(); + CurrentDynamoModel.ExecuteCommand(new DynCmd.UpdateModelValueCommand(Guid.Empty, nodes[0].GUID, nameof(NodeModel.Position), $"{nodes[0].X + 10};{nodes[0].Y}")); + CurrentDynamoModel.ExecuteCommand(new DynCmd.UpdateModelValueCommand(Guid.Empty, nodes[1].GUID, nameof(NodeModel.Position), $"{nodes[1].X + 10};{nodes[1].Y}")); + Assert.IsTrue(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + + // Undo only the second change -- still dirty, since we're not back at the saved depth. + CurrentDynamoModel.ExecuteCommand(new DynCmd.UndoRedoCommand(DynCmd.UndoRedoCommand.Operation.Undo)); + Assert.IsTrue(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + } + + /// + /// Regression test for a PR review comment on DYN-10717: HasUnsavedChanges can be set + /// directly by workspace-level/administrative code paths that never go through the + /// tagged undo-recording system (e.g. a unit-conversion node's selected units, + /// geometry scale factor, active linter, or custom graph metadata). Undoing an + /// unrelated, separately-tracked edit back to the exact saved undo-stack depth must + /// not silently wipe out that independent dirty signal. + /// + [Test] + public void TestIndependentDirtyFlagSurvivesUnrelatedUndo() + { + string openPath = Path.Combine(TestDirectory, "core", "LacingTest.dyn"); + OpenModel(openPath); + + // Simulate a workspace-level/administrative change that doesn't go through the + // tagged undo-recording system (e.g. LinterViewModel.ActiveLinter, GeometryScalingPopup). + CurrentDynamoModel.CurrentWorkspace.MarkAsIndependentlyModified(); + + var node = CurrentDynamoModel.CurrentWorkspace.Nodes.First(); + CurrentDynamoModel.CurrentWorkspace.RecordModelsForModification(new ModelBase[] { node }); + CurrentDynamoModel.ExecuteCommand(new DynCmd.UndoRedoCommand(DynCmd.UndoRedoCommand.Operation.Undo)); + + // Even though the tagged/tracked edit was fully undone (back to the saved + // undo-depth), the independent administrative dirty flag must keep the + // workspace marked unsaved. + Assert.IsTrue(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + } + // SaveImage //[Test] diff --git a/test/DynamoCoreTests/Models/DynamoModelCommandsTest.cs b/test/DynamoCoreTests/Models/DynamoModelCommandsTest.cs index 11e476489ea..92347ca122f 100644 --- a/test/DynamoCoreTests/Models/DynamoModelCommandsTest.cs +++ b/test/DynamoCoreTests/Models/DynamoModelCommandsTest.cs @@ -191,6 +191,69 @@ public void SelectModelImplTest() Assert.IsNotNull(selectCommand); } + /// + /// Regression test for DYN-10717: selecting or deselecting a model must not mark the + /// workspace as having unsaved changes. Selection is undo-tracked for UX purposes only + /// (so Ctrl+Z can restore it) and is never written to the saved file, so it must not + /// affect the Save button's dirty-flag gating. + /// + [Test] + [Category("UnitTests")] + public void SelectModelImplDoesNotMarkWorkspaceDirtyTest() + { + //Arrange + string openPath = Path.Combine(TestDirectory, "core", "DetailedPreviewMargin_Test.dyn"); + RunModel(openPath); + + var addNode = new DSFunction(CurrentDynamoModel.LibraryServices.GetFunctionDescriptor("+")); + CurrentDynamoModel.CurrentWorkspace.AddAndRegisterNode(addNode, false); + CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges = false; + + //Act -- select the node + CurrentDynamoModel.ExecuteCommand(new DynamoModel.SelectModelCommand(addNode.GUID, ModifierKeys.None)); + + //Assert + Assert.IsFalse(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + + //Act -- clear the selection + CurrentDynamoModel.ExecuteCommand(new DynamoModel.SelectModelCommand(Guid.Empty, ModifierKeys.None)); + + //Assert + Assert.IsFalse(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + } + + /// + /// Regression test for a PR review comment on DYN-10717: since selection changes are + /// still recorded on the undo stack (so Ctrl+Z can restore selection), undoing then + /// redoing a selection-only action group must not incorrectly mark the workspace + /// dirty, even though the undo-stack depth changes across the Undo/Redo cycle. + /// + [Test] + [Category("UnitTests")] + public void UndoRedoOfSelectionOnlyChangeDoesNotMarkWorkspaceDirtyTest() + { + //Arrange + string openPath = Path.Combine(TestDirectory, "core", "DetailedPreviewMargin_Test.dyn"); + RunModel(openPath); + + var addNode = new DSFunction(CurrentDynamoModel.LibraryServices.GetFunctionDescriptor("+")); + CurrentDynamoModel.CurrentWorkspace.AddAndRegisterNode(addNode, false); + CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges = false; + + //Act -- select the node, then undo and redo that selection change + CurrentDynamoModel.ExecuteCommand(new DynamoModel.SelectModelCommand(addNode.GUID, ModifierKeys.None)); + CurrentDynamoModel.ExecuteCommand(new DynamoModel.UndoRedoCommand(DynamoModel.UndoRedoCommand.Operation.Undo)); + + //Assert + Assert.IsFalse(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + + //Act -- redo the selection change + CurrentDynamoModel.ExecuteCommand(new DynamoModel.UndoRedoCommand(DynamoModel.UndoRedoCommand.Operation.Redo)); + + //Assert + Assert.IsFalse(CurrentDynamoModel.CurrentWorkspace.HasUnsavedChanges); + } + /// /// This test method will execute the SelectModelImpl method from the DynamoModel class and does not crash /// diff --git a/test/DynamoCoreTests/UndoRedoRecorderTests.cs b/test/DynamoCoreTests/UndoRedoRecorderTests.cs index 4eaa54cca17..08ff64106e7 100644 --- a/test/DynamoCoreTests/UndoRedoRecorderTests.cs +++ b/test/DynamoCoreTests/UndoRedoRecorderTests.cs @@ -122,6 +122,8 @@ internal DummyModel GetModel(int identifier) internal UndoRedoRecorder Recorder { get { return undoRecorder; } } + internal bool WasMarkedAsModified { get; private set; } + #endregion #region IUndoRedoRecorderClient Members @@ -157,7 +159,12 @@ public ModelBase GetModelForElement(XmlElement modelData) public void UpdateUndoRedoStack() { - + + } + + public void MarkAsModified() + { + WasMarkedAsModified = true; } #endregion @@ -191,6 +198,23 @@ public void TestDefaultRecorderStates() Assert.AreEqual(false, recorder.CanRedo); } + /// + /// Regression test for DYN-10717: recording a modification (e.g. a node/note/group + /// drag or resize) must notify the client so it can mark itself dirty, since that + /// state would otherwise be silently unsavable once Save is gated on the dirty flag. + /// + [Test] + [Category("UnitTests")] + public void TestRecordModificationForUndoMarksClientAsModified() + { + workspace.AddModel(new DummyModel(0, 10)); + Assert.IsFalse(workspace.WasMarkedAsModified); + + workspace.ModifyModel(0); + + Assert.IsTrue(workspace.WasMarkedAsModified); + } + [Test] [Category("UnitTests")] public void TestConstructor() diff --git a/test/DynamoCoreWpfTests/DynamoViewTests.cs b/test/DynamoCoreWpfTests/DynamoViewTests.cs index aa2d494b902..75fa49d32b9 100644 --- a/test/DynamoCoreWpfTests/DynamoViewTests.cs +++ b/test/DynamoCoreWpfTests/DynamoViewTests.cs @@ -19,6 +19,8 @@ using NUnit.Framework; using DynamoMLDataPipeline; using Dynamo.Wpf.UI; +using Dynamo.Wpf.UI.GuidedTour; +using DynamoCoreWpfTests.Utility; namespace DynamoCoreWpfTests @@ -136,6 +138,92 @@ public void TestHomeWorkspaceClosedBeforeCustomNode() Assert.IsTrue(View.saveButton.IsEnabled); } + /// + /// Asserts that the File menu's Save/Save As MenuItems are bound to the expected commands + /// and that IsEnabled reflects the live CanExecute() -- this is the actual regression + /// surface for DYN-10717 (the menu items had a hardcoded IsEnabled="False" in XAML that + /// never tracked CanExecute at all). + /// + private void AssertSaveMenuItemsReflectCanExecute() + { + DispatcherUtil.DoEvents(); + + Assert.AreSame(ViewModel.ShowSaveDialogIfNeededAndSaveResultCommand, View.saveThisButton.Command, + "saveThisButton is not bound to ShowSaveDialogIfNeededAndSaveResultCommand"); + Assert.AreSame(ViewModel.ShowSaveDialogAndSaveResultCommand, View.saveButton.Command, + "saveButton is not bound to ShowSaveDialogAndSaveResultCommand"); + + var expectedSave = ViewModel.ShowSaveDialogIfNeededAndSaveResultCommand.CanExecute(null); + var expectedSaveAs = ViewModel.ShowSaveDialogAndSaveResultCommand.CanExecute(null); + + Assert.AreEqual(expectedSave, View.saveThisButton.IsEnabled, + $"saveThisButton.IsEnabled ({View.saveThisButton.IsEnabled}) does not reflect ShowSaveDialogIfNeededAndSaveResultCommand.CanExecute() ({expectedSave})"); + Assert.AreEqual(expectedSaveAs, View.saveButton.IsEnabled, + $"saveButton.IsEnabled ({View.saveButton.IsEnabled}) does not reflect ShowSaveDialogAndSaveResultCommand.CanExecute() ({expectedSaveAs})"); + } + + [Test] + public void WhenDynamoLaunchesThenSaveAsIsEnabledButSaveIsDisabledUntilDirty() + { + // Regression test for DYN-10717: Save/Save As enablement is driven by a shared CanExecute (menu, hotkey, toolbar all in sync), with "Save" additionally gated on the workspace's dirty flag. + Assert.IsFalse(ViewModel.ShowSaveDialogIfNeededAndSaveResultCommand.CanExecute(null)); + Assert.IsTrue(ViewModel.ShowSaveDialogAndSaveResultCommand.CanExecute(null)); + AssertSaveMenuItemsReflectCanExecute(); + + ViewModel.HomeSpace.HasUnsavedChanges = true; + + Assert.IsTrue(ViewModel.ShowSaveDialogIfNeededAndSaveResultCommand.CanExecute(null)); + AssertSaveMenuItemsReflectCanExecute(); + } + + [Test] + public void WhenLastWorkspaceIsClosedThenSaveMenuItemsAreDisabled() + { + // Regression test for DYN-10717: closing the only open workspace shows the Start Page, where Save/Save As are intentionally disabled (this is not the bug; the bug was menu/hotkey/toolbar disagreeing with each other). + var wasTestMode = DynamoModel.IsTestMode; + try + { + DynamoModel.IsTestMode = false; + ViewModel.CloseHomeWorkspaceCommand.Execute(null); + } + finally + { + DynamoModel.IsTestMode = wasTestMode; + } + + Assert.IsTrue(ViewModel.ShowStartPage); + Assert.IsFalse(ViewModel.ShowSaveDialogIfNeededAndSaveResultCommand.CanExecute(null)); + Assert.IsFalse(ViewModel.ShowSaveDialogAndSaveResultCommand.CanExecute(null)); + AssertSaveMenuItemsReflectCanExecute(); + } + + [Test] + public void WhenGuidedTourIsActiveThenSaveMenuItemsAreDisabledUntilExit() + { + // Regression test for DYN-10717: Save/Save As must be disabled while a guided tour is active (via GuideFlowEvents.IsAnyGuideActive), and re-enabled once it ends. + ViewModel.HomeSpace.HasUnsavedChanges = true; + Assert.IsTrue(ViewModel.ShowSaveDialogIfNeededAndSaveResultCommand.CanExecute(null)); + Assert.IsTrue(ViewModel.ShowSaveDialogAndSaveResultCommand.CanExecute(null)); + AssertSaveMenuItemsReflectCanExecute(); + + try + { + GuideFlowEvents.OnGuidedTourStart("test"); + + Assert.IsFalse(ViewModel.ShowSaveDialogIfNeededAndSaveResultCommand.CanExecute(null)); + Assert.IsFalse(ViewModel.ShowSaveDialogAndSaveResultCommand.CanExecute(null)); + AssertSaveMenuItemsReflectCanExecute(); + } + finally + { + GuideFlowEvents.OnGuidedTourFinish("test"); + } + + Assert.IsTrue(ViewModel.ShowSaveDialogIfNeededAndSaveResultCommand.CanExecute(null)); + Assert.IsTrue(ViewModel.ShowSaveDialogAndSaveResultCommand.CanExecute(null)); + AssertSaveMenuItemsReflectCanExecute(); + } + [Test] public void ElementBinding_SaveAs() {