Heya,
Thanks for the quick review
On Thu, 20 Aug 2015 13:47:07 +0200 Raphael Hertzog <hertzog@debian.org>
wrote:
> First, the form that actually requires this view is only in
> distro_tracker.vendor.debian Django application. So the view
> should be in the same application. You should create
> distro_tracker/vendor/debian/views.py and put the view there.
>
> Then you should create distro_tracker/vendor/debian/tracker_urls.py
> with an "urlpatterns" attribute similar to the one available
> in distro_tracker/project/urls.py. It will be auto-imported there
> by some code at the end of this file (look for the loop over
> INSTALLED_APPS).
I was thinking doing this in the first place but I wasn't sure how to
include them so thanks for the clarification
> Then it would be nice if you could create a few unit tests ensuring
> that the view is actually working as expected:
> - submit simple valid data and ensure you are redirected where you want
> - submit valid data that needs to be urlencoded and ensure it works the
> way it should
> - submit invalid data ("q" arg missing, "package" arg missing) and ensures
> the page displayed has some sort of error message (and possibly use
> something else than HTTP 404 which is misleading, better use
> HttpResponseBadRequest)
>
Added some tests as well, hope they are what you suggested.
I attached the new patch as well.
Cheers,
Orestis
From 1c8538a7e4b53b122c8927b40ad2acdf81dfe857 Mon Sep 17 00:00:00 2001
From: Orestis Ioannou <orestis@oioannou.com>
Date: Fri, 21 Aug 2015 22:32:15 +0200
Subject: [PATCH] vendor/debian: Replace old PTS cgi script for codesearch
---
distro_tracker/vendor/debian/tests.py | 36 ++++++++++++++++++++++++++
distro_tracker/vendor/debian/tracker_panels.py | 4 +--
distro_tracker/vendor/debian/tracker_urls.py | 23 ++++++++++++++++
distro_tracker/vendor/debian/views.py | 33 +++++++++++++++++++++++
4 files changed, 94 insertions(+), 2 deletions(-)
create mode 100644 distro_tracker/vendor/debian/tracker_urls.py
create mode 100644 distro_tracker/vendor/debian/views.py
diff --git a/distro_tracker/vendor/debian/tests.py b/distro_tracker/vendor/debian/tests.py
index 4ef60f2..55d39da 100644
--- a/distro_tracker/vendor/debian/tests.py
+++ b/distro_tracker/vendor/debian/tests.py
@@ -24,6 +24,7 @@ from django.utils import six
from django.utils.six.moves import mock
from django.utils.encoding import force_bytes
from django.utils.functional import curry
+from django.utils.http import urlencode
from distro_tracker.mail.tests.tests_dispatch \
import DispatchTestHelperMixin, DispatchBaseTest
from distro_tracker.accounts.models import User
@@ -2713,6 +2714,41 @@ class CodeSearchLinksTest(TestCase):
self.assertFalse(self.browse_link_in_content(response_content))
self.assertFalse(self.search_form_in_content(response_content))
+ def test_code_search_view(self):
+ """
+ Tests that both required parameters are present and not empty and that
+ urlencode works properly.
+ """
+ # missing query paramter
+ response = self.client.get(reverse('dtracker-code-search'),
+ {'package': self.package.name})
+ self.assertEqual(response.status_code, 400)
+ self.assertIn('Both package and query are required parameters',
+ response.content.decode('utf-8'))
+ # missing package parameter
+ response = self.client.get(reverse('dtracker-code-search'),
+ {'query': 'def'})
+ self.assertEqual(response.status_code, 400)
+ # empty query
+ response = self.client.get(reverse('dtracker-code-search'),
+ {'package': self.package.name,
+ 'query': ''})
+ self.assertEqual(response.status_code, 400)
+ self.assertIn('Empty query is not allowed',
+ response.content.decode('utf-8'))
+ # redirection to codesearch
+ response = self.client.get(reverse('dtracker-code-search'),
+ {'package': self.package.name,
+ 'query': 'def'})
+ self.assertEqual(response.status_code, 302)
+ self.assertIn('codesearch.debian', response['Location'])
+ # verify url encoding
+ cs = 'https://codesearch.debian.net/search?'
+ search = 'def' + ' package:' + self.package.name
+ url = cs + urlencode({'q': search})
+ self.assertEqual('https://codesearch.debian.net/search?q=def+package%3A'
+ 'dummy', url)
+
class PopconLinkTest(TestCase):
diff --git a/distro_tracker/vendor/debian/tracker_panels.py b/distro_tracker/vendor/debian/tracker_panels.py
index a8d436a..cdfb400 100644
--- a/distro_tracker/vendor/debian/tracker_panels.py
+++ b/distro_tracker/vendor/debian/tracker_panels.py
@@ -132,10 +132,10 @@ class SourceCodeSearchLinks(LinksPanel.ItemProvider):
SOURCES_URL_TEMPLATE = 'https://sources.debian.net/src/{package}/{suite}/'
SEARCH_FORM_TEMPLATE = (
'<form class="code-search-form"'
- ' action="https://packages.qa.debian.org/cgi-bin/codesearch.cgi";'
+ ' action="/codesearch/"'
' method="get" target="_blank">'
'<input type="hidden" name="package" value="{package}">'
- '<input type="text" name="q" placeholder="search source code">'
+ '<input type="text" name="query" placeholder="search source code">'
'</form>')
def get_panel_items(self):
diff --git a/distro_tracker/vendor/debian/tracker_urls.py b/distro_tracker/vendor/debian/tracker_urls.py
new file mode 100644
index 0000000..022b210
--- /dev/null
+++ b/distro_tracker/vendor/debian/tracker_urls.py
@@ -0,0 +1,23 @@
+# Copyright 2015 The Distro Tracker Developers
+# See the COPYRIGHT file at the top-level directory of this distribution and
+# at http://deb.li/DTAuthors
+#
+# This file is part of Distro Tracker. It is subject to the license terms
+# in the LICENSE file found in the top-level directory of this
+# distribution and at http://deb.li/DTLicense. No part of Distro Tracker,
+# including this file, may be copied, modified, propagated, or distributed
+# except according to the terms contained in the LICENSE file.
+"""The URL routes for the vendor.debian app."""
+from __future__ import unicode_literals
+
+from django.conf.urls import patterns, url
+
+from distro_tracker.vendor.debian.views import CodeSearchView
+
+
+urlpatterns = patterns(
+ '',
+ # code search
+ url(r'^codesearch/$', CodeSearchView.as_view(),
+ name='dtracker-code-search'),
+)
diff --git a/distro_tracker/vendor/debian/views.py b/distro_tracker/vendor/debian/views.py
new file mode 100644
index 0000000..aed16f2
--- /dev/null
+++ b/distro_tracker/vendor/debian/views.py
@@ -0,0 +1,33 @@
+# Copyright 2015 The Distro Tracker Developers
+# See the COPYRIGHT file at the top-level directory of this distribution and
+# at http://deb.li/DTAuthors
+#
+# This file is part of Distro Tracker. It is subject to the license terms
+# in the LICENSE file found in the top-level directory of this
+# distribution and at http://deb.li/DTLicense. No part of Distro Tracker,
+# including this file, may be copied, modified, propagated, or distributed
+# except according to the terms contained in the LICENSE file.
+"""Views for the :mod:`distro_tracker.vendor` app."""
+from __future__ import unicode_literals
+
+from django.utils.http import urlencode
+from django.views.generic import View
+from django.http import HttpResponseBadRequest
+from django.shortcuts import redirect
+
+
+class CodeSearchView(View):
+
+ def get(self, request):
+ if 'query' not in request.GET or 'package' not in request.GET:
+ return HttpResponseBadRequest('Both package and query are required '
+ 'parameters')
+ if request.GET.get('query') == "":
+ return HttpResponseBadRequest('Empty query is not allowed')
+ q = request.GET.get('query')
+ package = request.GET.get('package')
+ cs = 'https://codesearch.debian.net/search?'
+ search = q + ' package:' + package
+
+ url = cs + urlencode({'q': search})
+ return redirect(url)
--
2.1.4
Attachment:
signature.asc
Description: OpenPGP digital signature