From 73014964cbc440d4839eb63f34a61ccf4a99176b Mon Sep 17 00:00:00 2001
From: Lance Edgar <lance@edbob.org>
Date: Sun, 11 Aug 2024 12:41:22 -0500
Subject: [PATCH] feat: add basic Create support for CRUD master view

---
 src/wuttaweb/forms/base.py                    |  58 ++++++++++
 src/wuttaweb/grids/base.py                    |   6 +-
 src/wuttaweb/templates/base.mako              |   9 +-
 .../templates/forms/vue_template.mako         |  20 ++--
 src/wuttaweb/templates/master/create.mako     |   7 ++
 src/wuttaweb/views/master.py                  |  77 +++++++++++++-
 src/wuttaweb/views/settings.py                |  24 ++++-
 tests/forms/test_base.py                      |  33 ++++++
 tests/views/test_master.py                    | 100 +++++++++++++-----
 tests/views/test_settings.py                  |  18 ++++
 10 files changed, 307 insertions(+), 45 deletions(-)
 create mode 100644 src/wuttaweb/templates/master/create.mako

diff --git a/src/wuttaweb/forms/base.py b/src/wuttaweb/forms/base.py
index b3dd1e2..b22698b 100644
--- a/src/wuttaweb/forms/base.py
+++ b/src/wuttaweb/forms/base.py
@@ -159,6 +159,18 @@ class Form:
 
        See also :meth:`set_readonly()` and :meth:`is_readonly()`.
 
+    .. attribute:: required_fields
+
+       A dict of "required" field flags.  Keys are field names, and
+       values are boolean flags indicating whether the field is
+       required.
+
+       Depending on :attr:`schema`, some fields may be "(not)
+       required" by default.  However ``required_fields`` keeps track
+       of any "overrides" per field.
+
+       See also :meth:`set_required()` and :meth:`is_required()`.
+
     .. attribute:: action_url
 
        String URL to which the form should be submitted, if applicable.
@@ -256,6 +268,7 @@ class Form:
             model_instance=None,
             readonly=False,
             readonly_fields=[],
+            required_fields={},
             labels={},
             action_url=None,
             cancel_url=None,
@@ -275,6 +288,7 @@ class Form:
         self.schema = schema
         self.readonly = readonly
         self.readonly_fields = set(readonly_fields or [])
+        self.required_fields = required_fields or {}
         self.labels = labels or {}
         self.action_url = action_url
         self.cancel_url = cancel_url
@@ -413,6 +427,41 @@ class Form:
                 return True
         return False
 
+    def set_required(self, key, required=True):
+        """
+        Enable or disable the "required" flag for a given field.
+
+        When a field is marked required, a value must be provided
+        or else it fails validation.
+
+        In practice if a field is "not required" then a default
+        "empty" value is assumed, should the user not provide one.
+
+        See also :meth:`is_required()`; this is tracked via
+        :attr:`required_fields`.
+
+        :param key: String key (fieldname) for the field.
+
+        :param required: New required flag for the field.  Usually a
+           boolean, but may also be ``None`` to remove any flag and
+           revert to default behavior for the field.
+        """
+        self.required_fields[key] = required
+
+    def is_required(self, key):
+        """
+        Returns boolean indicating if the given field is marked as
+        required.
+
+        See also :meth:`set_required()`.
+
+        :param key: Field key/name as string.
+
+        :returns: Value for the flag from :attr:`required_fields` if
+           present; otherwise ``None``.
+        """
+        return self.required_fields.get(key, None)
+
     def set_label(self, key, label):
         """
         Set the label for given field name.
@@ -452,6 +501,15 @@ class Form:
                     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
diff --git a/src/wuttaweb/grids/base.py b/src/wuttaweb/grids/base.py
index 5cb4dab..0706395 100644
--- a/src/wuttaweb/grids/base.py
+++ b/src/wuttaweb/grids/base.py
@@ -362,7 +362,11 @@ class GridAction:
         Default logic returns the output from :meth:`render_icon()`
         and :meth:`render_label()`.
         """
-        return self.render_icon() + ' ' + self.render_label()
+        html = [
+            self.render_icon(),
+            self.render_label(),
+        ]
+        return HTML.literal(' ').join(html)
 
     def render_icon(self):
         """
diff --git a/src/wuttaweb/templates/base.mako b/src/wuttaweb/templates/base.mako
index 487867c..ef5a295 100644
--- a/src/wuttaweb/templates/base.mako
+++ b/src/wuttaweb/templates/base.mako
@@ -214,13 +214,20 @@
             <div class="level-left">
 
               ## Current Context
-              <div id="current-context" class="level-item">
+              <div id="current-context" class="level-item"
+                   style="display: flex; gap: 1.5rem;">
                 % if index_title:
                     % if index_url:
                         <h1 class="title">${h.link_to(index_title, index_url)}</h1>
                     % else:
                         <h1 class="title">${index_title}</h1>
                     % endif
+                    % if master and master.creatable and not master.creating:
+                        <wutta-button once type="is-primary"
+                                      tag="a" href="${url(f'{route_prefix}.create')}"
+                                      icon-left="plus"
+                                      label="Create New" />
+                    % endif
                 % endif
               </div>
 
diff --git a/src/wuttaweb/templates/forms/vue_template.mako b/src/wuttaweb/templates/forms/vue_template.mako
index 93d1f7d..47e9fc3 100644
--- a/src/wuttaweb/templates/forms/vue_template.mako
+++ b/src/wuttaweb/templates/forms/vue_template.mako
@@ -54,15 +54,19 @@
 
   let ${form.vue_component}Data = {
 
-      ## field model values
-      % for key in form:
-          % if key in dform:
-              model_${key}: ${form.get_vue_field_value(key)|n},
-          % endif
-      % endfor
+      % if not form.readonly:
+
+          ## field model values
+          % for key in form:
+              % if key in dform:
+                  model_${key}: ${form.get_vue_field_value(key)|n},
+              % endif
+          % endfor
+
+          % if form.auto_disable_submit:
+              formSubmitting: false,
+          % endif
 
-      % if form.auto_disable_submit:
-          formSubmitting: false,
       % endif
   }
 
diff --git a/src/wuttaweb/templates/master/create.mako b/src/wuttaweb/templates/master/create.mako
new file mode 100644
index 0000000..5aebc1a
--- /dev/null
+++ b/src/wuttaweb/templates/master/create.mako
@@ -0,0 +1,7 @@
+## -*- coding: utf-8; -*-
+<%inherit file="/master/form.mako" />
+
+<%def name="title()">New ${model_title}</%def>
+
+
+${parent.body()}
diff --git a/src/wuttaweb/views/master.py b/src/wuttaweb/views/master.py
index 1ac6a9f..70f47fd 100644
--- a/src/wuttaweb/views/master.py
+++ b/src/wuttaweb/views/master.py
@@ -168,6 +168,12 @@ class MasterView(View):
 
        This is optional; see also :meth:`index_get_grid_columns()`.
 
+    .. attribute:: creatable
+
+       Boolean indicating whether the view model supports "creating" -
+       i.e. it should have a :meth:`create()` view.  Default value is
+       ``True``.
+
     .. attribute:: viewable
 
        Boolean indicating whether the view model supports "viewing" -
@@ -206,6 +212,7 @@ class MasterView(View):
     # features
     listable = True
     has_grid = True
+    creatable = True
     viewable = True
     editable = True
     deletable = True
@@ -213,6 +220,7 @@ class MasterView(View):
 
     # current action
     listing = False
+    creating = False
     viewing = False
     editing = False
     deleting = False
@@ -335,6 +343,62 @@ class MasterView(View):
         """
         return []
 
+    ##############################
+    # create methods
+    ##############################
+
+    def create(self):
+        """
+        View to "create" a new model record.
+
+        This usually corresponds to a URL like ``/widgets/new``.
+
+        By default, this view is included only if :attr:`creatable` is
+        true.
+
+        The default "create" view logic will show a form with field
+        widgets, allowing user to submit new values which are then
+        persisted to the DB (assuming typical SQLAlchemy model).
+
+        Subclass normally should not override this method, but rather
+        one of the related methods which are called (in)directly by
+        this one:
+
+        * :meth:`make_model_form()`
+        * :meth:`configure_form()`
+        * :meth:`create_save_form()`
+        """
+        self.creating = True
+        form = self.make_model_form(cancel_url_fallback=self.get_index_url())
+
+        if form.validate():
+            obj = self.create_save_form(form)
+            return self.redirect(self.get_action_url('view', obj))
+
+        context = {
+            'form': form,
+        }
+        return self.render_to_response('create', context)
+
+    def create_save_form(self, form):
+        """
+        This method is responsible for "converting" the validated form
+        data to a model instance, and then "saving" the result,
+        e.g. to DB.  It is called by :meth:`create()`.
+
+        Subclass may override this, or any of the related methods
+        called by this one:
+
+        * :meth:`objectify()`
+        * :meth:`persist()`
+
+        :returns: Should return the resulting model instance, e.g. as
+           produced by :meth:`objectify()`.
+        """
+        obj = self.objectify(form)
+        self.persist(obj)
+        return obj
+
     ##############################
     # view methods
     ##############################
@@ -419,7 +483,7 @@ class MasterView(View):
         """
         This method is responsible for "converting" the validated form
         data to a model instance, and then "saving" the result,
-        e.g. to DB.
+        e.g. to DB.  It is called by :meth:`edit()`.
 
         Subclass may override this, or any of the related methods
         called by this one:
@@ -427,8 +491,8 @@ class MasterView(View):
         * :meth:`objectify()`
         * :meth:`persist()`
 
-        :returns: This should return the resulting model instance,
-           which was produced by :meth:`objectify()`.
+        :returns: Should return the resulting model instance, e.g. as
+           produced by :meth:`objectify()`.
         """
         obj = self.objectify(form)
         self.persist(obj)
@@ -1395,6 +1459,13 @@ class MasterView(View):
             config.add_view(cls, attr='index',
                             route_name=route_prefix)
 
+        # create
+        if cls.creatable:
+            config.add_route(f'{route_prefix}.create',
+                             f'{url_prefix}/new')
+            config.add_view(cls, attr='create',
+                            route_name=f'{route_prefix}.create')
+
         # view
         if cls.viewable:
             instance_url_prefix = cls.get_instance_url_prefix()
diff --git a/src/wuttaweb/views/settings.py b/src/wuttaweb/views/settings.py
index 2da0951..7d60ee2 100644
--- a/src/wuttaweb/views/settings.py
+++ b/src/wuttaweb/views/settings.py
@@ -26,8 +26,9 @@ Views for app settings
 
 from collections import OrderedDict
 
-from wuttjamaican.db.model import Setting
+import colander
 
+from wuttjamaican.db.model import Setting
 from wuttaweb.views import MasterView
 from wuttaweb.util import get_libver, get_liburl
 from wuttaweb.db import Session
@@ -48,6 +49,7 @@ class AppInfoView(MasterView):
     model_title_plural = "App Info"
     route_prefix = 'appinfo'
     has_grid = False
+    creatable = False
     viewable = False
     editable = False
     deletable = False
@@ -177,6 +179,10 @@ class SettingView(MasterView):
         """ """
         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,
         }
 
@@ -188,7 +194,10 @@ class SettingView(MasterView):
         name = self.request.matchdict['name']
         setting = session.query(model.Setting).get(name)
         if setting:
-            return self.normalize_setting(setting)
+            setting = self.normalize_setting(setting)
+            if setting['value'] is None:
+                setting['value'] = colander.null
+            return setting
 
         return self.notfound()
 
@@ -197,10 +206,19 @@ class SettingView(MasterView):
         """ """
         return setting['name']
 
+    # TODO: master should handle this
+    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):
         """ """
-        name = self.get_instance(session=session)['name']
+        if self.creating:
+            name = setting['name']
+        else:
+            name = self.request.matchdict['name']
         session = session or Session()
         self.app.save_setting(session, name, setting['value'])
 
diff --git a/tests/forms/test_base.py b/tests/forms/test_base.py
index cd0998f..e838e14 100644
--- a/tests/forms/test_base.py
+++ b/tests/forms/test_base.py
@@ -135,6 +135,19 @@ class TestForm(TestCase):
         self.assertIsNone(form.schema)
         self.assertRaises(NotImplementedError, form.get_schema)
 
+        # schema nodes are required by default
+        form = self.make_form(fields=['foo', 'bar'])
+        schema = form.get_schema()
+        self.assertIs(schema['foo'].missing, colander.required)
+        self.assertIs(schema['bar'].missing, colander.required)
+
+        # but fields can be marked *not* required
+        form = self.make_form(fields=['foo', 'bar'])
+        form.set_required('bar', False)
+        schema = form.get_schema()
+        self.assertIs(schema['foo'].missing, colander.required)
+        self.assertIsNone(schema['bar'].missing)
+
     def test_get_deform(self):
         schema = self.make_schema()
 
@@ -218,6 +231,26 @@ class TestForm(TestCase):
         self.assertFalse(form.is_readonly('foo'))
         self.assertTrue(form.is_readonly('bar'))
 
+    def test_required_fields(self):
+        form = self.make_form(fields=['foo', 'bar'])
+        self.assertEqual(form.required_fields, {})
+        self.assertIsNone(form.is_required('foo'))
+
+        form.set_required('foo')
+        self.assertEqual(form.required_fields, {'foo': True})
+        self.assertTrue(form.is_required('foo'))
+        self.assertIsNone(form.is_required('bar'))
+
+        form.set_required('bar')
+        self.assertEqual(form.required_fields, {'foo': True, 'bar': True})
+        self.assertTrue(form.is_required('foo'))
+        self.assertTrue(form.is_required('bar'))
+
+        form.set_required('foo', False)
+        self.assertEqual(form.required_fields, {'foo': False, 'bar': True})
+        self.assertFalse(form.is_required('foo'))
+        self.assertTrue(form.is_required('bar'))
+
     def test_render_vue_tag(self):
         schema = self.make_schema()
         form = self.make_form(schema=schema)
diff --git a/tests/views/test_master.py b/tests/views/test_master.py
index 10724a4..6106ab0 100644
--- a/tests/views/test_master.py
+++ b/tests/views/test_master.py
@@ -320,22 +320,22 @@ class TestMasterView(WebTestCase):
 
         # basic sanity check using /master/index.mako
         # (nb. it skips /widgets/index.mako since that doesn't exist)
-        master.MasterView.model_name = 'Widget'
-        view = master.MasterView(self.request)
-        response = view.render_to_response('index', {})
-        self.assertIsInstance(response, Response)
-        del master.MasterView.model_name
+        with patch.multiple(master.MasterView, create=True,
+                            model_name='Widget',
+                            creatable=False):
+            view = master.MasterView(self.request)
+            response = view.render_to_response('index', {})
+            self.assertIsInstance(response, Response)
 
         # basic sanity check using /appinfo/index.mako
-        master.MasterView.model_name = 'AppInfo'
-        master.MasterView.route_prefix = 'appinfo'
-        master.MasterView.url_prefix = '/appinfo'
-        view = master.MasterView(self.request)
-        response = view.render_to_response('index', {})
-        self.assertIsInstance(response, Response)
-        del master.MasterView.model_name
-        del master.MasterView.route_prefix
-        del master.MasterView.url_prefix
+        with patch.multiple(master.MasterView, create=True,
+                            model_name='AppInfo',
+                            route_prefix='appinfo',
+                            url_prefix='/appinfo',
+                            creatable=False):
+            view = master.MasterView(self.request)
+            response = view.render_to_response('index', {})
+            self.assertIsInstance(response, Response)
 
         # bad template name causes error
         master.MasterView.model_name = 'Widget'
@@ -372,6 +372,55 @@ class TestMasterView(WebTestCase):
         del master.MasterView.model_key
         del master.MasterView.grid_columns
 
+    def test_create(self):
+        model = self.app.model
+
+        # sanity/coverage check using /settings/new
+        with patch.multiple(master.MasterView, create=True,
+                            model_name='Setting',
+                            model_key='name',
+                            form_fields=['name', 'value']):
+            view = master.MasterView(self.request)
+
+            # no setting yet
+            self.assertIsNone(self.app.get_setting(self.session, 'foo.bar'))
+
+            # get the form page
+            response = view.create()
+            self.assertIsInstance(response, Response)
+            self.assertEqual(response.status_code, 200)
+            # self.assertIn('frazzle', response.text)
+            # nb. no error
+            self.assertNotIn('Required', response.text)
+
+            def persist(setting):
+                self.app.save_setting(self.session, setting['name'], setting['value'])
+                self.session.commit()
+
+            # post request to save setting
+            self.request.method = 'POST'
+            self.request.POST = {
+                'name': 'foo.bar',
+                'value': 'fraggle',
+            }
+            with patch.object(view, 'persist', new=persist):
+                response = view.create()
+            # nb. should get redirect back to view page
+            self.assertEqual(response.status_code, 302)
+            # setting should now be in DB
+            self.assertEqual(self.app.get_setting(self.session, 'foo.bar'), 'fraggle')
+
+            # try another post with invalid data (value is required)
+            self.request.method = 'POST'
+            self.request.POST = {}
+            with patch.object(view, 'persist', new=persist):
+                response = view.create()
+            # nb. should get a form with errors
+            self.assertEqual(response.status_code, 200)
+            self.assertIn('Required', response.text)
+            # setting did not change in DB
+            self.assertEqual(self.app.get_setting(self.session, 'foo.bar'), 'fraggle')
+
     def test_view(self):
 
         # sanity/coverage check using /settings/XXX
@@ -500,11 +549,6 @@ class TestMasterView(WebTestCase):
     def test_configure(self):
         model = self.app.model
 
-        # setup
-        master.MasterView.model_name = 'AppInfo'
-        master.MasterView.route_prefix = 'appinfo'
-        master.MasterView.template_prefix = '/appinfo'
-
         # mock settings
         settings = [
             {'name': 'wutta.app_title'},
@@ -516,11 +560,14 @@ class TestMasterView(WebTestCase):
         ]
 
         view = master.MasterView(self.request)
-        with patch.object(self.request, 'current_route_url',
-                          return_value='/appinfo/configure'):
-            with patch.object(master.MasterView, 'configure_get_simple_settings',
-                              return_value=settings):
-                with patch.object(master, 'Session', return_value=self.session):
+        with patch.object(self.request, 'current_route_url', return_value='/appinfo/configure'):
+            with patch.object(master, 'Session', return_value=self.session):
+                with patch.multiple(master.MasterView, create=True,
+                                    model_name='AppInfo',
+                                    route_prefix='appinfo',
+                                    template_prefix='/appinfo',
+                                    creatable=False,
+                                    configure_get_simple_settings=MagicMock(return_value=settings)):
 
                     # get the form page
                     response = view.configure()
@@ -558,8 +605,3 @@ class TestMasterView(WebTestCase):
                     # should now have 0 settings
                     count = self.session.query(model.Setting).count()
                     self.assertEqual(count, 0)
-
-        # teardown
-        del master.MasterView.model_name
-        del master.MasterView.route_prefix
-        del master.MasterView.template_prefix
diff --git a/tests/views/test_settings.py b/tests/views/test_settings.py
index 4712664..499cff0 100644
--- a/tests/views/test_settings.py
+++ b/tests/views/test_settings.py
@@ -1,5 +1,7 @@
 # -*- coding: utf-8; -*-
 
+from unittest.mock import patch
+
 from tests.views.utils import WebTestCase
 
 from pyramid.httpexceptions import HTTPNotFound
@@ -66,6 +68,14 @@ class TestSettingView(WebTestCase):
         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())
+        self.assertNotIn('value', form.required_fields)
+        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()
@@ -82,6 +92,14 @@ class TestSettingView(WebTestCase):
         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()