From fc01fa283a941bf60791921fdf7ed93a808f18b5 Mon Sep 17 00:00:00 2001
From: Lance Edgar <lance@edbob.org>
Date: Sun, 11 Aug 2024 16:52:13 -0500
Subject: [PATCH] feat: add basic support for SQLAlchemy model in master view

must more to be done for this yet, but basics are in place for the
Setting view
---
 src/wuttaweb/forms/base.py     | 102 ++++++++----
 src/wuttaweb/grids/base.py     |  45 +++++-
 src/wuttaweb/util.py           |  19 +++
 src/wuttaweb/views/master.py   | 273 +++++++++++++++++++++------------
 src/wuttaweb/views/settings.py |  78 +---------
 tests/forms/test_base.py       |  35 ++++-
 tests/grids/test_base.py       |  19 +++
 tests/test_util.py             |  12 ++
 tests/views/test_master.py     | 118 +++++++++++++-
 tests/views/test_settings.py   |  65 +-------
 10 files changed, 506 insertions(+), 260 deletions(-)

diff --git a/src/wuttaweb/forms/base.py b/src/wuttaweb/forms/base.py
index b22698b..b7ca13c 100644
--- a/src/wuttaweb/forms/base.py
+++ b/src/wuttaweb/forms/base.py
@@ -32,7 +32,7 @@ import deform
 from pyramid.renderers import render
 from webhelpers2.html import HTML
 
-from wuttaweb.util import get_form_data
+from wuttaweb.util import get_form_data, get_model_fields
 
 
 log = logging.getLogger(__name__)
@@ -126,9 +126,10 @@ class Form:
 
     .. attribute:: model_class
 
-       Optional "class" for the model.  If set, this usually would be
-       a SQLAlchemy mapped class.  This may be used instead of
-       specifying the :attr:`schema`.
+       Model class for the form, if applicable.  When set, this is
+       usually a SQLAlchemy mapped class.  This (or
+       :attr:`model_instance`) may be used instead of specifying the
+       :attr:`schema`.
 
     .. attribute:: model_instance
 
@@ -309,11 +310,12 @@ class Form:
 
         self.model_class = model_class
         self.model_instance = model_instance
+        if self.model_instance and not self.model_class:
+            self.model_class = type(self.model_instance)
 
-        if fields is not None:
+        fields = fields or self.get_fields()
+        if fields:
             self.set_fields(fields)
-        elif self.schema:
-            self.set_fields([f.name for f in self.schema])
         else:
             self.fields = None
 
@@ -485,6 +487,43 @@ class Form:
         """
         return self.labels.get(key, self.app.make_title(key))
 
+    def get_fields(self):
+        """
+        Returns the official list of field names for the form, or
+        ``None``.
+
+        If :attr:`fields` is set and non-empty, it is returned.
+
+        Or, if :attr:`schema` is set, the field list is derived
+        from that.
+
+        Or, if :attr:`model_class` is set, the field list is derived
+        from that, via :meth:`get_model_fields()`.
+
+        Otherwise ``None`` is returned.
+        """
+        if hasattr(self, 'fields') and self.fields:
+            return self.fields
+
+        if self.schema:
+            return [field.name for field in self.schema]
+
+        fields = self.get_model_fields()
+        if fields:
+            return fields
+
+    def get_model_fields(self, model_class=None):
+        """
+        This method is a shortcut which calls
+        :func:`~wuttaweb.util.get_model_fields()`.
+
+        :param model_class: Optional model class for which to return
+           fields.  If not set, the form's :attr:`model_class` is
+           assumed.
+        """
+        return get_model_fields(self.config,
+                                model_class=model_class or self.model_class)
+
     def get_schema(self):
         """
         Return the :class:`colander:colander.Schema` object for the
@@ -495,26 +534,28 @@ class Form:
         """
         if not self.schema:
 
-            if self.fields:
-                schema = colander.Schema()
-                for name in self.fields:
-                    schema.add(colander.SchemaNode(
-                        colander.String(),
-                        name=name))
-
-                # apply required flags
-                for key, required in self.required_fields.items():
-                    if key in schema:
-                        if required is False:
-                            # TODO: (why) should we not use colander.null here?
-                            #schema[key].missing = colander.null
-                            schema[key].missing = None
-
-                self.schema = schema
-
-            else: # no fields
+            # get fields
+            fields = self.get_fields()
+            if not fields:
                 raise NotImplementedError
 
+            # make basic schema
+            schema = colander.Schema()
+            for name in fields:
+                schema.add(colander.SchemaNode(
+                    colander.String(),
+                    name=name))
+
+            # apply required flags
+            for key, required in self.required_fields.items():
+                if key in schema:
+                    if required is False:
+                        # TODO: (why) should we not use colander.null here?
+                        #schema[key].missing = colander.null
+                        schema[key].missing = None
+
+            self.schema = schema
+
         return self.schema
 
     def get_deform(self):
@@ -523,11 +564,20 @@ class Form:
         generating it automatically if necessary.
         """
         if not hasattr(self, 'deform_form'):
+            model = self.app.model
             schema = self.get_schema()
             kwargs = {}
 
             if self.model_instance:
-                kwargs['appstruct'] = self.model_instance
+                if isinstance(self.model_instance, model.Base):
+                    kwargs['appstruct'] = dict(self.model_instance)
+                else:
+                    kwargs['appstruct'] = self.model_instance
+
+                # TODO: ugh why is this necessary?
+                for key, value in list(kwargs['appstruct'].items()):
+                    if value is None:
+                        kwargs['appstruct'][key] = colander.null
 
             form = deform.Form(schema, **kwargs)
             self.deform_form = form
diff --git a/src/wuttaweb/grids/base.py b/src/wuttaweb/grids/base.py
index 0706395..0008699 100644
--- a/src/wuttaweb/grids/base.py
+++ b/src/wuttaweb/grids/base.py
@@ -28,6 +28,7 @@ from pyramid.renderers import render
 from webhelpers2.html import HTML
 
 from wuttaweb.forms import FieldList
+from wuttaweb.util import get_model_fields
 
 
 class Grid:
@@ -52,13 +53,19 @@ class Grid:
        Presumably unique key for the grid; used to track per-grid
        sort/filter settings etc.
 
+    .. attribute:: model_class
+
+       Model class for the grid, if applicable.  When set, this is
+       usually a SQLAlchemy mapped class.  This may be used for
+       deriving the default :attr:`columns` among other things.
+
     .. attribute:: columns
 
        :class:`~wuttaweb.forms.base.FieldList` instance containing
        string column names for the grid.  Columns will appear in the
        same order as they are in this list.
 
-       See also :meth:`set_columns()`.
+       See also :meth:`set_columns()` and :meth:`get_columns()`.
 
     .. attribute:: data
 
@@ -88,6 +95,7 @@ class Grid:
     def __init__(
             self,
             request,
+            model_class=None,
             key=None,
             columns=None,
             data=None,
@@ -96,6 +104,7 @@ class Grid:
             vue_tagname='wutta-grid',
     ):
         self.request = request
+        self.model_class = model_class
         self.key = key
         self.data = data
         self.actions = actions or []
@@ -105,11 +114,43 @@ class Grid:
         self.config = self.request.wutta_config
         self.app = self.config.get_app()
 
-        if columns is not None:
+        columns = columns or self.get_columns()
+        if columns:
             self.set_columns(columns)
         else:
             self.columns = None
 
+    def get_columns(self):
+        """
+        Returns the official list of column names for the grid, or
+        ``None``.
+
+        If :attr:`columns` is set and non-empty, it is returned.
+
+        Or, if :attr:`model_class` is set, the field list is derived
+        from that, via :meth:`get_model_columns()`.
+
+        Otherwise ``None`` is returned.
+        """
+        if hasattr(self, 'columns') and self.columns:
+            return self.columns
+
+        columns = self.get_model_columns()
+        if columns:
+            return columns
+
+    def get_model_columns(self, model_class=None):
+        """
+        This method is a shortcut which calls
+        :func:`~wuttaweb.util.get_model_fields()`.
+
+        :param model_class: Optional model class for which to return
+           fields.  If not set, the grid's :attr:`model_class` is
+           assumed.
+        """
+        return get_model_fields(self.config,
+                                model_class=model_class or self.model_class)
+
     @property
     def vue_component(self):
         """
diff --git a/src/wuttaweb/util.py b/src/wuttaweb/util.py
index 6d1d5f2..4c09478 100644
--- a/src/wuttaweb/util.py
+++ b/src/wuttaweb/util.py
@@ -357,3 +357,22 @@ def render_csrf_token(request, name='_csrf'):
     """
     token = get_csrf_token(request)
     return HTML.tag('div', tags.hidden(name, value=token), style='display:none;')
+
+
+def get_model_fields(config, model_class=None):
+    """
+    Convenience function to return a list of field names for the given
+    model class.
+
+    This logic only supports SQLAlchemy mapped classes and will use
+    that to determine the field listing if applicable.  Otherwise this
+    returns ``None``.
+    """
+    if model_class:
+        import sqlalchemy as sa
+        app = config.get_app()
+        model = app.model
+        if model_class and issubclass(model_class, model.Base):
+            mapper = sa.inspect(model_class)
+            fields = list([prop.key for prop in mapper.iterate_properties])
+            return fields
diff --git a/src/wuttaweb/views/master.py b/src/wuttaweb/views/master.py
index 70f47fd..8fa667f 100644
--- a/src/wuttaweb/views/master.py
+++ b/src/wuttaweb/views/master.py
@@ -24,6 +24,9 @@
 Base Logic for Master Views
 """
 
+import sqlalchemy as sa
+from sqlalchemy import orm
+
 from pyramid.renderers import render_to_response
 
 from wuttaweb.views import View
@@ -166,7 +169,7 @@ class MasterView(View):
 
        List of columns for the :meth:`index()` view grid.
 
-       This is optional; see also :meth:`index_get_grid_columns()`.
+       This is optional; see also :meth:`get_grid_columns()`.
 
     .. attribute:: creatable
 
@@ -246,7 +249,7 @@ class MasterView(View):
 
         See also related methods, which are called by this one:
 
-        * :meth:`index_make_grid()`
+        * :meth:`make_model_grid()`
         """
         self.listing = True
 
@@ -255,94 +258,10 @@ class MasterView(View):
         }
 
         if self.has_grid:
-            context['grid'] = self.index_make_grid()
+            context['grid'] = self.make_model_grid()
 
         return self.render_to_response('index', context)
 
-    def index_make_grid(self, **kwargs):
-        """
-        Create and return a :class:`~wuttaweb.grids.base.Grid`
-        instance for use with the :meth:`index()` view.
-
-        See also related methods, which are called by this one:
-
-        * :meth:`get_grid_key()`
-        * :meth:`index_get_grid_columns()`
-        * :meth:`index_get_grid_data()`
-        * :meth:`configure_grid()`
-        """
-        if 'key' not in kwargs:
-            kwargs['key'] = self.get_grid_key()
-
-        if 'columns' not in kwargs:
-            kwargs['columns'] = self.index_get_grid_columns()
-
-        if 'data' not in kwargs:
-            kwargs['data'] = self.index_get_grid_data()
-
-        if 'actions' not in kwargs:
-            actions = []
-
-            # TODO: should split this off into index_get_grid_actions() ?
-
-            if self.viewable:
-                actions.append(self.make_grid_action('view', icon='eye',
-                                                     url=self.get_action_url_view))
-
-            if self.editable:
-                actions.append(self.make_grid_action('edit', icon='edit',
-                                                     url=self.get_action_url_edit))
-
-            if self.deletable:
-                actions.append(self.make_grid_action('delete', icon='trash',
-                                                     url=self.get_action_url_delete,
-                                                     link_class='has-text-danger'))
-
-            kwargs['actions'] = actions
-
-        grid = self.make_grid(**kwargs)
-        self.configure_grid(grid)
-        return grid
-
-    def index_get_grid_columns(self):
-        """
-        Returns the default list of grid column names, for the
-        :meth:`index()` view.
-
-        This is called by :meth:`index_make_grid()`; in the resulting
-        :class:`~wuttaweb.grids.base.Grid` instance, this becomes
-        :attr:`~wuttaweb.grids.base.Grid.columns`.
-
-        This method may return ``None``, in which case the grid may
-        (try to) generate its own default list.
-
-        Subclass may define :attr:`grid_columns` for simple cases, or
-        can override this method if needed.
-
-        Also note that :meth:`configure_grid()` may be used to further
-        modify the final column set, regardless of what this method
-        returns.  So a common pattern is to declare all "supported"
-        columns by setting :attr:`grid_columns` but then optionally
-        remove or replace some of those within
-        :meth:`configure_grid()`.
-        """
-        if hasattr(self, 'grid_columns'):
-            return self.grid_columns
-
-    def index_get_grid_data(self):
-        """
-        Returns the grid data for the :meth:`index()` view.
-
-        This is called by :meth:`index_make_grid()`; in the resulting
-        :class:`~wuttaweb.grids.base.Grid` instance, this becomes
-        :attr:`~wuttaweb.grids.base.Grid.data`.
-
-        As of now there is not yet a "sane" default for this method;
-        it simply returns an empty list.  Subclass should override as
-        needed.
-        """
-        return []
-
     ##############################
     # create methods
     ##############################
@@ -573,7 +492,8 @@ class MasterView(View):
 
         This method is called by :meth:`delete_save_form()`.
         """
-        raise NotImplementedError
+        session = self.app.get_session(obj)
+        session.delete(obj)
 
     ##############################
     # configure methods
@@ -966,11 +886,119 @@ class MasterView(View):
             route_prefix = self.get_route_prefix()
             return self.request.route_url(route_prefix, **kwargs)
 
+    def make_model_grid(self, session=None, **kwargs):
+        """
+        Create and return a :class:`~wuttaweb.grids.base.Grid`
+        instance for use with the :meth:`index()` view.
+
+        See also related methods, which are called by this one:
+
+        * :meth:`get_grid_key()`
+        * :meth:`get_grid_columns()`
+        * :meth:`get_grid_data()`
+        * :meth:`configure_grid()`
+        """
+        if 'key' not in kwargs:
+            kwargs['key'] = self.get_grid_key()
+
+        if 'model_class' not in kwargs:
+            model_class = self.get_model_class()
+            if model_class:
+                kwargs['model_class'] = model_class
+
+        if 'columns' not in kwargs:
+            kwargs['columns'] = self.get_grid_columns()
+
+        if 'data' not in kwargs:
+            kwargs['data'] = self.get_grid_data(session=session)
+
+        if 'actions' not in kwargs:
+            actions = []
+
+            # TODO: should split this off into index_get_grid_actions() ?
+
+            if self.viewable:
+                actions.append(self.make_grid_action('view', icon='eye',
+                                                     url=self.get_action_url_view))
+
+            if self.editable:
+                actions.append(self.make_grid_action('edit', icon='edit',
+                                                     url=self.get_action_url_edit))
+
+            if self.deletable:
+                actions.append(self.make_grid_action('delete', icon='trash',
+                                                     url=self.get_action_url_delete,
+                                                     link_class='has-text-danger'))
+
+            kwargs['actions'] = actions
+
+        grid = self.make_grid(**kwargs)
+        self.configure_grid(grid)
+        return grid
+
+    def get_grid_columns(self):
+        """
+        Returns the default list of grid column names, for the
+        :meth:`index()` view.
+
+        This is called by :meth:`make_model_grid()`; in the resulting
+        :class:`~wuttaweb.grids.base.Grid` instance, this becomes
+        :attr:`~wuttaweb.grids.base.Grid.columns`.
+
+        This method may return ``None``, in which case the grid may
+        (try to) generate its own default list.
+
+        Subclass may define :attr:`grid_columns` for simple cases, or
+        can override this method if needed.
+
+        Also note that :meth:`configure_grid()` may be used to further
+        modify the final column set, regardless of what this method
+        returns.  So a common pattern is to declare all "supported"
+        columns by setting :attr:`grid_columns` but then optionally
+        remove or replace some of those within
+        :meth:`configure_grid()`.
+        """
+        if hasattr(self, 'grid_columns'):
+            return self.grid_columns
+
+    def get_grid_data(self, session=None):
+        """
+        Returns the grid data for the :meth:`index()` view.
+
+        This is called by :meth:`make_model_grid()`; in the resulting
+        :class:`~wuttaweb.grids.base.Grid` instance, this becomes
+        :attr:`~wuttaweb.grids.base.Grid.data`.
+
+        Default logic will call :meth:`get_query()` and if successful,
+        return the list from ``query.all()``.  Otherwise returns an
+        empty list.  Subclass should override as needed.
+        """
+        query = self.get_query(session=session)
+        if query is not None:
+            return query.all()
+
+        return []
+
+    def get_query(self, session=None):
+        """
+        Returns the main SQLAlchemy query object for the
+        :meth:`index()` view.  This is called by
+        :meth:`get_grid_data()`.
+
+        Default logic for this method returns a "plain" query on the
+        :attr:`model_class` if that is defined; otherwise ``None``.
+        """
+        model = self.app.model
+        model_class = self.get_model_class()
+        if model_class and issubclass(model_class, model.Base):
+            session = session or Session()
+            return session.query(model_class)
+
     def configure_grid(self, grid):
         """
         Configure the grid for the :meth:`index()` view.
 
-        This is called by :meth:`index_make_grid()`.
+        This is called by :meth:`make_model_grid()`.
 
         There is no default logic here; subclass should override as
         needed.  The ``grid`` param will already be "complete" and
@@ -981,7 +1009,7 @@ class MasterView(View):
             grid.set_link(key)
             # print("set link:", key)
 
-    def get_instance(self):
+    def get_instance(self, session=None):
         """
         This should return the "current" model instance based on the
         request details (e.g. route kwargs).
@@ -992,6 +1020,27 @@ class MasterView(View):
         There is no "sane" default logic here; subclass *must*
         override or else a ``NotImplementedError`` is raised.
         """
+        model_class = self.get_model_class()
+        if model_class:
+            session = session or Session()
+
+            def filtr(query, model_key):
+                key = self.request.matchdict[model_key]
+                query = query.filter(getattr(self.model_class, model_key) == key)
+                return query
+
+            query = session.query(model_class)
+
+            for key in self.get_model_key():
+                query = filtr(query, key)
+
+            try:
+                return query.one()
+            except orm.exc.NoResultFound:
+                pass
+
+            raise self.notfound()
+
         raise NotImplementedError("you must define get_instance() method "
                                   f" for view class: {self.__class__}")
 
@@ -1075,10 +1124,18 @@ class MasterView(View):
         * :meth:`get_form_fields()`
         * :meth:`configure_form()`
         """
+        if 'model_class' not in kwargs:
+            model_class = self.get_model_class()
+            if model_class:
+                kwargs['model_class'] = model_class
+
         kwargs['model_instance'] = model_instance
 
-        if 'fields' not in kwargs:
-            kwargs['fields'] = self.get_form_fields()
+        # if 'fields' not in kwargs:
+        if not kwargs.get('fields'):
+            fields = self.get_form_fields()
+            if fields:
+                kwargs['fields'] = fields
 
         form = self.make_form(**kwargs)
         self.configure_form(form)
@@ -1146,9 +1203,25 @@ class MasterView(View):
 
         See also :meth:`edit_save_form()` which calls this method.
         """
+        model = self.app.model
+
+        model_class = self.get_model_class()
+        if model_class and issubclass(model_class, model.Base):
+
+            # update instance attrs for sqlalchemy model
+            if self.creating:
+                obj = model_class()
+            else:
+                obj = form.model_instance
+            data = form.validated
+            for key in form.fields:
+                if key in data:
+                    setattr(obj, key, data[key])
+            return obj
+
         return form.validated
 
-    def persist(self, obj):
+    def persist(self, obj, session=None):
         """
         If applicable, this method should persist ("save") the given
         object's data (e.g. to DB), creating or updating it as needed.
@@ -1165,6 +1238,13 @@ class MasterView(View):
 
         See also :meth:`edit_save_form()` which calls this method.
         """
+        model = self.app.model
+        model_class = self.get_model_class()
+        if model_class and issubclass(model_class, model.Base):
+
+            # add sqlalchemy model to session
+            session = session or Session()
+            session.add(obj)
 
     ##############################
     # class methods
@@ -1285,6 +1365,11 @@ class MasterView(View):
                 keys = [keys]
             return tuple(keys)
 
+        model_class = cls.get_model_class()
+        if model_class:
+            mapper = sa.inspect(model_class)
+            return tuple([column.key for column in mapper.primary_key])
+
         raise AttributeError(f"you must define model_key for view class: {cls}")
 
     @classmethod
@@ -1390,7 +1475,7 @@ class MasterView(View):
         grid in the :meth:`index()` view.  This key may also be used
         as the basis (key prefix) for secondary grids.
 
-        This is called from :meth:`index_make_grid()`; in the
+        This is called from :meth:`make_model_grid()`; in the
         resulting :class:`~wuttaweb.grids.base.Grid` instance, this
         becomes :attr:`~wuttaweb.grids.base.Grid.key`.
 
diff --git a/src/wuttaweb/views/settings.py b/src/wuttaweb/views/settings.py
index 7d60ee2..4b72081 100644
--- a/src/wuttaweb/views/settings.py
+++ b/src/wuttaweb/views/settings.py
@@ -149,85 +149,19 @@ class SettingView(MasterView):
     model_class = Setting
     model_title = "Raw Setting"
 
-    # TODO: this should be deduced by master
-    model_key = 'name'
-
-    # TODO: try removing these
-    grid_columns = [
-        'name',
-        'value',
-    ]
-    form_fields = list(grid_columns)
-
-    # TODO: should define query, let master handle the rest
-    def index_get_grid_data(self, session=None):
+    # TODO: master should handle this, possibly via configure_form()
+    def get_query(self, session=None):
         """ """
         model = self.app.model
+        query = super().get_query(session=session)
+        return query.order_by(model.Setting.name)
 
-        session = session or Session()
-        query = session.query(model.Setting)\
-                          .order_by(model.Setting.name)
-
-        settings = []
-        for setting in query:
-            settings.append(self.normalize_setting(setting))
-
-        return settings
-
-    # TODO: master should handle this (but not as dict)
-    def normalize_setting(self, setting):
-        """ """
-        return {
-            'name': setting.name,
-            # TODO: when viewing the record, 'None' is displayed for null
-            # field values.  not so if we return colander.null here, but
-            # then that causes other problems..
-            #'value': setting.value if setting.value is not None else colander.null,
-            'value': setting.value,
-        }
-
-    # TODO: master should handle this
-    def get_instance(self, session=None):
-        """ """
-        model = self.app.model
-        session = session or Session()
-        name = self.request.matchdict['name']
-        setting = session.query(model.Setting).get(name)
-        if setting:
-            setting = self.normalize_setting(setting)
-            if setting['value'] is None:
-                setting['value'] = colander.null
-            return setting
-
-        return self.notfound()
-
-    # TODO: master should handle this
-    def get_instance_title(self, setting):
-        """ """
-        return setting['name']
-
-    # TODO: master should handle this
+    # TODO: master should handle this (per column nullable)
     def configure_form(self, f):
+        """ """
         super().configure_form(f)
-
         f.set_required('value', False)
 
-    # TODO: master should handle this
-    def persist(self, setting, session=None):
-        """ """
-        if self.creating:
-            name = setting['name']
-        else:
-            name = self.request.matchdict['name']
-        session = session or Session()
-        self.app.save_setting(session, name, setting['value'])
-
-    # TODO: master should handle this
-    def delete_instance(self, obj, session=None):
-        """ """
-        session = session or Session()
-        self.app.delete_setting(session, obj['name'])
-
 
 def defaults(config, **kwargs):
     base = globals()
diff --git a/tests/forms/test_base.py b/tests/forms/test_base.py
index e838e14..0b579b6 100644
--- a/tests/forms/test_base.py
+++ b/tests/forms/test_base.py
@@ -49,6 +49,7 @@ class TestForm(TestCase):
         self.config = WuttaConfig(defaults={
             'wutta.web.menus.handler_spec': 'tests.utils:NullMenuHandler',
         })
+        self.app = self.config.get_app()
         self.request = testing.DummyRequest(wutta_config=self.config, use_oruga=False)
 
         self.pyramid_config = testing.setUp(request=self.request, settings={
@@ -115,6 +116,7 @@ class TestForm(TestCase):
         self.assertEqual(form.fields, ['baz'])
 
     def test_get_schema(self):
+        model = self.app.model
         form = self.make_form()
         self.assertIsNone(form.schema)
 
@@ -135,6 +137,24 @@ class TestForm(TestCase):
         self.assertIsNone(form.schema)
         self.assertRaises(NotImplementedError, form.get_schema)
 
+        # schema is auto-generated if model_class provided
+        form = self.make_form(model_class=model.Setting)
+        schema = form.get_schema()
+        self.assertEqual(len(schema.children), 2)
+        self.assertIn('name', schema)
+        self.assertIn('value', schema)
+
+        # schema is auto-generated if model_instance provided
+        form = self.make_form(model_instance=model.Setting(name='uhoh'))
+        self.assertEqual(form.fields, ['name', 'value'])
+        self.assertIsNone(form.schema)
+        # nb. force method to get new fields
+        del form.fields
+        schema = form.get_schema()
+        self.assertEqual(len(schema.children), 2)
+        self.assertIn('name', schema)
+        self.assertIn('value', schema)
+
         # schema nodes are required by default
         form = self.make_form(fields=['foo', 'bar'])
         schema = form.get_schema()
@@ -149,6 +169,7 @@ class TestForm(TestCase):
         self.assertIsNone(schema['bar'].missing)
 
     def test_get_deform(self):
+        model = self.app.model
         schema = self.make_schema()
 
         # basic
@@ -158,12 +179,24 @@ class TestForm(TestCase):
         self.assertIsInstance(dform, deform.Form)
         self.assertIs(form.deform_form, dform)
 
-        # with model instance / cstruct
+        # with model instance as dict
         myobj = {'foo': 'one', 'bar': 'two'}
         form = self.make_form(schema=schema, model_instance=myobj)
         dform = form.get_deform()
         self.assertEqual(dform.cstruct, myobj)
 
+        # with sqlalchemy model instance
+        myobj = model.Setting(name='foo', value='bar')
+        form = self.make_form(model_instance=myobj)
+        dform = form.get_deform()
+        self.assertEqual(dform.cstruct, {'name': 'foo', 'value': 'bar'})
+
+        # sqlalchemy instance with null value
+        myobj = model.Setting(name='foo', value=None)
+        form = self.make_form(model_instance=myobj)
+        dform = form.get_deform()
+        self.assertEqual(dform.cstruct, {'name': 'foo', 'value': colander.null})
+
     def test_get_cancel_url(self):
 
         # is referrer by default
diff --git a/tests/grids/test_base.py b/tests/grids/test_base.py
index 7d32c33..6dbbcd7 100644
--- a/tests/grids/test_base.py
+++ b/tests/grids/test_base.py
@@ -16,6 +16,7 @@ class TestGrid(TestCase):
         self.config = WuttaConfig(defaults={
             'wutta.web.menus.handler_spec': 'tests.utils:NullMenuHandler',
         })
+        self.app = self.config.get_app()
 
         self.request = testing.DummyRequest(wutta_config=self.config, use_oruga=False)
 
@@ -50,6 +51,24 @@ class TestGrid(TestCase):
         grid = self.make_grid()
         self.assertEqual(grid.vue_component, 'WuttaGrid')
 
+    def test_get_columns(self):
+        model = self.app.model
+
+        # empty
+        grid = self.make_grid()
+        self.assertIsNone(grid.columns)
+        self.assertIsNone(grid.get_columns())
+
+        # explicit
+        grid = self.make_grid(columns=['foo', 'bar'])
+        self.assertEqual(grid.columns, ['foo', 'bar'])
+        self.assertEqual(grid.get_columns(), ['foo', 'bar'])
+
+        # derived from model
+        grid = self.make_grid(model_class=model.Setting)
+        self.assertEqual(grid.columns, ['name', 'value'])
+        self.assertEqual(grid.get_columns(), ['name', 'value'])
+
     def test_linked_columns(self):
         grid = self.make_grid(columns=['foo', 'bar'])
         self.assertEqual(grid.linked_columns, [])
diff --git a/tests/test_util.py b/tests/test_util.py
index 4d779c3..b120f69 100644
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -403,6 +403,18 @@ class TestGetFormData(TestCase):
         self.assertEqual(data, {'foo2': 'baz'})
 
 
+class TestGetModelFields(TestCase):
+
+    def setUp(self):
+        self.config = WuttaConfig()
+        self.app = self.config.get_app()
+
+    def test_basic(self):
+        model = self.app.model
+        fields = util.get_model_fields(self.config, model.Setting)
+        self.assertEqual(fields, ['name', 'value'])
+
+
 class TestGetCsrfToken(TestCase):
 
     def setUp(self):
diff --git a/tests/views/test_master.py b/tests/views/test_master.py
index 6106ab0..55a8acb 100644
--- a/tests/views/test_master.py
+++ b/tests/views/test_master.py
@@ -6,7 +6,7 @@ from unittest.mock import MagicMock, patch
 
 from pyramid import testing
 from pyramid.response import Response
-from pyramid.httpexceptions import HTTPFound
+from pyramid.httpexceptions import HTTPFound, HTTPNotFound
 
 from wuttjamaican.conf import WuttaConfig
 from wuttaweb.views import master
@@ -348,10 +348,120 @@ class TestMasterView(WebTestCase):
         self.assertEqual(view.get_index_title(), "Wutta Widgets")
         del master.MasterView.model_title_plural
 
+    def test_make_model_grid(self):
+        model = self.app.model
+
+        # no model class
+        with patch.multiple(master.MasterView, create=True,
+                            model_name='Widget',
+                            model_key='uuid'):
+            view = master.MasterView(self.request)
+            grid = view.make_model_grid()
+            self.assertIsNone(grid.model_class)
+
+        # explicit model class
+        with patch.multiple(master.MasterView, create=True,
+                            model_class=model.Setting):
+            grid = view.make_model_grid(session=self.session)
+            self.assertIs(grid.model_class, model.Setting)
+
     def test_get_instance(self):
+        model = self.app.model
+        self.app.save_setting(self.session, 'foo', 'bar')
+        self.session.commit()
+        self.assertEqual(self.session.query(model.Setting).count(), 1)
+
+        # default not implemented
         view = master.MasterView(self.request)
         self.assertRaises(NotImplementedError, view.get_instance)
 
+        # fetch from DB if model class is known
+        with patch.multiple(master.MasterView, create=True,
+                            model_class=model.Setting):
+            view = master.MasterView(self.request)
+
+            # existing setting is returned
+            self.request.matchdict = {'name': 'foo'}
+            setting = view.get_instance(session=self.session)
+            self.assertIsInstance(setting, model.Setting)
+            self.assertEqual(setting.name, 'foo')
+            self.assertEqual(setting.value, 'bar')
+
+            # missing setting not found
+            self.request.matchdict = {'name': 'blarg'}
+            self.assertRaises(HTTPNotFound, view.get_instance, session=self.session)
+
+    def test_make_model_form(self):
+        model = self.app.model
+
+        # no model class
+        with patch.multiple(master.MasterView, create=True,
+                            model_name='Widget',
+                            model_key='uuid'):
+            view = master.MasterView(self.request)
+            form = view.make_model_form()
+            self.assertIsNone(form.model_class)
+
+        # explicit model class
+        with patch.multiple(master.MasterView, create=True,
+                            model_class=model.Setting):
+            form = view.make_model_form()
+            self.assertIs(form.model_class, model.Setting)
+
+    def test_objectify(self):
+        model = self.app.model
+        self.app.save_setting(self.session, 'foo', 'bar')
+        self.session.commit()
+        self.assertEqual(self.session.query(model.Setting).count(), 1)
+
+        # no model class
+        with patch.multiple(master.MasterView, create=True,
+                            model_name='Widget',
+                            model_key='uuid'):
+            view = master.MasterView(self.request)
+            form = view.make_model_form()
+            form.validated = {'name': 'first'}
+            obj = view.objectify(form)
+            self.assertIs(obj, form.validated)
+
+        # explicit model class (editing)
+        with patch.multiple(master.MasterView, create=True,
+                            model_class=model.Setting,
+                            editing=True):
+            form = view.make_model_form()
+            form.validated = {'name': 'foo', 'value': 'blarg'}
+            form.model_instance = self.session.query(model.Setting).one()
+            obj = view.objectify(form)
+            self.assertIsInstance(obj, model.Setting)
+            self.assertEqual(obj.name, 'foo')
+            self.assertEqual(obj.value, 'blarg')
+
+        # explicit model class (creating)
+        with patch.multiple(master.MasterView, create=True,
+                            model_class=model.Setting,
+                            creating=True):
+            form = view.make_model_form()
+            form.validated = {'name': 'another', 'value': 'whatever'}
+            obj = view.objectify(form)
+            self.assertIsInstance(obj, model.Setting)
+            self.assertEqual(obj.name, 'another')
+            self.assertEqual(obj.value, 'whatever')
+
+    def test_persist(self):
+        model = self.app.model
+        with patch.multiple(master.MasterView, create=True,
+                            model_class=model.Setting):
+            view = master.MasterView(self.request)
+
+            # new instance is persisted
+            setting = model.Setting(name='foo', value='bar')
+            self.assertEqual(self.session.query(model.Setting).count(), 0)
+            view.persist(setting, session=self.session)
+            self.session.commit()
+            setting = self.session.query(model.Setting).one()
+            self.assertEqual(setting.name, 'foo')
+            self.assertEqual(setting.value, 'bar')
+
     ##############################
     # view methods
     ##############################
@@ -366,7 +476,7 @@ class TestMasterView(WebTestCase):
         response = view.index()
         # then again with data, to include view action url
         data = [{'name': 'foo', 'value': 'bar'}]
-        with patch.object(view, 'index_get_grid_data', return_value=data):
+        with patch.object(view, 'get_grid_data', return_value=data):
             response = view.index()
         del master.MasterView.model_name
         del master.MasterView.model_key
@@ -544,7 +654,9 @@ class TestMasterView(WebTestCase):
                             model_class=model.Setting,
                             form_fields=['name', 'value']):
             view = master.MasterView(self.request)
-            self.assertRaises(NotImplementedError, view.delete_instance, setting)
+            view.delete_instance(setting)
+            self.session.commit()
+            self.assertEqual(self.session.query(model.Setting).count(), 0)
 
     def test_configure(self):
         model = self.app.model
diff --git a/tests/views/test_settings.py b/tests/views/test_settings.py
index 499cff0..47533fc 100644
--- a/tests/views/test_settings.py
+++ b/tests/views/test_settings.py
@@ -35,39 +35,19 @@ class TestSettingView(WebTestCase):
     def make_view(self):
         return settings.SettingView(self.request)
 
-    def test_index_get_grid_data(self):
+    def test_get_grid_data(self):
 
         # empty data by default
         view = self.make_view()
-        data = view.index_get_grid_data(session=self.session)
+        data = view.get_grid_data(session=self.session)
         self.assertEqual(len(data), 0)
 
         # unless we save some settings
         self.app.save_setting(self.session, 'foo', 'bar')
         self.session.commit()
-        data = view.index_get_grid_data(session=self.session)
+        data = view.get_grid_data(session=self.session)
         self.assertEqual(len(data), 1)
 
-    def test_get_instance(self):
-        view = self.make_view()
-        self.request.matchdict = {'name': 'foo'}
-
-        # setting not found
-        setting = view.get_instance(session=self.session)
-        self.assertIsInstance(setting, HTTPNotFound)
-
-        # setting is returned
-        self.app.save_setting(self.session, 'foo', 'bar')
-        self.session.commit()
-        setting = view.get_instance(session=self.session)
-        self.assertEqual(setting, {'name': 'foo', 'value': 'bar'})
-
-    def test_get_instance_title(self):
-        setting = {'name': 'foo', 'value': 'bar'}
-        view = self.make_view()
-        title = view.get_instance_title(setting)
-        self.assertEqual(title, 'foo')
-
     def test_configure_form(self):
         view = self.make_view()
         form = view.make_form(fields=view.get_form_fields())
@@ -75,42 +55,3 @@ class TestSettingView(WebTestCase):
         view.configure_form(form)
         self.assertIn('value', form.required_fields)
         self.assertFalse(form.required_fields['value'])
-
-    def test_persist(self):
-        model = self.app.model
-        view = self.make_view()
-
-        # setup
-        self.app.save_setting(self.session, 'foo', 'bar')
-        self.session.commit()
-        self.assertEqual(self.session.query(model.Setting).count(), 1)
-
-        # setting is updated
-        self.request.matchdict = {'name': 'foo'}
-        view.persist({'name': 'foo', 'value': 'frazzle'}, session=self.session)
-        self.session.commit()
-        self.assertEqual(self.session.query(model.Setting).count(), 1)
-        self.assertEqual(self.app.get_setting(self.session, 'foo'), 'frazzle')
-
-        # new setting is created
-        self.request.matchdict = {}
-        with patch.object(view, 'creating', new=True):
-            view.persist({'name': 'foo', 'value': 'frazzle'}, session=self.session)
-            self.session.commit()
-        self.assertEqual(self.session.query(model.Setting).count(), 1)
-        self.assertEqual(self.app.get_setting(self.session, 'foo'), 'frazzle')
-
-    def test_delete_instance(self):
-        model = self.app.model
-        view = self.make_view()
-
-        # setup
-        self.app.save_setting(self.session, 'foo', 'bar')
-        self.session.commit()
-        self.assertEqual(self.session.query(model.Setting).count(), 1)
-
-        # setting is deleted
-        self.request.matchdict = {'name': 'foo'}
-        view.delete_instance({'name': 'foo', 'value': 'frazzle'}, session=self.session)
-        self.session.commit()
-        self.assertEqual(self.session.query(model.Setting).count(), 0)