Heya, Thanks for the quick review
On Thu, 20 Aug 2015 13:47:07 +0200 Raphael Hertzog <hert...@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 <ores...@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
signature.asc
Description: OpenPGP digital signature