Repository: hasgeek/coaster

Issue:
url property for models
Any REST API we build will require a `url` property for all the entity types. Coaster can provide it as a mixin. The `url` property will essentially return `self.url_for(_external=True)`.

Discussion:
jace: Several models have a `url` column. The column will override the property, but we'll have to careful about unintended mixups and should consider renaming those columns.

https://github.com/search?q=org%3Ahasgeek+%22url+%3D+db.Column%22&type=Code
jace: I'm a little concerned about forcing a `url` property to be available on all models just because `UrlForMixin` is a base class of `BaseMixin`. We need this to be more intent-based:

RoleMixin should look for a dictionary named `proxy_properties` or `__proxy_properties__` mapping names to functions. These will be made available in the proxy as properties.

Syntax 1 (read-only properties):

```python
class MyModel(BaseMixin, db.Model):
    proxy_properties = {
        'url': lambda self: self.url_for(),
        }
```

Syntax 2 (read/write, but verbose):
```python
class MyModel(BaseMixin, db.Model):
    proxy_properties = {
        'url': {'get': lambda self: self.url_for()},
        }
```

Neither syntax offers a way to declare what roles these properties are available under. Presumably, that will be left to `__roles__`.

On the other hand, if `url` turns out to be widely used property, this syntax is unnecessary verbiage in all models. We could take yet another approach: a new mixin dedicated to adding URL properties:

```python
class UrlPropertyMixin(object):
    @with_roles(read={'all'})
    @property
    def url(self):
        return self.url_for()
```

Models that need `url` can simply inherit from this class. Other models are not affected.
iambibhas: I like the `UrlPropertyMixin` idea but can we call it `absolute_url` instead of `url`? And let it be available to *all* models? I'm getting the idea from Django where each model has a `get_absolute_url()` method that's used internally in Django whenever a url is needed for a model. It can be overridden as well. https://docs.djangoproject.com/en/2.1/ref/models/instances/#get-absolute-url

This lets us get out of this `url` name confusion.
jace: If we call it `absolute_url`, it doesn't need to be in a separate mixin.
jace: Renamed. Good to merge, @iambibhas?

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index f29b756..37a6299 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -25,6 +25,8 @@
   methods
 * New: ``coaster.views.endpoint_for``: discover an endpoint given a URL
 * Changed: ``UuidMixin`` no longer provides ``url_id``
+* New: ``coaster.sqlalchemy.UrlForMixin`` now provides an ``absolute_url``
+  property
 
 
 0.6.0
diff --git a/coaster/sqlalchemy/mixins.py b/coaster/sqlalchemy/mixins.py
index 48e1e6b..8eed61a 100644
--- a/coaster/sqlalchemy/mixins.py
+++ b/coaster/sqlalchemy/mixins.py
@@ -248,6 +248,11 @@ class UrlForMixin(object):
         # url_for from flask
         return url_for(endpoint, **params)
 
+    @with_roles(read={'all'})
+    @property
+    def absolute_url(self):
+        return self.url_for(_external=True)
+
     @classmethod
     def is_url_for(cls, _action, _endpoint=None, _external=None, **paramattrs):
         """


Test patch:
diff --git a/tests/test_models.py b/tests/test_models.py
index bac428c..c7ebcf5 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -197,6 +197,7 @@ class DefaultValue(BaseMixin, db.Model):
     __tablename__ = 'default_value'
     value = db.Column(db.Unicode(100), default='default')
 
+
 auto_init_default(DefaultValue.value)
 
 
@@ -527,7 +528,7 @@ class TestCoasterModels(unittest.TestCase):
         self.session.commit()
         self.assertTrue(d.updated_at > updated_at)
 
-    def test_url_for(self):
+    def test_url_for_fail(self):
         d = UnnamedDocument(content="hello")
         self.assertEqual(d.url_for(), None)
 
diff --git a/tests/test_url_for.py b/tests/test_url_for.py
index a6b1ea1..5ed400b 100644
--- a/tests/test_url_for.py
+++ b/tests/test_url_for.py
@@ -94,3 +94,21 @@ class TestUrlFor(unittest.TestCase):
         self.assertEqual(doc2.url_for('edit'), 'http://localhost/1/document2/edit')
         self.assertEqual(doc2.url_for('edit', _external=False), '/1/document2/edit')
         self.assertEqual(doc2.url_for('edit', _external=True), 'http://localhost/1/document2/edit')
+
+    def test_url(self):
+        """
+        The .url property is the same as .url_for(_external=True)
+        """
+        # Make two documents
+        doc1 = NamedDocument(name=u'document1', title=u"Document 1")
+        self.session.add(doc1)
+        c1 = Container()  # Gets an autoincrementing id starting from 1
+        self.session.add(c1)
+        doc2 = ScopedNamedDocument(container=c1, name=u'document2', title=u"Document 2")
+        self.session.add(doc2)
+        self.session.commit()
+
+        assert doc1.absolute_url == doc1.url_for(_external=True)
+        assert doc1.absolute_url != doc1.url_for(_external=False)
+        assert doc2.absolute_url == doc2.url_for(_external=True)
+        assert doc2.absolute_url != doc2.url_for(_external=False)