PlantEditor - avoid pointless change on save

Move on_save_clicked from the presenter class to the editor class so it
can use the same `commit_changes()` as OK, etc. and avoid adding
pointless PlantChanges. related: 9a18386

bauble/plugins/garden/plant.py

@@ -1489,11 +1489,6 @@ def on_reason_changed(combo):
             self.on_loc_button_clicked,
             "edit",
         )
-        self.view.connect(
-            "pad_save_button",
-            "clicked",
-            self.on_save_clicked,
-        )
         if self.model.quantity == 0:
             self.view.widgets.notebook.set_sensitive(False)
             msg = _(
@@ -1524,18 +1519,6 @@ def on_response(_button, response):
         )
         self.view.widgets.plant_id_label.set_text(str(self.model.id or ""))
 
-    def on_save_clicked(self, *_args):
-        try:
-            self.commit_changes()
-            self._dirty = False
-            self.pictures_presenter._dirty = False
-            self.notes_presenter._dirty = False
-            self.prop_presenter._dirty = False
-        except SQLAlchemyError as e:
-            logger.debug("%s(%s)", type(e).__name__, e)
-        finally:
-            self.refresh_view()
-
     def acc_get_completions(self, text):
         """Get completions with any of the following combinations:
         'accession_code',
@@ -2015,6 +1998,13 @@ def __init__(self, model=None, parent=None, branch_mode=False):
         self._committed = []
 
         view = PlantEditorView(parent=self.parent)
+        # NOTE have to connect here rather than the presenter so that commit
+        # works as expected (e.g. pointless PlantChange)
+        view.connect(
+            "pad_save_button",
+            "clicked",
+            self.on_save_clicked,
+        )
         self.presenter = PlantEditorPresenter(
             self.model, view, branch_mode=branch_mode
         )
@@ -2034,6 +2024,18 @@ def compute_plant_split_changes(self):
             to_plant_change=self.presenter.change,
         )
 
+    def on_save_clicked(self, *_args):
+        try:
+            self.commit_changes()
+            self.presenter._dirty = False
+            self.presenter.pictures_presenter._dirty = False
+            self.presenter.notes_presenter._dirty = False
+            self.presenter.prop_presenter._dirty = False
+        except SQLAlchemyError as e:
+            logger.debug("%s(%s)", type(e).__name__, e)
+        finally:
+            self.presenter.refresh_view()
+
     def commit_changes(self):
         codes = utils.range_builder(self.model.code)
         if (

bauble/plugins/garden/test_garden.py

@@ -1223,6 +1223,27 @@ def test_commit_changes_does_not_generate_pointless_changes(self):
         editor.presenter.cleanup()
         del editor
 
+    def test_on_save_clicked(self):
+        mock_self = unittest.mock.Mock()
+        PlantEditor.on_save_clicked(mock_self)
+        mock_self.commit_changes.assert_called()
+        self.assertFalse(mock_self.presenter._dirty)
+        self.assertFalse(mock_self.presenter.pictures_presenter._dirty)
+        self.assertFalse(mock_self.presenter.notes_presenter._dirty)
+        self.assertFalse(mock_self.presenter.prop_presenter._dirty)
+        mock_self.session.rollback.assert_not_called()
+        mock_self.presenter.refresh_view.assert_called()
+
+        mock_self = unittest.mock.Mock()
+        mock_self.commit_changes.side_effect = SQLAlchemyError
+        PlantEditor.on_save_clicked(mock_self)
+        mock_self.commit_changes.assert_called()
+        self.assertTrue(mock_self.presenter._dirty)
+        self.assertTrue(mock_self.presenter.pictures_presenter._dirty)
+        self.assertTrue(mock_self.presenter.notes_presenter._dirty)
+        self.assertTrue(mock_self.presenter.prop_presenter._dirty)
+        mock_self.presenter.refresh_view.assert_called()
+
 
 class PlantEditorPresenterTests(GardenTestCase):
     def test_acc_get_completions(self):
@@ -1328,27 +1349,6 @@ def test_on_loc_button_clicked(self, mock_editor):
 
         del presenter
 
-    def test_on_save_clicked(self):
-        mock_self = unittest.mock.Mock()
-        PlantEditorPresenter.on_save_clicked(mock_self)
-        mock_self.commit_changes.assert_called()
-        self.assertFalse(mock_self._dirty)
-        self.assertFalse(mock_self.pictures_presenter._dirty)
-        self.assertFalse(mock_self.notes_presenter._dirty)
-        self.assertFalse(mock_self.prop_presenter._dirty)
-        mock_self.session.rollback.assert_not_called()
-        mock_self.refresh_view.assert_called()
-
-        mock_self = unittest.mock.Mock()
-        mock_self.commit_changes.side_effect = SQLAlchemyError
-        PlantEditorPresenter.on_save_clicked(mock_self)
-        mock_self.commit_changes.assert_called()
-        self.assertTrue(mock_self._dirty)
-        self.assertTrue(mock_self.pictures_presenter._dirty)
-        self.assertTrue(mock_self.notes_presenter._dirty)
-        self.assertTrue(mock_self.prop_presenter._dirty)
-        mock_self.refresh_view.assert_called()
-
 
 class PropagationTests(GardenTestCase):
     def __init__(self, *args):

bauble/view.py

@@ -723,7 +723,6 @@ def __init__(self, parent: Gtk.Paned, pic_pane: Gtk.Paned) -> None:
         cast(Gtk.Widget, pic_pane.get_child2()).connect(
             "size-allocate", self.on_pic_pane_size_allocation
         )
-        self._set_pic_pane_pos_timer_id = None
 
     def on_scrolled(self, adjustment: Gtk.Adjustment) -> None:
         """On scrolling add more pictures as needed.