Merge lp:~elachuni/ubuntu-webcatalog/distro-searches into lp:ubuntu-webcatalog

Proposed by Anthony Lenton
Status: Merged
Approved by: Anthony Lenton
Approved revision: 40
Merged at revision: 41
Proposed branch: lp:~elachuni/ubuntu-webcatalog/distro-searches
Merge into: lp:ubuntu-webcatalog
Diff against target: 285 lines (+109/-31)
5 files modified
src/webcatalog/models/applications.py (+1/-1)
src/webcatalog/templates/webcatalog/search_results.html (+19/-0)
src/webcatalog/tests/test_views.py (+67/-24)
src/webcatalog/urls.py (+1/-0)
src/webcatalog/views.py (+21/-6)
To merge this branch: bzr merge lp:~elachuni/ubuntu-webcatalog/distro-searches
Reviewer Review Type Date Requested Status
Matthew Nuzum (community) Approve
Review via email: mp+67257@code.launchpad.net

Commit message

Make searches distro-series specific

Description of the change

Overview
========
Make searches distro-series specific

Details
=======
Until now, search results would include apps for all distroseries, repeated and intermixed. So if an app is available for four different distroseries, you would get it repeated four times in the search results.
While coding the solution different options were considered:
 - Showing an app in search results only once, if it's available in any distroseries. This would require a largeish model change to separate an App from an AppInADistroSeries, with many AppInADistroSeries for each App. And you still never know what distroseries an app you see in your search results is available for.
 - Leave search results as is, but add a small label next to each app stating which distroseries it's for. This would still result in multiple instances of almost identical apps in your search results.
 - Group search results by distroseries. This is what packages.ubuntu.com does, but it doesn't play well with paging results.
 - The current solution, that's to have the search limit to a default distroseries, and allow the user to specify a different distroseries easily (using a right navigation bar in this case).

While I was there, I made distroseries show up in a reverse code_name order in our right sidebars, so that Oneiric is always at the top, and older distroseries are shown further down.

To post a comment you must log in.
Revision history for this message
Matthew Nuzum (newz) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/webcatalog/models/applications.py'
--- src/webcatalog/models/applications.py 2011-07-07 14:15:04 +0000
+++ src/webcatalog/models/applications.py 2011-07-07 21:04:30 +0000
@@ -104,7 +104,7 @@
104 def available_distroseries(self):104 def available_distroseries(self):
105 """Return the set of distroseries for which this app is available"""105 """Return the set of distroseries for which this app is available"""
106 return DistroSeries.objects.filter(106 return DistroSeries.objects.filter(
107 application__package_name=self.package_name).order_by('version')107 application__package_name=self.package_name).order_by('-code_name')
108108
109 def update_departments(self):109 def update_departments(self):
110 """Update the list of departments for this app"""110 """Update the list of departments for this app"""
111111
=== modified file 'src/webcatalog/templates/webcatalog/search_results.html'
--- src/webcatalog/templates/webcatalog/search_results.html 2011-06-28 15:03:12 +0000
+++ src/webcatalog/templates/webcatalog/search_results.html 2011-07-07 21:04:30 +0000
@@ -7,6 +7,23 @@
7{% block content %}7{% block content %}
8 {% include "webcatalog/breadcrumbs_snippet.html" %}8 {% include "webcatalog/breadcrumbs_snippet.html" %}
99
10 <div id="rightbar">
11 <div class="portlet">
12 <div class="portletheader">Search within</div>
13 <ul class="portletactions">
14 {% for ds in available_distroseries %}
15 <li class="item{% ifequal ds.code_name distroseries %} active{% endifequal %}{% if forloop.last %} last{% endif %}">
16 {% ifequal ds.code_name distroseries %}
17 <span>Ubuntu {{ ds.version }} ({{ ds.code_name }})</span>
18 {% else %}
19 <a href="{% url wc-search ds.code_name %}?q={{query}}">Ubuntu {{ ds.version }} ({{ ds.code_name }})</a>
20 {% endifequal %}</li>
21 {% endfor %}
22 </ul>
23 </div>
24 </div>
25
26<div class="main-column">
10{% if query %}27{% if query %}
11<h3>{% blocktrans %}Searched for "{{query}}"{% endblocktrans %}</h3>28<h3>{% blocktrans %}Searched for "{{query}}"{% endblocktrans %}</h3>
12{% if page.object_list %}29{% if page.object_list %}
@@ -21,4 +38,6 @@
21{% else %}38{% else %}
22<h3>{% trans "You didn't search for anything!" %}</h3>39<h3>{% trans "You didn't search for anything!" %}</h3>
23{% endif %}40{% endif %}
41</div>
42
24{% endblock %}43{% endblock %}
2544
=== modified file 'src/webcatalog/tests/test_views.py'
--- src/webcatalog/tests/test_views.py 2011-07-04 14:47:05 +0000
+++ src/webcatalog/tests/test_views.py 2011-07-07 21:04:30 +0000
@@ -212,8 +212,9 @@
212class SearchTestCase(TestCaseWithFactory):212class SearchTestCase(TestCaseWithFactory):
213 def test_no_search_retrieves_no_apps(self):213 def test_no_search_retrieves_no_apps(self):
214 # Ensure there's some data in the DB214 # Ensure there's some data in the DB
215 self.factory.make_application()215 distro = self.factory.make_distroseries(code_name='lucid')
216 url = reverse('wc-search')216 self.factory.make_application(distroseries=distro)
217 url = reverse('wc-search', args=[distro.code_name])
217 for data in [{'q': ''}, {}]:218 for data in [{'q': ''}, {}]:
218219
219 response = self.client.get(url, data=data)220 response = self.client.get(url, data=data)
@@ -222,10 +223,14 @@
222 self.assertContains(response, "You didn't search for anything")223 self.assertContains(response, "You didn't search for anything")
223224
224 def test_search_searches_package_name(self):225 def test_search_searches_package_name(self):
225 app1 = self.factory.make_application(package_name='foo')226 distro = self.factory.make_distroseries(code_name='lucid')
226 app2 = self.factory.make_application(package_name='bar')227 app1 = self.factory.make_application(package_name='foo',
227 app3 = self.factory.make_application(package_name='foobar')228 distroseries=distro)
228 url = reverse('wc-search')229 app2 = self.factory.make_application(package_name='bar',
230 distroseries=distro)
231 app3 = self.factory.make_application(package_name='foobar',
232 distroseries=distro)
233 url = reverse('wc-search', args=[distro.code_name])
229 expected = {'foo': set([app1, app3]), 'bar': set([app2, app3])}234 expected = {'foo': set([app1, app3]), 'bar': set([app2, app3])}
230 for q in expected:235 for q in expected:
231236
@@ -235,10 +240,14 @@
235 expected[q], set(response.context['page'].object_list))240 expected[q], set(response.context['page'].object_list))
236241
237 def test_search_searches_name(self):242 def test_search_searches_name(self):
238 app1 = self.factory.make_application(name='foo')243 distro = self.factory.make_distroseries(code_name='lucid')
239 app2 = self.factory.make_application(name='bar')244 app1 = self.factory.make_application(name='foo',
240 app3 = self.factory.make_application(name='foobar')245 distroseries=distro)
241 url = reverse('wc-search')246 app2 = self.factory.make_application(name='bar',
247 distroseries=distro)
248 app3 = self.factory.make_application(name='foobar',
249 distroseries=distro)
250 url = reverse('wc-search', args=[distro.code_name])
242 expected = {'foo': set([app1, app3]), 'bar': set([app2, app3])}251 expected = {'foo': set([app1, app3]), 'bar': set([app2, app3])}
243 for q in expected:252 for q in expected:
244253
@@ -248,43 +257,53 @@
248 expected[q], set(response.context['page'].object_list))257 expected[q], set(response.context['page'].object_list))
249258
250 def test_search_no_apps_found_says_so(self):259 def test_search_no_apps_found_says_so(self):
251 url = reverse('wc-search')260 distro = self.factory.make_distroseries(code_name='lucid')
261 url = reverse('wc-search', args=[distro.code_name])
252262
253 response = self.client.get(url, data={'q': 'sarasa'})263 response = self.client.get(url, data={'q': 'sarasa'})
254264
255 self.assertContains(response, "No applications found")265 self.assertContains(response, "No applications found")
256266
257 def test_response_shows_query_if_no_apps_found(self):267 def test_response_shows_query_if_no_apps_found(self):
268 distro = self.factory.make_distroseries(code_name='lucid')
258 query = 'someimprobablequery'269 query = 'someimprobablequery'
259 url = reverse('wc-search')270 url = reverse('wc-search', args=[distro.code_name])
260271
261 response = self.client.get(url, data={'q': query})272 response = self.client.get(url, data={'q': query})
262273
263 self.assertContains(response, query)274 self.assertContains(response, query)
264275
265 def test_response_shows_query_if_some_apps_found(self):276 def test_response_shows_query_if_some_apps_found(self):
277 distro = self.factory.make_distroseries(code_name='lucid')
266 query = 'someimprobablequery'278 query = 'someimprobablequery'
267 self.factory.make_application(package_name=query)279 self.factory.make_application(package_name=query, distroseries=distro)
268 url = reverse('wc-search')280 url = reverse('wc-search', args=[distro.code_name])
269281
270 response = self.client.get(url, data={'q': query})282 response = self.client.get(url, data={'q': query})
271283
272 self.assertContains(response, query)284 self.assertContains(response, query)
273285
274 def test_response_shows_number_of_apps_found(self):286 def test_response_shows_number_of_apps_found(self):
275 app1 = self.factory.make_application(package_name='foo')287 distro = self.factory.make_distroseries(code_name='lucid')
276 app2 = self.factory.make_application(package_name='foobar')288 app1 = self.factory.make_application(package_name='foo',
277 url = reverse('wc-search')289 distroseries=distro)
290 app2 = self.factory.make_application(package_name='foobar',
291 distroseries=distro)
292 url = reverse('wc-search', args=[distro.code_name])
278293
279 response = self.client.get(url, data={'q': 'foo'})294 response = self.client.get(url, data={'q': 'foo'})
280295
281 self.assertContains(response, "1 to 2 of 2")296 self.assertContains(response, "1 to 2 of 2")
282297
283 def test_response_shows_paginated_results(self):298 def test_response_shows_paginated_results(self):
284 app1 = self.factory.make_application(name='foo')299 distro = self.factory.make_distroseries(code_name='lucid')
285 app2 = self.factory.make_application(name='fobar')300 app1 = self.factory.make_application(name='foo',
286 app3 = self.factory.make_application(name='foobar')301 distroseries=distro)
287 url = reverse('wc-search')302 app2 = self.factory.make_application(name='fobar',
303 distroseries=distro)
304 app3 = self.factory.make_application(name='foobar',
305 distroseries=distro)
306 url = reverse('wc-search', args=[distro.code_name])
288307
289 with patch_settings(PAGE_BATCH_SIZE=2):308 with patch_settings(PAGE_BATCH_SIZE=2):
290 response = self.client.get(url, data={'q': 'fo'})309 response = self.client.get(url, data={'q': 'fo'})
@@ -299,8 +318,9 @@
299 self.assertContains(response, '<a href="?q=fo&page=2"', 2)318 self.assertContains(response, '<a href="?q=fo&page=2"', 2)
300319
301 def test_invalid_page_doesnt_error(self):320 def test_invalid_page_doesnt_error(self):
302 self.factory.make_application(name='foo')321 distro = self.factory.make_distroseries(code_name='lucid')
303 url = reverse('wc-search')322 self.factory.make_application(name='foo', distroseries=distro)
323 url = reverse('wc-search', args=[distro.code_name])
304324
305 response = self.client.get(url, data={'q': 'fo', 'page': 'aef8'})325 response = self.client.get(url, data={'q': 'fo', 'page': 'aef8'})
306 page = response.context['page']326 page = response.context['page']
@@ -314,6 +334,11 @@
314 page = response.context['page']334 page = response.context['page']
315 self.assertEqual(1, page.number)335 self.assertEqual(1, page.number)
316336
337 def test_invalid_distroseries_returns_404(self):
338 response = self.client.get(reverse('wc-search', args=['zirconic']))
339
340 self.assertEqual(404, response.status_code)
341
317342
318class OverviewTestCase(TestCaseWithFactory):343class OverviewTestCase(TestCaseWithFactory):
319 def test_index_contains_links_to_departments(self):344 def test_index_contains_links_to_departments(self):
@@ -413,4 +438,22 @@
413 response = self.client.get(reverse('wc-department', args=[438 response = self.client.get(reverse('wc-department', args=[
414 'amnesiac', dept.id]))439 'amnesiac', dept.id]))
415440
416 self.assertEqual(404, response.status_code)
417\ No newline at end of file441\ No newline at end of file
442 self.assertEqual(404, response.status_code)
443
444 def test_distroseries_are_ordered_by_reverse_code_name(self):
445 dept = self.factory.make_department('bar')
446 default = self.factory.make_distroseries(
447 code_name=settings.DEFAULT_DISTRO, version='11.04')
448 lucid = self.factory.make_distroseries(code_name='lucid',
449 version='10.04')
450 maverick = self.factory.make_distroseries(code_name='maverick',
451 version='10.10')
452
453 response = self.client.get(reverse('wc-department', args=[
454 settings.DEFAULT_DISTRO, dept.id]))
455
456 url = reverse('wc-department', args=['lucid', dept.id])
457 lucid_pos = response.content.find('<a href="{0}">Ubuntu'.format(url))
458 url = reverse('wc-department', args=['maverick', dept.id])
459 maver_pos = response.content.find('<a href="{0}">Ubuntu'.format(url))
460 self.assertTrue(lucid_pos > maver_pos)
418461
=== modified file 'src/webcatalog/urls.py'
--- src/webcatalog/urls.py 2011-07-01 16:18:58 +0000
+++ src/webcatalog/urls.py 2011-07-07 21:04:30 +0000
@@ -40,6 +40,7 @@
40 url(r'^applications/(?P<package_name>[-.+\w]+)/$', 'application_detail',40 url(r'^applications/(?P<package_name>[-.+\w]+)/$', 'application_detail',
41 name="wc-package-detail"),41 name="wc-package-detail"),
42 url(r'^search/$', 'search', name="wc-search"),42 url(r'^search/$', 'search', name="wc-search"),
43 url(r'^search/(?P<distro>[-.+\w]+)/$', 'search', name="wc-search"),
4344
44 (r'^api/', include('webcatalog.api.urls')),45 (r'^api/', include('webcatalog.api.urls')),
4546
4647
=== modified file 'src/webcatalog/views.py'
--- src/webcatalog/views.py 2011-07-04 15:46:48 +0000
+++ src/webcatalog/views.py 2011-07-07 21:04:30 +0000
@@ -59,17 +59,30 @@
59 return 159 return 1
6060
6161
62def search(request):62def search(request, distro=None):
63 query = request.GET.get('q', '')
64 if distro is None:
65 useragent = UserAgentString(request.META.get('HTTP_USER_AGENT', ''))
66 # Check for the distroseries in the useragent, if we have it,
67 # redirect there.
68 if useragent.distroseries:
69 distro = useragent.distroseries
70 else:
71 distro = settings.DEFAULT_DISTRO
72 return HttpResponseRedirect(reverse('wc-search', args=[distro]) +
73 '?' + urlencode({'q': query}))
74
75 # Attempt to retrieve the DistroSeries to ensure that it actually exists
76 get_object_or_404(DistroSeries, code_name=distro)
77
63 apps = []78 apps = []
64 query = ''
65 if 'q' in request.GET:79 if 'q' in request.GET:
66 query = request.GET.get('q', '')
67 terms = query.split()80 terms = query.split()
68 if terms:81 if terms:
69 ors = [Q(package_name__icontains=term) for term in terms]82 ors = [Q(package_name__icontains=term) for term in terms]
70 ors += [Q(name__icontains=term) for term in terms]83 ors += [Q(name__icontains=term) for term in terms]
71 apps = Application.objects.filter(reduce(operator.or_, ors))84 apps = Application.objects.filter(distroseries__code_name=distro)
72 apps = apps.order_by('name')85 apps = apps.filter(reduce(operator.or_, ors)).order_by('name')
7386
74 crumbs = [{'name': 'Get Software', 'url': reverse('wc-index')},87 crumbs = [{'name': 'Get Software', 'url': reverse('wc-index')},
75 {'name': 'Search results', 'url': reverse('wc-search') + '?' +88 {'name': 'Search results', 'url': reverse('wc-search') + '?' +
@@ -83,6 +96,8 @@
83 'query': query,96 'query': query,
84 'page': paginator.page(page_num),97 'page': paginator.page(page_num),
85 'breadcrumbs': crumbs,98 'breadcrumbs': crumbs,
99 'available_distroseries': DistroSeries.objects.order_by('-code_name'),
100 'distroseries': distro,
86 })101 })
87 return render_to_response('webcatalog/search_results.html',102 return render_to_response('webcatalog/search_results.html',
88 context_instance=context)103 context_instance=context)
@@ -125,7 +140,7 @@
125 'subdepts': subdepts,140 'subdepts': subdepts,
126 'page': paginator.page(page_num),141 'page': paginator.page(page_num),
127 'breadcrumbs': dept.crumbs(),142 'breadcrumbs': dept.crumbs(),
128 'available_distroseries': DistroSeries.objects.all(),143 'available_distroseries': DistroSeries.objects.order_by('-code_name'),
129 'distroseries': distro,144 'distroseries': distro,
130 })145 })
131 return render_to_response('webcatalog/department_overview.html',146 return render_to_response('webcatalog/department_overview.html',

Subscribers

People subscribed via source and target branches