From f96f365b51cd99c13b293bb9e7b471127f5e23af Mon Sep 17 00:00:00 2001
From: Thomas Robitaille <thomas.robitaille@gmail.com>
Date: Tue, 17 Sep 2024 10:54:07 +0100
Subject: [PATCH] Avoid broadcasting a create subset message with an empty
 subset state followed by an update message, instead just broadcast a create
 subset message with the correct subset state

---
 glue/core/edit_subset_mode.py            |  6 ++-
 glue/core/tests/test_edit_subset_mode.py | 47 +++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/glue/core/edit_subset_mode.py b/glue/core/edit_subset_mode.py
index 9eb83ff83..3784f8cf0 100644
--- a/glue/core/edit_subset_mode.py
+++ b/glue/core/edit_subset_mode.py
@@ -71,7 +71,11 @@ def _combine_data(self, new_state, override_mode=None):
             if self.data_collection is None:
                 raise RuntimeError("Must set data_collection before "
                                    "calling update")
-            self.edit_subset = [self.data_collection.new_subset_group()]
+            # We set the subset state directly here to avoid double messages
+            # (create then update). As the subset is new at this point,
+            # it doesn't make sense to apply a mode in any case.
+            self.edit_subset = [self.data_collection.new_subset_group(subset_state=new_state.copy())]
+            return
         subs = self._edit_subset
         for s in as_list(subs):
             mode(s, new_state)
diff --git a/glue/core/tests/test_edit_subset_mode.py b/glue/core/tests/test_edit_subset_mode.py
index 487ae395e..a6dffcc1d 100644
--- a/glue/core/tests/test_edit_subset_mode.py
+++ b/glue/core/tests/test_edit_subset_mode.py
@@ -1,12 +1,19 @@
 # pylint: disable=I0011,W0613,W0201,W0212,E1101,E1103
 
+import operator
+from unittest.mock import MagicMock
+
 import numpy as np
 
+from ..application_base import Application
+from ..hub import HubListener
+from ..message import SubsetCreateMessage, SubsetUpdateMessage
+from ..command import ApplySubsetState
 from ..data import Component, Data
 from ..data_collection import DataCollection
 from ..edit_subset_mode import (EditSubsetMode, ReplaceMode, OrMode, AndMode,
                                 XorMode, AndNotMode)
-from ..subset import ElementSubsetState
+from ..subset import ElementSubsetState, InequalitySubsetState
 
 
 class TestEditSubsetMode(object):
@@ -87,3 +94,41 @@ def test_combines_make_copy(self):
         self.edit_mode.edit_subset = self.data.new_subset()
         self.edit_mode.update(self.data, self.state2)
         assert self.edit_mode.edit_subset.subset_state is not self.state2
+
+
+def test_no_double_messaging():
+
+    # Make sure that when we create a new subset via EditSubsetMode, we don't
+    # get two separate messages for creation and updating, but instead just a
+    # single create message with the right subset state.
+
+    handler = MagicMock()
+
+    app = Application()
+
+    class Listener(HubListener):
+        pass
+
+    listener = Listener()
+
+    app.session.hub.subscribe(listener, SubsetCreateMessage, handler=handler)
+    app.session.hub.subscribe(listener, SubsetUpdateMessage, handler=handler)
+
+    data = Data(x=[1, 2, 3, 4, 5])
+
+    app.data_collection.append(data)
+
+    cmd = ApplySubsetState(data_collection=app.data_collection,
+                           subset_state=data.id['x'] >= 3,
+                           override_mode=ReplaceMode)
+
+    app.session.command_stack.do(cmd)
+
+    assert handler.call_count == 1
+    message = handler.call_args[0][0]
+    assert isinstance(message, SubsetCreateMessage)
+    print(type(message.subset.subset_state))
+    assert isinstance(message.subset.subset_state, InequalitySubsetState)
+    assert message.subset.subset_state.left is data.id['x']
+    assert message.subset.subset_state.operator is operator.ge
+    assert message.subset.subset_state.right == 3