From 4037da49c28de14ccbddf2e83f7825cb8dfeeca4 Mon Sep 17 00:00:00 2001
From: Huy Pham <42948410+hpham@users.noreply.github.com>
Date: Thu, 30 Apr 2020 12:48:51 -0700
Subject: [PATCH 1/2] Fix issue: node's services won't save when clearing all
 the services and add default services back to the node. Set core node's
 services to default services (instead of leaving it empty) when a new node is
 created.

---
 daemon/core/gui/coreclient.py          |  4 ++++
 daemon/core/gui/dialogs/nodeservice.py | 19 ++++++++++---------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/daemon/core/gui/coreclient.py b/daemon/core/gui/coreclient.py
index b6b13c58..4940d7ad 100644
--- a/daemon/core/gui/coreclient.py
+++ b/daemon/core/gui/coreclient.py
@@ -815,6 +815,10 @@ class CoreClient:
         if NodeUtils.is_custom(node_type, model):
             services = NodeUtils.get_custom_node_services(self.app.guiconfig, model)
             node.services[:] = services
+        else:
+            services = self.default_services.get(model, None)
+            if services:
+                node.services[:] = services
         logging.info(
             "add node(%s) to session(%s), coordinates(%s, %s)",
             node.name,
diff --git a/daemon/core/gui/dialogs/nodeservice.py b/daemon/core/gui/dialogs/nodeservice.py
index 4927fece..e8f67220 100644
--- a/daemon/core/gui/dialogs/nodeservice.py
+++ b/daemon/core/gui/dialogs/nodeservice.py
@@ -153,18 +153,19 @@ class NodeServiceDialog(Dialog):
             )
 
     def click_save(self):
-        # if node is custom type or current services are not the default services then
-        # set core node services and add node to modified services node set
+        core_node = self.canvas_node.core_node
+        # custom type node or CORE node with custom services
         if (
-            self.canvas_node.core_node.model not in self.app.core.default_services
-            or self.current_services
-            != self.app.core.default_services[self.canvas_node.core_node.model]
+            core_node.model not in self.app.core.default_services
+            or self.current_services != self.app.core.default_services[core_node.model]
         ):
-            self.canvas_node.core_node.services[:] = self.current_services
-            self.app.core.modified_service_nodes.add(self.canvas_node.core_node.id)
+            core_node.services[:] = self.current_services
+            self.app.core.modified_service_nodes.add(core_node.id)
+        # custom services CORE node but modified back to having default services
+        # or just CORE nodes that don't get any change
         else:
-            if len(self.canvas_node.core_node.services) > 0:
-                self.canvas_node.core_node.services[:] = []
+            core_node.services[:] = self.current_services
+            self.app.core.modified_service_nodes.discard(core_node.id)
         self.destroy()
 
     def click_cancel(self):

From 580641f5d96168aaffd7e77c5ef5a16ceef20973 Mon Sep 17 00:00:00 2001
From: Huy Pham <42948410+hpham@users.noreply.github.com>
Date: Thu, 30 Apr 2020 13:47:45 -0700
Subject: [PATCH 2/2] remove CoreClient.modified_service_node. When a new CORE
 node is created, assign default services right away (instead of leaving it
 empty), therefore no more confusion whether [] means empty service or means
 CORE node with default services

---
 daemon/core/gui/coreclient.py          |  7 +---
 daemon/core/gui/dialogs/nodeservice.py | 49 +++-----------------------
 daemon/core/gui/graph/graph.py         |  5 ---
 3 files changed, 6 insertions(+), 55 deletions(-)

diff --git a/daemon/core/gui/coreclient.py b/daemon/core/gui/coreclient.py
index 4940d7ad..f3ec6612 100644
--- a/daemon/core/gui/coreclient.py
+++ b/daemon/core/gui/coreclient.py
@@ -85,7 +85,6 @@ class CoreClient:
         self.handling_events = None
         self.xml_dir = None
         self.xml_file = None
-        self.modified_service_nodes = set()
 
     @property
     def client(self):
@@ -112,7 +111,6 @@ class CoreClient:
         self.links.clear()
         self.hooks.clear()
         self.emane_config = None
-        self.modified_service_nodes.clear()
         for mobility_player in self.mobility_players.values():
             mobility_player.handle_close()
         self.mobility_players.clear()
@@ -815,6 +813,7 @@ class CoreClient:
         if NodeUtils.is_custom(node_type, model):
             services = NodeUtils.get_custom_node_services(self.app.guiconfig, model)
             node.services[:] = services
+        # assign default services to CORE node
         else:
             services = self.default_services.get(model, None)
             if services:
@@ -840,7 +839,6 @@ class CoreClient:
                 logging.error("unknown node: %s", node_id)
                 continue
             del self.canvas_nodes[node_id]
-            self.modified_service_nodes.discard(node_id)
             for edge in canvas_node.edges:
                 if edge in edges:
                     continue
@@ -1056,9 +1054,6 @@ class CoreClient:
         )
         return dict(config)
 
-    def service_been_modified(self, node_id: int) -> bool:
-        return node_id in self.modified_service_nodes
-
     def execute_script(self, script):
         response = self.client.execute_script(script)
         logging.info("execute python script %s", response)
diff --git a/daemon/core/gui/dialogs/nodeservice.py b/daemon/core/gui/dialogs/nodeservice.py
index e8f67220..3e716627 100644
--- a/daemon/core/gui/dialogs/nodeservice.py
+++ b/daemon/core/gui/dialogs/nodeservice.py
@@ -3,11 +3,10 @@ core node services
 """
 import tkinter as tk
 from tkinter import messagebox, ttk
-from typing import TYPE_CHECKING, Any, Set
+from typing import TYPE_CHECKING, Any
 
 from core.gui.dialogs.dialog import Dialog
 from core.gui.dialogs.serviceconfig import ServiceConfigDialog
-from core.gui.nodeutils import NodeUtils
 from core.gui.themes import FRAME_PAD, PADX, PADY
 from core.gui.widgets import CheckboxList, ListboxScroll
 
@@ -17,13 +16,7 @@ if TYPE_CHECKING:
 
 
 class NodeServiceDialog(Dialog):
-    def __init__(
-        self,
-        master: Any,
-        app: "Application",
-        canvas_node: "CanvasNode",
-        services: Set[str] = None,
-    ):
+    def __init__(self, master: Any, app: "Application", canvas_node: "CanvasNode"):
         title = f"{canvas_node.core_node.name} Services"
         super().__init__(master, app, title, modal=True)
         self.app = app
@@ -32,24 +25,7 @@ class NodeServiceDialog(Dialog):
         self.groups = None
         self.services = None
         self.current = None
-        if services is None:
-            services = canvas_node.core_node.services
-            model = canvas_node.core_node.model
-            if len(services) == 0:
-                # not custom node type and node's services haven't been modified before
-                if not NodeUtils.is_custom(
-                    canvas_node.core_node.type, canvas_node.core_node.model
-                ) and not self.app.core.service_been_modified(self.node_id):
-                    services = set(self.app.core.default_services[model])
-                # services of default type nodes were modified to be empty
-                elif canvas_node.core_node.id in self.app.core.modified_service_nodes:
-                    services = set()
-                else:
-                    services = set(
-                        NodeUtils.get_custom_node_services(self.app.guiconfig, model)
-                    )
-            else:
-                services = set(services)
+        services = set(canvas_node.core_node.services)
         self.current_services = services
         self.draw()
 
@@ -103,7 +79,7 @@ class NodeServiceDialog(Dialog):
         button.grid(row=0, column=1, sticky="ew", padx=PADX)
         button = ttk.Button(frame, text="Remove", command=self.click_remove)
         button.grid(row=0, column=2, sticky="ew", padx=PADX)
-        button = ttk.Button(frame, text="Cancel", command=self.click_cancel)
+        button = ttk.Button(frame, text="Cancel", command=self.destroy)
         button.grid(row=0, column=3, sticky="ew")
 
         # trigger group change
@@ -154,22 +130,7 @@ class NodeServiceDialog(Dialog):
 
     def click_save(self):
         core_node = self.canvas_node.core_node
-        # custom type node or CORE node with custom services
-        if (
-            core_node.model not in self.app.core.default_services
-            or self.current_services != self.app.core.default_services[core_node.model]
-        ):
-            core_node.services[:] = self.current_services
-            self.app.core.modified_service_nodes.add(core_node.id)
-        # custom services CORE node but modified back to having default services
-        # or just CORE nodes that don't get any change
-        else:
-            core_node.services[:] = self.current_services
-            self.app.core.modified_service_nodes.discard(core_node.id)
-        self.destroy()
-
-    def click_cancel(self):
-        self.current_services = None
+        core_node.services[:] = self.current_services
         self.destroy()
 
     def click_remove(self):
diff --git a/daemon/core/gui/graph/graph.py b/daemon/core/gui/graph/graph.py
index 40a941b1..fb0f39e0 100644
--- a/daemon/core/gui/graph/graph.py
+++ b/daemon/core/gui/graph/graph.py
@@ -936,11 +936,6 @@ class CanvasGraph(tk.Canvas):
             node.service_file_configs = deepcopy(canvas_node.service_file_configs)
             node.config_service_configs = deepcopy(canvas_node.config_service_configs)
 
-            # add new node to modified_service_nodes set if that set contains the
-            # to_copy node
-            if self.core.service_been_modified(core_node.id):
-                self.core.modified_service_nodes.add(copy.id)
-
             copy_map[canvas_node.id] = node.id
             self.core.canvas_nodes[copy.id] = node
             self.nodes[node.id] = node