Fixed #24907 -- Updated contributing tutorial with a more recent example ticket.

This commit is contained in:
Markus Amalthea Magnuson 2015-06-05 01:06:03 +01:00 committed by Tim Graham
parent 7abf418eb1
commit 3fd754f12d
2 changed files with 205 additions and 200 deletions

View File

@ -97,15 +97,19 @@ The first step to contributing to Django is to get a copy of the source code.
From the command line, use the ``cd`` command to navigate to the directory
where you'll want your local copy of Django to live.
Download the Django source code repository using the following command::
Download the Django source code repository using the following command:
git clone https://github.com/django/django.git
.. code-block:: console
$ git clone https://github.com/django/django.git
.. note::
For users who wish to use `virtualenv`__, you can use::
For users who wish to use `virtualenv`__, you can use:
pip install -e /path/to/your/local/clone/django/
.. code-block:: console
$ pip install -e /path/to/your/local/clone/django/
(where ``django`` is the directory of your clone that contains
``setup.py``) to link your cloned checkout into a virtual environment. This
@ -117,7 +121,7 @@ __ http://www.virtualenv.org
Rolling back to a previous revision of Django
=============================================
For this tutorial, we'll be using ticket :ticket:`17549` as a case study, so we'll
For this tutorial, we'll be using ticket :ticket:`24788` as a case study, so we'll
rewind Django's version history in git to before that ticket's patch was
applied. This will allow us to go through all of the steps involved in writing
that patch from scratch, including running Django's test suite.
@ -128,36 +132,46 @@ development revision of Django when working on your own patch for a ticket!**
.. note::
The patch for this ticket was written by Ulrich Petri, and it was applied
to Django as `commit ac2052ebc84c45709ab5f0f25e685bf656ce79bc`__.
The patch for this ticket was written by Paweł Marczewski, and it was
applied to Django as `commit 4df7e8483b2679fc1cba3410f08960bac6f51115`__.
Consequently, we'll be using the revision of Django just prior to that,
`commit 39f5bc7fc3a4bb43ed8a1358b17fe0521a1a63ac`__.
`commit 4ccfc4439a7add24f8db4ef3960d02ef8ae09887`__.
__ https://github.com/django/django/commit/ac2052ebc84c45709ab5f0f25e685bf656ce79bc
__ https://github.com/django/django/commit/39f5bc7fc3a4bb43ed8a1358b17fe0521a1a63ac
__ https://github.com/django/django/commit/4df7e8483b2679fc1cba3410f08960bac6f51115
__ https://github.com/django/django/commit/4ccfc4439a7add24f8db4ef3960d02ef8ae09887
Navigate into Django's root directory (that's the one that contains ``django``,
``docs``, ``tests``, ``AUTHORS``, etc.). You can then check out the older
revision of Django that we'll be using in the tutorial below::
revision of Django that we'll be using in the tutorial below:
git checkout 39f5bc7fc3a4bb43ed8a1358b17fe0521a1a63ac
.. code-block:: console
$ git checkout 4ccfc4439a7add24f8db4ef3960d02ef8ae09887
Running Django's test suite for the first time
==============================================
When contributing to Django it's very important that your code changes don't
introduce bugs into other areas of Django. One way to check that Django still
introduce bugs into other areas of Django. One way to check that Django still
works after you make your changes is by running Django's test suite. If all
the tests still pass, then you can be reasonably sure that your changes
haven't completely broken Django. If you've never run Django's test suite
before, it's a good idea to run it once beforehand just to get familiar with
what its output is supposed to look like.
We can run the test suite by simply ``cd``-ing into the Django ``tests/``
directory and, if you're using GNU/Linux, Mac OS X or some other flavor of
Unix, run::
Before running the test suite, install its dependencies by first ``cd``-ing
into the Django ``tests/`` directory and then running:
PYTHONPATH=.. python runtests.py --settings=test_sqlite
.. code-block:: console
$ pip install -r requirements/py3.txt # or py2.txt if you are running Python 2
Now we are ready to run the test suite. If you're using GNU/Linux, Mac OS X or
some other flavor of Unix, run:
.. code-block:: console
$ PYTHONPATH=.. ./runtests.py
If you're on Windows, the above should work provided that you are using
"Git Bash" provided by the default Git install. GitHub has a `nice tutorial`__.
@ -171,7 +185,7 @@ __ https://help.github.com/articles/set-up-git#platform-windows
of ``tests``. ``virtualenv`` puts your copy of Django on the ``PYTHONPATH``
automatically.
Now sit back and relax. Django's entire test suite has over 4800 different
Now sit back and relax. Django's entire test suite has over 9,600 different
tests, so it can take anywhere from 5 to 15 minutes to run, depending on the
speed of your computer.
@ -234,56 +248,42 @@ Now for our hands-on example.
__ http://en.wikipedia.org/wiki/Test-driven_development
Writing some tests for ticket #17549
Writing some tests for ticket #24788
------------------------------------
Ticket :ticket:`17549` describes the following, small feature addition:
Ticket :ticket:`24788` proposes a small feature addition: the ability to
specify the class level attribute ``prefix`` on Form classes, so that::
It's useful for URLField to give you a way to open the URL; otherwise you
might as well use a CharField.
[…] forms which ship with apps could effectively namespace themselves such
that N overlapping form fields could be POSTed at once and resolved to the
correct form.
In order to resolve this ticket, we'll add a ``render`` method to the
``AdminURLFieldWidget`` in order to display a clickable link above the input
widget. Before we make those changes though, we're going to write a couple
tests to verify that our modification functions correctly and continues to
function correctly in the future.
In order to resolve this ticket, we'll add a ``prefix`` attribute to the
``BaseForm`` class. When creating instances of this class, passing a prefix to
the ``__init__()`` method will still set that prefix on the created instance.
But not passing a prefix (or passing ``None``) will use the class-level prefix.
Before we make those changes though, we're going to write a couple tests to
verify that our modification functions correctly and continues to function
correctly in the future.
Navigate to Django's ``tests/regressiontests/admin_widgets/`` folder and
open the ``tests.py`` file. Add the following code on line 269 right before the
``AdminFileWidgetTest`` class::
Navigate to Django's ``tests/forms_tests/tests/`` folder and open the
``test_forms.py`` file. Add the following code on line 1674 right before the
``test_forms_with_null_boolean`` function::
class AdminURLWidgetTest(DjangoTestCase):
def test_render(self):
w = widgets.AdminURLFieldWidget()
self.assertHTMLEqual(
conditional_escape(w.render('test', '')),
'<input class="vURLField" name="test" type="text" />'
)
self.assertHTMLEqual(
conditional_escape(w.render('test', 'http://example.com')),
'<p class="url">Currently:<a href="http://example.com">http://example.com</a><br />Change:<input class="vURLField" name="test" type="text" value="http://example.com" /></p>'
)
def test_class_prefix(self):
# Prefix can be also specified at the class level.
class Person(Form):
first_name = CharField()
prefix = 'foo'
def test_render_idn(self):
w = widgets.AdminURLFieldWidget()
self.assertHTMLEqual(
conditional_escape(w.render('test', 'http://example-äüö.com')),
'<p class="url">Currently:<a href="http://xn--example--7za4pnc.com">http://example-äüö.com</a><br />Change:<input class="vURLField" name="test" type="text" value="http://example-äüö.com" /></p>'
)
p = Person()
self.assertEqual(p.prefix, 'foo')
def test_render_quoting(self):
w = widgets.AdminURLFieldWidget()
self.assertHTMLEqual(
conditional_escape(w.render('test', 'http://example.com/<sometag>some text</sometag>')),
'<p class="url">Currently:<a href="http://example.com/%3Csometag%3Esome%20text%3C/sometag%3E">http://example.com/&lt;sometag&gt;some text&lt;/sometag&gt;</a><br />Change:<input class="vURLField" name="test" type="text" value="http://example.com/<sometag>some text</sometag>" /></p>'
)
self.assertHTMLEqual(
conditional_escape(w.render('test', 'http://example-äüö.com/<sometag>some text</sometag>')),
'<p class="url">Currently:<a href="http://xn--example--7za4pnc.com/%3Csometag%3Esome%20text%3C/sometag%3E">http://example-äüö.com/&lt;sometag&gt;some text&lt;/sometag&gt;</a><br />Change:<input class="vURLField" name="test" type="text" value="http://example-äüö.com/<sometag>some text</sometag>" /></p>'
)
p = Person(prefix='bar')
self.assertEqual(p.prefix, 'bar')
The new tests check to see that the ``render`` method we'll be adding works
correctly in a couple different situations.
This new test checks that setting a class level prefix works as expected, and
that passing a ``prefix`` parameter when creating an instance still works too.
.. admonition:: But this testing thing looks kinda hard...
@ -304,68 +304,67 @@ __ https://docs.python.org/library/unittest.html
Running your new test
---------------------
Remember that we haven't actually made any modifications to
``AdminURLFieldWidget`` yet, so our tests are going to fail. Let's run all the
tests in the ``model_forms_regress`` folder to make sure that's really what
happens. From the command line, ``cd`` into the Django ``tests/`` directory
and run::
Remember that we haven't actually made any modifications to ``BaseForm`` yet,
so our tests are going to fail. Let's run all the tests in the ``forms_tests``
folder to make sure that's really what happens. From the command line, ``cd``
into the Django ``tests/`` directory and run:
PYTHONPATH=.. python runtests.py --settings=test_sqlite admin_widgets
.. code-block:: console
If the tests ran correctly, you should see three failures corresponding to each
of the test methods we added. If all of the tests passed, then you'll want to
make sure that you added the new test shown above to the appropriate folder and
class.
$ PYTHONPATH=.. ./runtests.py forms_tests
If the tests ran correctly, you should see one failure corresponding to the test
method we added. If all of the tests passed, then you'll want to make sure that
you added the new test shown above to the appropriate folder and class.
Writing the code for your ticket
================================
Next we'll be adding the functionality described in ticket :ticket:`17549` to
Next we'll be adding the functionality described in ticket :ticket:`24788` to
Django.
Writing the code for ticket #17549
Writing the code for ticket #24788
----------------------------------
Navigate to the ``django/django/contrib/admin/`` folder and open the
``widgets.py`` file. Find the ``AdminURLFieldWidget`` class on line 302 and add
the following ``render`` method after the existing ``__init__`` method::
Navigate to the ``django/django/forms/`` folder and open the ``forms.py`` file.
Find the ``BaseForm`` class on line 72 and add the ``prefix`` class attribute
right after the ``field_order`` attribute::
def render(self, name, value, attrs=None):
html = super(AdminURLFieldWidget, self).render(name, value, attrs)
if value:
value = force_text(self._format_value(value))
final_attrs = {'href': mark_safe(smart_urlquote(value))}
html = format_html(
'<p class="url">{} <a {}>{}</a><br />{} {}</p>',
_('Currently:'), flatatt(final_attrs), value,
_('Change:'), html
)
return html
class BaseForm(object):
# This is the main implementation of all the Form logic. Note that this
# class is different than Form. See the comments by the Form class for more
# information. Any improvements to the form API should be made to *this*
# class, not to the Form class.
field_order = None
prefix = None
Verifying your test now passes
------------------------------
Once you're done modifying Django, we need to make sure that the tests we wrote
earlier pass, so we can see whether the code we wrote above is working
correctly. To run the tests in the ``admin_widgets`` folder, ``cd`` into the
Django ``tests/`` directory and run::
correctly. To run the tests in the ``forms_tests`` folder, ``cd`` into the
Django ``tests/`` directory and run:
PYTHONPATH=.. python runtests.py --settings=test_sqlite admin_widgets
.. code-block:: console
Oops, good thing we wrote those tests! You should still see 3 failures with
$ PYTHONPATH=.. ./runtests.py forms_tests
Oops, good thing we wrote those tests! You should still see one failure with
the following exception::
NameError: global name 'smart_urlquote' is not defined
AssertionError: None != 'foo'
We forgot to add the import for that method. Go ahead and add the
``smart_urlquote`` import at the end of line 13 of
``django/contrib/admin/widgets.py`` so it looks as follows::
We forgot to add the conditional statement in the ``__init__`` method. Go ahead
and change ``self.prefix = prefix`` that is now on line 87 of
``django/forms/forms.py``, adding a conditional statement::
from django.utils.html import escape, format_html, format_html_join, smart_urlquote
if prefix is not None:
self.prefix = prefix
Re-run the tests and everything should pass. If it doesn't, make sure you
correctly modified the ``AdminURLFieldWidget`` class as shown above and
copied the new tests correctly.
correctly modified the ``BaseForm`` class as shown above and copied the new test
correctly.
Running Django's test suite for the second time
===============================================
@ -377,27 +376,36 @@ passing the entire test suite doesn't guarantee your code is bug free, it does
help identify many bugs and regressions that might otherwise go unnoticed.
To run the entire Django test suite, ``cd`` into the Django ``tests/``
directory and run::
directory and run:
PYTHONPATH=.. python runtests.py --settings=test_sqlite
.. code-block:: console
As long as you don't see any failures, you're good to go. Note that this fix
also made a `small CSS change`__ to format the new widget. You can make the
change if you'd like, but we'll skip it for now in the interest of brevity.
$ PYTHONPATH=.. ./runtests.py
__ https://github.com/django/django/commit/ac2052ebc84c45709ab5f0f25e685bf656ce79bc#diff-0
As long as you don't see any failures, you're good to go.
Writing Documentation
=====================
This is a new feature, so it should be documented. Add the following on line
925 of ``django/docs/ref/models/fields.txt`` beneath the existing docs for
``URLField``::
This is a new feature, so it should be documented. Add the following section on
line 1068 (at the end of the file) of ``django/docs/ref/forms/api.txt``::
.. versionadded:: 1.5
The prefix can also be specified on the form class::
The current value of the field will be displayed as a clickable link above the
input widget.
>>> class PersonForm(forms.Form):
... ...
... prefix = 'person'
.. versionadded:: 1.9
The ability to specify ``prefix`` on the form class was added.
Since this new feature will be in an upcoming release it is also added to the
release notes for Django 1.9, on line 164 under the "Forms" section in the file
``docs/releases/1.9.txt``::
* A form prefix can be specified inside a form class, not only when
instantiating a form. See :ref:`form-prefix` for details.
For more information on writing documentation, including an explanation of what
the ``versionadded`` bit is all about, see
@ -410,113 +418,106 @@ Generating a patch for your changes
Now it's time to generate a patch file that can be uploaded to Trac or applied
to another copy of Django. To get a look at the content of your patch, run the
following command::
following command:
git diff
.. code-block:: console
$ git diff
This will display the differences between your current copy of Django (with
your changes) and the revision that you initially checked out earlier in the
tutorial.
Once you're done looking at the patch, hit the ``q`` key to exit back to the
command line. If the patch's content looked okay, you can run the following
command to save the patch file to your current working directory::
command line. If the patch's content looked okay, you can run the following
command to save the patch file to your current working directory:
git diff > 17549.diff
.. code-block:: console
You should now have a file in the root Django directory called ``17549.diff``.
$ git diff > 24788.diff
You should now have a file in the root Django directory called ``24788.diff``.
This patch file contains all your changes and should look this:
.. code-block:: diff
diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py
index 1e0bc2d..9e43a10 100644
--- a/django/contrib/admin/widgets.py
+++ b/django/contrib/admin/widgets.py
@@ -10,7 +10,7 @@ from django.contrib.admin.templatetags.admin_static import static
from django.core.urlresolvers import reverse
from django.forms.widgets import RadioFieldRenderer
from django.forms.util import flatatt
-from django.utils.html import escape, format_html, format_html_join
+from django.utils.html import escape, format_html, format_html_join, smart_urlquote
from django.utils.text import Truncator
from django.utils.translation import ugettext as _
from django.utils.safestring import mark_safe
@@ -306,6 +306,18 @@ class AdminURLFieldWidget(forms.TextInput):
final_attrs.update(attrs)
super(AdminURLFieldWidget, self).__init__(attrs=final_attrs)
diff --git a/django/forms/forms.py b/django/forms/forms.py
index 509709f..d1370de 100644
--- a/django/forms/forms.py
+++ b/django/forms/forms.py
@@ -75,6 +75,7 @@ class BaseForm(object):
# information. Any improvements to the form API should be made to *this*
# class, not to the Form class.
field_order = None
+ prefix = None
+ def render(self, name, value, attrs=None):
+ html = super(AdminURLFieldWidget, self).render(name, value, attrs)
+ if value:
+ value = force_text(self._format_value(value))
+ final_attrs = {'href': mark_safe(smart_urlquote(value))}
+ html = format_html(
+ '<p class="url">{} <a {}>{}</a><br />{} {}</p>',
+ _('Currently:'), flatatt(final_attrs), value,
+ _('Change:'), html
+ )
+ return html
def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
initial=None, error_class=ErrorList, label_suffix=None,
@@ -83,7 +84,8 @@ class BaseForm(object):
self.data = data or {}
self.files = files or {}
self.auto_id = auto_id
- self.prefix = prefix
+ if prefix is not None:
+ self.prefix = prefix
self.initial = initial or {}
self.error_class = error_class
# Translators: This is the default suffix added to form field labels
diff --git a/docs/ref/forms/api.txt b/docs/ref/forms/api.txt
index 3bc39cd..008170d 100644
--- a/docs/ref/forms/api.txt
+++ b/docs/ref/forms/api.txt
@@ -1065,3 +1065,13 @@ You can put several Django forms inside one ``<form>`` tag. To give each
>>> print(father.as_ul())
<li><label for="id_father-first_name">First name:</label> <input type="text" name="father-first_name" id="id_father-first_name" /></li>
<li><label for="id_father-last_name">Last name:</label> <input type="text" name="father-last_name" id="id_father-last_name" /></li>
+
class AdminIntegerFieldWidget(forms.TextInput):
class_name = 'vIntegerField'
diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt
index 809d56e..d44f85f 100644
--- a/docs/ref/models/fields.txt
+++ b/docs/ref/models/fields.txt
@@ -922,6 +922,10 @@ Like all :class:`CharField` subclasses, :class:`URLField` takes the optional
:attr:`~CharField.max_length`argument. If you don't specify
:attr:`~CharField.max_length`, a default of 200 is used.
+.. versionadded:: 1.5
+The prefix can also be specified on the form class::
+
+The current value of the field will be displayed as a clickable link above the
+input widget.
Relationship fields
===================
diff --git a/tests/regressiontests/admin_widgets/tests.py b/tests/regressiontests/admin_widgets/tests.py
index 4b11543..94acc6d 100644
--- a/tests/regressiontests/admin_widgets/tests.py
+++ b/tests/regressiontests/admin_widgets/tests.py
@@ -265,6 +265,35 @@ class AdminSplitDateTimeWidgetTest(DjangoTestCase):
'<p class="datetime">Datum: <input value="01.12.2007" type="text" class="vDateField" name="test_0" size="10" /><br />Zeit: <input value="09:30:00" type="text" class="vTimeField" name="test_1" size="8" /></p>',
)
+class AdminURLWidgetTest(DjangoTestCase):
+ def test_render(self):
+ w = widgets.AdminURLFieldWidget()
+ self.assertHTMLEqual(
+ conditional_escape(w.render('test', '')),
+ '<input class="vURLField" name="test" type="text" />'
+ )
+ self.assertHTMLEqual(
+ conditional_escape(w.render('test', 'http://example.com')),
+ '<p class="url">Currently:<a href="http://example.com">http://example.com</a><br />Change:<input class="vURLField" name="test" type="text" value="http://example.com" /></p>'
+ )
+ >>> class PersonForm(forms.Form):
+ ... ...
+ ... prefix = 'person'
+
+ def test_render_idn(self):
+ w = widgets.AdminURLFieldWidget()
+ self.assertHTMLEqual(
+ conditional_escape(w.render('test', 'http://example-äüö.com')),
+ '<p class="url">Currently:<a href="http://xn--example--7za4pnc.com">http://example-äüö.com</a><br />Change:<input class="vURLField" name="test" type="text" value="http://example-äüö.com" /></p>'
+ )
+.. versionadded:: 1.9
+
+ def test_render_quoting(self):
+ w = widgets.AdminURLFieldWidget()
+ self.assertHTMLEqual(
+ conditional_escape(w.render('test', 'http://example.com/<sometag>some text</sometag>')),
+ '<p class="url">Currently:<a href="http://example.com/%3Csometag%3Esome%20text%3C/sometag%3E">http://example.com/&lt;sometag&gt;some text&lt;/sometag&gt;</a><br />Change:<input class="vURLField" name="test" type="text" value="http://example.com/<sometag>some text</sometag>" /></p>'
+ )
+ self.assertHTMLEqual(
+ conditional_escape(w.render('test', 'http://example-äüö.com/<sometag>some text</sometag>')),
+ '<p class="url">Currently:<a href="http://xn--example--7za4pnc.com/%3Csometag%3Esome%20text%3C/sometag%3E">http://example-äüö.com/&lt;sometag&gt;some text&lt;/sometag&gt;</a><br />Change:<input class="vURLField" name="test" type="text" value="http://example-äüö.com/<sometag>some text</sometag>" /></p>'
+ )
+ The ability to specify ``prefix`` on the form class was added.
diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt
index 5b58f79..f9bb9de 100644
--- a/docs/releases/1.9.txt
+++ b/docs/releases/1.9.txt
@@ -161,6 +161,9 @@ Forms
:attr:`~django.forms.Form.field_order` attribute, the ``field_order``
constructor argument , or the :meth:`~django.forms.Form.order_fields` method.
class AdminFileWidgetTest(DjangoTestCase):
def test_render(self):
+* A form prefix can be specified inside a form class, not only when
+ instantiating a form. See :ref:`form-prefix` for details.
+
Generic Views
^^^^^^^^^^^^^
diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py
index 690f205..e07fae2 100644
--- a/tests/forms_tests/tests/test_forms.py
+++ b/tests/forms_tests/tests/test_forms.py
@@ -1671,6 +1671,18 @@ class FormsTestCase(SimpleTestCase):
self.assertEqual(p.cleaned_data['last_name'], 'Lennon')
self.assertEqual(p.cleaned_data['birthday'], datetime.date(1940, 10, 9))
+ def test_class_prefix(self):
+ # Prefix can be also specified at the class level.
+ class Person(Form):
+ first_name = CharField()
+ prefix = 'foo'
+
+ p = Person()
+ self.assertEqual(p.prefix, 'foo')
+
+ p = Person(prefix='bar')
+ self.assertEqual(p.prefix, 'bar')
+
def test_forms_with_null_boolean(self):
# NullBooleanField is a bit of a special case because its presentation (widget)
# is different than its data. This is handled transparently, though.
So what do I do next?
=====================
@ -529,10 +530,12 @@ oriented workflow </internals/contributing/writing-code/working-with-git>` is
recommended.
Since we never committed our changes locally, perform the following to get your
git branch back to a good starting point::
git branch back to a good starting point:
git reset --hard HEAD
git checkout master
.. code-block:: console
$ git reset --hard HEAD
$ git checkout master
More information for new contributors
-------------------------------------
@ -561,7 +564,7 @@ Finding your first real ticket
Once you've looked through some of that information, you'll be ready to go out
and find a ticket of your own to write a patch for. Pay special attention to
tickets with the "easy pickings" criterion. These tickets are often much
simpler in nature and are great for first time contributors. Once you're
simpler in nature and are great for first time contributors. Once you're
familiar with contributing to Django, you can move on to writing patches for
more difficult and complicated tickets.

View File

@ -455,6 +455,7 @@ makemessages
makemigrations
Mako
Mapnik
Marczewski
Marino
Markus
MBR
@ -545,6 +546,7 @@ Palau
parameterized
params
parens
Paweł
pdf
PEM
perl