CRUD forms

[jace]

CRUD views are a recurring pattern in HasGeek apps:

1. A model exists
2. A form exists and is used for both creating and editing.
3. A /new handler exists that presents a blank form using render_form and creates a new instance if the form validates, following which the user is redirected to the new instance.
4. A /edit handler exists that loads the object into the form and otherwise behaves exactly like /new.
5. A /deletehandler exists that asks for confirmation, then deletes the object and redirects to the parent object.

This pattern can be abstracted away so that (a) we can get past the pain of making views and (b) add new features in a centralised place instead of updating every view in every app. Sample usage:

from flask import g
from coaster.views import load_models
from baseframe.views import CrudView
from .. import app, lastuser
from ..models import MyModel
from ..forms import MyForm

my_model_view = CrudView(model=MyModel, form=MyForm, lastuser=lastuser, template='mymodel.html')

@app.route('/new', methods=['GET', 'POST'])
@my_model_view.new(login=True)
def my_model_new():
    return {
        'model_args': {'user': g.user},
        }

@app.route('<my_model>/edit', methods=['GET', 'POST'])
@my_model_view.edit(login=True)
@load_models(
    (MyModel, {'name': 'my_model'}, 'mymodel'),
    permission='edit')
def my_model_edit(mymodel):
    return {'instance': mymodel}

[jace]

Perhaps the decorators should be named create, read, update and delete to reflect the CRUD acronym.

[jace]

This is still too verbose. A full set of CRUD routes is still 20-30 lines of boilerplate. CrudView should be designed to produce boilerplate automatically, allowing the user to override only as necessary.

[jace]

As we've learnt from hasgeek/coaster#150 (StateManager), preserving flexibility for special behaviour is important, so that has to be the accounted for. We need these endpoints in order of priority, with these possible outcomes:

1. Read: Access Denied; Object
2. Update-Post: Access Denied; Form validation error; Edit success (HTTP 200 or 303)
3. Update-Get: Access Denied; Form(s)
4. Create-Post: Access Denied; Form validation error; New object (HTTP 201 or 303)
5. Create-Get: Access Denied; Form(s) (in case of a multi-step process)
6. Delete-Post: Access Denied; Soft Delete; Hard Delete (HTTP 200 or 303)
7. Delete-Get: Access Denied; Confirmation dialog

Notes:

• The distinction between HTTP 200/201 and 303 depends on how the request was made. A browser request must send the user to an appropriate destination to avoid disrupting user flow. A REST request may be confused between (a) a successful operation, and (b) the API endpoint itself being relocated.

• Returning a form (for create/update) and returning a form's validation errors are so similar in typical use that we use the same method for both. We could similarly merge the Create and Update code, using a blank object where one does not exist.

[jace]

The sample code above does not account for the is_url_for decorator introduced in hasgeek/coaster#77.

[jace]

Proper CRUD requires using the HTTP verbs GET, PUT, POST and DELETE instead of endpoint suffixes like /new, /edit and /delete. However, browsers can only send GET and POST requests, so we need a compromise:

• Use POST in lieu of PUT and DELETE but send an additional __method__ field in the form as an indicator to the backend.
• Support these named endpoints as a backup for browsers.

Mapped to the seven handlers above, this gets us (assuming a base URL of /parent/object):

Action	REST	Browser
Read	GET /parent/object	GET /parent/object
Update-Post	PUT /parent/object	POST /parent/object/edit
Update-Get	???	GET /parent/object/edit
Create-Post	POST /parent	POST /parent/new
Create-Get	???	GET /parent/new
Delete-Post	DELETE /parent/object	POST /parent/object/delete
Delete-Get	???	GET /parent/object/delete

[jace]

Flask provides a MethodView that makes the REST URLs easier to implement, but it's not good enough as we have nowhere to serve Update-Get, Create-Get and Delete-Get from.

[jace]

If we take the register_api example from Flask and make it a classmethod on the CrudView class, it'll go a long way towards solving this.

[jace]

CrudView needs a way to connect a URL to a loaded instance. load_models is showing its age, so we should consider something else better suited for CrudView. Consider the pending tickets:

• hasgeek/coaster#78: Support for joined loads, to avoid multiple database roundtrips
• hasgeek/coaster#35: Optional models, so that /new and /edit can be combined into a single handler
• hasgeek/coaster#17: OR queries, so that a model can be retrieved based on more than one field matching (for eg, name or suuid)
• hasgeek/coaster#14: Custom 404 handler that can hint the user about where to find what they're missing

[jace]

CrudView could be loader-agnostic by doing something like this:

def returns_loaded(parent, instance):
    return instance

class MyView(CrudView):
    model = MyModel
    parent = ParentModel
    form = MyForm

    loader = load_models(
        (ParentModel: {'name': 'parent'}, 'parent'),
        (MyModel, {'name': 'identity', 'parent': 'parent'}, 'instance')
        )(returns_loaded)

MyView.register_views(app, '/<parent>')

This sample code should be the entire boilerplate for registering CRUD views with default handlers.

[jace]

To ensure form.populate_obj(obj) does not cause damage, CrudView must always use a role access proxy (from hasgeek/coaster#109). This code should work transparently:

proxy = obj.access_for(g.user)
form.populate_obj(proxy)

Do note that g.user may become current_user with hasgeek/flask-lastuser#24, and the current_user symbol will become available for import in Baseframe when #68 is implemented.

[jace]

CrudView should also account for form context defined in #112.

[jace]

Another API proposal:

docview = ModelView(app, Document)
# docview is a helper for making views and offers the following methods:
# add_create, add_read, add_update, add_delete, add_view

docview.add_read(
    route='/<parent>/<document>',
    query=lambda parent, document: Document.query.join(DocParent).filter(Document.name == document, Document.parent.name == parent),
    template='document.html.jinja2',  # Passed to render_with, so this can be a dictionary as well
    json=True,  # Adds JSON output support
    permission='view',  # Optional (tested with `permission in document.permissions(current_auth.actor)`)
    roles={'all'},  # Optional (tested with `roles.intersection(document.current_roles)`)
    decorators=[],  # Optional decorators to be applied to the view (can be `requestargs`, `cors`, etc)
    )

add_read will accept either query or loader. Given a query, it will have .first_or_404() applied to it. add_view makes most parameters optional (except route) and will reuse whatever was given to add_read. add_update and add_delete will accept relative routes (without a leading /) such as edit and delete and will append them to add_read's route.

Problem: routes and route validation can be complicated, as this snippet from Hasjob shows:

https://github.com/hasgeek/hasjob/blob/26842786ea5ab4f6bdcb9ffd5c10b4b01a222d76/hasjob/views/listing.py#L34-L63

@app.route('/<domain>/<hashid>', methods=('GET', 'POST'), subdomain='<subdomain>')
@app.route('/<domain>/<hashid>', methods=('GET', 'POST'))
@app.route('/view/<hashid>', defaults={'domain': None}, methods=('GET', 'POST'), subdomain='<subdomain>')
@app.route('/view/<hashid>', defaults={'domain': None}, methods=('GET', 'POST'))
def jobdetail(domain, hashid):
    is_siteadmin = lastuser.has_permission('siteadmin')
    query = JobPost.fetch(hashid).options(
        db.subqueryload('locations'), db.subqueryload('taglinks'))
    post = query.first_or_404()

    # If we're on a board (that's not 'www') and this post isn't on this board,
    # redirect to (a) the first board it is on, or (b) on the root domain (which may
    # be the 'www' board, which is why we don't bother to redirect if we're currently
    # in the 'www' board)
    if g.board and g.board.not_root and post.link_to_board(g.board) is None:
        blink = post.postboards.first()
        if blink:
            return redirect(post.url_for(subdomain=blink.board.name, _external=True))
        else:
            return redirect(post.url_for(subdomain=None, _external=True))

    # If this post is past pending state and the domain doesn't match, redirect there
    if post.status not in POSTSTATUS.UNPUBLISHED and post.email_domain != domain:
        return redirect(post.url_for(), code=301)

    if post.status in POSTSTATUS.UNPUBLISHED:
        if not ((g.user and post.admin_is(g.user))):
            abort(403)
    if post.status in POSTSTATUS.GONE:
        abort(410)

Where do we accommodate (a) additional routes and (b) view validation?

1. The route parameter can accept a dictionary (as passed to app.route) or an iterable of dictionaries (multiple calls to app.route)
2. The view helper can offer decorators for various parts of a view:

docread = docview.add_read(…)

@docread.validate
def docread_validate(context, document):
    pass

@docread.prepare
def docread_prepare(context, document):
    # This is what happens if no `prepare` handler is provided
    return document.current_access()

(context will be docread itself, so it acts like a self parameter. This may be unnecessary though.)

Problem: The query/loader here can return only one item. This is normally not a problem because most objects have a .parent (or similar) object leading to the parent. A notable exception to this is the CommentSpace model in Funnel, which can be attached to multiple models and so has no way to know where it is attached except by querying all the models for a reference to itself. Effectively, the owner of a comment space cannot be discovered from the comment space. However, since admin permissions are inherited from the owner, we have the basis for how load_models and PermissionMixin work: each of the models in the chain is queried one at a time (and not in a joined load), checked for permissions, and these are then passed to the next item in the chain as the inherited parameter.

We haven't had this ambiguous parentage problem anywhere else (nodular even went out of the way to avoid it), but that doesn't mean it won't arise again.

[jace]

Here is yet another way to think about views. We make these assumptions:

1. A model represents the data, with some fundamental integrity checks (database column type, relationships with other models, etc)
2. A workflow wraps the model to represent how it can be manipulated. We've gone from a Docflow-based wrapper to integrating this directly into the model using RoleMixin and StateManager.
3. A form wraps the model/workflow to perform harsher data checks. Forms receive untrusted user input, so the checks must be thorough. Forms do not define the manner by which this data is actually received from the user.
4. A view binds the form and model. Views currently are standalone, so a model may have several views offering different ways to read or manipulate it. This needs consolidation similar to how model classes themselves integrate data descriptors with sanity check logic.

We've had an early attempt at view consolidation with Nodular's NodeView. It allowed view construction like this:

class MyNodeView(NodeView):
    @NodeView.route('/')
    def index(self):
        return u'index view'

    @NodeView.route('/edit', methods=['GET', 'POST'])
    @NodeView.route('/', methods=['PUT'])
    @NodeView.requires_permission('edit', 'siteadmin')
    def edit(self):
        return u'edit view'

    @NodeView.route('/delete', methods=['GET', 'POST'])
    @NodeView.route('/', methods=['DELETE'])
    @NodeView.requires_permission('delete', 'siteadmin')
    def delete(self):
        return u'delete view'

NodeView is heavy, creating a new router for the view. The app's router lands on /<path>/<to>/<object>/<path:everything> and the NodeView router then kicks in, applying this <path:everything> to the internal routes. We can make a lighter version of this that plugs directly into the main app's routes.

[jace]

CRUD views need a foundation. This is coming in hasgeek/coaster#167 with class-based views.