[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#779402: [Patch] vendor/debian: Replace the call to the old PTS cgi script for codesearch



Hello Orestis,

On Thu, 20 Aug 2015, Orestis Ioannou wrote:
> I ve implemented this replacement.
> You may find the patch attached

Thanks for this initial patch. I have a few comments and request for
changes if possible.

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).

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)

If you need more explanations on any of this, feel free to ask for
clarifications.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/


Reply to: