Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Improved the documentation search form and view.

The search form was designed so that, in the absence of a release ID,
it would yield search results corresponding to the current stable version.

However, there was a bug in the displaying of the form in that
the current release was not selected in the widget.

Another issue is that it was possible to trigger a 500 error by
passing an inexistant release ID to the page.

This commit fixes both those issues and a 404 is now raised when
passing an invalid release ID to the page.
  • Loading branch information...
commit 1bbdbdefc70dcd11169fea84ea593a0e7bbf73c1 1 parent 9753ef7
@bmispelon bmispelon authored
View
122 docs/fixtures/doc_test_fixtures.json
@@ -0,0 +1,122 @@
+[
+ {
+ "pk": 1,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "en",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git",
+ "is_default": false,
+ "version": "dev",
+ "docs_subdir": "docs"
+ }
+ },
+ {
+ "pk": 2,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "en",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git@stable/1.0.x",
+ "is_default": false,
+ "version": "1.0",
+ "docs_subdir": "docs"
+ }
+ },
+ {
+ "pk": 3,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "en",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git@stable/1.1.x",
+ "is_default": false,
+ "version": "1.1",
+ "docs_subdir": "docs"
+ }
+ },
+ {
+ "pk": 4,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "en",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git@stable/1.2.x",
+ "is_default": false,
+ "version": "1.2",
+ "docs_subdir": "docs"
+ }
+ },
+ {
+ "pk": 5,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "en",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git@stable/1.3.x",
+ "is_default": false,
+ "version": "1.3",
+ "docs_subdir": "docs"
+ }
+ },
+ {
+ "pk": 6,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "en",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git@stable/1.4.x",
+ "is_default": false,
+ "version": "1.4",
+ "docs_subdir": "docs"
+ }
+ },
+ {
+ "pk": 7,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "en",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git@stable/1.5.x",
+ "is_default": false,
+ "version": "1.5",
+ "docs_subdir": "docs"
+ }
+ },
+ {
+ "pk": 8,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "fr",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git@stable/1.5.x",
+ "is_default": false,
+ "version": "1.5",
+ "docs_subdir": "docs"
+ }
+ },
+ {
+ "pk": 9,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "en",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git@stable/1.6.x",
+ "is_default": true,
+ "version": "1.6",
+ "docs_subdir": "docs"
+ }
+ },
+ {
+ "pk": 10,
+ "model": "docs.documentrelease",
+ "fields": {
+ "lang": "fr",
+ "scm": "git",
+ "scm_url": "git://github.com/django/django.git@stable/1.6.x",
+ "is_default": false,
+ "version": "1.6",
+ "docs_subdir": "docs"
+ }
+ }
+]
View
55 docs/forms.py
@@ -3,32 +3,43 @@
from .models import DocumentRelease
+class DocumentReleaseChoiceField(forms.ModelChoiceField):
+ def __init__(self, *args, **kwargs):
+ kwargs.setdefault('required', False)
+ kwargs.setdefault('empty_label', None)
+ kwargs.setdefault('queryset', DocumentRelease.objects.order_by('version'))
+ super(DocumentReleaseChoiceField, self).__init__(*args, **kwargs)
+
+ def label_from_instance(self, obj):
+ return obj.human_version
+
+ def bound_data(self, data, initial):
+ """
+ If no data is given, return the initial data.
+ This allows for a default release to always be selected, even if none
+ was provided in the URL.
+ """
+ return data or initial
+
+
class DocSearchForm(haystack.forms.SearchForm):
+ release = DocumentReleaseChoiceField()
def __init__(self, data=None, **kwargs):
- if data and 'release' in data:
- self.initial_rel = DocumentRelease.objects.get(pk=data['release'])
- else:
- self.initial_rel = kwargs.pop('release', DocumentRelease.objects.current())
+ self.default_release = kwargs.pop('default_release')
super(DocSearchForm, self).__init__(data=data, **kwargs)
- self.fields['q'].widget = SearchInput()
- self.fields['release'] = DocumentReleaseChoiceField(
- queryset = DocumentRelease.objects.filter(lang=self.initial_rel.lang).order_by('version'),
- initial = self.initial_rel,
- empty_label = None,
- required = False,
- )
+ self.fields['q'].widget = forms.TextInput(attrs={'type': 'search'})
+ self.fields['release'].queryset = self.fields['release'].queryset.filter(lang=self.default_release.lang)
+ self.fields['release'].initial = self.default_release
def search(self):
- sqs = super(DocSearchForm, self).search()
- if self.is_valid():
- rel = self.cleaned_data['release'] or DocumentRelease.objects.current()
- sqs = sqs.filter(lang=rel.lang, version=rel.version)
- return sqs
-
-class DocumentReleaseChoiceField(forms.ModelChoiceField):
- def label_from_instance(self, obj):
- return obj.human_version
+ results = super(DocSearchForm, self).search()
+ assert self.cleaned_data # SearchForm.search() calls is_valid()
+ release = self.cleaned_data['release']
+ results = results.filter(lang=release.lang, version=release.version)
+ return results
-class SearchInput(forms.TextInput):
- input_type = 'search'
+ def clean_release(self):
+ """If no release is provided we fall back to the default release."""
+ release = self.cleaned_data.get('release')
+ return release if release else self.default_release
View
2  docs/templatetags/docs.py
@@ -13,7 +13,7 @@ def search_form(context, search_form_id='sidebar_search'):
auto_id = 'id_%s_%%s' % search_form_id
release = DocumentRelease.objects.get(version=context['version'], lang=context['lang'])
return {
- 'form': DocSearchForm(initial=request.GET, auto_id=auto_id, release=release),
+ 'form': DocSearchForm(request.GET, auto_id=auto_id, default_release=release),
'search_form_id': search_form_id,
}
View
48 docs/tests.py
@@ -0,0 +1,48 @@
+from django.test import TestCase
+
+from .forms import DocSearchForm
+from .models import DocumentRelease
+
+
+class SearchFormTestCase(TestCase):
+ fixtures = ['doc_test_fixtures']
+
+ def test_unbound_form(self):
+ """
+ An unbound form should have the default release selected.
+ """
+ release = DocumentRelease.objects.get(pk=1)
+ f = DocSearchForm(default_release=release)
+
+ self.assertIn('<option value="1" selected="selected">', f['release'].as_widget())
+
+ def test_bound_form(self):
+ """
+ If no release is passed to a bound form, the default release should
+ be selected.
+ """
+ release = DocumentRelease.objects.get(pk=1)
+ f = DocSearchForm({'q': 'foo'}, default_release=release)
+
+ self.assertIn('<option value="1" selected="selected">', f['release'].as_widget())
+
+ def test_bound_form_with_release(self):
+ """
+ If a release is passed to the form, it should superscede the default
+ release.
+ """
+ release = DocumentRelease.objects.get(pk=1)
+ f = DocSearchForm({'q': 'foo', 'release': 2}, default_release=release)
+
+ self.assertIn('<option value="2" selected="selected">', f['release'].as_widget())
+
+ def test_form_valid_without_release(self):
+ """
+ If no release is bound to the form, it is still valid and it yields
+ the default release passed at the construction.
+ """
+ release = DocumentRelease.objects.get(pk=1)
+ f = DocSearchForm({'q': 'foo'}, default_release=release)
+
+ self.assertTrue(f.is_valid())
+ self.assertEqual(f.cleaned_data['release'], release)
View
10 docs/views.py
@@ -6,7 +6,7 @@
import django.views.static
from django.core import urlresolvers
from django.http import Http404
-from django.shortcuts import render, redirect
+from django.shortcuts import render, redirect, get_object_or_404
from django.utils import translation
import haystack.views
@@ -109,9 +109,15 @@ def __init__(self, **kwargs):
})
super(DocSearchView, self).__init__(**kwargs)
+ def build_form(self, form_kwargs=None):
+ form_kwargs = {
+ 'default_release': get_object_or_404(DocumentRelease, pk=self.request.GET.get('release'))
+ }
+ return super(DocSearchView, self).build_form(form_kwargs)
+
def extra_context(self):
# Constuct a context that matches the rest of the doc page views.
- current_release = self.form.initial_rel
+ current_release = self.form.default_release
if current_release.lang != 'en':
translation.activate(current_release.lang)
return {
Please sign in to comment.
Something went wrong with that request. Please try again.