From 5b2d1dad53199fc760b4dcaf09846fcc329127cf Mon Sep 17 00:00:00 2001
From: Lance Edgar <lance@wuttaproject.org>
Date: Mon, 27 Jan 2025 17:07:42 -0600
Subject: [PATCH] feat: overhaul some User/Person form fields etc.

hoping this is more intuitive to use..
---
 src/wuttaweb/forms/schema.py  |  22 -----
 src/wuttaweb/forms/widgets.py |  58 ------------
 src/wuttaweb/views/people.py  |  67 +++++++++++---
 src/wuttaweb/views/users.py   |  68 ++++++++++----
 tests/forms/test_schema.py    |  14 ---
 tests/forms/test_widgets.py   |  47 +---------
 tests/views/test_people.py    |  64 ++++++++++---
 tests/views/test_users.py     | 166 +++++++++++++++++++++++++++-------
 8 files changed, 289 insertions(+), 217 deletions(-)

diff --git a/src/wuttaweb/forms/schema.py b/src/wuttaweb/forms/schema.py
index 77e443e..605b2c5 100644
--- a/src/wuttaweb/forms/schema.py
+++ b/src/wuttaweb/forms/schema.py
@@ -572,28 +572,6 @@ class RoleRefs(WuttaSet):
         return widgets.RoleRefsWidget(self.request, **kwargs)
 
 
-class UserRefs(WuttaSet):
-    """
-    Form schema type for the Role
-    :attr:`~wuttjamaican:wuttjamaican.db.model.auth.Role.users`
-    association proxy field.
-
-    This is a subclass of :class:`WuttaSet`.  It uses a ``set`` of
-    :class:`~wuttjamaican:wuttjamaican.db.model.auth.User` ``uuid``
-    values for underlying data format.
-    """
-
-    def widget_maker(self, **kwargs):
-        """
-        Constructs a default widget for the field.
-
-        :returns: Instance of
-           :class:`~wuttaweb.forms.widgets.UserRefsWidget`.
-        """
-        kwargs.setdefault('session', Session())
-        return widgets.UserRefsWidget(self.request, **kwargs)
-
-
 class Permissions(WuttaSet):
     """
     Form schema type for the Role
diff --git a/src/wuttaweb/forms/widgets.py b/src/wuttaweb/forms/widgets.py
index f87e8b1..03fe717 100644
--- a/src/wuttaweb/forms/widgets.py
+++ b/src/wuttaweb/forms/widgets.py
@@ -56,7 +56,6 @@ from webhelpers2.html import HTML
 from wuttjamaican.conf import parse_list
 
 from wuttaweb.db import Session
-from wuttaweb.grids import Grid
 
 
 class ObjectRefWidget(SelectWidget):
@@ -414,63 +413,6 @@ class RoleRefsWidget(WuttaCheckboxChoiceWidget):
         return super().serialize(field, cstruct, **kw)
 
 
-class UserRefsWidget(WuttaCheckboxChoiceWidget):
-    """
-    Widget for use with Role
-    :attr:`~wuttjamaican:wuttjamaican.db.model.auth.Role.users` field.
-    This is the default widget for the
-    :class:`~wuttaweb.forms.schema.UserRefs` type.
-
-    This is a subclass of :class:`WuttaCheckboxChoiceWidget`; however
-    it only supports readonly mode and does not use a template.
-    Rather, it generates and renders a
-    :class:`~wuttaweb.grids.base.Grid` showing the users list.
-    """
-
-    def serialize(self, field, cstruct, **kw):
-        """ """
-        readonly = kw.get('readonly', self.readonly)
-        if not readonly:
-            raise NotImplementedError("edit not allowed for this widget")
-
-        model = self.app.model
-        columns = ['username', 'active']
-
-        # generate data set for users
-        users = []
-        if cstruct:
-            for uuid in cstruct:
-                user = self.session.get(model.User, uuid)
-                if user:
-                    users.append(dict([(key, getattr(user, key))
-                                       for key in columns + ['uuid']]))
-
-        # do not render if no data
-        if not users:
-            return HTML.tag('span')
-
-        # grid
-        grid = Grid(self.request, key='roles.view.users',
-                    columns=columns, data=users)
-
-        # view action
-        if self.request.has_perm('users.view'):
-            url = lambda user, i: self.request.route_url('users.view', uuid=user['uuid'])
-            grid.add_action('view', icon='eye', url=url)
-            grid.set_link('person')
-            grid.set_link('username')
-
-        # edit action
-        if self.request.has_perm('users.edit'):
-            url = lambda user, i: self.request.route_url('users.edit', uuid=user['uuid'])
-            grid.add_action('edit', url=url)
-
-        # render as simple <b-table>
-        # nb. must indicate we are a part of this form
-        form = getattr(field.parent, 'wutta_form', None)
-        return grid.render_table_element(form)
-
-
 class PermissionsWidget(WuttaCheckboxChoiceWidget):
     """
     Widget for use with Role
diff --git a/src/wuttaweb/views/people.py b/src/wuttaweb/views/people.py
index 78cf931..c8ce87c 100644
--- a/src/wuttaweb/views/people.py
+++ b/src/wuttaweb/views/people.py
@@ -28,7 +28,6 @@ import sqlalchemy as sa
 
 from wuttjamaican.db.model import Person
 from wuttaweb.views import MasterView
-from wuttaweb.forms.schema import UserRefs
 
 
 class PersonView(MasterView):
@@ -62,6 +61,14 @@ class PersonView(MasterView):
         'full_name': {'active': True},
     }
 
+    form_fields = [
+        'full_name',
+        'first_name',
+        'middle_name',
+        'last_name',
+        'users',
+    ]
+
     def configure_grid(self, g):
         """ """
         super().configure_grid(g)
@@ -80,20 +87,54 @@ class PersonView(MasterView):
         super().configure_form(f)
         person = f.model_instance
 
-        # TODO: master should handle these? (nullable column)
-        f.set_required('first_name', False)
-        f.set_required('middle_name', False)
-        f.set_required('last_name', False)
+        # full_name
+        if self.creating or self.editing:
+            f.remove('full_name')
 
         # users
-        # nb. colanderalchemy wants to do some magic for the true
-        # 'users' relationship, so we use a different field name
-        f.remove('users')
-        if not (self.creating or self.editing):
-            f.append('_users')
-            f.set_readonly('_users')
-            f.set_node('_users', UserRefs(self.request))
-            f.set_default('_users', [u.uuid for u in person.users])
+        if self.viewing:
+            f.set_grid('users', self.make_users_grid(person))
+
+    def make_users_grid(self, person):
+        """
+        Make and return the grid for the Users field.
+
+        This grid is shown for the Users field when viewing a Person.
+
+        :returns: Fully configured :class:`~wuttaweb.grids.base.Grid`
+           instance.
+        """
+        model = self.app.model
+        route_prefix = self.get_route_prefix()
+
+        grid = self.make_grid(key=f'{route_prefix}.view.users',
+                              model_class=model.User,
+                              data=person.users,
+                              columns=[
+                                  'username',
+                                  'active',
+                              ])
+
+        if self.request.has_perm('users.view'):
+            url = lambda user, i: self.request.route_url('users.view', uuid=user.uuid)
+            grid.add_action('view', icon='eye', url=url)
+            grid.set_link('username')
+
+        if self.request.has_perm('users.edit'):
+            url = lambda user, i: self.request.route_url('users.edit', uuid=user.uuid)
+            grid.add_action('edit', url=url)
+
+        return grid
+
+    def objectify(self, form):
+        """ """
+        person = super().objectify(form)
+
+        # full_name
+        person.full_name = self.app.make_full_name(person.first_name,
+                                                   person.last_name)
+
+        return person
 
     def autocomplete_query(self, term):
         """ """
diff --git a/src/wuttaweb/views/users.py b/src/wuttaweb/views/users.py
index 5eac845..760152d 100644
--- a/src/wuttaweb/views/users.py
+++ b/src/wuttaweb/views/users.py
@@ -2,7 +2,7 @@
 ################################################################################
 #
 #  wuttaweb -- Web App for Wutta Framework
-#  Copyright © 2024 Lance Edgar
+#  Copyright © 2024-2025 Lance Edgar
 #
 #  This file is part of Wutta Framework.
 #
@@ -30,7 +30,6 @@ from wuttjamaican.db.model import User
 from wuttaweb.views import MasterView
 from wuttaweb.forms import widgets
 from wuttaweb.forms.schema import PersonRef, RoleRefs
-from wuttaweb.db import Session
 
 
 class UserView(MasterView):
@@ -61,6 +60,14 @@ class UserView(MasterView):
     }
     sort_defaults = 'username'
 
+    form_fields = [
+        'username',
+        'person',
+        'active',
+        'prevent_edit',
+        'roles',
+    ]
+
     def get_query(self, session=None):
         """ """
         query = super().get_query(session=session)
@@ -109,17 +116,24 @@ class UserView(MasterView):
         super().configure_form(f)
         user = f.model_instance
 
-        # never show these
-        f.remove('person_uuid',
-                 'role_refs')
-
-        # person
-        f.set_node('person', PersonRef(self.request, empty_option=True))
-        f.set_required('person', False)
-
         # username
         f.set_validator('username', self.unique_username)
 
+        # person
+        if self.creating or self.editing:
+            f.fields.insert_after('person', 'first_name')
+            f.set_required('first_name', False)
+            f.fields.insert_after('first_name', 'last_name')
+            f.set_required('last_name', False)
+            f.remove('person')
+            if self.editing:
+                person = user.person
+                if person:
+                    f.set_default('first_name', person.first_name)
+                    f.set_default('last_name', person.last_name)
+        else:
+            f.set_node('person', PersonRef(self.request))
+
         # password
         # nb. we must avoid 'password' as field name since
         # ColanderAlchemy wants to handle the raw/hashed value
@@ -140,7 +154,7 @@ class UserView(MasterView):
     def unique_username(self, node, value):
         """ """
         model = self.app.model
-        session = Session()
+        session = self.Session()
 
         query = session.query(model.User)\
                        .filter(model.User.username == value)
@@ -152,26 +166,48 @@ class UserView(MasterView):
         if query.count():
             node.raise_invalid("Username must be unique")
 
-    def objectify(self, form, session=None):
+    def objectify(self, form):
         """ """
+        model = self.app.model
+        auth = self.app.get_auth_handler()
         data = form.validated
 
         # normal logic first
         user = super().objectify(form)
 
+        # maybe update person name
+        if 'first_name' in form or 'last_name' in form:
+            first_name = data.get('first_name')
+            last_name = data.get('last_name')
+            if self.creating and (first_name or last_name):
+                user.person = auth.make_person(first_name=first_name, last_name=last_name)
+            elif self.editing:
+                if first_name or last_name:
+                    if user.person:
+                        person = user.person
+                        if 'first_name' in form:
+                            person.first_name = first_name
+                        if 'last_name' in form:
+                            person.last_name = last_name
+                        person.full_name = self.app.make_full_name(person.first_name,
+                                                                   person.last_name)
+                    else:
+                        user.person = auth.make_person(first_name=first_name, last_name=last_name)
+                elif user.person:
+                    user.person = None
+
         # maybe set user password
         if 'set_password' in form and data.get('set_password'):
-            auth = self.app.get_auth_handler()
             auth.set_user_password(user, data['set_password'])
 
         # update roles for user
         # TODO
         # if self.has_perm('edit_roles'):
-        self.update_roles(user, form, session=session)
+        self.update_roles(user, form)
 
         return user
 
-    def update_roles(self, user, form, session=None):
+    def update_roles(self, user, form):
         """ """
         # TODO
         # if not self.has_perm('edit_roles'):
@@ -181,7 +217,7 @@ class UserView(MasterView):
             return
 
         model = self.app.model
-        session = session or Session()
+        session = self.Session()
         auth = self.app.get_auth_handler()
 
         old_roles = set([role.uuid for role in user.roles])
diff --git a/tests/forms/test_schema.py b/tests/forms/test_schema.py
index 3187161..6914f61 100644
--- a/tests/forms/test_schema.py
+++ b/tests/forms/test_schema.py
@@ -377,20 +377,6 @@ class TestUserRef(WebTestCase):
             self.assertIn(f'/users/{user.uuid}', url)
 
 
-class TestUserRefs(DataTestCase):
-
-    def setUp(self):
-        self.setup_db()
-        self.request = testing.DummyRequest(wutta_config=self.config)
-
-    def test_widget_maker(self):
-        model = self.app.model
-        with patch.object(mod, 'Session', return_value=self.session):
-            typ = mod.UserRefs(self.request)
-            widget = typ.widget_maker()
-            self.assertIsInstance(widget, widgets.UserRefsWidget)
-
-
 class TestRoleRefs(DataTestCase):
 
     def setUp(self):
diff --git a/tests/forms/test_widgets.py b/tests/forms/test_widgets.py
index e571b88..cb42446 100644
--- a/tests/forms/test_widgets.py
+++ b/tests/forms/test_widgets.py
@@ -11,7 +11,7 @@ from pyramid import testing
 from wuttaweb import grids
 from wuttaweb.forms import widgets as mod
 from wuttaweb.forms import schema
-from wuttaweb.forms.schema import (FileDownload, PersonRef, RoleRefs, UserRefs, Permissions,
+from wuttaweb.forms.schema import (FileDownload, PersonRef, RoleRefs, Permissions,
                                    WuttaDateTime, EmailRecipients)
 from wuttaweb.testing import WebTestCase
 
@@ -300,51 +300,6 @@ class TestRoleRefsWidget(WebTestCase):
             self.assertIn(str(blokes.uuid.hex), html)
 
 
-class TestUserRefsWidget(WebTestCase):
-
-    def make_field(self, node, **kwargs):
-        # TODO: not sure why default renderer is in use even though
-        # pyramid_deform was included in setup?  but this works..
-        kwargs.setdefault('renderer', deform.Form.default_renderer)
-        return deform.Field(node, **kwargs)
-
-    def test_serialize(self):
-        model = self.app.model
-
-        # nb. we let the field construct the widget via our type
-        # node = colander.SchemaNode(UserRefs(self.request, session=self.session))
-        with patch.object(schema, 'Session', return_value=self.session):
-            node = colander.SchemaNode(UserRefs(self.request))
-            field = self.make_field(node)
-            widget = field.widget
-
-            # readonly is required
-            self.assertRaises(NotImplementedError, widget.serialize, field, set())
-            self.assertRaises(NotImplementedError, widget.serialize, field, set(), readonly=False)
-
-            # empty
-            html = widget.serialize(field, set(), readonly=True)
-            self.assertEqual(html, '<span></span>')
-
-            # with data, no actions
-            user = model.User(username='barney')
-            self.session.add(user)
-            self.session.commit()
-            html = widget.serialize(field, {user.uuid}, readonly=True)
-            self.assertIn('<b-table ', html)
-            self.assertNotIn('Actions', html)
-            self.assertNotIn('View', html)
-            self.assertNotIn('Edit', html)
-
-            # with view/edit actions
-            with patch.object(self.request, 'is_root', new=True):
-                html = widget.serialize(field, {user.uuid}, readonly=True)
-            self.assertIn('<b-table ', html)
-            self.assertIn('Actions', html)
-            self.assertIn('View', html)
-            self.assertIn('Edit', html)
-
-
 class TestPermissionsWidget(WebTestCase):
 
     def make_field(self, node, **kwargs):
diff --git a/tests/views/test_people.py b/tests/views/test_people.py
index 25abf9e..6f9e163 100644
--- a/tests/views/test_people.py
+++ b/tests/views/test_people.py
@@ -8,6 +8,8 @@ from pyramid.httpexceptions import HTTPNotFound
 
 from wuttaweb.views import people
 from wuttaweb.testing import WebTestCase
+from wuttaweb.forms.widgets import GridWidget
+from wuttaweb.grids import Grid
 
 
 class TestPersonView(WebTestCase):
@@ -34,27 +36,59 @@ class TestPersonView(WebTestCase):
     def test_configure_form(self):
         model = self.app.model
         view = self.make_view()
+
+        # full_name
         form = view.make_form(model_class=model.Person)
-
-        # required fields
+        self.assertIn('full_name', form)
         with patch.object(view, 'creating', new=True):
-            form.set_fields(form.get_model_fields())
-            self.assertEqual(form.required_fields, {})
             view.configure_form(form)
-            self.assertTrue(form.required_fields)
-            self.assertFalse(form.required_fields['middle_name'])
+            self.assertNotIn('full_name', form)
 
-        person = model.Person(full_name="Barney Rubble")
-        user = model.User(username='barney', person=person)
-        self.session.add(user)
-        self.session.commit()
-
-        # users field
+        # users
+        person = model.Person()
+        form = view.make_form(model_instance=person)
+        self.assertNotIn('users', form.widgets)
         with patch.object(view, 'viewing', new=True):
-            form = view.make_form(model_instance=person)
-            self.assertEqual(form.defaults, {})
             view.configure_form(form)
-            self.assertIn('_users', form.defaults)
+            self.assertIn('users', form.widgets)
+            self.assertIsInstance(form.widgets['users'], GridWidget)
+
+    def test_make_users_grid(self):
+        model = self.app.model
+        view = self.make_view()
+        person = model.Person(full_name="John Doe")
+
+        # basic
+        grid = view.make_users_grid(person)
+        self.assertIsInstance(grid, Grid)
+        self.assertFalse(grid.linked_columns)
+        self.assertFalse(grid.actions)
+
+        # view + edit actions
+        with patch.object(self.request, 'is_root', new=True):
+            grid = view.make_users_grid(person)
+            self.assertIsInstance(grid, Grid)
+            self.assertIn('username', grid.linked_columns)
+            self.assertEqual(len(grid.actions), 2)
+            self.assertEqual(grid.actions[0].key, 'view')
+            self.assertEqual(grid.actions[1].key, 'edit')
+
+    def test_objectify(self):
+        model = self.app.model
+        view = self.make_view()
+
+        # creating
+        form = view.make_model_form()
+        form.validated = {'first_name': 'Barney', 'last_name': 'Rubble'}
+        person = view.objectify(form)
+        self.assertEqual(person.full_name, 'Barney Rubble')
+
+        # editing
+        form = view.make_model_form(model_instance=person)
+        form.validated = {'first_name': 'Betty', 'last_name': 'Rubble'}
+        person2 = view.objectify(form)
+        self.assertEqual(person2.full_name, 'Betty Rubble')
+        self.assertIs(person2, person)
 
     def test_autocomplete_query(self):
         model = self.app.model
diff --git a/tests/views/test_users.py b/tests/views/test_users.py
index 1fa08f2..96f4404 100644
--- a/tests/views/test_users.py
+++ b/tests/views/test_users.py
@@ -64,17 +64,51 @@ class TestUserView(WebTestCase):
 
     def test_configure_form(self):
         model = self.app.model
-        barney = model.User(username='barney')
+        person = model.Person(first_name='Barney', last_name='Rubble', full_name='Barney Rubble')
+        barney = model.User(username='barney', person=person)
         self.session.add(barney)
         self.session.commit()
         view = self.make_view()
 
-        # person is *not* required
-        with patch.object(view, 'creating', new=True):
-            form = view.make_form(model_class=model.User)
-            self.assertIsNone(form.is_required('person'))
+        # person replaced with first/last name when creating or editing
+        with patch.object(view, 'viewing', new=True):
+            form = view.make_form(model_instance=barney)
+            self.assertIn('person', form)
+            self.assertNotIn('first_name', form)
+            self.assertNotIn('last_name', form)
             view.configure_form(form)
-            self.assertFalse(form.is_required('person'))
+            self.assertIn('person', form)
+            self.assertNotIn('first_name', form)
+            self.assertNotIn('last_name', form)
+        with patch.object(view, 'creating', new=True):
+            form = view.make_form(model_instance=barney)
+            self.assertIn('person', form)
+            self.assertNotIn('first_name', form)
+            self.assertNotIn('last_name', form)
+            view.configure_form(form)
+            self.assertNotIn('person', form)
+            self.assertIn('first_name', form)
+            self.assertIn('last_name', form)
+        with patch.object(view, 'editing', new=True):
+            form = view.make_form(model_instance=barney)
+            self.assertIn('person', form)
+            self.assertNotIn('first_name', form)
+            self.assertNotIn('last_name', form)
+            view.configure_form(form)
+            self.assertNotIn('person', form)
+            self.assertIn('first_name', form)
+            self.assertIn('last_name', form)
+
+        # first/last name have default values when editing
+        with patch.object(view, 'editing', new=True):
+            form = view.make_form(model_instance=barney)
+            self.assertNotIn('first_name', form.defaults)
+            self.assertNotIn('last_name', form.defaults)
+            view.configure_form(form)
+            self.assertIn('first_name', form.defaults)
+            self.assertEqual(form.defaults['first_name'], 'Barney')
+            self.assertIn('last_name', form.defaults)
+            self.assertEqual(form.defaults['last_name'], 'Rubble')
 
         # password removed (always, for now)
         with patch.object(view, 'viewing', new=True):
@@ -96,7 +130,7 @@ class TestUserView(WebTestCase):
         self.session.add(user)
         self.session.commit()
 
-        with patch.object(mod, 'Session', return_value=self.session):
+        with patch.object(view, 'Session', return_value=self.session):
 
             # invalid if same username in data
             node = colander.SchemaNode(colander.String(), name='username')
@@ -111,6 +145,8 @@ class TestUserView(WebTestCase):
     def test_objectify(self):
         model = self.app.model
         auth = self.app.get_auth_handler()
+        view = self.make_view()
+
         blokes = model.Role(name="Blokes")
         self.session.add(blokes)
         others = model.Role(name="Others")
@@ -120,30 +156,90 @@ class TestUserView(WebTestCase):
         barney.roles.append(blokes)
         self.session.add(barney)
         self.session.commit()
-        view = self.make_view()
-        view.editing = True
-        self.request.matchdict = {'uuid': barney.uuid}
 
-        # sanity check, user is just in 'blokes' role
-        self.session.refresh(barney)
-        self.assertEqual(len(barney.roles), 1)
-        self.assertEqual(barney.roles[0].name, "Blokes")
+        with patch.object(self.request, 'matchdict', new={'uuid': barney.uuid}):
+            with patch.object(view, 'editing', new=True):
 
-        # form can update user password
-        self.assertTrue(auth.check_user_password(barney, 'testpass'))
-        form = view.make_model_form(model_instance=barney)
-        form.validated = {'username': 'barney', 'set_password': 'testpass2'}
-        user = view.objectify(form, session=self.session)
-        self.assertIs(user, barney)
-        self.assertTrue(auth.check_user_password(barney, 'testpass2'))
+                # sanity check, user is just in 'blokes' role
+                self.session.refresh(barney)
+                self.assertEqual(len(barney.roles), 1)
+                self.assertEqual(barney.roles[0].name, "Blokes")
 
-        # form can update user roles
-        form = view.make_model_form(model_instance=barney)
-        form.validated = {'username': 'barney', 'roles': {others.uuid}}
-        user = view.objectify(form, session=self.session)
-        self.assertIs(user, barney)
-        self.assertEqual(len(user.roles), 1)
-        self.assertEqual(user.roles[0].name, "Others")
+                # form can update user password
+                self.assertTrue(auth.check_user_password(barney, 'testpass'))
+                form = view.make_model_form(model_instance=barney)
+                form.validated = {'username': 'barney', 'set_password': 'testpass2'}
+                with patch.object(view, 'Session', return_value=self.session):
+                    user = view.objectify(form)
+                self.assertIs(user, barney)
+                self.assertTrue(auth.check_user_password(barney, 'testpass2'))
+
+                # form can update user roles
+                form = view.make_model_form(model_instance=barney)
+                form.validated = {'username': 'barney', 'roles': {others.uuid}}
+                with patch.object(view, 'Session', return_value=self.session):
+                    user = view.objectify(form)
+                self.assertIs(user, barney)
+                self.assertEqual(len(user.roles), 1)
+                self.assertEqual(user.roles[0].name, "Others")
+
+                # person is auto-created
+                self.assertIsNone(barney.person)
+                form = view.make_model_form(model_instance=barney)
+                form.validated = {'username': 'barney', 'first_name': 'Barney', 'last_name': 'Rubble'}
+                with patch.object(view, 'Session', return_value=self.session):
+                    user = view.objectify(form)
+                self.assertIsNotNone(barney.person)
+                self.assertEqual(barney.person.first_name, 'Barney')
+                self.assertEqual(barney.person.last_name, 'Rubble')
+                self.assertEqual(barney.person.full_name, 'Barney Rubble')
+
+                # person is auto-removed
+                self.assertIsNotNone(barney.person)
+                form = view.make_model_form(model_instance=barney)
+                form.validated = {'username': 'barney', 'first_name': '', 'last_name': ''}
+                with patch.object(view, 'Session', return_value=self.session):
+                    user = view.objectify(form)
+                self.assertIsNone(barney.person)
+
+                # nb. re-attach the person
+                barney.person = self.session.query(model.Person).one()
+
+                # person name is updated
+                self.assertEqual(barney.person.first_name, 'Barney')
+                self.assertEqual(barney.person.last_name, 'Rubble')
+                self.assertEqual(barney.person.full_name, 'Barney Rubble')
+                form = view.make_model_form(model_instance=barney)
+                form.validated = {'username': 'barney', 'first_name': 'Fred', 'last_name': 'Flintstone'}
+                with patch.object(view, 'Session', return_value=self.session):
+                    user = view.objectify(form)
+                self.assertIsNotNone(barney.person)
+                self.assertEqual(barney.person.first_name, 'Fred')
+                self.assertEqual(barney.person.last_name, 'Flintstone')
+                self.assertEqual(barney.person.full_name, 'Fred Flintstone')
+
+        with patch.object(view, 'creating', new=True):
+
+            # person is auto-created when making new user
+            form = view.make_model_form()
+            form.validated = {'username': 'betty', 'first_name': 'Betty', 'last_name': 'Boop'}
+            with patch.object(view, 'Session', return_value=self.session):
+                user = view.objectify(form)
+            self.assertIsNotNone(user.person)
+            self.assertEqual(user.person.first_name, 'Betty')
+            self.assertEqual(user.person.last_name, 'Boop')
+            self.assertEqual(user.person.full_name, 'Betty Boop')
+
+            # nb. keep ref to last user
+            last_user = user
+
+            # person is *not* auto-created if no name provided
+            form = view.make_model_form()
+            form.validated = {'username': 'betty', 'first_name': '', 'last_name': ''}
+            with patch.object(view, 'Session', return_value=self.session):
+                user = view.objectify(form)
+            self.assertIsNone(user.person)
+            self.assertIsNot(user, last_user)
 
     def test_update_roles(self):
         model = self.app.model
@@ -166,7 +262,7 @@ class TestUserView(WebTestCase):
         # no error if data is missing roles
         form = view.make_model_form(model_instance=barney)
         form.validated = {'username': 'barneyx'}
-        user = view.objectify(form, session=self.session)
+        user = view.objectify(form)
         self.assertIs(user, barney)
         self.assertEqual(barney.username, 'barneyx')
 
@@ -182,7 +278,8 @@ class TestUserView(WebTestCase):
         form = view.make_model_form(model_instance=barney)
         form.validated = {'username': 'barney',
                           'roles': {admin.uuid, authed.uuid, anon.uuid, others.uuid}}
-        user = view.objectify(form, session=self.session)
+        with patch.object(view, 'Session', return_value=self.session):
+            user = view.objectify(form)
         self.assertIs(user, barney)
         self.assertEqual(len(user.roles), 1)
         self.assertEqual(user.roles[0].name, "Others")
@@ -194,7 +291,8 @@ class TestUserView(WebTestCase):
         form = view.make_model_form(model_instance=barney)
         form.validated = {'username': 'barney',
                           'roles': {admin.uuid, blokes.uuid, others.uuid}}
-        user = view.objectify(form, session=self.session)
+        with patch.object(view, 'Session', return_value=self.session):
+            user = view.objectify(form)
         self.assertIs(user, barney)
         self.assertEqual(len(user.roles), 3)
         role_uuids = set([role.uuid for role in user.roles])
@@ -205,7 +303,8 @@ class TestUserView(WebTestCase):
         form = view.make_model_form(model_instance=barney)
         form.validated = {'username': 'barney',
                           'roles': {blokes.uuid, others.uuid}}
-        user = view.objectify(form, session=self.session)
+        with patch.object(view, 'Session', return_value=self.session):
+            user = view.objectify(form)
         self.assertIs(user, barney)
         self.assertEqual(len(user.roles), 3)
 
@@ -214,6 +313,7 @@ class TestUserView(WebTestCase):
         form = view.make_model_form(model_instance=barney)
         form.validated = {'username': 'barney',
                           'roles': {blokes.uuid, others.uuid}}
-        user = view.objectify(form, session=self.session)
+        with patch.object(view, 'Session', return_value=self.session):
+            user = view.objectify(form)
         self.assertIs(user, barney)
         self.assertEqual(len(user.roles), 2)