Set Packageinfo Restart State (#9982)

QilongTang · web-flow · commit 499bd9b359fd · 2019-09-17T15:50:36.000-04:00
* Initial Commit

* workaround for setting restart state

* Add restart banner

* Property binding does not work for me:(

* Add Unit test

* Some clean up, now getter is more consistent

* Address Comments

* Comments

* Comments

* Comments and more tests
diff --git a/src/DynamoCore/Graph/Workspaces/PackageDependencyInfo.cs b/src/DynamoCore/Graph/Workspaces/PackageDependencyInfo.cs
@@ -12,7 +12,7 @@ public enum PackageDependencyState
         IncorrectVersion,  // Correct package but incorrect version. 
         Missing,           // package is completely missing.
         Warning,           // Actual package is missing but the nodes are resolved by some other package. 
-        RequiresRestart    // Restart needed in order to complete the uninstall of some package. 
+        RequiresRestart    // Restart needed in order to complete the uninstall of some package. Notice this would be only set when workspace dependency viewer is loaded.
     }
 
     /// <summary>
diff --git a/src/DynamoCore/Graph/Workspaces/WorkspaceModel.cs b/src/DynamoCore/Graph/Workspaces/WorkspaceModel.cs
@@ -553,7 +553,7 @@ internal List<INodeLibraryDependencyInfo> NodeLibraryDependencies
                         {
                             packageDependencies[saved].State = PackageDependencyState.Missing;
                         }
-                        // If the state is Missing for atleast one of the nodes,
+                        // If the state is Missing for at least one of the nodes,
                         // we set the state of the whole package dependency to Missing.
                         // Set other states accordingly, only if the PackageDependencyState(for that package)
                         // is not set to Missing by any of the other nodes. 
@@ -566,8 +566,9 @@ internal List<INodeLibraryDependencyInfo> NodeLibraryDependencies
                                 {
                                     packageDependencies[saved].State = PackageDependencyState.Loaded;
                                 }
-                                // if incorrect version of package is installed.
-                                else
+                                // If incorrect version of package is installed and not marked for uninstall,
+                                // set the state. Otherwise, keep the RequiresRestart state away from overwritten.
+                                else if(packageDependencies[saved].State != PackageDependencyState.RequiresRestart)
                                 {
                                     packageDependencies[saved].State = PackageDependencyState.IncorrectVersion;
                                 }
diff --git a/src/DynamoCoreWpf/ViewModels/PackageManager/IPackageInstaller.cs b/src/DynamoCoreWpf/ViewModels/PackageManager/IPackageInstaller.cs
@@ -1,9 +1,4 @@
-﻿using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-using Dynamo.Graph.Workspaces;
+﻿using Dynamo.Graph.Workspaces;
 
 namespace Dynamo.ViewModels
 {
diff --git a/src/DynamoPackages/Package.cs b/src/DynamoPackages/Package.cs
@@ -101,7 +101,7 @@ public bool TypesVisibleInManager
         public bool MarkedForUninstall
         {
             get { return markedForUninstall; }
-            private set { markedForUninstall = value; RaisePropertyChanged("MarkedForUninstall"); }
+            internal set { markedForUninstall = value; RaisePropertyChanged("MarkedForUninstall"); }
         }
 
         private string _group = "";
diff --git a/src/DynamoPackages/Properties/AssemblyInfo.cs b/src/DynamoPackages/Properties/AssemblyInfo.cs
@@ -15,3 +15,4 @@
 [assembly: InternalsVisibleTo("DynamoCoreWpf")]
 [assembly: InternalsVisibleTo("DynamoPackagesUI")]
 [assembly: InternalsVisibleTo("LibraryViewExtension")]
+[assembly: InternalsVisibleTo("DynamoCoreWpfTests")]
diff --git a/src/WorkspaceDependencyViewExtension/Properties/Resources.Designer.cs b/src/WorkspaceDependencyViewExtension/Properties/Resources.Designer.cs
diff --git a/src/WorkspaceDependencyViewExtension/Properties/Resources.en-US.resx b/src/WorkspaceDependencyViewExtension/Properties/Resources.en-US.resx
@@ -129,6 +129,9 @@
   <data name="DetailsMessageMissing" xml:space="preserve">
     <value>This graph depends on {0} {1}. Would you like to download and install this package?</value>
   </data>
+  <data name="DetailsMessageRequireRestart" xml:space="preserve">
+    <value>The package needs to be uninstalled before you can install a different version of it. Please restart Dynamo and then install the specified version.</value>
+  </data>
   <data name="DetailsMessageWarning" xml:space="preserve">
     <value>Some nodes in this graph were originally created with {0} v{1}, which you do not have installed. You may have a different version of {0} installed, or you may have a different package which resolves these nodes.</value>
   </data>
@@ -362,4 +365,7 @@
   <data name="ProvideFeedbackError" xml:space="preserve">
     <value>Could not redirect to the Dynamo forum page for feedback:</value>
   </data>
+  <data name="RestartBannerMessage" xml:space="preserve">
+    <value>An installed package is marked for uninstall. To complete the uninstall, please restart Dynamo.</value>
+  </data>
 </root>
diff --git a/src/WorkspaceDependencyViewExtension/Properties/Resources.resx b/src/WorkspaceDependencyViewExtension/Properties/Resources.resx
@@ -129,6 +129,9 @@
   <data name="DetailsMessageMissing" xml:space="preserve">
     <value>This graph depends on {0} {1}. Would you like to download and install this package?</value>
   </data>
+  <data name="DetailsMessageRequireRestart" xml:space="preserve">
+    <value>The package needs to be uninstalled before you can install a different version of it. Please restart Dynamo and then install the specified version.</value>
+  </data>
   <data name="DetailsMessageWarning" xml:space="preserve">
     <value>Some nodes in this graph were originally created with {0} v{1}, which you do not have installed. You may have a different version of {0} installed, or you may have a different package which resolves these nodes.</value>
   </data>
@@ -362,4 +365,7 @@
   <data name="ProvideFeedbackError" xml:space="preserve">
     <value>Could not redirect to the Dynamo forum page for feedback:</value>
   </data>
+  <data name="RestartBannerMessage" xml:space="preserve">
+    <value>An installed package is marked for uninstall. To complete the uninstall, please restart Dynamo.</value>
+  </data>
 </root>
diff --git a/src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml b/src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml
@@ -197,7 +197,23 @@
 
         <!-- Template for the feedback banner -->
         <DockPanel Name="Feedback" VerticalAlignment="Bottom" Grid.Row="2" Width="600">
-            <StatusBar Background="{StaticResource FeedbackSectionBackground}" DockPanel.Dock="Bottom" Height="55">
+            <!-- Restart Banner -->
+            <StatusBar
+                Name="RestartBanner"
+                Background="LightCoral"
+                DockPanel.Dock="Top"
+                Height="55"
+                Margin="1"
+                Visibility="Hidden">
+                <!-- Preview text -->
+                <StatusBarItem>
+                    <Border Height="50">
+                        <TextBlock TextWrapping="Wrap" Width="500" Padding="10,0,10,0" FontSize="15" Foreground="White" Text="{x:Static w:Resources.RestartBannerMessage}" VerticalAlignment="Center"/>
+                    </Border>
+                </StatusBarItem>
+            </StatusBar>
+            <!-- Feedback Banner -->
+            <StatusBar Background="{StaticResource FeedbackSectionBackground}" DockPanel.Dock="Bottom" Height="55" Margin="1">
                 <!-- Preview text -->
                 <StatusBarItem>
                     <Border Height="50">
diff --git a/src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml.cs b/src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml.cs
@@ -1,4 +1,5 @@
 ﻿using System;
+using System.Collections.Generic;
 using System.ComponentModel;
 using System.Drawing;
 using System.Linq;
@@ -21,18 +22,27 @@ public partial class WorkspaceDependencyView : UserControl
 
         private WorkspaceModel currentWorkspace;
 
-        private string FeedbackLink = "https://forum.dynamobim.com/t/call-for-feedback-on-dynamo-graph-package-dependency-display/37229";
+        /// <summary>
+        /// The hyper link where Dynamo user will be forwarded to for submitting comments.
+        /// </summary>
+        private readonly string FeedbackLink = "https://forum.dynamobim.com/t/call-for-feedback-on-dynamo-graph-package-dependency-display/37229";
 
         private ViewLoadedParams loadedParams;
         private WorkspaceDependencyViewExtension dependencyViewExtension;
 
         private IPackageInstaller packageInstaller;
 
-        private Boolean hasDependencyIssue = false;
+        /// <summary>
+        /// Internal cache of the data displayed in data grid, useful in unit testing.
+        /// You are not expected to modify this but rather inspection.
+        /// </summary>
+        internal IEnumerable<PackageDependencyRow> dataRows;
 
+        private Boolean hasDependencyIssue = false;
         /// <summary>
-        /// Property to check if the current workspace has any package dependencies
-        /// issue worth workspace author's attention
+        /// Property to check if the Dynamo active workspace has any package dependencies
+        /// issue worth workspace author's attention. This determines if the package dep 
+        /// viewer will be injected into right panel.
         /// </summary>
         private Boolean HasDependencyIssue
         {
@@ -52,10 +62,12 @@ private Boolean HasDependencyIssue
         /// </summary>
         private void ProvideFeedback(object sender, EventArgs e)
         {
-            try {
+            try
+            {
                 System.Diagnostics.Process.Start(FeedbackLink);
             }
-            catch (Exception ex) {
+            catch (Exception ex)
+            {
                 String message = Dynamo.WorkspaceDependency.Properties.Resources.ProvideFeedbackError + "\n\n" + ex.Message;
                 MessageBox.Show(message, "Error", MessageBoxButton.OK, MessageBoxImage.Error);
             }
@@ -106,23 +118,33 @@ private void OnWorkspacePropertyChanged(object sender, PropertyChangedEventArgs
         /// <param name="ws">workspace model</param>
         internal void DependencyRegen(WorkspaceModel ws)
         {
+            this.RestartBanner.Visibility = Visibility.Hidden;
             var packageDependencies = ws.NodeLibraryDependencies.Where(d => d is PackageDependencyInfo);
 
             if (packageDependencies.Any(d => d.State != PackageDependencyState.Loaded))
             {
                 HasDependencyIssue = true;
             }
 
-            PackageDependencyTable.ItemsSource = packageDependencies.Select(d => new PackageDependencyRow(d as PackageDependencyInfo));
+            // If package is set to uninstall state, update the package info
+            foreach (var package in dependencyViewExtension.pmExtension.PackageLoader.LocalPackages.Where(x => x.MarkedForUninstall))
+            {
+                (packageDependencies.Where(x => x.Name == package.Name).FirstOrDefault() as PackageDependencyInfo).State = PackageDependencyState.RequiresRestart;
+                this.RestartBanner.Visibility = Visibility.Visible;
+            }
+
+            dataRows = packageDependencies.Select(d => new PackageDependencyRow(d as PackageDependencyInfo));
+            PackageDependencyTable.ItemsSource = dataRows;
         }
 
         /// <summary>
         /// Constructor
         /// </summary>
         /// <param name="p">ViewLoadedParams</param>
-        public WorkspaceDependencyView(WorkspaceDependencyViewExtension viewExtension,ViewLoadedParams p)
+        public WorkspaceDependencyView(WorkspaceDependencyViewExtension viewExtension, ViewLoadedParams p)
         {
             InitializeComponent();
+            this.DataContext = this;
             currentWorkspace = p.CurrentWorkspaceModel as WorkspaceModel;
             p.CurrentWorkspaceChanged += OnWorkspaceChanged;
             p.CurrentWorkspaceCleared += OnWorkspaceCleared;
@@ -132,18 +154,33 @@ public WorkspaceDependencyView(WorkspaceDependencyViewExtension viewExtension,Vi
             dependencyViewExtension = viewExtension;
             DependencyRegen(currentWorkspace);
         }
-        
+
         /// <summary>
         /// Send a request to the package manager client to download this package and its dependencies
         /// </summary>
         /// <param name="sender"></param>
         /// <param name="e"></param>
         private void DownloadPackage(object sender, RoutedEventArgs e)
         {
-            var info = ((PackageDependencyRow)((Button)sender).DataContext).DependencyInfo;
-            var package = new PackageInfo(info.Name, info.Version);
+            try
+            {
+                var info = ((PackageDependencyRow)((Button)sender).DataContext).DependencyInfo;
+                DownloadSpecifiedPackageAndRefresh(info);
+            }
+            catch (Exception ex)
+            {
+                dependencyViewExtension.OnMessageLogged(LogMessage.Info(String.Format(Properties.Resources.DependencyViewExtensionErrorTemplate, ex.ToString())));
+            }
+        }
 
-            packageInstaller.DownloadAndInstallPackage(package);
+        /// <summary>
+        /// Downloaded the specified package according to serialized dyn
+        /// and refresh the view of dependency viewer
+        /// </summary>
+        /// <param name="info">Target PackageDependencyInfo to download</param>
+        internal void DownloadSpecifiedPackageAndRefresh(PackageDependencyInfo info)
+        {
+            packageInstaller.DownloadAndInstallPackage(info);
             DependencyRegen(currentWorkspace);
         }
 
@@ -160,7 +197,7 @@ private void KeepLocalPackage(object sender, RoutedEventArgs e)
                 var info = ((PackageDependencyRow)((Button)sender).DataContext).DependencyInfo;
                 UpdateWorkspaceToUseInstalledPackage(info);
             }
-            catch(Exception ex)
+            catch (Exception ex)
             {
                 dependencyViewExtension.OnMessageLogged(LogMessage.Info(String.Format(Properties.Resources.DependencyViewExtensionErrorTemplate, ex.ToString())));
             }
@@ -224,12 +261,12 @@ public string DetailsMessage
                 switch (DependencyInfo.State)
                 {
                     case PackageDependencyState.Loaded:
-                        message = string.Format(Properties.Resources.DetailsMessageLoaded, 
+                        message = string.Format(Properties.Resources.DetailsMessageLoaded,
                             DependencyInfo.Name, DependencyInfo.Version.ToString());
                         break;
 
                     case PackageDependencyState.Missing:
-                        message = string.Format(Properties.Resources.DetailsMessageMissing, 
+                        message = string.Format(Properties.Resources.DetailsMessageMissing,
                             DependencyInfo.Name, DependencyInfo.Version.ToString());
                         break;
 
@@ -238,8 +275,12 @@ public string DetailsMessage
                             DependencyInfo.Name, DependencyInfo.Version.ToString());
                         break;
 
+                    case PackageDependencyState.RequiresRestart:
+                        message = string.Format(Properties.Resources.DetailsMessageRequireRestart);
+                        break;
+
                     default:
-                        message = string.Format(Properties.Resources.DetailsMessageWarning, 
+                        message = string.Format(Properties.Resources.DetailsMessageWarning,
                             DependencyInfo.Name, DependencyInfo.Version.ToString());
                         break;
                 }
@@ -267,12 +308,16 @@ public ImageSource Icon
                         bitmap = Properties.Resources.NodeLibraryDependency_Missing;
                         break;
 
+                    case PackageDependencyState.RequiresRestart:
+                        bitmap = Properties.Resources.NodeLibraryDependency_Warning;
+                        break;
+
                     default:
                         bitmap = Properties.Resources.NodeLibraryDependency_Warning;
                         break;
                 }
 
-                return ResourceUtilities.ConvertToImageSource(bitmap); 
+                return ResourceUtilities.ConvertToImageSource(bitmap);
             }
         }
 
diff --git a/test/DynamoCoreWpfTests/ViewExtensions/WorkspaceDependencyViewExtensionTests.cs b/test/DynamoCoreWpfTests/ViewExtensions/WorkspaceDependencyViewExtensionTests.cs

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ public enum PackageDependencyState`
`12`	`12`	`IncorrectVersion, // Correct package but incorrect version.`
`13`	`13`	`Missing, // package is completely missing.`
`14`	`14`	`Warning, // Actual package is missing but the nodes are resolved by some other package.`
`15`		`- RequiresRestart // Restart needed in order to complete the uninstall of some package.`
	`15`	`+ RequiresRestart // Restart needed in order to complete the uninstall of some package. Notice this would be only set when workspace dependency viewer is loaded.`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`/// <summary>`
Original file line number	Diff line number	Diff line change
`@@ -553,7 +553,7 @@ internal List<INodeLibraryDependencyInfo> NodeLibraryDependencies`
`553`	`553`	`{`
`554`	`554`	`packageDependencies[saved].State = PackageDependencyState.Missing;`
`555`	`555`	`}`
`556`		`- // If the state is Missing for atleast one of the nodes,`
	`556`	`+ // If the state is Missing for at least one of the nodes,`
`557`	`557`	`// we set the state of the whole package dependency to Missing.`
`558`	`558`	`// Set other states accordingly, only if the PackageDependencyState(for that package)`
`559`	`559`	`// is not set to Missing by any of the other nodes.`
`@@ -566,8 +566,9 @@ internal List<INodeLibraryDependencyInfo> NodeLibraryDependencies`
`566`	`566`	`{`
`567`	`567`	`packageDependencies[saved].State = PackageDependencyState.Loaded;`
`568`	`568`	`}`
`569`		`- // if incorrect version of package is installed.`
`570`		`- else`
	`569`	`+ // If incorrect version of package is installed and not marked for uninstall,`
	`570`	`+ // set the state. Otherwise, keep the RequiresRestart state away from overwritten.`
	`571`	`+ else if(packageDependencies[saved].State != PackageDependencyState.RequiresRestart)`
`571`	`572`	`{`
`572`	`573`	`packageDependencies[saved].State = PackageDependencyState.IncorrectVersion;`
`573`	`574`	`}`
Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ public bool TypesVisibleInManager`
`101`	`101`	`public bool MarkedForUninstall`
`102`	`102`	`{`
`103`	`103`	`get { return markedForUninstall; }`
`104`		`- private set { markedForUninstall = value; RaisePropertyChanged("MarkedForUninstall"); }`
	`104`	`+ internal set { markedForUninstall = value; RaisePropertyChanged("MarkedForUninstall"); }`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`private string _group = "";`