From 364199afd0cdeb48668b02d2ff3601e9e04ed690 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Sat, 22 Oct 2016 16:40:17 -0400 Subject: [PATCH] fix the case in the bug report #291 where an exception was raised instead of a sensible flash message being shown when an object was renamed to an existing other objects name using a propertysheet --- demos/blog/blog/resources.py | 2 +- substanced/schema/__init__.py | 44 ++++++++++++++++++++++++----------- substanced/schema/tests.py | 13 +++++++++++ 3 files changed, 44 insertions(+), 15 deletions(-) diff --git a/demos/blog/blog/resources.py b/demos/blog/blog/resources.py index 70b33924..575cba9d 100644 --- a/demos/blog/blog/resources.py +++ b/demos/blog/blog/resources.py @@ -25,7 +25,7 @@ def now_default(node, kw): class BlogEntrySchema(Schema): name = NameSchemaNode( - editing=lambda c, r: r.registry.content.istype(c, 'BlogEntry') + editing=lambda c, r: r.registry.content.istype(c, 'Blog Entry') ) title = colander.SchemaNode( colander.String(), diff --git a/substanced/schema/__init__.py b/substanced/schema/__init__.py index 304f756d..49f6b976 100644 --- a/substanced/schema/__init__.py +++ b/substanced/schema/__init__.py @@ -125,24 +125,22 @@ def i_am_editing(context, request): def validator(self, node, value): context = self.bindings['context'] request = self.bindings['request'] + # By default, we are adding, which means ``context`` is the parent + # object that we're being added to. But if ``editing`` turns out to be + # true, we're making edits to the object itself, not adding the object + # for the first time, which means the context is the object itself. editing = self.editing - # By default, we are adding, meaning that we're checking the name - # against the raw context which is assumed to be the parent - # object that we're being added to. - if editing is not None: - if callable(editing): - editing = editing(context, request) - if editing: - # However, if this is true, we are editing, not adding, which - # means the raw context is the object itself, so we need to - # walk up its parent chain to get the folder to call - # ``check_name`` against. - context = context.__parent__ + try: + if editing is not None: + if callable(editing): + editing = editing(context, request) if editing: - value = context.validate_name(value) + value = self.is_edit_ok(context, request, node, value) else: - value = context.check_name(value) + value = self.is_add_ok(context, request, node, value) + except colander.Invalid: + raise except Exception as e: raise colander.Invalid(node, e.args[0], value) if len(value) > self.max_len: @@ -152,6 +150,24 @@ def validator(self, node, value): value ) + def is_edit_ok(self, context, request, node, value): + parent = context.__parent__ + value = parent.validate_name(value) + named = parent.get(value) + if named is not None: + # if the parent already has an object named ``value`` that + # is not the object being edited + if named is not context: + raise colander.Invalid( + node, + 'Sorry an object with that name already exists', + value, + ) + return value + + def is_add_ok(self, context, request, node, value): + return context.check_name(value) + class PermissionsSchemaNode(colander.SchemaNode): """ A SchemaNode which represents a set of permissions; uses a widget which collects all permissions from the introspection system. Deserializes to a diff --git a/substanced/schema/tests.py b/substanced/schema/tests.py index ed6c7c32..d2d9c98e 100644 --- a/substanced/schema/tests.py +++ b/substanced/schema/tests.py @@ -177,6 +177,19 @@ def editing(context, request): node.bindings = bindings self.assertEqual(node.validator(node, 'abc'), None) + def test_it_editing_targetname_exists(self): + bindings = self._makeBindings() + parent = testing.DummyResource() + parent['abc'] = testing.DummyResource() + def check_name(value): return 'abc' + parent.validate_name = check_name + bindings['context'].__parent__ = parent + bindings['request'].registry.content = DummyContent(True) + node = self._makeOne(editing=True) + node.bindings = bindings + self.assertRaises(colander.Invalid, node.validator, node, 'abc') + + class TestPermissionsSchemaNode(unittest.TestCase): def setUp(self): testing.setUp()