Permalink
Browse files

Developers can register 'sub'-urlpatterns for a page [#28513975]

  • Loading branch information...
1 parent dfe351a commit 403fba1ef26f7b5a164c6cecbcd6393406bf44d0 Andre Engelbrecht committed Oct 3, 2012
@@ -2,6 +2,7 @@
from django.utils import timezone
from ostinato.pages.models import PageContent
+from ostinato.pages.registry import page_content
from ostinato.statemachine.core import State, StateMachine
from odemo.models import Content
@@ -52,13 +53,15 @@ def get_absolute_url(self):
## Create a News List page
+@page_content.register
class NewsListPageContent(Content):
## We could store some information here, like how many items we want
## to show on the page.
class ContentOptions:
template = 'page_templates/news_list_page.html'
view = 'odemo.news.views.NewsPageView'
+ urls = 'odemo.news.urls'
class PageWithNews(Content):
@@ -0,0 +1,12 @@
+from django.conf.urls import patterns, include, url
+from django.views.generic import DetailView
+
+from odemo.news.models import NewsItem
+
+
+urlpatterns = patterns('',
+
+ url(r'^(?P<pk>\d+)/$', DetailView.as_view(model=NewsItem),
+ name='newsitem_detail'),
+
+)
@@ -6,7 +6,6 @@
from ostinato.pages.registry import page_content
from ostinato.pages.sitemaps import PageSitemap
from odemo.news.models import NewsItem
-from odemo.news.views import NewsPageView
admin.autodiscover()
@@ -18,9 +17,6 @@
}
urlpatterns = patterns('',
- url(r'^news/(?P<pk>\d+)/$', DetailView.as_view(model=NewsItem),
- name='newsitem_detail'),
-
(r'^sitemap\.xml$', 'django.contrib.sitemaps.views.sitemap',
{'sitemaps': sitemaps}),
@@ -30,10 +26,6 @@
(r'^grappelli/', include('grappelli.urls')),
)
-
urlpatterns += staticfiles_urlpatterns()
-
-urlpatterns += patterns('',
- url(r'^', include('ostinato.pages.urls')),
-)
+urlpatterns += patterns('', url(r'^', include('ostinato.pages.urls')), )
@@ -1,4 +1,5 @@
from django.db import models
+from django.core.exceptions import ObjectDoesNotExist
from django.utils import timezone
@@ -58,19 +59,46 @@ def get_breadcrumbs(self, for_page):
return to_return
def get_from_path(self, url_path):
- """ Returns a page object, base on the url path. """
+ """
+ Cycle through every slug in the path to try and find the current
+ page.
+
+ Returns a tuple containing the Page and any sub-urlpath that might
+ precede the page slug.
+
+ TODO: More descriptive doctype pls
+
+ TODO: We should probably look for the page based on more than just
+ the slug, since a sub-url might exist with the same "slug" as a page.
+ A possible solution would be to match the page with the parent (if any)
+
+ This leads me to believe we should be caching/indexing page urls
+ somewhere for a quick lookup.
+ """
+ page = None
path = url_path.split('/')
- path.reverse()
+ sub_path = []
- ## TODO: Maybe we should cache the page paths somewhere, so that
- ## we dont have to do a query for each page.
- for node in path:
+ while not page:
try:
- if node:
- page = self.get_query_set().get(slug=node)
- return page
- except:
- pass
+ page = self.get_query_set().get(slug=path[-1])
+
+ except ObjectDoesNotExist:
+ if path:
+ # Save the failed slug so we can return it later
+ sub_path.insert(0, path[-1])
+ path = path[:-1] # Remove the failed slug
+
+ if not path: break
+
+ if sub_path:
+ sub_path = '/' + '/'.join(sub_path)
+ else:
+ sub_path = None
+
+ if sub_path == '/':
+ sub_path = None
+ return page, sub_path
@@ -177,6 +177,7 @@ class ContentOptions:
view = 'ostinato.pages.views.PageView'
form = None
page_inlines = []
+ urls = None
def add_content(self, **kwargs):
@@ -0,0 +1,9 @@
+from django.conf.urls import patterns, include, url
+from django.views.generic import TemplateView
+
+
+urlpatterns = patterns('',
+ ## Just create a dummy url to be used with the tests
+ url(r'^with/sub/path/$', TemplateView.as_view(
+ template_name='pages/tests/basic_page.html'), name='sub_url_name'),
+)
@@ -63,6 +63,9 @@ class OtherPage(ContentMixin, PageContent):
class Meta:
verbose_name = 'Some Other Page'
+ class ContentOptions:
+ urls = 'ostinato.pages.test_sub_urls'
+
def create_pages():
user = User.objects.create(username='user1', password='secret',
@@ -90,6 +93,13 @@ def create_pages():
template='pages.basicpage',
parent=p,
)
+ p4 = Page.objects.create(
+ title="Application Page", slug="apppage", short_title='App Page',
+ author=user, show_in_nav=False,
+ created_date = "2012-04-10 12:14:51.203925+00:00",
+ modified_date = "2012-04-10 12:14:51.203925+00:00",
+ template='pages.otherpage'
+ )
## Create some content
LandingPage.objects.create(
@@ -131,7 +141,7 @@ def test_model_exists(self):
def test_related_lookup(self):
u = User.objects.get(username='user1')
pages = u.pages_authored.all()
- self.assertEqual(3, pages.count())
+ self.assertEqual(4, pages.count())
def test_unicode(self):
self.assertEqual('Page 1', Page.objects.get(id=1).__unicode__())
@@ -175,6 +185,11 @@ def test_page_contents(self):
c = LandingPage.objects.get(id=1)
self.assertEqual(c, p.contents)
+ def get_sub_url_for(self):
+ p = Page.objects.get(slug='apppage')
+ self.assertEqual('/apppage/with/sub/path/',
+ page.get_sub_url())
+
class PageManagerTestCase(TestCase):
@@ -184,7 +199,7 @@ def setUp(self):
create_pages()
def test_published(self):
- self.assertEqual([1, 2, 3],
+ self.assertEqual([1, 2, 3, 4],
list(Page.objects.published().values_list('id', flat=True)))
def test_get_empty_navbar(self):
@@ -231,8 +246,19 @@ def test_get_page_from_path(self):
rf = RequestFactory()
request = rf.get('/page-1/page-3/')
- self.assertEqual('page-3',
- Page.objects.get_from_path(request.path).slug)
+ page, sub_path = Page.objects.get_from_path(request.path)
+
+ self.assertEqual('page-3', page.slug)
+ self.assertFalse(sub_path)
+
+ def test_get_page_from_path_with_sub_path(self):
+ rf = RequestFactory()
+ request = rf.get('/page-1/apppage/with/sub/path/')
+
+ page, sub_path = Page.objects.get_from_path(request.path)
+
+ self.assertEqual('apppage', page.slug)
+ self.assertEqual('/with/sub/path/', sub_path)
class PageContentModelTestCase(TestCase):
@@ -335,6 +361,13 @@ def test_custom_view_context(self):
self.assertIn('custom', response.context)
self.assertEqual('Some Custom Context', response.context['custom'])
+ def test_page_with_sub_path(self):
+ response = self.client.get('/apppage/with/sub/path/')
+
+ self.assertEqual(200, response.status_code)
+ # Make sure that the page is in the new context
+ self.assertIn('page', response.context['params'])
+
class NavBarTemplateTagTestCase(TestCase):
@@ -1,11 +1,13 @@
from django.views.generic import View, TemplateView
from django.views.generic.edit import FormMixin
from django.shortcuts import get_object_or_404
+from django.template import RequestContext
from django.utils import simplejson as json
from django.utils.decorators import method_decorator
from django.core.serializers.json import DjangoJSONEncoder
-from django.core.urlresolvers import reverse
+from django.core.urlresolvers import reverse, resolve, Resolver404
from django.contrib.admin.views.decorators import staff_member_required
+from django.conf import settings
from django import http
from ostinato.pages.models import Page, PageWorkflow
@@ -23,12 +25,31 @@ def page_dispatch(request, *args, **kwargs):
## Some basic page checking and authorization
if 'path' in kwargs:
- path = kwargs['path'].split('/')
+ page, sub_path = Page.objects.get_from_path(kwargs['path'])
- if not path[-1]:
- path = path[:-1]
+ if not page:
+ raise http.Http404
+
+ # Here we check for sub_path and if the current page allows for that
+ if page and sub_path:
+ content = page.get_content_model()
+
+ if content.ContentOptions.urls:
+ if settings.APPEND_SLASH:
+ sub_path += '/'
+ try:
+ pattern = resolve(sub_path, content.ContentOptions.urls)
+ except Resolver404:
+ raise http.Http404
+
+ if pattern:
+ view, args, kwargs = pattern
+ # Now return the view, but make sure to add our page
+ # to the context first
+ kwargs.update({'page': page})
+ return view(request, *args, **kwargs)
- page = get_object_or_404(Page, slug=path[-1])
+ raise http.Http404
else:
# If we are looking at the root object, show the first root page

1 comment on commit 403fba1

@andrewebdev
Owner

This feature is in a experimental branch for now since I'm not yet convinced it's a good idea to allow this. It makes it too easy for a user/editor to move a page, and subsequently the entire sub-url structure below it. this will change url patterns.

URI patters shouldn't really be changing, and if they do there should be some kind of permanent redirect. Adding redirects would be simple enough, but means that we need to store the URI history in some way.

I don't like this, but I'm open for discussion and may be convinced to add it in future. For now I'm keeping this feature on a separate "experimental" branch.

Please sign in to comment.