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/
