use djangos named URLs and url-reverse instead of manually constructing urls

menu.py

@@ -1,4 +1,3 @@
-from urlparse import urlparse
 import util
 import cfg
 
@@ -70,7 +69,7 @@ class Menu(object):
         return request_path.startswith(self.url)
 
     def active_item(self, request):
-        """Return item list (e.g. submenu) of active menu item."""
+        """Return the first active item that is found"""
         for item in self.items:
             if request.path.startswith(item.url):
                 return item

module_loader.py

@@ -48,7 +48,7 @@ def load_modules():
             LOGGER.exception('Could not import modules/%s: %s',
                              name, exception)
 
-        _include_module_urls(full_name)
+        _include_module_urls(full_name, name)
 
     ordered_modules = []
     remaining_modules = dict(modules)
@@ -97,13 +97,13 @@ def _insert_modules(module_name, module, remaining_modules, ordered_modules):
     ordered_modules.append(module_name)
 
 
-def _include_module_urls(module_name):
+def _include_module_urls(module_name, namespace):
     """Include the module's URLs in global project URLs list"""
     url_module = module_name + '.urls'
     try:
         urls.urlpatterns += django.conf.urls.patterns(
             '', django.conf.urls.url(
-                r'', django.conf.urls.include(url_module)))
+                r'', django.conf.urls.include(url_module, namespace)))
     except ImportError:
         LOGGER.debug('No URLs for %s', module_name)
 

modules/apps/urls.py

@@ -24,5 +24,5 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.apps.apps',
-    url(r'^apps/$', 'index')
+    url(r'^apps/$', 'index', name='index')
     )

modules/config/urls.py

@@ -24,5 +24,5 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.config.config',
-    url(r'^sys/config/$', 'index'),
+    url(r'^sys/config/$', 'index', name='index'),
     )

modules/diagnostics/templates/diagnostics.html

@@ -24,8 +24,8 @@
 system to confirm that network services are running and configured
 properly. It may take a minute to complete.</p>
 
-<p><a class="btn btn-primary btn-large"
-href="{{ basehref }}/sys/diagnostics/test">Run diagnostic test
-&raquo;</a></p>
+<p><a class="btn btn-primary btn-large" href="{% url 'diagnostics:test' %}">
+  Run diagnostic test &raquo;
+</a></p>
 
 {% endblock %}

modules/diagnostics/urls.py

@@ -24,6 +24,6 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.diagnostics.diagnostics',
-    url(r'^sys/diagnostics/$', 'index'),
-    url(r'^sys/diagnostics/test/$', 'test'),
+    url(r'^sys/diagnostics/$', 'index', name='index'),
+    url(r'^sys/diagnostics/test/$', 'test', name='test'),
     )

modules/expert_mode/urls.py

@@ -24,5 +24,5 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.expert_mode.expert_mode',
-    url(r'^sys/expert/$', 'index'),
+    url(r'^sys/expert/$', 'index', name='index'),
     )

modules/firewall/urls.py

@@ -24,5 +24,5 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.firewall.firewall',
-    url(r'^sys/firewall/$', 'index')
+    url(r'^sys/firewall/$', 'index', name='index')
     )

modules/first_boot/first_boot.py

@@ -21,6 +21,7 @@ The Plinth first-connection process has several stages:
 from django import forms
 from django.contrib import messages
 from django.core import validators
+from django.core.urlresolvers import reverse
 from django.http.response import HttpResponseRedirect
 from django.template.response import TemplateResponse
 from gettext import gettext as _
@@ -94,7 +95,7 @@ def state0(request):
     """
     try:
         if _read_state() >= 5:
-            return HttpResponseRedirect(cfg.server_dir)
+            return HttpResponseRedirect(reverse('index'))
     except KeyError:
         pass
 
@@ -112,8 +113,7 @@ def state0(request):
             if success:
                 # Everything is good, permanently mark and move to page 2
                 _write_state(1)
-                return HttpResponseRedirect(
-                    cfg.server_dir + '/firstboot/state1')
+                return HttpResponseRedirect(reverse('first_boot:state1'))
     else:
         form = State0Form(initial=status, prefix='firstboot')
 

modules/first_boot/templates/firstboot_state1.html

@@ -23,7 +23,7 @@
 {% block main_block %}
 
   <p>Welcome screen not completely implemented yet.  Press <a
-    href="{{basehref }}/apps">continue</a> to see the rest of the
+      href="{% url 'apps:index' %}">continue</a> to see the rest of the
     web interface.</p>
 
   <ul>

modules/first_boot/urls.py

@@ -24,7 +24,7 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.first_boot.first_boot',
-    url(r'^firstboot/$', 'index'),
-    url(r'^firstboot/state0/$', 'state0'),
-    url(r'^firstboot/state1/$', 'state1')
+    url(r'^firstboot/$', 'index', name='index'),
+    url(r'^firstboot/state0/$', 'state0', name='state0'),
+    url(r'^firstboot/state1/$', 'state1', name='state1')
     )

modules/help/help.py

@@ -1,5 +1,6 @@
 import os
 from gettext import gettext as _
+from django.http import Http404
 from django.template.response import TemplateResponse
 
 import cfg
@@ -9,10 +10,10 @@ def init():
     """Initialize the Help module"""
     menu = cfg.main_menu.add_item(_('Documentation'), 'icon-book', '/help',
                                   101)
-    menu.add_item(_("Where to Get Help"), "icon-search", "/help/index", 5)
+    menu.add_item(_("Where to Get Help"), "icon-search", "/help/index/", 5)
     menu.add_item(_('Developer\'s Manual'), 'icon-info-sign',
-                  '/help/view/plinth', 10)
-    menu.add_item(_('FAQ'), 'icon-question-sign', '/help/view/faq', 20)
+                  '/help/page/plinth', 10)
+    menu.add_item(_('FAQ'), 'icon-question-sign', '/help/page/faq', 20)
     menu.add_item(_('%s Wiki' % cfg.box_name), 'icon-pencil',
                   'http://wiki.debian.org/FreedomBox', 30)
     menu.add_item(_('About'), 'icon-star', '/help/about', 100)
@@ -30,10 +31,13 @@ def about(request):
     return TemplateResponse(request, 'about.html', {'title': title})
 
 
-def default(request, page=''):
-    """Serve the documentation pages"""
-    with open(os.path.join('doc', '%s.part.html' % page), 'r') as input_file:
-        main = input_file.read()
+def helppage(request, page):
+    """Serve a help page from the 'doc' directory"""
+    try:
+        input_file = open(os.path.join('doc', '%s.part.html' % page), 'r')
+    except IOError:
+        raise Http404
+    main = input_file.read()
 
     title = _('%s Documentation') % cfg.product_name
     return TemplateResponse(request, 'base.html',

modules/help/templates/about.html

@@ -1,4 +1,5 @@
 {% extends 'base.html' %}
+{% load static %}
 {% comment %}
 #
 # This file is part of Plinth.
@@ -20,7 +21,7 @@
 
 {% block main_block %}
 
-<img src="{{ basehref }}/static/theme/img/freedombox-logo-250px.png"
+<img src="{% static 'theme/img/freedombox-logo-250px.png' %}"
 class="main-graphic" />
 
 <p>We live in a world where our use of the network is mediated by

modules/help/templates/help.html

@@ -23,7 +23,7 @@
 <p>There are a variety of places to go for help with
 {{ cfg.product_name }} and the box it runs on.</p>
 
-<p>This front end has a <a href="{{ basehref }}/help/view/plinth">
+<p>This front end has a <a href="{% url 'help:helppage' 'plinth' %}">
 developer's manual</a>.  It isn't complete, but it is the first place
 to look.  Feel free to offer suggestions, edits, and screenshots for
 completing it!</p>
@@ -39,7 +39,7 @@ about the {{ cfg.box_name }} and almost surely know nothing of this
 front end, but they are an incredible resource for general Debian
 issues.</p>
 
-<p>There is no <a href="{{ basehref }}/help/view/faq">FAQ</a> because
+<p>There is no <a href="{% url 'help:helppage' 'faq' %}">FAQ</a> because
 the question frequency is currently zero for all questions.</p>
 
 {% endblock %}

modules/help/urls.py

@@ -20,15 +20,14 @@ URLs for the Help module
 """
 
 from django.conf.urls import patterns, url
+from django.core.urlresolvers import reverse_lazy
+from django.views.generic import RedirectView
 
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.help.help',
-    url(r'^help/$', 'index'),
-    url(r'^help/index/$', 'index'),
-    url(r'^help/about/$', 'about'),
-    url(r'^help/view/(?P<page>design)/$', 'default'),
-    url(r'^help/view/(?P<page>plinth)/$', 'default'),
-    url(r'^help/view/(?P<page>hacking)/$', 'default'),
-    url(r'^help/view/(?P<page>faq)/$', 'default'),
-    )
+    url(r'^help/$', RedirectView.as_view(url=reverse_lazy('help:index'))),
+    url(r'^help/index/$', 'index', name='index'),
+    url(r'^help/about/$', 'about', name='about'),
+    url(r'^help/page/([\w]+)/$', 'helppage', name='helppage'),
+)

modules/lib/urls.py

@@ -23,11 +23,10 @@ from django.conf.urls import patterns, url
 
 import cfg
 
-
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     '',
     url(r'^accounts/login/$', 'django.contrib.auth.views.login',
-        {'template_name': 'login.html'}),
+        {'template_name': 'login.html'}, name='login'),
     url(r'^accounts/logout/$', 'django.contrib.auth.views.logout',
-        {'next_page': cfg.server_dir})
+        {'next_page': cfg.server_dir}, name='logout')
     )

modules/owncloud/templates/owncloud.html

@@ -28,7 +28,7 @@
     {{ form|bootstrap }}
 
     <p>When enabled, the owncloud installation will be available
-      from <a href="/owncloud">owncloud</a> on the web server.  Visit
+    from <a href="{% url 'owncloud:index' %}">owncloud</a> on the web server.  Visit
       this URL to set up the initial administration account for
       owncloud.</p>
 

modules/owncloud/urls.py

@@ -24,5 +24,5 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.owncloud.owncloud',
-    url(r'^apps/owncloud/$', 'index'),
+    url(r'^apps/owncloud/$', 'index', name='index'),
     )

modules/packages/urls.py

@@ -24,5 +24,5 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.packages.packages',
-    url(r'^sys/packages/$', 'index'),
+    url(r'^sys/packages/$', 'index', name='index'),
     )

modules/pagekite/templates/pagekite_introduction.html

@@ -49,7 +49,7 @@ there. In future, it might be possible to use your buddy's
 
 <p>
   <a class='btn btn-primary btn-lg'
-     href="{{ basehref }}/apps/pagekite/configure">Configure
+      href="{% url 'pagekite:configure' %}">Configure
      PageKite</a>
 </p>
 

modules/pagekite/urls.py

@@ -24,6 +24,6 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.pagekite.pagekite',
-    url(r'^apps/pagekite/$', 'index'),
-    url(r'^apps/pagekite/configure/$', 'configure'),
+    url(r'^apps/pagekite/$', 'index', name='index'),
+    url(r'^apps/pagekite/configure/$', 'configure', name='configure'),
     )

modules/system/urls.py

@@ -24,5 +24,5 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.system.system',
-    url(r'^sys/$', 'index'),
+    url(r'^sys/$', 'index', name='index'),
     )

modules/tor/urls.py

@@ -24,5 +24,5 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.tor.tor',
-    url(r'^apps/tor/$', 'index')
+    url(r'^apps/tor/$', 'index', name='index')
     )

modules/users/urls.py

@@ -24,7 +24,7 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.users.users',
-    url(r'^sys/users/$', 'index'),
-    url(r'^sys/users/add/$', 'add'),
-    url(r'^sys/users/edit/$', 'edit')
+    url(r'^sys/users/$', 'index', name='index'),
+    url(r'^sys/users/add/$', 'add', name='add'),
+    url(r'^sys/users/edit/$', 'edit', name='edit')
     )

modules/xmpp/urls.py

@@ -24,7 +24,7 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'modules.xmpp.xmpp',
-    url(r'^apps/xmpp/$', 'index'),
-    url(r'^apps/xmpp/configure/$', 'configure'),
-    url(r'^apps/xmpp/register/$', 'register')
+    url(r'^apps/xmpp/$', 'index', name='index'),
+    url(r'^apps/xmpp/configure/$', 'configure', name='configure'),
+    url(r'^apps/xmpp/register/$', 'register', name='register')
     )

plinth.py

@@ -35,6 +35,7 @@ def parse_arguments():
     parser.add_argument(
         '--pidfile', default='plinth.pid',
         help='specify a file in which the server may write its pid')
+    # TODO: server_dir is actually a url prefix; use a better variable name
     parser.add_argument(
         '--server_dir', default='/',
         help='web server path under which to serve')

templates/base.html

@@ -59,11 +59,11 @@
             <span class="icon-bar"></span>
             <span class="icon-bar"></span>
           </a>
-          <a href="{{ basehref }}" class="logo-top">
+          <a href="{% url 'index' %}" class="logo-top">
             <img src="{% static 'theme/img/freedombox-logo-32px.png' %}"
                  alt="FreedomBox" />
           </a>
-          <a class="brand" href="{{ basehref }}">FreedomBox</a>
+          <a class="brand" href="{% url 'index' %}">FreedomBox</a>
 	  {% block add_nav_and_login %}
             <div class="nav-collapse">
               <ul class="nav">
@@ -85,15 +85,15 @@
               {% if user.is_authenticated %}
                 <p class="navbar-text pull-right">
                   <i class="icon-user icon-white nav-icon"></i>
-                  Logged in as <a href="{{ user.username }}">{{ user.username }}</a>.
-                  <a href="{{ basehref }}/accounts/logout" title="Log out">
+                  Logged in as <a href="#">{{ user.username }}</a>.
+                  <a href="{% url 'lib:logout' %}" title="Log out">
                     Log out</a>.
                 </p>
               {% else %}
                 <p class="navbar-text pull-right">
                   Not logged in.
                   <i class="icon-user icon-white nav-icon"></i>
-                  <a href="{{ basehref }}/accounts/login" title="Log in">
+                  <a href="{% url 'lib:login' %}" title="Log in">
                     Log in</a>.
                 </p>
               {% endif %}

urls.py

@@ -24,5 +24,5 @@ from django.conf.urls import patterns, url
 
 urlpatterns = patterns(  # pylint: disable-msg=C0103
     'views',
-    url(r'^$', 'index')
+    url(r'^$', 'index', name='index')
     )

util.py

@@ -1,20 +1,4 @@
 import os
-from django.http.response import HttpResponseRedirect
-import cfg
-
-
-class PlinthRedirect(HttpResponseRedirect):
-    """
-    We do not fully use django and thus cannot use its named URLs to construct
-    links/redirects, so we have to take care of cfg.server_dir manually.
-    This temporary helper class makes sure that plinth-internal redirects
-    have the correct server_dir prefix.
-    """
-    def __init__(self, redirect_to, *args, **kwargs):
-        if not redirect_to.startswith(cfg.server_dir):
-            redirect_to = rel_urljoin([cfg.server_dir, redirect_to])
-        return super(PlinthRedirect, self).__init__(redirect_to,
-                                                    *args, **kwargs)
 
 
 def rel_urljoin(parts, prepend_slash=True):
@@ -22,7 +6,7 @@ def rel_urljoin(parts, prepend_slash=True):
     urllibs' urljoin joins ("foo", "/bar") to "/bar".
     Instead concatenate the parts with "/" to i.e. /foo/bar
     """
-    url =  '/'.join(s.strip('/') for s in parts)
+    url = '/'.join(s.strip('/') for s in parts)
     if prepend_slash and not url.startswith('/'):
         url = '/' + url
     return url

views.py

@@ -19,11 +19,12 @@
 Main Plinth views
 """
 
-from util import PlinthRedirect
 import logging
 
 import cfg
 from withsqlite.withsqlite import sqlite_db
+from django.http.response import HttpResponseRedirect
+from django.core.urlresolvers import reverse
 
 
 LOGGER = logging.getLogger(__name__)
@@ -37,13 +38,14 @@ def index(request):
             # Permanent redirect causes the browser to cache the redirect,
             # preventing the user from navigating to /plinth until the
             # browser is restarted.
-            return PlinthRedirect('/firstboot')
+            return HttpResponseRedirect(reverse('first_boot:index'))
 
         if database['state'] < 5:
             LOGGER.info('First boot state - %d', database['state'])
-            return PlinthRedirect('/firstboot/state%d' % database['state'])
+            return HttpResponseRedirect(reverse('first_boot:state%d' %
+                                                database['state']))
 
     if request.user.is_authenticated():
-        return PlinthRedirect('/apps')
+        return HttpResponseRedirect(reverse('apps:index'))
 
-    return PlinthRedirect('/help/about')
+    return HttpResponseRedirect(reverse('apps:about'))