From 120f2576aa95805c29bd1500d49eab94a8ecb98b Mon Sep 17 00:00:00 2001 From: RoDuth Date: Fri, 6 Dec 2024 15:10:23 +1000 Subject: [PATCH] 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: 9a183861 --- bauble/plugins/garden/plant.py | 36 +++++++++++++----------- bauble/plugins/garden/test_garden.py | 42 ++++++++++++++-------------- bauble/view.py | 1 - 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/bauble/plugins/garden/plant.py b/bauble/plugins/garden/plant.py index 66847703..d10da2b2 100755 --- a/bauble/plugins/garden/plant.py +++ b/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 ( diff --git a/bauble/plugins/garden/test_garden.py b/bauble/plugins/garden/test_garden.py index e71e8f6c..f4a28465 100644 --- a/bauble/plugins/garden/test_garden.py +++ b/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): diff --git a/bauble/view.py b/bauble/view.py index 259de4ce..447f99c0 100755 --- a/bauble/view.py +++ b/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.