Fix use of _items as a normal trait in PreferencesHelper and PreferencesPage

apptools/preferences/preferences_helper.py

@@ -75,21 +75,21 @@ def _anytrait_changed(self, trait_name, old, new):
         if self.preferences is None:
             return
 
+        if self._is_preference_trait(trait_name):
+            self.preferences.set("%s.%s" % (self._get_path(), trait_name), new)
+
         # If the trait was a list or dict '_items' trait then just treat it as
         # if the entire list or dict was changed.
-        if trait_name.endswith('_items'):
+        elif trait_name.endswith('_items'):
             trait_name = trait_name[:-6]
             if self._is_preference_trait(trait_name):
                 self.preferences.set(
                     '%s.%s' % (self._get_path(), trait_name),
                     getattr(self, trait_name)
                 )
-                return
 
-        # If we were the one that set the trait (because the underlying
-        # preferences node changed) then do nothing.
-        if self._is_preference_trait(trait_name):
-            self.preferences.set("%s.%s" % (self._get_path(), trait_name), new)
+        # If the change refers to a trait defined on this class, then
+        # the trait is not a preference trait and we do nothing.
 
         return
 
@@ -205,4 +205,4 @@ def _is_preference_trait(self, trait_name):
         ):
             return False
 
-        return True
+        return trait_name in self.editable_traits()

apptools/preferences/tests/test_preferences_helper.py

@@ -319,6 +319,32 @@ class MyPreferencesHelper(PreferencesHelper):
                 str(helper.list_of_list_of_str)
             )
 
+    def test_sync_anytrait_items_not_event(self):
+        """ Test sychronizing trait with name *_items which is a normal trait
+        rather than an event trait for listening to list/dict/set mutation.
+        """
+
+        class MyPreferencesHelper(PreferencesHelper):
+            preferences_path = Str('my_section')
+
+            names_items = Str()
+
+        helper = MyPreferencesHelper(preferences=self.preferences)
+        helper.names_items = "Hello"
+
+        self.preferences.save(self.tmpfile)
+        new_preferences = Preferences()
+        new_preferences.load(self.tmpfile)
+
+        self.assertEqual(
+            sorted(new_preferences.keys("my_section")),
+            ["names_items"]
+        )
+        self.assertEqual(
+            new_preferences.get("my_section.names_items"),
+            str(helper.names_items),
+        )
+
     def test_no_preferences_path(self):
         """ no preferences path """
 

apptools/preferences/ui/preferences_page.py

@@ -73,16 +73,15 @@ def _anytrait_changed(self, trait_name, old, new):
 
         """
 
-        # If the trait was a list or dict '_items' trait then just treat it as
-        # if the entire list or dict was changed.
-        if trait_name.endswith("_items"):
+        if self._is_preference_trait(trait_name):
+            self._changed[trait_name] = new
+        elif trait_name.endswith("_items"):
+            # If the trait was a list or dict '_items' trait then just treat it
+            # as if the entire list or dict was changed.
             trait_name = trait_name[:-6]
             if self._is_preference_trait(trait_name):
                 self._changed[trait_name] = getattr(self, trait_name)
 
-        elif self._is_preference_trait(trait_name):
-            self._changed[trait_name] = new
-
         return
 
     # fixme: Pretty much duplicated in 'PreferencesHelper' (except for the
@@ -97,4 +96,4 @@ def _is_preference_trait(self, trait_name):
         ):
             return False
 
-        return True
+        return trait_name in self.editable_traits()

apptools/preferences/ui/tests/test_preferences_page.py

@@ -2,7 +2,11 @@
 
 import unittest
 
-from traits.api import Enum, List, Str
+from traits.api import (
+    Enum, List, Str,
+    pop_exception_handler,
+    push_exception_handler,
+)
 from traitsui.api import Group, Item, View
 
 from apptools.preferences.api import Preferences
@@ -12,6 +16,10 @@
 class TestPreferencesPage(unittest.TestCase):
     """ Non-GUI Tests for PreferencesPage."""
 
+    def setUp(self):
+        push_exception_handler(reraise_exceptions=True)
+        self.addCleanup(pop_exception_handler)
+
     def test_preferences_page_apply(self):
         """ Test applying the preferences """
 
@@ -75,3 +83,27 @@ class MyPreferencesPage(PreferencesPage):
 
         self.assertEqual(preferences.get("my_ref.pref.names"), str(["1", "2"]))
         self.assertEqual(preferences.keys("my_ref.pref"), ["names"])
+
+    def test_sync_anytrait_items_overload(self):
+        """ Test sychronizing trait with name *_items not to be mistaken
+        as the event trait for mutating list/dict/set
+        """
+
+        class MyPreferencesPage(PreferencesPage):
+            preferences_path = Str('my_section')
+
+            names_items = Str()
+
+        preferences = Preferences()
+        pref_page = MyPreferencesPage(preferences=preferences)
+        pref_page.names_items = "Hello"
+        pref_page.apply()
+
+        self.assertEqual(
+            sorted(preferences.keys("my_section")),
+            ["names_items"]
+        )
+        self.assertEqual(
+            preferences.get("my_section.names_items"),
+            "Hello",
+        )

docs/releases/upcoming/226.bugfix.rst

@@ -0,0 +1 @@
+Fix synchronizing preference trait with name *_items (#226)