Skip to content

Commit

Permalink
Fix premature writing of preferences (#583)
Browse files Browse the repository at this point in the history
This PR will eventually fix #582. The fix itself is straightforward, but
I first want to check that my regression test fails reliably in the
various CI workflows.

The new test also required apptools >= 5.3.0 to pass, so the EDM-based
tests won't pass until apptools 5.3.0 is available in EDS.
  • Loading branch information
mdickinson authored Aug 27, 2024
1 parent 62b00f4 commit 0f1b647
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 1 deletion.
10 changes: 9 additions & 1 deletion envisage/ui/tasks/preferences_pane.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,15 @@ def trait_context(self):
else:
raise ValueError("A preferences pane must have a model!")

self._model = self.model.clone_traits()
# Make sure that we don't clone the preferences trait, since that
# can lead to the preferences node being updated prematurely.
# xref: enthought/envisage#582
traits_to_clone = [
trait_name
for trait_name in self.model.copyable_trait_names()
if trait_name != "preferences"
]
self._model = self.model.clone_traits(traits_to_clone)
self._model.preferences = None
return {"object": self._model, "controller": self, "handler": self}

Expand Down
66 changes: 66 additions & 0 deletions envisage/ui/tasks/tests/test_preferences_pane.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# (C) Copyright 2007-2024 Enthought, Inc., Austin, TX
# All rights reserved.
#
# This software is provided without warranty under the terms of the BSD
# license included in LICENSE.txt and may be redistributed only under
# the conditions described in the aforementioned license. The license
# is also available online at http://www.enthought.com/licenses/BSD.txt
#
# Thanks for using Enthought open source!

"""Tests for the PreferencesPane."""

import unittest

from apptools.preferences.api import (
IPreferences,
Preferences,
PreferencesHelper,
ScopedPreferences,
)
from traits.api import Instance, Str
from traitsui.api import Item, View

from envisage.ui.tasks.api import PreferencesPane


class MyPreferencesHelper(PreferencesHelper):
#: Redeclare preferences to force trait copying order.
preferences = Instance(IPreferences)

#: The node that contains the preferences.
preferences_path = Str("app")

#: The user's favourite colour
color = Str()


class MyPreferencesPane(PreferencesPane):
model_factory = MyPreferencesHelper

view = View(Item("color"))


class TestPreferencesPane(unittest.TestCase):
def test_no_preference_changes_without_apply(self):
# Regression test for https://github.com/enthought/envisage/issues/582

# Given scoped preferences where the default preferences layer has
# a value for app.color ...
default_preferences = Preferences(name="default")
application_preferences = Preferences(name="application")
preferences = ScopedPreferences(
scopes=[application_preferences, default_preferences]
)
default_preferences.set("app.color", "red")

# When we create the preferences helper and pane, and trigger the
# trait_context method (which will usually be called as part of
# creating the TraitsUI UI).
helper = MyPreferencesHelper(preferences=preferences)
self.assertIsNone(application_preferences.get("app.color"))
pane = MyPreferencesPane(model=helper)
pane.trait_context()["object"]

# Then the application preferences should not have been changed.
self.assertIsNone(application_preferences.get("app.color"))

0 comments on commit 0f1b647

Please sign in to comment.