Merge lp:~elachuni/ubuntu-webcatalog/breadcrumbs into lp:ubuntu-webcatalog

[Anthony Lenton]

Add a small breadcrumbs display at the top of the page, similar to software-center's

Overview
========
Add a small breadcrumbs display at the top of the page, similar to software-center's

Details
=======
The breadcrumbs shown at the top of the pages are currently different to the ones displayed within USC. They don't display the path you've walked through the application, but a canonical path for each page. For example, if after searching for "Foo" I click on "bygfoot", the breadcrumbs won't display "Get Software > Search results > bygfoot". They'll instead display "Get software > Games > bygfoot", as bygfoot is in the Games department.

We *could* switch to the former behaviour by sticking the breadcrumbs in the session, and keeping track of the pages the user has visited. That would be tricky (what happens if the user starts visiting arbitrary urls? the breadcrumb machine needs to realize what's a "valid path" within the site...) That's not what this branch does :)

[Matthew Nuzum (newz)]

Looks good. You did implement this the right way in my opinion. There is a back button to get people back to the search results, however this lets them go up a level to see related products.

review: Approve

=== modified file 'src/webcatalog/models.py'
--- src/webcatalog/models.py	2011-06-17 16:21:59 +0000
+++ src/webcatalog/models.py	2011-06-23 15:03:25 +0000
@@ -25,6 +25,7 @@
 import logging
 import re
 
+from django.core.urlresolvers import reverse
 from django.db import models
 
 from webcatalog.department_filters import department_filters
@@ -119,6 +120,16 @@
                     self.departments.add(dept)
                     break
 
+    def crumbs(self):
+        depts = self.departments.order_by('-parent')
+        if depts.count():
+            crumbs = depts[0].crumbs()
+        else:
+            crumbs = [{'name': 'Get Software', 'url': reverse('wc-index')}]
+        crumbs.append({'name': self.name, 'url': reverse('wc-package-detail',
+            args=[self.package_name])})
+        return crumbs
+
 class Department(models.Model):
     parent = models.ForeignKey('self', blank=True, null=True)
     name = models.CharField(max_length=64)
@@ -129,3 +140,12 @@
     @property
     def normalized_name(self):
         return re.sub(r'[-&.,;" \']', '', self.name)
+
+    def crumbs(self):
+        if self.parent:
+            crumbs = self.parent.crumbs()
+        else:
+            crumbs = [{'name': 'Get Software', 'url': reverse('wc-index')}]
+        crumbs.append({'name': self.name, 'url': reverse('wc-department',
+            args=[self.id])})
+        return crumbs

=== modified file 'src/webcatalog/static/css/webcatalog.css'
--- src/webcatalog/static/css/webcatalog.css	2011-06-07 20:53:56 +0000
+++ src/webcatalog/static/css/webcatalog.css	2011-06-23 15:03:25 +0000
@@ -145,3 +145,58 @@
     width:56px;
     height:48px;
 }
+
+.icon64 {
+    width: 64px;
+    height: 64px;
+}
+
+
+/* Breadcrumbs styles */
+#breadcrumbs {
+    height: 25px;
+    margin: 16px 0;
+}
+
+#breadcrumbs li {
+    list-style: none;
+    float: left;
+    line-height: 25px;
+    padding: 0 20px 0 7px;
+    background: transparent url("/assets/images/breadcrumbs_bg.png") no-repeat top right;
+    border-style: solid;
+    border-color: #928d8a;
+    border-width: 1px 0;
+}
+
+#breadcrumbs li a {
+    color: #444;
+}
+
+#breadcrumbs li.first {
+    -moz-border-radius: 4px 0 0 4px;
+    -webkit-border-radius: 4px 0 0 4px;
+    -khtml-border-radius: 4px 0 0 4px;
+    border-radius: 4px 0 0 4px;
+    border-width: 1px 0 1px 1px;
+}
+
+#breadcrumbs li.last {
+    -moz-border-radius: 0 4px 4px 0;
+    -webkit-border-radius: 0 4px 4px 0;
+    -khtml-border-radius: 0 4px 4px 0;
+    border-radius:0 4px 4px 0 ;
+    border-width: 1px 1px 1px 0;
+    background-position: top left;
+    padding-right: 7px;
+}
+
+#breadcrumbs li.first.last {
+    -moz-border-radius: 4px;
+    -webkit-border-radius: 4px;
+    -khtml-border-radius: 4px;
+    border-radius: 4px;
+    border-width: 1px;
+}
+
+/* End breadcrumbs style */

=== added file 'src/webcatalog/static/images/breadcrumbs_bg.png'
Binary files src/webcatalog/static/images/breadcrumbs_bg.png	1970-01-01 00:00:00 +0000 and src/webcatalog/static/images/breadcrumbs_bg.png	2011-06-23 15:03:25 +0000 differ
=== modified file 'src/webcatalog/templates/webcatalog/application_detail.html'
--- src/webcatalog/templates/webcatalog/application_detail.html	2011-06-17 16:21:59 +0000
+++ src/webcatalog/templates/webcatalog/application_detail.html	2011-06-23 15:03:25 +0000
@@ -2,14 +2,17 @@
 {% load i18n %}
 
 {% block title %}Application {{ application.name }}{% endblock %}
+{% block header %}Details for {{ application.name }}{% endblock %}
 
 {% block content %}
+    {% include "webcatalog/breadcrumbs_snippet.html" %}
+
   <div id="sc-mockup">
     <div class="header">
       {% if application.icon %}
-          <img src="{{ application.icon.url }}"/>
+          <img class="icon64" src="{{ application.icon.url }}"/>
       {% else %}
-          <img src="{{ STATIC_URL }}images/noicon_32.png"/>
+          <img class="icon64" src="{{ STATIC_URL }}images/noicon_32.png"/>
       {% endif %}
 
       <h2>{{ application.name }}</h2>

=== added file 'src/webcatalog/templates/webcatalog/application_overview_snippet.html'
--- src/webcatalog/templates/webcatalog/application_overview_snippet.html	1970-01-01 00:00:00 +0000
+++ src/webcatalog/templates/webcatalog/application_overview_snippet.html	2011-06-23 15:03:25 +0000
@@ -0,0 +1,11 @@
+<div class="app-overview-row">
+    {% if app.icon %}
+        <img src="{{ app.icon.url }}" width="32" height="32" />
+    {% else %}
+        <img src="{{ STATIC_URL }}images/noicon_32.png"/>
+    {% endif %}
+    <h3>
+    <a href="{% url wc-package-detail app.package_name %}">{{ app.name }}</a>
+    </h3>
+    <p>{{ app.comment }}</p>
+</div>

=== added file 'src/webcatalog/templates/webcatalog/breadcrumbs_snippet.html'
--- src/webcatalog/templates/webcatalog/breadcrumbs_snippet.html	1970-01-01 00:00:00 +0000
+++ src/webcatalog/templates/webcatalog/breadcrumbs_snippet.html	2011-06-23 15:03:25 +0000
@@ -0,0 +1,6 @@
+<ul id="breadcrumbs">
+{% for crumb in breadcrumbs %}
+    <li class="crumb {% if forloop.first %} first{% endif %}{% if forloop.last %} last{% endif %}">
+        <a href="{{ crumb.url }}">{{ crumb.name }}</a></li>
+{% endfor %}
+</ul>
\ No newline at end of file

=== modified file 'src/webcatalog/templates/webcatalog/department_overview.html'
--- src/webcatalog/templates/webcatalog/department_overview.html	2011-04-19 19:25:48 +0000
+++ src/webcatalog/templates/webcatalog/department_overview.html	2011-06-23 15:03:25 +0000
@@ -5,6 +5,8 @@
 {% block header %}{% trans "Ubuntu web catalog" %} &mdash; {{ dept.name }}{% endblock %}
 
 {% block content %}
+    {% include "webcatalog/breadcrumbs_snippet.html" %}
+
 {% if subdepts %}
 <h3>{% trans "Subsections" %}:</h3>
 
@@ -18,17 +20,7 @@
 {% if apps %}
 <p>{% blocktrans with napps=apps|length %}Showing {{napps}} applications.{% endblocktrans %}</p>
 {% for app in apps %}
-<div class="app-overview-row">
-    {% if app.icon %}
-        <img src="{{ app.icon.url }}" width="32" height="32" />
-    {% else %}
-        <img src="{{ STATIC_URL }}images/noicon_32.png"/>
-    {% endif %}
-    <h3>
-    <a href="{% url wc-package-detail app.package_name %}">{{ app.name }}</a>
-    </h3>
-    <p>{{ app.comment }}</p>
-</div>
+    {% include "webcatalog/application_overview_snippet.html" %}
 {% endfor %}
 {% else %}
 <p>{% trans "No applications found." %}</p>

=== modified file 'src/webcatalog/templates/webcatalog/index.html'
--- src/webcatalog/templates/webcatalog/index.html	2011-04-19 18:46:21 +0000
+++ src/webcatalog/templates/webcatalog/index.html	2011-06-23 15:03:25 +0000
@@ -6,6 +6,7 @@
 {% block search %}{% endblock %}
 
 {% block content %}
+
 <h3>{% trans "Browse application departments" %}:</h3>
 
 {% for dept in depts %}

=== modified file 'src/webcatalog/templates/webcatalog/search_results.html'
--- src/webcatalog/templates/webcatalog/search_results.html	2011-04-13 15:00:07 +0000
+++ src/webcatalog/templates/webcatalog/search_results.html	2011-06-23 15:03:25 +0000
@@ -5,23 +5,14 @@
 
 {% block header %}{% trans "Search results" %}{% endblock %}
 {% block content %}
+    {% include "webcatalog/breadcrumbs_snippet.html" %}
 
 {% if query %}
 <h3>{% blocktrans %}Searched for "{{query}}"{% endblocktrans %}</h3>
 {% if apps %}
 <p>{% blocktrans with napps=apps|length %}Found {{napps}} applications.{% endblocktrans %}</p>
 {% for app in apps %}
-<div class="app-overview-row">
-    {% if app.icon %}
-        <img src="{{ app.icon.url }}"/>
-    {% else %}
-        <img src="{{ STATIC_URL }}images/noicon_32.png"/>
-    {% endif %}
-    <h3>
-    <a href="{% url wc-package-detail app.package_name %}">{{ app.name }}</a>
-    </h3>
-    <p>{{ app.comment }}</p>
-</div>
+    {% include "webcatalog/application_overview_snippet.html" %}
 {% endfor %}
 {% else %}
 <p>{% trans "No applications found." %}</p>

=== modified file 'src/webcatalog/tests/test_models.py'
--- src/webcatalog/tests/test_models.py	2011-04-19 19:25:48 +0000
+++ src/webcatalog/tests/test_models.py	2011-06-23 15:03:25 +0000
@@ -22,6 +22,7 @@
     with_statement,
     )
 
+from django.core.urlresolvers import reverse
 
 from webcatalog.tests.factory import TestCaseWithFactory
 from webcatalog.models import Application
@@ -59,6 +60,43 @@
         self.assertEqual(1, app.departments.count())
         self.assertEqual('Games', app.departments.get().name)
 
+    def test_crumbs_no_department(self):
+        app = self.factory.make_application()
+        self.assertEquals([], list(app.departments.all()))
+        expected = [{'name': 'Get Software', 'url': reverse('wc-index')},
+            {'name': app.name, 'url': reverse('wc-package-detail',
+            args=[app.package_name])}]
+
+        self.assertEquals(expected, app.crumbs())
+
+    def test_crumbs_department_without_parent(self):
+        app = self.factory.make_application()
+        app.categories = 'Game;'
+        app.update_departments()
+        dept = app.departments.get()
+        expected = [{'name': 'Get Software', 'url': reverse('wc-index')},
+            {'name': dept.name, 'url': reverse('wc-department',
+                args=[dept.id])},
+            {'name': app.name, 'url': reverse('wc-package-detail',
+                args=[app.package_name])}]
+
+        self.assertEquals(expected, app.crumbs())
+
+    def test_crumbs_department_with_parent(self):
+        app = self.factory.make_application()
+        app.categories = 'RolePlaying;'
+        app.update_departments()
+        dept = app.departments.get()
+        expected = [{'name': 'Get Software', 'url': reverse('wc-index')},
+            {'name': dept.parent.name, 'url': reverse('wc-department',
+                args=[dept.parent.id])},
+            {'name': dept.name, 'url': reverse('wc-department',
+                args=[dept.id])},
+            {'name': app.name, 'url': reverse('wc-package-detail',
+                args=[app.package_name])}]
+
+        self.assertEquals(expected, app.crumbs())
+
 
 def DepartmentTestCse(TestCaseWithFactory):
     def test_normalized_name(self):
@@ -72,3 +110,22 @@
         for case, expected in cases.items():
             dept = self.factory.make_department(case)
             self.assertEquals(dept.normalized_name, expected)
+
+    def test_crumbs_no_parent(self):
+        dept = self.factory.make_department('Foo')
+        expected = [{'name': 'Get Software', 'url': reverse('wc-index')},
+            {'name': dept.name, 'url': reverse('wc-department',
+                args=[dept.id])}]
+
+        self.assertEquals(expected, dept.crumbs())
+
+    def test_crumbs_with_parent(self):
+        parent = self.factory.make_department('Foo')
+        dept = self.factory.make_department('Bar', parent=parent)
+        expected = [{'name': 'Get Software', 'url': reverse('wc-index')},
+            {'name': dept.parent.name, 'url': reverse('wc-department',
+                args=[dept.parent.id])},
+            {'name': dept.name, 'url': reverse('wc-department',
+                args=[dept.id])}]
+
+        self.assertEquals(expected, dept.crumbs())

=== modified file 'src/webcatalog/urls.py'
--- src/webcatalog/urls.py	2011-06-17 16:21:59 +0000
+++ src/webcatalog/urls.py	2011-06-23 15:03:25 +0000
@@ -36,8 +36,7 @@
     url(r'^$', 'index', name='wc-index'),
     url(r'^department/(?P<dept_id>\d+)/$', 'department_overview',
         name='wc-department'),
-    url(r'^applications/(?P<slug>[-.+\w]+)/$', DetailView.as_view(
-        model=Application, slug_field='package_name',
-        context_object_name='application'), name="wc-package-detail"),
+    url(r'^applications/(?P<package_name>[-.+\w]+)/$', 'application_detail',
+        name="wc-package-detail"),
     url(r'^search/$', 'search', name="wc-search"),
 )

=== modified file 'src/webcatalog/views.py'
--- src/webcatalog/views.py	2011-06-17 16:21:59 +0000
+++ src/webcatalog/views.py	2011-06-23 15:03:25 +0000
@@ -23,6 +23,9 @@
     )
 
 import operator
+from urllib import urlencode
+
+from django.core.urlresolvers import reverse
 from django.db.models import Q
 from django.template import RequestContext
 from django.shortcuts import (
@@ -52,9 +55,15 @@
             ors += [Q(name__icontains=term) for term in terms]
             apps = Application.objects.filter(reduce(operator.or_, ors))
 
+    crumbs = [{'name': 'Get Software', 'url': reverse('wc-index')},
+              {'name': 'Search results', 'url': reverse('wc-search') + '?' +
+                urlencode({'q': query})},
+             ]
+
     context = RequestContext(request, dict={
         'query': query,
         'apps': apps,
+        'breadcrumbs': crumbs,
         })
     return render_to_response('webcatalog/search_results.html',
         context_instance=context)
@@ -72,6 +81,18 @@
     context = RequestContext(request, dict={'dept': dept,
         'subdepts': subdepts,
         'apps': apps,
+        'breadcrumbs': dept.crumbs(),
         })
     return render_to_response('webcatalog/department_overview.html',
         context_instance=context)
+
+
+def application_detail(request, package_name):
+    app = get_object_or_404(Application, package_name=package_name)
+
+    context = RequestContext(request, {
+        'application': app,
+        'breadcrumbs': app.crumbs(),
+        })
+    return render_to_response('webcatalog/application_detail.html',
+        context_instance=context)