Repository: feder-observatory/stellarphot

Issue:
Save button is enabled after clicking edit even though no changes have been made
This sequence of events leads to an save button that is enabled even though no changes have been made:

1. Make a new camera or edit an existing one.
2. Click save (after making a change if necessary)
3. Click confirm if necessary
4. Click Edit
5. The save button will be enabled even though no changes have been made.

The root cause is [these lines](https://github.com/feder-observatory/stellarphot/blob/main/stellarphot/settings/custom_widgets.py#L114-L118) which are an attempt to make to prevent the save button dfrom being enabled unless the model is valid.

That is useful, but whether there are any unsaved changes also needs to be taken into account. That is available in `SaveButtonBar.unsaved_changes`.

Discussion:

Patch:
diff --git a/stellarphot/settings/custom_widgets.py b/stellarphot/settings/custom_widgets.py
index f92c6c8..160192e 100644
--- a/stellarphot/settings/custom_widgets.py
+++ b/stellarphot/settings/custom_widgets.py
@@ -105,19 +105,11 @@ class ChooseOrMakeNew(ipw.VBox):
         # but does no harm in the other cases (true of both lines below)
         self._item_widget.open_nested = True
 
-        self._item_widget.savebuttonbar.fns_onsave_add_action(self._save_confirmation())
-
-        # Link validation value to save button. The use of directional link
-        # is important to make sure the validation state controls
-        # the save button state and not the other way around.
-        ipw.dlink(
-            (self._item_widget.is_valid, "value"),
-            (self._item_widget.savebuttonbar.bn_save, "disabled"),
-            transform=lambda x: not x,
-        )
-
         # Set up some observers
 
+        # Respond to user clicking the save button
+        self._item_widget.savebuttonbar.fns_onsave_add_action(self._save_confirmation())
+
         # Respond to user interacting with a confirmation widget
         # Hide the save button bar so the user gets the confirmation instead
         self._confirm_edit.widget_to_hide = self._item_widget.savebuttonbar


Test patch:
diff --git a/stellarphot/settings/tests/test_custom_widgets.py b/stellarphot/settings/tests/test_custom_widgets.py
index 4606099..f37c1c5 100644
--- a/stellarphot/settings/tests/test_custom_widgets.py
+++ b/stellarphot/settings/tests/test_custom_widgets.py
@@ -267,6 +267,42 @@ class TestChooseOrMakeNew:
         # The edit button should now be displayed
         assert choose_or_make_new._edit_button.layout.display != "none"
 
+    def test_save_button_disabled_when_no_changes(self, tmp_path):
+        # Immediately after the edit button has been clicked, the save button should be
+        # disabled because no changes have been made yet.
+        #
+        # That should remain true even after the user makes a change, then saves it
+        # and then edits again but has not yet made any changes.
+        saved = SavedSettings(_testing_path=tmp_path)
+        camera = Camera(**TEST_CAMERA_VALUES)
+        saved.add_item(camera)
+
+        choose_or_make_new = ChooseOrMakeNew("camera", _testing_path=tmp_path)
+        # Simulate a click on the edit button...
+        choose_or_make_new._edit_button.click()
+
+        # The save button should be disabled
+        assert choose_or_make_new._item_widget.savebuttonbar.bn_save.disabled
+
+        # Edit a value in the camera so the save button is enabled
+        new_gain = 2 * TEST_CAMERA_VALUES["gain"]
+        choose_or_make_new._item_widget.di_widgets["gain"].value = str(new_gain)
+
+        # The save button should now be enabled
+        assert not choose_or_make_new._item_widget.savebuttonbar.bn_save.disabled
+
+        # Save the change
+        choose_or_make_new._item_widget.savebuttonbar.bn_save.click()
+
+        # Confirm the save
+        choose_or_make_new._confirm_edit._yes.click()
+
+        # Edit again...
+        choose_or_make_new._edit_button.click()
+
+        # The save button should be disabled
+        assert choose_or_make_new._item_widget.savebuttonbar.bn_save.disabled
+
 
 class TestConfirm:
     def test_initial_value(self):