From cdddf63302478c373378aec625fe49cb9b7d90c1 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 00:10:41 +0530
Subject: [PATCH 01/26] Rename view tests to be easier to locate

---
 tests/{test_loadmodels.py => test_views_loadmodels.py}  | 0
 tests/{test_render_with.py => test_views_renderwith.py} | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename tests/{test_loadmodels.py => test_views_loadmodels.py} (100%)
 rename tests/{test_render_with.py => test_views_renderwith.py} (100%)

diff --git a/tests/test_loadmodels.py b/tests/test_views_loadmodels.py
similarity index 100%
rename from tests/test_loadmodels.py
rename to tests/test_views_loadmodels.py
diff --git a/tests/test_render_with.py b/tests/test_views_renderwith.py
similarity index 100%
rename from tests/test_render_with.py
rename to tests/test_views_renderwith.py

From 822cf72f578a0fe5d39f89add316917c607d89bd Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 00:47:37 +0530
Subject: [PATCH 02/26] render_with tweaks

* Remove non-standard JSON mime types
* Massage view output into a dictionary if not already one
* Accept render_with(json=True) with no template specified
---
 coaster/views/decorators.py | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/coaster/views/decorators.py b/coaster/views/decorators.py
index aef38145..6a002535 100644
--- a/coaster/views/decorators.py
+++ b/coaster/views/decorators.py
@@ -18,7 +18,7 @@
 from flask import (abort, current_app, g, jsonify, make_response, redirect, render_template,
     request, Response, url_for)
 from ..auth import current_auth, add_auth_attribute
-from .misc import jsonp as render_jsonp
+from .misc import jsonp
 
 __all__ = [
     'RequestTypeError', 'RequestValueError',
@@ -322,7 +322,6 @@ def decorated_function(**kw):
     return inner
 
 
-
 def _best_mimetype_match(available_list, accept_mimetypes, default=None):
     for use_mimetype, quality in accept_mimetypes:
         for mimetype in available_list:
@@ -331,7 +330,21 @@ def _best_mimetype_match(available_list, accept_mimetypes, default=None):
     return default
 
 
-def render_with(template, json=False, jsonp=False):
+def dict_jsonify(param):
+    """Convert the parameter into a dictionary before calling jsonify, if it's not already one"""
+    if not isinstance(param, dict):
+        param = dict(param)
+    return jsonify(param)
+
+
+def dict_jsonp(param):
+    """Convert the parameter into a dictionary before calling jsonp, if it's not already one"""
+    if not isinstance(param, dict):
+        param = dict(param)
+    return jsonp(param)
+
+
+def render_with(template=None, json=False, jsonp=False):
     """
     Decorator to render the wrapped function with the given template (or dictionary
     of mimetype keys to templates, where the template is a string name of a template
@@ -392,15 +405,12 @@ def myview():
     """
     if jsonp:
         templates = {
-            'application/json': render_jsonp,
-            'text/json': render_jsonp,
-            'text/x-json': render_jsonp,
+            'application/json': dict_jsonp,
+            'application/javascript': dict_jsonp,
             }
     elif json:
         templates = {
-            'application/json': jsonify,
-            'text/json': jsonify,
-            'text/x-json': jsonify,
+            'application/json': dict_jsonify,
             }
     else:
         templates = {}
@@ -408,6 +418,8 @@ def myview():
         templates['text/html'] = template
     elif isinstance(template, dict):
         templates.update(template)
+    elif template is None and (json or jsonp):
+        pass
     else:  # pragma: no cover
         raise ValueError("Expected string or dict for template")
 
@@ -415,7 +427,7 @@ def myview():
     if '*/*' not in templates:
         templates['*/*'] = six.text_type
         default_mimetype = 'text/plain'
-        for mimetype in ('text/html', 'text/plain', 'application/json', 'text/json', 'text/x-json'):
+        for mimetype in ('text/html', 'text/plain', 'application/json'):
             if mimetype in templates:
                 templates['*/*'] = templates[mimetype]
                 default_mimetype = mimetype  # Remember which mimetype's handler is serving for */*

From 25ffb04ca1b383fae503dd3b96b3ecc1beef333c Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 01:23:30 +0530
Subject: [PATCH 03/26] Introducing class-based views

---
 CHANGES.rst                   |   2 +
 coaster/views/__init__.py     |   1 +
 coaster/views/classview.py    | 193 ++++++++++++++++++++++++++++++++++
 coaster/views/decorators.py   |  10 +-
 docs/conf.py                  |   4 +-
 docs/views/classview.rst      |   2 +
 docs/views/index.rst          |   1 +
 tests/test_views_classview.py | 108 +++++++++++++++++++
 8 files changed, 315 insertions(+), 6 deletions(-)
 create mode 100644 coaster/views/classview.py
 create mode 100644 docs/views/classview.rst
 create mode 100644 tests/test_views_classview.py

diff --git a/CHANGES.rst b/CHANGES.rst
index 84e21f06..0314906a 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -17,6 +17,8 @@
   ``coaster.views.requestargs``
 * New: ``coaster.auth`` module with a ``current_auth`` proxy that provides
   a standardised API for login managers to use
+* New: ``coaster.views.classview`` provides a new take on organising views
+  into a class
 
 
 0.6.0
diff --git a/coaster/views/__init__.py b/coaster/views/__init__.py
index 8b241dc8..820cc082 100644
--- a/coaster/views/__init__.py
+++ b/coaster/views/__init__.py
@@ -10,3 +10,4 @@
 from __future__ import absolute_import
 from .misc import *  # NOQA
 from .decorators import *  # NOQA
+from .classview import *  # NOQA
diff --git a/coaster/views/classview.py b/coaster/views/classview.py
new file mode 100644
index 00000000..d35eae5f
--- /dev/null
+++ b/coaster/views/classview.py
@@ -0,0 +1,193 @@
+# -*- coding: utf-8 -*-
+
+"""
+Class-based views
+-----------------
+
+Group related views into a class for easier management.
+"""
+
+from __future__ import unicode_literals
+import types
+
+__all__ = ['route', 'ClassView', 'ModelView']
+
+
+# :func:`route` wraps :class:`ViewDecorator` so that it can have an independent __doc__
+def route(rule, **options):
+    """
+    Decorator for defining routes on a :class:`ClassView` and its methods. Accepts the
+    same parameters that Flask's :meth:`~flask.Flask.route` accepts. See :class:`ClassView`
+    for usage notes.
+    """
+    return ViewDecorator(rule, **options)
+
+
+def rulejoin(class_rule, method_rule):
+    """
+    Join class and method rules::
+
+        >>> rulejoin('/', '')
+        '/'
+        >>> rulejoin('/', 'first')
+        '/first'
+        >>> rulejoin('/first', '/second')
+        '/second'
+        >>> rulejoin('/first', 'second')
+        '/first/second'
+        >>> rulejoin('/first/', 'second')
+        '/first/second'
+        >>> rulejoin('/first/<second>', '')
+        '/first/<second>'
+    """
+    if method_rule.startswith('/'):
+        return method_rule
+    else:
+        return class_rule + ('' if class_rule.endswith('/') or not method_rule else '/') + method_rule
+
+
+class ViewDecorator(object):
+    """
+    Internal object for :func:`route` decorated view methods.
+    """
+    def __init__(self, rule, **options):
+        self.routes = [(rule, options)]
+
+    def __call__(self, decorated):
+        # Are we decorating a ClassView? If so, annotate the ClassView and return it
+        if type(decorated) is type and issubclass(decorated, ClassView):
+            if '__routes__' not in decorated.__dict__:
+                decorated.__routes__ = []
+            decorated.__routes__.extend(self.routes)
+            return decorated
+
+        # Are we decorating another ViewDecorator? If so, copy routes and
+        # wrapped method from it.
+        elif isinstance(decorated, ViewDecorator):
+            self.routes.extend(decorated.routes)
+            self.method = decorated.method
+
+        # If neither ClassView nor ViewDecorator, assume it's a callable method
+        else:
+            self.method = decorated
+
+        self.name = self.__name__ = self.method.__name__
+        self.endpoint = self.name  # This will change once init_app calls __set_name__
+        self.__doc__ = self.method.__doc__
+        return self
+
+    # Normally Python 3.6+, but called manually by :meth:`ClassView.init_app`
+    def __set_name__(self, owner, name):
+        self.name = name
+
+    def __get__(self, obj, cls=None):
+        # Attempting to access this object from the class or instance should be
+        # indistinguishable from accessing the original, unwrapped method.
+        return types.MethodType(self.method, cls or type(obj), type)
+
+    def init_app(self, app, cls, callback=None):
+        """
+        Register routes for a given app and ClassView subclass
+        """
+        # Revisit endpoint to account for subclasses
+        endpoint = cls.__name__ + '_' + self.name
+
+        # Instantiate the ClassView and call the method with it
+        def view_func(*args, **kwargs):
+            return view_func.view_method(view_func.view_class(), *args, **kwargs)
+
+        view_func.__name__ = self.__name__
+        view_func.__doc__ = self.__doc__
+        # Stick `method` and `cls` into view_func to avoid creating a closure.
+        view_func.view_method = self.method
+        view_func.view_class = cls
+
+        for class_rule, class_options in cls.__routes__:
+            for method_rule, method_options in self.routes:
+                use_options = dict(method_options)
+                use_options.update(class_options)
+                endpoint = use_options.pop('endpoint', endpoint)
+                use_rule = rulejoin(class_rule, method_rule)
+                app.add_url_rule(use_rule, endpoint, view_func, **use_options)
+                if callback:
+                    callback(use_rule, endpoint, view_func, **use_options)
+
+
+class ClassView(object):
+    """
+    Base class for defining a collection of views that are related to each
+    other. Subclasses may define methods decorated with :func:`route`. When
+    :meth:`init_app` is called, these will be added as routes to the app.
+
+    Typical use::
+
+        @route('/')
+        class IndexView(ClassView):
+            @route('')
+            def index():
+                return render_template('index.html.jinja2')
+
+            @route('about')
+            def about():
+                return render_template('about.html.jinja2')
+
+        IndexView.init_app(app)
+
+    The :func:`route` decorator on the class specifies the base rule which is
+    prefixed to the rule specified on each view method. This example produces
+    two view handlers, for ``/`` and ``/about``.
+
+    A rudimentary CRUD view collection can be assembled like this::
+
+        @route('/doc/<name>')
+        class DocumentView(ClassView):
+            @route('')
+            @render_with('mydocument.html.jinja2', json=True)
+            def view(self, name):
+                document = MyDocument.query.filter_by(name=name).first_or_404()
+                return document.current_access()
+
+            @route('edit', methods=['POST'])
+            @requestform('title', 'content')
+            def edit(self, name, title, content):
+                document = MyDocument.query.filter_by(name=name).first_or_404()
+                document.title = title
+                document.content = content
+                return 'edited!'
+
+        DocumentView.init_app(app)
+
+    See :class:`ModelView` (TODO) for a better way to build views around a model.
+    """
+    # If the class did not get a @route decorator, provide a fallback route
+    __routes__ = [('', {})]
+
+    @classmethod
+    def init_app(cls, app, callback=None):
+        """
+        Register views on an app. If :attr:`callback` is specified, it will
+        be called after ``app.``:meth:`~flask.Flask.add_url_rule`, with the same
+        parameters.
+        """
+        processed = set()
+        for base in cls.__mro__:
+            for name, attr in base.__dict__.items():
+                if name in processed:
+                    continue
+
+                if isinstance(attr, ViewDecorator):
+                    processed.add(name)
+                    attr.__set_name__(base, name)  # Required for Python < 3.6
+                    attr.init_app(app, cls, callback=callback)
+
+
+class ModelView(ClassView):
+    """
+    Base class for constructing views around a model. Provides assistance for:
+
+    1. Loading instances based on URL parameters
+    2. Registering view handlers for Model.url_for() calls
+
+    TODO
+    """
+    pass  # TODO
diff --git a/coaster/views/decorators.py b/coaster/views/decorators.py
index 6a002535..cb9933b3 100644
--- a/coaster/views/decorators.py
+++ b/coaster/views/decorators.py
@@ -107,7 +107,7 @@ def datasource():
                 return request.values if request else {}
 
         @wraps(f)
-        def decorated_function(**kw):
+        def decorated_function(*args, **kw):
             values = datasource()
             for name, filt, is_list in namefilt:
                 # Process name if
@@ -122,7 +122,7 @@ def decorated_function(**kw):
                     except ValueError as e:
                         raise RequestValueError(e)
             try:
-                return f(**kw)
+                return f(*args, **kw)
             except TypeError as e:
                 raise RequestTypeError(e)
         return decorated_function
@@ -235,7 +235,7 @@ def show_page(folder, page):
     """
     def inner(f):
         @wraps(f)
-        def decorated_function(**kw):
+        def decorated_function(*args, **kw):
             permissions = None
             permission_required = kwargs.get('permission')
             url_check_attributes = kwargs.get('urlcheck', [])
@@ -315,9 +315,9 @@ def decorated_function(**kw):
             if permission_required and not (permission_required & permissions):
                 abort(403)
             if kwargs.get('kwargs'):
-                return f(kwargs=kw, **result)
+                return f(*args, kwargs=kw, **result)
             else:
-                return f(**result)
+                return f(*args, **result)
         return decorated_function
     return inner
 
diff --git a/docs/conf.py b/docs/conf.py
index adfaa982..ae971288 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -248,5 +248,7 @@
 # Example configuration for intersphinx: refer to the Python standard library.
 intersphinx_mapping = {
     'https://docs.python.org/2/': None,
-    'http://flask-sqlalchemy.pocoo.org/2.2/': None,
+    'http://flask.pocoo.org/docs/': None,
+    'http://docs.sqlalchemy.org/en/latest/': None,
+    'http://flask-sqlalchemy.pocoo.org/2.3/': None,
     }
diff --git a/docs/views/classview.rst b/docs/views/classview.rst
new file mode 100644
index 00000000..0e81f7c5
--- /dev/null
+++ b/docs/views/classview.rst
@@ -0,0 +1,2 @@
+.. automodule:: coaster.views.classview
+   :members:
diff --git a/docs/views/index.rst b/docs/views/index.rst
index 770e0c93..227fc3b9 100644
--- a/docs/views/index.rst
+++ b/docs/views/index.rst
@@ -5,3 +5,4 @@
 
    misc
    decorators
+   classview
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
new file mode 100644
index 00000000..f3265cd0
--- /dev/null
+++ b/tests/test_views_classview.py
@@ -0,0 +1,108 @@
+# -*- coding: utf-8 -*-
+
+from __future__ import absolute_import, unicode_literals
+
+import unittest
+from flask import Flask, json
+import six
+from coaster.sqlalchemy import BaseNameMixin, BaseScopedNameMixin
+from coaster.db import db
+from coaster.views import ClassView, route, requestform, render_with
+
+
+app = Flask(__name__)
+app.testing = True
+app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite://'
+app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
+db.init_app(app)
+
+
+# --- Models ------------------------------------------------------------------
+
+class ViewDocument(BaseNameMixin, db.Model):
+    __tablename__ = 'view_document'
+    __roles__ = {
+        'all': {
+            'read': {'name', 'title'}
+            }
+        }
+
+
+class ScopedViewDocument(BaseScopedNameMixin, db.Model):
+    __tablename__ = 'scoped_view_document'
+    parent_id = db.Column(None, db.ForeignKey('view_document.id'), nullable=False)
+    parent = db.relationship(ViewDocument, backref=db.backref('children', cascade='all, delete-orphan'))
+
+
+# --- Views -------------------------------------------------------------------
+
+@route('/')
+class IndexView(ClassView):
+    @route('')
+    def index(self):
+        return 'index'
+
+    @route('page')
+    def page(self):
+        return 'page'
+
+IndexView.init_app(app)
+
+
+@route('/doc/<name>')
+class DocumentView(ClassView):
+    @route('')
+    @render_with(json=True)
+    def view(self, name):
+        document = ViewDocument.query.filter_by(name=name).first_or_404()
+        return document.current_access()
+
+    @route('edit', methods=['POST'])  # Maps to /doc/<name>/edit
+    @route('/edit/<name>', methods=['POST'])  # Maps to /edit/<name>
+    @requestform('title')
+    def edit(self, name, title):
+        document = ViewDocument.query.filter_by(name=name).first_or_404()
+        document.title = title
+        return 'edited!'
+
+DocumentView.init_app(app)
+
+
+# --- Tests -------------------------------------------------------------------
+
+class TestClassView(unittest.TestCase):
+    app = app
+
+    def setUp(self):
+        self.ctx = self.app.test_request_context()
+        self.ctx.push()
+        db.create_all()
+        self.session = db.session
+        self.client = self.app.test_client()
+
+    def tearDown(self):
+        self.session.rollback()
+        db.drop_all()
+        self.ctx.pop()
+
+    def test_index(self):
+        rv = self.client.get('/')
+        assert rv.data == six.binary_type('index', 'utf-8')
+
+    def test_page(self):
+        rv = self.client.get('/page')
+        assert rv.data == six.binary_type('page', 'utf-8')
+
+    def test_document_404(self):
+        rv = self.client.get('/doc/non-existant')
+        assert rv.status_code == 404  # This 404 came from DocumentView.view
+
+    def test_document_view(self):
+        doc = ViewDocument(name='test1', title="Test")
+        self.session.add(doc)
+        self.session.commit()
+        rv = self.client.get('/doc/test1')
+        assert rv.status_code == 200
+        data = json.loads(rv.data)
+        assert data['name'] == 'test1'
+        assert data['title'] == "Test"

From f027e9407c86a06c19a0a7f45d7041ace98d0d4e Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 09:51:23 +0530
Subject: [PATCH 04/26] Fix tests for Python 2.7

---
 tests/test_views_classview.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index f3265cd0..34132f8d 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -4,7 +4,6 @@
 
 import unittest
 from flask import Flask, json
-import six
 from coaster.sqlalchemy import BaseNameMixin, BaseScopedNameMixin
 from coaster.db import db
 from coaster.views import ClassView, route, requestform, render_with
@@ -87,11 +86,11 @@ def tearDown(self):
 
     def test_index(self):
         rv = self.client.get('/')
-        assert rv.data == six.binary_type('index', 'utf-8')
+        assert rv.data == 'index'.encode('utf-8')
 
     def test_page(self):
         rv = self.client.get('/page')
-        assert rv.data == six.binary_type('page', 'utf-8')
+        assert rv.data == 'page'.encode('utf-8')
 
     def test_document_404(self):
         rv = self.client.get('/doc/non-existant')

From 635a4c576ae5927c3499f2a4ad519d7da372d748 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 10:12:04 +0530
Subject: [PATCH 05/26] b'str' works in Py2. Who knew?

---
 tests/test_views_classview.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index 34132f8d..e2101b1f 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -86,14 +86,14 @@ def tearDown(self):
 
     def test_index(self):
         rv = self.client.get('/')
-        assert rv.data == 'index'.encode('utf-8')
+        assert rv.data == b'index'
 
     def test_page(self):
         rv = self.client.get('/page')
-        assert rv.data == 'page'.encode('utf-8')
+        assert rv.data == b'page'
 
     def test_document_404(self):
-        rv = self.client.get('/doc/non-existant')
+        rv = self.client.get('/doc/this-doc-does-not-exist')
         assert rv.status_code == 404  # This 404 came from DocumentView.view
 
     def test_document_view(self):

From 01ccd878658afaf3695b1c51975e6128255e28d6 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 12:25:54 +0530
Subject: [PATCH 06/26] Allow subclasses to replace view handlers while keeping
 routes

---
 coaster/views/classview.py    | 12 ++++--
 tests/test_views_classview.py | 79 ++++++++++++++++++++++++++++++++++-
 2 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index d35eae5f..c1e14bd3 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -53,6 +53,12 @@ class ViewDecorator(object):
     def __init__(self, rule, **options):
         self.routes = [(rule, options)]
 
+    def reroute(self, f):
+        # Use type(self) instead of ViewDecorator so this works for (future) subclasses of ViewDecorator
+        r = type(self)('')
+        r.routes = self.routes
+        return r.__call__(f)
+
     def __call__(self, decorated):
         # Are we decorating a ClassView? If so, annotate the ClassView and return it
         if type(decorated) is type and issubclass(decorated, ClassView):
@@ -71,6 +77,7 @@ def __call__(self, decorated):
         else:
             self.method = decorated
 
+        self.method.reroute = self.reroute  # Place our reroute decorator into the method's namespace
         self.name = self.__name__ = self.method.__name__
         self.endpoint = self.name  # This will change once init_app calls __set_name__
         self.__doc__ = self.method.__doc__
@@ -83,7 +90,7 @@ def __set_name__(self, owner, name):
     def __get__(self, obj, cls=None):
         # Attempting to access this object from the class or instance should be
         # indistinguishable from accessing the original, unwrapped method.
-        return types.MethodType(self.method, cls or type(obj), type)
+        return types.MethodType(self.method, cls or type(obj))
 
     def init_app(self, app, cls, callback=None):
         """
@@ -174,9 +181,8 @@ def init_app(cls, app, callback=None):
             for name, attr in base.__dict__.items():
                 if name in processed:
                     continue
-
+                processed.add(name)
                 if isinstance(attr, ViewDecorator):
-                    processed.add(name)
                     attr.__set_name__(base, name)  # Required for Python < 3.6
                     attr.init_app(app, cls, callback=callback)
 
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index e2101b1f..c738bff1 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -5,7 +5,7 @@
 import unittest
 from flask import Flask, json
 from coaster.sqlalchemy import BaseNameMixin, BaseScopedNameMixin
-from coaster.db import db
+from coaster.db import SQLAlchemy
 from coaster.views import ClassView, route, requestform, render_with
 
 
@@ -13,7 +13,7 @@
 app.testing = True
 app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite://'
 app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
-db.init_app(app)
+db = SQLAlchemy(app)
 
 
 # --- Models ------------------------------------------------------------------
@@ -58,6 +58,7 @@ def view(self, name):
 
     @route('edit', methods=['POST'])  # Maps to /doc/<name>/edit
     @route('/edit/<name>', methods=['POST'])  # Maps to /edit/<name>
+    @route('', methods=['POST'])  # Maps to /doc/<name>
     @requestform('title')
     def edit(self, name, title):
         document = ViewDocument.query.filter_by(name=name).first_or_404()
@@ -67,6 +68,45 @@ def edit(self, name, title):
 DocumentView.init_app(app)
 
 
+class BaseView(ClassView):
+    @route('')
+    def first(self):
+        return 'first'
+
+    @route('second')
+    def second(self):
+        return 'second'
+
+    @route('third')
+    def third(self):
+        return 'third'
+
+    @route('inherited')
+    def inherited(self):
+        return 'inherited'
+
+    @route('also-inherited')
+    def also_inherited(self):
+        return 'also_inherited'
+
+
+@route('/subclasstest')
+class SubView(BaseView):
+    @BaseView.first.reroute
+    def first(self):
+        return 'rerouted-first'
+
+    @route('2')
+    @BaseView.second.reroute
+    def second(self):
+        return 'rerouted-second'
+
+    def third(self):
+        return 'removed-third'
+
+SubView.init_app(app)
+
+
 # --- Tests -------------------------------------------------------------------
 
 class TestClassView(unittest.TestCase):
@@ -105,3 +145,38 @@ def test_document_view(self):
         data = json.loads(rv.data)
         assert data['name'] == 'test1'
         assert data['title'] == "Test"
+
+    def test_document_edit(self):
+        doc = ViewDocument(name='test1', title="Test")
+        self.session.add(doc)
+        self.session.commit()
+        self.client.post('/doc/test1/edit', data={'title': "Edit 1"})
+        assert doc.title == "Edit 1"
+        self.client.post('/edit/test1', data={'title': "Edit 2"})
+        assert doc.title == "Edit 2"
+        self.client.post('/doc/test1', data={'title': "Edit 3"})
+        assert doc.title == "Edit 3"
+
+    def test_rerouted(self):
+        rv = self.client.get('/subclasstest')
+        assert rv.data != b'first'
+        assert rv.data == b'rerouted-first'
+        assert rv.status_code == 200
+        rv = self.client.get('/subclasstest/second')
+        assert rv.data != b'second'
+        assert rv.data == b'rerouted-second'
+        assert rv.status_code == 200
+        rv = self.client.get('/subclasstest/2')
+        assert rv.data != b'second'
+        assert rv.data == b'rerouted-second'
+        assert rv.status_code == 200
+
+    def test_unrouted(self):
+        rv = self.client.get('/subclasstest/third')
+        assert rv.data != b'third'
+        assert rv.data != b'unrouted-third'
+        assert rv.status_code == 404
+
+    def test_inherited(self):
+        rv = self.client.get('/subclasstest/inherited')
+        assert rv.data == b'inherited'

From 53eadc2733f569faaf946cb0b9bf33992274063f Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 13:00:03 +0530
Subject: [PATCH 07/26] Docstrings for ClassView tests

---
 tests/test_views_classview.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index c738bff1..ca6f16f6 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -125,18 +125,22 @@ def tearDown(self):
         self.ctx.pop()
 
     def test_index(self):
+        """Test index view (/)"""
         rv = self.client.get('/')
         assert rv.data == b'index'
 
     def test_page(self):
+        """Test page view (/page)"""
         rv = self.client.get('/page')
         assert rv.data == b'page'
 
     def test_document_404(self):
+        """Test 404 response from within a view"""
         rv = self.client.get('/doc/this-doc-does-not-exist')
         assert rv.status_code == 404  # This 404 came from DocumentView.view
 
     def test_document_view(self):
+        """Test document view (loaded from database)"""
         doc = ViewDocument(name='test1', title="Test")
         self.session.add(doc)
         self.session.commit()
@@ -147,6 +151,7 @@ def test_document_view(self):
         assert data['title'] == "Test"
 
     def test_document_edit(self):
+        """POST handler shares URL with GET handler but is routed to correctly"""
         doc = ViewDocument(name='test1', title="Test")
         self.session.add(doc)
         self.session.commit()
@@ -158,10 +163,14 @@ def test_document_edit(self):
         assert doc.title == "Edit 3"
 
     def test_rerouted(self):
+        """Subclass replaces view handler"""
         rv = self.client.get('/subclasstest')
         assert rv.data != b'first'
         assert rv.data == b'rerouted-first'
         assert rv.status_code == 200
+
+    def test_rerouted_with_new_routes(self):
+        """Subclass replaces view handler and adds new routes"""
         rv = self.client.get('/subclasstest/second')
         assert rv.data != b'second'
         assert rv.data == b'rerouted-second'
@@ -172,11 +181,13 @@ def test_rerouted(self):
         assert rv.status_code == 200
 
     def test_unrouted(self):
+        """Subclass removes a route from base class"""
         rv = self.client.get('/subclasstest/third')
         assert rv.data != b'third'
         assert rv.data != b'unrouted-third'
         assert rv.status_code == 404
 
     def test_inherited(self):
+        """Subclass inherits a view from the base class without modifying it"""
         rv = self.client.get('/subclasstest/inherited')
         assert rv.data == b'inherited'

From 87dc29505e75938e4b2718e1354357bf75c0724f Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 13:00:49 +0530
Subject: [PATCH 08/26] Add ClassView.get_view and support added routes on
 existing handlers

---
 coaster/views/classview.py    | 10 ++++++++++
 tests/test_views_classview.py | 27 +++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index c1e14bd3..1bd23436 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -169,6 +169,16 @@ def edit(self, name, title, content):
     # If the class did not get a @route decorator, provide a fallback route
     __routes__ = [('', {})]
 
+    @classmethod
+    def get_view(cls, view):
+        for base in cls.__mro__:
+            if view in base.__dict__:
+                r = base.__dict__[view]
+                if not isinstance(r, ViewDecorator):
+                    raise AttributeError(view)
+                return r
+        raise AttributeError(view)
+
     @classmethod
     def init_app(cls, app, callback=None):
         """
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index ca6f16f6..17b5fda5 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -104,6 +104,10 @@ def second(self):
     def third(self):
         return 'removed-third'
 
+    also_inherited = route('/inherited')(
+        route('inherited2')(
+            BaseView.get_view('also_inherited')))
+
 SubView.init_app(app)
 
 
@@ -191,3 +195,26 @@ def test_inherited(self):
         """Subclass inherits a view from the base class without modifying it"""
         rv = self.client.get('/subclasstest/inherited')
         assert rv.data == b'inherited'
+        assert rv.status_code == 200
+
+    def test_added_routes(self):
+        """Subclass adds more routes to a base class's view handler"""
+        rv = self.client.get('/subclasstest/also-inherited')  # From base class
+        assert rv.data == b'also_inherited'
+        rv = self.client.get('/subclasstest/inherited2')  # Added in sub class
+        assert rv.data == b'also_inherited'
+        rv = self.client.get('/inherited')  # Added in sub class
+        assert rv.data == b'also_inherited'
+
+    def test_get_view(self):
+        """The get_view method works for any view handlers available in the class"""
+        assert IndexView.get_view('index').name == 'index'
+        assert BaseView.get_view('first').name == 'first'
+        assert SubView.get_view('first').name == 'first'
+        assert BaseView.get_view('first') != SubView.get_view('first')
+        assert BaseView.get_view('inherited') == SubView.get_view('inherited')
+        assert BaseView.get_view('third').name == 'third'
+        with self.assertRaises(AttributeError):
+            SubView.get_view('third')
+        with self.assertRaises(AttributeError):
+            SubView.get_view('this_is_not_in_any_of_the_classes')

From 3f1041f38eb53a88b1d8bfa26e197aff4140f390 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 13:37:59 +0530
Subject: [PATCH 09/26] Replace ClassView.get_view with add_route_for

---
 coaster/views/classview.py    | 18 +++++++++++-------
 tests/test_views_classview.py | 24 ++++++++++--------------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index 1bd23436..067fb625 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -170,14 +170,18 @@ def edit(self, name, title, content):
     __routes__ = [('', {})]
 
     @classmethod
-    def get_view(cls, view):
+    def __get_raw_attr(cls, attr):
         for base in cls.__mro__:
-            if view in base.__dict__:
-                r = base.__dict__[view]
-                if not isinstance(r, ViewDecorator):
-                    raise AttributeError(view)
-                return r
-        raise AttributeError(view)
+            if attr in base.__dict__:
+                return base.__dict__[attr]
+        raise AttributeError(attr)
+
+    @classmethod
+    def add_route_for(cls, attr, rule, **options):
+        """
+        Add a route for an existing method or view in the class view.
+        """
+        setattr(cls, attr, route(rule, **options)(cls.__get_raw_attr(attr)))
 
     @classmethod
     def init_app(cls, app, callback=None):
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index 17b5fda5..5213652a 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -89,6 +89,9 @@ def inherited(self):
     def also_inherited(self):
         return 'also_inherited'
 
+    def latent_route(self):
+        return 'latent-route'
+
 
 @route('/subclasstest')
 class SubView(BaseView):
@@ -104,10 +107,10 @@ def second(self):
     def third(self):
         return 'removed-third'
 
-    also_inherited = route('/inherited')(
-        route('inherited2')(
-            BaseView.get_view('also_inherited')))
 
+SubView.add_route_for('also_inherited', '/inherited')
+SubView.add_route_for('also_inherited', 'inherited2')
+SubView.add_route_for('latent_route', 'latent')
 SubView.init_app(app)
 
 
@@ -205,16 +208,9 @@ def test_added_routes(self):
         assert rv.data == b'also_inherited'
         rv = self.client.get('/inherited')  # Added in sub class
         assert rv.data == b'also_inherited'
+        rv = self.client.get('/subclasstest/latent')
+        assert rv.data == b'latent-route'
 
-    def test_get_view(self):
-        """The get_view method works for any view handlers available in the class"""
-        assert IndexView.get_view('index').name == 'index'
-        assert BaseView.get_view('first').name == 'first'
-        assert SubView.get_view('first').name == 'first'
-        assert BaseView.get_view('first') != SubView.get_view('first')
-        assert BaseView.get_view('inherited') == SubView.get_view('inherited')
-        assert BaseView.get_view('third').name == 'third'
-        with self.assertRaises(AttributeError):
-            SubView.get_view('third')
+    def test_cant_route_missing_method(self):
         with self.assertRaises(AttributeError):
-            SubView.get_view('this_is_not_in_any_of_the_classes')
+            SubView.add_route_for('this_method_does_not_exist', '/missing')

From e86e491f6c99dd0d6aafef342a2749f839c8040a Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 14:25:02 +0530
Subject: [PATCH 10/26] Documentation and misc cleanup in ClassView

---
 coaster/views/classview.py    | 62 +++++++++++++++++++++++++----------
 tests/test_views_classview.py |  1 -
 2 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index 067fb625..378503f7 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -16,9 +16,9 @@
 # :func:`route` wraps :class:`ViewDecorator` so that it can have an independent __doc__
 def route(rule, **options):
     """
-    Decorator for defining routes on a :class:`ClassView` and its methods. Accepts the
-    same parameters that Flask's :meth:`~flask.Flask.route` accepts. See :class:`ClassView`
-    for usage notes.
+    Decorator for defining routes on a :class:`ClassView` and its methods.
+    Accepts the same parameters that Flask's ``app.``:meth:`~flask.Flask.route`
+    accepts. See :class:`ClassView` for usage notes.
     """
     return ViewDecorator(rule, **options)
 
@@ -71,16 +71,16 @@ def __call__(self, decorated):
         # wrapped method from it.
         elif isinstance(decorated, ViewDecorator):
             self.routes.extend(decorated.routes)
-            self.method = decorated.method
+            self.func = decorated.func
 
         # If neither ClassView nor ViewDecorator, assume it's a callable method
         else:
-            self.method = decorated
+            self.func = decorated
 
-        self.method.reroute = self.reroute  # Place our reroute decorator into the method's namespace
-        self.name = self.__name__ = self.method.__name__
+        self.func.reroute = self.reroute  # Place our reroute decorator into the method's namespace
+        self.name = self.__name__ = self.func.__name__
         self.endpoint = self.name  # This will change once init_app calls __set_name__
-        self.__doc__ = self.method.__doc__
+        self.__doc__ = self.func.__doc__
         return self
 
     # Normally Python 3.6+, but called manually by :meth:`ClassView.init_app`
@@ -90,7 +90,7 @@ def __set_name__(self, owner, name):
     def __get__(self, obj, cls=None):
         # Attempting to access this object from the class or instance should be
         # indistinguishable from accessing the original, unwrapped method.
-        return types.MethodType(self.method, cls or type(obj))
+        return types.MethodType(self.func, cls or type(obj))
 
     def init_app(self, app, cls, callback=None):
         """
@@ -101,12 +101,12 @@ def init_app(self, app, cls, callback=None):
 
         # Instantiate the ClassView and call the method with it
         def view_func(*args, **kwargs):
-            return view_func.view_method(view_func.view_class(), *args, **kwargs)
+            return view_func.wrapped_func(view_func.view_class(), *args, **kwargs)
 
         view_func.__name__ = self.__name__
         view_func.__doc__ = self.__doc__
         # Stick `method` and `cls` into view_func to avoid creating a closure.
-        view_func.view_method = self.method
+        view_func.wrapped_func = self.func
         view_func.view_class = cls
 
         for class_rule, class_options in cls.__routes__:
@@ -170,18 +170,44 @@ def edit(self, name, title, content):
     __routes__ = [('', {})]
 
     @classmethod
-    def __get_raw_attr(cls, attr):
+    def __get_raw_attr(cls, name):
         for base in cls.__mro__:
-            if attr in base.__dict__:
-                return base.__dict__[attr]
-        raise AttributeError(attr)
+            if name in base.__dict__:
+                return base.__dict__[name]
+        raise AttributeError(name)
 
     @classmethod
-    def add_route_for(cls, attr, rule, **options):
+    def add_route_for(cls, _name, rule, **options):
         """
-        Add a route for an existing method or view in the class view.
+        Add a route for an existing method or view in the class view. Useful
+        for modifying routes that a subclass inherits from a base class::
+
+            class BaseView(ClassView):
+                def latent_view(self):
+                    return 'latent-view'
+
+                @route('other')
+                def other_view(self):
+                    return 'other-view'
+
+            @route('/path')
+            class SubView(BaseView):
+                pass
+
+            SubView.add_route_for('latent_view', 'latent')
+            SubView.add_route_for('other_view', 'another')
+            SubView.init_app(app)
+
+            # Created routes:
+            # /path/latent -> SubView.latent (added)
+            # /path/other -> SubView.other (inherited)
+            # /path/another -> SubView.other (added)
+
+        :param _name: Name of the method or view on the class
+        :param rule: URL rule to be added
+        :param options: Additional options for :meth:`~flask.Flask.add_url_rule`
         """
-        setattr(cls, attr, route(rule, **options)(cls.__get_raw_attr(attr)))
+        setattr(cls, _name, route(rule, **options)(cls.__get_raw_attr(_name)))
 
     @classmethod
     def init_app(cls, app, callback=None):
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index 5213652a..8ad56359 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -107,7 +107,6 @@ def second(self):
     def third(self):
         return 'removed-third'
 
-
 SubView.add_route_for('also_inherited', '/inherited')
 SubView.add_route_for('also_inherited', 'inherited2')
 SubView.add_route_for('latent_route', 'latent')

From c99971bde5416c7d44c96204faf8cd4a8a6704b6 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 15:04:50 +0530
Subject: [PATCH 11/26] Add a view decorator wrapper to make reroute work

---
 coaster/views/classview.py    | 21 ++++++++++++++++-----
 tests/test_views_classview.py | 25 +++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index 378503f7..772d9961 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -69,7 +69,7 @@ def __call__(self, decorated):
 
         # Are we decorating another ViewDecorator? If so, copy routes and
         # wrapped method from it.
-        elif isinstance(decorated, ViewDecorator):
+        elif isinstance(decorated, (ViewDecorator, ViewDecoratorWrapper)):
             self.routes.extend(decorated.routes)
             self.func = decorated.func
 
@@ -77,7 +77,6 @@ def __call__(self, decorated):
         else:
             self.func = decorated
 
-        self.func.reroute = self.reroute  # Place our reroute decorator into the method's namespace
         self.name = self.__name__ = self.func.__name__
         self.endpoint = self.name  # This will change once init_app calls __set_name__
         self.__doc__ = self.func.__doc__
@@ -88,9 +87,7 @@ def __set_name__(self, owner, name):
         self.name = name
 
     def __get__(self, obj, cls=None):
-        # Attempting to access this object from the class or instance should be
-        # indistinguishable from accessing the original, unwrapped method.
-        return types.MethodType(self.func, cls or type(obj))
+        return ViewDecoratorWrapper(self, obj, cls)
 
     def init_app(self, app, cls, callback=None):
         """
@@ -120,6 +117,20 @@ def view_func(*args, **kwargs):
                     callback(use_rule, endpoint, view_func, **use_options)
 
 
+class ViewDecoratorWrapper(object):
+    """Wrapper for a view at runtime"""
+    def __init__(self, viewd, obj, cls=None):
+        self.__viewd = viewd
+        self.__obj = obj
+        self.__cls = cls
+
+    def __call__(self, *args, **kwargs):
+        return self.__viewd.func(self.__obj, *args, **kwargs)
+
+    def __getattr__(self, attr):
+        return getattr(self.__viewd, attr)
+
+
 class ClassView(object):
     """
     Base class for defining a collection of views that are related to each
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index 8ad56359..bc6e448d 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -113,6 +113,16 @@ def third(self):
 SubView.init_app(app)
 
 
+@route('/secondsub')
+class AnotherSubView(BaseView):
+    @route('2-2')
+    @BaseView.second.reroute
+    def second(self):
+        return 'also-rerouted-second'
+
+AnotherSubView.init_app(app)
+
+
 # --- Tests -------------------------------------------------------------------
 
 class TestClassView(unittest.TestCase):
@@ -211,5 +221,20 @@ def test_added_routes(self):
         assert rv.data == b'latent-route'
 
     def test_cant_route_missing_method(self):
+        """Routes can't be added for missing attributes"""
         with self.assertRaises(AttributeError):
             SubView.add_route_for('this_method_does_not_exist', '/missing')
+
+    def test_second_subview_reroute(self):
+        """Using reroute does not mutate the base class"""
+        rv = self.client.get('/secondsub/second')
+        assert rv.data != b'second'
+        assert rv.data == b'also-rerouted-second'
+        assert rv.status_code == 200
+        rv = self.client.get('/secondsub/2-2')
+        assert rv.data != b'second'
+        assert rv.data == b'also-rerouted-second'
+        assert rv.status_code == 200
+        # Confirm we did not accidentally acquire this from SubView's use of reroute
+        rv = self.client.get('/secondsub/2')
+        assert rv.status_code == 404

From 3346323a1495548b89dc4b43bc9182b370da8a49 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 14 Feb 2018 15:33:41 +0530
Subject: [PATCH 12/26] Add test for direct call to a view

---
 coaster/views/classview.py    |  2 +-
 tests/test_views_classview.py | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index 772d9961..d76f102e 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -8,7 +8,6 @@
 """
 
 from __future__ import unicode_literals
-import types
 
 __all__ = ['route', 'ClassView', 'ModelView']
 
@@ -125,6 +124,7 @@ def __init__(self, viewd, obj, cls=None):
         self.__cls = cls
 
     def __call__(self, *args, **kwargs):
+        """Treat this like a call to the method (and not to the view)"""
         return self.__viewd.func(self.__obj, *args, **kwargs)
 
     def __getattr__(self, attr):
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index bc6e448d..0f8e05b7 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -160,6 +160,7 @@ def test_document_view(self):
         doc = ViewDocument(name='test1', title="Test")
         self.session.add(doc)
         self.session.commit()
+
         rv = self.client.get('/doc/test1')
         assert rv.status_code == 200
         data = json.loads(rv.data)
@@ -171,6 +172,7 @@ def test_document_edit(self):
         doc = ViewDocument(name='test1', title="Test")
         self.session.add(doc)
         self.session.commit()
+
         self.client.post('/doc/test1/edit', data={'title': "Edit 1"})
         assert doc.title == "Edit 1"
         self.client.post('/edit/test1', data={'title': "Edit 2"})
@@ -178,6 +180,22 @@ def test_document_edit(self):
         self.client.post('/doc/test1', data={'title': "Edit 3"})
         assert doc.title == "Edit 3"
 
+    def test_callable_view(self):
+        """View handlers are callable as regular methods"""
+        doc = ViewDocument(name='test1', title="Test")
+        self.session.add(doc)
+        self.session.commit()
+
+        rv = DocumentView().view('test1')
+        assert rv.status_code == 200
+        data = json.loads(rv.data)
+        assert data['name'] == 'test1'
+        assert data['title'] == "Test"
+
+        rv = DocumentView().edit('test1', "Edited")
+        assert rv == 'edited!'
+        assert doc.title == "Edited"
+
     def test_rerouted(self):
         """Subclass replaces view handler"""
         rv = self.client.get('/subclasstest')

From 796f0dd00fada48c8b0da84a0de84a92350ec15b Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Thu, 15 Feb 2018 01:56:12 +0530
Subject: [PATCH 13/26] First pass at ModelView

---
 coaster/sqlalchemy/mixins.py  |   3 +
 coaster/views/classview.py    | 173 ++++++++++++++++++++++++++++++----
 tests/test_views_classview.py |  49 +++++++++-
 3 files changed, 204 insertions(+), 21 deletions(-)

diff --git a/coaster/sqlalchemy/mixins.py b/coaster/sqlalchemy/mixins.py
index 6456af4a..234e0b4c 100644
--- a/coaster/sqlalchemy/mixins.py
+++ b/coaster/sqlalchemy/mixins.py
@@ -262,6 +262,9 @@ def url_for(self, action='view', **kwargs):
 
     @classmethod
     def is_url_for(cls, _action, _endpoint=None, _external=None, **paramattrs):
+        """
+        View decorator that registers the view as a :meth:`url_for` target.
+        """
         def decorator(f):
             if 'url_for_endpoints' not in cls.__dict__:
                 cls.url_for_endpoints = {}  # Stick it into the class with the first endpoint
diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index d76f102e..2a7b659e 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -8,8 +8,10 @@
 """
 
 from __future__ import unicode_literals
+from functools import wraps, update_wrapper
+from werkzeug.routing import parse_rule
 
-__all__ = ['route', 'ClassView', 'ModelView']
+__all__ = ['route', 'ClassView', 'ModelView', 'UrlForView', 'InstanceLoader']
 
 
 # :func:`route` wraps :class:`ViewDecorator` so that it can have an independent __doc__
@@ -95,14 +97,30 @@ def init_app(self, app, cls, callback=None):
         # Revisit endpoint to account for subclasses
         endpoint = cls.__name__ + '_' + self.name
 
-        # Instantiate the ClassView and call the method with it
         def view_func(*args, **kwargs):
-            return view_func.wrapped_func(view_func.view_class(), *args, **kwargs)
-
-        view_func.__name__ = self.__name__
-        view_func.__doc__ = self.__doc__
-        # Stick `method` and `cls` into view_func to avoid creating a closure.
-        view_func.wrapped_func = self.func
+            # Instantiate the view class. We depend on its __init__ requiring no parameters
+            viewinst = view_func.view_class()
+            # Call the instance's before_request method
+            viewinst.before_request(view_func.__name__, **kwargs)
+            # Finally, call the view handler method
+            return view_func.wrapped_func(viewinst, *args, **kwargs)
+            # TODO: Support `after_request` as well. Note that it needs Response objects
+
+        # Decorate the view function with the class's desired decorators
+        wrapped_func = self.func
+        for decorator in cls.__decorators__:
+            wrapped_func = decorator(wrapped_func)
+
+        # Make view_func resemble the underlying view handler method
+        view_func = update_wrapper(view_func, wrapped_func)
+        # But give view_func the name of the method in the class (self.name),
+        # as this is important to the before_request method. self.name will
+        # differ from __name__ only if the view handler method was defined
+        # outside the class and then added to the class.
+        view_func.__name__ = self.name
+
+        # Stick `wrapped_func` and `cls` into view_func to avoid creating a closure.
+        view_func.wrapped_func = wrapped_func
         view_func.view_class = cls
 
         for class_rule, class_options in cls.__routes__:
@@ -127,8 +145,8 @@ def __call__(self, *args, **kwargs):
         """Treat this like a call to the method (and not to the view)"""
         return self.__viewd.func(self.__obj, *args, **kwargs)
 
-    def __getattr__(self, attr):
-        return getattr(self.__viewd, attr)
+    def __getattr__(self, name):
+        return getattr(self.__viewd, name)
 
 
 class ClassView(object):
@@ -151,9 +169,10 @@ def about():
 
         IndexView.init_app(app)
 
-    The :func:`route` decorator on the class specifies the base rule which is
+    The :func:`route` decorator on the class specifies the base rule, which is
     prefixed to the rule specified on each view method. This example produces
-    two view handlers, for ``/`` and ``/about``.
+    two view handlers, for ``/`` and ``/about``. Multiple :func:`route`
+    decorators may be used in both places.
 
     A rudimentary CRUD view collection can be assembled like this::
 
@@ -175,10 +194,21 @@ def edit(self, name, title, content):
 
         DocumentView.init_app(app)
 
-    See :class:`ModelView` (TODO) for a better way to build views around a model.
+    See :class:`ModelView` for a better way to build views around a model.
     """
     # If the class did not get a @route decorator, provide a fallback route
     __routes__ = [('', {})]
+    __decorators__ = []
+
+    def before_request(self, _view, **kwargs):
+        """
+        This method is called after the app's ``before_request`` handlers, and
+        before the class's view method. It receives the name of the view
+        method with all keyword arguments that will be sent to the view method
+        Subclasses and mixin classes may define their own
+        :meth:`before_request` to pre-process requests before the view method.
+        """
+        pass
 
     @classmethod
     def __get_raw_attr(cls, name):
@@ -190,8 +220,8 @@ def __get_raw_attr(cls, name):
     @classmethod
     def add_route_for(cls, _name, rule, **options):
         """
-        Add a route for an existing method or view in the class view. Useful
-        for modifying routes that a subclass inherits from a base class::
+        Add a route for an existing method or view. Useful for modifying routes
+        that a subclass inherits from a base class::
 
             class BaseView(ClassView):
                 def latent_view(self):
@@ -238,13 +268,116 @@ def init_app(cls, app, callback=None):
                     attr.init_app(app, cls, callback=callback)
 
 
+def _modelview_view_decorator(f):
+    @wraps(f)
+    def inner(self, **kwargs):
+        return f(self)
+    return inner
+
+
 class ModelView(ClassView):
     """
-    Base class for constructing views around a model. Provides assistance for:
+    Base class for constructing views around a model. Functionality is provided
+    via mixin classes that must precede :class:`ModelView` in base class order.
+    Two mixins are provided: :class:`UrlForView` and :class:`InstanceLoader`.
+    Sample use::
+
+        @route('/doc/<document>')
+        class DocumentView(UrlForView, InstanceLoader, ModelView):
+            model = Document
+            route_model_map = {
+                'document': 'name'
+                }
+
+            @route('')
+            @render_with(json=True)
+            def view(self):
+                return self.obj.current_access()
 
-    1. Loading instances based on URL parameters
-    2. Registering view handlers for Model.url_for() calls
+        DocumentView.init_app(app)
+
+    :class:`ModelView` makes one significant departure from :class:`ClassView`:
+    view handler methods no longer receive URL rule variables as keyword
+    parameters. They are placed at ``self.kwargs`` instead, as it is assumed
+    that the view handler method has no further use for them once
+    :meth:`loader` loads the instance.
+    """
+    __decorators__ = ClassView.__decorators__ + [_modelview_view_decorator]
+
+    #: The model that this view class represents, to be specified by subclasses.
+    model = None
+
+    #: A mapping of URL rule variables to attributes on the model. For example,
+    #: if the URL rule is ``/<parent>/<document>``, the attribute map can be::
+    #:
+    #:     route_model_map = {
+    #:         'document': 'name',
+    #:         'parent': 'parent.name',
+    #:         }
+    route_model_map = {}
+
+    def loader(self):  # pragma: no cover
+        """
+        Subclasses or mixin classes may override this method to provide a model
+        instance loader. The return value of this method will be placed at
+        ``self.obj``.
 
-    TODO
+        TODO: Consider allowing :meth:`loader` to place attributes on ``self``
+        by itself, to accommodate scenarios where multiple models need to be
+        loaded.
+        """
+        pass  # TODO: Maybe raise NotImplementedError?
+
+    def before_request(self, _view, **kwargs):
+        """
+        :class:`ModelView` overrides :meth:`~ClassView.before_request` to call
+        :meth:`loader`. Subclasses overriding this method must use
+        :func:`super` to ensure :meth:`loader` is called.
+        """
+        super(ModelView, self).before_request(_view, **kwargs)
+        self.kwargs = kwargs
+        self.obj = self.loader()
+
+
+class UrlForView(object):
+    """
+    Mixin class for :class:`ModelView` that registers view handler methods with
+    :class:`~coaster.sqlalchemy.mixins.UrlForMixin`'s
+    :meth:`~coaster.sqlalchemy.mixins.UrlForMixin.is_url_for`.
+    """
+    @classmethod
+    def init_app(cls, app, callback=None):
+        def register_view_on_model(rule, endpoint, view_func, **options):
+            # Only pass in the attrs that are included in the rule.
+            # 1. Extract list of variables from the rule
+            rulevars = (v for c, a, v in parse_rule(rule))
+            # Make a subset of cls.route_model_map with the required variables
+            params = {v: cls.route_model_map[v] for v in rulevars if v in cls.route_model_map}
+            # Hook up is_url_for with the view function's name, endpoint name and parameters
+            cls.model.is_url_for(view_func.__name__, endpoint, **params)(view_func)
+            if callback:
+                callback(rule, endpoint, view_func, **options)
+
+        super(ModelView, cls).init_app(app, register_view_on_model)
+
+
+class InstanceLoader(object):
+    """
+    Mixin class for :class:`ModelView` that provides a :meth:`loader` that
+    attempts to load an instance of the model based on attributes in the
+    :attr:`~ModelView.route_model_map` dictionary.
     """
-    pass  # TODO
+    def loader(self):
+        if any((name in self.route_model_map for name in self.kwargs)):
+            # We have a URL route attribute that matches one of the model's attributes.
+            # Attempt to load the model instance
+            filters = {self.route_model_map[key]: value
+                for key, value in self.kwargs.items()
+                if key in self.route_model_map}
+
+            # FIXME: filter keys may have periods to indicate sub-attributes.
+            # Instead of using `filter_by`, load attributes from the model using
+            # getattr and use `filter`. If we traverse a relationship to pick up
+            # an attribute from another model, we'll need a join with that model
+            # as well.
+            return self.model.query.filter_by(**filters).first_or_404()
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index 0f8e05b7..06fa8924 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -6,7 +6,7 @@
 from flask import Flask, json
 from coaster.sqlalchemy import BaseNameMixin, BaseScopedNameMixin
 from coaster.db import SQLAlchemy
-from coaster.views import ClassView, route, requestform, render_with
+from coaster.views import ClassView, ModelView, UrlForView, InstanceLoader, route, requestform, render_with
 
 
 app = Flask(__name__)
@@ -123,6 +123,33 @@ def second(self):
 AnotherSubView.init_app(app)
 
 
+@route('/model/<document>')
+class ModelDocumentView(UrlForView, InstanceLoader, ModelView):
+    model = ViewDocument
+    route_model_map = {
+        'document': 'name'
+        }
+
+    @route('')
+    @render_with(json=True)
+    def view(self):
+        return self.obj.current_access()
+
+    @route('edit', methods=['GET', 'POST'])
+    @route('', methods=['PUT'])
+    @render_with(json=True)
+    def edit(self):  # TODO
+        pass
+
+    @route('delete', methods=['GET', 'POST'])
+    @route('', methods=['DELETE'])
+    @render_with(json=True)
+    def delete(self):  # TODO
+        pass
+
+ModelDocumentView.init_app(app)
+
+
 # --- Tests -------------------------------------------------------------------
 
 class TestClassView(unittest.TestCase):
@@ -256,3 +283,23 @@ def test_second_subview_reroute(self):
         # Confirm we did not accidentally acquire this from SubView's use of reroute
         rv = self.client.get('/secondsub/2')
         assert rv.status_code == 404
+
+    def test_modelview_instanceloader_view(self):
+        """Test document view in ModelView with InstanceLoader"""
+        doc = ViewDocument(name='test1', title="Test")
+        self.session.add(doc)
+        self.session.commit()
+
+        rv = self.client.get('/model/test1')
+        assert rv.status_code == 200
+        data = json.loads(rv.data)
+        assert data['name'] == 'test1'
+        assert data['title'] == "Test"
+
+    def test_modelview_url_for(self):
+        """Test that ModelView provides model.is_url_for with appropriate parameters"""
+        doc1 = ViewDocument(name='test1', title="Test 1")
+        doc2 = ViewDocument(name='test2', title="Test 2")
+
+        assert doc1.url_for('view') == '/model/test1'
+        assert doc2.url_for('view') == '/model/test2'

From 7a1796286ef31e2d96e8c1165f4f0d9c6a02f306 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Thu, 15 Feb 2018 13:46:01 +0530
Subject: [PATCH 14/26] Support relationship traversal in InstanceLoader

---
 coaster/sqlalchemy/comparators.py | 13 +++++++++
 coaster/views/classview.py        | 48 +++++++++++++++++++++++++------
 tests/test_views_classview.py     | 39 ++++++++++++++++++++++++-
 3 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/coaster/sqlalchemy/comparators.py b/coaster/sqlalchemy/comparators.py
index b973bca0..b60da415 100644
--- a/coaster/sqlalchemy/comparators.py
+++ b/coaster/sqlalchemy/comparators.py
@@ -8,6 +8,7 @@
 from __future__ import absolute_import
 import uuid as uuid_
 from sqlalchemy.ext.hybrid import Comparator
+from flask import abort
 from flask_sqlalchemy import BaseQuery
 import six
 from ..utils import buid2uuid, suuid2uuid
@@ -38,6 +39,18 @@ def isempty(self):
         """
         return not self.session.query(self.exists()).scalar()
 
+    def one_or_404(self):
+        """
+        Extends :meth:`~sqlalchemy.orm.query.Query.one_or_none` to raise a 404
+        if no result is found. This method offers a safety net over
+        :meth:`~flask_sqlalchemy.BaseQuery.first_or_404` as it helps identify
+        poorly specified queries that could have returned more than one result.
+        """
+        result = self.one_or_none()
+        if not result:
+            abort(404)
+        return result
+
 
 class SplitIndexComparator(Comparator):
     """
diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index 2a7b659e..6414226a 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -10,6 +10,9 @@
 from __future__ import unicode_literals
 from functools import wraps, update_wrapper
 from werkzeug.routing import parse_rule
+from sqlalchemy.orm.attributes import InstrumentedAttribute
+from sqlalchemy.orm.mapper import Mapper
+from sqlalchemy.orm.properties import RelationshipProperty
 
 __all__ = ['route', 'ClassView', 'ModelView', 'UrlForView', 'InstanceLoader']
 
@@ -198,15 +201,18 @@ def edit(self, name, title, content):
     """
     # If the class did not get a @route decorator, provide a fallback route
     __routes__ = [('', {})]
+    #: Subclasses may define decorators here. These will be applied to every
+    #: view handler in the class, but only when called as a view and not
+    #: as a Python method call.
     __decorators__ = []
 
     def before_request(self, _view, **kwargs):
         """
         This method is called after the app's ``before_request`` handlers, and
         before the class's view method. It receives the name of the view
-        method with all keyword arguments that will be sent to the view method
+        method with all keyword arguments that will be sent to the view method.
         Subclasses and mixin classes may define their own
-        :meth:`before_request` to pre-process requests before the view method.
+        :meth:`before_request` to pre-process requests.
         """
         pass
 
@@ -306,6 +312,8 @@ def view(self):
 
     #: The model that this view class represents, to be specified by subclasses.
     model = None
+    #: A base query to use if the model needs special handling.
+    query = None
 
     #: A mapping of URL rule variables to attributes on the model. For example,
     #: if the URL rule is ``/<parent>/<document>``, the attribute map can be::
@@ -366,6 +374,9 @@ class InstanceLoader(object):
     Mixin class for :class:`ModelView` that provides a :meth:`loader` that
     attempts to load an instance of the model based on attributes in the
     :attr:`~ModelView.route_model_map` dictionary.
+
+    :class:`InstanceLoader` will traverse relationships (many-to-one or
+    one-to-one) and perform a SQL JOIN with the target class.
     """
     def loader(self):
         if any((name in self.route_model_map for name in self.kwargs)):
@@ -375,9 +386,30 @@ def loader(self):
                 for key, value in self.kwargs.items()
                 if key in self.route_model_map}
 
-            # FIXME: filter keys may have periods to indicate sub-attributes.
-            # Instead of using `filter_by`, load attributes from the model using
-            # getattr and use `filter`. If we traverse a relationship to pick up
-            # an attribute from another model, we'll need a join with that model
-            # as well.
-            return self.model.query.filter_by(**filters).first_or_404()
+            query = self.query or self.model.query
+            joined_models = set()
+            for name, value in filters.items():
+                if '.' in name:
+                    # Did we get something like `parent.name`?
+                    # Dig into it to find the source column
+                    source = self.model
+                    for subname in name.split('.'):
+                        source = getattr(source, subname)
+                        # Did we get to something like 'parent'? If it's a relationship, find
+                        # the source class, join it to the query, and then continue looking for
+                        # attributes over there
+                        if isinstance(source, InstrumentedAttribute):
+                            if isinstance(source.property, RelationshipProperty):
+                                if isinstance(source.property.argument, Mapper):
+                                    source = source.property.argument.class_
+                                else:
+                                    source = source.property.argument
+                                if source not in joined_models:
+                                    # SQL JOIN the other model
+                                    query = query.join(source)
+                                    # But ensure we don't JOIN twice
+                                    joined_models.add(source)
+                    query = query.filter(source == value)
+                else:
+                    query = query.filter(getattr(self.model, name) == value)
+            return query.one_or_404()
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index 06fa8924..b31670d5 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -32,6 +32,16 @@ class ScopedViewDocument(BaseScopedNameMixin, db.Model):
     parent_id = db.Column(None, db.ForeignKey('view_document.id'), nullable=False)
     parent = db.relationship(ViewDocument, backref=db.backref('children', cascade='all, delete-orphan'))
 
+    __roles__ = {
+        'all': {
+            'read': {'name', 'title', 'doctype'}
+            }
+        }
+
+    @property
+    def doctype(self):
+        return 'scoped-doc'
+
 
 # --- Views -------------------------------------------------------------------
 
@@ -127,7 +137,7 @@ def second(self):
 class ModelDocumentView(UrlForView, InstanceLoader, ModelView):
     model = ViewDocument
     route_model_map = {
-        'document': 'name'
+        'document': 'name',
         }
 
     @route('')
@@ -150,6 +160,17 @@ def delete(self):  # TODO
 ModelDocumentView.init_app(app)
 
 
+@route('/model/<parent>/<document>')
+class ScopedDocumentView(ModelDocumentView):
+    model = ScopedViewDocument
+    route_model_map = {
+        'document': 'name',
+        'parent': 'parent.name',
+        }
+
+ScopedDocumentView.init_app(app)
+
+
 # --- Tests -------------------------------------------------------------------
 
 class TestClassView(unittest.TestCase):
@@ -303,3 +324,19 @@ def test_modelview_url_for(self):
 
         assert doc1.url_for('view') == '/model/test1'
         assert doc2.url_for('view') == '/model/test2'
+
+    def test_scopedmodelview_view(self):
+        doc = ViewDocument(name='test1', title="Test 1")
+        sdoc = ScopedViewDocument(name='test2', title="Test 2", parent=doc)
+        self.session.add_all([doc, sdoc])
+        self.session.commit()
+
+        rv = self.client.get('/model/test1/test2')
+        assert rv.status_code == 200
+        data = json.loads(rv.data)
+        assert data['name'] == 'test2'
+        assert data['doctype'] == 'scoped-doc'
+
+        # The joined load actually worked
+        rv = self.client.get('/model/this-doc-does-not-exist/test2')
+        assert rv.status_code == 404

From 33195a7b8d74e2953cf59c52cf41ac26219683c0 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Tue, 20 Feb 2018 17:43:15 +0530
Subject: [PATCH 15/26] Added requires_permission decorator for current_auth

---
 CHANGES.rst                 |  1 +
 coaster/views/decorators.py | 30 ++++++++++++++++++++++++++-
 tests/test_views.py         | 41 +++++++++++++++++++++++++++++++++++--
 3 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index 9b4fe8d5..38ba5c4e 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -18,6 +18,7 @@
 * New: ``coaster.auth`` module with a ``current_auth`` proxy that provides
   a standardised API for login managers to use
 * New: ``is_collection`` util for testing if an item is a collection data type
+* New: ``coaster.views.requires_permission`` decorator
 * New: ``coaster.views.classview`` provides a new take on organising views
   into a class
 
diff --git a/coaster/views/decorators.py b/coaster/views/decorators.py
index 7699adbe..151a6c5d 100644
--- a/coaster/views/decorators.py
+++ b/coaster/views/decorators.py
@@ -25,7 +25,7 @@
     'RequestTypeError', 'RequestValueError',
     'requestargs', 'requestform', 'requestquery',
     'load_model', 'load_models',
-    'render_with', 'cors',
+    'render_with', 'cors', 'requires_permission',
     ]
 
 
@@ -594,3 +594,31 @@ def wrapper(*args, **kwargs):
             return resp
         return wrapper
     return inner
+
+
+def requires_permission(permission):
+    """
+    View decorator that requires a certain permission to be present in
+    ``current_auth.permissions`` before the view is allowed to proceed.
+    Aborts with ``403 Forbidden`` if the permission is not present.
+
+    :param permission: Permission that is required. If an iterable is provided,
+        any one permission must be available
+    """
+    def inner(f):
+        @wraps(f)
+        def wrapper(*args, **kwargs):
+            add_auth_attribute('login_required', True)
+            if not hasattr(current_auth, 'permissions'):
+                test = False
+            elif is_collection(permission):
+                test = bool(current_auth.permissions.intersection(permission))
+            else:
+                test = permission in current_auth.permissions
+            if not test:
+                abort(403)
+            return f(*args, **kwargs)
+
+        wrapper.requires_permission = permission
+        return wrapper
+    return inner
diff --git a/tests/test_views.py b/tests/test_views.py
index 11eb7338..6d4e8f30 100644
--- a/tests/test_views.py
+++ b/tests/test_views.py
@@ -1,10 +1,12 @@
 # -*- coding: utf-8 -*-
 
 import unittest
-from werkzeug.exceptions import BadRequest
+from werkzeug.exceptions import BadRequest, Forbidden
 from flask import Flask, session, json
 from coaster.app import load_config_from_file
-from coaster.views import get_current_url, get_next_url, jsonp, requestargs, requestquery, requestform
+from coaster.auth import current_auth, add_auth_attribute
+from coaster.views import (get_current_url, get_next_url, jsonp, requestargs, requestquery, requestform,
+    requires_permission)
 
 
 def index():
@@ -45,6 +47,18 @@ def requestcombo_test(query1, form1):
     return query1, form1
 
 
+@requires_permission('allow-this')
+def permission1():
+    return 'allowed1'
+
+
+@requires_permission({'allow-this', 'allow-that'})
+def permission2():
+    return 'allowed2'
+
+
+# --- Tests -------------------------------------------------------------------
+
 class TestCoasterViews(unittest.TestCase):
     def setUp(self):
         self.app = Flask(__name__)
@@ -134,3 +148,26 @@ def test_requestargs(self):
 
         # Calling without a request context works as well
         self.assertEqual(requestargs_test1(p1='1', p2=3, p3=[1, 2]), ('1', 3, [1, 2]))
+
+    def test_requires_permission(self):
+        with self.app.test_request_context():
+            with self.assertRaises(Forbidden):
+                permission1()
+            with self.assertRaises(Forbidden):
+                permission2()
+
+            add_auth_attribute('permissions', set())
+
+            with self.assertRaises(Forbidden):
+                permission1()
+            with self.assertRaises(Forbidden):
+                permission2()
+
+            current_auth.permissions.add('allow-that')  # FIXME! Shouldn't this be a frozenset?
+            with self.assertRaises(Forbidden):
+                permission1()
+            assert permission2() == 'allowed2'
+
+            current_auth.permissions.add('allow-this')
+            assert permission1() == 'allowed1'
+            assert permission2() == 'allowed2'

From 290749f20d7cbf2e1d5f1cf96bfc78f3cbc89d2c Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Tue, 20 Feb 2018 17:45:26 +0530
Subject: [PATCH 16/26] Fix PermissionMixin check

---
 coaster/sqlalchemy/mixins.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/coaster/sqlalchemy/mixins.py b/coaster/sqlalchemy/mixins.py
index 234e0b4c..d38800dc 100644
--- a/coaster/sqlalchemy/mixins.py
+++ b/coaster/sqlalchemy/mixins.py
@@ -627,8 +627,10 @@ def permissions(self, user, inherited=None):
         """
         if inherited is not None:
             return inherited | super(BaseScopedIdMixin, self).permissions(user)
-        else:
+        elif self.parent is not None and isinstance(self.parent, PermissionMixin):
             return self.parent.permissions(user) | super(BaseScopedIdMixin, self).permissions(user)
+        else:
+            return super(BaseScopedIdMixin, self).permissions(user)
 
 
 class BaseScopedIdNameMixin(BaseScopedIdMixin):

From 5e8b450789c948eb4b13020623b2695abf9cb2bf Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Tue, 20 Feb 2018 17:55:17 +0530
Subject: [PATCH 17/26] current_view proxy (also available in Jinja2 env)

---
 coaster/app.py                |  3 +++
 coaster/views/classview.py    | 12 ++++++++++--
 tests/test_views_classview.py | 11 ++++++++++-
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/coaster/app.py b/coaster/app.py
index f0f55f2d..28c3e430 100644
--- a/coaster/app.py
+++ b/coaster/app.py
@@ -17,6 +17,7 @@
     from flask.json import tojson_filter as _tojson_filter
 from . import logger
 from .auth import current_auth
+from .views import current_view
 
 __all__ = ['SandboxedFlask', 'init_app']
 
@@ -103,6 +104,8 @@ def init_app(app, env=None):
     """
     # Make current_auth available to app templates
     app.jinja_env.globals['current_auth'] = current_auth
+    # Make the current view available to app templates
+    app.jinja_env.globals['current_view'] = current_view
     # Disable Flask-SQLAlchemy events.
     # Apps that want it can turn it back on in their config
     app.config.setdefault('SQLALCHEMY_TRACK_MODIFICATIONS', False)
diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index 6414226a..e919562e 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -9,12 +9,18 @@
 
 from __future__ import unicode_literals
 from functools import wraps, update_wrapper
-from werkzeug.routing import parse_rule
 from sqlalchemy.orm.attributes import InstrumentedAttribute
 from sqlalchemy.orm.mapper import Mapper
 from sqlalchemy.orm.properties import RelationshipProperty
+from werkzeug.routing import parse_rule
+from werkzeug.local import LocalProxy
+from flask import _request_ctx_stack, has_request_context
+
+__all__ = ['route', 'current_view', 'ClassView', 'ModelView', 'UrlForView', 'InstanceLoader']
 
-__all__ = ['route', 'ClassView', 'ModelView', 'UrlForView', 'InstanceLoader']
+#: A proxy object that holds the currently executing :class:`ClassView` instance,
+#: for use in templates as context. Exposed to templates by :func:`coaster.app.init_app`.
+current_view = LocalProxy(lambda: has_request_context() and getattr(_request_ctx_stack.top, 'current_view', None))
 
 
 # :func:`route` wraps :class:`ViewDecorator` so that it can have an independent __doc__
@@ -103,6 +109,8 @@ def init_app(self, app, cls, callback=None):
         def view_func(*args, **kwargs):
             # Instantiate the view class. We depend on its __init__ requiring no parameters
             viewinst = view_func.view_class()
+            # Place it on the request stack for :obj:`current_view` to discover
+            _request_ctx_stack.top.current_view = viewinst
             # Call the instance's before_request method
             viewinst.before_request(view_func.__name__, **kwargs)
             # Finally, call the view handler method
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index b31670d5..de3c1fe9 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -6,7 +6,8 @@
 from flask import Flask, json
 from coaster.sqlalchemy import BaseNameMixin, BaseScopedNameMixin
 from coaster.db import SQLAlchemy
-from coaster.views import ClassView, ModelView, UrlForView, InstanceLoader, route, requestform, render_with
+from coaster.views import (ClassView, ModelView, UrlForView, InstanceLoader, route, requestform, render_with,
+    current_view)
 
 
 app = Flask(__name__)
@@ -55,6 +56,10 @@ def index(self):
     def page(self):
         return 'page'
 
+    @route('current_view')
+    def current_view_is_self(self):
+        return str(current_view == self)
+
 IndexView.init_app(app)
 
 
@@ -198,6 +203,10 @@ def test_page(self):
         rv = self.client.get('/page')
         assert rv.data == b'page'
 
+    def test_current_view(self):
+        rv = self.client.get('/current_view')
+        assert rv.data == b'True'
+
     def test_document_404(self):
         """Test 404 response from within a view"""
         rv = self.client.get('/doc/this-doc-does-not-exist')

From d364ea99287cd4b2ee18dda62e2db8f177f6cb7b Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 21 Feb 2018 01:55:41 +0530
Subject: [PATCH 18/26] Add support for permissions in InstanceLoader

---
 coaster/sqlalchemy/mixins.py  | 12 ++++++++-
 coaster/utils/classes.py      | 20 +++++++++++---
 coaster/views/classview.py    | 50 ++++++++++++++++++++++++++++-------
 tests/test_views_classview.py | 42 ++++++++++++++++++++++-------
 4 files changed, 100 insertions(+), 24 deletions(-)

diff --git a/coaster/sqlalchemy/mixins.py b/coaster/sqlalchemy/mixins.py
index d38800dc..7af666b8 100644
--- a/coaster/sqlalchemy/mixins.py
+++ b/coaster/sqlalchemy/mixins.py
@@ -31,7 +31,8 @@ class MyModel(BaseMixin, db.Model):
 from sqlalchemy_utils.types import UUIDType
 from flask import url_for
 import six
-from ..utils import make_name, uuid2suuid, uuid2buid, buid2uuid, suuid2uuid
+from ..utils import make_name, uuid2suuid, uuid2buid, buid2uuid, suuid2uuid, InspectableSet
+from ..auth import current_auth
 from .immutable_annotation import immutable
 from .roles import RoleMixin, with_roles
 from .comparators import Query, SqlSplitIdComparator, SqlHexUuidComparator, SqlBuidComparator, SqlSuuidComparator
@@ -225,6 +226,15 @@ def permissions(self, user, inherited=None):
         else:
             return set()
 
+    @property
+    def current_permissions(self):
+        """
+        :class:`~coaster.utils.classes.InspectableSet` containing currently
+        available permissions from this object, using
+        :obj:`~coaster.auth.current_auth`.
+        """
+        return InspectableSet(self.permissions(current_auth.actor))
+
 
 class UrlForMixin(object):
     """
diff --git a/coaster/utils/classes.py b/coaster/utils/classes.py
index 5e75c17e..c382f6ff 100644
--- a/coaster/utils/classes.py
+++ b/coaster/utils/classes.py
@@ -214,9 +214,10 @@ def value_for(cls, name):
 
 class InspectableSet(Set):
     """
-    Given a set, mimics a dictionary where the items are keys and have a value
-    of ``True``, and any other key has a value of ``False``. Also supports
-    attribute access. Useful in templates to simplify membership inspection::
+    Given a set, mimics a read-only dictionary where the items are keys and
+    have a value of ``True``, and any other key has a value of ``False``. Also
+    supports attribute access. Useful in templates to simplify membership
+    inspection::
 
         >>> myset = InspectableSet({'member', 'other'})
         >>> 'member' in myset
@@ -231,6 +232,19 @@ class InspectableSet(Set):
         True
         >>> myset['random']
         False
+        >>> joinset = myset | {'added'}
+        >>> isinstance(joinset, InspectableSet)
+        True
+        >>> joinset = joinset | InspectableSet({'inspectable'})
+        >>> isinstance(joinset, InspectableSet)
+        >>> 'member' in joinset
+        True
+        >>> 'other' in joinset
+        True
+        >>> 'added' in joinset
+        True
+        >>> 'inspectable' in joinset
+        True
     """
     def __init__(self, members):
         if not isinstance(members, set):
diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index e919562e..2de509ba 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -15,6 +15,9 @@
 from werkzeug.routing import parse_rule
 from werkzeug.local import LocalProxy
 from flask import _request_ctx_stack, has_request_context
+from ..auth import current_auth, add_auth_attribute
+from ..utils import InspectableSet
+from ..sqlalchemy import PermissionMixin
 
 __all__ = ['route', 'current_view', 'ClassView', 'ModelView', 'UrlForView', 'InstanceLoader']
 
@@ -112,7 +115,11 @@ def view_func(*args, **kwargs):
             # Place it on the request stack for :obj:`current_view` to discover
             _request_ctx_stack.top.current_view = viewinst
             # Call the instance's before_request method
-            viewinst.before_request(view_func.__name__, **kwargs)
+            result = viewinst.before_request(view_func.__name__, kwargs)
+            # Did the before_request handler return something? Assume it wants
+            # to interrupt the view and return it
+            if result is not None:
+                return result
             # Finally, call the view handler method
             return view_func.wrapped_func(viewinst, *args, **kwargs)
             # TODO: Support `after_request` as well. Note that it needs Response objects
@@ -214,13 +221,18 @@ def edit(self, name, title, content):
     #: as a Python method call.
     __decorators__ = []
 
-    def before_request(self, _view, **kwargs):
+    def before_request(self, view, kwargs):
         """
         This method is called after the app's ``before_request`` handlers, and
         before the class's view method. It receives the name of the view
         method with all keyword arguments that will be sent to the view method.
         Subclasses and mixin classes may define their own
         :meth:`before_request` to pre-process requests.
+
+        :param str view: The name of the view handler that will be called. The
+            view itself can be retrieved as ``getattr(self, view)``
+        :param dict kwargs: Parameters that will be sent to the view handler.
+            These are typically parameters from the URL rule
         """
         pass
 
@@ -282,7 +294,7 @@ def init_app(cls, app, callback=None):
                     attr.init_app(app, cls, callback=callback)
 
 
-def _modelview_view_decorator(f):
+def _modelview_view_decorator_remove_kwargs(f):
     @wraps(f)
     def inner(self, **kwargs):
         return f(self)
@@ -316,7 +328,7 @@ def view(self):
     that the view handler method has no further use for them once
     :meth:`loader` loads the instance.
     """
-    __decorators__ = ClassView.__decorators__ + [_modelview_view_decorator]
+    __decorators__ = ClassView.__decorators__ + [_modelview_view_decorator_remove_kwargs]
 
     #: The model that this view class represents, to be specified by subclasses.
     model = None
@@ -332,7 +344,7 @@ def view(self):
     #:         }
     route_model_map = {}
 
-    def loader(self):  # pragma: no cover
+    def loader(self, view):  # pragma: no cover
         """
         Subclasses or mixin classes may override this method to provide a model
         instance loader. The return value of this method will be placed at
@@ -344,15 +356,25 @@ def loader(self):  # pragma: no cover
         """
         pass  # TODO: Maybe raise NotImplementedError?
 
-    def before_request(self, _view, **kwargs):
+    def after_load(self, view):  # pragma: no cover
+        """
+        Subclasses or mixin classes may override this method to receive a call
+        after :meth:`loader` is called, to examine the loaded object. If this
+        method returns something, it will be used as the view's result and the
+        view itself will not be called.
+        """
+        pass
+
+    def before_request(self, view, kwargs):
         """
         :class:`ModelView` overrides :meth:`~ClassView.before_request` to call
         :meth:`loader`. Subclasses overriding this method must use
         :func:`super` to ensure :meth:`loader` is called.
         """
-        super(ModelView, self).before_request(_view, **kwargs)
+        super(ModelView, self).before_request(view, kwargs)
         self.kwargs = kwargs
-        self.obj = self.loader()
+        self.obj = self.loader(view)
+        return self.after_load(view)
 
 
 class UrlForView(object):
@@ -386,7 +408,7 @@ class InstanceLoader(object):
     :class:`InstanceLoader` will traverse relationships (many-to-one or
     one-to-one) and perform a SQL JOIN with the target class.
     """
-    def loader(self):
+    def loader(self, view):
         if any((name in self.route_model_map for name in self.kwargs)):
             # We have a URL route attribute that matches one of the model's attributes.
             # Attempt to load the model instance
@@ -420,4 +442,12 @@ def loader(self):
                     query = query.filter(source == value)
                 else:
                     query = query.filter(getattr(self.model, name) == value)
-            return query.one_or_404()
+            obj = query.one_or_404()
+            # Determine permissions available on the object for the current actor,
+            # but only if the view method has a requires_permission decorator
+            if hasattr(getattr(self, view).func, 'requires_permission') and isinstance(obj, PermissionMixin):
+                perms = obj.current_permissions
+                if hasattr(current_auth, 'permissions'):
+                    perms = perms | current_auth.permissions
+                add_auth_attribute('permissions', perms)
+            return obj
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index de3c1fe9..09d392a4 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -5,9 +5,10 @@
 import unittest
 from flask import Flask, json
 from coaster.sqlalchemy import BaseNameMixin, BaseScopedNameMixin
+from coaster.auth import add_auth_attribute
 from coaster.db import SQLAlchemy
-from coaster.views import (ClassView, ModelView, UrlForView, InstanceLoader, route, requestform, render_with,
-    current_view)
+from coaster.views import (ClassView, ModelView, UrlForView, InstanceLoader, route, requestargs, requestform,
+    render_with, current_view, requires_permission)
 
 
 app = Flask(__name__)
@@ -27,6 +28,13 @@ class ViewDocument(BaseNameMixin, db.Model):
             }
         }
 
+    def permissions(self, user, inherited=()):
+        perms = super(ViewDocument, self).permissions(user, inherited)
+        if user == 'this-is-the-owner':  # Our hack of a user object, for testing
+            perms.add('edit')
+            perms.add('delete')
+        return perms
+
 
 class ScopedViewDocument(BaseScopedNameMixin, db.Model):
     __tablename__ = 'scoped_view_document'
@@ -145,6 +153,12 @@ class ModelDocumentView(UrlForView, InstanceLoader, ModelView):
         'document': 'name',
         }
 
+    @requestargs('access_token')
+    def before_request(self, view, kwargs, access_token=None):
+        if access_token == 'owner-secret':
+            add_auth_attribute('user', 'this-is-the-owner')  # See ViewDocument.permissions
+        return super(ModelDocumentView, self).before_request(view, kwargs)
+
     @route('')
     @render_with(json=True)
     def view(self):
@@ -152,15 +166,10 @@ def view(self):
 
     @route('edit', methods=['GET', 'POST'])
     @route('', methods=['PUT'])
-    @render_with(json=True)
-    def edit(self):  # TODO
-        pass
+    @requires_permission('edit')
+    def edit(self):
+        return 'edit-called'
 
-    @route('delete', methods=['GET', 'POST'])
-    @route('', methods=['DELETE'])
-    @render_with(json=True)
-    def delete(self):  # TODO
-        pass
 
 ModelDocumentView.init_app(app)
 
@@ -326,6 +335,19 @@ def test_modelview_instanceloader_view(self):
         assert data['name'] == 'test1'
         assert data['title'] == "Test"
 
+    def test_modelview_instanceloader_requires_permission_edit(self):
+        """Test document edit in ModelView with InstanceLoader and requires_permission"""
+        doc = ViewDocument(name='test1', title="Test")
+        self.session.add(doc)
+        self.session.commit()
+
+        rv = self.client.post('/model/test1/edit')
+        assert rv.status_code == 403
+        rv = self.client.post('/model/test1/edit?access_token=owner-secret')
+        assert rv.status_code == 200
+        assert rv.data == b'edit-called'
+
+
     def test_modelview_url_for(self):
         """Test that ModelView provides model.is_url_for with appropriate parameters"""
         doc1 = ViewDocument(name='test1', title="Test 1")

From 3a51e9c91e6b5eec73ab1e90193042dcfc4283a8 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Wed, 21 Feb 2018 01:57:22 +0530
Subject: [PATCH 19/26] Fix doctest for InspectableSet

---
 coaster/utils/classes.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/coaster/utils/classes.py b/coaster/utils/classes.py
index c382f6ff..2fecd0df 100644
--- a/coaster/utils/classes.py
+++ b/coaster/utils/classes.py
@@ -237,6 +237,7 @@ class InspectableSet(Set):
         True
         >>> joinset = joinset | InspectableSet({'inspectable'})
         >>> isinstance(joinset, InspectableSet)
+        True
         >>> 'member' in joinset
         True
         >>> 'other' in joinset

From 7ec7561cff4629b1b31bff55849abbbfa1f8d7df Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Thu, 22 Feb 2018 16:11:13 +0530
Subject: [PATCH 20/26] Tweak ModelView for better sanity
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* ModelView no longer strips the view’s keyword arguments
* Mixin classes can add their own __decorators__
* before_request can no longer override the view response. Use a
decorator instead
* Consequently, there’s no longer an after_load method
---
 coaster/views/classview.py    | 66 +++++++++++++----------------------
 tests/test_views_classview.py |  4 +--
 2 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index 2de509ba..2722b656 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -8,7 +8,7 @@
 """
 
 from __future__ import unicode_literals
-from functools import wraps, update_wrapper
+from functools import update_wrapper
 from sqlalchemy.orm.attributes import InstrumentedAttribute
 from sqlalchemy.orm.mapper import Mapper
 from sqlalchemy.orm.properties import RelationshipProperty
@@ -16,10 +16,9 @@
 from werkzeug.local import LocalProxy
 from flask import _request_ctx_stack, has_request_context
 from ..auth import current_auth, add_auth_attribute
-from ..utils import InspectableSet
 from ..sqlalchemy import PermissionMixin
 
-__all__ = ['route', 'current_view', 'ClassView', 'ModelView', 'UrlForView', 'InstanceLoader']
+__all__ = ['route', 'rulejoin', 'current_view', 'ClassView', 'ModelView', 'UrlForView', 'InstanceLoader']
 
 #: A proxy object that holds the currently executing :class:`ClassView` instance,
 #: for use in templates as context. Exposed to templates by :func:`coaster.app.init_app`.
@@ -38,7 +37,9 @@ def route(rule, **options):
 
 def rulejoin(class_rule, method_rule):
     """
-    Join class and method rules::
+    Join class and method rules. Used internally by :class:`ClassView` to
+    combine rules from the :func:`route` decorators on the class and on the
+    individual view handler methods::
 
         >>> rulejoin('/', '')
         '/'
@@ -52,6 +53,8 @@ def rulejoin(class_rule, method_rule):
         '/first/second'
         >>> rulejoin('/first/<second>', '')
         '/first/<second>'
+        >>> rulejoin('/first/<second>', 'third')
+        '/first/<second>/third'
     """
     if method_rule.startswith('/'):
         return method_rule
@@ -106,32 +109,28 @@ def init_app(self, app, cls, callback=None):
         """
         Register routes for a given app and ClassView subclass
         """
-        # Revisit endpoint to account for subclasses
-        endpoint = cls.__name__ + '_' + self.name
-
         def view_func(*args, **kwargs):
             # Instantiate the view class. We depend on its __init__ requiring no parameters
             viewinst = view_func.view_class()
             # Place it on the request stack for :obj:`current_view` to discover
             _request_ctx_stack.top.current_view = viewinst
             # Call the instance's before_request method
-            result = viewinst.before_request(view_func.__name__, kwargs)
-            # Did the before_request handler return something? Assume it wants
-            # to interrupt the view and return it
-            if result is not None:
-                return result
+            viewinst.before_request(view_func.__name__, kwargs)
             # Finally, call the view handler method
             return view_func.wrapped_func(viewinst, *args, **kwargs)
             # TODO: Support `after_request` as well. Note that it needs Response objects
 
-        # Decorate the view function with the class's desired decorators
+        # Decorate the wrapped view function with the class's desired decorators.
+        # Mixin classes may provide their own decorators, and all of them will be applied.
         wrapped_func = self.func
-        for decorator in cls.__decorators__:
-            wrapped_func = decorator(wrapped_func)
+        for base in cls.__mro__:
+            if '__decorators__' in base.__dict__:
+                for decorator in base.__dict__['__decorators__']:
+                    wrapped_func = decorator(wrapped_func)
 
-        # Make view_func resemble the underlying view handler method
+        # Make view_func resemble the underlying view handler method...
         view_func = update_wrapper(view_func, wrapped_func)
-        # But give view_func the name of the method in the class (self.name),
+        # ...but give view_func the name of the method in the class (self.name),
         # as this is important to the before_request method. self.name will
         # differ from __name__ only if the view handler method was defined
         # outside the class and then added to the class.
@@ -141,6 +140,9 @@ def view_func(*args, **kwargs):
         view_func.wrapped_func = wrapped_func
         view_func.view_class = cls
 
+        # Revisit endpoint to account for subclasses
+        endpoint = cls.__name__ + '_' + self.name
+
         for class_rule, class_options in cls.__routes__:
             for method_rule, method_options in self.routes:
                 use_options = dict(method_options)
@@ -294,13 +296,6 @@ def init_app(cls, app, callback=None):
                     attr.init_app(app, cls, callback=callback)
 
 
-def _modelview_view_decorator_remove_kwargs(f):
-    @wraps(f)
-    def inner(self, **kwargs):
-        return f(self)
-    return inner
-
-
 class ModelView(ClassView):
     """
     Base class for constructing views around a model. Functionality is provided
@@ -317,18 +312,17 @@ class DocumentView(UrlForView, InstanceLoader, ModelView):
 
             @route('')
             @render_with(json=True)
-            def view(self):
+            def view(self, **kwargs):
                 return self.obj.current_access()
 
         DocumentView.init_app(app)
 
-    :class:`ModelView` makes one significant departure from :class:`ClassView`:
-    view handler methods no longer receive URL rule variables as keyword
-    parameters. They are placed at ``self.kwargs`` instead, as it is assumed
-    that the view handler method has no further use for them once
-    :meth:`loader` loads the instance.
+    :class:`ModelView` keeps a copy of the view's keyword arguments (passed in
+    by Flask from the URL rule parameters) in :attr:`kwargs`. :meth:`loader`
+    implementations are expected to use this to determine what to load. Views
+    will continue to receive keyword arguments as in :class:`ClassView`, but
+    typically will have no use for them if :meth:`loader` works as expected.
     """
-    __decorators__ = ClassView.__decorators__ + [_modelview_view_decorator_remove_kwargs]
 
     #: The model that this view class represents, to be specified by subclasses.
     model = None
@@ -356,15 +350,6 @@ def loader(self, view):  # pragma: no cover
         """
         pass  # TODO: Maybe raise NotImplementedError?
 
-    def after_load(self, view):  # pragma: no cover
-        """
-        Subclasses or mixin classes may override this method to receive a call
-        after :meth:`loader` is called, to examine the loaded object. If this
-        method returns something, it will be used as the view's result and the
-        view itself will not be called.
-        """
-        pass
-
     def before_request(self, view, kwargs):
         """
         :class:`ModelView` overrides :meth:`~ClassView.before_request` to call
@@ -374,7 +359,6 @@ def before_request(self, view, kwargs):
         super(ModelView, self).before_request(view, kwargs)
         self.kwargs = kwargs
         self.obj = self.loader(view)
-        return self.after_load(view)
 
 
 class UrlForView(object):
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index 09d392a4..543db404 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -161,13 +161,13 @@ def before_request(self, view, kwargs, access_token=None):
 
     @route('')
     @render_with(json=True)
-    def view(self):
+    def view(self, **kwargs):
         return self.obj.current_access()
 
     @route('edit', methods=['GET', 'POST'])
     @route('', methods=['PUT'])
     @requires_permission('edit')
-    def edit(self):
+    def edit(self, **kwargs):
         return 'edit-called'
 
 

From e70cf51e63e9decf21e95b47e93947a858ecff5d Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Thu, 22 Feb 2018 17:53:36 +0530
Subject: [PATCH 21/26] Fix url_name_suuid breakage with RoleMixin's proxy

---
 coaster/sqlalchemy/mixins.py | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/coaster/sqlalchemy/mixins.py b/coaster/sqlalchemy/mixins.py
index 7af666b8..8f1b74c7 100644
--- a/coaster/sqlalchemy/mixins.py
+++ b/coaster/sqlalchemy/mixins.py
@@ -549,7 +549,7 @@ def __init__(self, *args, **kw):
             self.make_name()
 
     def __repr__(self):
-        return '<%s %s "%s">' % (self.__class__.__name__, self.url_name, self.title)
+        return '<%s %s "%s">' % (self.__class__.__name__, self.url_id_name, self.title)
 
     def make_name(self):
         """Autogenerates a :attr:`name` from the :attr:`title`"""
@@ -584,11 +584,17 @@ def url_name_suuid(self):
         Returns a URL name combining :attr:`name` and :attr:`suuid` in name-suuid syntax.
         To use this, the class must derive from :class:`UuidMixin`.
         """
-        return '%s-%s' % (self.name, self.suuid)
+        if isinstance(self, UuidMixin):
+            return '%s-%s' % (self.name, self.suuid)
+        else:
+            return '%s-%s' % (self.name, self.id)
 
     @url_name_suuid.comparator
     def url_name_suuid(cls):
-        return SqlSuuidComparator(cls.uuid, splitindex=-1)
+        if issubclass(cls, UuidMixin):
+            return SqlSuuidComparator(cls.uuid, splitindex=-1)
+        else:
+            return SqlSplitIdComparator(cls.id, splitindex=-1)
 
 
 class BaseScopedIdMixin(BaseMixin):
@@ -697,7 +703,7 @@ def __init__(self, *args, **kw):
             self.make_name()
 
     def __repr__(self):
-        return '<%s %s "%s" of %s>' % (self.__class__.__name__, self.url_name, self.title,
+        return '<%s %s "%s" of %s>' % (self.__class__.__name__, self.url_id_name, self.title,
             repr(self.parent)[1:-1] if self.parent else None)
 
     @classmethod

From 43ec78eea1c18b10c257946e3e26b2c5d1d9fa95 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Thu, 22 Feb 2018 17:54:36 +0530
Subject: [PATCH 22/26] Misc cleanup in StateManager

---
 coaster/sqlalchemy/statemanager.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/coaster/sqlalchemy/statemanager.py b/coaster/sqlalchemy/statemanager.py
index eb79e8ef..de2f75e5 100644
--- a/coaster/sqlalchemy/statemanager.py
+++ b/coaster/sqlalchemy/statemanager.py
@@ -456,6 +456,9 @@ def add_transition(self, statemanager, from_, to, if_=None, data=None):
             'if': if_,             # Additional conditions that must ALL pass
             }
 
+    def __set_name__(self, owner, name):  # pragma: no cover
+        self.name = name
+
     # Make the transition a non-data descriptor
     def __get__(self, obj, cls=None):
         if obj is None:
@@ -501,6 +504,9 @@ def is_available(self):
         """
         return not self._state_invalid()
 
+    def __getattr__(self, name):
+        return getattr(self.statetransition, name)
+
     def __call__(self, *args, **kwargs):
         """Call the transition"""
         # Validate that each of the state managers is in the correct state
@@ -827,17 +833,17 @@ def group(self, items, keep_empty=False):
                     del groups[key]
         return groups
 
-    def __getattr__(self, attr):
+    def __getattr__(self, name):
         """
         Given the name of a state, returns:
 
-        1. If called on an instance, a boolean indicating if the state is active
+        1. If called on an instance, a ManagedStateWrapper, which implements __bool__
         2. If called on a class, a query filter
 
         Returns the default value or raises :exc:`AttributeError` on anything else.
         """
-        if hasattr(self.statemanager, attr):
-            mstate = getattr(self.statemanager, attr)
+        if hasattr(self.statemanager, name):
+            mstate = getattr(self.statemanager, name)
             if isinstance(mstate, (ManagedState, ManagedStateGroup)):
                 return mstate(self.obj, self.cls)
-        raise AttributeError("Not a state: %s" % attr)
+        raise AttributeError("Not a state: %s" % name)

From a54d23d41758c9e627106d4e9828d7074417634e Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Thu, 22 Feb 2018 18:14:39 +0530
Subject: [PATCH 23/26] Added url_change_check decorator and mixin class

---
 coaster/views/classview.py    | 82 ++++++++++++++++++++++++++++++++---
 tests/test_views_classview.py | 62 ++++++++++++++++++++++++--
 2 files changed, 133 insertions(+), 11 deletions(-)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index 2722b656..e94bff12 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -8,17 +8,22 @@
 """
 
 from __future__ import unicode_literals
-from functools import update_wrapper
+from functools import wraps, update_wrapper
 from sqlalchemy.orm.attributes import InstrumentedAttribute
 from sqlalchemy.orm.mapper import Mapper
 from sqlalchemy.orm.properties import RelationshipProperty
 from werkzeug.routing import parse_rule
 from werkzeug.local import LocalProxy
-from flask import _request_ctx_stack, has_request_context
+from flask import _request_ctx_stack, has_request_context, request, redirect
 from ..auth import current_auth, add_auth_attribute
 from ..sqlalchemy import PermissionMixin
 
-__all__ = ['route', 'rulejoin', 'current_view', 'ClassView', 'ModelView', 'UrlForView', 'InstanceLoader']
+__all__ = [
+    'route', 'rulejoin', 'current_view',  # Functions
+    'ClassView', 'ModelView',  # View base classes
+    'url_change_check',  # View decorators
+    'UrlForView', 'InstanceLoader', 'UrlChangeCheck'  # Mixin classes
+    ]
 
 #: A proxy object that holds the currently executing :class:`ClassView` instance,
 #: for use in templates as context. Exposed to templates by :func:`coaster.app.init_app`.
@@ -135,6 +140,13 @@ def view_func(*args, **kwargs):
         # differ from __name__ only if the view handler method was defined
         # outside the class and then added to the class.
         view_func.__name__ = self.name
+        # Note: While this fixes for external inspection, the wrappers we just
+        # applied will continue to have self.func.__name__, which will break them
+        # if they depend on it and if the method was defined outside the class
+        # with a different name from what it has inside the class. Ideally,
+        # the programmer should correct for this by always correcting __name__
+        # before adding it to the class. See url_change_check below for an
+        # exampe of how this may happen
 
         # Stick `wrapped_func` and `cls` into view_func to avoid creating a closure.
         view_func.wrapped_func = wrapped_func
@@ -358,6 +370,7 @@ def before_request(self, view, kwargs):
         """
         super(ModelView, self).before_request(view, kwargs)
         self.kwargs = kwargs
+        self.view_name = view
         self.obj = self.loader(view)
 
 
@@ -377,10 +390,65 @@ def register_view_on_model(rule, endpoint, view_func, **options):
             params = {v: cls.route_model_map[v] for v in rulevars if v in cls.route_model_map}
             # Hook up is_url_for with the view function's name, endpoint name and parameters
             cls.model.is_url_for(view_func.__name__, endpoint, **params)(view_func)
-            if callback:
+            if callback:  # pragma: no cover
                 callback(rule, endpoint, view_func, **options)
 
-        super(ModelView, cls).init_app(app, register_view_on_model)
+        super(ModelView, cls).init_app(app, callback=register_view_on_model)
+
+
+def url_change_check(f):
+    """
+    View method decorator that checks the URL of the loaded object in
+    ``self.obj`` against the URL in the request (using
+    ``self.obj.url_for(__name__)``). If the URLs do not match,
+    and the request is a ``GET``, it issues a redirect to the correct URL.
+    Usage::
+
+        @route('/doc/<document>')
+        class MyModelView(UrlForView, InstanceLoader, ModelView):
+            model = MyModel
+            route_model_map = {'document': 'url_id_name'}
+
+            @route('')
+            @url_change_check
+            @render_with(json=True)
+            def view(self, **kwargs):
+                return self.obj.current_access()
+
+    If the decorator is required for all view handlers in the class, use
+    :class:`UrlChangeCheck`.
+    """
+    @wraps(f)
+    def wrapper(self, *args, **kwargs):
+        if request.method == 'GET' and self.obj is not None:
+            correct_url = self.obj.url_for(f.__name__, _external=True)
+            if correct_url != request.base_url:
+                if request.query_string:
+                    correct_url = correct_url + '?' + request.query_string.decode()
+                return redirect(correct_url)  # TODO: Decide if this should be 302 (default) or 301
+        return f(self, *args, **kwargs)
+    return wrapper
+
+
+class UrlChangeCheck(UrlForView):
+    """
+    Mixin class for :class:`ModelView` and
+    :class:`~coaster.sqlalchemy.mixins.UrlForMixin` that applies the
+    :func:`url_change_check` decorator to all view handler methods. Subclasses
+    :class:`UrlForView`, which it depends on to register the view with the
+    model so that URLs can be generated. Usage::
+
+        @route('/doc/<document>')
+        class MyModelView(UrlChangeCheck, InstanceLoader, ModelView):
+            model = MyModel
+            route_model_map = {'document': 'url_id_name'}
+
+            @route('')
+            @render_with(json=True)
+            def view(self, **kwargs):
+                return self.obj.current_access()
+    """
+    __decorators__ = [url_change_check]
 
 
 class InstanceLoader(object):
@@ -390,7 +458,7 @@ class InstanceLoader(object):
     :attr:`~ModelView.route_model_map` dictionary.
 
     :class:`InstanceLoader` will traverse relationships (many-to-one or
-    one-to-one) and perform a SQL JOIN with the target class.
+    one-to-one) and perform a SQL ``JOIN`` with the target class.
     """
     def loader(self, view):
         if any((name in self.route_model_map for name in self.kwargs)):
@@ -415,7 +483,7 @@ def loader(self, view):
                         if isinstance(source, InstrumentedAttribute):
                             if isinstance(source.property, RelationshipProperty):
                                 if isinstance(source.property.argument, Mapper):
-                                    source = source.property.argument.class_
+                                    source = source.property.argument.class_  # Unlikely to be used. pragma: no cover
                                 else:
                                     source = source.property.argument
                                 if source not in joined_models:
diff --git a/tests/test_views_classview.py b/tests/test_views_classview.py
index 543db404..9f7da289 100644
--- a/tests/test_views_classview.py
+++ b/tests/test_views_classview.py
@@ -4,11 +4,12 @@
 
 import unittest
 from flask import Flask, json
-from coaster.sqlalchemy import BaseNameMixin, BaseScopedNameMixin
+from coaster.sqlalchemy import BaseNameMixin, BaseScopedNameMixin, BaseIdNameMixin
 from coaster.auth import add_auth_attribute
+from coaster.utils import InspectableSet
 from coaster.db import SQLAlchemy
-from coaster.views import (ClassView, ModelView, UrlForView, InstanceLoader, route, requestargs, requestform,
-    render_with, current_view, requires_permission)
+from coaster.views import (ClassView, ModelView, UrlForView, UrlChangeCheck, InstanceLoader,
+    route, requestargs, requestform, render_with, current_view, requires_permission)
 
 
 app = Flask(__name__)
@@ -52,6 +53,16 @@ def doctype(self):
         return 'scoped-doc'
 
 
+class RenameableDocument(BaseIdNameMixin, db.Model):
+    __tablename__ = 'renameable_document'
+    __uuid_primary_key__ = False  # So that we can get consistent `1-<name>` url_name in tests
+    __roles__ = {
+        'all': {
+            'read': {'name', 'title'}
+            }
+        }
+
+
 # --- Views -------------------------------------------------------------------
 
 @route('/')
@@ -155,6 +166,9 @@ class ModelDocumentView(UrlForView, InstanceLoader, ModelView):
 
     @requestargs('access_token')
     def before_request(self, view, kwargs, access_token=None):
+        if access_token == 'owner-admin-secret':
+            add_auth_attribute('permissions', InspectableSet({'siteadmin'}))
+            add_auth_attribute('user', 'this-is-the-owner')  # See ViewDocument.permissions
         if access_token == 'owner-secret':
             add_auth_attribute('user', 'this-is-the-owner')  # See ViewDocument.permissions
         return super(ModelDocumentView, self).before_request(view, kwargs)
@@ -185,6 +199,21 @@ class ScopedDocumentView(ModelDocumentView):
 ScopedDocumentView.init_app(app)
 
 
+@route('/rename/<document>')
+class RenameableDocumentView(UrlChangeCheck, InstanceLoader, ModelView):
+    model = RenameableDocument
+    route_model_map = {
+        'document': 'url_name',
+        }
+
+    @route('')
+    @render_with(json=True)
+    def view(self, **kwargs):
+        return self.obj.current_access()
+
+RenameableDocumentView.init_app(app)
+
+
 # --- Tests -------------------------------------------------------------------
 
 class TestClassView(unittest.TestCase):
@@ -346,7 +375,9 @@ def test_modelview_instanceloader_requires_permission_edit(self):
         rv = self.client.post('/model/test1/edit?access_token=owner-secret')
         assert rv.status_code == 200
         assert rv.data == b'edit-called'
-
+        rv = self.client.post('/model/test1/edit?access_token=owner-admin-secret')
+        assert rv.status_code == 200
+        assert rv.data == b'edit-called'
 
     def test_modelview_url_for(self):
         """Test that ModelView provides model.is_url_for with appropriate parameters"""
@@ -357,6 +388,7 @@ def test_modelview_url_for(self):
         assert doc2.url_for('view') == '/model/test2'
 
     def test_scopedmodelview_view(self):
+        """Test that InstanceLoader in a scoped model correctly loads parent"""
         doc = ViewDocument(name='test1', title="Test 1")
         sdoc = ScopedViewDocument(name='test2', title="Test 2", parent=doc)
         self.session.add_all([doc, sdoc])
@@ -371,3 +403,25 @@ def test_scopedmodelview_view(self):
         # The joined load actually worked
         rv = self.client.get('/model/this-doc-does-not-exist/test2')
         assert rv.status_code == 404
+
+    def test_redirectablemodel_view(self):
+        doc = RenameableDocument(name='test1', title="Test 1")
+        self.session.add(doc)
+        self.session.commit()
+
+        rv = self.client.get('/rename/1-test1')
+        assert rv.status_code == 200
+        data = json.loads(rv.data)
+        assert data['name'] == 'test1'
+
+        doc.name = 'renamed'
+        self.session.commit()
+
+        rv = self.client.get('/rename/1-test1?preserve=this')
+        assert rv.status_code == 302
+        assert rv.location == 'http://localhost/rename/1-renamed?preserve=this'
+
+        rv = self.client.get('/rename/1-renamed')
+        assert rv.status_code == 200
+        data = json.loads(rv.data)
+        assert data['name'] == 'renamed'

From ec3c4029961858969addee571bbad3c0026fcb71 Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Thu, 22 Feb 2018 18:31:25 +0530
Subject: [PATCH 24/26] Fix url_name_suuid+test

---
 coaster/sqlalchemy/mixins.py | 4 +++-
 tests/test_models.py         | 5 ++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/coaster/sqlalchemy/mixins.py b/coaster/sqlalchemy/mixins.py
index 8f1b74c7..cb78833c 100644
--- a/coaster/sqlalchemy/mixins.py
+++ b/coaster/sqlalchemy/mixins.py
@@ -587,12 +587,14 @@ def url_name_suuid(self):
         if isinstance(self, UuidMixin):
             return '%s-%s' % (self.name, self.suuid)
         else:
-            return '%s-%s' % (self.name, self.id)
+            return '%s-%s' % (self.name, self.url_id)
 
     @url_name_suuid.comparator
     def url_name_suuid(cls):
         if issubclass(cls, UuidMixin):
             return SqlSuuidComparator(cls.uuid, splitindex=-1)
+        elif cls.__uuid_primary_key__:
+            return SqlHexUuidComparator(cls.id, splitindex=-1)
         else:
             return SqlSplitIdComparator(cls.id, splitindex=-1)
 
diff --git a/tests/test_models.py b/tests/test_models.py
index b0810185..86ea4075 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -837,9 +837,8 @@ def test_uuid_url_id_name_suuid(self):
 
         self.assertEqual(u1.url_id, '74d588574a7611e78c27c38403d0935c')
         self.assertEqual(u1.url_id_name, '74d588574a7611e78c27c38403d0935c-test')
-        with self.assertRaises(AttributeError):
-            # No UuidMixin == No suuid or url_name_suuid attributes
-            self.assertEqual(u1.url_name_suuid, 'test-vVoaZTeXGiD4qrMtYNosnN')
+        # No UuidMixin means no suuid, so fallback to hex UUID
+        self.assertEqual(u1.url_name_suuid, 'test-74d588574a7611e78c27c38403d0935c')
         self.assertEqual(u2.url_id, '74d588574a7611e78c27c38403d0935c')
         self.assertEqual(u2.url_id_name, '74d588574a7611e78c27c38403d0935c-test')
         self.assertEqual(u2.url_name_suuid, 'test-vVoaZTeXGiD4qrMtYNosnN')

From c03a0357a43321e95ec2301320b6d3f012c528db Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Thu, 22 Feb 2018 18:45:35 +0530
Subject: [PATCH 25/26] Add an after_request handler to ClassView

---
 coaster/views/classview.py | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index e94bff12..dd1dc56b 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -14,7 +14,7 @@
 from sqlalchemy.orm.properties import RelationshipProperty
 from werkzeug.routing import parse_rule
 from werkzeug.local import LocalProxy
-from flask import _request_ctx_stack, has_request_context, request, redirect
+from flask import _request_ctx_stack, has_request_context, request, redirect, make_response
 from ..auth import current_auth, add_auth_attribute
 from ..sqlalchemy import PermissionMixin
 
@@ -122,8 +122,7 @@ def view_func(*args, **kwargs):
             # Call the instance's before_request method
             viewinst.before_request(view_func.__name__, kwargs)
             # Finally, call the view handler method
-            return view_func.wrapped_func(viewinst, *args, **kwargs)
-            # TODO: Support `after_request` as well. Note that it needs Response objects
+            return viewinst.after_request(make_response(view_func.wrapped_func(viewinst, *args, **kwargs)))
 
         # Decorate the wrapped view function with the class's desired decorators.
         # Mixin classes may provide their own decorators, and all of them will be applied.
@@ -239,7 +238,7 @@ def before_request(self, view, kwargs):
         """
         This method is called after the app's ``before_request`` handlers, and
         before the class's view method. It receives the name of the view
-        method with all keyword arguments that will be sent to the view method.
+        method, and all keyword arguments that will be sent to the view method.
         Subclasses and mixin classes may define their own
         :meth:`before_request` to pre-process requests.
 
@@ -250,6 +249,24 @@ def before_request(self, view, kwargs):
         """
         pass
 
+    def after_request(self, response):
+        """
+        This method is called with the response from the view handler method.
+        It must return a valid response object. Subclasses and mixin classes
+        may override this to perform any necessary post-processing::
+
+            class MyView(ClassView):
+                ...
+                def after_request(self, response):
+                    response = super(MyView, self).after_request(response)
+                    ...  # Process here
+                    return response
+
+        :param response: Response from the view handler method
+        :return: Response object
+        """
+        return response
+
     @classmethod
     def __get_raw_attr(cls, name):
         for base in cls.__mro__:

From a93fccfbe9616cb9beb8f4103c50fd66d516fe2a Mon Sep 17 00:00:00 2001
From: Kiran Jonnalagadda <kiran@hasgeek.com>
Date: Thu, 22 Feb 2018 18:53:09 +0530
Subject: [PATCH 26/26] Drop self.kwargs in ModelView and update documentation

---
 coaster/views/classview.py | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/coaster/views/classview.py b/coaster/views/classview.py
index dd1dc56b..cee7a7c1 100644
--- a/coaster/views/classview.py
+++ b/coaster/views/classview.py
@@ -346,10 +346,7 @@ def view(self, **kwargs):
 
         DocumentView.init_app(app)
 
-    :class:`ModelView` keeps a copy of the view's keyword arguments (passed in
-    by Flask from the URL rule parameters) in :attr:`kwargs`. :meth:`loader`
-    implementations are expected to use this to determine what to load. Views
-    will continue to receive keyword arguments as in :class:`ClassView`, but
+    Views will receive keyword arguments as in :class:`ClassView`, but
     typically will have no use for them if :meth:`loader` works as expected.
     """
 
@@ -361,23 +358,25 @@ def view(self, **kwargs):
     #: A mapping of URL rule variables to attributes on the model. For example,
     #: if the URL rule is ``/<parent>/<document>``, the attribute map can be::
     #:
+    #:     model = MyModel
     #:     route_model_map = {
-    #:         'document': 'name',
-    #:         'parent': 'parent.name',
+    #:         'document': 'name',       # Map 'document' in URL to MyModel.name
+    #:         'parent': 'parent.name',  # Map 'parent' to MyModel.parent.name
     #:         }
+    #:
+    #: The :class:`InstanceLoader` mixin class will convert this mapping into
+    #: SQLAlchemy attribute references to load the instance object.
     route_model_map = {}
 
-    def loader(self, view):  # pragma: no cover
+    def loader(self, view, kwargs):  # pragma: no cover
         """
         Subclasses or mixin classes may override this method to provide a model
         instance loader. The return value of this method will be placed at
         ``self.obj``.
 
-        TODO: Consider allowing :meth:`loader` to place attributes on ``self``
-        by itself, to accommodate scenarios where multiple models need to be
-        loaded.
+        :return: Object instance loaded from database
         """
-        pass  # TODO: Maybe raise NotImplementedError?
+        pass
 
     def before_request(self, view, kwargs):
         """
@@ -386,9 +385,7 @@ def before_request(self, view, kwargs):
         :func:`super` to ensure :meth:`loader` is called.
         """
         super(ModelView, self).before_request(view, kwargs)
-        self.kwargs = kwargs
-        self.view_name = view
-        self.obj = self.loader(view)
+        self.obj = self.loader(view, kwargs)
 
 
 class UrlForView(object):
@@ -477,12 +474,12 @@ class InstanceLoader(object):
     :class:`InstanceLoader` will traverse relationships (many-to-one or
     one-to-one) and perform a SQL ``JOIN`` with the target class.
     """
-    def loader(self, view):
-        if any((name in self.route_model_map for name in self.kwargs)):
+    def loader(self, view, kwargs):
+        if any((name in self.route_model_map for name in kwargs)):
             # We have a URL route attribute that matches one of the model's attributes.
             # Attempt to load the model instance
             filters = {self.route_model_map[key]: value
-                for key, value in self.kwargs.items()
+                for key, value in kwargs.items()
                 if key in self.route_model_map}
 
             query = self.query or self.model.query